[edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Sheng Wei]

PciIoMap () need to feedback the status of
mIoMmuProtocol->SetAttribute () return value.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652

Cc: Ray Ni <ray.ni@intel.com>
Cc: Huang Jenny <jenny.huang@intel.com>
Cc: Chiang Chris <chris.chiang@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 14bed54729..e85544d08d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1024,12 +1024,12 @@ PciIoMap (
           return EFI_INVALID_PARAMETER;
       }
 
-      mIoMmuProtocol->SetAttribute (
-                        mIoMmuProtocol,
-                        PciIoDevice->Handle,
-                        *Mapping,
-                        IoMmuAttribute
-                        );
+      Status = mIoMmuProtocol->SetAttribute (
+                                 mIoMmuProtocol,
+                                 PciIoDevice->Handle,
+                                 *Mapping,
+                                 IoMmuAttribute
+                                 );
     }
   }
 
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114136): https://edk2.groups.io/g/devel/message/114136
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Huang, Jenny]

Reviewed by Jenny Huang <jenny.huang@intel.com>

-----Original Message-----
From: Sheng, W <w.sheng@intel.com> 
Sent: Sunday, January 21, 2024 10:47 PM
To: devel@edk2.groups.io
Cc: Ni, Ray <ray.ni@intel.com>; Huang, Jenny <jenny.huang@intel.com>; Chiang, Chris <chris.chiang@intel.com>
Subject: [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

PciIoMap () need to feedback the status of
mIoMmuProtocol->SetAttribute () return value.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652

Cc: Ray Ni <ray.ni@intel.com>
Cc: Huang Jenny <jenny.huang@intel.com>
Cc: Chiang Chris <chris.chiang@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 14bed54729..e85544d08d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1024,12 +1024,12 @@ PciIoMap (
           return EFI_INVALID_PARAMETER;
       }
 
-      mIoMmuProtocol->SetAttribute (
-                        mIoMmuProtocol,
-                        PciIoDevice->Handle,
-                        *Mapping,
-                        IoMmuAttribute
-                        );
+      Status = mIoMmuProtocol->SetAttribute (
+                                 mIoMmuProtocol,
+                                 PciIoDevice->Handle,
+                                 *Mapping,
+                                 IoMmuAttribute
+                                 );
     }
   }
 
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114167): https://edk2.groups.io/g/devel/message/114167
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Ni, Ray]

Reviewed-by: Ray Ni <ray.ni@intel.com>

Thanks,
Ray
> -----Original Message-----
> From: Huang, Jenny <jenny.huang@intel.com>
> Sent: Tuesday, January 23, 2024 11:26 AM
> To: Sheng, W <w.sheng@intel.com>; devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chiang, Chris <chris.chiang@intel.com>
> Subject: RE: [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for
> PciIoMap
> 
> Reviewed by Jenny Huang <jenny.huang@intel.com>
> 
> -----Original Message-----
> From: Sheng, W <w.sheng@intel.com>
> Sent: Sunday, January 21, 2024 10:47 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Huang, Jenny <jenny.huang@intel.com>;
> Chiang, Chris <chris.chiang@intel.com>
> Subject: [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for
> PciIoMap
> 
> PciIoMap () need to feedback the status of
> mIoMmuProtocol->SetAttribute () return value.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Huang Jenny <jenny.huang@intel.com>
> Cc: Chiang Chris <chris.chiang@intel.com>
> Signed-off-by: Sheng Wei <w.sheng@intel.com>
> ---
>  MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> index 14bed54729..e85544d08d 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -1024,12 +1024,12 @@ PciIoMap (
>            return EFI_INVALID_PARAMETER;
>        }
> 
> -      mIoMmuProtocol->SetAttribute (
> -                        mIoMmuProtocol,
> -                        PciIoDevice->Handle,
> -                        *Mapping,
> -                        IoMmuAttribute
> -                        );
> +      Status = mIoMmuProtocol->SetAttribute (
> +                                 mIoMmuProtocol,
> +                                 PciIoDevice->Handle,
> +                                 *Mapping,
> +                                 IoMmuAttribute
> +                                 );
>      }
>    }
> 
> --
> 2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114282): https://edk2.groups.io/g/devel/message/114282
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Lendacky, Thomas via groups.io]

On 1/22/24 00:47, Sheng Wei via groups.io wrote:
> PciIoMap () need to feedback the status of
> mIoMmuProtocol->SetAttribute () return value.
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652

I'm still investigating, but this commit breaks booting all types of SEV 
guests. Without this patch, there is a boot device mapping and the Grub 
menu is displayed. But with this patch, I receive:

map: No mapping found.
Press ESC in 1 seconds to skip startup.nsh or any other key to continue.

and then drop to the shell prompt.

Thanks,
Tom

> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Huang Jenny <jenny.huang@intel.com>
> Cc: Chiang Chris <chris.chiang@intel.com>
> Signed-off-by: Sheng Wei <w.sheng@intel.com>
> ---
>   MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> index 14bed54729..e85544d08d 100644
> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
> @@ -1024,12 +1024,12 @@ PciIoMap (
>             return EFI_INVALID_PARAMETER;
>         }
>   
> -      mIoMmuProtocol->SetAttribute (
> -                        mIoMmuProtocol,
> -                        PciIoDevice->Handle,
> -                        *Mapping,
> -                        IoMmuAttribute
> -                        );
> +      Status = mIoMmuProtocol->SetAttribute (
> +                                 mIoMmuProtocol,
> +                                 PciIoDevice->Handle,
> +                                 *Mapping,
> +                                 IoMmuAttribute
> +                                 );
>       }
>     }
>   


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114621): https://edk2.groups.io/g/devel/message/114621
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Lendacky, Thomas via groups.io]

+Min

Adding Min to see if TDX is also experiencing issues around this recent 
change.

Thanks,
Tom

On 1/26/24 11:21, Tom Lendacky wrote:
> On 1/22/24 00:47, Sheng Wei via groups.io wrote:
>> PciIoMap () need to feedback the status of
>> mIoMmuProtocol->SetAttribute () return value.
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
> 
> I'm still investigating, but this commit breaks booting all types of SEV 
> guests. Without this patch, there is a boot device mapping and the Grub 
> menu is displayed. But with this patch, I receive:
> 
> map: No mapping found.
> Press ESC in 1 seconds to skip startup.nsh or any other key to continue.
> 
> and then drop to the shell prompt.
> 
> Thanks,
> Tom
> 
>>
>> Cc: Ray Ni <ray.ni@intel.com>
>> Cc: Huang Jenny <jenny.huang@intel.com>
>> Cc: Chiang Chris <chris.chiang@intel.com>
>> Signed-off-by: Sheng Wei <w.sheng@intel.com>
>> ---
>>   MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>
>> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
>> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
>> index 14bed54729..e85544d08d 100644
>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
>> @@ -1024,12 +1024,12 @@ PciIoMap (
>>             return EFI_INVALID_PARAMETER;
>>         }
>> -      mIoMmuProtocol->SetAttribute (
>> -                        mIoMmuProtocol,
>> -                        PciIoDevice->Handle,
>> -                        *Mapping,
>> -                        IoMmuAttribute
>> -                        );
>> +      Status = mIoMmuProtocol->SetAttribute (
>> +                                 mIoMmuProtocol,
>> +                                 PciIoDevice->Handle,
>> +                                 *Mapping,
>> +                                 IoMmuAttribute
>> +                                 );
>>       }
>>     }


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114622): https://edk2.groups.io/g/devel/message/114622
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Lendacky, Thomas via groups.io]

On 1/26/24 11:38, Tom Lendacky wrote:
> +Min
> 
> Adding Min to see if TDX is also experiencing issues around this recent 
> change.
> 
> Thanks,
> Tom
> 
> On 1/26/24 11:21, Tom Lendacky wrote:
>> On 1/22/24 00:47, Sheng Wei via groups.io wrote:
>>> PciIoMap () need to feedback the status of
>>> mIoMmuProtocol->SetAttribute () return value.
>>>
>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
>>
>> I'm still investigating, but this commit breaks booting all types of SEV 
>> guests. Without this patch, there is a boot device mapping and the Grub 
>> menu is displayed. But with this patch, I receive:
>>
>> map: No mapping found.
>> Press ESC in 1 seconds to skip startup.nsh or any other key to continue.
>>
>> and then drop to the shell prompt.

The IOMMU protocol is installed under OVMF when either SEV or TDX is 
active. The SetAttribute() function of this implementation has always 
returned EFI_UNSUPPORTED, which is now being passed pack to the caller of 
PciIoMap() and thus causing a failure.

Should the SetAttribute() function in OvmfPkg/IoMmuDxe/CcIoMmu.c return 
success by default?

Thanks,
Tom

>>
>> Thanks,
>> Tom
>>
>>>
>>> Cc: Ray Ni <ray.ni@intel.com>
>>> Cc: Huang Jenny <jenny.huang@intel.com>
>>> Cc: Chiang Chris <chris.chiang@intel.com>
>>> Signed-off-by: Sheng Wei <w.sheng@intel.com>
>>> ---
>>>   MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
>>>   1 file changed, 6 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c 
>>> b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
>>> index 14bed54729..e85544d08d 100644
>>> --- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
>>> +++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
>>> @@ -1024,12 +1024,12 @@ PciIoMap (
>>>             return EFI_INVALID_PARAMETER;
>>>         }
>>> -      mIoMmuProtocol->SetAttribute (
>>> -                        mIoMmuProtocol,
>>> -                        PciIoDevice->Handle,
>>> -                        *Mapping,
>>> -                        IoMmuAttribute
>>> -                        );
>>> +      Status = mIoMmuProtocol->SetAttribute (
>>> +                                 mIoMmuProtocol,
>>> +                                 PciIoDevice->Handle,
>>> +                                 *Mapping,
>>> +                                 IoMmuAttribute
>>> +                                 );
>>>       }
>>>     }


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114623): https://edk2.groups.io/g/devel/message/114623
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Min Xu]

