[Qemu-devel] [PATCH 0/3] qemu, pkeys: add memory protection-key support

[Qemu-devel] [PATCH 0/3] qemu, pkeys: add memory protection-key support

[Huaitong Han]

The protection-key feature provides an additional mechanism by which IA-32e
paging controls access to usermode addresses.

Hardware support for protection keys for user pages is enumerated with CPUID
feature flag CPUID.7.0.ECX[3]:PKU. Software support is CPUID.7.0.ECX[4]:OSPKE
with the setting of CR4.PKE(bit 22).

The PKRU register is XSAVE-managed state CPUID.D.0.EAX[9], the size of XSAVE
state component for PKRU is 8 bytes, the offset is 0xa80.

The specification of Protection Keys can be found at SDM (4.6.2, volume 3)
http://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-manual-325462.pdf.

Huaitong Han (3):
  qemu, pkeys: add pkeys support for qemu cpuid handling
  qemu, pkeys: add pkeys support for qemu xsave state handling
  qemu, pkeys: add pkeys support for qemu migration

 target-i386/cpu.c     | 23 ++++++++++++++++++++++-
 target-i386/cpu.h     |  7 +++++++
 target-i386/kvm.c     |  3 +++
 target-i386/machine.c | 23 +++++++++++++++++++++++
 4 files changed, 55 insertions(+), 1 deletion(-)

-- 
2.4.3

[Qemu-devel] [PATCH 1/3] qemu, pkeys: add pkeys support for qemu cpuid handling

[Huaitong Han]

This patch adds pkeys support for qemu cpuid handling.

Signed-off-by: Huaitong Han <huaitong.han@intel.com>
---
 target-i386/cpu.c | 21 ++++++++++++++++++++-
 target-i386/cpu.h |  4 ++++
 2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 4d1b085..575ad8d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -264,6 +264,17 @@ static const char *cpuid_7_0_ebx_feature_name[] = {
     NULL, NULL, "avx512pf", "avx512er", "avx512cd", NULL, NULL, NULL,
 };
 
