[PATCH] libxc/x86: don't hand through CPUID leaf 0x80000008 as is

[PATCH] libxc/x86: don't hand through CPUID leaf 0x80000008 as is

[Jan Beulich]

Just like for HVM the feature set should be used for EBX output, while
EAX should be restricted to the low 16 bits and ECX/EDX should be zero.

Short of there being white listing in place just like on the HVM side,
also zap leaves 6, 9, and 0x80000007 as well as unknown / reserved ones.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -575,12 +575,26 @@ static void xc_cpuid_pv_policy(xc_interf
         break;
     }
 
+    case 0x80000008:
+        regs[0] &= 0x0000ffffu;
+        regs[1] = info->featureset[featureword_of(X86_FEATURE_CLZERO)];
+        regs[2] = regs[3] = 0;
+        break;
+
     case 0x00000005: /* MONITOR/MWAIT */
+    case 0x00000006: /* Thermal and Power Management */
+    case 0x00000008: /* reserved */
+    case 0x00000009: /* Direct Cache Access */
     case 0x0000000b: /* Extended Topology Enumeration */
+    case 0x0000000c: /* reserved */
+    case DEF_MAX_BASE + 1 ... 0x0000ffff: /* unknown / reserved */
+    case 0x80000007: /* Power Management / RAS */
+    case 0x80000009: /* reserved */
     case 0x8000000a: /* SVM revision and features */
+    case 0x8000000b ... 0x80000018: /* reserved */
     case 0x8000001b: /* Instruction Based Sampling */
     case 0x8000001c: /* Light Weight Profiling */
-    case 0x8000001e: /* Extended topology reporting */
+    case max_c(DEF_MAX_INTELEXT, DEF_MAX_AMDEXT) + 1 ... 0x8000ffff:
         regs[0] = regs[1] = regs[2] = regs[3] = 0;
         break;
     }
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -408,6 +408,10 @@ int xc_ffs64(uint64_t x);
 #define max_t(type,x,y) \
         ({ type __x = (x); type __y = (y); __x > __y ? __x: __y; })
 
+/* Use these only in places where constant expressions are required. */
+#define min_c(x, y) ((x) < (y) ? (x) : (y))
+#define max_c(x, y) ((x) > (y) ? (x) : (y))
+
 #define DOMPRINTF(fmt, args...) xc_dom_printf(dom->xch, fmt, ## args)
 #define DOMPRINTF_CALLED(xch) xc_dom_printf((xch), "%s: called", __FUNCTION__)
 





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

Re: [PATCH] libxc/x86: don't hand through CPUID leaf 0x80000008 as is

[Andrew Cooper]

On 17/05/18 14:20, Jan Beulich wrote:
> Just like for HVM the feature set should be used for EBX output, while
> EAX should be restricted to the low 16 bits and ECX/EDX should be zero.
>
> Short of there being white listing in place just like on the HVM side,
> also zap leaves 6, 9, and 0x80000007 as well as unknown / reserved ones.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Do you want this for backporting?

The changes below are enforced by recalculate_cpuid_policy() (and in
particular, recaluclate_misc()) in the hypervisor for the past few
releases, and Sergey is currently in the process of making all of this
libxc logic disappear.

~Andrew

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

Re: [PATCH] libxc/x86: don't hand through CPUID leaf 0x80000008 as is

[Jan Beulich]

>>> On 17.05.18 at 15:26, <andrew.cooper3@citrix.com> wrote:
> On 17/05/18 14:20, Jan Beulich wrote:
>> Just like for HVM the feature set should be used for EBX output, while
>> EAX should be restricted to the low 16 bits and ECX/EDX should be zero.
>>
>> Short of there being white listing in place just like on the HVM side,
>> also zap leaves 6, 9, and 0x80000007 as well as unknown / reserved ones.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Do you want this for backporting?

Not really, at least that wasn't a primary goal.

> The changes below are enforced by recalculate_cpuid_policy() (and in
> particular, recaluclate_misc()) in the hypervisor for the past few
> releases, and Sergey is currently in the process of making all of this
> libxc logic disappear.

Well, maybe I'm simply confused: Commit d297b56682 ("x86/cpuid: Handling
of IBRS/IBPB, STIBP and IBRS for guests") introduced similar code into
xc_cpuid_hvm_policy() without doing the same for xc_cpuid_pv_policy().
That's pretty recent a commit, and one that has been backported all the
way through to 4.6. Are you saying that was a pointless change then?

Jan



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

Re: [PATCH] libxc/x86: don't hand through CPUID leaf 0x80000008 as is

[Andrew Cooper]

On 17/05/18 14:38, Jan Beulich wrote:
>>>> On 17.05.18 at 15:26, <andrew.cooper3@citrix.com> wrote:
>> On 17/05/18 14:20, Jan Beulich wrote:
>>> Just like for HVM the feature set should be used for EBX output, while
>>> EAX should be restricted to the low 16 bits and ECX/EDX should be zero.
>>>
>>> Short of there being white listing in place just like on the HVM side,
>>> also zap leaves 6, 9, and 0x80000007 as well as unknown / reserved ones.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Do you want this for backporting?
> Not really, at least that wasn't a primary goal.
>
>> The changes below are enforced by recalculate_cpuid_policy() (and in
>> particular, recaluclate_misc()) in the hypervisor for the past few
>> releases, and Sergey is currently in the process of making all of this
>> libxc logic disappear.
> Well, maybe I'm simply confused: Commit d297b56682 ("x86/cpuid: Handling
> of IBRS/IBPB, STIBP and IBRS for guests") introduced similar code into
> xc_cpuid_hvm_policy() without doing the same for xc_cpuid_pv_policy().
> That's pretty recent a commit, and one that has been backported all the
> way through to 4.6. Are you saying that was a pointless change then?

No sorry - you're completely correct.

Without the PV side, a guest will by default get the same settings as
dom0.  The reason why my XTF tests doesn't notice this is because libxl
uses a separate path, and XenServer uses a yet-different path.

The PV side wants to gain a matching clzero hunk.

My comment about recalculate_cpuid_policy() applies to the clamping part
of the change.

~Andrew

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