[Xen-devel] [PATCH 0/2] x86: Protected Processor Inventory Number (PPIN) support

[Xen-devel] [PATCH 0/2] x86: Protected Processor Inventory Number (PPIN) support

[Jan Beulich]

1: include the PPIN in MCE records when available
2: explicitly disallow guest access to PPIN

I will also see to get around to post the Linux side consumer
patch of the interface addition in patch 1.

Jan

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

[Xen-devel] [PATCH 1/2] x86: include the PPIN in MCE records when available

[Jan Beulich]

Quoting the respective Linux commit:

    Intel Xeons from Ivy Bridge onwards support a processor identification
    number set in the factory. To the user this is a handy unique number to
    identify a particular CPU. Intel can decode this to the fab/production
    run to track errors. On systems that have it, include it in the machine
    check record. I'm told that this would be helpful for users that run
    large data centers with multi-socket servers to keep track of which CPUs
    are seeing errors.

Newer AMD CPUs support this too, at different MSR numbers.

Take the opportunity and hide __MC_NMSRS from the public interface going
forward.

[Linux commit 3f5a7896a5096fd50030a04d4c3f28a7441e30a5]
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -35,6 +35,7 @@ bool __read_mostly mce_broadcast;
 bool is_mc_panic;
 DEFINE_PER_CPU_READ_MOSTLY(unsigned int, nr_mce_banks);
 unsigned int __read_mostly firstbank;
+unsigned int __read_mostly ppin_msr;
 uint8_t __read_mostly cmci_apic_vector;
 
 DEFINE_PER_CPU_READ_MOSTLY(struct mca_banks *, poll_bankmask);
@@ -999,10 +1000,17 @@ static void do_mc_get_cpu_info(void *v)
     /*
      * This part needs to run on the CPU itself.
      */
-    xcp->mc_nmsrvals = __MC_NMSRS;
+    xcp->mc_nmsrvals = 1;
     xcp->mc_msrvalues[0].reg = MSR_IA32_MCG_CAP;
     rdmsrl(MSR_IA32_MCG_CAP, xcp->mc_msrvalues[0].value);
 
+    if ( ppin_msr && xcp->mc_nmsrvals < ARRAY_SIZE(xcp->mc_msrvalues) )
+    {
+        xcp->mc_msrvalues[xcp->mc_nmsrvals].reg = ppin_msr;
+        rdmsrl(ppin_msr, xcp->mc_msrvalues[xcp->mc_nmsrvals].value);
+        ++xcp->mc_nmsrvals;
+    }
+
     if ( c->cpuid_level >= 1 )
     {
         cpuid(1, &junk, &ebx, &junk, &junk);
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -49,6 +49,7 @@ enum mcheck_type intel_mcheck_init(struc
 void amd_nonfatal_mcheck_init(struct cpuinfo_x86 *c);
 
 extern unsigned int firstbank;
+extern unsigned int ppin_msr;
 
 struct mcinfo_extended *intel_get_extended_msrs(
     struct mcinfo_global *mig, struct mc_info *mi);
--- a/xen/arch/x86/cpu/mcheck/mce_amd.c
+++ b/xen/arch/x86/cpu/mcheck/mce_amd.c
@@ -315,6 +315,26 @@ amd_mcheck_init(struct cpuinfo_x86 *ci)
     if ( quirkflag == MCEQUIRK_F10_GART )
         mcequirk_amd_apply(quirkflag);
 
+    if ( cpu_has(ci, X86_FEATURE_AMD_PPIN) &&
+         (ci == &boot_cpu_data || ppin_msr) )
+    {
+        uint64_t val;
+
+        rdmsrl(MSR_AMD_PPIN_CTL, val);
+
+        /* If PPIN is disabled, but not locked, try to enable. */
+        if ( !(val & (PPIN_ENABLE | PPIN_LOCKOUT)) )
+        {
+            wrmsr_safe(MSR_PPIN_CTL, val | PPIN_ENABLE);
+            rdmsrl(MSR_AMD_PPIN_CTL, val);
+        }
+
+        if ( (val & (PPIN_ENABLE | PPIN_LOCKOUT)) != PPIN_ENABLE )
+            ppin_msr = 0;
+        else if ( ci == &boot_cpu_data )
+            ppin_msr = MSR_AMD_PPIN;
+    }
+
     x86_mce_callback_register(amd_f10_handler);
     mce_recoverable_register(mc_amd_recoverable_scan);
     mce_register_addrcheck(mc_amd_addrcheck);
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
@@ -853,6 +853,43 @@ static void intel_init_mce(void)
     mce_uhandler_num = ARRAY_SIZE(intel_mce_uhandlers);
 }
 
+static void intel_init_ppin(const struct cpuinfo_x86 *c)
+{
+    /*
+     * Even if testing the presence of the MSR would be enough, we don't
+     * want to risk the situation where other models reuse this MSR for
+     * other purposes.
+     */
+    switch ( c->x86_model )
+    {
+        uint64_t val;
+
+    case 0x3e: /* IvyBridge X */
+    case 0x3f: /* Haswell X */
+    case 0x4f: /* Broadwell X */
+    case 0x55: /* Skylake X */
+    case 0x56: /* Broadwell Xeon D */
+    case 0x57: /* Knights Landing */
+    case 0x85: /* Knights Mill */
+
+        if ( (c != &boot_cpu_data && !ppin_msr) ||
+             rdmsr_safe(MSR_PPIN_CTL, val) )
+            return;
+
+        /* If PPIN is disabled, but not locked, try to enable. */
+        if ( !(val & (PPIN_ENABLE | PPIN_LOCKOUT)) )
+        {
+            wrmsr_safe(MSR_PPIN_CTL, val | PPIN_ENABLE);
+            rdmsr_safe(MSR_PPIN_CTL, val);
+        }
+
+        if ( (val & (PPIN_ENABLE | PPIN_LOCKOUT)) != PPIN_ENABLE )
+            ppin_msr = 0;
+        else if ( c == &boot_cpu_data )
+            ppin_msr = MSR_PPIN;
+    }
+}
+
 static void cpu_mcabank_free(unsigned int cpu)
 {
     struct mca_banks *cmci = per_cpu(no_cmci_banks, cpu);
@@ -941,6 +978,8 @@ enum mcheck_type intel_mcheck_init(struc
 
     intel_init_thermal(c);
 
+    intel_init_ppin(c);
+
     return mcheck_intel;
 }
 
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -45,6 +45,13 @@
 #define MSR_PRED_CMD			0x00000049
 #define PRED_CMD_IBPB			(_AC(1, ULL) << 0)
 
+/* Intel Protected Processor Inventory Number */
+#define MSR_PPIN_CTL			0x0000004e
+#define MSR_PPIN			0x0000004f
+
+#define PPIN_LOCKOUT			(_AC(1, ULL) << 0)
+#define PPIN_ENABLE			(_AC(1, ULL) << 1)
+
 #define MSR_ARCH_CAPABILITIES		0x0000010a
 #define ARCH_CAPS_RDCL_NO		(_AC(1, ULL) << 0)
 #define ARCH_CAPS_IBRS_ALL		(_AC(1, ULL) << 1)
@@ -278,6 +285,10 @@
 #define MSR_AMD_OSVW_ID_LENGTH          0xc0010140
 #define MSR_AMD_OSVW_STATUS             0xc0010141
 
+/* AMD Protected Processor Inventory Number */
+#define MSR_AMD_PPIN_CTL                0xc00102f0
+#define MSR_AMD_PPIN                    0xc00102f1
+
 /* K6 MSRs */
 #define MSR_K6_EFER			0xc0000080
 #define MSR_K6_STAR			0xc0000081
--- a/xen/include/public/arch-x86/cpufeatureset.h
+++ b/xen/include/public/arch-x86/cpufeatureset.h
@@ -247,6 +247,7 @@ XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2
 XEN_CPUFEATURE(RSTR_FP_ERR_PTRS, 8*32+ 2) /*A  (F)X{SAVE,RSTOR} always saves/restores FPU Error pointers */
 XEN_CPUFEATURE(WBNOINVD,      8*32+ 9) /*   WBNOINVD instruction */
 XEN_CPUFEATURE(IBPB,          8*32+12) /*A  IBPB support only (no IBRS, used by AMD) */
+XEN_CPUFEATURE(AMD_PPIN,      8*32+23) /*   Protected Processor Inventory Number */
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0.edx, word 9 */
 XEN_CPUFEATURE(AVX512_4VNNIW, 9*32+ 2) /*A  AVX512 Neural Network Instructions */
--- a/xen/include/public/arch-x86/xen-mca.h
+++ b/xen/include/public/arch-x86/xen-mca.h
@@ -246,7 +246,9 @@ typedef struct mc_info mc_info_t;
 DEFINE_XEN_GUEST_HANDLE(mc_info_t);
 
 #define __MC_MSR_ARRAYSIZE 8
+#if __XEN_INTERFACE_VERSION__ <= 0x00040d00
 #define __MC_NMSRS 1
+#endif
 #define MC_NCAPS	7	/* 7 CPU feature flag words */
 #define MC_CAPS_STD_EDX	0	/* cpuid level 0x00000001 (%edx) */
 #define MC_CAPS_AMD_EDX	1	/* cpuid level 0x80000001 (%edx) */
--- a/xen/include/public/xen-compat.h
+++ b/xen/include/public/xen-compat.h
@@ -27,7 +27,7 @@
 #ifndef __XEN_PUBLIC_XEN_COMPAT_H__
 #define __XEN_PUBLIC_XEN_COMPAT_H__
 
-#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040d00
+#define __XEN_LATEST_INTERFACE_VERSION__ 0x00040e00
 
 #if defined(__XEN__) || defined(__XEN_TOOLS__)
 /* Xen is built with matching headers and implements the latest interface. */


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

[Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Jan Beulich]

To fulfill the "protected" in its name, don't let the real hardware
values "shine through". Report a control register value expressing this.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
TBD: Do we want to permit Dom0 access?

--- a/xen/arch/x86/msr.c
+++ b/xen/arch/x86/msr.c
@@ -135,6 +135,8 @@ int guest_rdmsr(struct vcpu *v, uint32_t
     case MSR_TSX_FORCE_ABORT:
     case MSR_AMD64_LWP_CFG:
     case MSR_AMD64_LWP_CBADDR:
+    case MSR_PPIN:
+    case MSR_AMD_PPIN:
         /* Not offered to guests. */
         goto gp_fault;
 
@@ -237,6 +239,18 @@ int guest_rdmsr(struct vcpu *v, uint32_t
                                    ARRAY_SIZE(msrs->dr_mask))];
         break;
 
+    case MSR_PPIN_CTL:
+        if ( d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL )
+            goto gp_fault;
+        *val = PPIN_LOCKOUT;
+        break;
+
+    case MSR_AMD_PPIN_CTL:
+        if ( !cp->extd.amd_ppin )
+            goto gp_fault;
+        *val = PPIN_LOCKOUT;
+        break;
+
         /*
          * TODO: Implement when we have better topology representation.
     case MSR_INTEL_CORE_THREAD_COUNT:
@@ -273,10 +287,14 @@ int guest_wrmsr(struct vcpu *v, uint32_t
     case MSR_INTEL_CORE_THREAD_COUNT:
     case MSR_INTEL_PLATFORM_INFO:
     case MSR_ARCH_CAPABILITIES:
+    case MSR_PPIN:
+    case MSR_AMD_PPIN:
         /* Read-only */
     case MSR_TSX_FORCE_ABORT:
     case MSR_AMD64_LWP_CFG:
     case MSR_AMD64_LWP_CBADDR:
+    case MSR_PPIN_CTL:
+    case MSR_AMD_PPIN_CTL:
         /* Not offered to guests. */
         goto gp_fault;
 


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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Andrew Cooper]

On 30/10/2019 10:39, Jan Beulich wrote:
> To fulfill the "protected" in its name, don't let the real hardware
> values "shine through". Report a control register value expressing this.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Do we want to permit Dom0 access?

I would recommend reordering the two patches and putting this one first
(along with the enumeration details, along with a pair of feature
strings in xen-cpuid).  This patch at least wants backporting.

This would be far more simple if we don't permit dom0 access.  Yes, it
shares platform responsibility with Xen, but it also can't do anything
more with the value than Xen can, which is to simply print it out for #MCEs.

Avoiding giving dom0 access would remove the need to attempt to
virtualise something which is model specific on the Intel side, and
allow all 4 MSRs to be unconditional #GP's.  I for one don't want to
have to consider the migration implications of letting guests see this.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Jan Beulich]

