On 9/17/2021 2:26 PM, Shameerali Kolothum Thodi wrote:
>
>
>> -----Original Message-----
>> From: Jon Nettleton [mailto:[email protected]]
>> Sent: 16 September 2021 12:17
>> To: Shameerali Kolothum Thodi <[email protected]>
>> Cc: Robin Murphy <[email protected]>; Lorenzo Pieralisi
>> <[email protected]>; Laurentiu Tudor <[email protected]>;
>> linux-arm-kernel <[email protected]>; ACPI Devel Maling
>> List <[email protected]>; Linux IOMMU
>> <[email protected]>; Joerg Roedel <[email protected]>; Will
>> Deacon <[email protected]>; wanghuiqiang <[email protected]>;
>> Guohanjun (Hanjun Guo) <[email protected]>; Steven Price
>> <[email protected]>; Sami Mujawar <[email protected]>; Eric
>> Auger <[email protected]>; yangyicong <[email protected]>
>> Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node parsing
>>
>> On Thu, Sep 16, 2021 at 10:26 AM Shameerali Kolothum Thodi
>> <[email protected]> wrote:
>>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: Jon Nettleton [mailto:[email protected]]
>>>> Sent: 16 September 2021 08:52
>>>> To: Shameerali Kolothum Thodi
>> <[email protected]>
>>>> Cc: Robin Murphy <[email protected]>; Lorenzo Pieralisi
>>>> <[email protected]>; Laurentiu Tudor
>>>> <[email protected]>; linux-arm-kernel
>>>> <[email protected]>; ACPI Devel Maling List
>>>> <[email protected]>; Linux IOMMU
>>>> <[email protected]>; Joerg Roedel <[email protected]>;
>>>> Will Deacon <[email protected]>; wanghuiqiang
>>>> <[email protected]>; Guohanjun (Hanjun Guo)
>>>> <[email protected]>; Steven Price <[email protected]>; Sami
>>>> Mujawar <[email protected]>; Eric Auger
>> <[email protected]>;
>>>> yangyicong <[email protected]>
>>>> Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
>>>> parsing
>>>>
>>>> On Thu, Sep 16, 2021 at 9:26 AM Shameerali Kolothum Thodi
>>>> <[email protected]> wrote:
>>>>>
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: Jon Nettleton [mailto:[email protected]]
>>>>>> Sent: 06 September 2021 20:51
>>>>>> To: Robin Murphy <[email protected]>
>>>>>> Cc: Lorenzo Pieralisi <[email protected]>; Shameerali
>>>>>> Kolothum Thodi <[email protected]>; Laurentiu
>>>>>> Tudor <[email protected]>; linux-arm-kernel
>>>>>> <[email protected]>; ACPI Devel Maling List
>>>>>> <[email protected]>; Linux IOMMU
>>>>>> <[email protected]>; Linuxarm
>>>>>> <[email protected]>; Joerg Roedel <[email protected]>; Will
>>>>>> Deacon <[email protected]>; wanghuiqiang
>>>>>> <[email protected]>; Guohanjun (Hanjun Guo)
>>>>>> <[email protected]>; Steven Price <[email protected]>;
>>>>>> Sami Mujawar <[email protected]>; Eric Auger
>>>> <[email protected]>;
>>>>>> yangyicong <[email protected]>
>>>>>> Subject: Re: [PATCH v7 2/9] ACPI/IORT: Add support for RMR node
>>>>>> parsing
>>>>>>
>>>>> [...]
>>>>>
>>>>>>>>
>>>>>>>> On the prot value assignment based on the remapping flag,
>>>>>>>> I'd like to hear Robin/Joerg's opinion, I'd avoid being in a
>>>>>>>> situation where "normally" this would work but then we have
>>>>>>>> to quirk
>>>> it.
>>>>>>>>
>>>>>>>> Is this a valid assumption _always_ ?
>>>>>>>
>>>>>>> No. Certainly applying IOMMU_CACHE without reference to the
>>>>>>> device's _CCA attribute or how CPUs may be accessing a shared
>>>>>>> buffer could lead to a loss of coherency. At worst, applying
>>>>>>> IOMMU_MMIO to a device-private buffer *could* cause the device
>>>>>>> to lose coherency with itself if the memory underlying the RMR
>>>>>>> may have allocated into system caches. Note that the expected
>>>>>>> use for non-remappable RMRs is the device holding some sort of
>>>>>>> long-lived private data in system RAM - the MSI doorbell trick
>>>>>>> is far more of a niche
>>>> hack really.
>>>>>>>
>>>>>>> At the very least I think we need to refer to the device's
>>>>>>> memory access properties here.
>>>>>>>
>>>>>>> Jon, Laurentiu - how do RMRs correspond to the EFI memory map
>>>>>>> on your firmware? I'm starting to think that as long as the
>>>>>>> underlying memory is described appropriately there then we
>>>>>>> should be able to infer correct attributes from the EFI memory type
>> and flags.
>>>>>>
>>>>>> The devices are all cache coherent and marked as _CCA, 1. The
>>>>>> Memory regions are in the virt table as
>>>> ARM_MEMORY_REGION_ATTRIBUTE_DEVICE.
>>>>>>
>>>>>> The current chicken and egg problem we have is that during the
>>>>>> fsl-mc-bus initialization we call
>>>>>>
>>>>>> error = acpi_dma_configure_id(&pdev->dev, DEV_DMA_COHERENT,
>>>>>> &mc_stream_id);
>>>>>>
>>>>>> which gets deferred because the SMMU has not been initialized yet.
>>>>>> Then we initialize the RMR tables but there is no device
>>>>>> reference there to be able to query device properties, only the stream
>> id.
>>>>>> After the IORT tables are parsed and the SMMU is setup, on the
>>>>>> second device probe we associate everything based on the stream
>>>>>> id and the fsl-mc-bus device is able to claim its 1-1 DMA mappings.
>>>>>
>>>>> Can we solve this order problem by delaying the
>>>>> iommu_alloc_resv_region() to the
>>>>> iommu_dma_get_rmr_resv_regions(dev,
>>>>> list) ? We could invoke
>>>>> device_get_dma_attr() from there which I believe will return the
>>>>> _CCA
>>>> attribute.
>>>>>
>>>>> Or is that still early to invoke that?
>>>>
>>>> That looks like it should work. Do we then also need to parse
>>>> through the VirtualMemoryTable matching the start and end addresses
>>>> to determine the other memory attributes like MMIO?
>>>
>>> Yes. But that looks tricky as I can't find that readily available on
>>> Arm, like the efi_mem_attributes(). I will take a look.
>>>
>>> Please let me know if there is one or any other easy way to retrieve it.
>>
>> maybe we don't need to. Maybe it is enough to just move
>> iommu_alloc_resv_regions and then set the IOMMU_CACHE flag if type =
>> IOMMU_RESV_DIRECT_RELAXABLE and _CCN=1?
>
> It looks like we could simply call efi_mem_type() and check for
> EFI_MEMORY_MAPPED_IO. I have updated the code to set the
> RMR prot value based on _CCA and EFI md type. Please see the
> last commit on this branch here(not tested),
>
> https://github.com/hisilicon/kernel-dev/commits/private-v5.14-rc4-rmr-v7-ext
>
> Please take a look and let me know if this is good enough to solve this
> problem.
>
Sorry for the delay, I managed to test on a NXP LX2160A and things look
fine, so:
Tested-by: Laurentiu Tudor <[email protected]>
---
Best Regards, Laurentiu
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
