[edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision

[Leif Lindholm]

On Mon, Oct 14, 2019 at 02:08:10PM +0200, Philippe Mathieu-Daudé wrote:
> On 10/14/19 2:03 PM, Pete Batard wrote:
> > On 2019.10.14 12:53, Philippe Mathieu-Daudé wrote:
> > > On 10/14/19 1:44 PM, Pete Batard wrote:
> > > > Hi Philippe,
> > > > 
> > > > On 2019.10.14 11:01, Philippe Mathieu-Daudé wrote:
> > > > > Hi Pete,
> > > > > 
> > > > > On 10/11/19 1:07 PM, Pete Batard wrote:
> > > > > > The board revision is the proper channel to use to
> > > > > > detect the amount of
> > > > > > RAM available as bits [20-22] report the effective RAM
> > > > > > size for the board
> > > > > > starting with 256 MB (000b) and doubling in size for each value.
> > > > > > 
> > > > > > Signed-off-by: Pete Batard <pete at akeo.ie>
> > > > > > Reviewed-by: Leif Lindholm <leif.lindholm at linaro.org>
> > > > > > ---
> > > > > > Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > > | 18 ++++++++++++------
> > > > > >   1 file changed, 12 insertions(+), 6 deletions(-)
> > > > > > 
> > > > > > diff --git a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > > 
> > > > > > index b5dcff897a59..5abc82b8d363 100644
> > > > > > --- a/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > > 
> > > > > > +++ b/Platform/RaspberryPi/RPi3/Drivers/PlatformSmbiosDxe/PlatformSmbiosDxe.c
> > > > > > 
> > > > > > @@ -866,16 +866,22 @@ MemArrMapInfoUpdateSmbiosType19 (
> > > > > >     )
> > > > > >   {
> > > > > >     EFI_STATUS Status;
> > > > > > -  UINT32 Base;
> > > > > > -  UINT32 Size;
> > > > > > +  UINT32 BoardRevision = 0;
> > > > > > -  Status = mFwProtocol->GetArmMem (&Base, &Size);
> > > > > > +  // Note: Type 19 addresses are expressed in KB, not bytes
> > 
> > Comment to add:
> > 
> > // The memory layout used in all known Pi SoC's starts at 0
> 
> Thanks, Leif do you mind amending this comment?

No issue with that.
Have the rest of today off, but wil get back to this set tomorrow
morning.

/
    Leif

> Reviewed-by: Philippe Mathieu-Daude <philmd at redhat.com>
> 
> > > > > > +  mMemArrMapInfoType19.StartingAddress = 0;
> > > > > 
> > > > > Now you assume the ARM base RAM address is always 0, why?
> > > > 
> > > > Because, in the case that is of interest to us here (Broadcom
> > > > SoCs used for the various Raspberry Pi platforms), this is what
> > > > the documentation says.
> > > > 
> > > > If you look at something like https://www.raspberrypi.org/app/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> > > > (which, as per the Pi Foundation, also applies to the later
> > > > models) and especially the memory layout graphic that you see
> > > > early in the document, you can find that the ARM base RAM
> > > > address is indeed set to 0 always.
> > > > 
> > > > At this stage, we have no reason to think that we are going to
> > > > contend with a model where the RAM base address isn't 0.
> > > 
> > > OK. I have no idea what are Broadcom plans, I read somewhere they
> > > were going to use tricks to allow more than 4GB of RAM on the
> > > Raspberry Pi 4, so I was wondering, since they provide a firmware
> > > API call to get the RAM base address.
> > 
> > Well the thing is, this patch comes straight from work that is being
> > carried out to add support for the Pi 4 to edk2-platforms. So we pretty
> > much already validated that 0 is what needs to be used for the Pi 4 as
> > well...
> 
> OK, good news :)
> 
> > > Maybe you can add a comment that we expect all following boards to
> > > use RAM base at 0 (simply replying to this email, and eventually
> > > Leif would amend it previous to push).
> > 
> > Sure. Comment added above.
> 
> Thanks!
> 
> > Regards,
> > 
> > /Pete
> > 
> > > 
> > > Otherwise your patch looks OK.
> > > 
> > > Regards,
> > > 
> > > Phil.
> > > 
> > > > > > +  // The minimum RAM size used on any Raspberry Pi model is 256 MB
> > > > > > +  mMemArrMapInfoType19.EndingAddress = 256 * 1024;
> > > > > > +  Status = mFwProtocol->GetModelRevision (&BoardRevision);
> > > > > >     if (Status != EFI_SUCCESS) {
> > > > > > -    DEBUG ((DEBUG_ERROR, "Couldn't get the ARM memory
> > > > > > size: %r\n", Status));
> > > > > > +    DEBUG ((DEBUG_WARNING, "Couldn't get the board
> > > > > > memory size - defaulting to 256 MB: %r\n", Status));
> > > > > >     } else {
> > > > > > -    mMemArrMapInfoType19.StartingAddress = Base / 1024;
> > > > > > -    mMemArrMapInfoType19.EndingAddress = (Base + Size - 1) / 1024;
> > > > > > +    // www.raspberrypi.org/documentation/hardware/raspberrypi/revision-codes/README.md
> > > > > > 
> > > > > > +    // Bits [20-22] indicate the amount of memory
> > > > > > starting with 256MB (000b)
> > > > > > +    // and doubling in size for each value (001b = 512
> > > > > > MB, 010b = 1GB, etc.)
> > > > > > +    mMemArrMapInfoType19.EndingAddress <<=
> > > > > > (BoardRevision >> 20) & 0x07;
> > > > > >     }
> > > > > > +  mMemArrMapInfoType19.EndingAddress -= 1;
> > > > > >     LogSmbiosData
> > > > > > ((EFI_SMBIOS_TABLE_HEADER*)&mMemArrMapInfoType19,
> > > > > > mMemArrMapInfoType19Strings, NULL);
> > > > > >   }
> > > > > > 
> > > > > 
> > > > 
> > > > 
> > > > 
> > > > 
> > > 
> > 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#48897): https://edk2.groups.io/g/devel/message/48897
Mute This Topic: https://groups.io/mt/34485903/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-