Hi,
On 21/07/2016 19:25, Marc Zyngier wrote:
> On 21/07/16 18:22, Radim Krčmář wrote:
>> 2016-07-21 17:54+0100, Marc Zyngier:
>>> On 21/07/16 17:13, Radim Krčmář wrote:
>>>> 2016-07-18 13:25+0000, Eric Auger:
>>>>> Extend kvm_kernel_irq_routing_entry to transport the device id
>>>>> field, devid. A new flags field makes possible to indicate the
>>>>> devid is valid. Those additions are used for ARM GICv3 ITS MSI
>>>>> injection.
>>>>>
>>>>> Signed-off-by: Eric Auger <eric.au...@redhat.com>
>>>>> Acked-by: Christoffer Dall <christoffer.d...@linaro.org>
>>>>>
>>>>> ---
>>>>> v6 -> v7:
>>>>> - added msi_ prefix to flags and dev_id fields
>>>>>
>>>>> v4 -> v5:
>>>>> - add Christoffer's R-b
>>>>>
>>>>> v2 -> v3:
>>>>> - add flags
>>>>>
>>>>> v1 -> v2:
>>>>> - replace msi_msg field by a struct composed of msi_msg and devid
>>>>>
>>>>> RFC -> PATCH:
>>>>> - reword the commit message after change in first patch (uapi)
>>>>> ---
>>>>> include/linux/kvm_host.h | 6 +++++-
>>>>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>>>>> index c87fe6f..3d2cbb4 100644
>>>>> --- a/include/linux/kvm_host.h
>>>>> +++ b/include/linux/kvm_host.h
>>>>> @@ -317,7 +317,11 @@ struct kvm_kernel_irq_routing_entry {
>>>>> unsigned irqchip;
>>>>> unsigned pin;
>>>>> } irqchip;
>>>>> - struct msi_msg msi;
>>>>> + struct {
>>>>> + struct msi_msg msi;
>>>>> + u32 msi_flags;
>>>>> + u32 msi_devid;
>>>>
>>>> I'd much rather see them as msi.flags and msi.devid.
>>>
>>> That's not really possible, as msi_msg is the core data structure for
>>> MSIs, and nobody really expects this tructure to change.
>>>
>>>> I didn't notice a code that passes struct msi_msg anywhere, so using an
>>>> ad-hoc struct like irqchip or defining a new one would work fine.
>>>
>>> I guess we could have an arm-specific one:
>>>
>>> struct arm_msi {
>>> struct msi_msg msg;
>>> u32 flags;
>>> u32 devid;
>>> };
>>>
>>> and use that. Would that be OK with you?
>>
>> The feature wants to be arch-neutral, so I would rather ignore struct
>> msi_msg. kvm_kernel_irq_routing_entry practically mirrors routing
>> entries from KVM API and there we have:
>>
>> struct kvm_msi {
>> __u32 address_lo;
>> __u32 address_hi;
>> __u32 data;
>> __u32 flags;
>> __u32 devid;
>> __u8 pad[12];
>> };
>>
>> I think that something like
>>
>> struct {
>> u32 address_lo;
>> u32 address_hi;
>> u32 data;
>> u32 flags;
>> u32 devid;
>> } msi;
>>
>> would get us consistency, minimal changes to existing code, no namespace
>> hierarchy, and no "." vs "_" mistakes at a good price of some code
>> duplication and concept separation.
>
> Fair enough. Eric, can you give this a try?
Yes I will respin quickly replacing the struct msi_msg msi by the struct
proposed by Radim.
Thanks
Eric
>
> Thanks,
>
> M.
>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
