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

[Andre Przywara <andre.przywara@arm.com>]

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?

> 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.

Cheers,
Andre.