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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAhV-H6O+_EnmDU6kE-19PP+8tD3wwbzzuakE_vHZGCtjZWDbg@mail.gmail.com>
Date:   Mon, 5 Jun 2023 11:58:10 +0800
From:   Huacai Chen <chenhuacai@...nel.org>
To:     Bibo Mao <maobibo@...ngson.cn>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, helgaas@...nel.org,
        linux-pci@...r.kernel.org, Jianmin Lv <lvjianmin@...ngson.cn>,
        WANG Xuerui <kernel@...0n.name>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        loongarch@...ts.linux.dev, loongson-kernel@...ts.loongnix.cn
Subject: Re: [PATCH v2] PCI: Align pci memory space base address with page size

Hi, Bibo,

Three suggestions:
1, split to two patches.
2, the "(align < PAGE_SIZE)" condition can be removed.
3, you can unify the comments between ARM64 and LoongArch.

Huacai

On Mon, Jun 5, 2023 at 11:44 AM Bibo Mao <maobibo@...ngson.cn> 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@...ngson.cn>
> ---
>  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
> diff --git a/arch/loongarch/pci/pci.c b/arch/loongarch/pci/pci.c
> index 2726639150bc..943a48e60fb1 100644
> --- a/arch/loongarch/pci/pci.c
> +++ b/arch/loongarch/pci/pci.c
> @@ -83,6 +83,24 @@ 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.
> + */
> +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) && (align < PAGE_SIZE))
> +               start = ALIGN(start, PAGE_SIZE);
> +
> +       return start;
> +}
> +
>  static void pci_fixup_vgadev(struct pci_dev *pdev)
>  {
>         struct pci_dev *devp = NULL;
> --
> 2.27.0
>