| Message ID | 1530598891-21370-6-git-send-email-wanpengli@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
2018-07-03 14:21+0800, Wanpeng Li: > From: Wanpeng Li <wanpengli@tencent.com> > > The NMI delivery mode of ICR is used to deliver an NMI to the processor, > and the vector information is ignored. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Radim Krčmář <rkrcmar@redhat.com> > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > --- > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > @@ -479,7 +479,16 @@ static int __send_ipi_mask(const struct cpumask *mask, int vector) > } > } > > - ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, vector); > + switch (vector) { > + default: > + icr = APIC_DM_FIXED | vector; > + break; > + case NMI_VECTOR: > + icr = APIC_DM_NMI; I think it would be better to say that KVM interprets NMI_VECTOR and sends the interrupt as APIC_DM_NMI. > + break; > + } > + > + ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, icr); >
On Fri, 20 Jul 2018 at 00:31, Radim Krčmář <rkrcmar@redhat.com> wrote: > > 2018-07-03 14:21+0800, Wanpeng Li: > > From: Wanpeng Li <wanpengli@tencent.com> > > > > The NMI delivery mode of ICR is used to deliver an NMI to the processor, > > and the vector information is ignored. > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Radim Krčmář <rkrcmar@redhat.com> > > Cc: Vitaly Kuznetsov <vkuznets@redhat.com> > > Signed-off-by: Wanpeng Li <wanpengli@tencent.com> > > --- > > diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c > > @@ -479,7 +479,16 @@ static int __send_ipi_mask(const struct cpumask *mask, int vector) > > } > > } > > > > - ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, vector); > > + switch (vector) { > > + default: > > + icr = APIC_DM_FIXED | vector; > > + break; > > + case NMI_VECTOR: > > + icr = APIC_DM_NMI; > > I think it would be better to say that KVM interprets NMI_VECTOR and > sends the interrupt as APIC_DM_NMI. Yeah, in addition, SDM 10.6.1 also mentioned that: Delivery mode: 100 (NMI) Delivers an NMI interrupt to the target processor or processors. The vector information is ignored. Regards, Wanpeng Li
On 20/07/2018 05:53, Wanpeng Li wrote: >>> - ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, vector); >>> + switch (vector) { >>> + default: >>> + icr = APIC_DM_FIXED | vector; >>> + break; >>> + case NMI_VECTOR: >>> + icr = APIC_DM_NMI; >> I think it would be better to say that KVM interprets NMI_VECTOR and >> sends the interrupt as APIC_DM_NMI. It's not KVM, this is arch/x86/kernel/kvm.c so the guest side. Paolo > Yeah, in addition, SDM 10.6.1 also mentioned that: > Delivery mode: > 100 (NMI) Delivers an NMI interrupt to the target processor or > processors. The vector information is ignored.
2018-07-20 10:04+0200, Paolo Bonzini: > On 20/07/2018 05:53, Wanpeng Li wrote: > >>> - ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, vector); > >>> + switch (vector) { > >>> + default: > >>> + icr = APIC_DM_FIXED | vector; > >>> + break; > >>> + case NMI_VECTOR: > >>> + icr = APIC_DM_NMI; > >> I think it would be better to say that KVM interprets NMI_VECTOR and > >> sends the interrupt as APIC_DM_NMI. > > It's not KVM, this is arch/x86/kernel/kvm.c so the guest side. Yes, I was preparing to drop the delivery mode bits as they are not needed for NMI_VECTOR: real APIC can't send nor receive vectors 0-15, so we could just say that NMI_VECTOR is sendable through this interface and is delivered as APIC_DM_NMI. I didn't realize that this allows the other delivery modes. They are not useful [1], but we don't really have a better use for the three bits. The only other option so far is extending the cluster index to increase addressability ... I'm starting to reconsider as addressing another few millions VCPUs would probably be even more useless. Wanpeng, sorry for the confusion, feel free to use any version. Document the hypercall layout, though, thanks. --- 1: I think that OSes will use unicast INIT+SIPI as it's easier to call it after preparing structures for the new CPU; SMI is pointless; LowestPriority shouldn't be supported; and Reserved (APIC_DM_REMRD) is reused for KVM unhalt, where unicast is most likely.
On Fri, 20 Jul 2018 at 21:27, Radim Krcmar <rkrcmar@redhat.com> wrote: > > 2018-07-20 10:04+0200, Paolo Bonzini: > > On 20/07/2018 05:53, Wanpeng Li wrote: > > >>> - ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, vector); > > >>> + switch (vector) { > > >>> + default: > > >>> + icr = APIC_DM_FIXED | vector; > > >>> + break; > > >>> + case NMI_VECTOR: > > >>> + icr = APIC_DM_NMI; > > >> I think it would be better to say that KVM interprets NMI_VECTOR and > > >> sends the interrupt as APIC_DM_NMI. > > > > It's not KVM, this is arch/x86/kernel/kvm.c so the guest side. > > Yes, I was preparing to drop the delivery mode bits as they are not > needed for NMI_VECTOR: real APIC can't send nor receive vectors 0-15, so > we could just say that NMI_VECTOR is sendable through this interface and > is delivered as APIC_DM_NMI. > > I didn't realize that this allows the other delivery modes. They are > not useful [1], but we don't really have a better use for the three > bits. The only other option so far is extending the cluster index to > increase addressability ... I'm starting to reconsider as addressing > another few millions VCPUs would probably be even more useless. > > Wanpeng, sorry for the confusion, feel free to use any version. > Document the hypercall layout, though, No problem, you and Paolo always help me a lot, thanks for that! :) Regards, Wanpeng Li > > thanks. > > --- > 1: I think that OSes will use unicast INIT+SIPI as it's easier to call > it after preparing structures for the new CPU; SMI is pointless; > LowestPriority shouldn't be supported; and Reserved (APIC_DM_REMRD) > is reused for KVM unhalt, where unicast is most likely.
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c index 81be75b..b866518 100644 --- a/arch/x86/kernel/kvm.c +++ b/arch/x86/kernel/kvm.c @@ -459,7 +459,7 @@ static void __init sev_map_percpu_data(void) #ifdef CONFIG_X86_64 static int __send_ipi_mask(const struct cpumask *mask, int vector) { - unsigned long flags, ipi_bitmap_low = 0, ipi_bitmap_high = 0; + unsigned long flags, ipi_bitmap_low = 0, ipi_bitmap_high = 0, icr = 0; int cpu, apic_id, ret = 0; if (cpumask_empty(mask)) @@ -479,7 +479,16 @@ static int __send_ipi_mask(const struct cpumask *mask, int vector) } } - ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, vector); + switch (vector) { + default: + icr = APIC_DM_FIXED | vector; + break; + case NMI_VECTOR: + icr = APIC_DM_NMI; + break; + } + + ret = kvm_hypercall3(KVM_HC_SEND_IPI, ipi_bitmap_low, ipi_bitmap_high, icr); irq_restore_exit: local_irq_restore(flags); diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 46657ef..2e4c4d7 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6693,15 +6693,21 @@ static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid) * Return 0 if successfully added and 1 if discarded. */ static int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low, - unsigned long ipi_bitmap_high, int vector) + unsigned long ipi_bitmap_high, unsigned long icr) { int i; struct kvm_apic_map *map; struct kvm_vcpu *vcpu; - struct kvm_lapic_irq irq = { - .delivery_mode = APIC_DM_FIXED, - .vector = vector, - }; + struct kvm_lapic_irq irq = {0}; + + switch (icr & APIC_VECTOR_MASK) { + default: + irq.vector = icr & APIC_VECTOR_MASK; + break; + case NMI_VECTOR: + break; + } + irq.delivery_mode = icr & APIC_MODE_MASK; rcu_read_lock(); map = rcu_dereference(kvm->arch.apic_map);