On 2014/11/19 15:49, Marc Zyngier wrote:
> On Wed, Nov 19 2014 at 02:28:10 AM, Shannon Zhao <zhaoshenglong@xxxxxxxxxx> wrote:
>> On 2014/11/18 18:21, Marc Zyngier wrote:
>>> On 18/11/14 10:12, Shannon Zhao wrote:
>>>> When call kvm_vgic_inject_irq to inject irq, we can known which vcpu
>>>> the IRQ for by 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 | 32 ++++++++++++++++++++++++--------
>>>> 1 files changed, 24 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
>>>> index 82db9d6..9dad67a 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);
>>>> 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,21 @@ static void vgic_kick_vcpus(struct kvm *kvm)
>>>> }
>>>> }
>>>>
>>>> +static void vgic_kick_vcpu(struct kvm *kvm, int cpuid)
>>>> +{
>>>> + struct kvm_vcpu *vcpu;
>>>> +
>>>> + /*
>>>> + * We've injected an interrupt, kick the specified vcpu
>>>> + */
>>>> + if (cpuid < atomic_read(&kvm->online_vcpus)) {
>>>> + vcpu = kvm_get_vcpu(kvm, cpuid);
>>>> + if (vcpu != NULL) {
>>>> + kvm_vcpu_kick(vcpu);
>>>> + }
>>>> + }
>>>
>>> Why do you need all these checks? Why would cpuid be wrong at any time
>>> here, given that we've extracted it from supposedly valid information?
>>> Can you give an example of such a situation?
>>>
>>
>> Sorry, thanks for your indication.
>>
>> I considered the situation that guest offline one vcpu which a SPI bind to.
>> I didn't think clearly before and I have tested at this situation.
>> It's an overprotection.
>>
>> So remove the "vgic_kick_vcpu" and just call "kvm_vcpu_kick" in "kvm_vgic_inject_irq" ?
>>
>> int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num,
>> bool level)
>> {
>> if (likely(vgic_initialized(kvm)) &&
>> vgic_update_irq_pending(kvm, &cpuid, irq_num, level))
>> /*
>> * We've injected an interrupt, kick the specified vcpu
>> */
>> kvm_vcpu_kick(kvm_get_vcpu(kvm, cpuid));
>>
>> return 0;
>> }
>
> That would be a lot better. You may also want to rethink the way you use
> cpuid in vgic_update_irq_pending() so that you only update the returned
> value on the positive path (that should make the patch a lot smaller),
>
That's good:-)
> Thanks,
>
> M.
>
--
Shannon
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
