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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20140103171352.7db54a79@mantra.us.oracle.com>
Date:	Fri, 3 Jan 2014 17:13:52 -0800
From:	Mukesh Rathor <mukesh.rathor@...cle.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
Cc:	David Vrabel <david.vrabel@...rix.com>,
	linux-kernel@...r.kernel.org, xen-devel@...ts.xenproject.org,
	boris.ostrovsky@...cle.com, stefano.stabellini@...citrix.com
Subject: Re: [PATCH v12 03/18] xen/pvh: Early bootup changes in PV code
 (v2).

On Fri, 3 Jan 2014 12:35:55 -0500
Konrad Rzeszutek Wilk <konrad.wilk@...cle.com> wrote:

> 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@...cle.com> 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@...cle.com>
> > > > > 
> > > > > 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.

I am referring to using "cpuid" instruction instead of XEN_EMULATE_PREFIX.
cpuid is faster and long term better... there is no benefit using
XEN_EMULATE_PREFIX IMO. We can look at removing xen_cpuid() altogether for
PVH when/after pvh 32bit work gets done IMO.

The triple fault seems to be a new bug... I can create a bug, but for
now, with using cpuid instruction, that won't be an issue.

thanks
mukesh

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/