On 30.10.2019 12:43, Andrew Cooper wrote:
> On 30/10/2019 10:39, Jan Beulich wrote:
>> To fulfill the "protected" in its name, don't let the real hardware
>> values "shine through". Report a control register value expressing this.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> TBD: Do we want to permit Dom0 access?
> 
> I would recommend reordering the two patches and putting this one first
> (along with the enumeration details, along with a pair of feature
> strings in xen-cpuid).  This patch at least wants backporting.

Well, the reason for this ordering is because this way Dom0
doesn't transiently lose all access.

As to xen-cpuid - I admit I simply forgot to update it, largely
due to there not being any CPUID bit on the Intel side. That part
would obviously live in whichever patch we elect to be first.

> This would be far more simple if we don't permit dom0 access.  Yes, it
> shares platform responsibility with Xen, but it also can't do anything
> more with the value than Xen can, which is to simply print it out for #MCEs.

Okay, then let's not expose it. I'll drop the TBD.

> Avoiding giving dom0 access would remove the need to attempt to
> virtualise something which is model specific on the Intel side, and
> allow all 4 MSRs to be unconditional #GP's.  I for one don't want to
> have to consider the migration implications of letting guests see this.

I don't understand the connection between Dom0 access and possible
migration implications. I'm also struggling to see how, for a well
behaved guest, there would need to be any migration considerations
in the first place: Once it has read the control register and found
the lockout bit set, it shouldn't make any further access attempts.
Similarly once it got a #GP(0) upon access, it wouldn't try again.
There would be an issue only if we actually supplied PPIN values,
even if it were just fake ones.

