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

[Simon Glass]

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.

>
>> 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 :-)

Regards,
Simon