On Thu, May 15, 2014 at 4:30 AM, Wei Liu <wei.l...@citrix.com> wrote: > On Wed, May 14, 2014 at 12:34:14PM -0400, Gabriel L. Somlo wrote: >> On Wed, May 14, 2014 at 02:55:15PM +0100, Wei Liu wrote: >> > On Wed, May 14, 2014 at 03:31:48PM +0200, Laszlo Ersek wrote: >> > > On 05/14/14 00:13, Gabriel L. Somlo wrote: >> > > > The SMBIOS specification requires some structure types to >> > > > contain reference fields to other structures' handles. When >> > > > InstallAllStructures() rebuilds the SMBIOS tables by traversing >> > > > an existing source table, the use of SMBIOS_HANDLE_PI_RESERVED >> > > > causes automatically generated, arbitrary handle numbers to be >> > > > assigned to each cloned structure. This causes all reference >> > > > handle fields to become invalid. >> > > > >> > > > This patch modifies InstallAllStructures() to reuse the original >> > > > handle numbers supplied by the underlying VM, preserving the >> > > > correctness of any included handle references. >> > > > >> > > > Contributed-under: TianoCore Contribution Agreement 1.0 >> > > > Signed-off-by: Gabriel Somlo <so...@cmu.edu> >> > > > --- >> > > > OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c | 2 +- >> > > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > > >> > > > diff --git a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c >> > > > b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c >> > > > index 42a5132..ac48fb7 100644 >> > > > --- a/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c >> > > > +++ b/OvmfPkg/SmbiosPlatformDxe/SmbiosPlatformDxe.c >> > > > @@ -106,7 +106,7 @@ InstallAllStructures ( >> > > > // >> > > > // Log the SMBIOS data for this structure >> > > > // >> > > > - SmbiosHandle = SMBIOS_HANDLE_PI_RESERVED; >> > > > + SmbiosHandle = SmbiosTable.Hdr->Handle; >> > > > Status = Smbios->Add ( >> > > > Smbios, >> > > > NULL, >> > > > >> > > >> > > This patch should be tested by Xen people (I'm CC'ing Wei Liu). In >> > > general it seems reasonable to me; it matches the smbios spec and the >> > > qemu-side code. >> > > >> > > Reviewed-by: Laszlo Ersek <ler...@redhat.com> >> > >> > Thanks for the heads up. Anthony (CC'ed) is working on OVMF now and he's >> > also a maintainer for QEMU in Xen, so he might have a better idea than I >> > do. He gracefully offers help on testing this change. He shall let you >> > know if there's any problem. >> >> Here's a quick example of what I'm talking about: >> >> Handle 0x1000, DMI type 16, 23 bytes >> Physical Memory Array >> Location: Other >> Use: System Memory >> Error Correction Type: Multi-bit ECC >> Maximum Capacity: 2 GB >> Error Information Handle: Not Provided >> Number Of Devices: 1 >> ... >> >> Handle 0x1300, DMI type 19, 31 bytes >> Memory Array Mapped Address >> Starting Address: 0x00000000000 >> Ending Address: 0x0007FFFFFFF >> Range Size: 2 GB >> Physical Array Handle: 0x1000 >> Partition Width: 1 >> >> The second struct (type 19) has a "Physical Array Handle set to >> 0x1000, which points it at the first one (type 16). If we renumber >> the structs and change the type16 handle without also editing the >> contents of the Phys. Array Handle in type 19, we end up with a >> "dangling reference"... Better to say it's the responsibility of >> the underlying VM to cross-link the structures correctly, and just >> use the original handle numbers as given. >> > > Ah, I see. Then it's definitely something that needs fixing.
It does seem like a good change. I expect Xen entries potentially reference each other in the same way. So, Wei, Anthony, do you agree that this change should be okay for OVMF on Xen? -Jordan ------------------------------------------------------------------------------ "Accelerate Dev Cycles with Automated Cross-Browser Testing - For FREE Instantly run your Selenium tests across 300+ browser/OS combos. Get unparalleled scalability from the best Selenium testing platform available Simple to use. Nothing to install. Get started now for free." http://p.sf.net/sfu/SauceLabs _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
