Date   

Re: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit

Jeremy Linton
 

Hi,

On 5/10/21 11:56 AM, Ard Biesheuvel wrote:
On Thu, 15 Apr 2021 at 21:22, Jeremy Linton <jeremy.linton@arm.com> wrote:

Under normal circumstances GenetSimpleNetworkTransmit won't be
called unless the rest of the network stack detects the link is
up. So, during normal operation when the adapter is initialized
the link naturally transitions to link up, and then is ready for
activity later in the boot sequence. If that hasn't happened by
the time PXE runs then it will itself wait for the link.

OTOH, the normal distro PXE sequence involves PXE loading shim
which in turn loads grub, which tries to read machine specific
configs, modules, and grub.cfg in order to prepare the boot menu.
Then, once a grub selection is picked, it might try to load the
kernel+initrd.

In this sequence the network stack is shutdown and restarted
multiple times. Grub though, starts up, notices its been network
booted, reads saved network parameters and immediately tries to
transmit data assuming the link is still up.

When that happens grub will print "couldn't send network packet"
and if that lasts long enough it fails to load grub.cfg and the
user gets dropped to the grub prompt because no one in the path
bothers to assure the link state has transitioned back up.
This is an excellent problem description. Could we dedicate a couple
of paragraphs to the solution as well? Especially given that 'wait for
random # of seconds' seems to be the taken approach here, which seems
a bit sloppy to me, given that we should be able to detect whether the
link is up, no?
Isn't that what GenericPhyUpdateConfig is doing? Checking for linkup, and updating the phy state? So once the link goes up, it return EFI_SUCCESS and the loop terminates.

I was actually a bit concerned that the 10s wasn't enough since AFAIK a large part of the delay is the remote side.




For reference: https://github.com/pftf/RPi4/issues/113

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24 ++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 1bda18f157..29c76d8495 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -13,6 +13,7 @@
#include <Library/DebugLib.h>
#include <Library/DmaLib.h>
#include <Library/NetLib.h>
+#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/SimpleNetwork.h>

#include "BcmGenetDxe.h"
@@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit (

if (!Genet->SnpMode.MediaPresent) {
//
- // Don't bother transmitting if there's no link.
+ // We should only really get here if the link was up
+ // and is now down due to a stop/shutdown sequence, and
+ // the app (grub) doesn't bother to check link state
+ // because it was up a moment before.
+ // Lets wait a bit for the link to resume, rather than
+ // failing to send. In the case of grub it works either way
+ // but we can't be sure that is universally true, and
+ // hanging for a couple seconds is nicer than a screen of
+ // grub send failure messages.
//
- return EFI_NOT_READY;
+ int retries = 1000;
+ DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__));
+ do {
+ gBS->Stall (10000);
+ Status = GenericPhyUpdateConfig (&Genet->Phy);
+ } while (EFI_ERROR (Status) && retries--);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: no link\n", __FUNCTION__));
+ return EFI_NOT_READY;
+ } else {
+ Genet->SnpMode.MediaPresent = TRUE;
+ }
}

if (HeaderSize != 0) {
--
2.13.7





Re: [PATCH 2/2] Platform/RaspberryPi: Increase genet dma window

Ard Biesheuvel
 

On Thu, 15 Apr 2021 at 21:22, Jeremy Linton <jeremy.linton@arm.com> wrote:

The genet is capable of addressing the entire memory space
on the RPI4. Lets allow it to dma into those regions.
This solves intermittent issues with grub/etc being able
to communicate when the 3G limit is lifted on 8G boards.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
Acked-by: Ard Biesheuvel <ardb@kernel.org>


---
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc
index ddf4dd6a41..facf34f034 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -698,7 +698,7 @@
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf {
<PcdsFixedAtBuild>
gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x00000000
- gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffff
+ gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffffff
}

#
--
2.13.7






Re: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit

Ard Biesheuvel
 

On Thu, 15 Apr 2021 at 21:22, Jeremy Linton <jeremy.linton@arm.com> wrote:

Under normal circumstances GenetSimpleNetworkTransmit won't be
called unless the rest of the network stack detects the link is
up. So, during normal operation when the adapter is initialized
the link naturally transitions to link up, and then is ready for
activity later in the boot sequence. If that hasn't happened by
the time PXE runs then it will itself wait for the link.

OTOH, the normal distro PXE sequence involves PXE loading shim
which in turn loads grub, which tries to read machine specific
configs, modules, and grub.cfg in order to prepare the boot menu.
Then, once a grub selection is picked, it might try to load the
kernel+initrd.

In this sequence the network stack is shutdown and restarted
multiple times. Grub though, starts up, notices its been network
booted, reads saved network parameters and immediately tries to
transmit data assuming the link is still up.

When that happens grub will print "couldn't send network packet"
and if that lasts long enough it fails to load grub.cfg and the
user gets dropped to the grub prompt because no one in the path
bothers to assure the link state has transitioned back up.
This is an excellent problem description. Could we dedicate a couple
of paragraphs to the solution as well? Especially given that 'wait for
random # of seconds' seems to be the taken approach here, which seems
a bit sloppy to me, given that we should be able to detect whether the
link is up, no?



For reference: https://github.com/pftf/RPi4/issues/113

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24 ++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 1bda18f157..29c76d8495 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -13,6 +13,7 @@
#include <Library/DebugLib.h>
#include <Library/DmaLib.h>
#include <Library/NetLib.h>
+#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/SimpleNetwork.h>

#include "BcmGenetDxe.h"
@@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit (

if (!Genet->SnpMode.MediaPresent) {
//
- // Don't bother transmitting if there's no link.
+ // We should only really get here if the link was up
+ // and is now down due to a stop/shutdown sequence, and
+ // the app (grub) doesn't bother to check link state
+ // because it was up a moment before.
+ // Lets wait a bit for the link to resume, rather than
+ // failing to send. In the case of grub it works either way
+ // but we can't be sure that is universally true, and
+ // hanging for a couple seconds is nicer than a screen of
+ // grub send failure messages.
//
- return EFI_NOT_READY;
+ int retries = 1000;
+ DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__));
+ do {
+ gBS->Stall (10000);
+ Status = GenericPhyUpdateConfig (&Genet->Phy);
+ } while (EFI_ERROR (Status) && retries--);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: no link\n", __FUNCTION__));
+ return EFI_NOT_READY;
+ } else {
+ Genet->SnpMode.MediaPresent = TRUE;
+ }
}

if (HeaderSize != 0) {
--
2.13.7






Re: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

Jared, please confirm this is a "Reviewed-By"

-----Original Message-----
From: Jared McNeill <jmcneill@invisible.ca>
Sent: Friday, April 30, 2021 6:30 AM
To: Jeremy Linton <Jeremy.Linton@arm.com>
Cc: devel@edk2.groups.io; Ard Biesheuvel <Ard.Biesheuvel@arm.com>;
leif@nuviainc.com; Pete Batard <pete@akeo.ie>; Samer El-Haj-Mahmoud
<Samer.El-Haj-Mahmoud@arm.com>; Andrei Warkentin
(awarkentin@vmware.com) <awarkentin@vmware.com>
Subject: Re: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in
transmit

LGTM.

Take care,
Jared


On Apr 29, 2021, at 5:19 PM, Jeremy Linton <jeremy.linton@arm.com>
wrote:

+Jared McNeill for review.

Thanks,

On 4/15/21 2:22 PM, Jeremy Linton wrote:
Under normal circumstances GenetSimpleNetworkTransmit won't be
called
unless the rest of the network stack detects the link is up. So,
during normal operation when the adapter is initialized the link
naturally transitions to link up, and then is ready for activity
later in the boot sequence. If that hasn't happened by the time PXE
runs then it will itself wait for the link.
OTOH, the normal distro PXE sequence involves PXE loading shim which
in turn loads grub, which tries to read machine specific configs,
modules, and grub.cfg in order to prepare the boot menu.
Then, once a grub selection is picked, it might try to load the
kernel+initrd.
In this sequence the network stack is shutdown and restarted multiple
times. Grub though, starts up, notices its been network booted, reads
saved network parameters and immediately tries to transmit data
assuming the link is still up.
When that happens grub will print "couldn't send network packet"
and if that lasts long enough it fails to load grub.cfg and the user
gets dropped to the grub prompt because no one in the path bothers to
assure the link state has transitioned back up.
For reference: https://github.com/pftf/RPi4/issues/113
Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24
++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-) diff --git
a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 1bda18f157..29c76d8495 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -13,6 +13,7 @@
#include <Library/DebugLib.h>
#include <Library/DmaLib.h>
#include <Library/NetLib.h>
+#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/SimpleNetwork.h>
#include "BcmGenetDxe.h"
@@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit (
if (!Genet->SnpMode.MediaPresent) {
//
- // Don't bother transmitting if there's no link.
+ // We should only really get here if the link was up
+ // and is now down due to a stop/shutdown sequence, and
+ // the app (grub) doesn't bother to check link state
+ // because it was up a moment before.
+ // Lets wait a bit for the link to resume, rather than
+ // failing to send. In the case of grub it works either way
+ // but we can't be sure that is universally true, and
+ // hanging for a couple seconds is nicer than a screen of
+ // grub send failure messages.
//
- return EFI_NOT_READY;
+ int retries = 1000;
+ DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__));
+ do {
+ gBS->Stall (10000);
+ Status = GenericPhyUpdateConfig (&Genet->Phy);
+ } while (EFI_ERROR (Status) && retries--);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: no link\n", __FUNCTION__));
+ return EFI_NOT_READY;
+ } else {
+ Genet->SnpMode.MediaPresent = TRUE;
+ }
}
if (HeaderSize != 0) {
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 15, 2021 3:22 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>; Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH 1/2] Silicon/Broadcom/BcmGenetDxe: Delay for linkup in
transmit

Under normal circumstances GenetSimpleNetworkTransmit won't be called
unless the rest of the network stack detects the link is up. So, during normal
operation when the adapter is initialized the link naturally transitions to link
up, and then is ready for activity later in the boot sequence. If that hasn't
happened by the time PXE runs then it will itself wait for the link.

OTOH, the normal distro PXE sequence involves PXE loading shim which in
turn loads grub, which tries to read machine specific configs, modules, and
grub.cfg in order to prepare the boot menu.
Then, once a grub selection is picked, it might try to load the
kernel+initrd.

In this sequence the network stack is shutdown and restarted multiple times.
Grub though, starts up, notices its been network booted, reads saved
network parameters and immediately tries to transmit data assuming the link
is still up.

When that happens grub will print "couldn't send network packet"
and if that lasts long enough it fails to load grub.cfg and the user gets
dropped to the grub prompt because no one in the path bothers to assure
the link state has transitioned back up.

For reference: https://github.com/pftf/RPi4/issues/113

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
.../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24
++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
index 1bda18f157..29c76d8495 100644
--- a/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
+++ b/Silicon/Broadcom/Drivers/Net/BcmGenetDxe/SimpleNetwork.c
@@ -13,6 +13,7 @@
#include <Library/DebugLib.h>
#include <Library/DmaLib.h>
#include <Library/NetLib.h>
+#include <Library/UefiBootServicesTableLib.h>
#include <Protocol/SimpleNetwork.h>

#include "BcmGenetDxe.h"
@@ -590,9 +591,28 @@ GenetSimpleNetworkTransmit (

if (!Genet->SnpMode.MediaPresent) {
//
- // Don't bother transmitting if there's no link.
+ // We should only really get here if the link was up
+ // and is now down due to a stop/shutdown sequence, and
+ // the app (grub) doesn't bother to check link state
+ // because it was up a moment before.
+ // Lets wait a bit for the link to resume, rather than
+ // failing to send. In the case of grub it works either way
+ // but we can't be sure that is universally true, and
+ // hanging for a couple seconds is nicer than a screen of
+ // grub send failure messages.
//
- return EFI_NOT_READY;
+ int retries = 1000;
+ DEBUG ((DEBUG_INFO, "%a: Waiting 10s for link\n", __FUNCTION__));
+ do {
+ gBS->Stall (10000);
+ Status = GenericPhyUpdateConfig (&Genet->Phy);
+ } while (EFI_ERROR (Status) && retries--);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((DEBUG_ERROR, "%a: no link\n", __FUNCTION__));
+ return EFI_NOT_READY;
+ } else {
+ Genet->SnpMode.MediaPresent = TRUE;
+ }
}

if (HeaderSize != 0) {
--
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH BUG 0/2] rpi: Fix PXE issues with grub

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 15, 2021 3:22 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>; Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH BUG 0/2] rpi: Fix PXE issues with grub

When PXE booting with grub the network link isn't given a chance to resume
so grub's transmit calls fail. This results in failed boots. Similarly the DMA
range for the adapter isn't right since it doesn't have a 32-bit restriction.
Again this keeps grub from failing on 8G devices,

Jeremy Linton (2):
Silicon/Broadcom/BcmGenetDxe: Delay for linkup in transmit
Platform/RaspberryPi: Increase genet dma window

Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
.../Drivers/Net/BcmGenetDxe/SimpleNetwork.c | 24
++++++++++++++++++++--
2 files changed, 23 insertions(+), 3 deletions(-)

--
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 2/2] Platform/RaspberryPi: Increase genet dma window

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

Jared, I assume your response can be taken as a "Reviewed-By", correct?

-----Original Message-----
From: Jared McNeill <jmcneill@invisible.ca>
Sent: Friday, April 30, 2021 9:08 PM
To: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>
Cc: Jeremy Linton <Jeremy.Linton@arm.com>; devel@edk2.groups.io; Ard
Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com; pete@akeo.ie;
Andrei Warkentin (awarkentin@vmware.com) <awarkentin@vmware.com>
Subject: Re: [PATCH 2/2] Platform/RaspberryPi: Increase genet dma window

LGTM!

Take care,
Jared


On Apr 30, 2021, at 3:15 PM, Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com> wrote:

+Jared

Reviewed-By: Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 15, 2021 3:22 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>;
Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>;
Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH 2/2] Platform/RaspberryPi: Increase genet dma window

The genet is capable of addressing the entire memory space on the RPI4.
Lets allow it to dma into those regions.
This solves intermittent issues with grub/etc being able to
communicate when the 3G limit is lifted on 8G boards.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
b/Platform/RaspberryPi/RPi4/RPi4.dsc
index ddf4dd6a41..facf34f034 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -698,7 +698,7 @@
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf {
<PcdsFixedAtBuild>
gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x00000000
- gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffff
+ gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffffff
}

#
--
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 2/2] Platform/RaspberryPi: Increase genet dma window

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 15, 2021 3:22 PM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>; Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH 2/2] Platform/RaspberryPi: Increase genet dma window

The genet is capable of addressing the entire memory space on the RPI4.
Lets allow it to dma into those regions.
This solves intermittent issues with grub/etc being able to communicate
when the 3G limit is lifted on 8G boards.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/RPi4/RPi4.dsc | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc
b/Platform/RaspberryPi/RPi4/RPi4.dsc
index ddf4dd6a41..facf34f034 100644
--- a/Platform/RaspberryPi/RPi4/RPi4.dsc
+++ b/Platform/RaspberryPi/RPi4/RPi4.dsc
@@ -698,7 +698,7 @@
Silicon/Broadcom/Drivers/Net/BcmGenetDxe/BcmGenetDxe.inf {
<PcdsFixedAtBuild>
gEmbeddedTokenSpaceGuid.PcdDmaDeviceOffset|0x00000000
- gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffff
+ gEmbeddedTokenSpaceGuid.PcdDmaDeviceLimit|0xffffffffff
}

#
--
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

-----Original Message-----
From: Pete Batard <pete@akeo.ie>
Sent: Thursday, April 8, 2021 5:48 AM
To: Jeremy Linton <Jeremy.Linton@arm.com>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com; Samer
El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Andrei Warkentin
(awarkentin@vmware.com) <awarkentin@vmware.com>
Subject: Re: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA
consumer

On 2021.04.08 06:58, Jeremy Linton wrote:
Bridge devices should be marked as producers so that their children
can consume the resources. In linux if this isn't true then the
translation gets ignored and the DMA values are incorrect. This fixes
DMA on all the devices that need a translation.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +-
Platform/RaspberryPi/AcpiTables/Emmc.asl | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index d116f965e1..32cd5fc9f9 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -205,7 +205,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
"RPI", 2)
// Only the first GB is available.

// Bus 0xC0000000 -> CPU 0x00000000.

//

- QWordMemory (ResourceConsumer,

+ QWordMemory (ResourceProducer,

,

MinFixed,

MaxFixed,

diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl
b/Platform/RaspberryPi/AcpiTables/Emmc.asl
index 179dd3ecdb..0fbc2a79ea 100644
--- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -32,7 +32,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN",
"RPI4EMMC", 2)
}



