From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by mx.groups.io with SMTP id smtpd.web10.2613.1571055439392477219 for ; Mon, 14 Oct 2019 05:17:19 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=hSN9xfZ1; spf=pass (domain: linaro.org, ip: 209.85.221.46, mailfrom: leif.lindholm@linaro.org) Received: by mail-wr1-f46.google.com with SMTP id j11so19487673wrp.1 for ; Mon, 14 Oct 2019 05:17:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=kCUVJwZgJURMhbQWsDZ/gH70zb1PQr8hEqZefyIhj6g=; b=hSN9xfZ10wDKoWG3whuhtapjYTQLWsk3eIAg6faAhw7bQamA7NVrMW7TBY2QLgvRrN Gxu1YxOki+UfXHxvVFJmJ7tcN2O8eSrW4oD+Dw0wLaFpFyG+9FA6EwfoWjvkcZlVHq5P 86+w6ML+jwkvvpKXU4ygBZ4NxS7fy5LW5K8MzC2g5Y5HjiCggumV3M4vqJiSeVX0XtmC gRwa4kqNyfXR1uffLHsNqI91P7veIFdQvFmCO6DLbRl/d12dhEHn2MX3iKG1yXuMK9wr p4YbLmB32Bc1JzmphmzBxi2TBF83tvihR6ZcpB+KsXZdtq3UZqNiBfeIErwhKI2SxIHG T6mA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to:user-agent; bh=kCUVJwZgJURMhbQWsDZ/gH70zb1PQr8hEqZefyIhj6g=; b=mVlu6CIHNj3wwj445qOeSzGzI66EDQLKexPLWAwbPAVgsMLiB0gTG+zsD4QHyhypdG F9N33KZU/EAjxukSuj9hNje8a423Ei+j71P8UDOWASo4V8sUluTO27H1PU8JmJEuru8R RvhHpq+tBNpLfG6OKD8XPxSbYxaq/sVecP92bLgt7gu0LfYxLFGsiLIdajWM0KCagFvK 7S9F6VylwnH3e6IyPF3OzSRmC0S456mHxDJQFLLvLzQNFU4lPpL486c00BJ2Fj4+MOGx rQz8m4ZInck8OmRlgZtZUFTlMQk5UWInDrh40CS6FWLQ7jEUcwVJXS1rKZ5xtNnXJT1g MDyA== X-Gm-Message-State: APjAAAV5EwPh5fwHEaSs+1E0xhEMkeNFIdozct3jv9FhE/vABs+O+RfD E82SRDj0ihT5H/Ac71KqyHn9fw== X-Google-Smtp-Source: APXvYqyy0ZAiJveevJXImm7VtaH50otGwTNo1dGuCMi48XjQW7FR17kP6e75LLWmIfBJqnXsAKrF+A== X-Received: by 2002:adf:f24c:: with SMTP id b12mr16982949wrp.82.1571055437860; Mon, 14 Oct 2019 05:17:17 -0700 (PDT) Return-Path: Received: from bivouac.eciton.net (bivouac.eciton.net. [2a00:1098:0:86:1000:23:0:2]) by smtp.gmail.com with ESMTPSA id y186sm37401388wmd.26.2019.10.14.05.17.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 14 Oct 2019 05:17:17 -0700 (PDT) Date: Mon, 14 Oct 2019 13:17:15 +0100 From: "Leif Lindholm" To: Philippe =?iso-8859-1?Q?Mathieu-Daud=E9?= Cc: Pete Batard , devel@edk2.groups.io, ard.biesheuvel@linaro.org Subject: Re: [edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/RPi3/PlatformSmbiosDxe: Derive RAM size from board revision Message-ID: <20191014121715.GD25504@bivouac.eciton.net> References: <20191011110746.1952-1-pete@akeo.ie> <20191011110746.1952-6-pete@akeo.ie> <6782958e-fe36-8296-803a-ba8b90e4f985@redhat.com> <2c9a0255-3a90-cfde-f5bd-1893c01b299a@akeo.ie> <25a46221-1fc5-7795-2b5f-7564809579ae@redhat.com> MIME-Version: 1.0 In-Reply-To: <25a46221-1fc5-7795-2b5f-7564809579ae@redhat.com> User-Agent: Mutt/1.10.1 (2018-07-13) Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit 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 > > > > > > Reviewed-by: Leif Lindholm > > > > > > --- > > > > > > 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 > > > > > > > +  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); > > > > > >   } > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >