Re: [PATCH 05/15] s390x: protvirt: Sync PV state

[Thomas Huth]

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 <address@hidden>
> ---
>  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