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

From: Huacai Chen
Date: Fri Jun 02 2023 - 00:11:29 EST


+cc Bjorn

Hi, Bibo,

On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@xxxxxxxxxxx> 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@xxxxxxxxxxx>
> ---
> 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
>