On Saturday, January 27, 2024 1:56 AM, Tom Lendacky wrote:
> On 1/26/24 11:38, Tom Lendacky wrote:
> > +Min
> >
> > Adding Min to see if TDX is also experiencing issues around this
> > recent change.
It impacts TDX as well.
> >
> > On 1/26/24 11:21, Tom Lendacky wrote:
> >> On 1/22/24 00:47, Sheng Wei via groups.io wrote:
> >>> PciIoMap () need to feedback the status of
> >>> mIoMmuProtocol->SetAttribute () return value.
> >>>
> >>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
> >>
> >> I'm still investigating, but this commit breaks booting all types of
> >> SEV guests. Without this patch, there is a boot device mapping and
> >> the Grub menu is displayed. But with this patch, I receive:
> >>
> >> map: No mapping found.
> >> Press ESC in 1 seconds to skip startup.nsh or any other key to continue.
> >>
> >> and then drop to the shell prompt.
> 
> The IOMMU protocol is installed under OVMF when either SEV or TDX is
> active. The SetAttribute() function of this implementation has always
> returned EFI_UNSUPPORTED, which is now being passed pack to the caller of
> PciIoMap() and thus causing a failure.
> 
> Should the SetAttribute() function in OvmfPkg/IoMmuDxe/CcIoMmu.c return
> success by default?
I am good to return success in SetAttribute() function.

Thanks
Min


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114675): https://edk2.groups.io/g/devel/message/114675
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Laszlo Ersek]

On 1/26/24 18:56, Lendacky, Thomas via groups.io wrote:
> On 1/26/24 11:38, Tom Lendacky wrote:
>> +Min
>>
>> Adding Min to see if TDX is also experiencing issues around this
>> recent change.
>>
>> Thanks,
>> Tom
>>
>> On 1/26/24 11:21, Tom Lendacky wrote:
>>> On 1/22/24 00:47, Sheng Wei via groups.io wrote:
>>>> PciIoMap () need to feedback the status of
>>>> mIoMmuProtocol->SetAttribute () return value.
>>>>
>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
>>>
>>> I'm still investigating, but this commit breaks booting all types of
>>> SEV guests. Without this patch, there is a boot device mapping and
>>> the Grub menu is displayed. But with this patch, I receive:
>>>
>>> map: No mapping found.
>>> Press ESC in 1 seconds to skip startup.nsh or any other key to continue.
>>>
>>> and then drop to the shell prompt.
> 
> The IOMMU protocol is installed under OVMF when either SEV or TDX is
> active. The SetAttribute() function of this implementation has always
> returned EFI_UNSUPPORTED, which is now being passed pack to the caller
> of PciIoMap() and thus causing a failure.
> 
> Should the SetAttribute() function in OvmfPkg/IoMmuDxe/CcIoMmu.c return
> success by default?

I don't understand why the EDKII_IOMMU_PROTOCOL.SetAttribute() member
function exists in the first place. The documentation on
EDKII_IOMMU_SET_ATTRIBUTE says that having specified BusMasterRead
earlier as Operation for Map() corresponds to EDKII_IOMMU_ACCESS_READ,
BusMasterWrite to EDKII_IOMMU_ACCESS_WRITE, and BusMasterCommonBuffer to
EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE. This makes no sense to
me because (a) these settings carry zero new information related to the
earlier Map() call, and (b) the Map() call will have set up the
IOMMU/memory config already, based on Operation.

In PciIoMap() [MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c], there is first a
call to RootBridgeIoMap()
[MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c], which in turn
calls Map() on the IOMMU protocol. Thus, I don't see why PciBusDxe has
to do anything extra here, in the first place. I wouldn't call
EDKII_IOMMU_PROTOCOL.SetAttribute(); what's more, I'd argue that
EDKII_IOMMU_PROTOCOL.SetAttribute() itself is mostly useless. (Not sure
about "ACPI" devices; i.e. not PCI(e)).

Yet more confusion is that in the documentation of
EDKII_IOMMU_SET_ATTRIBUTE [MdeModulePkg/Include/Protocol/IoMmu.h], the
EFI_SUCCESS return status is described as "The IoMmuAccess is set for
the memory range specified by DeviceAddress and Length" -- but there
aren't even parameters called "DeviceAddress" and "Length"!

