Re: [Qemu-devel] [PATCH v6 4/8] kvm-all: Pass requester ID to MSI routing functions

[Auger Eric]

Hi Peter,

On 22/09/2016 16:35, Peter Maydell wrote:
> On 9 September 2016 at 18:30, Eric Auger <address@hidden> wrote:
>> From: Pavel Fedin <address@hidden>
>>
>> Introduce global kvm_msi_use_devid flag and pass the device ID,
>> if needed, while building the MSI route entry. Device IDs are
>> required by the ARM GICv3 ITS (IRQ remapping function is based on
>> this information).
>>
>> Signed-off-by: Pavel Fedin <address@hidden>
>> Signed-off-by: Eric Auger <address@hidden>
>>
>> ---
>> v5 -> v6:
>> - move the flag in include/sysemu/kvm.h and populate the devid
>>   information in kvm-all.c
>>
>> v3 -> v4:
>> - OR route->flags with KVM_MSI_VALID_DEVID
>> ---
>>  include/sysemu/kvm.h | 1 +
>>  kvm-all.c            | 9 +++++++++
>>  kvm-stub.c           | 1 +
>>  3 files changed, 11 insertions(+)
>>
>> diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
>> index c9c2436..c31c810 100644
>> --- a/include/sysemu/kvm.h
>> +++ b/include/sysemu/kvm.h
>> @@ -53,6 +53,7 @@ extern bool kvm_gsi_direct_mapping;
>>  extern bool kvm_readonly_mem_allowed;
>>  extern bool kvm_direct_msi_allowed;
>>  extern bool kvm_ioeventfd_any_length_allowed;
>> +extern bool kvm_msi_use_devid;
>>
>>  #if defined CONFIG_KVM || !defined NEED_CPU_H
>>  #define kvm_enabled()           (kvm_allowed)
> 
> You also need the appropriate #define, like all the other flags
> here (with a doc comment, and with the version that's just defined
> as '(false)').
> 
> The idea is that tests of the condition then don't need to
> be guarded by ifdef CONFIG_KVM.

Hum yes I totally forgot this.
> 
>> diff --git a/kvm-all.c b/kvm-all.c
>> index ebf35b0..df3bcde 100644
>> --- a/kvm-all.c
>> +++ b/kvm-all.c
>> @@ -119,6 +119,7 @@ bool kvm_readonly_mem_allowed;
>>  bool kvm_vm_attributes_allowed;
>>  bool kvm_direct_msi_allowed;
>>  bool kvm_ioeventfd_any_length_allowed;
>> +bool kvm_msi_use_devid;
>>
>>  static const KVMCapabilityInfo kvm_required_capabilites[] = {
>>      KVM_CAP_INFO(USER_MEMORY),
>> @@ -1275,6 +1276,10 @@ int kvm_irqchip_add_msi_route(KVMState *s, int 
>> vector, PCIDevice *dev)
>>      kroute.u.msi.address_lo = (uint32_t)msg.address;
>>      kroute.u.msi.address_hi = msg.address >> 32;
>>      kroute.u.msi.data = le32_to_cpu(msg.data);
>> +    if (kvm_msi_use_devid) {
> 
> these should use the macro, not direct access to the flag variable.
Sure

Thanks

Eric
> 
>> +        kroute.flags = KVM_MSI_VALID_DEVID;
>> +        kroute.u.msi.devid = pci_requester_id(dev);
>> +    }
>>      if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, dev)) {
>>          kvm_irqchip_release_virq(s, virq);
>>          return -EINVAL;
>> @@ -1308,6 +1313,10 @@ int kvm_irqchip_update_msi_route(KVMState *s, int 
>> virq, MSIMessage msg,
>>      kroute.u.msi.address_lo = (uint32_t)msg.address;
>>      kroute.u.msi.address_hi = msg.address >> 32;
>>      kroute.u.msi.data = le32_to_cpu(msg.data);
>> +    if (kvm_msi_use_devid) {
>> +        kroute.flags = KVM_MSI_VALID_DEVID;
>> +        kroute.u.msi.devid = pci_requester_id(dev);
>> +    }
>>      if (kvm_arch_fixup_msi_route(&kroute, msg.address, msg.data, dev)) {
>>          return -EINVAL;
>>      }
>> diff --git a/kvm-stub.c b/kvm-stub.c
>> index 64e23f6..6f896e9 100644
>> --- a/kvm-stub.c
>> +++ b/kvm-stub.c
>> @@ -31,6 +31,7 @@ bool kvm_gsi_direct_mapping;
>>  bool kvm_allowed;
>>  bool kvm_readonly_mem_allowed;
>>  bool kvm_ioeventfd_any_length_allowed;
>> +bool kvm_msi_use_devid;
>>
>>  int kvm_destroy_vcpu(CPUState *cpu)
>>  {
> 
> thanks
> -- PMM
>