Re: [PATCH v2] PCI: Align pci memory space base address with page size

[Huacai Chen]

On Mon, Jun 5, 2023 at 10:51 PM Will Deacon <will@xxxxxxxxxx> wrote:
>
> On Mon, Jun 05, 2023 at 11:44:23AM +0800, Bibo Mao wrote:
> > Some PCI devices has only 4K memory space size, it is normal in general
> > machines and aligned with page size. However some architectures which
> > support different page size, default page size on LoongArch is 16K, and
> > ARM64 supports page size varying from 4K to 64K. On machines where larger
> > page size is use, memory space region of two different pci devices may be
> > in one page. It is not safe with mmu protection, also VFIO pci device
> > driver requires base address of pci memory space page aligned, so that it
> > can be memory mapped to qemu user space when it is pass-through to vm.
> >
> > It consumes more pci memory resource with page size alignment requirement,
> > on 64 bit system it should not be a problem. And UEFI bios set pci memory
> > base address with 4K fixed-size aligned, the safer solution is to align
> > with larger size on UEFI BIOS stage on these architectures, linux kernel
> > can reuse resource from UEFI bios. For new devices such hotplug pci
> > devices and sriov devices, pci resource is assigned in Linux kernel side.
> >
> > Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> > ---
> >  arch/arm64/kernel/pci.c  | 13 +++++++++++++
> >  arch/loongarch/pci/pci.c | 18 ++++++++++++++++++
> >  2 files changed, 31 insertions(+)
> >
> > diff --git a/arch/arm64/kernel/pci.c b/arch/arm64/kernel/pci.c
> > index 2276689b5411..e2f7b176b156 100644
> > --- a/arch/arm64/kernel/pci.c
> > +++ b/arch/arm64/kernel/pci.c
> > @@ -232,4 +232,17 @@ void pcibios_remove_bus(struct pci_bus *bus)
> >       acpi_pci_remove_bus(bus);
> >  }
> >
> > +resource_size_t pcibios_align_resource(void *data, const struct resource *res,
> > +                             resource_size_t size, resource_size_t align)
> > +{
> > +     resource_size_t start = res->start;
> > +
> > +     /*
> > +      * align base address of pci memory resource with page size
> > +      */
> > +     if ((res->flags & IORESOURCE_MEM) && (align < PAGE_SIZE))
> > +             start = ALIGN(start, PAGE_SIZE);
> > +
> > +     return start;
> > +}
> >  #endif
>
> If this is needed by all architectures with !4k page size, why don't we
> instead handle the alignment in the core code, given that it is aware
> of the page size?
Agree, modifying the common weak version of pcibios_align_resource()
seems better. Maybe we can use CONFIG_VFIO_PCI to guard the align
operation.

Huacai
>
> Will