Then *even more* problems: EFI_INVALID_PARAMETER is supposed to be
returned for DeviceHandle being invalid, EFI_UNSUPPORTED is supposed to
be reutrned for DeviceHandle being unknown. So not only does the IOMMU
driver have to recognize handles it doesn't know about, it even has to
*distinguish* "plain unknown" from "invalid". That makes no sense at
all. The IOMMU protocol has no use for a DeviceHandle anywhere else in
the first place.

Either way, the right thing to do in OvmfPkg/IoMmuDxe should be:

- in IoMmuMap() [OvmfPkg/IoMmuDxe/CcIoMmu.c], we already save the
requested operation in MapInfo->Operation (note that this is not
EFI_PCI_IO_PROTOCOL_OPERATION, but EDKII_IOMMU_OPERATION: the former is
mapped to the latter in the root bridge protocol impl., IIRC, but the
latter also encodes 32-bit vs. 64-bit in the enum!)

- therefore in IoMmuSetAttribute() [OvmfPkg/IoMmuDxe/CcIoMmu.c], we
should just ignore DeviceHandle, cast Mapping back to MAP_INFO (we could
make an attempt to look it up first in "mMapInfos", but that's a total
waste of time: just don't pass in crap, and then you won't get undefined
behavior), and then compare MapInfo->Operation against IoMmuAccess.

For operations Read and Read64, accept EDKII_IOMMU_ACCESS_READ.

For operations Write and Write64, accept EDKII_IOMMU_ACCESS_WRITE.

For operations CommonBuffer and CommonBuffer64, accept
EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE.

Always accept 0 as IoMmuAccess, too [*].

"Accept" means do nothing, and return EFI_SUCCESS.

Reject anything else with EFI_INVALID_PARAMETER or EFI_UNSUPPORTED.

This would at least allow for a superficial consistency check, without
(a) incurring large performance penalty (such as list walking) and (b)
turning the SetAttribute() member function into a total joke. (Really,
IMO it should not even exist at the protocol specification level!)

[*] OK, you might want to know why we should accept 0 too. Here's why:
we have *another* mIoMmuProtocol->SetAttribute() call, namely in
PciIoUnmap() [MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c]. It passes in
IoMmuAccess=0.

Here's the sad bit: while this patch -- now commit 049695a0b1e2 --
propagates the return status of mIoMmuProtocol->SetAttribute() out of
PciIoMap(), the patch doesn't do the same for the other such call in
PciIoUnmap(). That seems like a bug (omission) itself, but once that one
gets fixed, we'll need to permit IoMmuAccess=0 too in OVMF's IOMMU driver.

I guess I could live with OVMF's IoMmuMap () doing nothing,
successfully, rather than doing nothing, unsuccessfully, *if*
EDKII_IOMMU_PROTOCOL.SetAttribute() had some public documentation rooted
in reality, *and* if PciBusDxe called that function with proper error
checking everywhere. Otherwise it's just another terrible, mis-specified
API, that platforms must route around.

Wow I didn't expect to be worked up this much about it, but here I am. I
dunno, seeing crap interfaces just makes me unreasonably irate.

Laszlo



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114718): https://edk2.groups.io/g/devel/message/114718
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Lendacky, Thomas via groups.io]

