Re: [PATCH] LoongArch: Align pci memory base address with page size

[bibo, mao]

在 2023/6/2 12:11, Huacai Chen 写道:
> +cc Bjorn
> 
> Hi, Bibo,
> 
> On Fri, Jun 2, 2023 at 11:07 AM Bibo Mao <maobibo@xxxxxxxxxxx> 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@xxxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxxxxxxxxx
> To unsubscribe send an email to loongson-kernel-leave@xxxxxxxxxxxxxxxxx