| Message ID | 20210122235049.3107620-2-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Revert dirty tracking for GPRs | expand |
On 1/22/21 5:50 PM, Sean Christopherson wrote: > Drop the per-GPR dirty checks when synchronizing GPRs to the GHCB, the > GRPs' dirty bits are set from time zero and never cleared, i.e. will Ah, missed that, bad assumption on my part. > always be seen as dirty. The obvious alternative would be to clear > the dirty bits when appropriate, but removing the dirty checks is > desirable as it allows reverting GPR dirty+available tracking, which > adds overhead to all flavors of x86 VMs. > > Note, unconditionally writing the GPRs in the GHCB is tacitly allowed > by the GHCB spec, which allows the hypervisor (or guest) to provide > unnecessary info; it's the guest's responsibility to consume only what > it needs (the hypervisor is untrusted after all). > > The guest and hypervisor can supply additional state if desired but > must not rely on that additional state being provided. Yes, that's true. I'm ok with removing the tracking if that's desired. Otherwise, we can add a vcpu->arch.regs_dirty = 0 in sev_es_sync_from_ghcb(). Thanks, Tom > > Cc: Brijesh Singh <brijesh.singh@amd.com> > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Fixes: 291bd20d5d88 ("KVM: SVM: Add initial support for a VMGEXIT VMEXIT") > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/kvm/svm/sev.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c > index c8ffdbc81709..ac652bc476ae 100644 > --- a/arch/x86/kvm/svm/sev.c > +++ b/arch/x86/kvm/svm/sev.c > @@ -1415,16 +1415,13 @@ static void sev_es_sync_to_ghcb(struct vcpu_svm *svm) > * to be returned: > * GPRs RAX, RBX, RCX, RDX > * > - * Copy their values to the GHCB if they are dirty. > + * Copy their values, even if they may not have been written during the > + * VM-Exit. It's the guest's responsibility to not consume random data. > */ > - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RAX)) > - ghcb_set_rax(ghcb, vcpu->arch.regs[VCPU_REGS_RAX]); > - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RBX)) > - ghcb_set_rbx(ghcb, vcpu->arch.regs[VCPU_REGS_RBX]); > - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RCX)) > - ghcb_set_rcx(ghcb, vcpu->arch.regs[VCPU_REGS_RCX]); > - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RDX)) > - ghcb_set_rdx(ghcb, vcpu->arch.regs[VCPU_REGS_RDX]); > + ghcb_set_rax(ghcb, vcpu->arch.regs[VCPU_REGS_RAX]); > + ghcb_set_rbx(ghcb, vcpu->arch.regs[VCPU_REGS_RBX]); > + ghcb_set_rcx(ghcb, vcpu->arch.regs[VCPU_REGS_RCX]); > + ghcb_set_rdx(ghcb, vcpu->arch.regs[VCPU_REGS_RDX]); > } > > static void sev_es_sync_from_ghcb(struct vcpu_svm *svm) >
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c index c8ffdbc81709..ac652bc476ae 100644 --- a/arch/x86/kvm/svm/sev.c +++ b/arch/x86/kvm/svm/sev.c @@ -1415,16 +1415,13 @@ static void sev_es_sync_to_ghcb(struct vcpu_svm *svm) * to be returned: * GPRs RAX, RBX, RCX, RDX * - * Copy their values to the GHCB if they are dirty. + * Copy their values, even if they may not have been written during the + * VM-Exit. It's the guest's responsibility to not consume random data. */ - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RAX)) - ghcb_set_rax(ghcb, vcpu->arch.regs[VCPU_REGS_RAX]); - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RBX)) - ghcb_set_rbx(ghcb, vcpu->arch.regs[VCPU_REGS_RBX]); - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RCX)) - ghcb_set_rcx(ghcb, vcpu->arch.regs[VCPU_REGS_RCX]); - if (kvm_register_is_dirty(vcpu, VCPU_REGS_RDX)) - ghcb_set_rdx(ghcb, vcpu->arch.regs[VCPU_REGS_RDX]); + ghcb_set_rax(ghcb, vcpu->arch.regs[VCPU_REGS_RAX]); + ghcb_set_rbx(ghcb, vcpu->arch.regs[VCPU_REGS_RBX]); + ghcb_set_rcx(ghcb, vcpu->arch.regs[VCPU_REGS_RCX]); + ghcb_set_rdx(ghcb, vcpu->arch.regs[VCPU_REGS_RDX]); } static void sev_es_sync_from_ghcb(struct vcpu_svm *svm)
Drop the per-GPR dirty checks when synchronizing GPRs to the GHCB, the GRPs' dirty bits are set from time zero and never cleared, i.e. will always be seen as dirty. The obvious alternative would be to clear the dirty bits when appropriate, but removing the dirty checks is desirable as it allows reverting GPR dirty+available tracking, which adds overhead to all flavors of x86 VMs. Note, unconditionally writing the GPRs in the GHCB is tacitly allowed by the GHCB spec, which allows the hypervisor (or guest) to provide unnecessary info; it's the guest's responsibility to consume only what it needs (the hypervisor is untrusted after all). The guest and hypervisor can supply additional state if desired but must not rely on that additional state being provided. Cc: Brijesh Singh <brijesh.singh@amd.com> Cc: Tom Lendacky <thomas.lendacky@amd.com> Fixes: 291bd20d5d88 ("KVM: SVM: Add initial support for a VMGEXIT VMEXIT") Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/svm/sev.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)