Message ID | 20230602030732.1047696-1-maobibo@loongson.cn |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp768206vqr; Thu, 1 Jun 2023 20:27:41 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5SxaCIJQZ8xWkgv4qgumftq05GgCEnikqZBV3ga+cBB5dABpI0HS8/ouRsXtJ3PL6ZA1IF X-Received: by 2002:a05:6a00:1891:b0:63b:859f:f094 with SMTP id x17-20020a056a00189100b0063b859ff094mr10394565pfh.20.1685676461583; Thu, 01 Jun 2023 20:27:41 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1685676461; cv=none; d=google.com; s=arc-20160816; b=LWGEFRX9pyE2VNEJANtBYFRKteiHNlI3j4Pid/XmHKxLA9VJcrex+C6NygA2IYLfvG bAlZrxKLJjP4fPe/j1eY5bUWihv4hq/546Qn5V6J1F8QumkrVdUdYbWb2ZEirUILdHrc IkYc1xyXtJUQdEj8V/Sdc7LW9l+COqdvPCmKFxo/D25famUUsvWm6++X+xyJ37Tvx5H1 UoVWMHRZgmuahKpx1etOh5FB+vnRGXLKY/xSYOFYDd+me7k52ElHtjqJdRObUuGgERjX GF2z9xYnDacuYo5VyyadcmULbX+O6tIqHuftVRB5L2a7NFQd285vXYWszKaEKGQb3On2 Rj5Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from; bh=GuEZzZL3m8LF3JuHrckmT8We2i0eI15GrBup7U2VxH0=; b=LgH7Dxtce/zL+Vg22QQ3TGF63quIvMIh4lY6RyPvLaeX97p9vH8HH+OF3QwnzvgHRn 4ZuZfCganfyELLSkdvqFUDDYI4W1lkKpOdWMBX8z+pQddl8quc7ZjqJjoyuIownaX0az 15hrBGxPZsiLAGUu3ffFHuxxSrL3IPvSrvSSxqWSlaKLCUiWmN5oeE5R/a9YGk9Id6nf nvrEf/Sx/i/T/blzSVZ7bjuyJ8pjci8dTVvIfxEhwrr+MN11prua38zDAsIAkJ0ZP9GR kk4CmuxNLLEKtDcYt8jaaQd9s8BFi49xzB9kWjs0i84yJunB1KCuTLnRPBiVpjS5rAm5 42xg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id o5-20020a639205000000b0053075988265si317787pgd.59.2023.06.01.20.27.28; Thu, 01 Jun 2023 20:27:41 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S233218AbjFBDHj (ORCPT <rfc822;limurcpp@gmail.com> + 99 others); Thu, 1 Jun 2023 23:07:39 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55888 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229868AbjFBDHh (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 1 Jun 2023 23:07:37 -0400 Received: from mail.loongson.cn (mail.loongson.cn [114.242.206.163]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 2E21018C for <linux-kernel@vger.kernel.org>; Thu, 1 Jun 2023 20:07:34 -0700 (PDT) Received: from loongson.cn (unknown [10.2.9.158]) by gateway (Coremail) with SMTP id _____8AxhPD1XHlkvH4DAA--.7574S3; Fri, 02 Jun 2023 11:07:33 +0800 (CST) Received: from kvm-1-158.loongson.cn (unknown [10.2.9.158]) by localhost.localdomain (Coremail) with SMTP id AQAAf8DxfTv0XHlk3jaFAA--.19342S2; Fri, 02 Jun 2023 11:07:32 +0800 (CST) From: Bibo Mao <maobibo@loongson.cn> To: Huacai Chen <chenhuacai@kernel.org>, WANG Xuerui <kernel@xen0n.name>, Jianmin Lv <lvjianmin@loongson.cn> Cc: loongarch@lists.linux.dev, linux-kernel@vger.kernel.org, loongson-kernel@lists.loongnix.cn Subject: [PATCH] LoongArch: Align pci memory base address with page size Date: Fri, 2 Jun 2023 11:07:32 +0800 Message-Id: <20230602030732.1047696-1-maobibo@loongson.cn> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: AQAAf8DxfTv0XHlk3jaFAA--.19342S2 X-CM-SenderInfo: xpdruxter6z05rqj20fqof0/ X-Coremail-Antispam: 1Uk129KBjvJXoW7uw1UWFyrGryDtryUCFW7urg_yoW8Aw1rpF yxAa9xAFs5KrnIqwsxtw1DZFnYga92ka13JrWfAr9xGanxZ34xZrn3AryUZFW8Ar4kGFW0 qF4Yyr4UWFyUAa7anT9S1TB71UUUUUDqnTZGkaVYY2UrUUUUj1kv1TuYvTs0mT0YCTnIWj qI5I8CrVACY4xI64kE6c02F40Ex7xfYxn0WfASr-VFAUDa7-sFnT9fnUUIcSsGvfJTRUUU b7AYFVCjjxCrM7AC8VAFwI0_Jr0_Gr1l1xkIjI8I6I8E6xAIw20EY4v20xvaj40_Wr0E3s 1l1IIY67AEw4v_Jrv_JF1l8cAvFVAK0II2c7xJM28CjxkF64kEwVA0rcxSw2x7M28EF7xv wVC0I7IYx2IY67AKxVW5JVW7JwA2z4x0Y4vE2Ix0cI8IcVCY1x0267AKxVW8JVWxJwA2z4 x0Y4vEx4A2jsIE14v26r4UJVWxJr1l84ACjcxK6I8E87Iv6xkF7I0E14v26r4UJVWxJr1l e2I262IYc4CY6c8Ij28IcVAaY2xG8wAqjxCEc2xF0cIa020Ex4CE44I27wAqx4xG64xvF2 IEw4CE5I8CrVC2j2WlYx0E2Ix0cI8IcVAFwI0_Jrv_JF1lYx0Ex4A2jsIE14v26r1j6r4U McvjeVCFs4IE7xkEbVWUJVW8JwACjcxG0xvY0x0EwIxGrwCF04k20xvY0x0EwIxGrwCFx2 IqxVCFs4IE7xkEbVWUJVW8JwC20s026c02F40E14v26r1j6r18MI8I3I0E7480Y4vE14v2 6r106r1rMI8E67AF67kF1VAFwI0_JF0_Jw1lIxkGc2Ij64vIr41lIxAIcVC0I7IYx2IY67 AKxVWUJVWUCwCI42IY6xIIjxv20xvEc7CjxVAFwI0_Jr0_Gr1lIxAIcVCF04k26cxKx2IY s7xG6r1j6r1xMIIF0xvEx4A2jsIE14v26r1j6r4UMIIF0xvEx4A2jsIEc7CjxVAFwI0_Jr 0_GrUvcSsGvfC2KfnxnUUI43ZEXa7IU8zwZ7UUUUU== X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE, SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1767559881024846502?= X-GMAIL-MSGID: =?utf-8?q?1767559881024846502?= |
Series |
LoongArch: Align pci memory base address with page size
|
|
Commit Message
maobibo
June 2, 2023, 3:07 a.m. UTC
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@loongson.cn>
---
arch/loongarch/pci/pci.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
Comments
+cc Bjorn Hi, Bibo, On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@loongson.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@loongson.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. Huacai > + } > + } > + return start; > +} > + > static void pci_fixup_vgadev(struct pci_dev *pdev) > { > struct pci_dev *devp = NULL; > -- > 2.27.0 >
在 2023/6/2 12:11, Huacai Chen 写道: > +cc Bjorn > > Hi, Bibo, > > On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@loongson.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@loongson.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. 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. 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@lists.loongnix.cn > To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn
On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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. 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@lists.loongnix.cn > > To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > >
在 2023/6/2 14:55, Huacai Chen 写道: > On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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? You can search function pnv_pci_default_alignment or pcibios_align_resource about alpha architecture. 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@lists.loongnix.cn >>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn >> >> > _______________________________________________ > Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn > To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn
On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@loongson.cn> wrote: > > > > 在 2023/6/2 14:55, Huacai Chen 写道: > > On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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. 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@lists.loongnix.cn > >>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > >> > >> > > _______________________________________________ > > Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn > > To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > >
在 2023/6/2 16:00, Huacai Chen 写道: > On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@loongson.cn> wrote: >> >> >> >> 在 2023/6/2 14:55, Huacai Chen 写道: >>> On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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?? > > 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@lists.loongnix.cn >>>>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn >>>> >>>> >>> _______________________________________________ >>> Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn >>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn >> >>
[+cc linux-pci, beginning of thread: https://lore.kernel.org/all/20230602030732.1047696-1-maobibo@loongson.cn/] On Fri, Jun 02, 2023 at 12:11:02PM +0800, Huacai Chen wrote: > On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@loongson.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@loongson.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. > > Huacai > > + } > > + } > > + return start; > > +} > > + > > static void pci_fixup_vgadev(struct pci_dev *pdev) > > { > > struct pci_dev *devp = NULL; > > -- > > 2.27.0 > >
在 2023/6/2 16:00, Huacai Chen 写道: > On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@loongson.cn> wrote: >> >> >> >> 在 2023/6/2 14:55, Huacai Chen 写道: >>> On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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. pcibios_default_alignment is in effective for all the pci devices and bridges, in function pci_reassigndev_resource_alignment if it is set with alignment limit, it will disable memory access and force to reassign memory resource for the device even if it is already aligned . It will bring some negative effect such as pci VGA device for GOP use. pcibios_align_resource is relative safer and it will reassigned resource for specified bar. I am not familiar with pci, pci experts will give the best answer. Another way, I checkout source code of uefi bios, it seems that memory resource is aligned with fixed 4K size. There is such code: https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumeratorSupport.c if (PciIoDevice->PciBar[BarIndex].Length < (SIZE_4KB)) { // // Force minimum 4KByte alignment for Virtualization technology for Directed I/O // PciIoDevice->PciBar[BarIndex].Alignment = (SIZE_4KB - 1); } else { PciIoDevice->PciBar[BarIndex].Alignment = PciIoDevice->PciBar[BarIndex].Length - 1; } Regards Bibo, Mao > > 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@lists.loongnix.cn >>>>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn >>>> >>>> >>> _______________________________________________ >>> Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn >>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn >> >>
On Fri, Jun 2, 2023 at 5:40 PM bibo, mao <maobibo@loongson.cn> wrote: > > > > 在 2023/6/2 16:00, Huacai Chen 写道: > > On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@loongson.cn> wrote: > >> > >> > >> > >> 在 2023/6/2 14:55, Huacai Chen 写道: > >>> On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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@lists.loongnix.cn > >>>>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > >>>> > >>>> > >>> _______________________________________________ > >>> Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn > >>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > >> > >> > >
> -----原始邮件----- > 发件人: "Huacai Chen" <chenhuacai@kernel.org> > 发送时间:2023-06-04 20:33:20 (星期日) > 收件人: "bibo, mao" <maobibo@loongson.cn> > 抄送: loongson-kernel@lists.loongnix.cn, "Bjorn Helgaas" <helgaas@kernel.org>, "WANG Xuerui" <kernel@xen0n.name>, "Jianmin Lv" <lvjianmin@loongson.cn>, loongarch@lists.linux.dev, linux-kernel@vger.kernel.org > 主题: Re: [PATCH] LoongArch: Align pci memory base address with page size > > On Fri, Jun 2, 2023 at 5:40 PM bibo, mao <maobibo@loongson.cn> wrote: > > > > > > > > 在 2023/6/2 16:00, Huacai Chen 写道: > > > On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@loongson.cn> wrote: > > >> > > >> > > >> > > >> 在 2023/6/2 14:55, Huacai Chen 写道: > > >>> On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@loongson.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@loongson.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@loongson.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. Good suggestion, will do it in next version. It will be better with more people involved. Regards Bibo, Mao > > 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@lists.loongnix.cn > > >>>>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > > >>>> > > >>>> > > >>> _______________________________________________ > > >>> Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn > > >>> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn > > >> > > >> > > > > 本邮件及其附件含有龙芯中科的商业秘密信息,仅限于发送给上面地址中列出的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制或散发)本邮件及其附件中的信息。如果您错收本邮件,请您立即电话或邮件通知发件人并删除本邮件。 This email and its attachments contain confidential information from Loongson Technology , which is intended only for the person or entity whose address is listed above. Any use of the information contained herein in any way (including, but not limited to, total or partial disclosure, reproduction or dissemination) by persons other than the intended recipient(s) is prohibited. If you receive this email in error, please notify the sender by phone or email immediately and delete it.
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); + } + } + return start; +} + static void pci_fixup_vgadev(struct pci_dev *pdev) { struct pci_dev *devp = NULL;