Re: [Xen-devel] [edk2] [PATCH RFC 06/14] OvmfPkg/XenPlatformPei: Add xen PVH specific code

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Jan 10, 2017 at 05:54:49PM +0100, Laszlo Ersek wrote:
> On 01/10/17 17:18, Anthony PERARD wrote:
> > On Thu, Jan 05, 2017 at 11:30:32AM +0100, Laszlo Ersek wrote:
> >> On 12/08/16 16:33, Anthony PERARD wrote:
> >>> diff --git a/OvmfPkg/XenPlatformPei/Platform.c 
> >>> b/OvmfPkg/XenPlatformPei/Platform.c
> >>> index bf78878..9fc713c 100644
> >>> --- a/OvmfPkg/XenPlatformPei/Platform.c
> >>> +++ b/OvmfPkg/XenPlatformPei/Platform.c
> >>> @@ -362,6 +362,10 @@ MiscInitialization (
> >>>        AcpiCtlReg = POWER_MGMT_REGISTER_Q35 (ICH9_ACPI_CNTL);
> >>>        AcpiEnBit  = ICH9_ACPI_CNTL_ACPI_EN;
> >>>        break;
> >>> +    case 0xffff:
> >>> +      // xen PVH, ignore
> >>> +      PcdStatus = PcdSet16S (PcdOvmfHostBridgePciDevId, 
> >>> mHostBridgeDevId);
> >>> +      return;
> >>
> >> Can we create a new macro for 0xFFFF in
> >> "OvmfPkg/Include/OvmfPlatforms.h", and use that here?
> >>
> >> Normally I would suggest a separate header file under "OvmfPkg/Include",
> >> similar to "Q35MchIch9.h" and "I440FxPiix4.h", and to include that new
> >> file in "OvmfPlatforms.h", but here we only need a single "chipset
> >> identifier", so I guess that can go directly into "OvmfPlatforms.h".
> >>
> >> I mainly suggest this because in patch 08/14, the same 0xFFFF case label
> >> is being added to code shared with QEMU, and using a verbose macro there
> >> is much better than a magic number. In turn, we should use the same
> >> macro here, I think.
> > 
> > This value is just the result of a read to an incorrect location, and
> > the return value is -1. There is no PCI when running in Xen PVH, at
> > least for now. I think I should try harder to find out if OVMF is
> > running in PVH, and so I would not need this case 0xffff at all.
> > 
> 
> Right, that would be best.
> 
> In fact, this affects two modules, (Xen)PlatformPei and
> PlatformBootManagerLib. In both, we already have the XenDetected()
> function. Maybe you can add a XenPvhDetected() function as well (which
> would return TRUE only for PVH, while the original XenDetected() would
> continue returning TRUE for both HVM and PVH). Then the PCI host bridge
> ID checking could be entirely short-circuited if XenPvhDetected()
> returned TRUE first.

It's very likely that when doing PCI-passthrough to a PVH guest an emulated
PCI host bridge is going to be available, so short-circuiting the bridge check
just based on PVH detection would then be wrong.

Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel