Re: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi

[Eric Auger]

On 07/06/2015 05:52 PM, Andre Przywara wrote:
> Salut Eric,
> 
> ....
> 
>>>  ITS code in qemu just does:
>>>
>>> ---cut ---
>>>     msi_supported = true;
>>>     kvm_msi_flags = KVM_MSI_VALID_DEVID;
>>>     kvm_msi_via_irqfd_allowed = kvm_has_gsi_routing();
>>>     kvm_gsi_routing_allowed = kvm_msi_via_irqfd_allowed;
>>> --- cut ---
>>>
>>>  I set KVM_MSI_VALID_DEVID unconditionally here just because it will never be checked if
>>> kvm_msi_via_irqfd_allowed is false, it's just qemu specifics. The more canonical form would perhaps
>>> be:
>>> --- cut ---
>>> if (kvm_has_gsi_routing()) {
>>>     kvm_msi_flags = KVM_MSI_VALID_DEVID;
>> Personally I prefer a capability rather than hardcoding a global
>> variable value in the qemu interrupt controller code. All the more so
>> typically there is KVM GSI routing cap that could/should? be queried
>> instead of hardcoding the value I think.
>>
>> So not sure whether we eventually concluded;-)
>> - introduce a KVM_CAP_MSI_DEVID capability? All OK except Pavel not
>> convinced?
> 
> OK for me.
> 
>> - userspaces puts the devid in struct kvm_irq_routing_msi pad field:
>> consensus (we do not intrduce a new kvm_irq_routing_ext_msi)
> 
> OK for me.
> 
>> - userspace tells it conveyed a devid by setting
>> A) the kvm_irq_routing_entry's field?
> 
> You mean kvm_irq_routing_entry's "flags" here?
yes!!
> 
>> B) the kvm_irq_routing_entry's type
> 
> So personally I don't like it so much to use the generic flags field to
> specify the meaning within one particular type only. Using a new type
> instead seems to be more consistent, reusing an existing struct for that
> sounds even better.
> As written before (and coded in my branch) we can collapse that into the
> existing MSI type while translating that into the kernel internal
> routing structure to make the kernel code changes minimal.
> 
>> no consensus. If there is a cap, does it really matter?
> 
> I guess not. But I prefer the new type anyway, as it also has a known
> error path for older kernels.
I am fine with the new type too.

Eric
> 
> Cheers,
> Andre.
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/