[U-Boot] [PATCH v5 04/13] sandbox: smbios: Update to support sandbox

[Alexander Graf]

On 12.06.18 23:56, Simon Glass wrote:
> Hi Alex,
> 
> On 12 June 2018 at 08:05, Alexander Graf <agraf at suse.de> wrote:
>>
>>
>> On 12.06.18 15:48, Simon Glass wrote:
>>> Hi Alex,
>>>
>>> On 12 June 2018 at 02:12, Alexander Graf <agraf at suse.de> wrote:
>>>>
>>>>
>>>> On 12.06.18 07:26, 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>
>>>>> ---
>>>>>
>>>>> Changes in v5: None
>>>>> Changes in v4: None
>>>>> Changes in v3:
>>>>> - Drop incorrect map_sysmem() in write_smbios_table()
>>>>>
>>>>> Changes in v2: None
>>>>>
>>>>>  lib/smbios.c | 32 ++++++++++++++++++++++++--------
>>>>>  1 file changed, 24 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/lib/smbios.c b/lib/smbios.c
>>>>> index df3d26b071..fc3dabcbc1 100644
>>>>> --- a/lib/smbios.c
>>>>> +++ b/lib/smbios.c
>>>>> @@ -6,6 +6,7 @@
>>>>>   */
>>>>>
>>>>>  #include <common.h>
>>>>> +#include <mapmem.h>
>>>>>  #include <smbios.h>
>>>>>  #include <tables_csum.h>
>>>>>  #include <version.h>
>>>>> @@ -72,9 +73,10 @@ static int smbios_string_table_len(char *start)
>>>>>
>>>>>  static int smbios_write_type0(ulong *current, int handle)
>>>>
>>>> Please change the function argument to indicate that we're no longer
>>>> dealing with pointers, but instead with "u-boot physical addresses".
>>>>
>>>> Same for the other functions obviously :).
>>>
>>> That actually hasn't changed. We are currently passing a U-Boot
>>> address around and it is a ulong, as it normally is in U-Boot. What
>>> has changed is that sandbox does not have a direct mapping between
>>> U-Boot address and memory address, so we have to do the mapping.
>>>
>>> While it is try that the ulong can be converted to a pointer with a
>>> cast normally, this is not possible with sandbox, so things that need
>>> to convert the ulong to a pointer need to do a conversion.
>>
>> Oh, I missed the * in *current. So it already does get passed as a
>> number which then is cast back into a pointer.
>>
>> That however means that the smbios tables are now u-boot address space
>> relative. So anything that tries to read them from within EFI context
>> will explode.
> 
> Aren't these tables for the Linux kernel? If so, then this doesn't matter.
> 
> But if EFI reads them, we are in trouble. We cannot always put a
> 64-bit address into a 32-bit word.
> 
> I suppose we could support it on 32-bit sandbox, but not a lot of
> people use that.

Sorry, I must've misremembered. I can not see a single reference of a
physical address in the SMBIOS code. So your patch looks correct to me.


Alex