From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 22 Oct 2018 11:49:49 -0600 Subject: [U-Boot] [PATCH v11 3/6] sandbox: smbios: Update to support sandbox In-Reply-To: <88699824-4387-02d7-3119-a29006c6c02e@suse.de> References: <20181015141750.56480-1-sjg@chromium.org> <20181015141750.56480-4-sjg@chromium.org> <88699824-4387-02d7-3119-a29006c6c02e@suse.de> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Alex, On 19 October 2018 at 01:27, Alexander Graf wrote: > > > On 19.10.18 05:25, Simon Glass wrote: >> Hi Alex, >> >> On 16 October 2018 at 06:55, Alexander Graf 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 >>> >>> 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. > >> 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 :-) Regards, Simon