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

Message ID 20230609022047.2195689-1-maobibo@loongson.cn
State New
Headers
Series [v4] PCI: Align pci memory space base address with page size |

Commit Message

maobibo June 9, 2023, 2:20 a.m. UTC
  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@loongson.cn>
---
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(+)
  

Comments

Huacai Chen June 9, 2023, 2:29 a.m. UTC | #1
Hi, Bibo,

On Fri, Jun 9, 2023 at 10:20 AM Bibo Mao <maobibo@loongson.cn> 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@loongson.cn>
> ---
> 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.

Huacai
>         size = resource_size(res);
>         ret = _pci_assign_resource(dev, resno, size, align);
>
> --
> 2.27.0
>
  
maobibo June 9, 2023, 3:47 a.m. UTC | #2
在 2023/6/9 10:29, Huacai Chen 写道:
> Hi, Bibo,
> 
> On Fri, Jun 9, 2023 at 10:20 AM Bibo Mao <maobibo@loongson.cn> 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@loongson.cn>
>> ---
>> 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]

Regards
Bibo, Mao
> 
> Huacai
>>         size = resource_size(res);
>>         ret = _pci_assign_resource(dev, resno, size, align);
>>
>> --
>> 2.27.0
>>
  
Huacai Chen June 9, 2023, 4:21 a.m. UTC | #3
On Fri, Jun 9, 2023 at 11:47 AM bibo, mao <maobibo@loongson.cn> wrote:
>
>
>
> 在 2023/6/9 10:29, Huacai Chen 写道:
> > Hi, Bibo,
> >
> > On Fri, Jun 9, 2023 at 10:20 AM Bibo Mao <maobibo@loongson.cn> 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@loongson.cn>
> >> ---
> >> 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@loongson.cn>

>
> Regards
> Bibo, Mao
> >
> > Huacai
> >>         size = resource_size(res);
> >>         ret = _pci_assign_resource(dev, resno, size, align);
> >>
> >> --
> >> 2.27.0
> >>
>
  
Huacai Chen June 15, 2023, 6:45 a.m. UTC | #4
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@kernel.org> wrote:
>
> On Fri, Jun 9, 2023 at 11:47 AM bibo, mao <maobibo@loongson.cn> wrote:
> >
> >
> >
> > 在 2023/6/9 10:29, Huacai Chen 写道:
> > > Hi, Bibo,
> > >
> > > On Fri, Jun 9, 2023 at 10:20 AM Bibo Mao <maobibo@loongson.cn> 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@loongson.cn>
> > >> ---
> > >> 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@loongson.cn>
>
> >
> > Regards
> > Bibo, Mao
> > >
> > > Huacai
> > >>         size = resource_size(res);
> > >>         ret = _pci_assign_resource(dev, resno, size, align);
> > >>
> > >> --
> > >> 2.27.0
> > >>
> >
  
Will Deacon June 16, 2023, 9:31 a.m. UTC | #5
On Fri, Jun 09, 2023 at 10:20:47AM +0800, Bibo Mao 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.

Shouldn't we also select this new Kconfig option on arm64 then?

Will
  
bibo, mao June 16, 2023, 10:59 a.m. UTC | #6
在 2023/6/16 17:31, Will Deacon 写道:
> On Fri, Jun 09, 2023 at 10:20:47AM +0800, Bibo Mao 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.
> 
> Shouldn't we also select this new Kconfig option on arm64 then?
OK, will add this option on arm64 also in next version.

Regards
Bibo, Mao

> 
> Will
  

Patch

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
 	size = resource_size(res);
 	ret = _pci_assign_resource(dev, resno, size, align);