Re: [Qemu-devel] [RFC/RFT PATCH 2/5] vfio/pci: Add base BAR MemoryRegion

[Alex Williamson]

On Tue, 19 Dec 2017 14:44:51 +1100
Alexey Kardashevskiy <address@hidden> wrote:

> On 18/12/17 16:02, Alex Williamson wrote:
> > Add one more layer to our stack of MemoryRegions, this base region
> > allows us to register BARs independently of the vfio region or to
> > extend the size of BARs which do map to a region.  This will be
> > useful when we want hypervisor defined BARs or sections of BARs,
> > for purposes such as relocating MSI-X emulation.  We therefore call
> > msix_init() based on this new base MemoryRegion, while the quirks,
> > which only modify regions still operate on those sub-MemoryRegions.  
> 
> 
> Looks ok, one worry though - the default config produces this:
> 
> 
> memory-region: address@hidden
>   0000000000000000-ffffffffffffffff (prio 0, i/o): address@hidden
>     0000210000000000-000021000000ffff (prio 1, i/o): 0001:03:00.0 base BAR 1
>       0000210000000000-000021000000ffff (prio 0, i/o): 0001:03:00.0 BAR 1
>       000021000000e000-000021000000e5ff (prio 0, i/o): msix-table
>       000021000000f000-000021000000f00f (prio 0, i/o): msix-pba [disabled]
>     0000210000040000-000021000007ffff (prio 1, i/o): 0001:03:00.0 base BAR 3
>       0000210000040000-000021000007ffff (prio 0, i/o): 0001:03:00.0 BAR 3
>         0000210000040000-000021000007ffff (prio 0, ramd): 0001:03:00.0 BAR
> 3 mmaps[0]
> 
> 
> Where "BAR 1" and "msix-table" overlap. It resolves correctly:
> 
> 
> FlatView #1
>  AS "memory", root: system
>  AS "cpu-memory", root: system
>  Root memory region: system
>   0000000000000000-000000007fffffff (prio 0, ram): ppc_spapr.ram
>   0000210000000000-000021000000dfff (prio 0, i/o): 0001:03:00.0 BAR 1
>   000021000000e000-000021000000e5ff (prio 0, i/o): msix-table
>   000021000000e600-000021000000ffff (prio 0, i/o): 0001:03:00.0 BAR 1
> @000000000000e600
>   0000210000040000-000021000007ffff (prio 0, ramd): 0001:03:00.0 BAR 3 
> mmaps[0]
> 
> 
> but this looks like an accident - should not we raise the msix-table
> priority or lower the BAR1 priority (to -1)?

I was worried about this too, but the way I read the code is that the
last registered subregion with the same priority wins.  Therefore so
long as msix_init() is called after we layer anything else onto the base
BAR MemoryRegion that might interfere with it, everything should work
correctly.  It certainly might make sense for msix_init() to add
subregions with a higher priority, but then you immediately get into
the road block of what priority should it use, which is a bit of a rat
hole since we can make it work as-is with proper ordering.  Thanks,

Alex