[U-Boot] [PATCH v11 3/6] sandbox: smbios: Update to support sandbox

[Alexander Graf]

> Am 22.10.2018 um 18:49 schrieb Simon Glass <sjg at chromium.org>:
> 
> Hi Alex,
> 
>> On 19 October 2018 at 01:27, Alexander Graf <agraf at suse.de> wrote:
>> 
>> 
>>> On 19.10.18 05:25, Simon Glass wrote:
>>> Hi Alex,
>>> 
>>>> On 16 October 2018 at 06:55, Alexander Graf <agraf at suse.de> wrote:
>>>> 
>>>> 
>>>>> On 15.10.18 16:17, Simon Glass wrote:
>>>>> At present this code casts addresses to pointers so cannot be used with
>>>>> sandbox. Update it to use mapmem instead.
>>>>> 
>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
>>>> 
>>>> Unfortunately this won't work. The SMBIOS2 structure itself contains a
>>>> physical pointer to the target address (which in EFI lands really has to
>>>> be linear physical pointer). This pointer gets set based on "addr" in
>>>> write_smbios_table():
>>>> 
>>>>        tables = addr;
>>>>        [...]
>>>>        se->struct_table_address = tables;
>>> 
>>> Does that actually matter? We will never actually boot anything on
>>> sandbox that will use that address.
>> 
>> Why not? We can boot the UEFI Shell today - and that can use the "address".
> 
> OK, so UEFI shell uses the SMBIOS tables? I didn't know that.

There is a command to print the contents of smbios tables - and that one does use them :).

> 
>> 
>>> Also sandbox addresses are always <4GB (they start at 0).
>> 
>> U-Boot addresses are <4GB but pointers are >4GB.
> 
> Actually I have applied a patch to fix that.
> 
>> 
>> U-Boot addresses are also a U-Boot internal concept. We don't have any
>> means to expose their semantics to UEFI applications. So anything inside
>> a data structure that is shared with UEFI that says "address" really
>> means "pointer".
>> 
>> So since any UEFI application that we execute is only aware of pointers,
>> we can not represent the pointer in the field. And that breaks every
>> consumer of it.
> 
> Yes we must use pointers in this case, I agree. This needs a
> map_sysmem() call and a check that it does not overflow.
> 
>> 
>>> 
>>>> 
>>>> So I think the only thing we can do for now is to just graciously fail
>>>> SMBIOS generation (maybe only on sandbox?) when we can not find a
>>>> pointer that is < U32_MAX.
>>>> 
>>>> The shortcoming above was fixed with SMBIOS3, so the "good" path forward
>>>> would be to add SMBIOS3 support and just not rely on 32bit pointers at
>>>> all. I don't remember OTOH if SMBIOS3 stores offsets or 64bit pointers
>>>> to the tables. Depending on that we can either use your maps or we can't.
>>> 
>>> Maybe I prefer device tree as it avoid this sort of thing :-)
>> 
>> DT is orthogonal to SMBIOS. SMBIOS describes OEM information, such as
>> "chassy name", "how DIMM slots are populated", etc.
>> 
>> Sure, you could start and map SMBIOS information into a Linux specific
>> DT node, but what's the point if we have SMBIOS already ;).
> 
> I'm not suggesting we do it, just whining about these legacy data structures :-)

:)

Alex

> 
> Regards,
> Simon