Re: [edk2-devel] [BUG/PATCH] Platform/RaspberryPi: Only enable IORT when 3G limit is disabled.

["Jeremy Linton" <jeremy.linton@arm.com>]

On 2/8/21 11:36 AM, Andrei Warkentin wrote:
> I'd rather we not set the PcdRamMoreThan3GB on a device with 1 or 2GB. I recognise you're using this to choose behavior for an old Linux version, but it's still non-sensical to the end user and it becomes a maintenance burden since the logic is no longer clear, so you might as well add a boot option to disable the IORT (and document as being for older Linux kernels) or just not install it if you explicitly being booted on a system with less RAM. I.e.
> 
>   if (PcdGet32 (PcdRamLimitTo3GB) || mModelInstalledMB < 2 * 1024) {
> 
>        return FALSE;
> 
>      }

Yah that works now too I guess. I split this off from the emmc2 patch 
where I was lowering the IORT limit to 1G to force lower allocation.

But I ended up just providing an option to disable DMA, which is more 
reliable anyway.

> ________________________________
> From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Jeremy Linton via groups.io <jeremy.linton=arm.com@groups.io>
> Sent: Monday, February 1, 2021 4:44 PM
> To: devel@edk2.groups.io <devel@edk2.groups.io>
> Cc: pete@akeo.ie <pete@akeo.ie>; Andrei Warkentin <awarkentin@vmware.com>; samer.el-haj-mahmoud@arm.com <samer.el-haj-mahmoud@arm.com>; leif@nuviainc.com <leif@nuviainc.com>; ard.biesheuvel@arm.com <ard.biesheuvel@arm.com>; Jeremy Linton <jeremy.linton@arm.com>
> Subject: [edk2-devel] [BUG/PATCH] Platform/RaspberryPi: Only enable IORT when 3G limit is disabled.
> 
> The 3G limit, and the 2G IORT are intended to solve
> the same linux problem, and limit PCI DMA operations
> to the first 3G of RAM. Older linux kernels, as
> used with RHEL/Centos, trigger an assertion*
> when a DMA operation starts that doesn't fit within
> the 2G range specified by the IORT.
> 
> The simple solution is to only enable the IORT
> when the 3G flag is disabled.
> 
> * https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fpftf%2FRPi4%2Fissues%2F123&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C92f310e07997434a267408d8c702f698%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637478162838925480%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=dKbis45PDc6ciBMWvSime3ZhQJGOwYLN0OHTnhMSvvU%3D&amp;reserved=0
> 
> Fixes: dac891da5cf3 ("Platform/RaspberryPi/AcpiTables: add a IORT ACPI table to limit XHCI DMA")
> Signed-off-by: Jeremy Linton <jeremy.linton@arm.com>
> ---
>   Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 26 +++++++++++++++++-----
>   1 file changed, 21 insertions(+), 5 deletions(-)
> 
> diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> index 19ef950f10..9581bc41e1 100644
> --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c
> @@ -217,13 +217,23 @@ SetupVariables (
>       ASSERT_EFI_ERROR (Status);
> 
>     }
> 
> 
> 
> -  if (mModelFamily >= 4 && mModelInstalledMB > 3 * 1024) {
> 
> +  if (mModelFamily >= 4) {
> 
> +    if (mModelInstalledMB > 3 * 1024) {
> 
> +      /*
> 
> +       * This allows changing PcdRamLimitTo3GB in forms.
> 
> +       */
> 
> +      Status = PcdSet32S (PcdRamMoreThan3GB, 1);
> 
> +      ASSERT_EFI_ERROR (Status);
> 
> +    } else {
> 
> +      Status = PcdSet32S (PcdRamMoreThan3GB, 0);
> 
> +      ASSERT_EFI_ERROR (Status);
> 
> +    }
> 
> +
> 
>       /*
> 
> -     * This allows changing PcdRamLimitTo3GB in forms.
> 
> +     * Enable the "3G ram limit" on 2GB device because we
> 
> +     * are going to utlize that limit as a new/old kernel
> 
> +     * flag and also disable the IORT if the 3G limit is set.
> 
>        */
> 
> -    Status = PcdSet32S (PcdRamMoreThan3GB, 1);
> 
> -    ASSERT_EFI_ERROR (Status);
> 
> -
> 
>       Size = sizeof (UINT32);
> 
>       Status = gRT->GetVariable (L"RamLimitTo3GB",
> 
>                                  &gConfigDxeFormSetGuid,
> 
> @@ -755,6 +765,12 @@ HandleDynamicNamespace (
>       DEBUG ((DEBUG_ERROR, "Found namespace table not in table list.\n"));
> 
> 
> 
>       return FALSE;
> 
> +  case SIGNATURE_32 ('I', 'O', 'R', 'T'):
> 
> +    // only enable the IORT on machines with >3G and no limit
> 
> +    // to avoid problems with rhel/centos
> 
> +    if (PcdGet32 (PcdRamLimitTo3GB)) {
> 
> +      return FALSE;
> 
> +    }
> 
>     }
> 
> 
> 
>     return TRUE;
> 
> --
> 2.13.7
> 
> 
> 
> -=-=-=-=-=-=
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#71032): https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F71032&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C92f310e07997434a267408d8c702f698%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637478162838925480%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=asFWjKVaiPH31M3SCUx8TQtzDoucFNTf8TqcYW%2Bo0uY%3D&amp;reserved=0
> Mute This Topic: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F80300339%2F4387333&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C92f310e07997434a267408d8c702f698%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637478162838925480%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Njvni%2BMK0z38Xi3rw2xwPNiMBmesole6evqVHwZB87c%3D&amp;reserved=0
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=04%7C01%7Cawarkentin%40vmware.com%7C92f310e07997434a267408d8c702f698%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637478162838925480%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=DaNks8FMX0pgeoEOY3xITNK46wbvbahKWAPnIe5kaRk%3D&amp;reserved=0 [awarkentin@vmware.com]
> -=-=-=-=-=-=
> 
> 
>