Jan

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Andrew Cooper]

On 30/10/2019 12:02, Jan Beulich wrote:
> On 30.10.2019 12:43, Andrew Cooper wrote:
>> On 30/10/2019 10:39, Jan Beulich wrote:
>>> To fulfill the "protected" in its name, don't let the real hardware
>>> values "shine through". Report a control register value expressing this.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> TBD: Do we want to permit Dom0 access?
>> I would recommend reordering the two patches and putting this one first
>> (along with the enumeration details, along with a pair of feature
>> strings in xen-cpuid).  This patch at least wants backporting.
> Well, the reason for this ordering is because this way Dom0
> doesn't transiently lose all access.

Nothing pre-existing can be used reliably by dom0 because of the
raz/write-discard behaviour.

I wouldn't complicate patch ordering because of this.

>
> As to xen-cpuid - I admit I simply forgot to update it, largely
> due to there not being any CPUID bit on the Intel side. That part
> would obviously live in whichever patch we elect to be first.
>
>> This would be far more simple if we don't permit dom0 access.  Yes, it
>> shares platform responsibility with Xen, but it also can't do anything
>> more with the value than Xen can, which is to simply print it out for #MCEs.
> Okay, then let's not expose it. I'll drop the TBD.

I'll re-review with dom0 access in mind, but start a new thread.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Jan Beulich]

