Re: [PATCH v4 2/2] xen/pci: replace call to is_memory_hole to pci_check_bar

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 7 Sep 2022, Julien Grall wrote:
> Hi Jan,
> 
> On 07/09/2022 13:06, Jan Beulich wrote:
> > On 07.09.2022 13:09, Rahul Singh wrote:
> > > is_memory_hole was implemented for x86 and not for ARM when introduced.
> > > Replace is_memory_hole call to pci_check_bar as function should check
> > > if device BAR is in defined memory range. Also, add an implementation
> > > for ARM which is required for PCI passthrough.
> > > 
> > > On x86, pci_check_bar will call is_memory_hole which will check if BAR
> > > is not overlapping with any memory region defined in the memory map.
> > > 
> > > On ARM, pci_check_bar will go through the host bridge ranges and check
> > > if the BAR is in the range of defined ranges.
> > > 
> > > Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
> > > ---
> > > Changes in v4:
> > >   - check "s <= e" before callback
> > >   - Add TODO comment for revisiting the function pci_check_bar() when
> > >     ACPI PCI passthrough support is added.
> > >   - Not Added the Jan Acked-by as patch is modified.
> > 
> > Hmm, I don't see any change to the parts the ack covers (x86 and common),
> > so please re-instate. I'm not in the position to ack Arm changes; things
> > would be different for a Reviewed-by without scope restriction.
> 
> I would like the question about the BAR alignment to be resolved before giving
> my Reviewed-by/Acked-by here.
> 
> Hopefully Stefano can clarify it.

We had a discussion this morning and this is the summary.

BAR addresses and Root Complex window addresses are not guaranteed to be
page aligned.

If they are not guaranteed to be page aligned we should check for
alignment, however, this is not the right location for the alignment
check. It is a good idea to add a TODO comment in the code.

So that we don't relax the check in case the addresses are not aligned,
we should drop the use of PFN_UP and PFN_DOWN in this check.