[PATCH 02/13] KVM: extend struct kvm_msi to hold a 32-bit device ID

[Andre Przywara]

Hi Christoffer,

thanks for your time to reviewing this! Was probably no pleasure ;-)

On 28/06/15 20:12, Christoffer Dall wrote:
> On Fri, May 29, 2015 at 10:53:18AM +0100, Andre Przywara wrote:
>> The ARM GICv3 ITS MSI controller requires a device ID to be able to
>> assign the proper interrupt vector. On real hardware, this ID is
>> sampled from the bus. To be able to emulate an ITS controller, extend
>> the KVM MSI interface to let userspace provide such a device ID. For
>> PCI devices, the device ID is simply the 16-bit bus-device-function
>> triplet, which should be easily available to the userland tool.
>>
>> Signed-off-by: Andre Przywara <andre.przywara at arm.com>
>> ---
>>  Documentation/virtual/kvm/api.txt | 8 ++++++--
>>  include/uapi/linux/kvm.h          | 4 +++-
>>  2 files changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 9fa2bf8..891d64a 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -2121,10 +2121,14 @@ struct kvm_msi {
>>  	__u32 address_hi;
>>  	__u32 data;
>>  	__u32 flags;
>> -	__u8  pad[16];
>> +	__u32 devid;
>> +	__u8  pad[12];
>>  };
>>  
>> -No flags are defined so far. The corresponding field must be 0.
>> +flags: KVM_MSI_VALID_DEVID: devid is valid, otherwise ignored.
> 
> I don't see what the 'otherwise ignored' part of the sentence here is
> meant to say, that the flags field is otherwise ignored for other value?

No, that the devid field is ignored if this bit isn't set. I can
rephrase this to be more explicit.

> That's not what the current API doc specifies, it specifies that the
> remainder of the field must be 0.
> 
>> +devid: If KVM_MSI_VALID_DEVID is set, contains a value to identify the device
>> +       that wrote the MSI message. For PCI, this is usually a BFD
>> +       identifier in the lower 16 bits.
> 
> I assume plus something else that uniquely identifies the PCI
> controller?

Well yes, the device ID is a unique device identifier within a system,
the BFD use case was just to illustrate this and give a hint to
userspace what to fill in there. I will explain this better in v2.

So are you OK with that extension of the API in general? Just asking
because there is a lot that depends on it.

Cheers,
Andre.