[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/5] Add support for loading SMBIOS OEM strings from a file
From: |
Laszlo Ersek |
Subject: |
Re: [PATCH 0/5] Add support for loading SMBIOS OEM strings from a file |
Date: |
Wed, 9 Sep 2020 12:58:07 +0200 |
On 09/09/20 11:50, Daniel P. Berrangé wrote:
> On Wed, Sep 09, 2020 at 11:44:40AM +0200, Laszlo Ersek wrote:
>> On 09/08/20 18:54, Daniel P. Berrangé wrote:
>>> I previously added support for SMBIOS OEM strings tables but only
>>> allowed for data to be passed inline. Potential users indicated they
>>> wanted to pass some quite large data blobs which is inconvenient todo
>>> inline. Thus I'm adding support for passing the data from a file.
>>>
>>> In testing this I discovered the hard way that on x86 we're limited to
>>> using the SMBIOS 2.1 entry point currently. This has a maximum size of
>>> 0xffff, and if you exceed this all sorts of wierd behaviour happens.
>>>
>>> QEMU forces SMBIOS 2.1 on x86 because the default SeaBIOS firmware
>>> does not support SMBIOS 3.0. The EDK2 firmware supports SMBIOS 3.0 and
>>> QEMU defaults to this on the ARM virt machine type.
>>>
>>> This series adds support for checking the SMBIOS 2.1 limits to protect
>>> users from impossible to diagnose problems.
>>>
>>> There is also a fix needed to SeaBIOS which fails to check for
>>> integer overflow when it appends the type 0 table.
>>>
>>>
>>> https://mail.coreboot.org/hyperkitty/list/seabios@seabios.org/thread/3EMIOY6YS6MG5UQN3JJJS2A3DJZOVFR6/
>>>
>>> IIUC, SMBIOS 3.0 should onlky be limited by what you can fit into RAM,
>>> but in testing, EDK2 appears to hang shortly after the SMBIOS 3.0 data
>>> size exceeds 128 KB. I've not spotted an obvious flaw in EDK2 or QEMU,
>>> nor do I attempt to enforce a limit in QEMU for SMBIOS 3.0.
>
> snip
>
>> So we're exceeding "__brk_limit".
>>
>> ... I'm quite getting out of my league here, but "__brk_limit" seems to
>> be controlled by "brk_reservation" in "arch/x86/kernel/vmlinux.lds.S"...
>> and ultimately through the RESERVE_BRK() macro:
>>
>> [arch/x86/include/asm/setup.h]
>>
>>> /*
>>> * Reserve space in the brk section. The name must be unique within
>>> * the file, and somewhat descriptive. The size is in bytes. Must be
>>> * used at file scope.
>>> *
>>> * (This uses a temp function to wrap the asm so we can pass it the
>>> * size parameter; otherwise we wouldn't be able to. We can't use a
>>> * "section" attribute on a normal variable because it always ends up
>>> * being @progbits, which ends up allocating space in the vmlinux
>>> * executable.)
>>> */
>>> #define RESERVE_BRK(name,sz) \
>>
>> OK, so let's see RESERVE_BRK() invocations... The relevant match is
>> likely the one below:
>>
>>> arch/x86/kernel/setup.c:RESERVE_BRK(dmi_alloc, 65536);
>>
>> ... Then see kernel commits:
>>
>> - 6de6cb442e76 ("x86: use brk allocation for DMI", 2009-03-14)
>>
>> - 796216a57fe4 ("x86: allow extend_brk users to reserve brk space",
>> 2009-03-14)
>>
>> - e808bae2407a ("x86: Do not reserve brk for DMI if it's not going to be
>> used", 2010-02-25)
>>
>> Commit 796216a57fe4 is helpful:
>>
>>> Add RESERVE_BRK(name, size) macro to reserve space in the brk
>>> area. This should be a conservative (ie, larger) estimate of
>>> how much space might possibly be required from the brk area.
>>> Any unused space will be freed, so there's no real downside
>>> on making the reservation too large (within limits).
>>
>> So it seems like the 64K limit could be increased, but still
>> - it requires guest kernels to be rebuilt,
>> - it doesn't seem suitable for passing MBs of data (on x86 anyway).
>
> Yeah, this feels like we're just venturing into a bad part of town.
> Simplest is probably to just document that applications should never
> expect more than 64kb of total SMBIOS data to be viable regardless
> of the SMBIOS entry point.
Sounds OK to me personally.
In your experience, would that limit satisfy (for example) the CoreOS /
Ignition use case?
> Given this, I'm thinking it might be overkill to even both with
> supporting SMBIOS 3.0 for x86, unless it offers some other compelling
> benefit over SMBIOS 2.1 that you know of ?
I think the 32-bit entry point is sufficient for x86.
If memory serves, we only started to care about the 64-bit entry point
for aarch64. See for example
https://github.com/tianocore/edk2/commit/ca6d61b22658
x86 always has RAM under 4GB though.
Thanks
Laszlo
Thanks
Laszlo
- Re: [PATCH 4/5] hw/smbios: use qapi for SMBIOS entry point type enum, (continued)