在 2023/6/2 16:00, Huacai Chen 写道:
> On Fri, Jun 2, 2023 at 3:35 PM bibo, mao <maobibo@xxxxxxxxxxx> wrote:
>>
>>
>>
>> 在 2023/6/2 14:55, Huacai Chen 写道:
>>> On Fri, Jun 2, 2023 at 2:48 PM bibo, mao <maobibo@xxxxxxxxxxx> wrote:
>>>>
>>>>
>>>>
>>>> 在 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.
>>> 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@xxxxxxxxxxxxxxxxx
>>>>> To unsubscribe send an email to loongson-kernel-leave@xxxxxxxxxxxxxxxxx
>>>>
>>>>
>>> _______________________________________________
>>> Loongson-kernel mailing list -- loongson-kernel@xxxxxxxxxxxxxxxxx
>>> To unsubscribe send an email to loongson-kernel-leave@xxxxxxxxxxxxxxxxx
>>
>>
