Hi Heinrich,
On 18 February 2018 at 05:14, Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
> On 12/04/2017 10:28 PM, 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 <s...@chromium.org>
>> ---
>>
>> Changes in v2: None
>>
>> lib/smbios.c | 38 +++++++++++++++++++++++++++++---------
>> 1 file changed, 29 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/smbios.c b/lib/smbios.c
>> index 8f19ad89c1..56481d448d 100644
>> --- a/lib/smbios.c
>> +++ b/lib/smbios.c
>> @@ -7,6 +7,7 @@
>> */
>>
>> #include <common.h>
>> +#include <mapmem.h>
>> #include <smbios.h>
>> #include <tables_csum.h>
>> #include <version.h>
>> @@ -75,9 +76,10 @@ static int smbios_string_table_len(char *start)
>>
>> static int smbios_write_type0(ulong *current, int handle)
>> {
>> - struct smbios_type0 *t = (struct smbios_type0 *)*current;
>> + struct smbios_type0 *t;
>> int len = sizeof(struct smbios_type0);
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type0));
>> fill_smbios_header(t, SMBIOS_BIOS_INFORMATION, len, handle);
>> t->vendor = smbios_add_string(t->eos, "U-Boot");
>> @@ -104,16 +106,18 @@ static int smbios_write_type0(ulong *current, int
>> handle)
>>
>> len = t->length + smbios_string_table_len(t->eos);
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>>
>> static int smbios_write_type1(ulong *current, int handle)
>> {
>> - struct smbios_type1 *t = (struct smbios_type1 *)*current;
>> + struct smbios_type1 *t;
>> int len = sizeof(struct smbios_type1);
>> char *serial_str = env_get("serial#");
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type1));
>> fill_smbios_header(t, SMBIOS_SYSTEM_INFORMATION, len, handle);
>> t->manufacturer = smbios_add_string(t->eos,
>> CONFIG_SMBIOS_MANUFACTURER);
>> @@ -125,15 +129,17 @@ static int smbios_write_type1(ulong *current, int
>> handle)
>>
>> len = t->length + smbios_string_table_len(t->eos);
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>>
>> static int smbios_write_type2(ulong *current, int handle)
>> {
>> - struct smbios_type2 *t = (struct smbios_type2 *)*current;
>> + struct smbios_type2 *t;
>> int len = sizeof(struct smbios_type2);
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type2));
>> fill_smbios_header(t, SMBIOS_BOARD_INFORMATION, len, handle);
>> t->manufacturer = smbios_add_string(t->eos,
>> CONFIG_SMBIOS_MANUFACTURER);
>> @@ -143,15 +149,17 @@ static int smbios_write_type2(ulong *current, int
>> handle)
>>
>> len = t->length + smbios_string_table_len(t->eos);
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>>
>> static int smbios_write_type3(ulong *current, int handle)
>> {
>> - struct smbios_type3 *t = (struct smbios_type3 *)*current;
>> + struct smbios_type3 *t;
>> int len = sizeof(struct smbios_type3);
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type3));
>> fill_smbios_header(t, SMBIOS_SYSTEM_ENCLOSURE, len, handle);
>> t->manufacturer = smbios_add_string(t->eos,
>> CONFIG_SMBIOS_MANUFACTURER);
>> @@ -163,6 +171,7 @@ static int smbios_write_type3(ulong *current, int handle)
>>
>> len = t->length + smbios_string_table_len(t->eos);
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>> @@ -201,9 +210,10 @@ static void smbios_write_type4_dm(struct smbios_type4
>> *t)
>>
>> static int smbios_write_type4(ulong *current, int handle)
>> {
>> - struct smbios_type4 *t = (struct smbios_type4 *)*current;
>> + struct smbios_type4 *t;
>> int len = sizeof(struct smbios_type4);
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type4));
>> fill_smbios_header(t, SMBIOS_PROCESSOR_INFORMATION, len, handle);
>> t->processor_type = SMBIOS_PROCESSOR_TYPE_CENTRAL;
>> @@ -217,32 +227,37 @@ static int smbios_write_type4(ulong *current, int
>> handle)
>>
>> len = t->length + smbios_string_table_len(t->eos);
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>>
>> static int smbios_write_type32(ulong *current, int handle)
>> {
>> - struct smbios_type32 *t = (struct smbios_type32 *)*current;
>> + struct smbios_type32 *t;
>> int len = sizeof(struct smbios_type32);
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type32));
>> fill_smbios_header(t, SMBIOS_SYSTEM_BOOT_INFORMATION, len, handle);
>>
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>>
>> static int smbios_write_type127(ulong *current, int handle)
>> {
>> - struct smbios_type127 *t = (struct smbios_type127 *)*current;
>> + struct smbios_type127 *t;
>> int len = sizeof(struct smbios_type127);
>>
>> + t = map_sysmem(*current, len);
>> memset(t, 0, sizeof(struct smbios_type127));
>> fill_smbios_header(t, SMBIOS_END_OF_TABLE, len, handle);
>>
>> *current += len;
>> + unmap_sysmem(t);
>>
>> return len;
>> }
>> @@ -271,7 +286,7 @@ ulong write_smbios_table(ulong addr)
>> /* 16 byte align the table address */
>> addr = ALIGN(addr, 16);
>>
>> - se = (struct smbios_entry *)(uintptr_t)addr;
>> + se = map_sysmem(addr, sizeof(struct smbios_entry));
>> memset(se, 0, sizeof(struct smbios_entry));
>>
>> addr += sizeof(struct smbios_entry);
>> @@ -280,9 +295,13 @@ ulong write_smbios_table(ulong addr)
>>
>> /* populate minimum required tables */
>> for (i = 0; i < ARRAY_SIZE(smbios_write_funcs); i++) {
>> - int tmp = smbios_write_funcs[i]((ulong *)&addr, handle++);
>> + ulong *ptr = map_sysmem(addr, 0);
>
> This replaces &addr by addr and breaks booting on ARM64.
>
> Did you mean:
> ulong *ptr = map_sysmem((phys_addr_t)&addr, 0);
No, actually I don't think this is really needed at all since the
version is done by the functions it calls.
>
> The coding would get much easier to maintain if the map_sysmem()
> function would be eliminated from U-Boot.
>
> Why don't you simply use the address range that mmap() has provided?
> This would avoid double book keeping.
It is on my mind. But the address is used in U-Boot (e.g. in scripts).
Every other board has a defined and known SDRAM start address, and I
think it is best that sandbox has this too. It is also a good marker
of the conversion between an address and a pointer. IMO using a cast
is a bit ad-hoc.
Regards,
Simon
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
