Re: [PATCH] LoongArch: Align pci memory base address with page size

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc linux-pci, beginning of thread:
https://lore.kernel.org/all/20230602030732.1047696-1-maobibo@loongson.cn/]

On Fri, Jun 02, 2023 at 12:11:02PM +0800, Huacai Chen wrote:
> On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@loongson.cn> wrote:
> >
> > LoongArch linux kernel uses 16K page size by default, some pci devices have
> > only 4K memory size, it is normal in general architectures. However memory
> > space of different pci devices will share one physical page address space.
> > This is not safe for mmu protection, also UIO and VFIO requires base
> > address of pci memory space page aligned.
> >
> > This patch adds check with function pcibios_align_resource, and set base
> > address of resource page aligned.
> >
> > Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> > ---
> >  arch/loongarch/pci/pci.c | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> >
> > diff --git a/arch/loongarch/pci/pci.c b/arch/loongarch/pci/pci.c
> > index 2726639150bc..1380f3672ba2 100644
> > --- a/arch/loongarch/pci/pci.c
> > +++ b/arch/loongarch/pci/pci.c
> > @@ -83,6 +83,29 @@ int pcibios_alloc_irq(struct pci_dev *dev)
> >         return acpi_pci_irq_enable(dev);
> >  }
> >
> > +/*
> > + * memory space size of some pci cards is 4K, it is separated with
> > + * different pages for generic architectures, so that mmu protection can
> > + * work with different pci cards. However page size for LoongArch system
> > + * is 16K, memory space of different pci cards may share the same page
> > + * on LoongArch, it is not safe here.
> > + * Also uio drivers and vfio drivers sugguests that base address of memory
> > + * space should page aligned. This function aligns base address with page size
> > + */
> > +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;
> > +
> > +       if (res->flags & IORESOURCE_MEM) {
> > +               if (align & (PAGE_SIZE - 1)) {
> > +                       align = PAGE_SIZE;
> > +                       start = ALIGN(start, align);
> I don't know whether this patch is really needed, but the logic here
> has some problems.
> 
> For example, if PAGE_SIZE=16KB, align=18KB, what should we do? Align
> to 16KB or align to 32KB? IMO it should align to 32KB, but in your
> patch it will align to 16KB.
> 
> Huacai
> > +               }
> > +       }
> > +       return start;
> > +}
> > +
> >  static void pci_fixup_vgadev(struct pci_dev *pdev)
> >  {
> >         struct pci_dev *devp = NULL;
> > --
> > 2.27.0
> >