On 1/29/24 11:20, Laszlo Ersek wrote:
> On 1/26/24 18:56, Lendacky, Thomas via groups.io wrote:
>> On 1/26/24 11:38, Tom Lendacky wrote:
>>> +Min
>>>
>>> Adding Min to see if TDX is also experiencing issues around this
>>> recent change.
>>>
>>> Thanks,
>>> Tom
>>>
>>> On 1/26/24 11:21, Tom Lendacky wrote:
>>>> On 1/22/24 00:47, Sheng Wei via groups.io wrote:
>>>>> PciIoMap () need to feedback the status of
>>>>> mIoMmuProtocol->SetAttribute () return value.
>>>>>
>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
>>>>
>>>> I'm still investigating, but this commit breaks booting all types of
>>>> SEV guests. Without this patch, there is a boot device mapping and
>>>> the Grub menu is displayed. But with this patch, I receive:
>>>>
>>>> map: No mapping found.
>>>> Press ESC in 1 seconds to skip startup.nsh or any other key to continue.
>>>>
>>>> and then drop to the shell prompt.
>>
>> The IOMMU protocol is installed under OVMF when either SEV or TDX is
>> active. The SetAttribute() function of this implementation has always
>> returned EFI_UNSUPPORTED, which is now being passed pack to the caller
>> of PciIoMap() and thus causing a failure.
>>
>> Should the SetAttribute() function in OvmfPkg/IoMmuDxe/CcIoMmu.c return
>> success by default?
> 
> I don't understand why the EDKII_IOMMU_PROTOCOL.SetAttribute() member
> function exists in the first place. The documentation on
> EDKII_IOMMU_SET_ATTRIBUTE says that having specified BusMasterRead
> earlier as Operation for Map() corresponds to EDKII_IOMMU_ACCESS_READ,
> BusMasterWrite to EDKII_IOMMU_ACCESS_WRITE, and BusMasterCommonBuffer to
> EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE. This makes no sense to
> me because (a) these settings carry zero new information related to the
> earlier Map() call, and (b) the Map() call will have set up the
> IOMMU/memory config already, based on Operation.
> 
> In PciIoMap() [MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c], there is first a
> call to RootBridgeIoMap()
> [MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c], which in turn
> calls Map() on the IOMMU protocol. Thus, I don't see why PciBusDxe has
> to do anything extra here, in the first place. I wouldn't call
> EDKII_IOMMU_PROTOCOL.SetAttribute(); what's more, I'd argue that
> EDKII_IOMMU_PROTOCOL.SetAttribute() itself is mostly useless. (Not sure
> about "ACPI" devices; i.e. not PCI(e)).
> 
> Yet more confusion is that in the documentation of
> EDKII_IOMMU_SET_ATTRIBUTE [MdeModulePkg/Include/Protocol/IoMmu.h], the
> EFI_SUCCESS return status is described as "The IoMmuAccess is set for
> the memory range specified by DeviceAddress and Length" -- but there
> aren't even parameters called "DeviceAddress" and "Length"!
> 
> Then *even more* problems: EFI_INVALID_PARAMETER is supposed to be
> returned for DeviceHandle being invalid, EFI_UNSUPPORTED is supposed to
> be reutrned for DeviceHandle being unknown. So not only does the IOMMU
> driver have to recognize handles it doesn't know about, it even has to
> *distinguish* "plain unknown" from "invalid". That makes no sense at
> all. The IOMMU protocol has no use for a DeviceHandle anywhere else in
> the first place.
> 
> Either way, the right thing to do in OvmfPkg/IoMmuDxe should be:
> 
> - in IoMmuMap() [OvmfPkg/IoMmuDxe/CcIoMmu.c], we already save the
> requested operation in MapInfo->Operation (note that this is not
> EFI_PCI_IO_PROTOCOL_OPERATION, but EDKII_IOMMU_OPERATION: the former is
> mapped to the latter in the root bridge protocol impl., IIRC, but the
> latter also encodes 32-bit vs. 64-bit in the enum!)
> 
> - therefore in IoMmuSetAttribute() [OvmfPkg/IoMmuDxe/CcIoMmu.c], we
> should just ignore DeviceHandle, cast Mapping back to MAP_INFO (we could
> make an attempt to look it up first in "mMapInfos", but that's a total
> waste of time: just don't pass in crap, and then you won't get undefined
> behavior), and then compare MapInfo->Operation against IoMmuAccess.
> 
> For operations Read and Read64, accept EDKII_IOMMU_ACCESS_READ.
> 
> For operations Write and Write64, accept EDKII_IOMMU_ACCESS_WRITE.
> 
> For operations CommonBuffer and CommonBuffer64, accept
> EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE.
> 
> Always accept 0 as IoMmuAccess, too [*].
> 
> "Accept" means do nothing, and return EFI_SUCCESS.
> 
> Reject anything else with EFI_INVALID_PARAMETER or EFI_UNSUPPORTED.

Laszlo, if you mean something like this, I'll submit it as a proper patch:


OvmfPkg/IoMmuDxe: Provide an implementation for SetAttribute

From: Tom Lendacky <thomas.lendacky@amd.com>

A recent change to the PciIoMap() function now propagates the return code
from the IoMmu protocol SetAttribute() operation. The implementation of
this operation in OvmfPkg/IoMmuDxe/CcIoMmu.c returns EFI_UNSUPPORTED,
resulting in a failure to boot the guest.

Provide an implementation for SetAttribute() that validates that the IoMmu
access method being requested against the IoMmu mapping operation.

Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
  OvmfPkg/IoMmuDxe/CcIoMmu.c |   50 +++++++++++++++++++++++++++++++++++++++++++-
  1 file changed, 49 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/IoMmuDxe/CcIoMmu.c b/OvmfPkg/IoMmuDxe/CcIoMmu.c
