| Message ID | 1532104133-15777-1-git-send-email-wanpengli@tencent.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20/07/2018 18:28, Wanpeng Li wrote: > +a0: ipi_bitmap low 64 bits > +a1: ipi_bitmap high 64 bits > +a2: the lowest APIC ID in bitmap > +a3: APIC ICR > + > +The hypercall lets a guest send multicast IPIs at most can handle > +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall > +on 32-bit machines. > + > +Returns 0 if successfully delivery the IPIs and 1 if discarded. This description does not mention what happens in 32-bit mode. > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index 2b812b3..016c7e2 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6691,6 +6691,41 @@ static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid) > kvm_irq_delivery_to_apic(kvm, NULL, &lapic_irq, NULL); > } > > +/* > + * 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 min, int vector, int op_64_bit) > +{ > + int i; > + struct kvm_apic_map *map; > + struct kvm_vcpu *vcpu; > + struct kvm_lapic_irq irq = { > + .delivery_mode = APIC_DM_FIXED, > + .vector = vector, > + }; > + > + rcu_read_lock(); > + map = rcu_dereference(kvm->arch.apic_map); > + > + for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { > + vcpu = map->phys_map[min + i]->vcpu; > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > + return 1; > + } > + > + if (op_64_bit) { > + for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { > + vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > + return 1; > + } > + } The second loop processes the second argument, and it should always run, even in 32-bit mode. However, the phys_map index should be min + i + 32 in 32-bit mode and min + i + 64 in 64-bit mode. (Using BITS_PER_LONG in the for_each_set_bit length is not a bug instead; you could write it explicitly as 32 in 32-bit mode, and 64 in 64-bit mode, but I think it's a little bit more efficient if it's constant). Paolo > +
On Mon, 23 Jul 2018 at 13:52, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 20/07/2018 18:28, Wanpeng Li wrote: > > +a0: ipi_bitmap low 64 bits > > +a1: ipi_bitmap high 64 bits > > +a2: the lowest APIC ID in bitmap > > +a3: APIC ICR > > + > > +The hypercall lets a guest send multicast IPIs at most can handle > > +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall > > +on 32-bit machines. > > + > > +Returns 0 if successfully delivery the IPIs and 1 if discarded. > > This description does not mention what happens in 32-bit mode. Will do in next version. > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 2b812b3..016c7e2 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -6691,6 +6691,41 @@ static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid) > > kvm_irq_delivery_to_apic(kvm, NULL, &lapic_irq, NULL); > > } > > > > +/* > > + * 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 min, int vector, int op_64_bit) > > +{ > > + int i; > > + struct kvm_apic_map *map; > > + struct kvm_vcpu *vcpu; > > + struct kvm_lapic_irq irq = { > > + .delivery_mode = APIC_DM_FIXED, > > + .vector = vector, > > + }; > > + > > + rcu_read_lock(); > > + map = rcu_dereference(kvm->arch.apic_map); > > + > > + for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { > > + vcpu = map->phys_map[min + i]->vcpu; > > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > > + return 1; > > + } > > + > > + if (op_64_bit) { > > + for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { > > + vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; > > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > > + return 1; > > + } > > + } > > The second loop processes the second argument, and it should always run, > even in 32-bit mode. However, the phys_map index should be min + i + 32 > in 32-bit mode and min + i + 64 in 64-bit mode. (Using BITS_PER_LONG in > the for_each_set_bit length is not a bug instead; you could write it > explicitly as 32 in 32-bit mode, and 64 in 64-bit mode, but I think it's > a little bit more efficient if it's constant). Good catch, below should fix it. diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index c9dbc2c..c118040 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6701,6 +6701,7 @@ static int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low, struct kvm_apic_map *map; struct kvm_vcpu *vcpu; struct kvm_lapic_irq irq = {0}; + int cluster_size = op_64_bit ? 64 : 32; switch (icr & APIC_VECTOR_MASK) { default: @@ -6714,18 +6715,16 @@ static int kvm_pv_send_ipi(struct kvm *kvm, unsigned long ipi_bitmap_low, rcu_read_lock(); map = rcu_dereference(kvm->arch.apic_map); - for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { + for_each_set_bit(i, &ipi_bitmap_low, cluster_size) { vcpu = map->phys_map[min + i]->vcpu; if (!kvm_apic_set_irq(vcpu, &irq, NULL)) return 1; } - if (op_64_bit) { - for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { - vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; - if (!kvm_apic_set_irq(vcpu, &irq, NULL)) - return 1; - } + for_each_set_bit(i, &ipi_bitmap_high, cluster_size) { + vcpu = map->phys_map[min + i + cluster_size]->vcpu; + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) + return 1; } rcu_read_unlock();
On 23/07/2018 08:00, Wanpeng Li wrote: > On Mon, 23 Jul 2018 at 13:52, Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> On 20/07/2018 18:28, Wanpeng Li wrote: >>> +a0: ipi_bitmap low 64 bits >>> +a1: ipi_bitmap high 64 bits >>> +a2: the lowest APIC ID in bitmap >>> +a3: APIC ICR >>> + >>> +The hypercall lets a guest send multicast IPIs at most can handle >>> +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall >>> +on 32-bit machines. >>> + >>> +Returns 0 if successfully delivery the IPIs and 1 if discarded. >> >> This description does not mention what happens in 32-bit mode. > > Will do in next version. > >> >>> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >>> index 2b812b3..016c7e2 100644 >>> --- a/arch/x86/kvm/x86.c >>> +++ b/arch/x86/kvm/x86.c >>> @@ -6691,6 +6691,41 @@ static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid) >>> kvm_irq_delivery_to_apic(kvm, NULL, &lapic_irq, NULL); >>> } >>> >>> +/* >>> + * 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 min, int vector, int op_64_bit) >>> +{ >>> + int i; >>> + struct kvm_apic_map *map; >>> + struct kvm_vcpu *vcpu; >>> + struct kvm_lapic_irq irq = { >>> + .delivery_mode = APIC_DM_FIXED, >>> + .vector = vector, >>> + }; >>> + >>> + rcu_read_lock(); >>> + map = rcu_dereference(kvm->arch.apic_map); >>> + >>> + for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { >>> + vcpu = map->phys_map[min + i]->vcpu; >>> + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) >>> + return 1; >>> + } >>> + >>> + if (op_64_bit) { >>> + for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { >>> + vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; >>> + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) >>> + return 1; >>> + } >>> + } >> >> The second loop processes the second argument, and it should always run, >> even in 32-bit mode. However, the phys_map index should be min + i + 32 >> in 32-bit mode and min + i + 64 in 64-bit mode. (Using BITS_PER_LONG in >> the for_each_set_bit length is not a bug instead; you could write it >> explicitly as 32 in 32-bit mode, and 64 in 64-bit mode, but I think it's >> a little bit more efficient if it's constant). > > Good catch, below should fix it. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c9dbc2c..c118040 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6701,6 +6701,7 @@ static int kvm_pv_send_ipi(struct kvm *kvm, > unsigned long ipi_bitmap_low, > struct kvm_apic_map *map; > struct kvm_vcpu *vcpu; > struct kvm_lapic_irq irq = {0}; > + int cluster_size = op_64_bit ? 64 : 32; > > switch (icr & APIC_VECTOR_MASK) { > default: > @@ -6714,18 +6715,16 @@ static int kvm_pv_send_ipi(struct kvm *kvm, > unsigned long ipi_bitmap_low, > rcu_read_lock(); > map = rcu_dereference(kvm->arch.apic_map); > > - for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { > + for_each_set_bit(i, &ipi_bitmap_low, cluster_size) { > vcpu = map->phys_map[min + i]->vcpu; > if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > return 1; > } > > - if (op_64_bit) { > - for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { > - vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; > - if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > - return 1; > - } > + for_each_set_bit(i, &ipi_bitmap_high, cluster_size) { > + vcpu = map->phys_map[min + i + cluster_size]->vcpu; > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > + return 1; > } > > rcu_read_unlock(); > Yes, that should work. Thanks! Paolo
On Mon, 23 Jul 2018 at 14:00, Wanpeng Li <kernellwp@gmail.com> wrote: > > On Mon, 23 Jul 2018 at 13:52, Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > On 20/07/2018 18:28, Wanpeng Li wrote: > > > +a0: ipi_bitmap low 64 bits > > > +a1: ipi_bitmap high 64 bits > > > +a2: the lowest APIC ID in bitmap > > > +a3: APIC ICR > > > + > > > +The hypercall lets a guest send multicast IPIs at most can handle > > > +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall > > > +on 32-bit machines. > > > + > > > +Returns 0 if successfully delivery the IPIs and 1 if discarded. > > > > This description does not mention what happens in 32-bit mode. Sorry, I think I mentioned "64 vCPUs per hypercall on 32-bit machines" above. Regards, Wanpeng Li > > Will do in next version. > > > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > > index 2b812b3..016c7e2 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -6691,6 +6691,41 @@ static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid) > > > kvm_irq_delivery_to_apic(kvm, NULL, &lapic_irq, NULL); > > > } > > > > > > +/* > > > + * 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 min, int vector, int op_64_bit) > > > +{ > > > + int i; > > > + struct kvm_apic_map *map; > > > + struct kvm_vcpu *vcpu; > > > + struct kvm_lapic_irq irq = { > > > + .delivery_mode = APIC_DM_FIXED, > > > + .vector = vector, > > > + }; > > > + > > > + rcu_read_lock(); > > > + map = rcu_dereference(kvm->arch.apic_map); > > > + > > > + for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { > > > + vcpu = map->phys_map[min + i]->vcpu; > > > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > > > + return 1; > > > + } > > > + > > > + if (op_64_bit) { > > > + for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { > > > + vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; > > > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > > > + return 1; > > > + } > > > + } > > > > The second loop processes the second argument, and it should always run, > > even in 32-bit mode. However, the phys_map index should be min + i + 32 > > in 32-bit mode and min + i + 64 in 64-bit mode. (Using BITS_PER_LONG in > > the for_each_set_bit length is not a bug instead; you could write it > > explicitly as 32 in 32-bit mode, and 64 in 64-bit mode, but I think it's > > a little bit more efficient if it's constant). > > Good catch, below should fix it. > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > index c9dbc2c..c118040 100644 > --- a/arch/x86/kvm/x86.c > +++ b/arch/x86/kvm/x86.c > @@ -6701,6 +6701,7 @@ static int kvm_pv_send_ipi(struct kvm *kvm, > unsigned long ipi_bitmap_low, > struct kvm_apic_map *map; > struct kvm_vcpu *vcpu; > struct kvm_lapic_irq irq = {0}; > + int cluster_size = op_64_bit ? 64 : 32; > > switch (icr & APIC_VECTOR_MASK) { > default: > @@ -6714,18 +6715,16 @@ static int kvm_pv_send_ipi(struct kvm *kvm, > unsigned long ipi_bitmap_low, > rcu_read_lock(); > map = rcu_dereference(kvm->arch.apic_map); > > - for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { > + for_each_set_bit(i, &ipi_bitmap_low, cluster_size) { > vcpu = map->phys_map[min + i]->vcpu; > if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > return 1; > } > > - if (op_64_bit) { > - for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { > - vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; > - if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > - return 1; > - } > + for_each_set_bit(i, &ipi_bitmap_high, cluster_size) { > + vcpu = map->phys_map[min + i + cluster_size]->vcpu; > + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) > + return 1; > } > > rcu_read_unlock();
On 23/07/2018 08:10, Wanpeng Li wrote: >>> On 20/07/2018 18:28, Wanpeng Li wrote: >>>> +a0: ipi_bitmap low 64 bits >>>> +a1: ipi_bitmap high 64 bits >>>> +a2: the lowest APIC ID in bitmap >>>> +a3: APIC ICR >>>> + >>>> +The hypercall lets a guest send multicast IPIs at most can handle >>>> +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall >>>> +on 32-bit machines. >>>> + >>>> +Returns 0 if successfully delivery the IPIs and 1 if discarded. >>> This description does not mention what happens in 32-bit mode. > Sorry, I think I mentioned "64 vCPUs per hypercall on 32-bit machines" above. Yes, but the description of a0 and a1 is not accurate. Something like a0: lower part of the bitmap of destination APIC IDs a1: higher part of the bitmap of destination APIC IDs .... The hypercall lets a guest send multicast IPIs, with at most 128 128 destinations per hypercall in 64-bit mode and 64 vCPUs per hypercall in 32-bit mode. The destinations are represented by a bitmap contained in the first two arguments (a0 and a1). Bit 0 of a0 corresponds to the APIC ID in the third argument (a2), bit 1 corresponds to the APIC ID a2+1, and so on. Thanks, Paolo
On Mon, 23 Jul 2018 at 14:19, Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 23/07/2018 08:10, Wanpeng Li wrote: > >>> On 20/07/2018 18:28, Wanpeng Li wrote: > >>>> +a0: ipi_bitmap low 64 bits > >>>> +a1: ipi_bitmap high 64 bits > >>>> +a2: the lowest APIC ID in bitmap > >>>> +a3: APIC ICR > >>>> + > >>>> +The hypercall lets a guest send multicast IPIs at most can handle > >>>> +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall > >>>> +on 32-bit machines. > >>>> + > >>>> +Returns 0 if successfully delivery the IPIs and 1 if discarded. > >>> This description does not mention what happens in 32-bit mode. > > Sorry, I think I mentioned "64 vCPUs per hypercall on 32-bit machines" above. > > Yes, but the description of a0 and a1 is not accurate. Something like > > a0: lower part of the bitmap of destination APIC IDs > a1: higher part of the bitmap of destination APIC IDs > .... > > The hypercall lets a guest send multicast IPIs, with at most 128 > 128 destinations per hypercall in 64-bit mode and 64 vCPUs per > hypercall in 32-bit mode. The destinations are represented by a bitmap > contained in the first two arguments (a0 and a1). Bit 0 of a0 > corresponds to the APIC ID in the third argument (a2), bit 1 corresponds > to the APIC ID a2+1, and so on. Thanks to the great description, I will fold this to the next version. Regards, Wanpeng Li
diff --git a/Documentation/virtual/kvm/hypercalls.txt b/Documentation/virtual/kvm/hypercalls.txt index a890529..912b877 100644 --- a/Documentation/virtual/kvm/hypercalls.txt +++ b/Documentation/virtual/kvm/hypercalls.txt @@ -121,3 +121,20 @@ compute the CLOCK_REALTIME for its clock, at the same instant. Returns KVM_EOPNOTSUPP if the host does not use TSC clocksource, or if clock type is different than KVM_CLOCK_PAIRING_WALLCLOCK. + +6. KVM_HC_SEND_IPI +------------------------ +Architecture: x86 +Status: active +Purpose: Hypercall used to send IPIs. + +a0: ipi_bitmap low 64 bits +a1: ipi_bitmap high 64 bits +a2: the lowest APIC ID in bitmap +a3: APIC ICR + +The hypercall lets a guest send multicast IPIs at most can handle +128 vCPUs per hypercall on 64-bit machines and 64 vCPUs per hypercall +on 32-bit machines. + +Returns 0 if successfully delivery the IPIs and 1 if discarded. diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index 2b812b3..016c7e2 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -6691,6 +6691,41 @@ static void kvm_pv_kick_cpu_op(struct kvm *kvm, unsigned long flags, int apicid) kvm_irq_delivery_to_apic(kvm, NULL, &lapic_irq, NULL); } +/* + * 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 min, int vector, int op_64_bit) +{ + int i; + struct kvm_apic_map *map; + struct kvm_vcpu *vcpu; + struct kvm_lapic_irq irq = { + .delivery_mode = APIC_DM_FIXED, + .vector = vector, + }; + + rcu_read_lock(); + map = rcu_dereference(kvm->arch.apic_map); + + for_each_set_bit(i, &ipi_bitmap_low, BITS_PER_LONG) { + vcpu = map->phys_map[min + i]->vcpu; + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) + return 1; + } + + if (op_64_bit) { + for_each_set_bit(i, &ipi_bitmap_high, BITS_PER_LONG) { + vcpu = map->phys_map[min + i + BITS_PER_LONG]->vcpu; + if (!kvm_apic_set_irq(vcpu, &irq, NULL)) + return 1; + } + } + + rcu_read_unlock(); + return 0; +} + void kvm_vcpu_deactivate_apicv(struct kvm_vcpu *vcpu) { vcpu->arch.apicv_active = false; @@ -6739,6 +6774,9 @@ int kvm_emulate_hypercall(struct kvm_vcpu *vcpu) case KVM_HC_CLOCK_PAIRING: ret = kvm_pv_clock_pairing(vcpu, a0, a1); break; + case KVM_HC_SEND_IPI: + ret = kvm_pv_send_ipi(vcpu->kvm, a0, a1, a2, a3, op_64_bit); + break; #endif default: ret = -KVM_ENOSYS;