Hi Pavel, On 02/07/15 08:26, Pavel Fedin wrote: > Hello! > >> -----Original Message----- >> From: kvm-owner@xxxxxxxxxxxxxxx [mailto:kvm-owner@xxxxxxxxxxxxxxx] On Behalf Of Eric Auger >> Sent: Monday, June 29, 2015 6:37 PM >> To: eric.auger@xxxxxx; eric.auger@xxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx; >> marc.zyngier@xxxxxxx; christoffer.dall@xxxxxxxxxx; andre.przywara@xxxxxxx; >> kvmarm@xxxxxxxxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx >> Cc: linux-kernel@xxxxxxxxxxxxxxx; patches@xxxxxxxxxx; p.fedin@xxxxxxxxxxx; pbonzini@xxxxxxxxxx >> Subject: [PATCH 1/7] KVM: api: add kvm_irq_routing_extended_msi >> >> On ARM, the MSI msg (address and data) comes along with >> out-of-band device ID information. The device ID encodes the device >> that composes the MSI msg. Let's create a new routing entry type, >> dubbed KVM_IRQ_ROUTING_EXTENDED_MSI and use the __u32 pad space >> to convey the device ID. >> >> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >> >> --- >> >> RFC -> PATCH >> - remove kvm_irq_routing_extended_msi and use union instead >> --- >> Documentation/virtual/kvm/api.txt | 9 ++++++++- >> include/uapi/linux/kvm.h | 6 +++++- >> 2 files changed, 13 insertions(+), 2 deletions(-) >> >> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt >> index d20fd94..6426ae9 100644 >> --- a/Documentation/virtual/kvm/api.txt >> +++ b/Documentation/virtual/kvm/api.txt >> @@ -1414,7 +1414,10 @@ struct kvm_irq_routing_entry { >> __u32 gsi; >> __u32 type; >> __u32 flags; >> - __u32 pad; >> + union { >> + __u32 pad; >> + __u32 devid; >> + }; >> union { >> struct kvm_irq_routing_irqchip irqchip; >> struct kvm_irq_routing_msi msi; > > devid is actually a part of MSI bunch. Shouldn't it be a part of struct kvm_irq_routing_msi then? > It also has reserved pad. > >> @@ -1427,6 +1430,10 @@ struct kvm_irq_routing_entry { >> #define KVM_IRQ_ROUTING_IRQCHIP 1 >> #define KVM_IRQ_ROUTING_MSI 2 >> #define KVM_IRQ_ROUTING_S390_ADAPTER 3 >> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4 >> + >> +In case of KVM_IRQ_ROUTING_EXTENDED_MSI routing type, devid is used to convey >> +the device ID. >> >> No flags are specified so far, the corresponding field must be set to zero. > > What if we use KVM_MSI_VALID_DEVID flag instead of new KVM_IRQ_ROUTING_EXTENDED_MSI definition? I > believe this would make an API more consistent and introduce less new definitions. I like this approach, but it runs into problems: As you read above the current documentation says that the flags field must be zero and the current KVM_SET_GSI_ROUTING handler bails out if it isn't. So userland would need to know whether it's safe to set that field. Introducing a new KVM_CAP_... value seems overkill if we could just have a new routing entry type. So we could still reuse the existing struct kvm_irq_routing_msi (and extend that with the devid field), but we would have to add a new routing type number. Maybe we could collapse this into the existing MSI type + flag when handing it further down the kernel? Cheers, Andre. > >> >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index 2a23705..8484681 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -841,12 +841,16 @@ struct kvm_irq_routing_s390_adapter { >> #define KVM_IRQ_ROUTING_IRQCHIP 1 >> #define KVM_IRQ_ROUTING_MSI 2 >> #define KVM_IRQ_ROUTING_S390_ADAPTER 3 >> +#define KVM_IRQ_ROUTING_EXTENDED_MSI 4 >> >> struct kvm_irq_routing_entry { >> __u32 gsi; >> __u32 type; >> __u32 flags; >> - __u32 pad; >> + union { >> + __u32 pad; >> + __u32 devid; >> + }; >> union { >> struct kvm_irq_routing_irqchip irqchip; >> struct kvm_irq_routing_msi msi; >> -- >> 1.9.1 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe kvm" in >> the body of a message to majordomo@xxxxxxxxxxxxxxx >> More majordomo info at http://vger.kernel.org/majordomo-info.html > > Kind regards, > Pavel Fedin > Expert Engineer > Samsung Electronics Research center Russia > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
