| Message ID | 20201119121150.3316-1-shameerali.kolothum.thodi@huawei.com |
|---|---|
| Headers | show |
| Series | ACPI/IORT: Support for IORT RMR node | expand |
On 19/11/2020 12:11, Shameer Kolothum wrote: > RFC v1 --> v2: > - Added a generic interface for IOMMU drivers to retrieve all the > RMR info associated with a given IOMMU. > - SMMUv3 driver gets the RMR list during probe() and installs > bypass STEs for all the SIDs in the RMR list. This is to keep > the ongoing traffic alive(if any) during SMMUv3 reset. This is > based on the suggestions received for v1 to take care of the > EFI framebuffer use case. Only sanity tested for now. Hi Shameer, Sorry for not looking at this before. Do you have any plans to implement support in the SMMUv2 driver? The platform I've been testing the EFI framebuffer support on has the display controller behind SMMUv2, so as it stands this series doesn't work. I did hack something up for SMMUv2 so I was able to test the first 4 patches. > - During the probe/attach device, SMMUv3 driver reserves any > RMR region associated with the device such that there is a unity > mapping for them in SMMU. For the EFI framebuffer use case there is no device to attach so I believe we are left with just the stream ID in bypass mode - which is definitely an improvement (the display works!) but not actually a unity mapping of the RMR range. I'm not sure whether it's worth fixing this or not, but I just wanted to point out there's still a need for a driver for the device before the bypass mode is replaced with the unity mapping. Thanks, Steve
Hi Steve, > -----Original Message----- > From: Steven Price [mailto:steven.price@arm.com] > Sent: 10 December 2020 10:26 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org; devel@acpica.org > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > <guohanjun@huawei.com>; Jonathan Cameron > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > On 19/11/2020 12:11, Shameer Kolothum wrote: > > RFC v1 --> v2: > > - Added a generic interface for IOMMU drivers to retrieve all the > > RMR info associated with a given IOMMU. > > - SMMUv3 driver gets the RMR list during probe() and installs > > bypass STEs for all the SIDs in the RMR list. This is to keep > > the ongoing traffic alive(if any) during SMMUv3 reset. This is > > based on the suggestions received for v1 to take care of the > > EFI framebuffer use case. Only sanity tested for now. > > Hi Shameer, > > Sorry for not looking at this before. > > Do you have any plans to implement support in the SMMUv2 driver? The > platform I've been testing the EFI framebuffer support on has the > display controller behind SMMUv2, so as it stands this series doesn't > work. I did hack something up for SMMUv2 so I was able to test the first > 4 patches. Thanks for taking a look. Sure, I can look into adding the support for SMMUv2. > > > - During the probe/attach device, SMMUv3 driver reserves any > > RMR region associated with the device such that there is a unity > > mapping for them in SMMU. > > For the EFI framebuffer use case there is no device to attach so I > believe we are left with just the stream ID in bypass mode - which is > definitely an improvement (the display works!) Cool. That’s good to know. but not actually a unity > mapping of the RMR range. I'm not sure whether it's worth fixing this or > not, but I just wanted to point out there's still a need for a driver > for the device before the bypass mode is replaced with the unity mapping. I am not sure either. My idea was we will have bypass STE setup for all devices with RMR initially and when the corresponding driver takes over(if that happens) we will have the unity mapping setup properly for the RMR regions. And for cases like the above, it will remain in the bypass mode. Do you see any problem(security?) if the dev streams remain in bypass mode for this dev? Or is it possible to have a stub driver for this dev, so that we will have the probe/attach invoked and everything will fall in place? TBH, I haven't looked into creating a temp domain for these types of the devices and also not sure how we benefit from that compared to the STE bypass mode. Thoughts/Ideas welcome. Thanks, Shameer
On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > Hi Steve, > >> -----Original Message----- >> From: Steven Price [mailto:steven.price@arm.com] >> Sent: 10 December 2020 10:26 >> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; >> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; >> iommu@lists.linux-foundation.org; devel@acpica.org >> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; >> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang >> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) >> <guohanjun@huawei.com>; Jonathan Cameron >> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com >> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node >> >> On 19/11/2020 12:11, Shameer Kolothum wrote: >>> RFC v1 --> v2: >>> - Added a generic interface for IOMMU drivers to retrieve all the >>> RMR info associated with a given IOMMU. >>> - SMMUv3 driver gets the RMR list during probe() and installs >>> bypass STEs for all the SIDs in the RMR list. This is to keep >>> the ongoing traffic alive(if any) during SMMUv3 reset. This is >>> based on the suggestions received for v1 to take care of the >>> EFI framebuffer use case. Only sanity tested for now. >> >> Hi Shameer, >> >> Sorry for not looking at this before. >> >> Do you have any plans to implement support in the SMMUv2 driver? The >> platform I've been testing the EFI framebuffer support on has the >> display controller behind SMMUv2, so as it stands this series doesn't >> work. I did hack something up for SMMUv2 so I was able to test the first >> 4 patches. > > Thanks for taking a look. Sure, I can look into adding the support for SMMUv2. > >> >>> - During the probe/attach device, SMMUv3 driver reserves any >>> RMR region associated with the device such that there is a unity >>> mapping for them in SMMU. >> >> For the EFI framebuffer use case there is no device to attach so I >> believe we are left with just the stream ID in bypass mode - which is >> definitely an improvement (the display works!) > > Cool. That’s good to know. > > but not actually a unity >> mapping of the RMR range. I'm not sure whether it's worth fixing this or >> not, but I just wanted to point out there's still a need for a driver >> for the device before the bypass mode is replaced with the unity mapping. > > I am not sure either. My idea was we will have bypass STE setup for all devices > with RMR initially and when the corresponding driver takes over(if that happens) > we will have the unity mapping setup properly for the RMR regions. And for cases > like the above, it will remain in the bypass mode. > > Do you see any problem(security?) if the dev streams remain in bypass mode for > this dev? Or is it possible to have a stub driver for this dev, so that we will have > the probe/attach invoked and everything will fall in place? The downside is that if a driver never binds to that device, it remains bypassed. If some other externally-controlled malicious device could manage to spoof that device's requester ID, that could potentially be exploited. > TBH, I haven't looked into creating a temp domain for these types of the devices > and also not sure how we benefit from that compared to the STE bypass mode. That said, setting up temporary translation contexts that ensure any access can *only* be to RMR regions until a driver takes over is an awful lot more work. I'm inclined to be pragmatic here and say we should get things working at all with the simple bypass STE/S2CR method, then look at adding the higher-security effort on top. Right now systems that need this are either broken (but effectively secure) or using default bypass with SMMUv2. People who prefer to maintain security over functionality in the interim can maintain that status quo by simply continuing to not describe any RMRs. Robin.
On 14/12/2020 12:33, Robin Murphy wrote: > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: >> Hi Steve, >> >>> -----Original Message----- >>> From: Steven Price [mailto:steven.price@arm.com] >>> Sent: 10 December 2020 10:26 >>> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; >>> iommu@lists.linux-foundation.org; devel@acpica.org >>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) >>> <guohanjun@huawei.com>; Jonathan Cameron >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node >>> >>> On 19/11/2020 12:11, Shameer Kolothum wrote: >>>> RFC v1 --> v2: >>>> - Added a generic interface for IOMMU drivers to retrieve all the >>>> RMR info associated with a given IOMMU. >>>> - SMMUv3 driver gets the RMR list during probe() and installs >>>> bypass STEs for all the SIDs in the RMR list. This is to keep >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This is >>>> based on the suggestions received for v1 to take care of the >>>> EFI framebuffer use case. Only sanity tested for now. >>> >>> Hi Shameer, >>> >>> Sorry for not looking at this before. >>> >>> Do you have any plans to implement support in the SMMUv2 driver? The >>> platform I've been testing the EFI framebuffer support on has the >>> display controller behind SMMUv2, so as it stands this series doesn't >>> work. I did hack something up for SMMUv2 so I was able to test the first >>> 4 patches. >> >> Thanks for taking a look. Sure, I can look into adding the support for >> SMMUv2. Great, thanks! >>> >>>> - During the probe/attach device, SMMUv3 driver reserves any >>>> RMR region associated with the device such that there is a unity >>>> mapping for them in SMMU. >>> >>> For the EFI framebuffer use case there is no device to attach so I >>> believe we are left with just the stream ID in bypass mode - which is >>> definitely an improvement (the display works!) >> >> Cool. That’s good to know. >> >> but not actually a unity >>> mapping of the RMR range. I'm not sure whether it's worth fixing this or >>> not, but I just wanted to point out there's still a need for a driver >>> for the device before the bypass mode is replaced with the unity >>> mapping. >> >> I am not sure either. My idea was we will have bypass STE setup for >> all devices >> with RMR initially and when the corresponding driver takes over(if >> that happens) >> we will have the unity mapping setup properly for the RMR regions. And >> for cases >> like the above, it will remain in the bypass mode. >> >> Do you see any problem(security?) if the dev streams remain in bypass >> mode for >> this dev? Or is it possible to have a stub driver for this dev, so >> that we will have >> the probe/attach invoked and everything will fall in place? > > The downside is that if a driver never binds to that device, it remains > bypassed. If some other externally-controlled malicious device could > manage to spoof that device's requester ID, that could potentially be > exploited. > >> TBH, I haven't looked into creating a temp domain for these types of >> the devices >> and also not sure how we benefit from that compared to the STE bypass >> mode. > > That said, setting up temporary translation contexts that ensure any > access can *only* be to RMR regions until a driver takes over is an > awful lot more work. I'm inclined to be pragmatic here and say we should > get things working at all with the simple bypass STE/S2CR method, then > look at adding the higher-security effort on top. > > Right now systems that need this are either broken (but effectively > secure) or using default bypass with SMMUv2. People who prefer to > maintain security over functionality in the interim can maintain that > status quo by simply continuing to not describe any RMRs. I agree with Robin, let's get this working with the bypass mode (until the device binds) like you've currently got. It's much better than what we have otherwise. Then once that is merged we can look at the temporary translation context or stub driver. The temporary translation context would be 'neatest', but I'm only aware of the EFI framebuffer use case and for that it might be possible to do something simpler - if indeed anything is needed at all. I'm not sure how much we need to be worried about malicious devices spoofing requester IDs. Thanks, Steve
> -----Original Message----- > From: Steven Price [mailto:steven.price@arm.com] > Sent: 14 December 2020 13:43 > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com>; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org; devel@acpica.org > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > On 14/12/2020 12:33, Robin Murphy wrote: > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > >> Hi Steve, > >> > >>> -----Original Message----- > >>> From: Steven Price [mailto:steven.price@arm.com] > >>> Sent: 10 December 2020 10:26 > >>> To: Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com>; > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > >>> iommu@lists.linux-foundation.org; devel@acpica.org > >>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > >>> <guohanjun@huawei.com>; Jonathan Cameron > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > >>> > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > >>>> RFC v1 --> v2: > >>>> - Added a generic interface for IOMMU drivers to retrieve all the > >>>> RMR info associated with a given IOMMU. > >>>> - SMMUv3 driver gets the RMR list during probe() and installs > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This is > >>>> based on the suggestions received for v1 to take care of the > >>>> EFI framebuffer use case. Only sanity tested for now. > >>> > >>> Hi Shameer, > >>> > >>> Sorry for not looking at this before. > >>> > >>> Do you have any plans to implement support in the SMMUv2 driver? The > >>> platform I've been testing the EFI framebuffer support on has the > >>> display controller behind SMMUv2, so as it stands this series doesn't > >>> work. I did hack something up for SMMUv2 so I was able to test the first > >>> 4 patches. > >> > >> Thanks for taking a look. Sure, I can look into adding the support for > >> SMMUv2. > > Great, thanks! > > >>> > >>>> - During the probe/attach device, SMMUv3 driver reserves any > >>>> RMR region associated with the device such that there is a unity > >>>> mapping for them in SMMU. > >>> > >>> For the EFI framebuffer use case there is no device to attach so I > >>> believe we are left with just the stream ID in bypass mode - which is > >>> definitely an improvement (the display works!) > >> > >> Cool. That’s good to know. > >> > >> but not actually a unity > >>> mapping of the RMR range. I'm not sure whether it's worth fixing this or > >>> not, but I just wanted to point out there's still a need for a driver > >>> for the device before the bypass mode is replaced with the unity > >>> mapping. > >> > >> I am not sure either. My idea was we will have bypass STE setup for > >> all devices > >> with RMR initially and when the corresponding driver takes over(if > >> that happens) > >> we will have the unity mapping setup properly for the RMR regions. And > >> for cases > >> like the above, it will remain in the bypass mode. > >> > >> Do you see any problem(security?) if the dev streams remain in bypass > >> mode for > >> this dev? Or is it possible to have a stub driver for this dev, so > >> that we will have > >> the probe/attach invoked and everything will fall in place? > > > > The downside is that if a driver never binds to that device, it remains > > bypassed. If some other externally-controlled malicious device could > > manage to spoof that device's requester ID, that could potentially be > > exploited. > > > >> TBH, I haven't looked into creating a temp domain for these types of > >> the devices > >> and also not sure how we benefit from that compared to the STE bypass > >> mode. > > > > That said, setting up temporary translation contexts that ensure any > > access can *only* be to RMR regions until a driver takes over is an > > awful lot more work. I'm inclined to be pragmatic here and say we should > > get things working at all with the simple bypass STE/S2CR method, then > > look at adding the higher-security effort on top. > > > > Right now systems that need this are either broken (but effectively > > secure) or using default bypass with SMMUv2. People who prefer to > > maintain security over functionality in the interim can maintain that > > status quo by simply continuing to not describe any RMRs. > > I agree with Robin, let's get this working with the bypass mode (until > the device binds) like you've currently got. It's much better than what > we have otherwise. Then once that is merged we can look at the temporary > translation context or stub driver. The temporary translation context > would be 'neatest', but I'm only aware of the EFI framebuffer use case > and for that it might be possible to do something simpler - if indeed > anything is needed at all. I'm not sure how much we need to be worried > about malicious devices spoofing requester IDs. Perfect. I will keep the STE bypass and respin the series once the update to the IORT rev E is made public(hope that will happen soon). In the meantime, appreciate any feedback on the rest of the patches in this series. Thanks, Shameer
On Mon, Dec 14, 2020 at 3:48 PM Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > -----Original Message----- > > From: Steven Price [mailto:steven.price@arm.com] > > Sent: 14 December 2020 13:43 > > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com>; > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > iommu@lists.linux-foundation.org; devel@acpica.org > > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun > > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > On 14/12/2020 12:33, Robin Murphy wrote: > > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > > >> Hi Steve, > > >> > > >>> -----Original Message----- > > >>> From: Steven Price [mailto:steven.price@arm.com] > > >>> Sent: 10 December 2020 10:26 > > >>> To: Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com>; > > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > >>> iommu@lists.linux-foundation.org; devel@acpica.org > > >>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > > >>> <guohanjun@huawei.com>; Jonathan Cameron > > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > >>> > > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > > >>>> RFC v1 --> v2: > > >>>> - Added a generic interface for IOMMU drivers to retrieve all the > > >>>> RMR info associated with a given IOMMU. > > >>>> - SMMUv3 driver gets the RMR list during probe() and installs > > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This is > > >>>> based on the suggestions received for v1 to take care of the > > >>>> EFI framebuffer use case. Only sanity tested for now. > > >>> > > >>> Hi Shameer, > > >>> > > >>> Sorry for not looking at this before. > > >>> > > >>> Do you have any plans to implement support in the SMMUv2 driver? The > > >>> platform I've been testing the EFI framebuffer support on has the > > >>> display controller behind SMMUv2, so as it stands this series doesn't > > >>> work. I did hack something up for SMMUv2 so I was able to test the first > > >>> 4 patches. > > >> > > >> Thanks for taking a look. Sure, I can look into adding the support for > > >> SMMUv2. > > > > Great, thanks! > > > > >>> > > >>>> - During the probe/attach device, SMMUv3 driver reserves any > > >>>> RMR region associated with the device such that there is a unity > > >>>> mapping for them in SMMU. > > >>> > > >>> For the EFI framebuffer use case there is no device to attach so I > > >>> believe we are left with just the stream ID in bypass mode - which is > > >>> definitely an improvement (the display works!) > > >> > > >> Cool. That’s good to know. > > >> > > >> but not actually a unity > > >>> mapping of the RMR range. I'm not sure whether it's worth fixing this or > > >>> not, but I just wanted to point out there's still a need for a driver > > >>> for the device before the bypass mode is replaced with the unity > > >>> mapping. > > >> > > >> I am not sure either. My idea was we will have bypass STE setup for > > >> all devices > > >> with RMR initially and when the corresponding driver takes over(if > > >> that happens) > > >> we will have the unity mapping setup properly for the RMR regions. And > > >> for cases > > >> like the above, it will remain in the bypass mode. > > >> > > >> Do you see any problem(security?) if the dev streams remain in bypass > > >> mode for > > >> this dev? Or is it possible to have a stub driver for this dev, so > > >> that we will have > > >> the probe/attach invoked and everything will fall in place? > > > > > > The downside is that if a driver never binds to that device, it remains > > > bypassed. If some other externally-controlled malicious device could > > > manage to spoof that device's requester ID, that could potentially be > > > exploited. > > > > > >> TBH, I haven't looked into creating a temp domain for these types of > > >> the devices > > >> and also not sure how we benefit from that compared to the STE bypass > > >> mode. > > > > > > That said, setting up temporary translation contexts that ensure any > > > access can *only* be to RMR regions until a driver takes over is an > > > awful lot more work. I'm inclined to be pragmatic here and say we should > > > get things working at all with the simple bypass STE/S2CR method, then > > > look at adding the higher-security effort on top. > > > > > > Right now systems that need this are either broken (but effectively > > > secure) or using default bypass with SMMUv2. People who prefer to > > > maintain security over functionality in the interim can maintain that > > > status quo by simply continuing to not describe any RMRs. > > > > I agree with Robin, let's get this working with the bypass mode (until > > the device binds) like you've currently got. It's much better than what > > we have otherwise. Then once that is merged we can look at the temporary > > translation context or stub driver. The temporary translation context > > would be 'neatest', but I'm only aware of the EFI framebuffer use case > > and for that it might be possible to do something simpler - if indeed > > anything is needed at all. I'm not sure how much we need to be worried > > about malicious devices spoofing requester IDs. > > Perfect. I will keep the STE bypass and respin the series once the update > to the IORT rev E is made public(hope that will happen soon). In the > meantime, appreciate any feedback on the rest of the patches in this series. Shameer, I am pretty sure rev E is already public. You can find it here. https://developer.arm.com/documentation/den0049/latest/ It is also marked non-confidential. I also have initial patches for edk2 and the HoneyComb LX2160a ACPI tables adding RMR nodes that partially work with your patches. This is with basic SMMUv2 support but since you have more experience this this I am more than happy to work with you on your patchset. -Jon > > Thanks, > Shameer > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> -----Original Message----- > From: Jon Nettleton [mailto:jon@solid-run.com] > Sent: 17 December 2020 14:48 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; Guohanjun > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm <linuxarm@huawei.com>; > Jonathan Cameron <jonathan.cameron@huawei.com>; > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > On Mon, Dec 14, 2020 at 3:48 PM Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Steven Price [mailto:steven.price@arm.com] > > > Sent: 14 December 2020 13:43 > > > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum Thodi > > > <shameerali.kolothum.thodi@huawei.com>; > > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > iommu@lists.linux-foundation.org; devel@acpica.org > > > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun > > > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > > > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > > On 14/12/2020 12:33, Robin Murphy wrote: > > > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > > > >> Hi Steve, > > > >> > > > >>> -----Original Message----- > > > >>> From: Steven Price [mailto:steven.price@arm.com] > > > >>> Sent: 10 December 2020 10:26 > > > >>> To: Shameerali Kolothum Thodi > > > <shameerali.kolothum.thodi@huawei.com>; > > > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > >>> iommu@lists.linux-foundation.org; devel@acpica.org > > > >>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > > > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > > > >>> <guohanjun@huawei.com>; Jonathan Cameron > > > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > >>> > > > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > > > >>>> RFC v1 --> v2: > > > >>>> - Added a generic interface for IOMMU drivers to retrieve all the > > > >>>> RMR info associated with a given IOMMU. > > > >>>> - SMMUv3 driver gets the RMR list during probe() and installs > > > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > > > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This is > > > >>>> based on the suggestions received for v1 to take care of the > > > >>>> EFI framebuffer use case. Only sanity tested for now. > > > >>> > > > >>> Hi Shameer, > > > >>> > > > >>> Sorry for not looking at this before. > > > >>> > > > >>> Do you have any plans to implement support in the SMMUv2 driver? > The > > > >>> platform I've been testing the EFI framebuffer support on has the > > > >>> display controller behind SMMUv2, so as it stands this series doesn't > > > >>> work. I did hack something up for SMMUv2 so I was able to test the > first > > > >>> 4 patches. > > > >> > > > >> Thanks for taking a look. Sure, I can look into adding the support for > > > >> SMMUv2. > > > > > > Great, thanks! > > > > > > >>> > > > >>>> - During the probe/attach device, SMMUv3 driver reserves any > > > >>>> RMR region associated with the device such that there is a > unity > > > >>>> mapping for them in SMMU. > > > >>> > > > >>> For the EFI framebuffer use case there is no device to attach so I > > > >>> believe we are left with just the stream ID in bypass mode - which is > > > >>> definitely an improvement (the display works!) > > > >> > > > >> Cool. That’s good to know. > > > >> > > > >> but not actually a unity > > > >>> mapping of the RMR range. I'm not sure whether it's worth fixing this > or > > > >>> not, but I just wanted to point out there's still a need for a driver > > > >>> for the device before the bypass mode is replaced with the unity > > > >>> mapping. > > > >> > > > >> I am not sure either. My idea was we will have bypass STE setup for > > > >> all devices > > > >> with RMR initially and when the corresponding driver takes over(if > > > >> that happens) > > > >> we will have the unity mapping setup properly for the RMR regions. And > > > >> for cases > > > >> like the above, it will remain in the bypass mode. > > > >> > > > >> Do you see any problem(security?) if the dev streams remain in bypass > > > >> mode for > > > >> this dev? Or is it possible to have a stub driver for this dev, so > > > >> that we will have > > > >> the probe/attach invoked and everything will fall in place? > > > > > > > > The downside is that if a driver never binds to that device, it remains > > > > bypassed. If some other externally-controlled malicious device could > > > > manage to spoof that device's requester ID, that could potentially be > > > > exploited. > > > > > > > >> TBH, I haven't looked into creating a temp domain for these types of > > > >> the devices > > > >> and also not sure how we benefit from that compared to the STE bypass > > > >> mode. > > > > > > > > That said, setting up temporary translation contexts that ensure any > > > > access can *only* be to RMR regions until a driver takes over is an > > > > awful lot more work. I'm inclined to be pragmatic here and say we should > > > > get things working at all with the simple bypass STE/S2CR method, then > > > > look at adding the higher-security effort on top. > > > > > > > > Right now systems that need this are either broken (but effectively > > > > secure) or using default bypass with SMMUv2. People who prefer to > > > > maintain security over functionality in the interim can maintain that > > > > status quo by simply continuing to not describe any RMRs. > > > > > > I agree with Robin, let's get this working with the bypass mode (until > > > the device binds) like you've currently got. It's much better than what > > > we have otherwise. Then once that is merged we can look at the temporary > > > translation context or stub driver. The temporary translation context > > > would be 'neatest', but I'm only aware of the EFI framebuffer use case > > > and for that it might be possible to do something simpler - if indeed > > > anything is needed at all. I'm not sure how much we need to be worried > > > about malicious devices spoofing requester IDs. > > > > Perfect. I will keep the STE bypass and respin the series once the update > > to the IORT rev E is made public(hope that will happen soon). In the > > meantime, appreciate any feedback on the rest of the patches in this series. > > Shameer, Hi Jon, > > I am pretty sure rev E is already public. You can find it here. > > https://developer.arm.com/documentation/den0049/latest/ > > It is also marked non-confidential. Yes, Rev E is already out there. But I am told that ARM folks are working on some updates to the IORT spec, especially around the RMR topic. Hopefully it will be out soon. > > I also have initial patches for edk2 and the HoneyComb LX2160a > ACPI tables adding RMR nodes that partially work with your patches. Thanks for the update and good to know that it is useful. Shameer > This is with basic SMMUv2 support but since you have more experience > this this I am more than happy to work with you on your patchset. > > -Jon > > > > > > Thanks, > > Shameer > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Dec 17, 2020 at 4:42 PM Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > -----Original Message----- > > From: Jon Nettleton [mailto:jon@solid-run.com] > > Sent: 17 December 2020 14:48 > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; Guohanjun > > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm <linuxarm@huawei.com>; > > Jonathan Cameron <jonathan.cameron@huawei.com>; > > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > On Mon, Dec 14, 2020 at 3:48 PM Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Steven Price [mailto:steven.price@arm.com] > > > > Sent: 14 December 2020 13:43 > > > > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum Thodi > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > iommu@lists.linux-foundation.org; devel@acpica.org > > > > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun > > > > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > > > > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > > > > On 14/12/2020 12:33, Robin Murphy wrote: > > > > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > > > > >> Hi Steve, > > > > >> > > > > >>> -----Original Message----- > > > > >>> From: Steven Price [mailto:steven.price@arm.com] > > > > >>> Sent: 10 December 2020 10:26 > > > > >>> To: Shameerali Kolothum Thodi > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > >>> iommu@lists.linux-foundation.org; devel@acpica.org > > > > >>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > > > > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > > > > >>> <guohanjun@huawei.com>; Jonathan Cameron > > > > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > >>> > > > > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > > > > >>>> RFC v1 --> v2: > > > > >>>> - Added a generic interface for IOMMU drivers to retrieve all the > > > > >>>> RMR info associated with a given IOMMU. > > > > >>>> - SMMUv3 driver gets the RMR list during probe() and installs > > > > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > > > > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This is > > > > >>>> based on the suggestions received for v1 to take care of the > > > > >>>> EFI framebuffer use case. Only sanity tested for now. > > > > >>> > > > > >>> Hi Shameer, > > > > >>> > > > > >>> Sorry for not looking at this before. > > > > >>> > > > > >>> Do you have any plans to implement support in the SMMUv2 driver? > > The > > > > >>> platform I've been testing the EFI framebuffer support on has the > > > > >>> display controller behind SMMUv2, so as it stands this series doesn't > > > > >>> work. I did hack something up for SMMUv2 so I was able to test the > > first > > > > >>> 4 patches. > > > > >> > > > > >> Thanks for taking a look. Sure, I can look into adding the support for > > > > >> SMMUv2. > > > > > > > > Great, thanks! > > > > > > > > >>> > > > > >>>> - During the probe/attach device, SMMUv3 driver reserves any > > > > >>>> RMR region associated with the device such that there is a > > unity > > > > >>>> mapping for them in SMMU. > > > > >>> > > > > >>> For the EFI framebuffer use case there is no device to attach so I > > > > >>> believe we are left with just the stream ID in bypass mode - which is > > > > >>> definitely an improvement (the display works!) > > > > >> > > > > >> Cool. That’s good to know. > > > > >> > > > > >> but not actually a unity > > > > >>> mapping of the RMR range. I'm not sure whether it's worth fixing this > > or > > > > >>> not, but I just wanted to point out there's still a need for a driver > > > > >>> for the device before the bypass mode is replaced with the unity > > > > >>> mapping. > > > > >> > > > > >> I am not sure either. My idea was we will have bypass STE setup for > > > > >> all devices > > > > >> with RMR initially and when the corresponding driver takes over(if > > > > >> that happens) > > > > >> we will have the unity mapping setup properly for the RMR regions. And > > > > >> for cases > > > > >> like the above, it will remain in the bypass mode. > > > > >> > > > > >> Do you see any problem(security?) if the dev streams remain in bypass > > > > >> mode for > > > > >> this dev? Or is it possible to have a stub driver for this dev, so > > > > >> that we will have > > > > >> the probe/attach invoked and everything will fall in place? > > > > > > > > > > The downside is that if a driver never binds to that device, it remains > > > > > bypassed. If some other externally-controlled malicious device could > > > > > manage to spoof that device's requester ID, that could potentially be > > > > > exploited. > > > > > > > > > >> TBH, I haven't looked into creating a temp domain for these types of > > > > >> the devices > > > > >> and also not sure how we benefit from that compared to the STE bypass > > > > >> mode. > > > > > > > > > > That said, setting up temporary translation contexts that ensure any > > > > > access can *only* be to RMR regions until a driver takes over is an > > > > > awful lot more work. I'm inclined to be pragmatic here and say we should > > > > > get things working at all with the simple bypass STE/S2CR method, then > > > > > look at adding the higher-security effort on top. > > > > > > > > > > Right now systems that need this are either broken (but effectively > > > > > secure) or using default bypass with SMMUv2. People who prefer to > > > > > maintain security over functionality in the interim can maintain that > > > > > status quo by simply continuing to not describe any RMRs. > > > > > > > > I agree with Robin, let's get this working with the bypass mode (until > > > > the device binds) like you've currently got. It's much better than what > > > > we have otherwise. Then once that is merged we can look at the temporary > > > > translation context or stub driver. The temporary translation context > > > > would be 'neatest', but I'm only aware of the EFI framebuffer use case > > > > and for that it might be possible to do something simpler - if indeed > > > > anything is needed at all. I'm not sure how much we need to be worried > > > > about malicious devices spoofing requester IDs. > > > > > > Perfect. I will keep the STE bypass and respin the series once the update > > > to the IORT rev E is made public(hope that will happen soon). In the > > > meantime, appreciate any feedback on the rest of the patches in this series. > > > > Shameer, > > Hi Jon, > > > > > I am pretty sure rev E is already public. You can find it here. > > > > https://developer.arm.com/documentation/den0049/latest/ > > > > It is also marked non-confidential. > > Yes, Rev E is already out there. But I am told that ARM folks are working on > some updates to the IORT spec, especially around the RMR topic. Hopefully > it will be out soon. Yes there are some changes coming to the SPEC but I don't know if it is worth holding up your patchset as an initial implementation. If you would like I am more than happy to bring this up as a topic for the next Steering Committee meeting. Jon > > > > > I also have initial patches for edk2 and the HoneyComb LX2160a > > ACPI tables adding RMR nodes that partially work with your patches. > > Thanks for the update and good to know that it is useful. > > Shameer > > > This is with basic SMMUv2 support but since you have more experience > > this this I am more than happy to work with you on your patchset. > > > > -Jon > > > > > > > > > > Thanks, > > > Shameer > > > _______________________________________________ > > > linux-arm-kernel mailing list > > > linux-arm-kernel@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Thu, Dec 17, 2020 at 4:53 PM Jon Nettleton <jon@solid-run.com> wrote: > > On Thu, Dec 17, 2020 at 4:42 PM Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > -----Original Message----- > > > From: Jon Nettleton [mailto:jon@solid-run.com] > > > Sent: 17 December 2020 14:48 > > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > > > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > > > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > > > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; Guohanjun > > > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm <linuxarm@huawei.com>; > > > Jonathan Cameron <jonathan.cameron@huawei.com>; > > > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > > On Mon, Dec 14, 2020 at 3:48 PM Shameerali Kolothum Thodi > > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Steven Price [mailto:steven.price@arm.com] > > > > > Sent: 14 December 2020 13:43 > > > > > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum Thodi > > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > > iommu@lists.linux-foundation.org; devel@acpica.org > > > > > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > > > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; Guohanjun > > > > > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > > > > > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > > > > > > On 14/12/2020 12:33, Robin Murphy wrote: > > > > > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > > > > > >> Hi Steve, > > > > > >> > > > > > >>> -----Original Message----- > > > > > >>> From: Steven Price [mailto:steven.price@arm.com] > > > > > >>> Sent: 10 December 2020 10:26 > > > > > >>> To: Shameerali Kolothum Thodi > > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > > >>> iommu@lists.linux-foundation.org; devel@acpica.org > > > > > >>> Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > > > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > > > > > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > > > > > >>> <guohanjun@huawei.com>; Jonathan Cameron > > > > > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > >>> > > > > > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > > > > > >>>> RFC v1 --> v2: > > > > > >>>> - Added a generic interface for IOMMU drivers to retrieve all the > > > > > >>>> RMR info associated with a given IOMMU. > > > > > >>>> - SMMUv3 driver gets the RMR list during probe() and installs > > > > > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > > > > > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This is > > > > > >>>> based on the suggestions received for v1 to take care of the > > > > > >>>> EFI framebuffer use case. Only sanity tested for now. > > > > > >>> > > > > > >>> Hi Shameer, > > > > > >>> > > > > > >>> Sorry for not looking at this before. > > > > > >>> > > > > > >>> Do you have any plans to implement support in the SMMUv2 driver? > > > The > > > > > >>> platform I've been testing the EFI framebuffer support on has the > > > > > >>> display controller behind SMMUv2, so as it stands this series doesn't > > > > > >>> work. I did hack something up for SMMUv2 so I was able to test the > > > first > > > > > >>> 4 patches. > > > > > >> > > > > > >> Thanks for taking a look. Sure, I can look into adding the support for > > > > > >> SMMUv2. > > > > > > > > > > Great, thanks! > > > > > > > > > > >>> > > > > > >>>> - During the probe/attach device, SMMUv3 driver reserves any > > > > > >>>> RMR region associated with the device such that there is a > > > unity > > > > > >>>> mapping for them in SMMU. > > > > > >>> > > > > > >>> For the EFI framebuffer use case there is no device to attach so I > > > > > >>> believe we are left with just the stream ID in bypass mode - which is > > > > > >>> definitely an improvement (the display works!) > > > > > >> > > > > > >> Cool. That’s good to know. > > > > > >> > > > > > >> but not actually a unity > > > > > >>> mapping of the RMR range. I'm not sure whether it's worth fixing this > > > or > > > > > >>> not, but I just wanted to point out there's still a need for a driver > > > > > >>> for the device before the bypass mode is replaced with the unity > > > > > >>> mapping. > > > > > >> > > > > > >> I am not sure either. My idea was we will have bypass STE setup for > > > > > >> all devices > > > > > >> with RMR initially and when the corresponding driver takes over(if > > > > > >> that happens) > > > > > >> we will have the unity mapping setup properly for the RMR regions. And > > > > > >> for cases > > > > > >> like the above, it will remain in the bypass mode. > > > > > >> > > > > > >> Do you see any problem(security?) if the dev streams remain in bypass > > > > > >> mode for > > > > > >> this dev? Or is it possible to have a stub driver for this dev, so > > > > > >> that we will have > > > > > >> the probe/attach invoked and everything will fall in place? > > > > > > > > > > > > The downside is that if a driver never binds to that device, it remains > > > > > > bypassed. If some other externally-controlled malicious device could > > > > > > manage to spoof that device's requester ID, that could potentially be > > > > > > exploited. > > > > > > > > > > > >> TBH, I haven't looked into creating a temp domain for these types of > > > > > >> the devices > > > > > >> and also not sure how we benefit from that compared to the STE bypass > > > > > >> mode. > > > > > > > > > > > > That said, setting up temporary translation contexts that ensure any > > > > > > access can *only* be to RMR regions until a driver takes over is an > > > > > > awful lot more work. I'm inclined to be pragmatic here and say we should > > > > > > get things working at all with the simple bypass STE/S2CR method, then > > > > > > look at adding the higher-security effort on top. > > > > > > > > > > > > Right now systems that need this are either broken (but effectively > > > > > > secure) or using default bypass with SMMUv2. People who prefer to > > > > > > maintain security over functionality in the interim can maintain that > > > > > > status quo by simply continuing to not describe any RMRs. > > > > > > > > > > I agree with Robin, let's get this working with the bypass mode (until > > > > > the device binds) like you've currently got. It's much better than what > > > > > we have otherwise. Then once that is merged we can look at the temporary > > > > > translation context or stub driver. The temporary translation context > > > > > would be 'neatest', but I'm only aware of the EFI framebuffer use case > > > > > and for that it might be possible to do something simpler - if indeed > > > > > anything is needed at all. I'm not sure how much we need to be worried > > > > > about malicious devices spoofing requester IDs. > > > > > > > > Perfect. I will keep the STE bypass and respin the series once the update > > > > to the IORT rev E is made public(hope that will happen soon). In the > > > > meantime, appreciate any feedback on the rest of the patches in this series. > > > > > > Shameer, > > > > Hi Jon, > > > > > > > > I am pretty sure rev E is already public. You can find it here. > > > > > > https://developer.arm.com/documentation/den0049/latest/ > > > > > > It is also marked non-confidential. > > > > Yes, Rev E is already out there. But I am told that ARM folks are working on > > some updates to the IORT spec, especially around the RMR topic. Hopefully > > it will be out soon. > > Yes there are some changes coming to the SPEC but I don't know if it is > worth holding up your patchset as an initial implementation. If you would > like I am more than happy to bring this up as a topic for the next Steering > Committee meeting. > > Jon Shameer, My first attempt at smmuv2 support can be found in this kernel branch. https://github.com/SolidRun/linux-stable/commits/linux-5.10.y-cex7 It is functioning if the bypass SMRs are setup in the firmware and RMR's are exposed in the ACPI tables. Different from your situation we do want the device to reclaim the RMR's associated with it on initialization, and I am still seeing issues there. I need to spend more time figuring out why this is not working properly. -Jon > > > > > > > > > I also have initial patches for edk2 and the HoneyComb LX2160a > > > ACPI tables adding RMR nodes that partially work with your patches. > > > > Thanks for the update and good to know that it is useful. > > > > Shameer > > > > > This is with basic SMMUv2 support but since you have more experience > > > this this I am more than happy to work with you on your patchset. > > > > > > -Jon > > > > > > > > > > > > > > Thanks, > > > > Shameer > > > > _______________________________________________ > > > > linux-arm-kernel mailing list > > > > linux-arm-kernel@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> -----Original Message----- > From: Jon Nettleton [mailto:jon@solid-run.com] > Sent: 18 December 2020 10:53 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; Guohanjun > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm <linuxarm@huawei.com>; > Jonathan Cameron <jonathan.cameron@huawei.com>; > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > On Thu, Dec 17, 2020 at 4:53 PM Jon Nettleton <jon@solid-run.com> wrote: > > > > On Thu, Dec 17, 2020 at 4:42 PM Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > -----Original Message----- > > > > From: Jon Nettleton [mailto:jon@solid-run.com] > > > > Sent: 17 December 2020 14:48 > > > > To: Shameerali Kolothum Thodi > <shameerali.kolothum.thodi@huawei.com> > > > > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > > > > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > > > > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > > > > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; > Guohanjun > > > > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm > <linuxarm@huawei.com>; > > > > Jonathan Cameron <jonathan.cameron@huawei.com>; > > > > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > > > > On Mon, Dec 14, 2020 at 3:48 PM Shameerali Kolothum Thodi > > > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > From: Steven Price [mailto:steven.price@arm.com] > > > > > > Sent: 14 December 2020 13:43 > > > > > > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum > Thodi > > > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > > > iommu@lists.linux-foundation.org; devel@acpica.org > > > > > > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > > > > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; > Guohanjun > > > > > > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > > > > > > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR > node > > > > > > > > > > > > On 14/12/2020 12:33, Robin Murphy wrote: > > > > > > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > > > > > > >> Hi Steve, > > > > > > >> > > > > > > >>> -----Original Message----- > > > > > > >>> From: Steven Price [mailto:steven.price@arm.com] > > > > > > >>> Sent: 10 December 2020 10:26 > > > > > > >>> To: Shameerali Kolothum Thodi > > > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > > > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > > > >>> iommu@lists.linux-foundation.org; devel@acpica.org > > > > > > >>> Cc: Linuxarm <linuxarm@huawei.com>; > lorenzo.pieralisi@arm.com; > > > > > > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > > > > > > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > > > > > > >>> <guohanjun@huawei.com>; Jonathan Cameron > > > > > > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > > > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR > node > > > > > > >>> > > > > > > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > > > > > > >>>> RFC v1 --> v2: > > > > > > >>>> - Added a generic interface for IOMMU drivers to retrieve all > the > > > > > > >>>> RMR info associated with a given IOMMU. > > > > > > >>>> - SMMUv3 driver gets the RMR list during probe() and > installs > > > > > > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > > > > > > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This > is > > > > > > >>>> based on the suggestions received for v1 to take care of > the > > > > > > >>>> EFI framebuffer use case. Only sanity tested for now. > > > > > > >>> > > > > > > >>> Hi Shameer, > > > > > > >>> > > > > > > >>> Sorry for not looking at this before. > > > > > > >>> > > > > > > >>> Do you have any plans to implement support in the SMMUv2 > driver? > > > > The > > > > > > >>> platform I've been testing the EFI framebuffer support on has the > > > > > > >>> display controller behind SMMUv2, so as it stands this series > doesn't > > > > > > >>> work. I did hack something up for SMMUv2 so I was able to test > the > > > > first > > > > > > >>> 4 patches. > > > > > > >> > > > > > > >> Thanks for taking a look. Sure, I can look into adding the support for > > > > > > >> SMMUv2. > > > > > > > > > > > > Great, thanks! > > > > > > > > > > > > >>> > > > > > > >>>> - During the probe/attach device, SMMUv3 driver reserves > any > > > > > > >>>> RMR region associated with the device such that there is > a > > > > unity > > > > > > >>>> mapping for them in SMMU. > > > > > > >>> > > > > > > >>> For the EFI framebuffer use case there is no device to attach so I > > > > > > >>> believe we are left with just the stream ID in bypass mode - which > is > > > > > > >>> definitely an improvement (the display works!) > > > > > > >> > > > > > > >> Cool. That’s good to know. > > > > > > >> > > > > > > >> but not actually a unity > > > > > > >>> mapping of the RMR range. I'm not sure whether it's worth fixing > this > > > > or > > > > > > >>> not, but I just wanted to point out there's still a need for a driver > > > > > > >>> for the device before the bypass mode is replaced with the unity > > > > > > >>> mapping. > > > > > > >> > > > > > > >> I am not sure either. My idea was we will have bypass STE setup for > > > > > > >> all devices > > > > > > >> with RMR initially and when the corresponding driver takes over(if > > > > > > >> that happens) > > > > > > >> we will have the unity mapping setup properly for the RMR regions. > And > > > > > > >> for cases > > > > > > >> like the above, it will remain in the bypass mode. > > > > > > >> > > > > > > >> Do you see any problem(security?) if the dev streams remain in > bypass > > > > > > >> mode for > > > > > > >> this dev? Or is it possible to have a stub driver for this dev, so > > > > > > >> that we will have > > > > > > >> the probe/attach invoked and everything will fall in place? > > > > > > > > > > > > > > The downside is that if a driver never binds to that device, it remains > > > > > > > bypassed. If some other externally-controlled malicious device could > > > > > > > manage to spoof that device's requester ID, that could potentially be > > > > > > > exploited. > > > > > > > > > > > > > >> TBH, I haven't looked into creating a temp domain for these types > of > > > > > > >> the devices > > > > > > >> and also not sure how we benefit from that compared to the STE > bypass > > > > > > >> mode. > > > > > > > > > > > > > > That said, setting up temporary translation contexts that ensure any > > > > > > > access can *only* be to RMR regions until a driver takes over is an > > > > > > > awful lot more work. I'm inclined to be pragmatic here and say we > should > > > > > > > get things working at all with the simple bypass STE/S2CR method, > then > > > > > > > look at adding the higher-security effort on top. > > > > > > > > > > > > > > Right now systems that need this are either broken (but effectively > > > > > > > secure) or using default bypass with SMMUv2. People who prefer to > > > > > > > maintain security over functionality in the interim can maintain that > > > > > > > status quo by simply continuing to not describe any RMRs. > > > > > > > > > > > > I agree with Robin, let's get this working with the bypass mode (until > > > > > > the device binds) like you've currently got. It's much better than what > > > > > > we have otherwise. Then once that is merged we can look at the > temporary > > > > > > translation context or stub driver. The temporary translation context > > > > > > would be 'neatest', but I'm only aware of the EFI framebuffer use case > > > > > > and for that it might be possible to do something simpler - if indeed > > > > > > anything is needed at all. I'm not sure how much we need to be > worried > > > > > > about malicious devices spoofing requester IDs. > > > > > > > > > > Perfect. I will keep the STE bypass and respin the series once the update > > > > > to the IORT rev E is made public(hope that will happen soon). In the > > > > > meantime, appreciate any feedback on the rest of the patches in this > series. > > > > > > > > Shameer, > > > > > > Hi Jon, > > > > > > > > > > > I am pretty sure rev E is already public. You can find it here. > > > > > > > > https://developer.arm.com/documentation/den0049/latest/ > > > > > > > > It is also marked non-confidential. > > > > > > Yes, Rev E is already out there. But I am told that ARM folks are working on > > > some updates to the IORT spec, especially around the RMR topic. Hopefully > > > it will be out soon. > > > > Yes there are some changes coming to the SPEC but I don't know if it is > > worth holding up your patchset as an initial implementation. If you would > > like I am more than happy to bring this up as a topic for the next Steering > > Committee meeting. > > > > Jon > > Shameer, > Hi Jon, > My first attempt at smmuv2 support can be found in this kernel branch. > > https://github.com/SolidRun/linux-stable/commits/linux-5.10.y-cex7 > > It is functioning if the bypass SMRs are setup in the firmware and RMR's > are exposed in the ACPI tables. Ok. Thanks for sharing it. Happy to carry those patches as part of this series if you are fine with it. Different from your situation we do want > the device to reclaim the RMR's associated with it on initialization, and I > am still seeing issues there. I need to spend more time figuring out why > this is not working properly. I am not sure what your requirement is here. So if the kernel driver eventually comes up and takes control of the device, you no longer require the bypass/identity mapping for these regions. Is that correct? Shameer > -Jon > > > > > > > > > > > > > > I also have initial patches for edk2 and the HoneyComb LX2160a > > > > ACPI tables adding RMR nodes that partially work with your patches. > > > > > > Thanks for the update and good to know that it is useful. > > > > > > Shameer > > > > > > > This is with basic SMMUv2 support but since you have more experience > > > > this this I am more than happy to work with you on your patchset. > > > > > > > > -Jon > > > > > > > > > > > > > > > > > > Thanks, > > > > > Shameer > > > > > _______________________________________________ > > > > > linux-arm-kernel mailing list > > > > > linux-arm-kernel@lists.infradead.org > > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
On Mon, Jan 4, 2021 at 9:55 AM Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > -----Original Message----- > > From: Jon Nettleton [mailto:jon@solid-run.com] > > Sent: 18 December 2020 10:53 > > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> > > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; Guohanjun > > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm <linuxarm@huawei.com>; > > Jonathan Cameron <jonathan.cameron@huawei.com>; > > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > On Thu, Dec 17, 2020 at 4:53 PM Jon Nettleton <jon@solid-run.com> wrote: > > > > > > On Thu, Dec 17, 2020 at 4:42 PM Shameerali Kolothum Thodi > > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > From: Jon Nettleton [mailto:jon@solid-run.com] > > > > > Sent: 17 December 2020 14:48 > > > > > To: Shameerali Kolothum Thodi > > <shameerali.kolothum.thodi@huawei.com> > > > > > Cc: Steven Price <steven.price@arm.com>; Robin Murphy > > > > > <robin.murphy@arm.com>; linux-arm-kernel@lists.infradead.org; > > > > > linux-acpi@vger.kernel.org; iommu@lists.linux-foundation.org; > > > > > devel@acpica.org; lorenzo.pieralisi@arm.com; joro@8bytes.org; > > Guohanjun > > > > > (Hanjun Guo) <guohanjun@huawei.com>; Linuxarm > > <linuxarm@huawei.com>; > > > > > Jonathan Cameron <jonathan.cameron@huawei.com>; > > > > > Sami.Mujawar@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > > > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > > > > > > > > > On Mon, Dec 14, 2020 at 3:48 PM Shameerali Kolothum Thodi > > > > > <shameerali.kolothum.thodi@huawei.com> wrote: > > > > > > > > > > > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > From: Steven Price [mailto:steven.price@arm.com] > > > > > > > Sent: 14 December 2020 13:43 > > > > > > > To: Robin Murphy <robin.murphy@arm.com>; Shameerali Kolothum > > Thodi > > > > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > > > > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > > > > iommu@lists.linux-foundation.org; devel@acpica.org > > > > > > > Cc: Linuxarm <linuxarm@huawei.com>; lorenzo.pieralisi@arm.com; > > > > > > > joro@8bytes.org; wanghuiqiang <wanghuiqiang@huawei.com>; > > Guohanjun > > > > > > > (Hanjun Guo) <guohanjun@huawei.com>; Jonathan Cameron > > > > > > > <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > > > > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR > > node > > > > > > > > > > > > > > On 14/12/2020 12:33, Robin Murphy wrote: > > > > > > > > On 2020-12-14 10:55, Shameerali Kolothum Thodi wrote: > > > > > > > >> Hi Steve, > > > > > > > >> > > > > > > > >>> -----Original Message----- > > > > > > > >>> From: Steven Price [mailto:steven.price@arm.com] > > > > > > > >>> Sent: 10 December 2020 10:26 > > > > > > > >>> To: Shameerali Kolothum Thodi > > > > > > > <shameerali.kolothum.thodi@huawei.com>; > > > > > > > >>> linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > > > > > > > >>> iommu@lists.linux-foundation.org; devel@acpica.org > > > > > > > >>> Cc: Linuxarm <linuxarm@huawei.com>; > > lorenzo.pieralisi@arm.com; > > > > > > > >>> joro@8bytes.org; robin.murphy@arm.com; wanghuiqiang > > > > > > > >>> <wanghuiqiang@huawei.com>; Guohanjun (Hanjun Guo) > > > > > > > >>> <guohanjun@huawei.com>; Jonathan Cameron > > > > > > > >>> <jonathan.cameron@huawei.com>; Sami.Mujawar@arm.com > > > > > > > >>> Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR > > node > > > > > > > >>> > > > > > > > >>> On 19/11/2020 12:11, Shameer Kolothum wrote: > > > > > > > >>>> RFC v1 --> v2: > > > > > > > >>>> - Added a generic interface for IOMMU drivers to retrieve all > > the > > > > > > > >>>> RMR info associated with a given IOMMU. > > > > > > > >>>> - SMMUv3 driver gets the RMR list during probe() and > > installs > > > > > > > >>>> bypass STEs for all the SIDs in the RMR list. This is to keep > > > > > > > >>>> the ongoing traffic alive(if any) during SMMUv3 reset. This > > is > > > > > > > >>>> based on the suggestions received for v1 to take care of > > the > > > > > > > >>>> EFI framebuffer use case. Only sanity tested for now. > > > > > > > >>> > > > > > > > >>> Hi Shameer, > > > > > > > >>> > > > > > > > >>> Sorry for not looking at this before. > > > > > > > >>> > > > > > > > >>> Do you have any plans to implement support in the SMMUv2 > > driver? > > > > > The > > > > > > > >>> platform I've been testing the EFI framebuffer support on has the > > > > > > > >>> display controller behind SMMUv2, so as it stands this series > > doesn't > > > > > > > >>> work. I did hack something up for SMMUv2 so I was able to test > > the > > > > > first > > > > > > > >>> 4 patches. > > > > > > > >> > > > > > > > >> Thanks for taking a look. Sure, I can look into adding the support for > > > > > > > >> SMMUv2. > > > > > > > > > > > > > > Great, thanks! > > > > > > > > > > > > > > >>> > > > > > > > >>>> - During the probe/attach device, SMMUv3 driver reserves > > any > > > > > > > >>>> RMR region associated with the device such that there is > > a > > > > > unity > > > > > > > >>>> mapping for them in SMMU. > > > > > > > >>> > > > > > > > >>> For the EFI framebuffer use case there is no device to attach so I > > > > > > > >>> believe we are left with just the stream ID in bypass mode - which > > is > > > > > > > >>> definitely an improvement (the display works!) > > > > > > > >> > > > > > > > >> Cool. That’s good to know. > > > > > > > >> > > > > > > > >> but not actually a unity > > > > > > > >>> mapping of the RMR range. I'm not sure whether it's worth fixing > > this > > > > > or > > > > > > > >>> not, but I just wanted to point out there's still a need for a driver > > > > > > > >>> for the device before the bypass mode is replaced with the unity > > > > > > > >>> mapping. > > > > > > > >> > > > > > > > >> I am not sure either. My idea was we will have bypass STE setup for > > > > > > > >> all devices > > > > > > > >> with RMR initially and when the corresponding driver takes over(if > > > > > > > >> that happens) > > > > > > > >> we will have the unity mapping setup properly for the RMR regions. > > And > > > > > > > >> for cases > > > > > > > >> like the above, it will remain in the bypass mode. > > > > > > > >> > > > > > > > >> Do you see any problem(security?) if the dev streams remain in > > bypass > > > > > > > >> mode for > > > > > > > >> this dev? Or is it possible to have a stub driver for this dev, so > > > > > > > >> that we will have > > > > > > > >> the probe/attach invoked and everything will fall in place? > > > > > > > > > > > > > > > > The downside is that if a driver never binds to that device, it remains > > > > > > > > bypassed. If some other externally-controlled malicious device could > > > > > > > > manage to spoof that device's requester ID, that could potentially be > > > > > > > > exploited. > > > > > > > > > > > > > > > >> TBH, I haven't looked into creating a temp domain for these types > > of > > > > > > > >> the devices > > > > > > > >> and also not sure how we benefit from that compared to the STE > > bypass > > > > > > > >> mode. > > > > > > > > > > > > > > > > That said, setting up temporary translation contexts that ensure any > > > > > > > > access can *only* be to RMR regions until a driver takes over is an > > > > > > > > awful lot more work. I'm inclined to be pragmatic here and say we > > should > > > > > > > > get things working at all with the simple bypass STE/S2CR method, > > then > > > > > > > > look at adding the higher-security effort on top. > > > > > > > > > > > > > > > > Right now systems that need this are either broken (but effectively > > > > > > > > secure) or using default bypass with SMMUv2. People who prefer to > > > > > > > > maintain security over functionality in the interim can maintain that > > > > > > > > status quo by simply continuing to not describe any RMRs. > > > > > > > > > > > > > > I agree with Robin, let's get this working with the bypass mode (until > > > > > > > the device binds) like you've currently got. It's much better than what > > > > > > > we have otherwise. Then once that is merged we can look at the > > temporary > > > > > > > translation context or stub driver. The temporary translation context > > > > > > > would be 'neatest', but I'm only aware of the EFI framebuffer use case > > > > > > > and for that it might be possible to do something simpler - if indeed > > > > > > > anything is needed at all. I'm not sure how much we need to be > > worried > > > > > > > about malicious devices spoofing requester IDs. > > > > > > > > > > > > Perfect. I will keep the STE bypass and respin the series once the update > > > > > > to the IORT rev E is made public(hope that will happen soon). In the > > > > > > meantime, appreciate any feedback on the rest of the patches in this > > series. > > > > > > > > > > Shameer, > > > > > > > > Hi Jon, > > > > > > > > > > > > > > I am pretty sure rev E is already public. You can find it here. > > > > > > > > > > https://developer.arm.com/documentation/den0049/latest/ > > > > > > > > > > It is also marked non-confidential. > > > > > > > > Yes, Rev E is already out there. But I am told that ARM folks are working on > > > > some updates to the IORT spec, especially around the RMR topic. Hopefully > > > > it will be out soon. > > > > > > Yes there are some changes coming to the SPEC but I don't know if it is > > > worth holding up your patchset as an initial implementation. If you would > > > like I am more than happy to bring this up as a topic for the next Steering > > > Committee meeting. > > > > > > Jon > > > > Shameer, > > > > Hi Jon, > > > My first attempt at smmuv2 support can be found in this kernel branch. > > > > https://github.com/SolidRun/linux-stable/commits/linux-5.10.y-cex7 > > > > It is functioning if the bypass SMRs are setup in the firmware and RMR's > > are exposed in the ACPI tables. > > Ok. Thanks for sharing it. Happy to carry those patches as part of this > series if you are fine with it. That is fine with me. I have also cc'd Laurentiu from NXP as part of the code is lifted from his integration with the Qualcom patches submitted for device-tree handling. We may want to also start a conversation if the RMR list should be populated from the device-tree codepaths and then have the SMMU* drivers share the same mechanism for adding the bypass mappings and then letting drivers reclaim the DMA regions. I think there was an Nvidia patchset that did something similar based on DT reserved memory regions and smmu mappings. > > Different from your situation we do want > > the device to reclaim the RMR's associated with it on initialization, and I > > am still seeing issues there. I need to spend more time figuring out why > > this is not working properly. > > I am not sure what your requirement is here. So if the kernel driver eventually > comes up and takes control of the device, you no longer require the bypass/identity > mapping for these regions. Is that correct? Yes this is basically correct. I believe some of the issues I have run into will be covered in the next release of the RMR specs. For now this patchset does give me basic working functionality, and the future refinements will add more fine grained security that I am looking for. As I mentioned before, it would be nice to get this functionality moving forward and then we can refine it as spec updates are made public. -Jon > > Shameer > > > -Jon > > > > > > > > > > > > > > > > > > > I also have initial patches for edk2 and the HoneyComb LX2160a > > > > > ACPI tables adding RMR nodes that partially work with your patches. > > > > > > > > Thanks for the update and good to know that it is useful. > > > > > > > > Shameer > > > > > > > > > This is with basic SMMUv2 support but since you have more experience > > > > > this this I am more than happy to work with you on your patchset. > > > > > > > > > > -Jon > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > Shameer > > > > > > _______________________________________________ > > > > > > linux-arm-kernel mailing list > > > > > > linux-arm-kernel@lists.infradead.org > > > > > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Hi Shameer, On 11/19/20 1:11 PM, Shameer Kolothum wrote: > RFC v1 --> v2: > - Added a generic interface for IOMMU drivers to retrieve all the > RMR info associated with a given IOMMU. > - SMMUv3 driver gets the RMR list during probe() and installs > bypass STEs for all the SIDs in the RMR list. This is to keep > the ongoing traffic alive(if any) during SMMUv3 reset. This is > based on the suggestions received for v1 to take care of the > EFI framebuffer use case. Only sanity tested for now. > - During the probe/attach device, SMMUv3 driver reserves any > RMR region associated with the device such that there is a unity > mapping for them in SMMU. > --- > > The series adds support to IORT RMR nodes specified in IORT > Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory > ranges that are used by endpoints and require a unity mapping > in SMMU. > > We have faced issues with 3408iMR RAID controller cards which > fail to boot when SMMU is enabled. This is because these controllers > make use of host memory for various caching related purposes and when > SMMU is enabled the iMR firmware fails to access these memory regions > as there is no mapping for them. IORT RMR provides a way for UEFI to > describe and report these memory regions so that the kernel can make > a unity mapping for these in SMMU. > > RFC because, Patch #1 is to update the actbl2.h and should be done > through acpica update. I have send out a pull request[1] for that. What is the state of this series? I wondered if I should consider using it for nested SMMU to avoid handling nested binding for MSI, as suggested by Jean. Are there any blocker? Thanks Eric > > Tests: > > With a UEFI, that reports the RMR for the dev, > .... > [16F0h 5872 1] Type : 06 > [16F1h 5873 2] Length : 007C > [16F3h 5875 1] Revision : 00 > [1038h 0056 2] Reserved : 00000000 > [1038h 0056 2] Identifier : 00000000 > [16F8h 5880 4] Mapping Count : 00000001 > [16FCh 5884 4] Mapping Offset : 00000040 > > [1700h 5888 4] Number of RMR Descriptors : 00000002 > [1704h 5892 4] RMR Descriptor Offset : 00000018 > > [1708h 5896 8] Base Address of RMR : 0000E6400000 > [1710h 5904 8] Length of RMR : 000000100000 > [1718h 5912 4] Reserved : 00000000 > > [171Ch 5916 8] Base Address of RMR : 0000000027B00000 > [1724h 5924 8] Length of RMR : 0000000000C00000 > [172Ch 5932 4] Reserved : 00000000 > > [1730h 5936 4] Input base : 00000000 > [1734h 5940 4] ID Count : 00000001 > [1738h 5944 4] Output Base : 00000003 > [173Ch 5948 4] Output Reference : 00000064 > [1740h 5952 4] Flags (decoded below) : 00000001 > Single Mapping : 1 > ... > > Without the series the RAID controller initialization fails as > below, > > ... > [ 12.631117] megaraid_sas 0000:03:00.0: FW supports sync cache : Yes > [ 12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009 > [ 18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED for SCSI host 0 > [ 23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to ready state > [ 106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault code:0x10000 subcode:0x0 func:megasas_transition_to_ready > [ 106.695186] megaraid_sas 0000:03:00.0: System Register set: > [ 106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to ready for scsi0. > [ 106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw 6407 > estuary:/$ > > With the series, now the kernel has direct mapping for the dev as > below, > > estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions > 0x0000000008000000 0x00000000080fffff msi > 0x0000000027b00000 0x00000000286fffff direct > 0x00000000e6400000 0x00000000e64fffff direct > estuary:/$ > > .... > [ 12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009 > [ 12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: 0 max_lds: 32 > [ 12.746628] megaraid_sas 0000:03:00.0: controller type : iMR(0MB) > [ 12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR) : Enabled > [ 12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support : Yes > [ 12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes > [ 12.771931] megaraid_sas 0000:03:00.0: FW provided TM TaskAbort/Reset timeou: 6 secs/60 secs > [ 12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map support : Yes > [ 12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining support : No > [ 12.819179] megaraid_sas 0000:03:00.0: NVME page size : (4096) > [ 12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is called outbound_intr_mask:0x40000000 > [ 12.835199] megaraid_sas 0000:03:00.0: INIT adapter done > [ 12.873932] megaraid_sas 0000:03:00.0: pci id : (0x1000)/(0x0017)/(0x19e5)/(0xd213) > [ 12.881644] megaraid_sas 0000:03:00.0: unevenspan support : no > [ 12.887451] megaraid_sas 0000:03:00.0: firmware crash dump : no > [ 12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map : enabled > > RAID controller init is now success and can detect the drives > attached as well. > > Thanks, > Shameer > > [0]. https://developer.arm.com/documentation/den0049/latest/ > [1]. https://github.com/acpica/acpica/pull/638 > > Shameer Kolothum (8): > ACPICA: IORT: Update for revision E > ACPI/IORT: Add support for RMR node parsing > iommu/dma: Introduce generic helper to retrieve RMR info > ACPI/IORT: Add RMR memory regions reservation helper > iommu/arm-smmu-v3: Introduce strtab init helper > iommu/arm-smmu-v3: Add bypass flag to arm_smmu_write_strtab_ent() > iommu/arm-smmu-v3: Get associated RMR info and install bypass STE > iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev > > drivers/acpi/arm64/iort.c | 182 +++++++++++++++++++- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 112 ++++++++++-- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 + > drivers/iommu/dma-iommu.c | 39 +++++ > include/acpi/actbl2.h | 25 ++- > include/linux/acpi_iort.h | 6 + > include/linux/dma-iommu.h | 7 + > include/linux/iommu.h | 16 ++ > 8 files changed, 367 insertions(+), 22 deletions(-) >
Hi Eric, > -----Original Message----- > From: Auger Eric [mailto:eric.auger@redhat.com] > Sent: 09 April 2021 10:51 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org; devel@acpica.org > Cc: Linuxarm <linuxarm@huawei.com>; steven.price@arm.com; Guohanjun > (Hanjun Guo) <guohanjun@huawei.com>; Sami.Mujawar@arm.com; > robin.murphy@arm.com; wanghuiqiang <wanghuiqiang@huawei.com> > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > Hi Shameer, > > On 11/19/20 1:11 PM, Shameer Kolothum wrote: > > RFC v1 --> v2: > > - Added a generic interface for IOMMU drivers to retrieve all the > > RMR info associated with a given IOMMU. > > - SMMUv3 driver gets the RMR list during probe() and installs > > bypass STEs for all the SIDs in the RMR list. This is to keep > > the ongoing traffic alive(if any) during SMMUv3 reset. This is > > based on the suggestions received for v1 to take care of the > > EFI framebuffer use case. Only sanity tested for now. > > - During the probe/attach device, SMMUv3 driver reserves any > > RMR region associated with the device such that there is a unity > > mapping for them in SMMU. > > --- > > > > The series adds support to IORT RMR nodes specified in IORT > > Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory > > ranges that are used by endpoints and require a unity mapping > > in SMMU. > > > > We have faced issues with 3408iMR RAID controller cards which > > fail to boot when SMMU is enabled. This is because these controllers > > make use of host memory for various caching related purposes and when > > SMMU is enabled the iMR firmware fails to access these memory regions > > as there is no mapping for them. IORT RMR provides a way for UEFI to > > describe and report these memory regions so that the kernel can make > > a unity mapping for these in SMMU. > > > > RFC because, Patch #1 is to update the actbl2.h and should be done > > through acpica update. I have send out a pull request[1] for that. > > What is the state of this series? I wondered if I should consider using > it for nested SMMU to avoid handling nested binding for MSI, as > suggested by Jean. Are there any blocker? There were few issues with the revision E spec and those are now sorted with an updated E.b. The ACPICA pull request for E.b is now merged[1] and the Linux header update patch[2] is also out there(I think it is now queued for 5.13). I will soon respin this series. Thanks, Shameer 1. https://github.com/acpica/acpica/pull/682 2. https://lore.kernel.org/linux-acpi/20210406213028.718796-22-erik.kaneda@intel.com/ > > Thanks > > Eric > > > > Tests: > > > > With a UEFI, that reports the RMR for the dev, > > .... > > [16F0h 5872 1] Type : 06 > > [16F1h 5873 2] Length : 007C > > [16F3h 5875 1] Revision : 00 > > [1038h 0056 2] Reserved : 00000000 > > [1038h 0056 2] Identifier : 00000000 > > [16F8h 5880 4] Mapping Count : 00000001 > > [16FCh 5884 4] Mapping Offset : 00000040 > > > > [1700h 5888 4] Number of RMR Descriptors : 00000002 > > [1704h 5892 4] RMR Descriptor Offset : 00000018 > > > > [1708h 5896 8] Base Address of RMR : 0000E6400000 > > [1710h 5904 8] Length of RMR : 000000100000 > > [1718h 5912 4] Reserved : 00000000 > > > > [171Ch 5916 8] Base Address of RMR : 0000000027B00000 > > [1724h 5924 8] Length of RMR : 0000000000C00000 > > [172Ch 5932 4] Reserved : 00000000 > > > > [1730h 5936 4] Input base : 00000000 > > [1734h 5940 4] ID Count : 00000001 > > [1738h 5944 4] Output Base : 00000003 > > [173Ch 5948 4] Output Reference : 00000064 > > [1740h 5952 4] Flags (decoded below) : 00000001 > > Single Mapping : 1 > > ... > > > > Without the series the RAID controller initialization fails as > > below, > > > > ... > > [ 12.631117] megaraid_sas 0000:03:00.0: FW supports sync > cache : Yes > > [ 12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is > called outbound_intr_mask:0x40000009 > > [ 18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED > for SCSI host 0 > > [ 23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to > ready state > > [ 106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault > code:0x10000 subcode:0x0 func:megasas_transition_to_ready > > [ 106.695186] megaraid_sas 0000:03:00.0: System Register set: > > [ 106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to > ready for scsi0. > > [ 106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw > 6407 > > estuary:/$ > > > > With the series, now the kernel has direct mapping for the dev as > > below, > > > > estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions > > 0x0000000008000000 0x00000000080fffff msi > > 0x0000000027b00000 0x00000000286fffff direct > > 0x00000000e6400000 0x00000000e64fffff direct > > estuary:/$ > > > > .... > > [ 12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is > called outbound_intr_mask:0x40000009 > > [ 12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: > 0 max_lds: 32 > > [ 12.746628] megaraid_sas 0000:03:00.0: controller type : > iMR(0MB) > > [ 12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR) : > Enabled > > [ 12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support : Yes > > [ 12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes > > [ 12.771931] megaraid_sas 0000:03:00.0: FW provided TM > TaskAbort/Reset timeou: 6 secs/60 secs > > [ 12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map > support : Yes > > [ 12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining > support : No > > [ 12.819179] megaraid_sas 0000:03:00.0: NVME page size : > (4096) > > [ 12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is > called outbound_intr_mask:0x40000000 > > [ 12.835199] megaraid_sas 0000:03:00.0: INIT adapter done > > [ 12.873932] megaraid_sas 0000:03:00.0: pci id : > (0x1000)/(0x0017)/(0x19e5)/(0xd213) > > [ 12.881644] megaraid_sas 0000:03:00.0: unevenspan support : no > > [ 12.887451] megaraid_sas 0000:03:00.0: firmware crash dump : no > > [ 12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map : > enabled > > > > RAID controller init is now success and can detect the drives > > attached as well. > > > > Thanks, > > Shameer > > > > [0]. https://developer.arm.com/documentation/den0049/latest/ > > [1]. https://github.com/acpica/acpica/pull/638 > > > > Shameer Kolothum (8): > > ACPICA: IORT: Update for revision E > > ACPI/IORT: Add support for RMR node parsing > > iommu/dma: Introduce generic helper to retrieve RMR info > > ACPI/IORT: Add RMR memory regions reservation helper > > iommu/arm-smmu-v3: Introduce strtab init helper > > iommu/arm-smmu-v3: Add bypass flag to arm_smmu_write_strtab_ent() > > iommu/arm-smmu-v3: Get associated RMR info and install bypass STE > > iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev > > > > drivers/acpi/arm64/iort.c | 182 > +++++++++++++++++++- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 112 ++++++++++-- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 + > > drivers/iommu/dma-iommu.c | 39 +++++ > > include/acpi/actbl2.h | 25 ++- > > include/linux/acpi_iort.h | 6 + > > include/linux/dma-iommu.h | 7 + > > include/linux/iommu.h | 16 ++ > > 8 files changed, 367 insertions(+), 22 deletions(-) > >
Hi Shameer, + Jean-Philippe On 11/19/20 1:11 PM, Shameer Kolothum wrote: > RFC v1 --> v2: > - Added a generic interface for IOMMU drivers to retrieve all the > RMR info associated with a given IOMMU. > - SMMUv3 driver gets the RMR list during probe() and installs > bypass STEs for all the SIDs in the RMR list. This is to keep > the ongoing traffic alive(if any) during SMMUv3 reset. This is > based on the suggestions received for v1 to take care of the > EFI framebuffer use case. Only sanity tested for now. > - During the probe/attach device, SMMUv3 driver reserves any > RMR region associated with the device such that there is a unity > mapping for them in SMMU. > --- > > The series adds support to IORT RMR nodes specified in IORT > Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory > ranges that are used by endpoints and require a unity mapping > in SMMU. > > We have faced issues with 3408iMR RAID controller cards which > fail to boot when SMMU is enabled. This is because these controllers > make use of host memory for various caching related purposes and when > SMMU is enabled the iMR firmware fails to access these memory regions > as there is no mapping for them. IORT RMR provides a way for UEFI to > describe and report these memory regions so that the kernel can make > a unity mapping for these in SMMU. > > RFC because, Patch #1 is to update the actbl2.h and should be done > through acpica update. I have send out a pull request[1] for that. > > Tests: > > With a UEFI, that reports the RMR for the dev, > .... > [16F0h 5872 1] Type : 06 > [16F1h 5873 2] Length : 007C > [16F3h 5875 1] Revision : 00 > [1038h 0056 2] Reserved : 00000000 > [1038h 0056 2] Identifier : 00000000 > [16F8h 5880 4] Mapping Count : 00000001 > [16FCh 5884 4] Mapping Offset : 00000040 > > [1700h 5888 4] Number of RMR Descriptors : 00000002 > [1704h 5892 4] RMR Descriptor Offset : 00000018 > > [1708h 5896 8] Base Address of RMR : 0000E6400000 > [1710h 5904 8] Length of RMR : 000000100000 > [1718h 5912 4] Reserved : 00000000 > > [171Ch 5916 8] Base Address of RMR : 0000000027B00000 > [1724h 5924 8] Length of RMR : 0000000000C00000 > [172Ch 5932 4] Reserved : 00000000 > > [1730h 5936 4] Input base : 00000000 > [1734h 5940 4] ID Count : 00000001 > [1738h 5944 4] Output Base : 00000003 > [173Ch 5948 4] Output Reference : 00000064 > [1740h 5952 4] Flags (decoded below) : 00000001 > Single Mapping : 1 Following Jean-Philippe's suggestion I have used your series for nested stage SMMUv3 integration, ie. to simplify the MSI nested stage mapping. Host allocates hIOVA -> physical doorbell (pDB) as it normally does for VFIO device passthrough. IOVA Range is 0x8000000 - 0x8100000. I expose this MIS IOVA range to the guest as an RMR and as a result guest has a flat mapping for this range. As the physical device is programmed with hIOVA we have the following mapping: IOVA IPA PA hIOVA -> hIOVA -> pDB S1 s2 This works. The only weird thing is that I need to expose 256 RMRs due to the 'Single Mapping' mandatory flag. I need to have 1 RMR per potential SID on the bus. I will post a new version of SMMUv3 nested stage soon for people to test & compare. Obviously this removes a bunch of code on both SMMU/VFIO and QEMU code so I think this solution looks better overall. Thanks Eric > ... > > Without the series the RAID controller initialization fails as > below, > > ... > [ 12.631117] megaraid_sas 0000:03:00.0: FW supports sync cache : Yes > [ 12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009 > [ 18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED for SCSI host 0 > [ 23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to ready state > [ 106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault code:0x10000 subcode:0x0 func:megasas_transition_to_ready > [ 106.695186] megaraid_sas 0000:03:00.0: System Register set: > [ 106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to ready for scsi0. > [ 106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw 6407 > estuary:/$ > > With the series, now the kernel has direct mapping for the dev as > below, > > estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions > 0x0000000008000000 0x00000000080fffff msi > 0x0000000027b00000 0x00000000286fffff direct > 0x00000000e6400000 0x00000000e64fffff direct > estuary:/$ > > .... > [ 12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009 > [ 12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: 0 max_lds: 32 > [ 12.746628] megaraid_sas 0000:03:00.0: controller type : iMR(0MB) > [ 12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR) : Enabled > [ 12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support : Yes > [ 12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes > [ 12.771931] megaraid_sas 0000:03:00.0: FW provided TM TaskAbort/Reset timeou: 6 secs/60 secs > [ 12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map support : Yes > [ 12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining support : No > [ 12.819179] megaraid_sas 0000:03:00.0: NVME page size : (4096) > [ 12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is called outbound_intr_mask:0x40000000 > [ 12.835199] megaraid_sas 0000:03:00.0: INIT adapter done > [ 12.873932] megaraid_sas 0000:03:00.0: pci id : (0x1000)/(0x0017)/(0x19e5)/(0xd213) > [ 12.881644] megaraid_sas 0000:03:00.0: unevenspan support : no > [ 12.887451] megaraid_sas 0000:03:00.0: firmware crash dump : no > [ 12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map : enabled > > RAID controller init is now success and can detect the drives > attached as well. > > Thanks, > Shameer > > [0]. https://developer.arm.com/documentation/den0049/latest/ > [1]. https://github.com/acpica/acpica/pull/638 > > Shameer Kolothum (8): > ACPICA: IORT: Update for revision E > ACPI/IORT: Add support for RMR node parsing > iommu/dma: Introduce generic helper to retrieve RMR info > ACPI/IORT: Add RMR memory regions reservation helper > iommu/arm-smmu-v3: Introduce strtab init helper > iommu/arm-smmu-v3: Add bypass flag to arm_smmu_write_strtab_ent() > iommu/arm-smmu-v3: Get associated RMR info and install bypass STE > iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev > > drivers/acpi/arm64/iort.c | 182 +++++++++++++++++++- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 112 ++++++++++-- > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 + > drivers/iommu/dma-iommu.c | 39 +++++ > include/acpi/actbl2.h | 25 ++- > include/linux/acpi_iort.h | 6 + > include/linux/dma-iommu.h | 7 + > include/linux/iommu.h | 16 ++ > 8 files changed, 367 insertions(+), 22 deletions(-) >
[+Lorenzo] > -----Original Message----- > From: Auger Eric [mailto:eric.auger@redhat.com] > Sent: 15 April 2021 10:49 > To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>; > linux-arm-kernel@lists.infradead.org; linux-acpi@vger.kernel.org; > iommu@lists.linux-foundation.org; devel@acpica.org > Cc: Linuxarm <linuxarm@huawei.com>; steven.price@arm.com; Guohanjun > (Hanjun Guo) <guohanjun@huawei.com>; Sami.Mujawar@arm.com; > robin.murphy@arm.com; wanghuiqiang <wanghuiqiang@huawei.com>; > Jean-Philippe Brucker <jean-philippe@linaro.org> > Subject: Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node > > Hi Shameer, > > + Jean-Philippe > > > On 11/19/20 1:11 PM, Shameer Kolothum wrote: > > RFC v1 --> v2: > > - Added a generic interface for IOMMU drivers to retrieve all the > > RMR info associated with a given IOMMU. > > - SMMUv3 driver gets the RMR list during probe() and installs > > bypass STEs for all the SIDs in the RMR list. This is to keep > > the ongoing traffic alive(if any) during SMMUv3 reset. This is > > based on the suggestions received for v1 to take care of the > > EFI framebuffer use case. Only sanity tested for now. > > - During the probe/attach device, SMMUv3 driver reserves any > > RMR region associated with the device such that there is a unity > > mapping for them in SMMU. > > --- > > > > The series adds support to IORT RMR nodes specified in IORT > > Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory > > ranges that are used by endpoints and require a unity mapping > > in SMMU. > > > > We have faced issues with 3408iMR RAID controller cards which > > fail to boot when SMMU is enabled. This is because these controllers > > make use of host memory for various caching related purposes and when > > SMMU is enabled the iMR firmware fails to access these memory regions > > as there is no mapping for them. IORT RMR provides a way for UEFI to > > describe and report these memory regions so that the kernel can make > > a unity mapping for these in SMMU. > > > > RFC because, Patch #1 is to update the actbl2.h and should be done > > through acpica update. I have send out a pull request[1] for that. > > > > Tests: > > > > With a UEFI, that reports the RMR for the dev, > > .... > > [16F0h 5872 1] Type : 06 > > [16F1h 5873 2] Length : 007C > > [16F3h 5875 1] Revision : 00 > > [1038h 0056 2] Reserved : 00000000 > > [1038h 0056 2] Identifier : 00000000 > > [16F8h 5880 4] Mapping Count : 00000001 > > [16FCh 5884 4] Mapping Offset : 00000040 > > > > [1700h 5888 4] Number of RMR Descriptors : 00000002 > > [1704h 5892 4] RMR Descriptor Offset : 00000018 > > > > [1708h 5896 8] Base Address of RMR : 0000E6400000 > > [1710h 5904 8] Length of RMR : 000000100000 > > [1718h 5912 4] Reserved : 00000000 > > > > [171Ch 5916 8] Base Address of RMR : 0000000027B00000 > > [1724h 5924 8] Length of RMR : 0000000000C00000 > > [172Ch 5932 4] Reserved : 00000000 > > > > [1730h 5936 4] Input base : 00000000 > > [1734h 5940 4] ID Count : 00000001 > > [1738h 5944 4] Output Base : 00000003 > > [173Ch 5948 4] Output Reference : 00000064 > > [1740h 5952 4] Flags (decoded below) : 00000001 > > Single Mapping : 1 > > Following Jean-Philippe's suggestion I have used your series for nested > stage SMMUv3 integration, ie. to simplify the MSI nested stage mapping. > > Host allocates hIOVA -> physical doorbell (pDB) as it normally does for > VFIO device passthrough. IOVA Range is 0x8000000 - 0x8100000. > > I expose this MIS IOVA range to the guest as an RMR and as a result > guest has a flat mapping for this range. As the physical device is > programmed with hIOVA we have the following mapping: > > IOVA IPA PA > hIOVA -> hIOVA -> pDB > S1 s2 > > This works. > > The only weird thing is that I need to expose 256 RMRs due to the > 'Single Mapping' mandatory flag. I need to have 1 RMR per potential SID > on the bus. Please see the discussion here, https://op-lists.linaro.org/pipermail/linaro-open-discussions/2021-April/000150.html Hi Lorenzo, May be this is a use case for relaxing that single mapping requirement. Thanks, Shameer > > I will post a new version of SMMUv3 nested stage soon for people to test > & compare. Obviously this removes a bunch of code on both SMMU/VFIO and > QEMU code so I think this solution looks better overall. > > Thanks > > Eric > > ... > > > > Without the series the RAID controller initialization fails as > > below, > > > > ... > > [ 12.631117] megaraid_sas 0000:03:00.0: FW supports sync > cache : Yes > > [ 12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is > called outbound_intr_mask:0x40000009 > > [ 18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED > for SCSI host 0 > > [ 23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to > ready state > > [ 106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault > code:0x10000 subcode:0x0 func:megasas_transition_to_ready > > [ 106.695186] megaraid_sas 0000:03:00.0: System Register set: > > [ 106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to > ready for scsi0. > > [ 106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw > 6407 > > estuary:/$ > > > > With the series, now the kernel has direct mapping for the dev as > > below, > > > > estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions > > 0x0000000008000000 0x00000000080fffff msi > > 0x0000000027b00000 0x00000000286fffff direct > > 0x00000000e6400000 0x00000000e64fffff direct > > estuary:/$ > > > > .... > > [ 12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is > called outbound_intr_mask:0x40000009 > > [ 12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: > 0 max_lds: 32 > > [ 12.746628] megaraid_sas 0000:03:00.0: controller type : > iMR(0MB) > > [ 12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR) : > Enabled > > [ 12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support : Yes > > [ 12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes > > [ 12.771931] megaraid_sas 0000:03:00.0: FW provided TM > TaskAbort/Reset timeou: 6 secs/60 secs > > [ 12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map > support : Yes > > [ 12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining > support : No > > [ 12.819179] megaraid_sas 0000:03:00.0: NVME page size : > (4096) > > [ 12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is > called outbound_intr_mask:0x40000000 > > [ 12.835199] megaraid_sas 0000:03:00.0: INIT adapter done > > [ 12.873932] megaraid_sas 0000:03:00.0: pci id : > (0x1000)/(0x0017)/(0x19e5)/(0xd213) > > [ 12.881644] megaraid_sas 0000:03:00.0: unevenspan support : no > > [ 12.887451] megaraid_sas 0000:03:00.0: firmware crash dump : no > > [ 12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map : > enabled > > > > RAID controller init is now success and can detect the drives > > attached as well. > > > > Thanks, > > Shameer > > > > [0]. https://developer.arm.com/documentation/den0049/latest/ > > [1]. https://github.com/acpica/acpica/pull/638 > > > > Shameer Kolothum (8): > > ACPICA: IORT: Update for revision E > > ACPI/IORT: Add support for RMR node parsing > > iommu/dma: Introduce generic helper to retrieve RMR info > > ACPI/IORT: Add RMR memory regions reservation helper > > iommu/arm-smmu-v3: Introduce strtab init helper > > iommu/arm-smmu-v3: Add bypass flag to arm_smmu_write_strtab_ent() > > iommu/arm-smmu-v3: Get associated RMR info and install bypass STE > > iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev > > > > drivers/acpi/arm64/iort.c | 182 > +++++++++++++++++++- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 112 ++++++++++-- > > drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 + > > drivers/iommu/dma-iommu.c | 39 +++++ > > include/acpi/actbl2.h | 25 ++- > > include/linux/acpi_iort.h | 6 + > > include/linux/dma-iommu.h | 7 + > > include/linux/iommu.h | 16 ++ > > 8 files changed, 367 insertions(+), 22 deletions(-) > >
RFC v1 --> v2: - Added a generic interface for IOMMU drivers to retrieve all the RMR info associated with a given IOMMU. - SMMUv3 driver gets the RMR list during probe() and installs bypass STEs for all the SIDs in the RMR list. This is to keep the ongoing traffic alive(if any) during SMMUv3 reset. This is based on the suggestions received for v1 to take care of the EFI framebuffer use case. Only sanity tested for now. - During the probe/attach device, SMMUv3 driver reserves any RMR region associated with the device such that there is a unity mapping for them in SMMU. --- The series adds support to IORT RMR nodes specified in IORT Revision E -ARM DEN 0049E[0]. RMR nodes are used to describe memory ranges that are used by endpoints and require a unity mapping in SMMU. We have faced issues with 3408iMR RAID controller cards which fail to boot when SMMU is enabled. This is because these controllers make use of host memory for various caching related purposes and when SMMU is enabled the iMR firmware fails to access these memory regions as there is no mapping for them. IORT RMR provides a way for UEFI to describe and report these memory regions so that the kernel can make a unity mapping for these in SMMU. RFC because, Patch #1 is to update the actbl2.h and should be done through acpica update. I have send out a pull request[1] for that. Tests: With a UEFI, that reports the RMR for the dev, .... [16F0h 5872 1] Type : 06 [16F1h 5873 2] Length : 007C [16F3h 5875 1] Revision : 00 [1038h 0056 2] Reserved : 00000000 [1038h 0056 2] Identifier : 00000000 [16F8h 5880 4] Mapping Count : 00000001 [16FCh 5884 4] Mapping Offset : 00000040 [1700h 5888 4] Number of RMR Descriptors : 00000002 [1704h 5892 4] RMR Descriptor Offset : 00000018 [1708h 5896 8] Base Address of RMR : 0000E6400000 [1710h 5904 8] Length of RMR : 000000100000 [1718h 5912 4] Reserved : 00000000 [171Ch 5916 8] Base Address of RMR : 0000000027B00000 [1724h 5924 8] Length of RMR : 0000000000C00000 [172Ch 5932 4] Reserved : 00000000 [1730h 5936 4] Input base : 00000000 [1734h 5940 4] ID Count : 00000001 [1738h 5944 4] Output Base : 00000003 [173Ch 5948 4] Output Reference : 00000064 [1740h 5952 4] Flags (decoded below) : 00000001 Single Mapping : 1 ... Without the series the RAID controller initialization fails as below, ... [ 12.631117] megaraid_sas 0000:03:00.0: FW supports sync cache : Yes [ 12.637360] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009 [ 18.776377] megaraid_sas 0000:03:00.0: Init cmd return status FAILED for SCSI host 0 [ 23.019383] megaraid_sas 0000:03:00.0: Waiting for FW to come to ready state [ 106.684281] megaraid_sas 0000:03:00.0: FW in FAULT state, Fault code:0x10000 subcode:0x0 func:megasas_transition_to_ready [ 106.695186] megaraid_sas 0000:03:00.0: System Register set: [ 106.889787] megaraid_sas 0000:03:00.0: Failed to transition controller to ready for scsi0. [ 106.910475] megaraid_sas 0000:03:00.0: Failed from megasas_init_fw 6407 estuary:/$ With the series, now the kernel has direct mapping for the dev as below, estuary:/$ cat /sys/kernel/iommu_groups/0/reserved_regions 0x0000000008000000 0x00000000080fffff msi 0x0000000027b00000 0x00000000286fffff direct 0x00000000e6400000 0x00000000e64fffff direct estuary:/$ .... [ 12.254318] megaraid_sas 0000:03:00.0: megasas_disable_intr_fusion is called outbound_intr_mask:0x40000009 [ 12.739089] megaraid_sas 0000:03:00.0: FW provided supportMaxExtLDs: 0 max_lds: 32 [ 12.746628] megaraid_sas 0000:03:00.0: controller type : iMR(0MB) [ 12.752694] megaraid_sas 0000:03:00.0: Online Controller Reset(OCR) : Enabled [ 12.759798] megaraid_sas 0000:03:00.0: Secure JBOD support : Yes [ 12.765778] megaraid_sas 0000:03:00.0: NVMe passthru support : Yes [ 12.771931] megaraid_sas 0000:03:00.0: FW provided TM TaskAbort/Reset timeou: 6 secs/60 secs [ 12.780503] megaraid_sas 0000:03:00.0: JBOD sequence map support : Yes [ 12.787000] megaraid_sas 0000:03:00.0: PCI Lane Margining support : No [ 12.819179] megaraid_sas 0000:03:00.0: NVME page size : (4096) [ 12.825672] megaraid_sas 0000:03:00.0: megasas_enable_intr_fusion is called outbound_intr_mask:0x40000000 [ 12.835199] megaraid_sas 0000:03:00.0: INIT adapter done [ 12.873932] megaraid_sas 0000:03:00.0: pci id : (0x1000)/(0x0017)/(0x19e5)/(0xd213) [ 12.881644] megaraid_sas 0000:03:00.0: unevenspan support : no [ 12.887451] megaraid_sas 0000:03:00.0: firmware crash dump : no [ 12.893344] megaraid_sas 0000:03:00.0: JBOD sequence map : enabled RAID controller init is now success and can detect the drives attached as well. Thanks, Shameer [0]. https://developer.arm.com/documentation/den0049/latest/ [1]. https://github.com/acpica/acpica/pull/638 Shameer Kolothum (8): ACPICA: IORT: Update for revision E ACPI/IORT: Add support for RMR node parsing iommu/dma: Introduce generic helper to retrieve RMR info ACPI/IORT: Add RMR memory regions reservation helper iommu/arm-smmu-v3: Introduce strtab init helper iommu/arm-smmu-v3: Add bypass flag to arm_smmu_write_strtab_ent() iommu/arm-smmu-v3: Get associated RMR info and install bypass STE iommu/arm-smmu-v3: Reserve any RMR regions associated with a dev drivers/acpi/arm64/iort.c | 182 +++++++++++++++++++- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 112 ++++++++++-- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 2 + drivers/iommu/dma-iommu.c | 39 +++++ include/acpi/actbl2.h | 25 ++- include/linux/acpi_iort.h | 6 + include/linux/dma-iommu.h | 7 + include/linux/iommu.h | 16 ++ 8 files changed, 367 insertions(+), 22 deletions(-) -- 2.17.1