[v6,4/8] kvm-all: Pass requester ID to MSI routing functions

From: Pavel Fedin <p.fedin@samsung.com>

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 <p.fedin@samsung.com>
Signed-off-by: Eric Auger <eric.auger@redhat.com>

---
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(+)

On 9 September 2016 at 18:30, Eric Auger <eric.auger@redhat.com> wrote:
> From: Pavel Fedin <p.fedin@samsung.com>
>
> 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 <p.fedin@samsung.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>
> ---
> 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.

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

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

Hi Peter,

On 22/09/2016 16:35, Peter Maydell wrote:
> On 9 September 2016 at 18:30, Eric Auger <eric.auger@redhat.com> wrote:
>> From: Pavel Fedin <p.fedin@samsung.com>
>>
>> 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 <p.fedin@samsung.com>
>> Signed-off-by: Eric Auger <eric.auger@redhat.com>
>>
>> ---
>> 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
>