Re: [Xen-devel] [PATCH v3 7/8] x86emul: support RDPRU

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 03/09/2019 10:41, Jan Beulich wrote:
> While the PM doesn't say so, this assumes that the MPERF value read this
> way gets scaled similarly to its reading through RDMSR.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

This wants the following hunks merged, to at least keep the
intercept/exit codes up to date.  This is from my alternative series
which intercepted and terminated RDPRU with #UD.

diff --git a/xen/include/asm-x86/hvm/svm/vmcb.h
b/xen/include/asm-x86/hvm/svm/vmcb.h
index 5c710286f7..2bf0d50f6d 100644
--- a/xen/include/asm-x86/hvm/svm/vmcb.h
+++ b/xen/include/asm-x86/hvm/svm/vmcb.h
@@ -76,7 +76,8 @@ enum GenericIntercept2bits
     GENERAL2_INTERCEPT_MONITOR = 1 << 10,
     GENERAL2_INTERCEPT_MWAIT   = 1 << 11,
     GENERAL2_INTERCEPT_MWAIT_CONDITIONAL = 1 << 12,
-    GENERAL2_INTERCEPT_XSETBV  = 1 << 13
+    GENERAL2_INTERCEPT_XSETBV  = 1 << 13,
+    GENERAL2_INTERCEPT_RDPRU   = 1 << 14,
 };


@@ -300,6 +301,7 @@ enum VMEXIT_EXITCODE
     VMEXIT_MWAIT            = 139, /* 0x8b */
     VMEXIT_MWAIT_CONDITIONAL= 140, /* 0x8c */
     VMEXIT_XSETBV           = 141, /* 0x8d */
+    VMEXIT_RDPRU            = 142, /* 0x8e */
     VMEXIT_NPF              = 1024, /* 0x400, nested paging fault */
     VMEXIT_INVALID          =  -1
 };


> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -545,6 +545,11 @@ void recalculate_cpuid_policy(struct dom
>  
>      p->extd.maxlinaddr = p->extd.lm ? 48 : 32;
>  
> +    if ( p->extd.rdpru )
> +        p->extd.rdpru_max = min(p->extd.rdpru_max, max->extd.rdpru_max);
> +    else
> +        p->extd.rdpru_max = 0;
> +
>      recalculate_xstate(p);
>      recalculate_misc(p);

The CPUID logic needs quite a bit more than this, and to be safe on
migrate.  For one, recalculate_xstate() unilaterally clobbers this to 0.

Shall I do a prep patch getting the CPUID side of things in order?

>  
> --- a/xen/arch/x86/x86_emulate/x86_emulate.c
> +++ b/xen/arch/x86/x86_emulate/x86_emulate.c
> @@ -5670,6 +5671,52 @@ x86_emulate(
>                  limit -= sizeof(zero);
>              }
>              break;
> +
> +        case 0xfd: /* rdpru */
> +            vcpu_must_have(rdpru);
> +
> +            if ( !mode_ring0() )
> +            {
> +                fail_if(!ops->read_cr);
> +                if ( (rc = ops->read_cr(4, &cr4, ctxt)) != X86EMUL_OKAY )
> +                    goto done;
> +                generate_exception_if(cr4 & X86_CR4_TSD, EXC_UD);
> +            }
> +
> +            switch ( _regs.ecx )
> +            {
> +            case 0:  n = MSR_IA32_MPERF; break;
> +            case 1:  n = MSR_IA32_APERF; break;
> +            default: n = 0; break;
> +            }
> +            if ( _regs.ecx > ctxt->cpuid->extd.rdpru_max )
> +                n = 0;

This can be folded into the switch statement.  Something like (
_regs.ecx | -(_regs.ecx > ctxt->cpuid->extd.rdpru_max) )

Also, the sentinel might better be -1, which is not in a defined MSR
block.  MSR 0 is a P5-compat MCE MSR, even on AMD hardware.

~Andrew

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