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;
Message-ID: <749898e2-01c2-9f74-f939-fac36227431a@loongson.cn>
Date:   Mon, 5 Jun 2023 14:31:52 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH v2] PCI: Align pci memory space base address with page
 size
Content-Language: en-US
To:     Huacai Chen <chenhuacai@kernel.org>
Cc:     Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>, helgaas@kernel.org,
        linux-pci@vger.kernel.org, Jianmin Lv <lvjianmin@loongson.cn>,
        WANG Xuerui <kernel@xen0n.name>,
        linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
        loongarch@lists.linux.dev
References: <20230605034423.1528206-1-maobibo@loongson.cn>
 <CAAhV-H6O+_EnmDU6kE-19PP+8tD3wwbzzuakE_vHZGCtjZWDbg@mail.gmail.com>
From:   "bibo, mao" <maobibo@loongson.cn>
In-Reply-To: <CAAhV-H6O+_EnmDU6kE-19PP+8tD3wwbzzuakE_vHZGCtjZWDbg@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk


在 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;
}

or do you mean something this?
       if (res->flags & IORESOURCE_MEM) {
		align = max_t(resource_size_t, PAGE_SIZE, align);
		start = ALIGN(start, align);
	}
   

> 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@loongson.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@loongson.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
>>
> _______________________________________________
> Loongson-kernel mailing list -- loongson-kernel@lists.loongnix.cn
> To unsubscribe send an email to loongson-kernel-leave@lists.loongnix.cn