Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

2024-03-12 Thread Leif Lindholm

On 2024-03-12 09:58, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 17:56, Leif Lindholm  wrote:


On 2024-03-12 09:50, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  wrote:


On 2024-03-12 08:17, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.


It's unclear to me why this change is safe (looking forward).
Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?



Basically. NonCoherentIoMmuDxe is just a vehicle to allow
NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
not intended to ever do anything more than that.


Not that it needs to happen for this
(Reviewed-by: Leif Lindholm )
but maybe we ought to consider renaming it then?
DummyIoMmuDxe?


Fair point. Or PassThroughIoMmuDxe perhaps?


Works for me.
Or, hmm...
Is there a risk that sounds a bit like a driver that actively configures 
IoMmus into passthrough mode?


/
Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116697): https://edk2.groups.io/g/devel/message/116697
Mute This Topic: https://groups.io/mt/104886877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

2024-03-12 Thread Ard Biesheuvel
On Tue, 12 Mar 2024 at 17:56, Leif Lindholm  wrote:
>
> On 2024-03-12 09:50, Ard Biesheuvel wrote:
> > On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  
> > wrote:
> >>
> >> On 2024-03-12 08:17, Ard Biesheuvel wrote:
> >>> From: Ard Biesheuvel 
> >>>
> >>> NonCoherentIoMmuSetAttribute() does nothing except return
> >>> EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
> >>> bus driver will fail a PCI I/O Map() operation if the SetAttributes
> >>> fails.
> >>>
> >>> So return EFI_SUCCESS instead.
> >>
> >> It's unclear to me why this change is safe (looking forward).
> >> Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?
> >>
> >
> > Basically. NonCoherentIoMmuDxe is just a vehicle to allow
> > NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
> > not intended to ever do anything more than that.
>
> Not that it needs to happen for this
> (Reviewed-by: Leif Lindholm )
> but maybe we ought to consider renaming it then?
> DummyIoMmuDxe?
>

Fair point. Or PassThroughIoMmuDxe perhaps?


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116696): https://edk2.groups.io/g/devel/message/116696
Mute This Topic: https://groups.io/mt/104886877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

2024-03-12 Thread Leif Lindholm

On 2024-03-12 09:50, Ard Biesheuvel wrote:

On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  wrote:


On 2024-03-12 08:17, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.


It's unclear to me why this change is safe (looking forward).
Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?



Basically. NonCoherentIoMmuDxe is just a vehicle to allow
NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
not intended to ever do anything more than that.


Not that it needs to happen for this
(Reviewed-by: Leif Lindholm )
but maybe we ought to consider renaming it then?
DummyIoMmuDxe?

/
Leif



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116695): https://edk2.groups.io/g/devel/message/116695
Mute This Topic: https://groups.io/mt/104886877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

2024-03-12 Thread Ard Biesheuvel
On Tue, 12 Mar 2024 at 17:38, Leif Lindholm  wrote:
>
> On 2024-03-12 08:17, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel 
> >
> > NonCoherentIoMmuSetAttribute() does nothing except return
> > EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
> > bus driver will fail a PCI I/O Map() operation if the SetAttributes
> > fails.
> >
> > So return EFI_SUCCESS instead.
>
> It's unclear to me why this change is safe (looking forward).
> Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?
>

Basically. NonCoherentIoMmuDxe is just a vehicle to allow
NonCoherentDmaLib to be plugged into the PCI host bridge driver. It is
not intended to ever do anything more than that.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116693): https://edk2.groups.io/g/devel/message/116693
Mute This Topic: https://groups.io/mt/104886877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-




Re: [edk2-devel] [PATCH 1/1] EmbeddedPkg/NonCoherentIoMmuDxe: Make SetAttributes always succeed

2024-03-12 Thread Leif Lindholm

On 2024-03-12 08:17, Ard Biesheuvel wrote:

From: Ard Biesheuvel 

NonCoherentIoMmuSetAttribute() does nothing except return
EFI_UNSUPPORTED. This was fine when it was introduced, but now, the PCI
bus driver will fail a PCI I/O Map() operation if the SetAttributes
fails.

So return EFI_SUCCESS instead.


It's unclear to me why this change is safe (looking forward).
Does NonCoherentIoMmuDxe also imply no IoMmu actually exists?

/
Leif


Signed-off-by: Ard Biesheuvel 
---
This fixes a regression on Raspberry Pi4, which no longer boots.

  EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c 
b/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
index 4e7a5698c162..f02a76a62ea8 100644
--- a/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
+++ b/EmbeddedPkg/Drivers/NonCoherentIoMmuDxe/NonCoherentIoMmuDxe.c
@@ -70,7 +70,7 @@ NonCoherentIoMmuSetAttribute (
IN UINT64IoMmuAccess
)
  {
-  return EFI_UNSUPPORTED;
+  return EFI_SUCCESS;
  }
  
  /**




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#116691): https://edk2.groups.io/g/devel/message/116691
Mute This Topic: https://groups.io/mt/104886877/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-