| Message ID | 20190903142954.3429-3-graf@amazon.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: Only use posted interrupts for Fixes/LowPrio MSIs | expand |
> On 3 Sep 2019, at 17:29, Alexander Graf <graf@amazon.com> wrote: > > We can easily route hardware interrupts directly into VM context when > they target the "Fixed" or "LowPriority" delivery modes. > > However, on modes such as "SMI" or "Init", we need to go via KVM code > to actually put the vCPU into a different mode of operation, so we can > not post the interrupt > > Add code in the SVM PI logic to explicitly refuse to establish posted > mappings for advanced IRQ deliver modes. > > This fixes a bug I have with code which configures real hardware to > inject virtual SMIs into my guest. > > Signed-off-by: Alexander Graf <graf@amazon.com> Nit: I prefer to squash both commits into one that change both VMX & SVM. As it’s exactly the same change. > --- > arch/x86/kvm/svm.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c > index 1f220a85514f..9a6ea78c3239 100644 > --- a/arch/x86/kvm/svm.c > +++ b/arch/x86/kvm/svm.c > @@ -5266,6 +5266,21 @@ get_pi_vcpu_info(struct kvm *kvm, struct kvm_kernel_irq_routing_entry *e, > return -1; > } > > + switch (irq.delivery_mode) { > + case dest_Fixed: > + case dest_LowestPrio: > + break; > + default: > + /* > + * For non-trivial interrupt events, we need to go > + * through the full KVM IRQ code, so refuse to take > + * any direct PI assignments here. > + */ > + pr_debug("SVM: %s: use legacy intr remap mode for irq %u\n", > + __func__, irq.vector); > + return -1; > + } > + Prefer changing printed string to something different than the !kvm_intr_is_single_vcpu() case. To assist debugging. Having said that, Reviewed-by: Liran Alon <liran.alon@oracle.com> -Liran > pr_debug("SVM: %s: use GA mode for irq %u\n", __func__, > irq.vector); > *svm = to_svm(vcpu); > @@ -5314,6 +5329,7 @@ static int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq, > * 1. When cannot target interrupt to a specific vcpu. > * 2. Unsetting posted interrupt. > * 3. APIC virtialization is disabled for the vcpu. > + * 4. IRQ has extended delivery mode (SMI, INIT, etc) > */ > if (!get_pi_vcpu_info(kvm, e, &vcpu_info, &svm) && set && > kvm_vcpu_apicv_active(&svm->vcpu)) { > -- > 2.17.1 > > > > > Amazon Development Center Germany GmbH > Krausenstr. 38 > 10117 Berlin > Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich > Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B > Sitz: Berlin > Ust-ID: DE 289 237 879 > > >
On 04.09.19 01:20, Liran Alon wrote: > > >> On 3 Sep 2019, at 17:29, Alexander Graf <graf@amazon.com> wrote: >> >> We can easily route hardware interrupts directly into VM context when >> they target the "Fixed" or "LowPriority" delivery modes. >> >> However, on modes such as "SMI" or "Init", we need to go via KVM code >> to actually put the vCPU into a different mode of operation, so we can >> not post the interrupt >> >> Add code in the SVM PI logic to explicitly refuse to establish posted >> mappings for advanced IRQ deliver modes. >> >> This fixes a bug I have with code which configures real hardware to >> inject virtual SMIs into my guest. >> >> Signed-off-by: Alexander Graf <graf@amazon.com> > > Nit: I prefer to squash both commits into one that change both VMX & SVM. > As it’s exactly the same change. It's the same change (hence the same patch set), but they touch different files and so for bisectability it's still convenient to have them as different commits. I'd really prefer to have them stay separate. Thanks a lot for the review! :) Alex Amazon Development Center Germany GmbH Krausenstr. 38 10117 Berlin Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B Sitz: Berlin Ust-ID: DE 289 237 879
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 1f220a85514f..9a6ea78c3239 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -5266,6 +5266,21 @@ get_pi_vcpu_info(struct kvm *kvm, struct kvm_kernel_irq_routing_entry *e, return -1; } + switch (irq.delivery_mode) { + case dest_Fixed: + case dest_LowestPrio: + break; + default: + /* + * For non-trivial interrupt events, we need to go + * through the full KVM IRQ code, so refuse to take + * any direct PI assignments here. + */ + pr_debug("SVM: %s: use legacy intr remap mode for irq %u\n", + __func__, irq.vector); + return -1; + } + pr_debug("SVM: %s: use GA mode for irq %u\n", __func__, irq.vector); *svm = to_svm(vcpu); @@ -5314,6 +5329,7 @@ static int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq, * 1. When cannot target interrupt to a specific vcpu. * 2. Unsetting posted interrupt. * 3. APIC virtialization is disabled for the vcpu. + * 4. IRQ has extended delivery mode (SMI, INIT, etc) */ if (!get_pi_vcpu_info(kvm, e, &vcpu_info, &svm) && set && kvm_vcpu_apicv_active(&svm->vcpu)) {
We can easily route hardware interrupts directly into VM context when they target the "Fixed" or "LowPriority" delivery modes. However, on modes such as "SMI" or "Init", we need to go via KVM code to actually put the vCPU into a different mode of operation, so we can not post the interrupt Add code in the SVM PI logic to explicitly refuse to establish posted mappings for advanced IRQ deliver modes. This fixes a bug I have with code which configures real hardware to inject virtual SMIs into my guest. Signed-off-by: Alexander Graf <graf@amazon.com> --- arch/x86/kvm/svm.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)