lkml.org 
[lkml]   [2021]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] KVM: x86: Always set kvm_run->if_flag
From
Date
On 12/6/21 10:31 PM, Marc Orr wrote:
> The kvm_run struct's if_flag is apart of the userspace/kernel API. The
> SEV-ES patches failed to set this flag because it's no longer needed by
> QEMU (according to the comment in the source code). However, other
> hypervisors may make use of this flag. Therefore, set the flag for
> guests with encrypted regiesters (i.e., with guest_state_protected set).
>
> Fixes: f1c6366e3043 ("KVM: SVM: Add required changes to support intercepts under SEV-ES")
> Signed-off-by: Marc Orr <marcorr@google.com>
> ---
> arch/x86/include/asm/kvm-x86-ops.h | 1 +
> arch/x86/include/asm/kvm_host.h | 1 +
> arch/x86/kvm/svm/svm.c | 8 ++++++++
> arch/x86/kvm/vmx/vmx.c | 6 ++++++
> arch/x86/kvm/x86.c | 9 +--------
> 5 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm-x86-ops.h b/arch/x86/include/asm/kvm-x86-ops.h
> index cefe1d81e2e8..9e50da3ed01a 100644
> --- a/arch/x86/include/asm/kvm-x86-ops.h
> +++ b/arch/x86/include/asm/kvm-x86-ops.h
> @@ -47,6 +47,7 @@ KVM_X86_OP(set_dr7)
> KVM_X86_OP(cache_reg)
> KVM_X86_OP(get_rflags)
> KVM_X86_OP(set_rflags)
> +KVM_X86_OP(get_if_flag)
> KVM_X86_OP(tlb_flush_all)
> KVM_X86_OP(tlb_flush_current)
> KVM_X86_OP_NULL(tlb_remote_flush)
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 860ed500580c..a7f868ff23e7 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -1349,6 +1349,7 @@ struct kvm_x86_ops {
> void (*cache_reg)(struct kvm_vcpu *vcpu, enum kvm_reg reg);
> unsigned long (*get_rflags)(struct kvm_vcpu *vcpu);
> void (*set_rflags)(struct kvm_vcpu *vcpu, unsigned long rflags);
> + bool (*get_if_flag)(struct kvm_vcpu *vcpu);
>
> void (*tlb_flush_all)(struct kvm_vcpu *vcpu);
> void (*tlb_flush_current)(struct kvm_vcpu *vcpu);
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d0f68d11ec70..91608f8c0cde 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1585,6 +1585,13 @@ static void svm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
> to_svm(vcpu)->vmcb->save.rflags = rflags;
> }
>
> +static bool svm_get_if_flag(struct kvm_vcpu *vcpu)
> +{
> + struct vmcb *vmcb = to_svm(vcpu)->vmcb;
> +
> + return !!(vmcb->control.int_state & SVM_GUEST_INTERRUPT_MASK);

I'm not sure if this is always valid to use for non SEV-ES guests. Maybe
the better thing would be:

return sev_es_guest(vcpu->kvm) ? vmcb->control.int_state & SVM_GUEST_INTERRUPT_MASK
: kvm_get_rflags(vcpu) & X86_EFLAGS_IF;

(Since this function returns a bool, I don't think you need the !!)

Thanks,
Tom

> +}
> +
> static void svm_cache_reg(struct kvm_vcpu *vcpu, enum kvm_reg reg)
> {
> switch (reg) {
> @@ -4621,6 +4628,7 @@ static struct kvm_x86_ops svm_x86_ops __initdata = {
> .cache_reg = svm_cache_reg,
> .get_rflags = svm_get_rflags,
> .set_rflags = svm_set_rflags,
> + .get_if_flag = svm_get_if_flag,
>
> .tlb_flush_all = svm_flush_tlb,
> .tlb_flush_current = svm_flush_tlb,
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 9453743ce0c4..6056baa13977 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1363,6 +1363,11 @@ void vmx_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags)
> vmx->emulation_required = vmx_emulation_required(vcpu);
> }
>
> +static bool vmx_get_if_flag(struct kvm_vcpu *vcpu)
> +{
> + return !!(vmx_get_rflags(vcpu) & X86_EFLAGS_IF);
> +}
> +
> u32 vmx_get_interrupt_shadow(struct kvm_vcpu *vcpu)
> {
> u32 interruptibility = vmcs_read32(GUEST_INTERRUPTIBILITY_INFO);
> @@ -7575,6 +7580,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = {
> .cache_reg = vmx_cache_reg,
> .get_rflags = vmx_get_rflags,
> .set_rflags = vmx_set_rflags,
> + .get_if_flag = vmx_get_if_flag,
>
> .tlb_flush_all = vmx_flush_tlb_all,
> .tlb_flush_current = vmx_flush_tlb_current,
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e0aa4dd53c7f..45e836db5bcd 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -8995,14 +8995,7 @@ static void post_kvm_run_save(struct kvm_vcpu *vcpu)
> {
> struct kvm_run *kvm_run = vcpu->run;
>
> - /*
> - * if_flag is obsolete and useless, so do not bother
> - * setting it for SEV-ES guests. Userspace can just
> - * use kvm_run->ready_for_interrupt_injection.
> - */
> - kvm_run->if_flag = !vcpu->arch.guest_state_protected
> - && (kvm_get_rflags(vcpu) & X86_EFLAGS_IF) != 0;
> -
> + kvm_run->if_flag = static_call(kvm_x86_get_if_flag)(vcpu);
> kvm_run->cr8 = kvm_get_cr8(vcpu);
> kvm_run->apic_base = kvm_get_apic_base(vcpu);
>
>

\
 
 \ /
  Last update: 2021-12-07 15:44    [W:0.089 / U:0.888 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site