On Mon, Jun 5, 2023 at 2:31 PM bibo, mao <maobibo@xxxxxxxxxxx> wrote: > > > > 在 2023/6/5 11:58, Huacai Chen 写道: > > Hi, Bibo, > > > > Three suggestions: > > 1, split to two patches. > will do. > > 2, the "(align < PAGE_SIZE)" condition can be removed. > With "(align < PAGE_SIZE)" condition, there is little modification compared > to the weak function. > resource_size_t __weak pcibios_align_resource(void *data, > const struct resource *res, > resource_size_t size, > resource_size_t align) > { > return res->start; > } Why? I think "align > PAGE_SIZE" doesn't ensure res->start is aligned here. > > or do you mean something this? > if (res->flags & IORESOURCE_MEM) { > align = max_t(resource_size_t, PAGE_SIZE, align); > start = ALIGN(start, align); > } > No, I mean use "start = ALIGN(start, PAGE_SIZE)" unconditionally. Huacai > > > 3, you can unify the comments between ARM64 and LoongArch. > will do. > > Regards > Bibo, Mao > > > > > Huacai > > > > On Mon, Jun 5, 2023 at 11:44 AM Bibo Mao <maobibo@xxxxxxxxxxx> 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@xxxxxxxxxxx> > >> --- > >> 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 > >> > > _______________________________________________ > > Loongson-kernel mailing list -- loongson-kernel@xxxxxxxxxxxxxxxxx > > To unsubscribe send an email to loongson-kernel-leave@xxxxxxxxxxxxxxxxx >