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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi, Bjorn,

What's your opinion? This patch indeed fixes a problem on LoongArch,
and in theory other non-4K page platforms also need it. If someone
disagrees, maybe we can use the old way: provide a LoongArch-specific
pcibios_align_resource() at this time.

Huacai

On Fri, Jun 9, 2023 at 12:21 PM Huacai Chen <chenhuacai@xxxxxxxxxx> wrote:
>
> On Fri, Jun 9, 2023 at 11:47 AM bibo, mao <maobibo@xxxxxxxxxxx> wrote:
> >
> >
> >
> > 在 2023/6/9 10:29, Huacai Chen 写道:
> > > Hi, Bibo,
> > >
> > > On Fri, Jun 9, 2023 at 10:20 AM Bibo Mao <maobibo@xxxxxxxxxxx> wrote:
> > >>
> > >> Some PCI devices have 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 passed-through to vm.
> > >>
> > >> It consumes more pci memory resource with page size alignment requirement,
> > >> here extra option PCI_MEMRES_PAGE_ALIGN is added, it can be enabled by
> > >> different architectures.
> > >>
> > >> Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx>
> > >> ---
> > >> Change history
> > >> v4: add extra kernel option PCI_MEMRES_PAGE_ALIGN to set memory resource
> > >>     page aligned.
> > >>
> > >> v3: move alignment requirement to generic pci code
> > >>
> > >> v2: add pci resource alignment requirement in arch specified function
> > >>     pcibios_align_resource on arm64/LoongArch platforms
> > >>
> > >> ---
> > >>  arch/loongarch/Kconfig  | 1 +
> > >>  drivers/pci/Kconfig     | 3 +++
> > >>  drivers/pci/setup-res.c | 7 +++++++
> > >>  3 files changed, 11 insertions(+)
> > >>
> > >> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
> > >> index d38b066fc931..65b2f6ba9f8e 100644
> > >> --- a/arch/loongarch/Kconfig
> > >> +++ b/arch/loongarch/Kconfig
> > >> @@ -142,6 +142,7 @@ config LOONGARCH
> > >>         select PCI_LOONGSON
> > >>         select PCI_MSI_ARCH_FALLBACKS
> > >>         select PCI_QUIRKS
> > >> +       select PCI_MEMRES_PAGE_ALIGN
> > >>         select PERF_USE_VMALLOC
> > >>         select RTC_LIB
> > >>         select SMP
> > >> diff --git a/drivers/pci/Kconfig b/drivers/pci/Kconfig
> > >> index 9309f2469b41..9be5f85ff9dc 100644
> > >> --- a/drivers/pci/Kconfig
> > >> +++ b/drivers/pci/Kconfig
> > >> @@ -128,6 +128,9 @@ config PCI_LOCKLESS_CONFIG
> > >>  config PCI_BRIDGE_EMUL
> > >>         bool
> > >>
> > >> +config PCI_MEMRES_PAGE_ALIGN
> > >> +       bool
> > >> +
> > >>  config PCI_IOV
> > >>         bool "PCI IOV support"
> > >>         select PCI_ATS
> > >> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > >> index 967f9a758923..6ad76734a670 100644
> > >> --- a/drivers/pci/setup-res.c
> > >> +++ b/drivers/pci/setup-res.c
> > >> @@ -339,6 +339,13 @@ int pci_assign_resource(struct pci_dev *dev, int resno)
> > >>                 return -EINVAL;
> > >>         }
> > >>
> > >> +#ifdef CONFIG_PCI_MEMRES_PAGE_ALIGN
> > >> +       /*
> > >> +        * force minimum page alignment for vfio pci usage
> > >> +        */
> > >> +       if (res->flags & IORESOURCE_MEM)
> > >> +               align = max_t(resource_size_t, PAGE_SIZE, align);
> > >> +#endif
> > > Does this really have its effect? The common version of
> > > pcibios_align_resource() simply returns res->start, and doesn't care
> > > about the 'align' parameter.
> > yes, it works. The is output of command " lspci -vvv | grep Region" on my
> > 3C5000+7A2000 box. After the patch base address of all pci mem resource
> > is aligned with 16K.
> >
> > output without the patch:
> >         Region 0: Memory at e0045240000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045248000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045250000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045258000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045260000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045271400 (64-bit, non-prefetchable) [size=256]
> >         Region 2: Memory at e0040000000 (64-bit, non-prefetchable) [size=64M]
> >         Region 4: Memory at e0045200000 (64-bit, non-prefetchable) [size=64K]
> >         Region 0: Memory at e0045210000 (64-bit, non-prefetchable) [size=64K]
> >         Region 0: Memory at e0045220000 (64-bit, non-prefetchable) [size=64K]
> >         Region 0: Memory at e0045230000 (64-bit, non-prefetchable) [size=64K]
> >         Region 5: Memory at e0045271000 (32-bit, non-prefetchable) [size=1K]
> >         Region 0: Memory at e0045268000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045269000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526a000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526b000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526c000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526d000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526e000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526f000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045270000 (64-bit, non-prefetchable) [size=4K]
> >         Region 2: Memory at e0044000000 (64-bit, non-prefetchable) [size=16M]
> >         Region 0: Memory at e0045100000 (64-bit, non-prefetchable) [size=1M]
> >         Region 0: Memory at e0045000000 (64-bit, non-prefetchable) [size=16K]
> >
> > out put with the patch:
> >         Region 0: Memory at e0045240000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045248000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045250000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045258000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045260000 (64-bit, non-prefetchable) [size=32K]
> >         Region 0: Memory at e0045290000 (64-bit, non-prefetchable) [size=256]
> >         Region 2: Memory at e0040000000 (64-bit, non-prefetchable) [size=64M]
> >         Region 4: Memory at e0045200000 (64-bit, non-prefetchable) [size=64K]
> >         Region 0: Memory at e0045210000 (64-bit, non-prefetchable) [size=64K]
> >         Region 0: Memory at e0045220000 (64-bit, non-prefetchable) [size=64K]
> >         Region 0: Memory at e0045230000 (64-bit, non-prefetchable) [size=64K]
> >         Region 5: Memory at e004528c000 (32-bit, non-prefetchable) [size=1K]
> >         Region 0: Memory at e0045268000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004526c000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045270000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045274000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045278000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e004527c000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045280000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045284000 (64-bit, non-prefetchable) [size=4K]
> >         Region 0: Memory at e0045288000 (64-bit, non-prefetchable) [size=4K]
> >         Region 2: Memory at e0044000000 (64-bit, non-prefetchable) [size=16M]
> >         Region 0: Memory at e0045100000 (64-bit, non-prefetchable) [size=1M]
> >         Region 0: Memory at e0045000000 (64-bit, non-prefetchable) [size=16K]
> OK, thanks.
>
> Reviewed-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
>
> >
> > Regards
> > Bibo, Mao
> > >
> > > Huacai
> > >>         size = resource_size(res);
> > >>         ret = _pci_assign_resource(dev, resno, size, align);
> > >>
> > >> --
> > >> 2.27.0
> > >>
> >




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux