Messages in this thread | | | Subject | Re: [PATCH] svm: Fix AVIC incomplete IPI emulation | From | Oren Twaig <> | Date | Fri, 8 Mar 2019 23:56:02 +0200 |
| |
Hi,
Any help appreciated..
Thanks,
Oren Twaig
On 3/4/2019 8:15 PM, Oren Twaig wrote: > Hello Suravee, > > According to AMD's SDM, the target-not-running incomplete > ipi exit is only received if any of the destination cpus had the > > not-running bit set in the avic backing page. However, not > > before the CPU _already_ set the relevant IRR bit in all these cpus. > > > In this change, the patch forces KVM to send another interrupt > > to the vcpu whether SVM already did that or not. Which means > > the vcpu/s, under some conditions, can get an EXTRA interrupt > > it never intended to get. > > > Example: > 1. vcpu B: Is in "not-running" state. > 2. vcpu A: Writes to the ICR to send vector 80 to vcpu B > 3. vcpu A: SVM updates vcpu B IRR with bit 80 > 4. vcpu A: SVM exits on incomplete IPI target-not-running exit. > 5. vcpu A: Now stops executing any code @ hypervisor level. > 6. vcpu B: Due to another interrupt (like lapic timer) > resumes running the guest. While handling interrupts, > it also handles interrupt vector 80 (as it's in his IRR) > 7. vcpu A: resumes executing the below code and sends > an _additional_interrupt to vcpu B. > > Overall, vcpu B got two interrupts. The second is unwanted and > not documented in the system architecture. > > Can you please elaborate more to why the implementation > > below conflict with the specifications (which was the code > > before this commit) ? > > > Thanks, > Oren Twaig > > > > From "Suthikulpanit, Suravee" <> > > Subject [PATCH] svm: Fix AVIC incomplete IPI emulation > > Date Tue, 22 Jan 2019 10:25:13 +0000 > > share > > From: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > > > In case of incomplete IPI with invalid interrupt type, the current > > SVM driver does not properly emulate the IPI, and fails to boot > > FreeBSD guests with multiple vcpus when enabling AVIC. > > > > Fix this by update APIC ICR high/low registers, which also > > emulate sending the IPI. > > > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > > --- > > arch/x86/kvm/svm.c | 19 ++++--------------- > > 1 file changed, 4 insertions(+), 15 deletions(-) > > > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > > index 2aff835a65ed..8a0c9a1f6ac8 100644 > > --- a/arch/x86/kvm/svm.c > > +++ b/arch/x86/kvm/svm.c > > @@ -4504,25 +4504,14 @@ static int > avic_incomplete_ipi_interception(struct vcpu_svm *svm) > > kvm_lapic_reg_write(apic, APIC_ICR, icrl); > > break; > > case AVIC_IPI_FAILURE_TARGET_NOT_RUNNING: { > > - int i; > > - struct kvm_vcpu *vcpu; > > - struct kvm *kvm = svm->vcpu.kvm; > > struct kvm_lapic *apic = svm->vcpu.arch.apic; > > > /* > > - * At this point, we expect that the AVIC HW has already > > - * set the appropriate IRR bits on the valid target > > - * vcpus. So, we just need to kick the appropriate vcpu. > > + * Update ICR high and low, then emulate sending IPI, > > + * which is handled when writing APIC_ICR. > > */ > > - kvm_for_each_vcpu(i, vcpu, kvm) { > > - bool m = kvm_apic_match_dest(vcpu, apic, > > - icrl & KVM_APIC_SHORT_MASK, > > - GET_APIC_DEST_FIELD(icrh), > > - icrl & KVM_APIC_DEST_MASK); > > - > > - if (m && !avic_vcpu_is_running(vcpu)) > > - kvm_vcpu_wake_up(vcpu); > > - } > > + kvm_lapic_reg_write(apic, APIC_ICR2, icrh); > > + kvm_lapic_reg_write(apic, APIC_ICR, icrl); > > break; > > } > > case AVIC_IPI_FAILURE_INVALID_TARGET: > > -- > > 2.17.1 >
| |