On 01.11.2019 19:35, Andrew Cooper wrote:
> On 30/10/2019 12:02, Jan Beulich wrote:
>> On 30.10.2019 12:43, Andrew Cooper wrote:
>>> On 30/10/2019 10:39, Jan Beulich wrote:
>>>> To fulfill the "protected" in its name, don't let the real hardware
>>>> values "shine through". Report a control register value expressing this.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> TBD: Do we want to permit Dom0 access?
>>> I would recommend reordering the two patches and putting this one first
>>> (along with the enumeration details, along with a pair of feature
>>> strings in xen-cpuid).  This patch at least wants backporting.
>> Well, the reason for this ordering is because this way Dom0
>> doesn't transiently lose all access.
> 
> Nothing pre-existing can be used reliably by dom0 because of the
> raz/write-discard behaviour.

Why "raz" - default behavior for "un-enumerated" MSRs is to hand
out the raw hardware value. I agree Dom0 can't _enable_ the PPIN
MSR (due to the write-discard default behavior), but on systems
where the firmware does the enabling it could still have read the
values.

Jan

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Eslam Elnikety]

Thanks for this series, Jan.

On 30.10.19 11:39, Jan Beulich wrote:
> To fulfill the "protected" in its name, don't let the real hardware
> values "shine through". Report a control register value expressing this.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> TBD: Do we want to permit Dom0 access?

It would be nice to give an administrator a way to get PPIN outside the 
context of an MCE when needed.

> 
> --- a/xen/arch/x86/msr.c
> +++ b/xen/arch/x86/msr.c
> @@ -135,6 +135,8 @@ int guest_rdmsr(struct vcpu *v, uint32_t
>       case MSR_TSX_FORCE_ABORT:
>       case MSR_AMD64_LWP_CFG:
>       case MSR_AMD64_LWP_CBADDR:
> +    case MSR_PPIN:
> +    case MSR_AMD_PPIN:
>           /* Not offered to guests. */
>           goto gp_fault;
>   
> @@ -237,6 +239,18 @@ int guest_rdmsr(struct vcpu *v, uint32_t
>                                      ARRAY_SIZE(msrs->dr_mask))];
>           break;
>   
> +    case MSR_PPIN_CTL:
> +        if ( d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL )
> +            goto gp_fault;
> +        *val = PPIN_LOCKOUT;
> +        break;
> +
> +    case MSR_AMD_PPIN_CTL:
> +        if ( !cp->extd.amd_ppin )
> +            goto gp_fault;
> +        *val = PPIN_LOCKOUT;
> +        break;
> +

nit: It is not clear to me why you use "d->arch.cpuid->.." (and not 
"cp->..") in the first if condition.

-- Eslam

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Andrew Cooper]

On 01/11/2019 14:00, Eslam Elnikety wrote:
> Thanks for this series, Jan.
>
> On 30.10.19 11:39, Jan Beulich wrote:
>> To fulfill the "protected" in its name, don't let the real hardware
>> values "shine through". Report a control register value expressing this.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> TBD: Do we want to permit Dom0 access?
>
> It would be nice to give an administrator a way to get PPIN outside
> the context of an MCE when needed.

I suppose this is a reasonable request.  We should expose it to the
hardware domain.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Andrew Cooper]

On 01/11/2019 14:29, Andrew Cooper wrote:
> On 01/11/2019 14:00, Eslam Elnikety wrote:
>> Thanks for this series, Jan.
>>
>> On 30.10.19 11:39, Jan Beulich wrote:
>>> To fulfill the "protected" in its name, don't let the real hardware
>>> values "shine through". Report a control register value expressing this.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> TBD: Do we want to permit Dom0 access?
>> It would be nice to give an administrator a way to get PPIN outside
>> the context of an MCE when needed.
> I suppose this is a reasonable request.  We should expose it to the
> hardware domain.

