Mike,

Thanks - I agree that HOBs are part of the PI specification although it's clear 
in the specs that the intent is passing data from PEI to DXE (case in point: 
the string 'HOB' appears in the DXE volume 200 times and in the SMM volume zero 
times other than the revision history table).  It's even more strange to see a 
library called DxeHobLib be used in the build description files for SMM phase 
components.  I don't have a problem with the use of HOBs in SMM it was just a 
surprise that it's required for SMM to work when using this library.  It sounds 
like some PI SMM spec verbiage about HOBs being exposed to SMM is required.

For the CPU physical address bits base library abstraction it would be great if 
I could work with someone on the Intel side on the abstraction since I don't 
have a good setup to verify it.  Would this be a good item for me to tee up on 
the issue tracker?

Thanks,

Eugene

> -----Original Message-----
> From: Kinney, Michael D [mailto:[email protected]]
> Sent: Wednesday, February 03, 2016 11:54 AM
> To: Cohen, Eugene <[email protected]>; [email protected]; Fan,
> Jeff <[email protected]>; Kinney, Michael D
> <[email protected]>
> Subject: RE: MdePkg/SmmMemLib: Insufficient abstraction for CPU
> physical address size
> 
> Eugene,
> 
> HOBs and SMM are both part of the PI Specifications.  From an
> implementation perspective, HOBs used in entry points of SMM core
> and SMM drivers can provide important platform/CPU state
> information.  So I think use of HOBs from SMM modules is allowed.
> 
> You do bring up some good points, and I do agree that it is possible to
> define library abstractions that can be used to increase compatibility of
> EDK II modules.
> 
> Mike
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:[email protected]] On
> Behalf Of Cohen, Eugene
> > Sent: Wednesday, February 3, 2016 8:23 AM
> > To: [email protected]; Kinney, Michael D
> <[email protected]>; Fan, Jeff
> > <[email protected]>
> > Subject: [edk2] MdePkg/SmmMemLib: Insufficient abstraction for
> CPU physical address
> > size
> >
> > In SmmMemLib there's a routine to determine the maximum
> physical address supported by a
> > processor:
> >
> >
> >   //
> >   // Get physical address bits supported.
> >   //
> >   Hob = GetFirstHob (EFI_HOB_TYPE_CPU);
> >   if (Hob != NULL) {
> >     PhysicalAddressBits = ((EFI_HOB_CPU *) Hob)-
> >SizeOfMemorySpace;
> >   } else {
> >     AsmCpuid (0x80000000, &RegEax, NULL, NULL, NULL);
> >     if (RegEax >= 0x80000008) {
> >       AsmCpuid (0x80000008, &RegEax, NULL, NULL, NULL);
> >       PhysicalAddressBits = (UINT8) RegEax;
> >     } else {
> >       PhysicalAddressBits = 36;
> >     }
> > >   }
> >
> >
> >
> > This is problematic in a couple ways:
> >
> >
> >
> > 1. AsmCpuid is not portable across architectures (understandably we
> would have said
> > that SMM is an IA thing originally)
> >
> > 2. The PI spec for SMM does not specify the use of HOBs (in fact I
> was very surprised
> > to even see HOBs in SMM, and even more confounded by platforms
> using the "DxeHobLib" in
> > the SMM core and drivers!)
> >
> >
> >
> > I think this would be a good job for a base library interface to handle -
> (e.g. UINTN
> > GetCpuPhysicalAddressBits().  You could then have a version that
> uses HOBs (MdePkg),
> > another IA specific one (UefiCpuPackage) that uses CPU ID, and one
> that works for ARM
> > (ArmPkg)  that reads ID registers.
> >
> >
> >
> > Does that sound reasonable?
> >
> >
> >
> > [Presumably this is just the beginning - we see AsmCpuId show up in
> a lot of modules
> > that need to be architecture-agnostic like BootScript (1G page
> support), CapsulePei (1G
> > page support), and AcpiS3SaveDxe (1G pages and physical address
> size).]
> >
> >
> >
> > Eugene
> > _______________________________________________
> > edk2-devel mailing list
> > [email protected]
> > https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel

Reply via email to