| 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
| ||
|
Message-ID: <CAAhV-H6Sp7k5RnSOE53BWXjm7d9Y2km0Z2xSCSxQFZkRm29d1w@mail.gmail.com>
Date: Sun, 4 Jun 2023 20:33:20 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: "bibo, mao" <maobibo@...ngson.cn>
Cc: loongson-kernel@...ts.loongnix.cn,
Bjorn Helgaas <helgaas@...nel.org>,
WANG Xuerui <kernel@...0n.name>,
Jianmin Lv <lvjianmin@...ngson.cn>, loongarch@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] LoongArch: Align pci memory base address with page size
On Fri, Jun 2, 2023 at 5:40 PM bibo, mao <maobibo@...ngson.cn> wrote:
>
>
>
> 在 2023/6/2 16:00, Huacai Chen 写道:
> > On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@...ngson.cn> wrote:
> >>
> >>
> >>
> >> 在 2023/6/2 14:55, Huacai Chen 写道:
> >>> On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@...ngson.cn> wrote:
> >>>>
> >>>>
> >>>>
> >>>> 在 2023/6/2 12:11, Huacai Chen 写道:
> >>>>> +cc Bjorn
> >>>>>
> >>>>> Hi, Bibo,
> >>>>>
> >>>>> On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@...ngson.cn> 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@...ngson.cn>
> >>>>>> ---
> >>>>>> 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.
> >>>> In general pci device is aligned by size, and its value is a power of 2 in value.
> >>>> I do not see such devices with 18K alignment requirements.
> >>> If so, you can simply ignore "align" and use start = ALIGN(start, PAGE_SIZE);
> >>>
> >>>>
> >>>> By pci local bus spec, there are such lines:
> >>>>
> >>>> "Devices are free to consume more address space than required, but decoding down
> >>>> to a 4 KB space for memory is suggested for devices that need less than that amount. For
> >>>> instance, a device that has 64 bytes of registers to be mapped into Memory Space may
> >>>> consume up to 4 KB of address space in order to minimize the number of bits in the address
> >>>> decoder."
> >>>>
> >>>> I cannot think whether it is necessary simply from judging whether other
> >>>> architectures have similar code. If so, LoongArch system just always follows others.
> >>>> It is actually one problem since LoongArch uses 16K page size.
> >>> As I know, both MIPS and ARM64 can use non-4K pages, but when I grep
> >>> pcibios_align_resource in the arch directory, none of them do
> >>> PAGE_SIZE alignment.
> >> Here is piece of code in drivers/vfio/pci/vfio_pci_core.c
> >> /*
> >> * Here we don't handle the case when the BAR is not page
> >> * aligned because we can't expect the BAR will be
> >> * assigned into the same location in a page in guest
> >> * when we passthrough the BAR. And it's hard to access
> >> * this BAR in userspace because we have no way to get
> >> * the BAR's location in a page.
> >> */
> >> no_mmap:
> >> vdev->bar_mmap_supported[bar] = false;
> >>
> >> Do you think it is a issue or not?
> > May be or may not be, if it should be aligned to PAGE_SIZE, then MIPS
> > and ARM64 also need this.
> >
> >>
> >> You can search function pnv_pci_default_alignment or pcibios_align_resource about
> >> alpha architecture.
> > Alpha's pcibios_align_resource() have nothing to do with PAGE_SIZE,
> > pnv_pci_default_alignment() seems to be the case. But if alignment is
> > really needed, I think it is better to provide a
> > pcibios_default_alignment() as powerpc.
> I will double check which is better. Just be curious, how do you think it is a problem
> or not, just checking whether other arches have similar code??
You are probably right but I'm not sure, maybe you can submit a patch
for ARM64 to get more people involved.
Huacai
>
>
> >
> > Huacai
> >>
> >> Regards
> >> Bibo, mao
> >>
> >>>
> >>> Huacai
> >>>
> >>>>
> >>>> Regards
> >>>> Bibo, Mao
> >>>>>
> >>>>> Huacai
> >>>>>> + }
> >>>>>> + }
> >>>>>> + 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@...ts.loongnix.cn
> >>>>> To unsubscribe send an email to loongson-kernel-leave@...ts.loongnix.cn
> >>>>
> >>>>
> >>> _______________________________________________
> >>> Loongson-kernel mailing list -- loongson-kernel@...ts.loongnix.cn
> >>> To unsubscribe send an email to loongson-kernel-leave@...ts.loongnix.cn
> >>
> >>
>
>
Powered by blists - more mailing lists