All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexander Graf <agraf@suse.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v11 3/6] sandbox: smbios: Update to support sandbox
Date: Mon, 22 Oct 2018 19:32:18 +0100	[thread overview]
Message-ID: <B62CB75A-E600-4094-A25C-4B12533E9412@suse.de> (raw)
In-Reply-To: <CAPnjgZ0_OECAkgXHHPQ_HTJZzoQC-rbA4oPBWBRJw7Q8-oYcrA@mail.gmail.com>



> Am 22.10.2018 um 18:49 schrieb Simon Glass <sjg@chromium.org>:
> 
> Hi Alex,
> 
>> On 19 October 2018 at 01:27, Alexander Graf <agraf@suse.de> wrote:
>> 
>> 
>>> On 19.10.18 05:25, Simon Glass wrote:
>>> Hi Alex,
>>> 
>>>> On 16 October 2018 at 06:55, Alexander Graf <agraf@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@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.

There is a command to print the contents of smbios tables - and that one does use them :).

> 
>> 
>>> Also sandbox addresses are always <4GB (they start@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 :-)

:)

Alex

> 
> Regards,
> Simon

  reply	other threads:[~2018-10-22 18:32 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-15 14:17 [U-Boot] [PATCH v11 0/6] efi_loader: Code refactoring and improvement Simon Glass
2018-10-15 14:17 ` [U-Boot] [PATCH v11 1/6] sandbox: Put CPUs under a cpu-bus node Simon Glass
2018-10-15 17:05   ` Heinrich Schuchardt
2018-10-15 14:17 ` [U-Boot] [PATCH v11 2/6] efi_loader: Drop setup_ok Simon Glass
2018-10-15 17:16   ` Heinrich Schuchardt
2018-10-19  3:25     ` Simon Glass
2018-10-19  5:53       ` Heinrich Schuchardt
2018-10-15 14:17 ` [U-Boot] [PATCH v11 3/6] sandbox: smbios: Update to support sandbox Simon Glass
2018-10-16 12:55   ` Alexander Graf
2018-10-19  3:25     ` Simon Glass
2018-10-19  7:27       ` Alexander Graf
2018-10-22 17:49         ` Simon Glass
2018-10-22 18:32           ` Alexander Graf [this message]
2018-11-04 18:39             ` Simon Glass
2018-10-15 14:17 ` [U-Boot] [PATCH v11 4/6] efi: Split out test init/uninit into functions Simon Glass
2018-10-15 17:34   ` Heinrich Schuchardt
2018-11-04 18:46     ` Simon Glass
2018-10-15 14:17 ` [U-Boot] [PATCH v11 5/6] efi: Create a function to set up for running EFI code Simon Glass
2018-10-15 14:17 ` [U-Boot] [PATCH v11 6/6] efi: Rename bootefi_test_finish() to bootefi_run_finish() Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=B62CB75A-E600-4094-A25C-4B12533E9412@suse.de \
    --to=agraf@suse.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.