Re: [Xen-devel] [edk2-devel] [PATCH v4 20/35] OvmfPkg/XenPlatformPei: Introduce XenPvhDetected

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

On Thu, Aug 08, 2019 at 11:38:13AM +0100, Anthony PERARD wrote:
> On Wed, Aug 07, 2019 at 05:03:46PM +0200, Roger Pau Monné wrote:
> > On Mon, Jul 29, 2019 at 04:39:29PM +0100, Anthony PERARD wrote:
> > > +BOOLEAN
> > > +XenPvhDetected (
> > > +  VOID
> > > +  )
> > > +{
> > > +  //
> > > +  // This function should only be used after XenConnect
> > > +  //
> > > +  ASSERT (mXenInfo.VersionMajor != 0);
> > 
> > That's IMO dangerous. Using the version as an indication that
> > XenConnect has run seems like a bad idea, since returning a major
> > version of 0 is a valid number to return. Can't you check against
> > something else that doesn't depends on hypervisor provided data? (ie:
> > like some allocations or such that happen in XenConnect)
> > 
> > A paranoid could provider could even return major == 0 and minor == 0
> > in order to attempt to hide the Xen version used, since guests are not
> > supposed to infer anything from the Xen version, available hypervisor
> > features are reported by other means.
> 
> I'm sure a paranoid provider wouldn't use a debug build of OVMF :-). So
> that assert doesn't matter. There's nothing dangerous in a `nop'! :-D
> 
> But I could use mXenInfo.HyperPages instead.

It's just a nit, and TBH it's quite unlikely for anyone to report a
major version of 0, it's just that if you have something else to
assert for initialization it might be safer.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel