[05/15] s390x: protvirt: Sync PV state
diff mbox series

Message ID 20191120114334.2287-6-frankja@linux.ibm.com
State New
Headers show
Series
  • s390x: Protected Virtualization support
Related show

Commit Message

Janosch Frank Nov. 20, 2019, 11:43 a.m. UTC
We do not always have the SIE intercept code handy at each place where
we do emulation. Unfortunately emulation for secure guests often
differ slightly from normal emulation and we need to make decisions
based on the protected state of the VCPU.

Let's sync the protected state and make it available.

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
---
 linux-headers/asm-s390/kvm.h | 1 +
 target/s390x/cpu.h           | 1 +
 target/s390x/kvm.c           | 4 ++++
 3 files changed, 6 insertions(+)

Comments

Cornelia Huck Nov. 21, 2019, 1:25 p.m. UTC | #1
On Wed, 20 Nov 2019 06:43:24 -0500
Janosch Frank <frankja@linux.ibm.com> wrote:

> We do not always have the SIE intercept code handy at each place where
> we do emulation. Unfortunately emulation for secure guests often
> differ slightly from normal emulation and we need to make decisions
> based on the protected state of the VCPU.
> 
> Let's sync the protected state and make it available.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  linux-headers/asm-s390/kvm.h | 1 +
>  target/s390x/cpu.h           | 1 +
>  target/s390x/kvm.c           | 4 ++++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index 41976d33f0..7c46cf6078 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -231,6 +231,7 @@ struct kvm_guest_debug_arch {
>  #define KVM_SYNC_GSCB   (1UL << 9)
>  #define KVM_SYNC_BPBC   (1UL << 10)
>  #define KVM_SYNC_ETOKEN (1UL << 11)
> +#define KVM_SYNC_PV	(1UL << 12)

That should go into the previous patch (will be picked up by header
sync).

>  /* length and alignment of the sdnx as a power of two */
>  #define SDNXC 8
>  #define SDNXL (1UL << SDNXC)
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 17460ed7b3..a787221772 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -116,6 +116,7 @@ struct CPUS390XState {
>  
>      /* Fields up to this point are cleared by a CPU reset */
>      struct {} end_reset_fields;
> +    bool pv; /* protected virtualization */
>  
>  #if !defined(CONFIG_USER_ONLY)
>      uint32_t core_id; /* PoP "CPU address", same as cpu_index */
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index c24c869e77..418154ccfe 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -676,6 +676,10 @@ int kvm_arch_get_registers(CPUState *cs)
>          env->etoken_extension = cs->kvm_run->s.regs.etoken_extension;
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_PV)) {
> +        env->pv = !!cs->kvm_run->s.regs.pv;
> +    }
> +
>      /* pfault parameters */
>      if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
>          env->pfault_token = cs->kvm_run->s.regs.pft;

As you add a new field to the cpu state... you probably can't migrate
protected guests, so you don't need a new vmstate subsection?
Janosch Frank Nov. 21, 2019, 1:43 p.m. UTC | #2
On 11/21/19 2:25 PM, Cornelia Huck wrote:
> On Wed, 20 Nov 2019 06:43:24 -0500
> Janosch Frank <frankja@linux.ibm.com> wrote:
> 
>> We do not always have the SIE intercept code handy at each place where
>> we do emulation. Unfortunately emulation for secure guests often
>> differ slightly from normal emulation and we need to make decisions
>> based on the protected state of the VCPU.
>>
>> Let's sync the protected state and make it available.
>>
>> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
>> ---
>>  linux-headers/asm-s390/kvm.h | 1 +
>>  target/s390x/cpu.h           | 1 +
>>  target/s390x/kvm.c           | 4 ++++
>>  3 files changed, 6 insertions(+)
>>
>> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
>> index 41976d33f0..7c46cf6078 100644
>> --- a/linux-headers/asm-s390/kvm.h
>> +++ b/linux-headers/asm-s390/kvm.h
>> @@ -231,6 +231,7 @@ struct kvm_guest_debug_arch {
>>  #define KVM_SYNC_GSCB   (1UL << 9)
>>  #define KVM_SYNC_BPBC   (1UL << 10)
>>  #define KVM_SYNC_ETOKEN (1UL << 11)
>> +#define KVM_SYNC_PV	(1UL << 12)
> 
> That should go into the previous patch (will be picked up by header
> sync).

Yes, will be fixed in a second

> 
>>  /* length and alignment of the sdnx as a power of two */
>>  #define SDNXC 8
>>  #define SDNXL (1UL << SDNXC)
>> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
>> index 17460ed7b3..a787221772 100644
>> --- a/target/s390x/cpu.h
>> +++ b/target/s390x/cpu.h
>> @@ -116,6 +116,7 @@ struct CPUS390XState {
>>  
>>      /* Fields up to this point are cleared by a CPU reset */
>>      struct {} end_reset_fields;
>> +    bool pv; /* protected virtualization */
>>  
>>  #if !defined(CONFIG_USER_ONLY)
>>      uint32_t core_id; /* PoP "CPU address", same as cpu_index */
>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>> index c24c869e77..418154ccfe 100644
>> --- a/target/s390x/kvm.c
>> +++ b/target/s390x/kvm.c
>> @@ -676,6 +676,10 @@ int kvm_arch_get_registers(CPUState *cs)
>>          env->etoken_extension = cs->kvm_run->s.regs.etoken_extension;
>>      }
>>  
>> +    if (can_sync_regs(cs, KVM_SYNC_PV)) {
>> +        env->pv = !!cs->kvm_run->s.regs.pv;
>> +    }
>> +
>>      /* pfault parameters */
>>      if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
>>          env->pfault_token = cs->kvm_run->s.regs.pft;
> 
> As you add a new field to the cpu state... you probably can't migrate
> protected guests, so you don't need a new vmstate subsection?
> 

We can't migrate currently, but it's on out todo list
Thomas Huth Nov. 21, 2019, 2:43 p.m. UTC | #3
On 20/11/2019 12.43, Janosch Frank wrote:
> We do not always have the SIE intercept code handy at each place where
> we do emulation. Unfortunately emulation for secure guests often
> differ slightly from normal emulation and we need to make decisions
> based on the protected state of the VCPU.
> 
> Let's sync the protected state and make it available.
> 
> Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
> ---
>  linux-headers/asm-s390/kvm.h | 1 +
>  target/s390x/cpu.h           | 1 +
>  target/s390x/kvm.c           | 4 ++++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
> index 41976d33f0..7c46cf6078 100644
> --- a/linux-headers/asm-s390/kvm.h
> +++ b/linux-headers/asm-s390/kvm.h
> @@ -231,6 +231,7 @@ struct kvm_guest_debug_arch {
>  #define KVM_SYNC_GSCB   (1UL << 9)
>  #define KVM_SYNC_BPBC   (1UL << 10)
>  #define KVM_SYNC_ETOKEN (1UL << 11)
> +#define KVM_SYNC_PV	(1UL << 12)
>  /* length and alignment of the sdnx as a power of two */
>  #define SDNXC 8
>  #define SDNXL (1UL << SDNXC)
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 17460ed7b3..a787221772 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -116,6 +116,7 @@ struct CPUS390XState {
>  
>      /* Fields up to this point are cleared by a CPU reset */
>      struct {} end_reset_fields;
> +    bool pv; /* protected virtualization */
>  
>  #if !defined(CONFIG_USER_ONLY)
>      uint32_t core_id; /* PoP "CPU address", same as cpu_index */
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index c24c869e77..418154ccfe 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -676,6 +676,10 @@ int kvm_arch_get_registers(CPUState *cs)
>          env->etoken_extension = cs->kvm_run->s.regs.etoken_extension;
>      }
>  
> +    if (can_sync_regs(cs, KVM_SYNC_PV)) {
> +        env->pv = !!cs->kvm_run->s.regs.pv;
> +    }

Does this really need to be sync'ed each time during
kvm_arch_get_registers()? I mean, this is not a state that continuously
changes ... so when I guest enters PV mode, I assume that we have to do
some stuff in QEMU anyway, so we could set the "pv = true" in that case.
And during reset, we could clear it again. Or do I miss something?

 Thomas

Patch
diff mbox series

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 41976d33f0..7c46cf6078 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -231,6 +231,7 @@  struct kvm_guest_debug_arch {
 #define KVM_SYNC_GSCB   (1UL << 9)
 #define KVM_SYNC_BPBC   (1UL << 10)
 #define KVM_SYNC_ETOKEN (1UL << 11)
+#define KVM_SYNC_PV	(1UL << 12)
 /* length and alignment of the sdnx as a power of two */
 #define SDNXC 8
 #define SDNXL (1UL << SDNXC)
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 17460ed7b3..a787221772 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -116,6 +116,7 @@  struct CPUS390XState {
 
     /* Fields up to this point are cleared by a CPU reset */
     struct {} end_reset_fields;
+    bool pv; /* protected virtualization */
 
 #if !defined(CONFIG_USER_ONLY)
     uint32_t core_id; /* PoP "CPU address", same as cpu_index */
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index c24c869e77..418154ccfe 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -676,6 +676,10 @@  int kvm_arch_get_registers(CPUState *cs)
         env->etoken_extension = cs->kvm_run->s.regs.etoken_extension;
     }
 
+    if (can_sync_regs(cs, KVM_SYNC_PV)) {
+        env->pv = !!cs->kvm_run->s.regs.pv;
+    }
+
     /* pfault parameters */
     if (can_sync_regs(cs, KVM_SYNC_PFAULT)) {
         env->pfault_token = cs->kvm_run->s.regs.pft;