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

On 07/06/2015 05:52 PM, Andre Przywara wrote:

Salut Eric,

....

quoted

quoted

 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.

quoted

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

quoted

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

quoted

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.

quoted

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.