On 17/11/14 12:26, Shannon Zhao wrote:
> Hi Marc,
>
> On 2014/11/14 20:11, Marc Zyngier wrote:
>> Shannon,
>>
>> On 14/11/14 11:09, Shannon Zhao wrote:
>>> When call kvm_vgic_inject_irq to inject irq, we've already known
>>> which vcpu the IRQ for form the cpuid. So we should just kick this
>>> one vcpu to avoid iterate through all.
>
>>
>> Have you actually tested this? Hint: cpuid is *not* what you think, and
>> I strongly suggest you play with interrupt affinity in the guest before
>> posting such changes.
>>
>
> So sorry that I didn't consider all cases.
>
> There are three places calling the function "kvm_vgic_inject_irq" :
> 1) kvm_timer_inject_irq
> The "cpuid" is the real cpuid.
> kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
> timer->irq->irq,
> timer->irq->level);
>
> 2) kvm_vm_ioctl_irq_line as irq_type is KVM_ARM_IRQ_TYPE_PPI
> The "cpuid" is also the real cpuid.
> kvm_vgic_inject_irq(kvm, vcpu->vcpu_id, irq_num, level);
>
> 3) kvm_vm_ioctl_irq_line as irq_type is KVM_ARM_IRQ_TYPE_SPI
> This case is that I missed I think.
> The "cpuid" is always zero and should get the cpuid through dist->irq_spi_cpu[irq_num - VGIC_NR_PRIVATE_IRQS].
> kvm_vgic_inject_irq(kvm, 0, irq_num, level);
>
> So before kicking the vcpu we can get the cpuid through dist->irq_spi_cpu[irq_num - VGIC_NR_PRIVATE_IRQS],
> then kick the specific vcpu, right?
>
> If right, I make another patch and test on Fastmodel and ARM64 hardware platform as you suggested.
>
> When call kvm_vgic_inject_irq to inject irq, we can known which vcpu
> the IRQ for through the irq_num and the cpuid. So we should just kick
> this vcpu to avoid iterating through all.
>
> Signed-off-by: Shannon Zhao <zhaoshenglong@xxxxxxxxxx>
> ---
> virt/kvm/arm/vgic.c | 22 +++++++++++++++++++++-
> 1 files changed, 21 insertions(+), 1 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
> index 82db9d6..860c3ff 100644
> --- a/virt/kvm/arm/vgic.c
> +++ b/virt/kvm/arm/vgic.c
> @@ -95,6 +95,7 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu);
> static void vgic_retire_lr(int lr_nr, int irq, struct kvm_vcpu *vcpu);
> static void vgic_update_state(struct kvm *kvm);
> static void vgic_kick_vcpus(struct kvm *kvm);
> +static void vgic_kick_vcpu(struct kvm *kvm, int cpuid, unsigned int irq_num);
> static u8 *vgic_get_sgi_sources(struct vgic_dist *dist, int vcpu_id, int sgi);
> static void vgic_dispatch_sgi(struct kvm_vcpu *vcpu, u32 reg);
> static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
> @@ -1589,6 +1590,25 @@ static void vgic_kick_vcpus(struct kvm *kvm)
> }
> }
>
> +static void vgic_kick_vcpu(struct kvm *kvm, int cpuid, unsigned int irq_num)
> +{
> + struct vgic_dist *dist = &kvm->arch.vgic;
> + struct kvm_vcpu *vcpu;
> +
> + /*
> + * We've injected an interrupt, kick the specified vcpu
> + */
> + if (irq_num >= VGIC_NR_PRIVATE_IRQS)
> + cpuid = dist->irq_spi_cpu[irq_num - VGIC_NR_PRIVATE_IRQS];
> +
> + if (cpuid < atomic_read(&kvm->online_vcpus)) {
> + vcpu = kvm_get_vcpu(kvm, cpuid);
> + if (vcpu != NULL) {
> + kvm_vcpu_kick(vcpu);
> + }
> + }
> +}
> +
> static int vgic_validate_injection(struct kvm_vcpu *vcpu, int irq, int level)
> {
> int edge_triggered = vgic_irq_is_edge(vcpu, irq);
> @@ -1694,7 +1714,7 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
> {
> if (likely(vgic_initialized(kvm)) &&
> vgic_update_irq_pending(kvm, cpuid, irq_num, level))
> - vgic_kick_vcpus(kvm);
> + vgic_kick_vcpu(kvm, cpuid, irq_num);
>
> return 0;
> }
>
Instead of working around things and recomputing the vcpu that needs to
be kicked, why don't you just change vgic_update_irq_pending to *return*
the vcpu number that needs to be kicked?
M.
--
Jazz is not dead. It just smells funny...
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
