[PATCH 1/1] sandbox: ensure that state->ram_buf is in low memory
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
Ensure that state->ram_buf is initialized by the first invocation of os_malloc().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- arch/sandbox/cpu/start.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index e87365e800..1388dba895 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -439,6 +439,14 @@ int main(int argc, char *argv[]) int size; int ret;
+ /* + * This must be the first invocation of os_malloc() to have + * state->ram_buf in the low 4 GiB. + */ + ret = state_init(); + if (ret) + goto err; + /* * Copy argv[] so that we can pass the arguments in the original * sequence when resetting the sandbox. @@ -453,10 +461,6 @@ int main(int argc, char *argv[]) gd = &data; gd->arch.text_base = os_find_text_base();
- ret = state_init(); - if (ret) - goto err; - state = state_get_current(); if (os_parse_args(state, argc, argv)) return 1; -- 2.30.2
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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.
Ensure that state->ram_buf is initialized by the first invocation of os_malloc().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/sandbox/cpu/start.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Regards, Simon
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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 7f752735e020This 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.
Best regards
Heinrich
Ensure that state->ram_buf is initialized by the first invocation of os_malloc().
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
arch/sandbox/cpu/start.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-)
Regards, Simon
Hi Heinrich,
On Wed, 12 May 2021 at 15:28, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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 7f752735e020This 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.
Regards, Simon
On 5/13/21 4:36 PM, Simon Glass wrote:
Hi Heinrich,
On Wed, 12 May 2021 at 15:28, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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 7f752735e020This 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.
Best regards
Heinrich
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt xypron.glpk@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@gmx.de wrote:
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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 7f752735e020This 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
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt xypron.glpk@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@gmx.de wrote:
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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 7f752735e020This 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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00 *.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
Best regards
Heinrich
Hi Heinrich,
On Fri, 14 May 2021 at 00:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt xypron.glpk@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@gmx.de wrote:
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt xypron.glpk@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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00 *.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
OK, I am not sure what to make of that except that it is every bit as impenetrable as I would expect from UEFI.
Is this on a Risc-V host? I am not sure why the first alloc would get the requested address and not the second. Is it because we specify 0x10000000 as the requested address? Is that what you actually get in this case?
If we drop that value on subsequent calls to mmap(), does it fix the problem?
I think we should update the commit message to say we are hoping to get 0x10000000 for the RAM buffer (although of course there is a 4K region at the start of it) and that xx arch provides large addresses for subsequent mmap()s.
Also, how about upgrading the smbios warning to an error. I think that is what it is and it should stop execution.
With that said:
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon
Am 14. Mai 2021 22:44:32 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Fri, 14 May 2021 at 00:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt
xypron.glpk@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@gmx.de wrote:
Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass
> Hi Heinrich, > > On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt
> 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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the
cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00
*.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
OK, I am not sure what to make of that except that it is every bit as impenetrable as I would expect from UEFI.
This is a dump of the SMBIOS tables. Why do you refer to UEFI?
Is this on a Risc-V host? I am not sure why the first alloc would get the requested address and not the second. Is it because we specify 0x10000000 as the requested address? Is that what you actually get in this case?
The observation is reproducible on x86_64.
Two mmap calls cannot receive the same address without calling munmap in between.
The first call *might* return the address 0x10000000 that you ask for.
If the first call cannot receive it because it is not available, why should subsequent calls succeed?
If we drop that value on subsequent calls to mmap(), does it fix the problem?
If you drop that value you get a random address. What would be better in this case?
I think we should update the commit message to say we are hoping to get 0x10000000 for the RAM buffer (although of course there is a 4K region at the start of it) and that xx arch provides large addresses for subsequent mmap()s.
What 4 KiB region are you referring to?
Why do you expect any specific address in subsequent calls? The man-page states "If another mapping already exists there, the kernel picks a new address that may or may not depend on the hint."
For performance reasons the kernel should not care about the hint in this case.
Also, how about upgrading the smbios warning to an error. I think that is what it is and it should stop execution.
For errors we use log_err(). But where is the error? mmap() comes with absolutely no guarantee that it will assign the address that you are requesting or anything close to it.
Smbios is not important enough to make the sandbox unusable in this case.
Best regards
Heinrich
With that said:
Reviewed-by: Simon Glass sjg@chromium.org
Regards, Simon
Hi Heinrich,
On Fri, 14 May 2021 at 18:34, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 14. Mai 2021 22:44:32 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Fri, 14 May 2021 at 00:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt
xypron.glpk@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@gmx.de wrote:
> > Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass
>> Hi Heinrich, >> >> On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt
>> 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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the
cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00
*.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
OK, I am not sure what to make of that except that it is every bit as impenetrable as I would expect from UEFI.
This is a dump of the SMBIOS tables. Why do you refer to UEFI?
How do you fit so many questions in one email? :-)
I assumed smbiosview is a UEFI command because of the prompt. I have never heard of the command.
Is this on a Risc-V host? I am not sure why the first alloc would get the requested address and not the second. Is it because we specify 0x10000000 as the requested address? Is that what you actually get in this case?
The observation is reproducible on x86_64.
Two mmap calls cannot receive the same address without calling munmap in between.
The first call *might* return the address 0x10000000 that you ask for.
If the first call cannot receive it because it is not available, why should subsequent calls succeed?
The goal is to keep the addresses small since 64-bit addresses are a pain when debugging. I had hoped that mmap() would try to find an address near the one requested, for subsequent calls, but perhaps that hope is forlorn. It certainly doesn't seem to do that on x86.
If we drop that value on subsequent calls to mmap(), does it fix the problem?
If you drop that value you get a random address. What would be better in this case?
I just tried it on x86 and we get a 'random' address anyway, whatever is passed, once it has used the requested address. I think the address hint only works if it works. If not, then linux starts allocating from another address. Perhaps this linux behaviour has changed in recent years.
I think we should update the commit message to say we are hoping to get 0x10000000 for the RAM buffer (although of course there is a 4K region at the start of it) and that xx arch provides large addresses for subsequent mmap()s.
What 4 KiB region are you referring to?
If you look at os_malloc() you will see that it adds a 4KB header before each block it allocates.
Why do you expect any specific address in subsequent calls? The man-page states "If another mapping already exists there, the kernel picks a new address that may or may not depend on the hint."
For performance reasons the kernel should not care about the hint in this case.
OK.
Also, how about upgrading the smbios warning to an error. I think that is what it is and it should stop execution.
For errors we use log_err(). But where is the error? mmap() comes with absolutely no guarantee that it will assign the address that you are requesting or anything close to it.
Smbios is not important enough to make the sandbox unusable in this case.
Well if the tables are invalid we should not pass them on...can we at least do that?
Regards, Simon [..]
On 5/15/21 5:19 PM, Simon Glass wrote:
Hi Heinrich,
On Fri, 14 May 2021 at 18:34, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 14. Mai 2021 22:44:32 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Fri, 14 May 2021 at 00:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt
xypron.glpk@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@gmx.de wrote:
>> >> Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass
>>> Hi Heinrich, >>> >>> On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt
>>> 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 addressesto
* 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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the
cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00
*.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
OK, I am not sure what to make of that except that it is every bit as impenetrable as I would expect from UEFI.
This is a dump of the SMBIOS tables. Why do you refer to UEFI?
How do you fit so many questions in one email? :-)
I assumed smbiosview is a UEFI command because of the prompt. I have never heard of the command.
Is this on a Risc-V host? I am not sure why the first alloc would get the requested address and not the second. Is it because we specify 0x10000000 as the requested address? Is that what you actually get in this case?
The observation is reproducible on x86_64.
Two mmap calls cannot receive the same address without calling munmap in between.
The first call *might* return the address 0x10000000 that you ask for.
If the first call cannot receive it because it is not available, why should subsequent calls succeed?
The goal is to keep the addresses small since 64-bit addresses are a pain when debugging. I had hoped that mmap() would try to find an address near the one requested, for subsequent calls, but perhaps that hope is forlorn. It certainly doesn't seem to do that on x86.
If we drop that value on subsequent calls to mmap(), does it fix the problem?
If you drop that value you get a random address. What would be better in this case?
I just tried it on x86 and we get a 'random' address anyway, whatever is passed, once it has used the requested address. I think the address hint only works if it works. If not, then linux starts allocating from another address. Perhaps this linux behaviour has changed in recent years.
I think we should update the commit message to say we are hoping to get 0x10000000 for the RAM buffer (although of course there is a 4K region at the start of it) and that xx arch provides large addresses for subsequent mmap()s.
What 4 KiB region are you referring to?
If you look at os_malloc() you will see that it adds a 4KB header before each block it allocates.
Why do you expect any specific address in subsequent calls? The man-page states "If another mapping already exists there, the kernel picks a new address that may or may not depend on the hint."
For performance reasons the kernel should not care about the hint in this case.
OK.
Also, how about upgrading the smbios warning to an error. I think that is what it is and it should stop execution.
For errors we use log_err(). But where is the error? mmap() comes with absolutely no guarantee that it will assign the address that you are requesting or anything close to it.
Smbios is not important enough to make the sandbox unusable in this case.
Well if the tables are invalid we should not pass them on...can we at least do that?
That would be separate patch?
Best regards
Heinrich
Regards, Simon [..]
Hi Heinrich,
On Sat, 15 May 2021 at 09:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/15/21 5:19 PM, Simon Glass wrote:
Hi Heinrich,
On Fri, 14 May 2021 at 18:34, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 14. Mai 2021 22:44:32 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Fri, 14 May 2021 at 00:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt
xypron.glpk@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@gmx.de wrote:
>>> >>> Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass
>>>> Hi Heinrich, >>>> >>>> On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt
>>>> 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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the
cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00
*.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
OK, I am not sure what to make of that except that it is every bit as impenetrable as I would expect from UEFI.
This is a dump of the SMBIOS tables. Why do you refer to UEFI?
How do you fit so many questions in one email? :-)
I assumed smbiosview is a UEFI command because of the prompt. I have never heard of the command.
Is this on a Risc-V host? I am not sure why the first alloc would get the requested address and not the second. Is it because we specify 0x10000000 as the requested address? Is that what you actually get in this case?
The observation is reproducible on x86_64.
Two mmap calls cannot receive the same address without calling munmap in between.
The first call *might* return the address 0x10000000 that you ask for.
If the first call cannot receive it because it is not available, why should subsequent calls succeed?
The goal is to keep the addresses small since 64-bit addresses are a pain when debugging. I had hoped that mmap() would try to find an address near the one requested, for subsequent calls, but perhaps that hope is forlorn. It certainly doesn't seem to do that on x86.
If we drop that value on subsequent calls to mmap(), does it fix the problem?
If you drop that value you get a random address. What would be better in this case?
I just tried it on x86 and we get a 'random' address anyway, whatever is passed, once it has used the requested address. I think the address hint only works if it works. If not, then linux starts allocating from another address. Perhaps this linux behaviour has changed in recent years.
I think we should update the commit message to say we are hoping to get 0x10000000 for the RAM buffer (although of course there is a 4K region at the start of it) and that xx arch provides large addresses for subsequent mmap()s.
What 4 KiB region are you referring to?
If you look at os_malloc() you will see that it adds a 4KB header before each block it allocates.
Why do you expect any specific address in subsequent calls? The man-page states "If another mapping already exists there, the kernel picks a new address that may or may not depend on the hint."
For performance reasons the kernel should not care about the hint in this case.
OK.
Also, how about upgrading the smbios warning to an error. I think that is what it is and it should stop execution.
For errors we use log_err(). But where is the error? mmap() comes with absolutely no guarantee that it will assign the address that you are requesting or anything close to it.
Smbios is not important enough to make the sandbox unusable in this case.
Well if the tables are invalid we should not pass them on...can we at least do that?
That would be separate patch?
Yes indeed.
Regards, Simon
Hi Heinrich,
On Sat, 15 May 2021 at 09:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/15/21 5:19 PM, Simon Glass wrote:
Hi Heinrich,
On Fri, 14 May 2021 at 18:34, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Am 14. Mai 2021 22:44:32 MESZ schrieb Simon Glass sjg@chromium.org:
Hi Heinrich,
On Fri, 14 May 2021 at 00:11, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 5/14/21 1:56 AM, Simon Glass wrote:
Hi Heinrich,
On Thu, 13 May 2021 at 08:53, Heinrich Schuchardt
xypron.glpk@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@gmx.de wrote:
>>> >>> Am 12. Mai 2021 18:01:17 MESZ schrieb Simon Glass
>>>> Hi Heinrich, >>>> >>>> On Tue, 11 May 2021 at 13:03, Heinrich Schuchardt
>>>> 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?
The UEFI shell has a command smbiosview to display the SMBIOS table.
With the current U-Boot sandbox it crashes. I do not know what the
cause is:
FS0:> smbiosview
Segmentation violation pc = 0x7fb0df88d17e, pc_reloc = 0x7fb0cf88d17e
UEFI image [0x00007fb0df836000:0x00007fb0df920c5f] pc=0x5717e '/Shell_x64.efi'
Here is some of the output for my laptop:
SMBIOS Entry Point Structure: Anchor String: _SM_ EPS Checksum: 0xEE Entry Point Len: 31 Version: 3.1 Number of Structures: 62 Max Struct size: 145 Table Address: 0x986E9000 Table Length: 2316 Entry Point revision: 0x0 SMBIOS BCD Revision: 0x31 Inter Anchor: _DMI_ Inter Checksum: 0x4E Formatted Area: 00000000: 00 00 00 00 00 *.....*
========================================================= Query Structure, conditions are: QueryType = Random QueryHandle = Random ShowType = SHOW_DETAIL
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $ ========================================================= Type=18, Handle=0x0 Dump Structure as: Index=0,Length=0x19,Addr=0x986E9019 00000000: 12 17 00 00 03 02 02 00-00 00 00 00 00 00 80 00 *................* 00000010: 00 00 80 00 00 00 80 00-00
*.........*
Enter to continue, :q to exit, :[0-3] to change mode, /? for help $Structure Type: 32-bit Memory Error Information Format part Len : 23 Structure Handle: 0 32-bit Memory Error Information - Type: OK Memory Error - Error granularity: Unknown Memory Error - Error Operation: Unknown VendorSyndrome: 0x0 MemoryArrayErrorAddress: 0x80000000 DeviceErrorAddress: 0x80000000 ErrorResolution: 0x80000000
OK, I am not sure what to make of that except that it is every bit as impenetrable as I would expect from UEFI.
This is a dump of the SMBIOS tables. Why do you refer to UEFI?
How do you fit so many questions in one email? :-)
I assumed smbiosview is a UEFI command because of the prompt. I have never heard of the command.
Is this on a Risc-V host? I am not sure why the first alloc would get the requested address and not the second. Is it because we specify 0x10000000 as the requested address? Is that what you actually get in this case?
The observation is reproducible on x86_64.
Two mmap calls cannot receive the same address without calling munmap in between.
The first call *might* return the address 0x10000000 that you ask for.
If the first call cannot receive it because it is not available, why should subsequent calls succeed?
The goal is to keep the addresses small since 64-bit addresses are a pain when debugging. I had hoped that mmap() would try to find an address near the one requested, for subsequent calls, but perhaps that hope is forlorn. It certainly doesn't seem to do that on x86.
If we drop that value on subsequent calls to mmap(), does it fix the problem?
If you drop that value you get a random address. What would be better in this case?
I just tried it on x86 and we get a 'random' address anyway, whatever is passed, once it has used the requested address. I think the address hint only works if it works. If not, then linux starts allocating from another address. Perhaps this linux behaviour has changed in recent years.
I think we should update the commit message to say we are hoping to get 0x10000000 for the RAM buffer (although of course there is a 4K region at the start of it) and that xx arch provides large addresses for subsequent mmap()s.
What 4 KiB region are you referring to?
If you look at os_malloc() you will see that it adds a 4KB header before each block it allocates.
Why do you expect any specific address in subsequent calls? The man-page states "If another mapping already exists there, the kernel picks a new address that may or may not depend on the hint."
For performance reasons the kernel should not care about the hint in this case.
OK.
Also, how about upgrading the smbios warning to an error. I think that is what it is and it should stop execution.
For errors we use log_err(). But where is the error? mmap() comes with absolutely no guarantee that it will assign the address that you are requesting or anything close to it.
Smbios is not important enough to make the sandbox unusable in this case.
Well if the tables are invalid we should not pass them on...can we at least do that?
That would be separate patch?
Yes indeed.
Regards, Simon
Applied to u-boot-dm/next, thanks!
participants (2)
-
Heinrich Schuchardt -
Simon Glass