[PATCH 1/1] sandbox: ensure that state->ram_buf is in low memory

[Simon Glass]

Hi Heinrich,

On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 5/13/21 4:36 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Wed, 12 May 2021 at 15:28, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> >>
> >> Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass <sjg at chromium.org>:
> >>> Hi Heinrich,
> >>>
> >>> On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt <xypron.glpk at gmx.de>
> >>> wrote:
> >>>>
> >>>> Addresses in state->ram_buf must be in the low 4 GiB of the address
> >>> space.
> >>>> Otherwise we cannot correctly fill SMBIOS tables. This shows up in
> >>> warnings
> >>>> like:
> >>>>
> >>>>      WARNING: SMBIOS table_address overflow 7f752735e020
> >>>
> >>> This sounds like a bug in the smbios-table code. For sandbox it should
> >>> perhaps use addresses instead of pointers.
> >>>
> >>> I think that code (that I unfortunately wrote) was an expeditious way
> >>> of getting it running, but is not correct.
> >>
> >> The field you are filling is only 32bit wide. I wonder how that table is meant to work on systems where the lowest memory address is above 4 GiB. Such ARMv8 systems exist.
> >
> > map_to_sysmem() will give you a 32-bit wide address. Yes SMBIOS is
> > legacy and designed for 4GB.
>
> I know map_to_sysmem(). But you wrote in lib/smbios.c:487:
>
> /*
>   * We must use a pointer here so things work correctly on sandbox. The
>   * user of this table is not aware of the mapping of addresses to
>   * sandbox's DRAM buffer.
>   */
>
> For testing you could start a binary with command 'bootefi' or 'booti'
> and that binary would analyze the table. So yes, your comment holds true
> and you must not use map_to_sysmem() here.

Yes, that is a good point. Perhaps I was not mad when I wrote that.
What is using the tables on sandbox?

Regards,
SImon