index b83a9690062b..8bd95bfc6b90 100644
--- a/OvmfPkg/IoMmuDxe/CcIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/CcIoMmu.c
@@ -751,7 +751,55 @@ IoMmuSetAttribute (
    IN UINT64                IoMmuAccess
    )
  {
-  return EFI_UNSUPPORTED;
+  MAP_INFO    *MapInfo;
+  EFI_STATUS  Status;
+
+  DEBUG ((DEBUG_VERBOSE, "%a: Mapping=0x%p Access=%ld\n", __func__, Mapping, IoMmuAccess));
+
+  if (Mapping == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  Status = EFI_SUCCESS;
+
+  //
+  // An IoMmuAccess value of 0 is always accepted, validate any non-zero value.
+  //
+  if (IoMmuAccess != 0) {
+    MapInfo = (MAP_INFO *)Mapping;
+
+    //
+    // The mapping operation already implied the access mode. Validate that
+    // the supplied access mode matches operation access mode.
+    //
+    switch (MapInfo->Operation) {
+      case EdkiiIoMmuOperationBusMasterRead:
+      case EdkiiIoMmuOperationBusMasterRead64:
+        if (IoMmuAccess != EDKII_IOMMU_ACCESS_READ) {
+          Status = EFI_INVALID_PARAMETER;
+        }
+        break;
+
+      case EdkiiIoMmuOperationBusMasterWrite:
+      case EdkiiIoMmuOperationBusMasterWrite64:
+        if (IoMmuAccess != EDKII_IOMMU_ACCESS_WRITE) {
+          Status = EFI_INVALID_PARAMETER;
+        }
+        break;
+
+      case EdkiiIoMmuOperationBusMasterCommonBuffer:
+      case EdkiiIoMmuOperationBusMasterCommonBuffer64:
+        if (IoMmuAccess != (EDKII_IOMMU_ACCESS_READ | EDKII_IOMMU_ACCESS_WRITE)) {
+          Status = EFI_INVALID_PARAMETER;
+        }
+        break;
+
+      default:
+        Status = EFI_UNSUPPORTED;
+    }
+  }
+
+  return Status;
  }
  
  EDKII_IOMMU_PROTOCOL  mIoMmu = {


> 
> This would at least allow for a superficial consistency check, without
> (a) incurring large performance penalty (such as list walking) and (b)
> turning the SetAttribute() member function into a total joke. (Really,
> IMO it should not even exist at the protocol specification level!)
> 
> [*] OK, you might want to know why we should accept 0 too. Here's why:
> we have *another* mIoMmuProtocol->SetAttribute() call, namely in
> PciIoUnmap() [MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c]. It passes in
> IoMmuAccess=0.
> 
> Here's the sad bit: while this patch -- now commit 049695a0b1e2 --
> propagates the return status of mIoMmuProtocol->SetAttribute() out of
> PciIoMap(), the patch doesn't do the same for the other such call in
> PciIoUnmap(). That seems like a bug (omission) itself, but once that one
> gets fixed, we'll need to permit IoMmuAccess=0 too in OVMF's IOMMU driver.
> 
> I guess I could live with OVMF's IoMmuMap () doing nothing,
> successfully, rather than doing nothing, unsuccessfully, *if*
> EDKII_IOMMU_PROTOCOL.SetAttribute() had some public documentation rooted
> in reality, *and* if PciBusDxe called that function with proper error
> checking everywhere. Otherwise it's just another terrible, mis-specified
> API, that platforms must route around.
> 
> Wow I didn't expect to be worked up this much about it, but here I am. I
> dunno, seeing crap interfaces just makes me unreasonably irate.
> 
> Laszlo
> 


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114738): https://edk2.groups.io/g/devel/message/114738
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Laszlo Ersek]

