Re: [Xen-devel] [PATCH v12 03/18] xen/pvh: Early bootup changes in PV code (v2).

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Thu, Jan 02, 2014 at 05:34:38PM -0800, Mukesh Rathor wrote:
> On Thu, 2 Jan 2014 13:32:21 -0500
> Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx> wrote:
> 
> > On Thu, Jan 02, 2014 at 03:32:33PM +0000, David Vrabel wrote:
> > > On 01/01/14 04:35, Konrad Rzeszutek Wilk wrote:
> > > > From: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> > > > 
> > > > In the bootup code for PVH we can trap cpuid via vmexit, so don't
> > > > need to use emulated prefix call. We also check for vector
> > > > callback early on, as it is a required feature. PVH also runs at
> > > > default kernel IOPL.
> > > > 
> > > > Finally, pure PV settings are moved to a separate function that
> > > > are only called for pure PV, ie, pv with pvmmu. They are also
> > > > #ifdef with CONFIG_XEN_PVMMU.
> > > [...]
> > > > @@ -331,12 +333,15 @@ static void xen_cpuid(unsigned int *ax,
> > > > unsigned int *bx, break;
> > > >         }
> > > >  
> > > > -       asm(XEN_EMULATE_PREFIX "cpuid"
> > > > -               : "=a" (*ax),
> > > > -                 "=b" (*bx),
> > > > -                 "=c" (*cx),
> > > > -                 "=d" (*dx)
> > > > -               : "0" (*ax), "2" (*cx));
> > > > +       if (xen_pvh_domain())
> > > > +               native_cpuid(ax, bx, cx, dx);
> > > > +       else
> > > > +               asm(XEN_EMULATE_PREFIX "cpuid"
> > > > +                       : "=a" (*ax),
> > > > +                       "=b" (*bx),
> > > > +                       "=c" (*cx),
> > > > +                       "=d" (*dx)
> > > > +                       : "0" (*ax), "2" (*cx));
> > > 
> > > For this one off cpuid call it seems preferrable to me to use the
> > > emulate prefix rather than diverge from PV.
> > 
> > This was before the PV cpuid was deemed OK to be used on PVH.
> > Will rip this out to use the same version.
> 
> Whats wrong with using native cpuid? That is one of the benefits that
> cpuid can be trapped via vmexit, and also there is talk of making PV
> cpuid trap obsolete in the future. I suggest leaving it native.

I chatted with David, Andrew and Roger on IRC about this. I like the
idea of using xen_cpuid because:
 1) It filters some of the CPUID flags that guests should not use. There is
    the 'aperfmperf,'x2apic', 'xsave', and whether the MWAIT_LEAF
    should be exposed (so that the ACPI AML code can call the right
    initialization code to use the extended C3 states instead of the
    legacy IOPORT ones). All of that is in xen_cpuid.
   
 2) It works, while we can concentrate on making 1) work in the
    hypervisor/toolstack.

Meaning that the future way would be to use the native cpuid and have
the hypervisor/toolstack setup the proper cpuid. In other words - use
the xen_cpuid as is until that code for filtering is in the hypervisor.


Except that PVH does not work the PV cpuid at all. I get a triple fault.
The instruction it fails at is at the 'XEN_EMULATE_PREFIX'.

Mukesh, can you point me to the patch where the PV cpuid functionality
is enabled?

Anyhow, as it stands, I will just use the native cpuid.

> 
> Mukesh
> 

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