Name (_DMA, ResourceTemplate() {

- QWordMemory (ResourceConsumer,

+ QWordMemory (ResourceProducer,

,

MinFixed,

MaxFixed,
Reviewed-by: Pete Batard <pete@akeo.ie>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

Samer El-Haj-Mahmoud
 

+Ard's new e-mail address

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 8, 2021 1:59 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>; Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA
consumer

Bridge devices should be marked as producers so that their children can
consume the resources. In linux if this isn't true then the translation gets
ignored and the DMA values are incorrect. This fixes DMA on all the devices
that need a translation.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +-
Platform/RaspberryPi/AcpiTables/Emmc.asl | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index d116f965e1..32cd5fc9f9 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -205,7 +205,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN",
"RPI", 2)
// Only the first GB is available. // Bus 0xC0000000 -> CPU
0x00000000. //- QWordMemory (ResourceConsumer,+
QWordMemory (ResourceProducer, , MinFixed,
MaxFixed,diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl
b/Platform/RaspberryPi/AcpiTables/Emmc.asl
index 179dd3ecdb..0fbc2a79ea 100644
--- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -32,7 +32,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN",
"RPI4EMMC", 2)
} Name (_DMA, ResourceTemplate() {- QWordMemory
(ResourceConsumer,+ QWordMemory (ResourceProducer, ,
MinFixed, MaxFixed,--
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

Samer El-Haj-Mahmoud
 

+ Ard’s new e-mail address

 

 

From: Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Sent: Friday, April 30, 2021 2:05 PM
To: devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Andrei Warkentin (awarkentin@...) <awarkentin@...>; Jeremy Linton <Jeremy.Linton@...>
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: RE: [edk2-devel] [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 

Update: UEFI Forum ASWG (ACPI spec working group) approved the submitted ECR as an errata for future ACPI 6.4 spec publication.

 

We can go ahead and proceed with this patch as submitted, based on that ECR clarification.

 

With that,

 

Reviewed-By: Samer El-Haj-Mahmoud Samer.El-Haj-Mahmoud@...

 

Thanks,

--Samer

 

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer El-Haj-Mahmoud via groups.io
Sent: Thursday, April 29, 2021 10:03 AM
To: devel@edk2.groups.io; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>; Andrei Warkentin (awarkentin@...) <awarkentin@...>; Jeremy Linton <Jeremy.Linton@...>; rfc@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [edk2-devel] [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 

Any further comments on the ACPI ECR documented in: https://bugzilla.tianocore.org/show_bug.cgi?id=3335 ?

 

I already have comments from Jeremey and Andrew saying it looks good. If there are no objections, I will let ASWG know to approve the ECR for future ACPI spec publication.

 

Thanks,

--Samer

 

 

 

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer El-Haj-Mahmoud via groups.io
Sent: Tuesday, April 13, 2021 12:45 PM
To: Andrei Warkentin (awarkentin@...) <awarkentin@...>; Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [edk2-devel] [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 

I just got to this thread. Apologies for the delay.

 

I went through the ACPI spec. Here is what I see:

 

https://uefi.org/specs/ACPI/6.4/19_ASL_Reference/ACPI_Source_Language_Reference.html#qwordmemory-qword-memory-resource-descriptor-macro

 

ResourceUsage specifies whether the Memory range is consumed by this device (ResourceConsumer) or passed on to child devices (ResourceProducer). If nothing is specified, then ResourceConsumer is assumed.”
 
https://uefi.org/specs/ACPI/6.4/06_Device_Configuration/Device_Configuration.html#dma-direct-memory-access
 
“ It specifies the ranges the bus controller (bridge) decodes on the child-side of its interface. (This is analogous to the _CRS object, which describes the resources that the bus controller decodes on the parent-side of its interface.) Any ranges described in the resources of a _DMA object can be used by child devices for DMA or bus master transactions..”
 
The way I read the spec, this wording in the _DMA definition “Any ranges described in the resources of a _DMA object can be used by child devices..” suggests that this should be a ResourceProducer, per the QWordMemory resource descriptor definition above
 

The _DMA example in section 6.2.4 uses a “ResourceConsumer”, when it should really be “ResourceProducer” according to these definitions: It describes , the child devices view of the address range, so the "translation" added is the CPU's view of the same range.

 
I submitted a “code first” ECR to correct the ACPI spec example (here : https://bugzilla.tianocore.org/show_bug.cgi?id=3335). Please provide feedback on the BZ (or this thread) whether you agree or not, so we can take this to ASWG/UEFI Forum for discussion and approval
 
Thanks,
--Samer
 
 

 

From: Andrei Warkentin <awarkentin@...>
Sent: Thursday, April 8, 2021 10:24 AM
To: Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 

I don't know... the ACPI spec is weird.

 

 

...lists ResourceConsumer for _DMA.

 

A

 


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 3/3] Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 

Bridge devices should be marked as producers so that their
children can consume the resources. In linux if this isn't
true then the translation gets ignored and the DMA values
are incorrect. This fixes DMA on all the devices that
need a translation.

Signed-off-by: Jeremy Linton <jeremy.linton@...>
---
 Platform/RaspberryPi/AcpiTables/Dsdt.asl | 2 +-
 Platform/RaspberryPi/AcpiTables/Emmc.asl | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Dsdt.asl b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
index d116f965e1..32cd5fc9f9 100644
--- a/Platform/RaspberryPi/AcpiTables/Dsdt.asl
+++ b/Platform/RaspberryPi/AcpiTables/Dsdt.asl
@@ -205,7 +205,7 @@ DefinitionBlock ("Dsdt.aml", "DSDT", 5, "RPIFDN", "RPI", 2)
         // Only the first GB is available.

         // Bus 0xC0000000 -> CPU 0x00000000.

         //

-        QWordMemory (ResourceConsumer,

+        QWordMemory (ResourceProducer,

           ,

           MinFixed,

           MaxFixed,

diff --git a/Platform/RaspberryPi/AcpiTables/Emmc.asl b/Platform/RaspberryPi/AcpiTables/Emmc.asl
index 179dd3ecdb..0fbc2a79ea 100644
--- a/Platform/RaspberryPi/AcpiTables/Emmc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Emmc.asl
@@ -32,7 +32,7 @@ DefinitionBlock (__FILE__, "SSDT", 5, "RPIFDN", "RPI4EMMC", 2)
       }

 

       Name (_DMA, ResourceTemplate() {

-        QWordMemory (ResourceConsumer,

+        QWordMemory (ResourceProducer,

           ,

           MinFixed,

           MaxFixed,

--
2.13.7

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components

Samer El-Haj-Mahmoud
 

+ Ard’s new e-mail address

 

 

From: Andrei Warkentin <awarkentin@...>
Sent: Thursday, April 8, 2021 10:17 AM
To: Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components

 

Reviewed-by: Andrei Warkentin <awarkentin@...>


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components

 

Add some additional IORT nodes for the USB & EMMC devices, realistically
we probably only need to have a single node with the lowest AddressSizeLimit
but this is conceptually "cleaner" should anyone actually try and use these
values rather than the _DMA provided ones.

Signed-off-by: Jeremy Linton <jeremy.linton@...>
---
 Platform/RaspberryPi/AcpiTables/Iort.aslc | 44 ++++++++++++++++++++++++++++++-
 1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc b/Platform/RaspberryPi/AcpiTables/Iort.aslc
index 00720194bb..810307ae37 100644
--- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
@@ -20,6 +20,8 @@ typedef struct {
 typedef struct {

   EFI_ACPI_6_0_IO_REMAPPING_TABLE      Iort;

   RPI4_NC_NODE                         NamedCompNode;

+  RPI4_NC_NODE                         NamedCompNode2;

+  RPI4_NC_NODE                         NamedCompNode3;

 } RPI4_IO_REMAPPING_STRUCTURE;

 

 STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {

@@ -27,7 +29,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
     ACPI_HEADER (EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,

                  RPI4_IO_REMAPPING_STRUCTURE,

                  EFI_ACPI_IO_REMAPPING_TABLE_REVISION),

-    1,                                              // NumNodes

+    3,                                              // NumNodes

     sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE),       // NodeOffset

     0                                               // Reserved

   }, {

@@ -50,6 +52,46 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
     }, {

       "\\_SB_.SCB0.XHC0"                            // ObjectName

     }

+  }, {

+    // gpu/dwc usb named component node

+    {

+      {

+        EFI_ACPI_IORT_TYPE_NAMED_COMP,              // Type

+        sizeof (RPI4_NC_NODE),                      // Length

+        0x0,                                        // Revision

+        0x0,                                        // Reserved

+        0x0,                                        // NumIdMappings

+        0x0,                                        // IdReference

+      },

+      0x0,                                          // Flags

+      0x0,                                          // CacheCoherent

+      0x0,                                          // AllocationHints

+      0x0,                                          // Reserved

+      0x0,                                          // MemoryAccessFlags

+      30,                                           // AddressSizeLimit

+    }, {

+      "\\_SB_.GDV0.USB0"                            // ObjectName

+    }

+  }, {

+    // emmc2 named component node

+    {

+      {

+        EFI_ACPI_IORT_TYPE_NAMED_COMP,              // Type

+        sizeof (RPI4_NC_NODE),                      // Length

+        0x0,                                        // Revision

+        0x0,                                        // Reserved

+        0x0,                                        // NumIdMappings

+        0x0,                                        // IdReference

+      },

+      0x0,                                          // Flags

+      0x0,                                          // CacheCoherent

+      0x0,                                          // AllocationHints

+      0x0,                                          // Reserved

+      0x0,                                          // MemoryAccessFlags

+      30,                                           // AddressSizeLimit

+    }, {

+      "\\_SB_.GDV1.SDC3"                            // ObjectName

+    }

   }

 };

 

--
2.13.7

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components

Samer El-Haj-Mahmoud
 

+ Ard’s new e-mail address

-----Original Message-----
From: Pete Batard <pete@akeo.ie>
Sent: Thursday, April 8, 2021 5:48 AM
To: Jeremy Linton <Jeremy.Linton@arm.com>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com; Samer
El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Andrei Warkentin
(awarkentin@vmware.com) <awarkentin@vmware.com>
Subject: Re: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further
named components

On 2021.04.08 06:58, Jeremy Linton wrote:
Add some additional IORT nodes for the USB & EMMC devices,
realistically we probably only need to have a single node with the
lowest AddressSizeLimit but this is conceptually "cleaner" should
anyone actually try and use these values rather than the _DMA provided
ones.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Iort.aslc | 44
++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc
b/Platform/RaspberryPi/AcpiTables/Iort.aslc
index 00720194bb..810307ae37 100644
--- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
@@ -20,6 +20,8 @@ typedef struct {
typedef struct {

EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort;

RPI4_NC_NODE NamedCompNode;

+ RPI4_NC_NODE NamedCompNode2;

+ RPI4_NC_NODE NamedCompNode3;

} RPI4_IO_REMAPPING_STRUCTURE;



STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {

@@ -27,7 +29,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
ACPI_HEADER (EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,

RPI4_IO_REMAPPING_STRUCTURE,

EFI_ACPI_IO_REMAPPING_TABLE_REVISION),

- 1, // NumNodes

+ 3, // NumNodes

sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE), // NodeOffset

0 // Reserved

}, {

@@ -50,6 +52,46 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
}, {

"\\_SB_.SCB0.XHC0" // ObjectName

}

+ }, {

+ // gpu/dwc usb named component node

+ {

+ {

+ EFI_ACPI_IORT_TYPE_NAMED_COMP, // Type

+ sizeof (RPI4_NC_NODE), // Length

+ 0x0, // Revision

+ 0x0, // Reserved

+ 0x0, // NumIdMappings

+ 0x0, // IdReference

+ },

+ 0x0, // Flags

+ 0x0, // CacheCoherent

+ 0x0, // AllocationHints

+ 0x0, // Reserved

+ 0x0, // MemoryAccessFlags

+ 30, // AddressSizeLimit

+ }, {

+ "\\_SB_.GDV0.USB0" // ObjectName

+ }

+ }, {

+ // emmc2 named component node

+ {

+ {

+ EFI_ACPI_IORT_TYPE_NAMED_COMP, // Type

+ sizeof (RPI4_NC_NODE), // Length

+ 0x0, // Revision

+ 0x0, // Reserved

+ 0x0, // NumIdMappings

+ 0x0, // IdReference

+ },

+ 0x0, // Flags

+ 0x0, // CacheCoherent

+ 0x0, // AllocationHints

+ 0x0, // Reserved

+ 0x0, // MemoryAccessFlags

+ 30, // AddressSizeLimit

+ }, {

+ "\\_SB_.GDV1.SDC3" // ObjectName

+ }

}

};


Reviewed-by: Pete Batard <pete@akeo.ie>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named components

Samer El-Haj-Mahmoud
 

+ Ard's new e-mail address

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 8, 2021 1:59 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>; Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH 2/3] Platform/RaspberryPi/AcpiTables: Add further named
components

Add some additional IORT nodes for the USB & EMMC devices, realistically
we probably only need to have a single node with the lowest
AddressSizeLimit
but this is conceptually "cleaner" should anyone actually try and use these
values rather than the _DMA provided ones.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Iort.aslc | 44
++++++++++++++++++++++++++++++-
1 file changed, 43 insertions(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Iort.aslc
b/Platform/RaspberryPi/AcpiTables/Iort.aslc
index 00720194bb..810307ae37 100644
--- a/Platform/RaspberryPi/AcpiTables/Iort.aslc
+++ b/Platform/RaspberryPi/AcpiTables/Iort.aslc
@@ -20,6 +20,8 @@ typedef struct {
typedef struct {

EFI_ACPI_6_0_IO_REMAPPING_TABLE Iort;

RPI4_NC_NODE NamedCompNode;

+ RPI4_NC_NODE NamedCompNode2;

+ RPI4_NC_NODE NamedCompNode3;

} RPI4_IO_REMAPPING_STRUCTURE;



STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {

@@ -27,7 +29,7 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
ACPI_HEADER (EFI_ACPI_6_0_IO_REMAPPING_TABLE_SIGNATURE,

RPI4_IO_REMAPPING_STRUCTURE,

EFI_ACPI_IO_REMAPPING_TABLE_REVISION),

- 1, // NumNodes

+ 3, // NumNodes

sizeof (EFI_ACPI_6_0_IO_REMAPPING_TABLE), // NodeOffset

0 // Reserved

}, {

@@ -50,6 +52,46 @@ STATIC RPI4_IO_REMAPPING_STRUCTURE Iort = {
}, {

"\\_SB_.SCB0.XHC0" // ObjectName

}

+ }, {

+ // gpu/dwc usb named component node

+ {

+ {

+ EFI_ACPI_IORT_TYPE_NAMED_COMP, // Type

+ sizeof (RPI4_NC_NODE), // Length

+ 0x0, // Revision

+ 0x0, // Reserved

+ 0x0, // NumIdMappings

+ 0x0, // IdReference

+ },

+ 0x0, // Flags

+ 0x0, // CacheCoherent

+ 0x0, // AllocationHints

+ 0x0, // Reserved

+ 0x0, // MemoryAccessFlags

+ 30, // AddressSizeLimit

+ }, {

+ "\\_SB_.GDV0.USB0" // ObjectName

+ }

+ }, {

+ // emmc2 named component node

+ {

+ {

+ EFI_ACPI_IORT_TYPE_NAMED_COMP, // Type

+ sizeof (RPI4_NC_NODE), // Length

+ 0x0, // Revision

+ 0x0, // Reserved

+ 0x0, // NumIdMappings

+ 0x0, // IdReference

+ },

+ 0x0, // Flags

+ 0x0, // CacheCoherent

+ 0x0, // AllocationHints

+ 0x0, // Reserved

+ 0x0, // MemoryAccessFlags

+ 30, // AddressSizeLimit

+ }, {

+ "\\_SB_.GDV1.SDC3" // ObjectName

+ }

}

};



--
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode

Samer El-Haj-Mahmoud
 

+ Ard’s new e-mail address

 

 

From: Andrei Warkentin <awarkentin@...>
Sent: Thursday, April 8, 2021 10:18 AM
To: Pete Batard <pete@...>; Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode

 

Reviewed-by: Andrei Warkentin <awarkentin@...>


From: Pete Batard <pete@...>
Sent: Thursday, April 8, 2021 4:48 AM
To: Jeremy Linton <jeremy.linton@...>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>
Subject: Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode

 

On 2021.04.08 06:58, Jeremy Linton wrote:
> The arasan caps registers are no longer being overridden by
> the brcm iproc driver, so we should be assuring that the
> "High Speed Support" bit 21 is set in the capability
> register. This significantly improves the wifi perf
> using linux.
>
> Signed-off-by: Jeremy Linton <jeremy.linton@...>
> ---
>   Platform/RaspberryPi/AcpiTables/Sdhc.asl | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> index 0430ab7d2d..42776e33bb 100644
> --- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> +++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
> @@ -52,7 +52,7 @@ Device (SDC1)
>     Name (_DSD, Package () {
>
>       ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
>
>       Package () {
>
> -      Package () { "sdhci-caps", 0x0100fa81 },
>
> +      Package () { "sdhci-caps", 0x0120fa81 },
>
>       }
>
>     })
>
>  
>

Reviewed-by: Pete Batard <pete@...>

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode

Samer El-Haj-Mahmoud
 

+ Ard’s new e-mail address

-----Original Message-----
From: Pete Batard <pete@akeo.ie>
Sent: Thursday, April 8, 2021 5:48 AM
To: Jeremy Linton <Jeremy.Linton@arm.com>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com; Samer
El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Andrei Warkentin
(awarkentin@vmware.com) <awarkentin@vmware.com>
Subject: Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan
hispeed mode

On 2021.04.08 06:58, Jeremy Linton wrote:
The arasan caps registers are no longer being overridden by the brcm
iproc driver, so we should be assuring that the "High Speed Support"
bit 21 is set in the capability register. This significantly improves
the wifi perf using linux.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Sdhc.asl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
index 0430ab7d2d..42776e33bb 100644
--- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
@@ -52,7 +52,7 @@ Device (SDC1)
Name (_DSD, Package () {

ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),

Package () {

- Package () { "sdhci-caps", 0x0100fa81 },

+ Package () { "sdhci-caps", 0x0120fa81 },

}

})


Reviewed-by: Pete Batard <pete@akeo.ie>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode

Samer El-Haj-Mahmoud
 

+ Ard's new e-mail address

-----Original Message-----
From: Jeremy Linton <jeremy.linton@arm.com>
Sent: Thursday, April 8, 2021 1:59 AM
To: devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@arm.com>; leif@nuviainc.com;
pete@akeo.ie; Samer El-Haj-Mahmoud <Samer.El-Haj-
Mahmoud@arm.com>; Andrei Warkentin (awarkentin@vmware.com)
<awarkentin@vmware.com>; Jeremy Linton <Jeremy.Linton@arm.com>
Subject: [PATCH 1/3] Platform/RaspberryPi/Acpitables: Enable Arasan
hispeed mode

The arasan caps registers are no longer being overridden by the brcm iproc
driver, so we should be assuring that the "High Speed Support" bit 21 is set in
the capability register. This significantly improves the wifi perf using linux.

Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
---
Platform/RaspberryPi/AcpiTables/Sdhc.asl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
index 0430ab7d2d..42776e33bb 100644
--- a/Platform/RaspberryPi/AcpiTables/Sdhc.asl
+++ b/Platform/RaspberryPi/AcpiTables/Sdhc.asl
@@ -52,7 +52,7 @@ Device (SDC1)
Name (_DSD, Package () { ToUUID("daffd814-6eba-4d8c-8a91-
bc9bbf4aa301"), Package () {- Package () { "sdhci-caps", 0x0100fa81 },+
Package () { "sdhci-caps", 0x0120fa81 }, } }) --
2.13.7
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 0/3] SD+USB perf/DMA fixes

Samer El-Haj-Mahmoud
 

Adding Ard’s new e-mail address

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Samer El-Haj-Mahmoud via groups.io
Sent: Friday, April 30, 2021 2:28 PM
To: Andrei Warkentin (awarkentin@...) <awarkentin@...>; Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [edk2-devel] [PATCH 0/3] SD+USB perf/DMA fixes

 

This is now clarified in an ACPI spec ECR (https://bugzilla.tianocore.org/show_bug.cgi?id=3335). The example will be updated in a future spec errata to use ResourceProducer.

 

I think this patch can resume as it is not gated by the spec anymore.

 

 

 

From: Andrei Warkentin <awarkentin@...>
Sent: Thursday, April 8, 2021 10:25 AM
To: Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [PATCH 0/3] SD+USB perf/DMA fixes

 

I think Linux's behavior needs to be reconciled with the ACPI spec, which uses _DMA with ResourceConsumer, not ResourceProducer.

 

A


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To:
devel@edk2.groups.io <devel@edk2.groups.io>
Cc:
ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 0/3] SD+USB perf/DMA fixes

 

A large part of why the emmc & dwc2 usb
controllers haven't been working properly is
because the "bus" _DMA was incorrectly tagged
as a consumer, when it needs to be a producer.

That is why linux has been dropping the
translation value portions of _DMA().

Since the emmc2 dma (with the old B0 SoC), and the
dwc2 is expected to work, lets add matching 30 bit
IORT entries for them.

Finally, in the shuffle the high speed cap bit override
was dropped from the linux patches, and I failed
to add it back to the firmware values, this caused
the wifi perf to be lower than it should have been.

Jeremy Linton (3):
  Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode
  Platform/RaspberryPi/AcpiTables: Add further named components
  Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 Platform/RaspberryPi/AcpiTables/Dsdt.asl  |  2 +-
 Platform/RaspberryPi/AcpiTables/Emmc.asl  |  2 +-
 Platform/RaspberryPi/AcpiTables/Iort.aslc | 44 ++++++++++++++++++++++++++++++-
 Platform/RaspberryPi/AcpiTables/Sdhc.asl  |  2 +-
 4 files changed, 46 insertions(+), 4 deletions(-)

--
2.13.7

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [PATCH 0/3] SD+USB perf/DMA fixes

Samer El-Haj-Mahmoud
 

Adding Ard’s new e-mail

 

From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Andrei Warkentin via groups.io
Sent: Friday, April 30, 2021 4:30 PM
To: Jeremy Linton <Jeremy.Linton@...>; devel@edk2.groups.io
Cc: Ard Biesheuvel <Ard.Biesheuvel@...>; leif@...; pete@...; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@...>
Subject: Re: [edk2-devel] [PATCH 0/3] SD+USB perf/DMA fixes

 

LGTM 

 

Reviewed-by: Andrei Warkentin <awarkentin@...>


From: Jeremy Linton <jeremy.linton@...>
Sent: Thursday, April 8, 2021 12:58 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: ard.biesheuvel@... <ard.biesheuvel@...>; leif@... <leif@...>; pete@... <pete@...>; samer.el-haj-mahmoud@... <samer.el-haj-mahmoud@...>; Andrei Warkentin <awarkentin@...>; Jeremy Linton <jeremy.linton@...>
Subject: [PATCH 0/3] SD+USB perf/DMA fixes

 

A large part of why the emmc & dwc2 usb
controllers haven't been working properly is
because the "bus" _DMA was incorrectly tagged
as a consumer, when it needs to be a producer.

That is why linux has been dropping the
translation value portions of _DMA().

Since the emmc2 dma (with the old B0 SoC), and the
dwc2 is expected to work, lets add matching 30 bit
IORT entries for them.

Finally, in the shuffle the high speed cap bit override
was dropped from the linux patches, and I failed
to add it back to the firmware values, this caused
the wifi perf to be lower than it should have been.

Jeremy Linton (3):
  Platform/RaspberryPi/Acpitables: Enable Arasan hispeed mode
  Platform/RaspberryPi/AcpiTables: Add further named components
  Platform/RaspberryPi/AcpiTables: Correct _DMA consumer

 Platform/RaspberryPi/AcpiTables/Dsdt.asl  |  2 +-
 Platform/RaspberryPi/AcpiTables/Emmc.asl  |  2 +-
 Platform/RaspberryPi/AcpiTables/Iort.aslc | 44 ++++++++++++++++++++++++++++++-
 Platform/RaspberryPi/AcpiTables/Sdhc.asl  |  2 +-
 4 files changed, 46 insertions(+), 4 deletions(-)

--
2.13.7

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.


Re: [edk2-platforms PATCH 0/6] Marvell SD/MMC updates

Marcin Wojtas
 

pon., 10 maj 2021 o 18:07 Ard Biesheuvel <ardb@kernel.org> napisał(a):

On Fri, 30 Apr 2021 at 20:04, Marcin Wojtas <mw@semihalf.com> wrote:

Hi,


pon., 19 kwi 2021 o 10:52 Marcin Wojtas <mw@semihalf.com> napisał(a):

pon., 19 kwi 2021 o 10:49 Marcin Wojtas <mw@semihalf.com> napisał(a):

Hi,

This series applies modifications to the MMC settings
on the platforms based on the Marvell SoCs.
Where possible, higher speeds are enabled.
Moreover a DSDT description is added, which allows
to make use of the SD/MMC in the OS booted with ACPI.

More details can be found in the commit logs.
The patchest is publicly available in the github:
https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/misc-uspstream-r20210416
Correct link:
https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/mmc-upstream-r20210419

I am looking forward to your review.
Do you have any comments/remarks to the patchset?
For the series,

Acked-by: Ard Biesheuvel <ardb@kernel.org>


Pushed as 7661dfff1528..c7e1631a673f
Thanks!
Marcin

thanks,


Best regards,
Marcin

Marcin Wojtas (6):
Marvell/Armada80x0Db: Update CP0 MMC settings
Marvell/Armada80x0Db: Introduce SD/MMC ACPI description
Marvell/Armada70x0Db: Update CP0 MMC settings
Marvell/Armada70x0Db: Introduce SD/MMC ACPI description
Marvell/Cn913xDb: Update AP807 MMC settings
Marvell/Cn913xDb: Introduce SD/MMC ACPI description

Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.h | 1 +
Platform/Marvell/Cn913xDb/NonDiscoverableInitLib/NonDiscoverableInitLib.h | 1 +
Platform/Marvell/Armada70x0Db/Armada70x0DbBoardDescLib/Armada70x0DbBoardDescLib.c | 2 +-
Platform/Marvell/Armada70x0Db/NonDiscoverableInitLib/NonDiscoverableInitLib.c | 79 +++++++++++++++-----
Platform/Marvell/Armada80x0Db/Armada80x0DbBoardDescLib/Armada80x0DbBoardDescLib.c | 2 +-
Platform/Marvell/Cn913xDb/BoardDescriptionLib/Cn9130DbABoardDescLib.c | 8 +-
Platform/Marvell/Cn913xDb/NonDiscoverableInitLib/NonDiscoverableInitLib.c | 23 ++++++
Silicon/Marvell/Armada7k8k/AcpiTables/Armada70x0Db/Dsdt.asl | 56 ++++++++++++++
Silicon/Marvell/Armada7k8k/AcpiTables/Armada80x0Db/Dsdt.asl | 59 +++++++++++++++
Silicon/Marvell/OcteonTx/AcpiTables/T91/Cn913xDbA/Dsdt.asl | 59 +++++++++++++++
10 files changed, 266 insertions(+), 24 deletions(-)

--
2.29.0

9661 - 9680 of 84490