Re: [Qemu-arm] [Qemu-devel] [RFC 2/3] intc/arm_gic: Support PPI injection for more than 256 vpus

[Auger Eric]

Hi Zenghui,

On 8/29/19 4:53 AM, Zenghui Yu wrote:
> Hi Eric,
> 
> On 2019/8/28 0:05, Eric Auger wrote:
>> Host kernels that expose the KVM_CAP_ARM_IRQ_LINE_LAYOUT_2 capability
>> allow injection of PPIs along with vcpu ids larger than 255. Let's
>> encode the vpcu id on 12 bits according to the upgraded KVM_IRQ_LINE
>> ABI when needed.
>>
>> Without that patch qemu exits with "kvm_set_irq: Invalid argument"
>> message.
>>
>> Signed-off-by: Eric Auger <address@hidden>
>> Reported-by: Zenghui Yu <address@hidden>
>> ---
>>   hw/intc/arm_gic_kvm.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/hw/intc/arm_gic_kvm.c b/hw/intc/arm_gic_kvm.c
>> index b56fda144f..889293e97f 100644
>> --- a/hw/intc/arm_gic_kvm.c
>> +++ b/hw/intc/arm_gic_kvm.c
>> @@ -56,6 +56,7 @@ void kvm_arm_gic_set_irq(uint32_t num_irq, int irq,
>> int level)
>>        * CPU number and interrupt number.
>>        */
>>       int kvm_irq, irqtype, cpu;
>> +    int cpu_idx1 = 0, cpu_idx2 = 0;
>>         if (irq < (num_irq - GIC_INTERNAL)) {
>>           /* External interrupt. The kernel numbers these like the GIC
>> @@ -63,17 +64,20 @@ void kvm_arm_gic_set_irq(uint32_t num_irq, int
>> irq, int level)
>>            * internal ones.
>>            */
>>           irqtype = KVM_ARM_IRQ_TYPE_SPI;
>> -        cpu = 0;
>>           irq += GIC_INTERNAL;
>>       } else {
>>           /* Internal interrupt: decode into (cpu, interrupt id) */
>>           irqtype = KVM_ARM_IRQ_TYPE_PPI;
>>           irq -= (num_irq - GIC_INTERNAL);
>>           cpu = irq / GIC_INTERNAL;
>> +        cpu_idx2 = cpu / 256;
>> +        cpu_idx1 = cpu % 256;
>>           irq %= GIC_INTERNAL;
>>       }
>> -    kvm_irq = (irqtype << KVM_ARM_IRQ_TYPE_SHIFT)
>> -        | (cpu << KVM_ARM_IRQ_VCPU_SHIFT) | irq;
>> +    kvm_irq = (irqtype << KVM_ARM_IRQ_TYPE_SHIFT) |
>> +              (cpu_idx1 << KVM_ARM_IRQ_VCPU_SHIFT) |
>> +              ((cpu_idx2 & KVM_ARM_IRQ_VCPU2_MASK) <<
>> KVM_ARM_IRQ_VCPU2_SHIFT) |
>> +              irq;
>>         kvm_set_irq(kvm_state, kvm_irq, !!level);
>>   }
>>
> 
> For confirmation, should we also adjust the vcpu_index in
> arm_cpu_kvm_set_irq(), just like above?

I am not familiar with this path. in arm_cpu_initfn(), there is a
comment saying "VIRQ and VFIQ are unused with KVM but we add them to
maintain the same interface as non-KVM CPUs." So I don't know when that
code gets executed.

But maybe it would be more cautious to implement your suggestion here as
well.

Maybe Peter can provide more info here?

Thanks

Eric


> 
> 
> Thanks,
> zenghui
> 
>