From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: Is: PVH dom0 - MWAIT detection logic to get deeper C-states exposed in ACPI AML code. Was:Re: [PATCH v2 10/30] xen/x86: Annotate VM applicability in featureset Date: Thu, 18 Feb 2016 16:48:35 +0000 Message-ID: <56C5F5E3.3020108@citrix.com> References: <1454679743-18133-1-git-send-email-andrew.cooper3@citrix.com> <1454679743-18133-11-git-send-email-andrew.cooper3@citrix.com> <56BE1EEB02000078000D1896@prv-mh.provo.novell.com> <56BE198A.2000408@citrix.com> <56C1A66302000078000D1F8C@prv-mh.provo.novell.com> <56C1E2FA.2020701@citrix.com> <56C1F3B002000078000D22D6@prv-mh.provo.novell.com> <56C1E679.7060405@citrix.com> <56C1F68902000078000D2319@prv-mh.provo.novell.com> <56C1F1B5.10409@citrix.com> <20160217190208.GF24109@char.us.oracle.com> <56C5DD19.8070209@citrix.com> <56C5DF5F.10200@citrix.com> <56C5F04A.2020809@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <56C5F04A.2020809@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Boris Ostrovsky , =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= , Konrad Rzeszutek Wilk Cc: Jan Beulich , Xen-devel List-Id: xen-devel@lists.xenproject.org On 18/02/16 16:24, Boris Ostrovsky wrote: > > > On 02/18/2016 10:12 AM, Andrew Cooper wrote: >> On 18/02/16 15:02, Roger Pau Monn=E9 wrote: >>> El 17/2/16 a les 20:02, Konrad Rzeszutek Wilk ha escrit: >>>> On Mon, Feb 15, 2016 at 03:41:41PM +0000, Andrew Cooper wrote: >>>>> On 15/02/16 15:02, Jan Beulich wrote: >>>>>>>>> On 15.02.16 at 15:53, wrote: >>>>>>> On 15/02/16 14:50, Jan Beulich wrote: >>>>>>>>>>> On 15.02.16 at 15:38, wrote: >>>>>>>>> On 15/02/16 09:20, Jan Beulich wrote: >>>>>>>>>>>>> On 12.02.16 at 18:42, wrote: >>>>>>>>>>> On 12/02/16 17:05, Jan Beulich wrote: >>>>>>>>>>>>>>> On 05.02.16 at 14:42, wrote: >>>>>>>>>>>>> #define X86_FEATURE_MWAITX ( 3*32+29) /* MWAIT >>>>>>>>>>>>> extension >>>>>>>>>>> (MONITORX/MWAITX) */ >>>>>>>>>>>> Why not exposed to HVM (also for _MWAIT as I now notice)? >>>>>>>>>>> Because that is a good chunk of extra work to support. We >>>>>>>>>>> would need to >>>>>>>>>>> use 4K monitor widths, and extra p2m handling. >>>>>>>>>> I don't understand: The base (_MWAIT) feature being exposed to >>>>>>>>>> guests today, and kernels making use of the feature when >>>>>>>>>> available >>>>>>>>>> suggests to me that things work. Are you saying you know >>>>>>>>>> otherwise? (And if there really is a reason to mask the >>>>>>>>>> feature all of >>>>>>>>>> the sudden, this should again be justified in the commit >>>>>>>>>> message.) >>>>>>>>> PV guests had it clobbered by Xen in traps.c >>>>>>>>> >>>>>>>>> HVM guests have: >>>>>>>>> >>>>>>>>> vmx.c: >>>>>>>>> case EXIT_REASON_MWAIT_INSTRUCTION: >>>>>>>>> case EXIT_REASON_MONITOR_INSTRUCTION: >>>>>>>>> [...] >>>>>>>>> hvm_inject_hw_exception(TRAP_invalid_op, >>>>>>>>> HVM_DELIVER_NO_ERROR_CODE); >>>>>>>>> break; >>>>>>>>> >>>>>>>>> and svm.c: >>>>>>>>> case VMEXIT_MONITOR: >>>>>>>>> case VMEXIT_MWAIT: >>>>>>>>> hvm_inject_hw_exception(TRAP_invalid_op, >>>>>>>>> HVM_DELIVER_NO_ERROR_CODE); >>>>>>>>> break; >>>>>>>>> >>>>>>>>> I don't see how a guest could actually use this feature. >>>>>>>> Do you see the respective intercepts getting enabled anywhere? >>>>>>>> (I don't outside of nested code, which I didn't check in detail.) >>>>>>> Yes - the intercepts are always enabled to prevent the guest >>>>>>> actually >>>>>>> putting the processor to sleep. >>>>>> Hmm, you're right, somehow I've managed to ignore the relevant >>>>>> lines grep reported. Yet - how do things work then, without the >>>>>> MWAIT feature flag currently getting cleared? >>>>> I have never observed it being used. Do you have some local >>>>> patches in >>>>> the SLES hypervisor? >>>>> >>>>> There is some gross layer violation in xen/enlighten.c to pretend >>>>> that >>>>> MWAIT is present to trick the ACPI code into evaluating _CST() >>>>> methods >>>>> to report back to Xen. (This is yet another PV-ism which will >>>>> cause a >>>>> headache for a DMLite dom0) >>>> Yes indeed. CC-ing Roger, and Boris. >>> Yes, all this is indeed not very nice, and we would ideally like to get >>> rid of it on PVHv2. > > We will have to come up with something else: AIUI the whole point of > xen_check_mwait() is to come up with proper ECX and EDX values for the > MWAIT CPUID leaf. Those value are expected to be reported from > xen_cpuid() pv_op so that acpi_processor_ffh_state_probe_cpu() can set > C states structures properly. > > The problem is that PVH executes CPUID instruction natively. (And so > this must have been broken on PVHv1 as well). Currently, mwait is unusable by any domains, and will not be offered in any cpuid policy. How a particular dom0 goes about deciding to enumerate the ACPI objects is its own business, but personally I think it is a layering violation to have the enumeration of an existing ACPI object based on a feature bit. Dom0, being suitably enlightened, should know that its job is to service Xen when it comes to ACPI, and unilaterally collect and upload everything it can. ~Andrew