| Message ID | 20220204214205.3306634-3-seanjc@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: x86: Prep work for VMX IPI virtualization | expand |
>--- a/arch/x86/kvm/vmx/vmx.c >+++ b/arch/x86/kvm/vmx/vmx.c >@@ -5302,9 +5302,16 @@ static int handle_apic_eoi_induced(struct kvm_vcpu *vcpu) > static int handle_apic_write(struct kvm_vcpu *vcpu) > { > unsigned long exit_qualification = vmx_get_exit_qual(vcpu); >- u32 offset = exit_qualification & 0xfff; > >- /* APIC-write VM exit is trap-like and thus no need to adjust IP */ >+ /* >+ * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and >+ * hardware has done any necessary aliasing, offset adjustments, etc... >+ * for the access. I.e. the correct value has already been written to >+ * the vAPIC page for the correct 16-byte chunk. KVM needs only to >+ * retrieve the register value and emulate the access. >+ */ >+ u32 offset = exit_qualification & 0xff0; Can we take this opportunity to remove offset/exit_qualification? They are used just once. >+ > kvm_apic_write_nodecode(vcpu, offset); > return 1; > } >-- >2.35.0.263.gb82422642f-goog >
On Tue, Feb 15, 2022, Chao Gao wrote: > >--- a/arch/x86/kvm/vmx/vmx.c > >+++ b/arch/x86/kvm/vmx/vmx.c > >@@ -5302,9 +5302,16 @@ static int handle_apic_eoi_induced(struct kvm_vcpu *vcpu) > > static int handle_apic_write(struct kvm_vcpu *vcpu) > > { > > unsigned long exit_qualification = vmx_get_exit_qual(vcpu); > >- u32 offset = exit_qualification & 0xfff; > > > >- /* APIC-write VM exit is trap-like and thus no need to adjust IP */ > >+ /* > >+ * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and > >+ * hardware has done any necessary aliasing, offset adjustments, etc... > >+ * for the access. I.e. the correct value has already been written to > >+ * the vAPIC page for the correct 16-byte chunk. KVM needs only to > >+ * retrieve the register value and emulate the access. > >+ */ > >+ u32 offset = exit_qualification & 0xff0; > > Can we take this opportunity to remove offset/exit_qualification? > They are used just once. Definitely should have dropped exit_qualification, not sure why I didn't. I'd prefer to keep offset to document what is held in vmcs.EXIT_QUALIFICATION without having to add an explicit comment.
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c index 4662469240bc..fbce455a9d17 100644 --- a/arch/x86/kvm/lapic.c +++ b/arch/x86/kvm/lapic.c @@ -2188,9 +2188,6 @@ void kvm_apic_write_nodecode(struct kvm_vcpu *vcpu, u32 offset) { u32 val = 0; - /* hw has done the conditional check and inst decode */ - offset &= 0xff0; - kvm_lapic_reg_read(vcpu->arch.apic, offset, 4, &val); /* TODO: optimize to just emulate side effect w/o one more write */ diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c index b1165bb13a5a..1b135473677b 100644 --- a/arch/x86/kvm/vmx/vmx.c +++ b/arch/x86/kvm/vmx/vmx.c @@ -5302,9 +5302,16 @@ static int handle_apic_eoi_induced(struct kvm_vcpu *vcpu) static int handle_apic_write(struct kvm_vcpu *vcpu) { unsigned long exit_qualification = vmx_get_exit_qual(vcpu); - u32 offset = exit_qualification & 0xfff; - /* APIC-write VM exit is trap-like and thus no need to adjust IP */ + /* + * APIC-write VM-Exit is trap-like, KVM doesn't need to advance RIP and + * hardware has done any necessary aliasing, offset adjustments, etc... + * for the access. I.e. the correct value has already been written to + * the vAPIC page for the correct 16-byte chunk. KVM needs only to + * retrieve the register value and emulate the access. + */ + u32 offset = exit_qualification & 0xff0; + kvm_apic_write_nodecode(vcpu, offset); return 1; }
Move the vAPIC offset adjustments done in the APIC-write trap path from common x86 to VMX in anticipation of using the nodecode path for SVM's AVIC. The adjustment reflects hardware behavior, i.e. it's technically a property of VMX, no common x86. SVM's AVIC behavior is identical, so it's a bit of a moot point, the goal is purely to make it easier to understand why the adjustment is ok. No functional change intended. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/kvm/lapic.c | 3 --- arch/x86/kvm/vmx/vmx.c | 11 +++++++++-- 2 files changed, 9 insertions(+), 5 deletions(-)