Actually on further thoughts, I'm going to backtrack slightly.

It is reasonable to give to dom0, but it is not reasonable to provide it
by emulating the MSR interface.  The problem is that dom0's result is
sensitive to where it happens to be scheduled.

The only sane way of letting dom0 have access is via a hypercall which
returns "no PPIN" or all sockets information in one go, irrespective of
which socket the current vcpu happens to be executing on.

This leaves us back in the (substantially easier) position of not having
to virtualise the MSR interface to begin with.

~Andrew

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Jan Beulich]

On 01.11.2019 19:49, Andrew Cooper wrote:
> On 01/11/2019 14:29, Andrew Cooper wrote:
>> On 01/11/2019 14:00, Eslam Elnikety wrote:
>>> Thanks for this series, Jan.
>>>
>>> On 30.10.19 11:39, Jan Beulich wrote:
>>>> To fulfill the "protected" in its name, don't let the real hardware
>>>> values "shine through". Report a control register value expressing this.
>>>>
>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>> ---
>>>> TBD: Do we want to permit Dom0 access?
>>> It would be nice to give an administrator a way to get PPIN outside
>>> the context of an MCE when needed.
>> I suppose this is a reasonable request.  We should expose it to the
>> hardware domain.
> 
> Actually on further thoughts, I'm going to backtrack slightly.
> 
> It is reasonable to give to dom0, but it is not reasonable to provide it
> by emulating the MSR interface.  The problem is that dom0's result is
> sensitive to where it happens to be scheduled.
> 
> The only sane way of letting dom0 have access is via a hypercall which
> returns "no PPIN" or all sockets information in one go, irrespective of
> which socket the current vcpu happens to be executing on.
> 
> This leaves us back in the (substantially easier) position of not having
> to virtualise the MSR interface to begin with.

Right, hence my question whether to make this a new sysctl subop,
or whether to permit reading of this one MSR via XENPF_resource_op
(or yet some other mechanism). Personally I'd go the
XENPF_resource_op route.

Jan

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Jan Beulich]

On 01.11.2019 15:29, Andrew Cooper wrote:
> On 01/11/2019 14:00, Eslam Elnikety wrote:
>> Thanks for this series, Jan.
>>
>> On 30.10.19 11:39, Jan Beulich wrote:
>>> To fulfill the "protected" in its name, don't let the real hardware
>>> values "shine through". Report a control register value expressing this.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> TBD: Do we want to permit Dom0 access?
>>
>> It would be nice to give an administrator a way to get PPIN outside
>> the context of an MCE when needed.
> 
> I suppose this is a reasonable request.  We should expose it to the
> hardware domain.

Via (new) sysctl (or platform op) or by allowing direct MSR read access?
(If the former, I'd want to make this addition a separate patch.)

Jan

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

Re: [Xen-devel] [PATCH 2/2] x86: explicitly disallow guest access to PPIN

[Jan Beulich]

On 01.11.2019 15:00, Eslam Elnikety wrote:
> On 30.10.19 11:39, Jan Beulich wrote:
>> @@ -237,6 +239,18 @@ int guest_rdmsr(struct vcpu *v, uint32_t
>>                                      ARRAY_SIZE(msrs->dr_mask))];
>>           break;
>>   
>> +    case MSR_PPIN_CTL:
>> +        if ( d->arch.cpuid->x86_vendor != X86_VENDOR_INTEL )
>> +            goto gp_fault;
>> +        *val = PPIN_LOCKOUT;
>> +        break;
>> +
>> +    case MSR_AMD_PPIN_CTL:
>> +        if ( !cp->extd.amd_ppin )
>> +            goto gp_fault;
>> +        *val = PPIN_LOCKOUT;
>> +        break;
>> +
> 
> nit: It is not clear to me why you use "d->arch.cpuid->.." (and not 
> "cp->..") in the first if condition.

Simple oversight; corrected for v2.

Jan

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