On Wed, Jun 01, 2016 at 11:12:17AM +0200, Laszlo Ersek wrote:
> On 06/01/16 09:51, Gary Lin wrote:
> > On Tue, May 31, 2016 at 12:21:45AM -0700, Jordan Justen wrote:
> >> On 2016-05-30 20:19:42, Gary Lin wrote:
> >>> When OVMF tried to load the file-based NvVars,
> >>
> >> Tangent: Will Xen ever add a r/w flash emulation for variables like
> >> QEMU/KVM?
> >>
> >>> it checked all the PCI
> >>> instances and connected the drivers to the mass storage device. However,
> >>> Xen registered its PCI device with a special class id (0xFF80), so
> >>> ConnectRecursivelyIfPciMassStorage() couldn't recognize it and skipped the
> >>> driver connecting for Xen PCI devices. In the end, the Xen block device
> >>> wasn't initialized until EfiBootManagerConnectAll() was called, and it's
> >>> already too late to load NvVars.
> >>>
> >>> This commit connects the Xen drivers in
> >>> ConnectRecursivelyIfPciMassStorage()
> >>> so that Xen can use the file-based NvVars.
> >>>
> >>> Cc: Jordan Justen <jordan.l.jus...@intel.com>
> >>> Cc: Laszlo Ersek <ler...@redhat.com>
> >>> Contributed-under: TianoCore Contribution Agreement 1.0
> >>> Signed-off-by: Gary Lin <g...@suse.com>
> >>> ---
> >>> OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 12
> >>> ++++++++++--
> >>> .../PlatformBootManagerLib/PlatformBootManagerLib.inf | 1 +
> >>> 2 files changed, 11 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> >>> b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> >>> index befcc57..249b8bc 100644
> >>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> >>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> >>> @@ -1049,8 +1049,15 @@ ConnectRecursivelyIfPciMassStorage (
> >>> EFI_STATUS Status;
> >>> EFI_DEVICE_PATH_PROTOCOL *DevicePath;
> >>> CHAR16 *DevPathStr;
> >>> + BOOLEAN IsPciMassStorage;
> >>>
> >>
> >> I don't think the debug message change is worth adding this variable.
> >>
> >>> - if (IS_CLASS1 (PciHeader, PCI_CLASS_MASS_STORAGE)) {
> >>> + IsPciMassStorage = IS_CLASS1 (PciHeader, PCI_CLASS_MASS_STORAGE);
> >>> + //
> >>> + // Recognize PCI Mass Storage, and Xen PCI devices
> >>> + //
> >>> + if (IsPciMassStorage ||
> >>> + (PcdGetBool (PcdPciDisableBusEnumeration) &&
> >>
> >> How about instead get XenDetected from OvmfPkg/AcpiPlatformDxe/Xen.c?
> >> Then use that rather than PcdPciDisableBusEnumeration?
> >>
> > I didn't notice that XenDetected is in AcpiPlatformDxe until I started
> > to implement the code. Is there any suggested library to move XenDetected
> > to? A standalone header/library for the function seems too much but I
> > didn't feel it belongs to any existed libraries in OvmfPkg.
>
> I think Jordan meant to copy the XenDetected() function verbatim, from
> AcpiPlatformDxe to PlatformBootManagerLib. This function is really small
> so it shouldn't be a problem.
>
> Also, since the XenDetected() function looks for a GUID HOB (IIRC),
> which performs a linear search in the HOB list, Jordan suggested the
> speedup below (with the static variable caching the result of the
> search). It is relevant because ConnectRecursivelyIfPciMassStorage() is
> called several times.
>
> That's how I understood Jordan anyway.
>
Okay, that makes sense. I just try to avoid duplicating code but the
cost seems too high in this case. Will send v3 later.
Thanks,
Gary Lin
> Thanks,
> Laszlo
>
> > Thanks,
> >
> > Gary Lin
> >
> >> I think XenDetected could be improved for multiple uses by adding a
> >> static INTN:
> >>
> >> * Initialize to -1, for need to locate HOB
> >>
> >> * 0 for Xen is not detected
> >>
> >> * 1 for Xen is detected.
> >>
> >> -Jordan
> >>
> >>> + IS_CLASS2 (PciHeader, 0xFF, 0x80))) {
> >>> DevicePath = NULL;
> >>> Status = gBS->HandleProtocol (
> >>> Handle,
> >>> @@ -1068,7 +1075,8 @@ ConnectRecursivelyIfPciMassStorage (
> >>> if (DevPathStr != NULL) {
> >>> DEBUG((
> >>> EFI_D_INFO,
> >>> - "Found Mass Storage device: %s\n",
> >>> + "Found %s device: %s\n",
> >>> + IsPciMassStorage ? L"Mass Storage" : L"Xen",
> >>> DevPathStr
> >>> ));
> >>> FreePool(DevPathStr);
> >>> diff --git
> >>> a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> >>> b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> >>> index 5fcee3c..cd28be0 100644
> >>> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> >>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> >>> @@ -62,6 +62,7 @@ [Pcd]
> >>> gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId
> >>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut
> >>> gEfiIntelFrameworkModulePkgTokenSpaceGuid.PcdShellFile
> >>> + gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration
> >>>
> >>> [Pcd.IA32, Pcd.X64]
> >>> gEfiMdePkgTokenSpaceGuid.PcdFSBClock
> >>> --
> >>> 2.8.3
> >>>
> >>
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