+static const char *cpuid_7_0_ecx_feature_name[] = {
+    NULL, NULL, "pku", "ospke",
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+    NULL, NULL, NULL, NULL,
+};
+
 static const char *cpuid_apm_edx_feature_name[] = {
     NULL, NULL, NULL, NULL,
     NULL, NULL, NULL, NULL,
@@ -351,6 +362,7 @@ static const char *cpuid_6_feature_name[] = {
           CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
           CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
           CPUID_7_0_EBX_RDSEED */
+#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE)
 #define TCG_APM_FEATURES 0
 #define TCG_6_EAX_FEATURES CPUID_6_EAX_ARAT
 
@@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
         .cpuid_reg = R_EBX,
         .tcg_features = TCG_7_0_EBX_FEATURES,
     },
+    [FEAT_7_0_ECX] = {
+        .feat_names = cpuid_7_0_ecx_feature_name,
+        .cpuid_eax = 7,
+        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
+        .cpuid_reg = R_ECX,
+        .tcg_features = TCG_7_0_ECX_FEATURES,
+    },
     [FEAT_8000_0007_EDX] = {
         .feat_names = cpuid_apm_edx_feature_name,
         .cpuid_eax = 0x80000007,
@@ -2401,7 +2420,7 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
         if (count == 0) {
             *eax = 0; /* Maximum ECX value for sub-leaves */
             *ebx = env->features[FEAT_7_0_EBX]; /* Feature flags */
-            *ecx = 0; /* Reserved */
+            *ecx = env->features[FEAT_7_0_ECX]; /* Feature flags */
             *edx = 0; /* Reserved */
         } else {
             *eax = 0;
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index ead2832..c2e7501 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -408,6 +408,7 @@ typedef enum FeatureWord {
     FEAT_1_EDX,         /* CPUID[1].EDX */
     FEAT_1_ECX,         /* CPUID[1].ECX */
     FEAT_7_0_EBX,       /* CPUID[EAX=7,ECX=0].EBX */
+    FEAT_7_0_ECX,       /* CPUID[EAX=7,ECX=0].ECX */
     FEAT_8000_0001_EDX, /* CPUID[8000_0001].EDX */
     FEAT_8000_0001_ECX, /* CPUID[8000_0001].ECX */
     FEAT_8000_0007_EDX, /* CPUID[8000_0007].EDX */
@@ -576,6 +577,9 @@ typedef uint32_t FeatureWordArray[FEATURE_WORDS];
 #define CPUID_7_0_EBX_AVX512ER (1U << 27) /* AVX-512 Exponential and Reciprocal */
 #define CPUID_7_0_EBX_AVX512CD (1U << 28) /* AVX-512 Conflict Detection */
 
+#define CPUID_7_0_ECX_PKU      (1U << 3)
+#define CPUID_7_0_ECX_OSPKE    (1U << 4)
+
 #define CPUID_XSAVE_XSAVEOPT   (1U << 0)
 #define CPUID_XSAVE_XSAVEC     (1U << 1)
 #define CPUID_XSAVE_XGETBV1    (1U << 2)
-- 
2.4.3

Re: [Qemu-devel] [PATCH 1/3] qemu, pkeys: add pkeys support for qemu cpuid handling

[Paolo Bonzini]

On 09/11/2015 12:55, Huaitong Han wrote:
> @@ -351,6 +362,7 @@ static const char *cpuid_6_feature_name[] = {
>            CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
>            CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
>            CPUID_7_0_EBX_RDSEED */
> +#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE)

This should be zero.  Apart from this detail, the QEMU parts look good.

Paolo

>  #define TCG_APM_FEATURES 0
>  #define TCG_6_EAX_FEATURES CPUID_6_EAX_ARAT
>  
> @@ -408,6 +420,13 @@ static FeatureWordInfo feature_word_info[FEATURE_WORDS] = {
>          .cpuid_reg = R_EBX,
>          .tcg_features = TCG_7_0_EBX_FEATURES,
>      },
> +    [FEAT_7_0_ECX] = {
> +        .feat_names = cpuid_7_0_ecx_feature_name,
> +        .cpuid_eax = 7,
> +        .cpuid_needs_ecx = true, .cpuid_ecx = 0,
> +        .cpuid_reg = R_ECX,
> +        .tcg_features = TCG_7_0_ECX_FEATURES,
> +    },

Re: [Qemu-devel] [PATCH 1/3] qemu, pkeys: add pkeys support for qemu cpuid handling

[Andreas Färber]

Am 09.11.2015 um 13:24 schrieb Paolo Bonzini:
> On 09/11/2015 12:55, Huaitong Han wrote:
>> @@ -351,6 +362,7 @@ static const char *cpuid_6_feature_name[] = {
>>            CPUID_7_0_EBX_FSGSBASE, CPUID_7_0_EBX_HLE, CPUID_7_0_EBX_AVX2,
>>            CPUID_7_0_EBX_ERMS, CPUID_7_0_EBX_INVPCID, CPUID_7_0_EBX_RTM,
>>            CPUID_7_0_EBX_RDSEED */
>> +#define TCG_7_0_ECX_FEATURES (CPUID_7_0_ECX_PKU | CPUID_7_0_ECX_OSPKE)
> 
> This should be zero.  Apart from this detail, the QEMU parts look good.

...except for the subjects, which should be "target-i386: add pkeys
support for cpuid handling" etc. - no need to put qemu into a QEMU
commit subject, especially not twice.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg)

[Qemu-devel] [PATCH 2/3] qemu, pkeys: add pkeys support for qemu xsave state handling

[Huaitong Han]

This patch adds pkeys support for qemu xsave state handling.

Signed-off-by: Huaitong Han <huaitong.han@intel.com>
---
 target-i386/cpu.c | 2 ++
 target-i386/cpu.h | 3 +++
 target-i386/kvm.c | 3 +++
 3 files changed, 8 insertions(+)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 575ad8d..7a6a3f8 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -487,6 +487,8 @@ static const ExtSaveArea ext_save_areas[] = {
             .offset = 0x480, .size = 0x200 },
     [7] = { .feature = FEAT_7_0_EBX, .bits = CPUID_7_0_EBX_AVX512F,
             .offset = 0x680, .size = 0x400 },
+    [9] = { .feature = FEAT_7_0_ECX, .bits = CPUID_7_0_ECX_PKU,
+            .offset = 0xA80, .size = 0x8 },
 };
 
 const char *get_register_name_32(unsigned int reg)
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index c2e7501..2230b3e 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -401,6 +401,7 @@
 #define XSTATE_OPMASK                   (1ULL << 5)
 #define XSTATE_ZMM_Hi256                (1ULL << 6)
 #define XSTATE_Hi16_ZMM                 (1ULL << 7)
+#define XSTATE_PKRU                     (1ULL << 9)
 
 
 /* CPUID feature words */
@@ -984,6 +985,8 @@ typedef struct CPUX86State {
     uint64_t xcr0;
     uint64_t xss;
 
+    uint32_t pkru;
+
     TPRAccess tpr_access_type;
 } CPUX86State;
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 066d03d..12164a6 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1092,6 +1092,7 @@ static int kvm_put_fpu(X86CPU *cpu)
 #define XSAVE_OPMASK      272
 #define XSAVE_ZMM_Hi256   288
 #define XSAVE_Hi16_ZMM    416
+#define XSAVE_PKRU        672
 
 static int kvm_put_xsave(X86CPU *cpu)
 {
@@ -1145,6 +1146,7 @@ static int kvm_put_xsave(X86CPU *cpu)
 #ifdef TARGET_X86_64
     memcpy(&xsave->region[XSAVE_Hi16_ZMM], &env->xmm_regs[16],
             16 * sizeof env->xmm_regs[16]);
+    memcpy(&xsave->region[XSAVE_PKRU], &env->pkru, sizeof env->pkru);
 #endif
     r = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_XSAVE, xsave);
     return r;
@@ -1516,6 +1518,7 @@ static int kvm_get_xsave(X86CPU *cpu)
 #ifdef TARGET_X86_64
     memcpy(&env->xmm_regs[16], &xsave->region[XSAVE_Hi16_ZMM],
            16 * sizeof env->xmm_regs[16]);
+    memcpy(&xsave->region[XSAVE_PKRU], &env->pkru, sizeof env->pkru);
 #endif
     return 0;
 }
-- 
2.4.3

Re: [Qemu-devel] [PATCH 2/3] qemu, pkeys: add pkeys support for qemu xsave state handling

[Eduardo Habkost]

On Mon, Nov 09, 2015 at 07:55:33PM +0800, Huaitong Han wrote:
> This patch adds pkeys support for qemu xsave state handling.
> 
> Signed-off-by: Huaitong Han <huaitong.han@intel.com>
[...]
> @@ -1145,6 +1146,7 @@ static int kvm_put_xsave(X86CPU *cpu)
>  #ifdef TARGET_X86_64
>      memcpy(&xsave->region[XSAVE_Hi16_ZMM], &env->xmm_regs[16],
>              16 * sizeof env->xmm_regs[16]);
> +    memcpy(&xsave->region[XSAVE_PKRU], &env->pkru, sizeof env->pkru);
>  #endif
>      r = kvm_vcpu_ioctl(CPU(cpu), KVM_SET_XSAVE, xsave);
>      return r;
> @@ -1516,6 +1518,7 @@ static int kvm_get_xsave(X86CPU *cpu)
>  #ifdef TARGET_X86_64
>      memcpy(&env->xmm_regs[16], &xsave->region[XSAVE_Hi16_ZMM],
>             16 * sizeof env->xmm_regs[16]);
> +    memcpy(&xsave->region[XSAVE_PKRU], &env->pkru, sizeof env->pkru);

Did you mean:
  memcpy(&env->pkru, &xsave->region[XSAVE_PKRU], sizeof env->pkru)

-- 
Eduardo

[Qemu-devel] [PATCH 3/3] qemu, pkeys: add pkeys support for qemu migration

[Huaitong Han]

This patch adds pkeys support for qemu migration.

Signed-off-by: Huaitong Han <huaitong.han@intel.com>
---
 target-i386/machine.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/target-i386/machine.c b/target-i386/machine.c
index a0df64b..1b190c7 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -725,6 +725,26 @@ static const VMStateDescription vmstate_xss = {
         VMSTATE_END_OF_LIST()
     }
 };
+#ifdef TARGET_X86_64
+static bool pkru_needed(void *opaque)
+{
+    X86CPU *cpu = opaque;
+    CPUX86State *env = &cpu->env;
+
+    return env->pkru != 0;
+}
+
+static const VMStateDescription vmstate_pkru = {
+    .name = "cpu/pkru",
+    .version_id = 1,
+    .minimum_version_id = 1,
+    .needed = pkru_needed,
+    .fields = (VMStateField[]){
+        VMSTATE_UINT32(env.pkru, X86CPU),
+        VMSTATE_END_OF_LIST()
+    }
+};
+#endif
 
 VMStateDescription vmstate_x86_cpu = {
     .name = "cpu",
@@ -844,6 +864,9 @@ VMStateDescription vmstate_x86_cpu = {
         &vmstate_msr_hyperv_time,
         &vmstate_avx512,
         &vmstate_xss,
+#ifdef TARGET_X86_64
+        &vmstate_pkru,
+#endif
         NULL
     }
 };
-- 
2.4.3