On 1/29/24 20:30, Tom Lendacky wrote:
> On 1/29/24 11:20, Laszlo Ersek wrote:
>> On 1/26/24 18:56, Lendacky, Thomas via groups.io wrote:
>>> On 1/26/24 11:38, Tom Lendacky wrote:
>>>> +Min
>>>>
>>>> Adding Min to see if TDX is also experiencing issues around this
>>>> recent change.
>>>>
>>>> Thanks,
>>>> Tom
>>>>
>>>> On 1/26/24 11:21, Tom Lendacky wrote:
>>>>> On 1/22/24 00:47, Sheng Wei via groups.io wrote:
>>>>>> PciIoMap () need to feedback the status of
>>>>>> mIoMmuProtocol->SetAttribute () return value.
>>>>>>
>>>>>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652
>>>>>
>>>>> I'm still investigating, but this commit breaks booting all types of
>>>>> SEV guests. Without this patch, there is a boot device mapping and
>>>>> the Grub menu is displayed. But with this patch, I receive:
>>>>>
>>>>> map: No mapping found.
>>>>> Press ESC in 1 seconds to skip startup.nsh or any other key to
>>>>> continue.
>>>>>
>>>>> and then drop to the shell prompt.
>>>
>>> The IOMMU protocol is installed under OVMF when either SEV or TDX is
>>> active. The SetAttribute() function of this implementation has always
>>> returned EFI_UNSUPPORTED, which is now being passed pack to the caller
>>> of PciIoMap() and thus causing a failure.
>>>
>>> Should the SetAttribute() function in OvmfPkg/IoMmuDxe/CcIoMmu.c return
>>> success by default?
>>
>> I don't understand why the EDKII_IOMMU_PROTOCOL.SetAttribute() member
>> function exists in the first place. The documentation on
>> EDKII_IOMMU_SET_ATTRIBUTE says that having specified BusMasterRead
>> earlier as Operation for Map() corresponds to EDKII_IOMMU_ACCESS_READ,
>> BusMasterWrite to EDKII_IOMMU_ACCESS_WRITE, and BusMasterCommonBuffer to
>> EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE. This makes no sense to
>> me because (a) these settings carry zero new information related to the
>> earlier Map() call, and (b) the Map() call will have set up the
>> IOMMU/memory config already, based on Operation.
>>
>> In PciIoMap() [MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c], there is first a
>> call to RootBridgeIoMap()
>> [MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciRootBridgeIo.c], which in turn
>> calls Map() on the IOMMU protocol. Thus, I don't see why PciBusDxe has
>> to do anything extra here, in the first place. I wouldn't call
>> EDKII_IOMMU_PROTOCOL.SetAttribute(); what's more, I'd argue that
>> EDKII_IOMMU_PROTOCOL.SetAttribute() itself is mostly useless. (Not sure
>> about "ACPI" devices; i.e. not PCI(e)).
>>
>> Yet more confusion is that in the documentation of
>> EDKII_IOMMU_SET_ATTRIBUTE [MdeModulePkg/Include/Protocol/IoMmu.h], the
>> EFI_SUCCESS return status is described as "The IoMmuAccess is set for
>> the memory range specified by DeviceAddress and Length" -- but there
>> aren't even parameters called "DeviceAddress" and "Length"!
>>
>> Then *even more* problems: EFI_INVALID_PARAMETER is supposed to be
>> returned for DeviceHandle being invalid, EFI_UNSUPPORTED is supposed to
>> be reutrned for DeviceHandle being unknown. So not only does the IOMMU
>> driver have to recognize handles it doesn't know about, it even has to
>> *distinguish* "plain unknown" from "invalid". That makes no sense at
>> all. The IOMMU protocol has no use for a DeviceHandle anywhere else in
>> the first place.
>>
>> Either way, the right thing to do in OvmfPkg/IoMmuDxe should be:
>>
>> - in IoMmuMap() [OvmfPkg/IoMmuDxe/CcIoMmu.c], we already save the
>> requested operation in MapInfo->Operation (note that this is not
>> EFI_PCI_IO_PROTOCOL_OPERATION, but EDKII_IOMMU_OPERATION: the former is
>> mapped to the latter in the root bridge protocol impl., IIRC, but the
>> latter also encodes 32-bit vs. 64-bit in the enum!)
>>
>> - therefore in IoMmuSetAttribute() [OvmfPkg/IoMmuDxe/CcIoMmu.c], we
>> should just ignore DeviceHandle, cast Mapping back to MAP_INFO (we could
>> make an attempt to look it up first in "mMapInfos", but that's a total
>> waste of time: just don't pass in crap, and then you won't get undefined
>> behavior), and then compare MapInfo->Operation against IoMmuAccess.
>>
>> For operations Read and Read64, accept EDKII_IOMMU_ACCESS_READ.
>>
>> For operations Write and Write64, accept EDKII_IOMMU_ACCESS_WRITE.
>>
>> For operations CommonBuffer and CommonBuffer64, accept
>> EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE.
>>
>> Always accept 0 as IoMmuAccess, too [*].
>>
>> "Accept" means do nothing, and return EFI_SUCCESS.
>>
>> Reject anything else with EFI_INVALID_PARAMETER or EFI_UNSUPPORTED.
> 
> Laszlo, if you mean something like this, I'll submit it as a proper patch:
> 
> 
> OvmfPkg/IoMmuDxe: Provide an implementation for SetAttribute
> 
> From: Tom Lendacky <thomas.lendacky@amd.com>
> 
> A recent change to the PciIoMap() function now propagates the return code
> from the IoMmu protocol SetAttribute() operation. The implementation of
> this operation in OvmfPkg/IoMmuDxe/CcIoMmu.c returns EFI_UNSUPPORTED,
> resulting in a failure to boot the guest.
> 
> Provide an implementation for SetAttribute() that validates that the IoMmu
> access method being requested against the IoMmu mapping operation.
> 
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  OvmfPkg/IoMmuDxe/CcIoMmu.c |   50
> +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 49 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/IoMmuDxe/CcIoMmu.c b/OvmfPkg/IoMmuDxe/CcIoMmu.c
> index b83a9690062b..8bd95bfc6b90 100644
> --- a/OvmfPkg/IoMmuDxe/CcIoMmu.c
> +++ b/OvmfPkg/IoMmuDxe/CcIoMmu.c
> @@ -751,7 +751,55 @@ IoMmuSetAttribute (
>    IN UINT64                IoMmuAccess
>    )
>  {
> -  return EFI_UNSUPPORTED;
> +  MAP_INFO    *MapInfo;
> +  EFI_STATUS  Status;
> +
> +  DEBUG ((DEBUG_VERBOSE, "%a: Mapping=0x%p Access=%ld\n", __func__,
> Mapping, IoMmuAccess));

(1) %lu is better for formatting UINT64 (%ld is for INT64)

> +
> +  if (Mapping == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = EFI_SUCCESS;
> +
> +  //
> +  // An IoMmuAccess value of 0 is always accepted, validate any
> non-zero value.
> +  //
> +  if (IoMmuAccess != 0) {
> +    MapInfo = (MAP_INFO *)Mapping;
> +
> +    //
> +    // The mapping operation already implied the access mode. Validate
> that
> +    // the supplied access mode matches operation access mode.
> +    //
> +    switch (MapInfo->Operation) {
> +      case EdkiiIoMmuOperationBusMasterRead:
> +      case EdkiiIoMmuOperationBusMasterRead64:
> +        if (IoMmuAccess != EDKII_IOMMU_ACCESS_READ) {
> +          Status = EFI_INVALID_PARAMETER;
> +        }
> +        break;
> +
> +      case EdkiiIoMmuOperationBusMasterWrite:
> +      case EdkiiIoMmuOperationBusMasterWrite64:
> +        if (IoMmuAccess != EDKII_IOMMU_ACCESS_WRITE) {
> +          Status = EFI_INVALID_PARAMETER;
> +        }
> +        break;
> +
> +      case EdkiiIoMmuOperationBusMasterCommonBuffer:
> +      case EdkiiIoMmuOperationBusMasterCommonBuffer64:
> +        if (IoMmuAccess != (EDKII_IOMMU_ACCESS_READ |
> EDKII_IOMMU_ACCESS_WRITE)) {
> +          Status = EFI_INVALID_PARAMETER;
> +        }
> +        break;
> +
> +      default:
> +        Status = EFI_UNSUPPORTED;
> +    }
> +  }
> +
> +  return Status;
>  }
>  
>  EDKII_IOMMU_PROTOCOL  mIoMmu = {

With (1) updated:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

(feel free to include that in the proper posting at once)

Thanks!
Laszlo

> 
> 
>>
>> This would at least allow for a superficial consistency check, without
>> (a) incurring large performance penalty (such as list walking) and (b)
>> turning the SetAttribute() member function into a total joke. (Really,
>> IMO it should not even exist at the protocol specification level!)
>>
>> [*] OK, you might want to know why we should accept 0 too. Here's why:
>> we have *another* mIoMmuProtocol->SetAttribute() call, namely in
>> PciIoUnmap() [MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c]. It passes in
>> IoMmuAccess=0.
>>
>> Here's the sad bit: while this patch -- now commit 049695a0b1e2 --
>> propagates the return status of mIoMmuProtocol->SetAttribute() out of
>> PciIoMap(), the patch doesn't do the same for the other such call in
>> PciIoUnmap(). That seems like a bug (omission) itself, but once that one
>> gets fixed, we'll need to permit IoMmuAccess=0 too in OVMF's IOMMU
>> driver.
>>
>> I guess I could live with OVMF's IoMmuMap () doing nothing,
>> successfully, rather than doing nothing, unsuccessfully, *if*
>> EDKII_IOMMU_PROTOCOL.SetAttribute() had some public documentation rooted
>> in reality, *and* if PciBusDxe called that function with proper error
>> checking everywhere. Otherwise it's just another terrible, mis-specified
>> API, that platforms must route around.
>>
>> Wow I didn't expect to be worked up this much about it, but here I am. I
>> dunno, seeing crap interfaces just makes me unreasonably irate.
>>
>> Laszlo
>>
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114801): https://edk2.groups.io/g/devel/message/114801
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/leave/12367111/7686176/1913456212/xyzzy [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

[edk2-devel] [PATCH] MdeModulePkg/PciBusDxe: Add feedback status for PciIoMap

[Sheng Wei]

PciIoMap () need to feedback the status of
mIoMmuProtocol->SetAttribute () return value.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4652

Cc: Ray Ni <ray.ni@intel.com>
Cc: Huang, Jenny <jenny.huang@intel.com>
Cc: Chiang, Chris <chris.chiang@intel.com>
Signed-off-by: Sheng Wei <w.sheng@intel.com>
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
index 14bed54729..e85544d08d 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c
@@ -1024,12 +1024,12 @@ PciIoMap (
           return EFI_INVALID_PARAMETER;
       }
 
-      mIoMmuProtocol->SetAttribute (
-                        mIoMmuProtocol,
-                        PciIoDevice->Handle,
-                        *Mapping,
-                        IoMmuAttribute
-                        );
+      Status = mIoMmuProtocol->SetAttribute (
+                                 mIoMmuProtocol,
+                                 PciIoDevice->Handle,
+                                 *Mapping,
+                                 IoMmuAttribute
+                                 );
     }
   }
 
-- 
2.26.2.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114135): https://edk2.groups.io/g/devel/message/114135
Mute This Topic: https://groups.io/mt/103881889/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-