From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web09.1991.1583232145675522291 for ; Tue, 03 Mar 2020 02:42:26 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=T0KTM+uM; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id v4so3669606wrs.8 for ; Tue, 03 Mar 2020 02:42:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=ilq6PHzBca6qGDxJSnDWFXgwbmV8XIrXcUTSdbLqSFY=; b=T0KTM+uMSdsgIAn3gnAGLWQgpvXeFNRzDWTtmLM78yTuWEtTqU1hbiyP043GfHEGSK 8g4bbBFtnCGib+Tce/9/4z4SlnpdmxhBoQu3Y6y9RHdQcQ1ZmmawWIMnIMkwnJP/QErt ICzQ6m8J+uPphMWIk6IOSUfBc3CG3RGvyrhb9WBwZwAjpB8KHR13zfP2Q6ErCntcO2ZO 5Pyb1oMiWr30p22EtoeFuKDH6dzqByjyxtix9unKLhi4Kvw2kxQZiHqt5upViCVyRGTR NMsrdM04fcNv274iNWHMWeFr7N5w3a6hWEBexgYUZkNJV1tmkw0YdJFe5b+fRySi7V3s B/mg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ilq6PHzBca6qGDxJSnDWFXgwbmV8XIrXcUTSdbLqSFY=; b=PrXLka6NoG2MrKG6QIvxPNc/Ez0nMW+dbf0fxB13N4FDpyrjafqWcV0jjKdisVlRSb Ai+boRpuKQbo/gMVZ+DkHz0QC8pBYlKmW18aUfZfVJRnoKWnZzC7HPKXRBT9SJLkmMqa lT96zUT0glGA+P8eCbCIfz5Fc1iYUHQb5KLWKuxYECizCV/KNgARiP0CHneua4sKXA0R zqjpmMoVWEVFPeQzT6W5o3UJ2YHpm7shuEvzjMzEeXvJuXqbNE1yj4aDMjFYt2a00lsP ZVUh1mt3hmAyk/iuyKms4vTj7WvLbMlRQjhk3FZhQKOxQ9xQK19fbdiOJIMc10KasHov Loww== X-Gm-Message-State: ANhLgQ2uU29qBqsqJ9zV5VrTD+nByI4oYFA+69u6z6u7/4wj21ZYlnme GN/M8gDr/HGfXaqmZbrdzCjR/2YmLKYGjny3AFKnqA== X-Google-Smtp-Source: ADFU+vvFbaWwMaPnp7I5coCfj25Cr/08Tv3zpQoA398FAD/kYYoVO67eR8ajBLlAdNZGgHwZB2LFiXY3PhqAJQ1kzBY= X-Received: by 2002:a05:6000:110b:: with SMTP id z11mr4996265wrw.252.1583232144082; Tue, 03 Mar 2020 02:42:24 -0800 (PST) MIME-Version: 1.0 References: <20200303103339.7468-1-pete@akeo.ie> <20200303103339.7468-4-pete@akeo.ie> In-Reply-To: <20200303103339.7468-4-pete@akeo.ie> From: "Ard Biesheuvel" Date: Tue, 3 Mar 2020 11:42:13 +0100 Message-ID: Subject: Re: [edk2-devel][PATCH 3/5] Platform/RPi: Make 3GB/4GB a runtime (BIOS setup) choice To: Pete Batard Cc: edk2-devel-groups-io , Leif Lindholm , =?UTF-8?Q?Philippe_Mathieu=2DDaud=C3=A9?= , Andrei Warkentin Content-Type: text/plain; charset="UTF-8" On Tue, 3 Mar 2020 at 11:33, Pete Batard wrote: > > From: Andrei Warkentin > > Currently some OSes (e.g FreeBSD) can make full use of the maximum > 4 GB of RAM a Raspberry Pi 4 can offer, whereas others (e.g. Linux) > must be restricted to only the first 3 GB. > > Previously this was a compile-time choice chosen by PcdAcpiBasicMode, > and now we make it user-selectable. The default is a 3 GB limit. > > Signed-off-by: Pete Batard > --- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c | 58 ++++++++++++++++---- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf | 8 ++- > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni | 11 ++++ > Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr | 41 ++++++++++++++ > Platform/RaspberryPi/RPi3/RPi3.dsc | 6 ++ > Platform/RaspberryPi/RPi4/RPi4.dsc | 6 ++ > Platform/RaspberryPi/RaspberryPi.dec | 4 +- > 7 files changed, 120 insertions(+), 14 deletions(-) > > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > index 2f48ca0dd758..451a419a5358 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.c > @@ -11,11 +11,13 @@ > #include > #include > #include > +#include > #include > #include > #include > #include > #include > +#include > #include > #include > #include > @@ -26,6 +28,8 @@ extern UINT8 ConfigDxeHiiBin[]; > extern UINT8 ConfigDxeStrings[]; > > STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol; > +STATIC UINT32 mModelFamily = 0; > +STATIC UINT32 mModelInstalledMB = 0; > > /* > * The GUID inside Platform/RaspberryPi/RPi3/AcpiTables/AcpiTables.inf and > @@ -129,6 +133,24 @@ SetupVariables ( > PcdSet32 (PcdCustomCpuClock, PcdGet32 (PcdCustomCpuClock)); > } > > + if (mModelFamily >= 4 && mModelInstalledMB > 3 * 1024) { > + /* > + * This allows changing PcdRamLimitTo3GB in forms. > + */ > + PcdSet32 (PcdRamMoreThan3GB, 1); > + > + Size = sizeof (UINT32); > + Status = gRT->GetVariable (L"RamLimitTo3GB", > + &gConfigDxeFormSetGuid, > + NULL, &Size, &Var32); > + if (EFI_ERROR (Status)) { > + PcdSet32 (PcdRamLimitTo3GB, PcdGet32 (PcdRamLimitTo3GB)); > + } > + } else { > + PcdSet32 (PcdRamMoreThan3GB, 0); > + PcdSet32 (PcdRamLimitTo3GB, 0); > + } > + > Size = sizeof (UINT32); > Status = gRT->GetVariable (L"SdIsArasan", > &gConfigDxeFormSetGuid, > @@ -224,7 +246,6 @@ ApplyVariables ( > UINT32 CpuClock = PcdGet32 (PcdCpuClock); > UINT32 CustomCpuClock = PcdGet32 (PcdCustomCpuClock); > UINT32 Rate = 0; > - UINT32 ModelFamily = 0; > > if (CpuClock != 0) { > if (CpuClock == 2) { > @@ -258,15 +279,18 @@ ApplyVariables ( > DEBUG ((DEBUG_INFO, "Current CPU speed is %uHz\n", Rate)); > } > > - Status = mFwProtocol->GetModelFamily (&ModelFamily); > - if (Status != EFI_SUCCESS) { > - DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi model family: %r\n", Status)); > - } else { > - DEBUG ((DEBUG_INFO, "Current Raspberry Pi model family is 0x%x\n", ModelFamily)); > + if (mModelFamily >= 4 && PcdGet32 (PcdRamLimitTo3GB) == 0) { > + Status = gDS->AddMemorySpace (EfiGcdMemoryTypeSystemMemory, BASE_1GB + SIZE_2GB, > + SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS), > + EFI_MEMORY_UC | EFI_MEMORY_WC | EFI_MEMORY_WT | EFI_MEMORY_WB); > + ASSERT_EFI_ERROR (Status); > + Status = gDS->SetMemorySpaceAttributes (BASE_1GB + SIZE_2GB, > + SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS), > + EFI_MEMORY_WB); > + ASSERT_EFI_ERROR (Status); > } > This looks a bit hacky and ad-hoc to me. What is SIZE_1GB - (SIZE_4GB - BCM2711_SOC_REGISTERS), supposed to mean? Can we declare it somewhere and document it, rather than drop it into the middle of a function call? > - > - if (ModelFamily == 3) { > + if (mModelFamily == 3) { > /* > * Pi 3: either Arasan or SdHost goes to SD card. > * > @@ -316,7 +340,7 @@ ApplyVariables ( > GpioPinFuncSet (52, Gpio48Group); > GpioPinFuncSet (53, Gpio48Group); > > - } else if (ModelFamily == 4) { > + } else if (mModelFamily == 4) { > /* > * Pi 4: either Arasan or eMMC2 goes to SD card. > */ > @@ -352,7 +376,7 @@ ApplyVariables ( > GpioPinFuncSet (39, GPIO_FSEL_ALT3); > } > } else { > - DEBUG ((DEBUG_ERROR, "Model Family %d not supported...\n", ModelFamily)); > + DEBUG ((DEBUG_ERROR, "Model Family %d not supported...\n", mModelFamily)); > } > > /* > @@ -400,6 +424,20 @@ ConfigInitialize ( > return Status; > } > > + Status = mFwProtocol->GetModelFamily (&mModelFamily); > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi model family: %r\n", Status)); > + } else { > + DEBUG ((DEBUG_INFO, "Current Raspberry Pi model family is %d\n", mModelFamily)); > + } > + > + Status = mFwProtocol->GetModelInstalledMB (&mModelInstalledMB); > + if (Status != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "Couldn't get the Raspberry Pi installed RAM size: %r\n", Status)); > + } else { > + DEBUG ((DEBUG_INFO, "Current Raspberry Pi installed RAM size is %d MB\n", mModelInstalledMB)); > + } > + > Status = SetupVariables (); > if (Status != EFI_SUCCESS) { > DEBUG ((DEBUG_ERROR, "Couldn't not setup NV vars: %r\n", Status)); > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > index dc726cc6d934..407aac89c7b3 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxe.inf > @@ -33,6 +33,7 @@ [Packages] > ArmPlatformPkg/ArmPlatformPkg.dec > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > + Silicon/Broadcom/Bcm27xx/Bcm27xx.dec > Silicon/Broadcom/Bcm283x/Bcm283x.dec > Platform/RaspberryPi/RaspberryPi.dec > EmbeddedPkg/EmbeddedPkg.dec > @@ -53,10 +54,11 @@ [Guids] > gConfigDxeFormSetGuid > > [Protocols] > - gRaspberryPiFirmwareProtocolGuid ## CONSUMES > + gRaspberryPiFirmwareProtocolGuid ## CONSUMES > gRaspberryPiConfigAppliedProtocolGuid ## PRODUCES > > [Pcd] > + gBcm27xxTokenSpaceGuid.PcdBcm27xxRegistersAddress > gBcm283xTokenSpaceGuid.PcdBcm283xRegistersAddress > gRaspberryPiTokenSpaceGuid.PcdCpuClock > gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock > @@ -70,8 +72,8 @@ [Pcd] > gRaspberryPiTokenSpaceGuid.PcdDebugShowUEFIExit > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot > - > -[FeaturePcd] > + gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB > + gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB > > [Depex] > gPcdProtocolGuid AND gRaspberryPiFirmwareProtocolGuid > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > index 9b4076635f05..830533a9dc49 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.uni > @@ -35,6 +35,17 @@ > #string STR_CHIPSET_SD_SDHOST #language en-US "Broadcom SDHOST" > #string STR_CHIPSET_SD_ARASAN #language en-US "Arasan SDHCI" > > +/* > + * Advanced configuration. > + */ > + > +#string STR_ADVANCED_FORM_TITLE #language en-US "Advanced Configuration" > + > +#string STR_ADVANCED_3GB_PROMPT #language en-US "Limit RAM to 3 GB" > +#string STR_ADVANCED_3GB_HELP #language en-US "OSes not supporting ACPI DMA constraints require a 3 GB limit or face broken xHCI USB" > +#string STR_ADVANCED_3GB_OFF #language en-US "Disabled" > +#string STR_ADVANCED_3GB_ON #language en-US "Enabled" > + > /* > * MMC/SD configuration. > */ > diff --git a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > index 60bfdbd4d17e..483edd7459c5 100644 > --- a/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > +++ b/Platform/RaspberryPi/Drivers/ConfigDxe/ConfigDxeHii.vfr > @@ -73,6 +73,18 @@ typedef struct { > UINT32 Routing; > } CHIPSET_SD_VARSTORE_DATA; > > +typedef struct { > + /* > + * Always set by ConfigDxe prior to HII init to reflect > + * platform capability. > + */ > + UINT32 Supported; > +} ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA; > + > +typedef struct { > + UINT32 Enabled; > +} ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA; > + > typedef struct { > /* > * 0 - Don't disable multi-block. > @@ -140,6 +152,16 @@ formset > name = SdIsArasan, > guid = CONFIGDXE_FORM_SET_GUID; > > + efivarstore ADVANCED_RAM_MORE_THAN_3GB_VARSTORE_DATA, > + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > + name = RamMoreThan3GB, > + guid = CONFIGDXE_FORM_SET_GUID; > + > + efivarstore ADVANCED_RAM_LIMIT_TO_3GB_VARSTORE_DATA, > + attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > + name = RamLimitTo3GB, > + guid = CONFIGDXE_FORM_SET_GUID; > + > efivarstore MMC_DISMULTI_VARSTORE_DATA, > attribute = EFI_VARIABLE_BOOTSERVICE_ACCESS | EFI_VARIABLE_RUNTIME_ACCESS | EFI_VARIABLE_NON_VOLATILE, > name = MmcDisableMulti, > @@ -193,6 +215,10 @@ formset > prompt = STRING_TOKEN(STR_CHIPSET_FORM_TITLE), > help = STRING_TOKEN(STR_NULL_STRING); > > + goto 0x1006, > + prompt = STRING_TOKEN(STR_ADVANCED_FORM_TITLE), > + help = STRING_TOKEN(STR_NULL_STRING); > + > goto 0x1003, > prompt = STRING_TOKEN(STR_MMC_FORM_TITLE), > help = STRING_TOKEN(STR_NULL_STRING); > @@ -240,6 +266,21 @@ formset > endoneof; > endform; > > + form formid = 0x1006, > + title = STRING_TOKEN(STR_ADVANCED_FORM_TITLE); > + subtitle text = STRING_TOKEN(STR_NULL_STRING); > + > + grayoutif ideqval RamMoreThan3GB.Supported == 0; > + oneof varid = RamLimitTo3GB.Enabled, > + prompt = STRING_TOKEN(STR_ADVANCED_3GB_PROMPT), > + help = STRING_TOKEN(STR_ADVANCED_3GB_HELP), > + flags = NUMERIC_SIZE_4 | INTERACTIVE | RESET_REQUIRED, > + option text = STRING_TOKEN(STR_ADVANCED_3GB_OFF), value = 0, flags = DEFAULT; > + option text = STRING_TOKEN(STR_ADVANCED_3GB_ON), value = 1, flags = 0; > + endoneof; > + endif; > + endform; > + > form formid = 0x1003, > title = STRING_TOKEN(STR_MMC_FORM_TITLE); > subtitle text = STRING_TOKEN(STR_MMC_FORM_SUBTITLE); > diff --git a/Platform/RaspberryPi/RPi3/RPi3.dsc b/Platform/RaspberryPi/RPi3/RPi3.dsc > index df5b246af1f8..48e1a32e1d24 100644 > --- a/Platform/RaspberryPi/RPi3/RPi3.dsc > +++ b/Platform/RaspberryPi/RPi3/RPi3.dsc > @@ -439,6 +439,12 @@ [PcdsDynamicHii.common.DEFAULT] > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0xff > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|L"DisplayEnableSShot"|gConfigDxeFormSetGuid|0x0|1 > > + # > + # Supporting > 3GB of memory. > + # > + gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0 > + gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|0 > + > # > # Common UEFI ones. > # > diff --git a/Platform/RaspberryPi/RPi4/RPi4.dsc b/Platform/RaspberryPi/RPi4/RPi4.dsc > index 60a5e38da778..3ce2c3e4d519 100644 > --- a/Platform/RaspberryPi/RPi4/RPi4.dsc > +++ b/Platform/RaspberryPi/RPi4/RPi4.dsc > @@ -477,6 +477,12 @@ [PcdsDynamicHii.common.DEFAULT] > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|L"DisplayEnableScaledVModes"|gConfigDxeFormSetGuid|0x0|0xff > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|L"DisplayEnableSShot"|gConfigDxeFormSetGuid|0x0|1 > > + # > + # Supporting > 3GB of memory. > + # > + gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|L"RamMoreThan3GB"|gConfigDxeFormSetGuid|0x0|0 > + gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|L"RamLimitTo3GB"|gConfigDxeFormSetGuid|0x0|1 > + > # > # Common UEFI ones. > # > diff --git a/Platform/RaspberryPi/RaspberryPi.dec b/Platform/RaspberryPi/RaspberryPi.dec > index bc378ffbfb8d..7f2c37ac9a7f 100644 > --- a/Platform/RaspberryPi/RaspberryPi.dec > +++ b/Platform/RaspberryPi/RaspberryPi.dec > @@ -57,6 +57,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] > gRaspberryPiTokenSpaceGuid.PcdCustomCpuClock|0|UINT32|0x00000016 > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableScaledVModes|0|UINT8|0x00000017 > gRaspberryPiTokenSpaceGuid.PcdDisplayEnableSShot|0|UINT32|0x00000018 > + gRaspberryPiTokenSpaceGuid.PcdRamMoreThan3GB|0|UINT32|0x00000019 > + gRaspberryPiTokenSpaceGuid.PcdRamLimitTo3GB|0|UINT32|0x0000001A > > [PcdsFeatureFlag.common] > - gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x00000019 > + gRaspberryPiTokenSpaceGuid.PcdAcpiBasicMode|FALSE|BOOLEAN|0x0000001B > -- > 2.21.0.windows.1 >