diff mbox

[10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector

Message ID b2a111f211422dc2db9b44d53651359998f2c926.1345125266.git.jan.kiszka@siemens.com (mailing list archive)
State New, archived
Headers show

Commit Message

Jan Kiszka Aug. 16, 2012, 1:54 p.m. UTC
The refactored version cleanly hides the KVM IOCTL structure from the
users and also zeros out the padding field.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    7 ++-----
 qemu-kvm.c             |    8 --------
 qemu-kvm.h             |    4 ----
 target-i386/kvm.c      |   13 +++++++++++++
 target-i386/kvm_i386.h |    2 ++
 5 files changed, 17 insertions(+), 17 deletions(-)

Comments

Alex Williamson Aug. 16, 2012, 4:21 p.m. UTC | #1
On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
> The refactored version cleanly hides the KVM IOCTL structure from the
> users and also zeros out the padding field.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/device-assignment.c |    7 ++-----
>  qemu-kvm.c             |    8 --------
>  qemu-kvm.h             |    4 ----
>  target-i386/kvm.c      |   13 +++++++++++++
>  target-i386/kvm_i386.h |    2 ++
>  5 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 0e2f8e6..af8a5aa 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>      uint16_t entries_nr = 0;
>      int i, r = 0;
> -    struct kvm_assigned_msix_entry msix_entry;
>      MSIXTableEntry *entry = adev->msix_table;
>  
>      /* Get the usable entry number for allocating */
> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>      adev->irq_entries_nr = adev->msix_max;
>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
>  
> -    msix_entry.assigned_dev_id = adev->dev_id;
>      entry = adev->msix_table;
>      for (i = 0; i < adev->msix_max; i++, entry++) {
>          if (msix_masked(entry)) {
> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>  
>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
>  
> -        msix_entry.gsi = adev->entry[i].gsi;
> -        msix_entry.entry = i;
> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
> +                                       adev->entry[i].gsi);
>          if (r) {
>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
>              break;
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 1a2a4fd..ec1911f 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
>  #endif
>  }
>  
> -#ifdef KVM_CAP_DEVICE_MSIX
> -int kvm_assign_set_msix_entry(KVMState *s,
> -                              struct kvm_assigned_msix_entry *entry)
> -{
> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> -}
> -#endif
> -
>  #if !defined(TARGET_I386)
>  void kvm_arch_init_irq_routing(KVMState *s)
>  {
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 3fd6046..ad628d5 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
>                               struct kvm_irq_routing_entry *newentry);
>  
> -
> -int kvm_assign_set_msix_entry(KVMState *s,
> -                              struct kvm_assigned_msix_entry *entry);
> -
>  #endif /* CONFIG_KVM */
>  
>  #endif
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 676f45b..e9353ed 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
>  }
>  
> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> +                               int virq)
> +{
> +    struct kvm_assigned_msix_entry msix_entry = {
> +        .assigned_dev_id = dev_id,
> +        .gsi = virq,
> +        .entry = vector,
> +    };
> +
> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));

nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,

Alex


> +    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
> +}
> +
>  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
>  {
>      return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
> diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> index aac14eb..bd3b398 100644
> --- a/target-i386/kvm_i386.h
> +++ b/target-i386/kvm_i386.h
> @@ -30,6 +30,8 @@ int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
>  bool kvm_device_msix_supported(KVMState *s);
>  int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>                                   uint32_t nr_vectors);
> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> +                               int virq);
>  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
>  
>  #endif



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka Aug. 16, 2012, 4:30 p.m. UTC | #2
On 2012-08-16 18:21, Alex Williamson wrote:
> On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
>> The refactored version cleanly hides the KVM IOCTL structure from the
>> users and also zeros out the padding field.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/device-assignment.c |    7 ++-----
>>  qemu-kvm.c             |    8 --------
>>  qemu-kvm.h             |    4 ----
>>  target-i386/kvm.c      |   13 +++++++++++++
>>  target-i386/kvm_i386.h |    2 ++
>>  5 files changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>> index 0e2f8e6..af8a5aa 100644
>> --- a/hw/device-assignment.c
>> +++ b/hw/device-assignment.c
>> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>>      uint16_t entries_nr = 0;
>>      int i, r = 0;
>> -    struct kvm_assigned_msix_entry msix_entry;
>>      MSIXTableEntry *entry = adev->msix_table;
>>  
>>      /* Get the usable entry number for allocating */
>> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>      adev->irq_entries_nr = adev->msix_max;
>>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
>>  
>> -    msix_entry.assigned_dev_id = adev->dev_id;
>>      entry = adev->msix_table;
>>      for (i = 0; i < adev->msix_max; i++, entry++) {
>>          if (msix_masked(entry)) {
>> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>  
>>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
>>  
>> -        msix_entry.gsi = adev->entry[i].gsi;
>> -        msix_entry.entry = i;
>> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
>> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
>> +                                       adev->entry[i].gsi);
>>          if (r) {
>>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
>>              break;
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index 1a2a4fd..ec1911f 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
>>  #endif
>>  }
>>  
>> -#ifdef KVM_CAP_DEVICE_MSIX
>> -int kvm_assign_set_msix_entry(KVMState *s,
>> -                              struct kvm_assigned_msix_entry *entry)
>> -{
>> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
>> -}
>> -#endif
>> -
>>  #if !defined(TARGET_I386)
>>  void kvm_arch_init_irq_routing(KVMState *s)
>>  {
>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>> index 3fd6046..ad628d5 100644
>> --- a/qemu-kvm.h
>> +++ b/qemu-kvm.h
>> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
>>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
>>                               struct kvm_irq_routing_entry *newentry);
>>  
>> -
>> -int kvm_assign_set_msix_entry(KVMState *s,
>> -                              struct kvm_assigned_msix_entry *entry);
>> -
>>  #endif /* CONFIG_KVM */
>>  
>>  #endif
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 676f45b..e9353ed 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
>>  }
>>  
>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
>> +                               int virq)
>> +{
>> +    struct kvm_assigned_msix_entry msix_entry = {
>> +        .assigned_dev_id = dev_id,
>> +        .gsi = virq,
>> +        .entry = vector,
>> +    };
>> +
>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> 
> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,

I think to remember it has to be .padding = { 0, 0, 0 } (due to three
padding elements) to be standard conforming, but that would still be
nicer than the memset, indeed.

Jan
Avi Kivity Aug. 16, 2012, 4:34 p.m. UTC | #3
On 08/16/2012 07:21 PM, Alex Williamson wrote:
>>  
>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
>> +                               int virq)
>> +{
>> +    struct kvm_assigned_msix_entry msix_entry = {
>> +        .assigned_dev_id = dev_id,
>> +        .gsi = virq,
>> +        .entry = vector,
>> +    };
>> +
>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> 
> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,

It can be done with a null statement.  The msix_entry initialization
above zero-initializes all fields that are not mentioned in the initializer.
Alex Williamson Aug. 16, 2012, 4:43 p.m. UTC | #4
On Thu, 2012-08-16 at 19:34 +0300, Avi Kivity wrote:
> On 08/16/2012 07:21 PM, Alex Williamson wrote:
> >>  
> >> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> >> +                               int virq)
> >> +{
> >> +    struct kvm_assigned_msix_entry msix_entry = {
> >> +        .assigned_dev_id = dev_id,
> >> +        .gsi = virq,
> >> +        .entry = vector,
> >> +    };
> >> +
> >> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> > 
> > nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,
> 
> It can be done with a null statement.  The msix_entry initialization
> above zero-initializes all fields that are not mentioned in the initializer.

Yeah, I thought that was probably the case as well.  Thanks for
confirming.  The .padding = 0 in the previous patch could also be
dropped then.

Alex

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jan Kiszka Aug. 16, 2012, 5:30 p.m. UTC | #5
On 2012-08-16 18:30, Jan Kiszka wrote:
> On 2012-08-16 18:21, Alex Williamson wrote:
>> On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
>>> The refactored version cleanly hides the KVM IOCTL structure from the
>>> users and also zeros out the padding field.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  hw/device-assignment.c |    7 ++-----
>>>  qemu-kvm.c             |    8 --------
>>>  qemu-kvm.h             |    4 ----
>>>  target-i386/kvm.c      |   13 +++++++++++++
>>>  target-i386/kvm_i386.h |    2 ++
>>>  5 files changed, 17 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>>> index 0e2f8e6..af8a5aa 100644
>>> --- a/hw/device-assignment.c
>>> +++ b/hw/device-assignment.c
>>> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>>>      uint16_t entries_nr = 0;
>>>      int i, r = 0;
>>> -    struct kvm_assigned_msix_entry msix_entry;
>>>      MSIXTableEntry *entry = adev->msix_table;
>>>  
>>>      /* Get the usable entry number for allocating */
>>> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>>      adev->irq_entries_nr = adev->msix_max;
>>>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
>>>  
>>> -    msix_entry.assigned_dev_id = adev->dev_id;
>>>      entry = adev->msix_table;
>>>      for (i = 0; i < adev->msix_max; i++, entry++) {
>>>          if (msix_masked(entry)) {
>>> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>>  
>>>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
>>>  
>>> -        msix_entry.gsi = adev->entry[i].gsi;
>>> -        msix_entry.entry = i;
>>> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
>>> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
>>> +                                       adev->entry[i].gsi);
>>>          if (r) {
>>>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
>>>              break;
>>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>>> index 1a2a4fd..ec1911f 100644
>>> --- a/qemu-kvm.c
>>> +++ b/qemu-kvm.c
>>> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
>>>  #endif
>>>  }
>>>  
>>> -#ifdef KVM_CAP_DEVICE_MSIX
>>> -int kvm_assign_set_msix_entry(KVMState *s,
>>> -                              struct kvm_assigned_msix_entry *entry)
>>> -{
>>> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
>>> -}
>>> -#endif
>>> -
>>>  #if !defined(TARGET_I386)
>>>  void kvm_arch_init_irq_routing(KVMState *s)
>>>  {
>>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>>> index 3fd6046..ad628d5 100644
>>> --- a/qemu-kvm.h
>>> +++ b/qemu-kvm.h
>>> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
>>>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
>>>                               struct kvm_irq_routing_entry *newentry);
>>>  
>>> -
>>> -int kvm_assign_set_msix_entry(KVMState *s,
>>> -                              struct kvm_assigned_msix_entry *entry);
>>> -
>>>  #endif /* CONFIG_KVM */
>>>  
>>>  #endif
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> index 676f45b..e9353ed 100644
>>> --- a/target-i386/kvm.c
>>> +++ b/target-i386/kvm.c
>>> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>>>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
>>>  }
>>>  
>>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
>>> +                               int virq)
>>> +{
>>> +    struct kvm_assigned_msix_entry msix_entry = {
>>> +        .assigned_dev_id = dev_id,
>>> +        .gsi = virq,
>>> +        .entry = vector,
>>> +    };
>>> +
>>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
>>
>> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,
> 
> I think to remember it has to be .padding = { 0, 0, 0 } (due to three
> padding elements) to be standard conforming, but that would still be
> nicer than the memset, indeed.

I've found some minor inconsistencies in the IOCTL struct
zero-initializations. Will send v2, just waiting in case you have more
comments.

Thanks,
Jan
Alex Williamson Aug. 16, 2012, 6:23 p.m. UTC | #6
On Thu, 2012-08-16 at 19:30 +0200, Jan Kiszka wrote:
> On 2012-08-16 18:30, Jan Kiszka wrote:
> > On 2012-08-16 18:21, Alex Williamson wrote:
> >> On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
> >>> The refactored version cleanly hides the KVM IOCTL structure from the
> >>> users and also zeros out the padding field.
> >>>
> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>> ---
> >>>  hw/device-assignment.c |    7 ++-----
> >>>  qemu-kvm.c             |    8 --------
> >>>  qemu-kvm.h             |    4 ----
> >>>  target-i386/kvm.c      |   13 +++++++++++++
> >>>  target-i386/kvm_i386.h |    2 ++
> >>>  5 files changed, 17 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> >>> index 0e2f8e6..af8a5aa 100644
> >>> --- a/hw/device-assignment.c
> >>> +++ b/hw/device-assignment.c
> >>> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> >>>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> >>>      uint16_t entries_nr = 0;
> >>>      int i, r = 0;
> >>> -    struct kvm_assigned_msix_entry msix_entry;
> >>>      MSIXTableEntry *entry = adev->msix_table;
> >>>  
> >>>      /* Get the usable entry number for allocating */
> >>> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> >>>      adev->irq_entries_nr = adev->msix_max;
> >>>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
> >>>  
> >>> -    msix_entry.assigned_dev_id = adev->dev_id;
> >>>      entry = adev->msix_table;
> >>>      for (i = 0; i < adev->msix_max; i++, entry++) {
> >>>          if (msix_masked(entry)) {
> >>> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> >>>  
> >>>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
> >>>  
> >>> -        msix_entry.gsi = adev->entry[i].gsi;
> >>> -        msix_entry.entry = i;
> >>> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
> >>> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
> >>> +                                       adev->entry[i].gsi);
> >>>          if (r) {
> >>>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
> >>>              break;
> >>> diff --git a/qemu-kvm.c b/qemu-kvm.c
> >>> index 1a2a4fd..ec1911f 100644
> >>> --- a/qemu-kvm.c
> >>> +++ b/qemu-kvm.c
> >>> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
> >>>  #endif
> >>>  }
> >>>  
> >>> -#ifdef KVM_CAP_DEVICE_MSIX
> >>> -int kvm_assign_set_msix_entry(KVMState *s,
> >>> -                              struct kvm_assigned_msix_entry *entry)
> >>> -{
> >>> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> >>> -}
> >>> -#endif
> >>> -
> >>>  #if !defined(TARGET_I386)
> >>>  void kvm_arch_init_irq_routing(KVMState *s)
> >>>  {
> >>> diff --git a/qemu-kvm.h b/qemu-kvm.h
> >>> index 3fd6046..ad628d5 100644
> >>> --- a/qemu-kvm.h
> >>> +++ b/qemu-kvm.h
> >>> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
> >>>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
> >>>                               struct kvm_irq_routing_entry *newentry);
> >>>  
> >>> -
> >>> -int kvm_assign_set_msix_entry(KVMState *s,
> >>> -                              struct kvm_assigned_msix_entry *entry);
> >>> -
> >>>  #endif /* CONFIG_KVM */
> >>>  
> >>>  #endif
> >>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >>> index 676f45b..e9353ed 100644
> >>> --- a/target-i386/kvm.c
> >>> +++ b/target-i386/kvm.c
> >>> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
> >>>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
> >>>  }
> >>>  
> >>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> >>> +                               int virq)
> >>> +{
> >>> +    struct kvm_assigned_msix_entry msix_entry = {
> >>> +        .assigned_dev_id = dev_id,
> >>> +        .gsi = virq,
> >>> +        .entry = vector,
> >>> +    };
> >>> +
> >>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> >>
> >> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,
> > 
> > I think to remember it has to be .padding = { 0, 0, 0 } (due to three
> > padding elements) to be standard conforming, but that would still be
> > nicer than the memset, indeed.
> 
> I've found some minor inconsistencies in the IOCTL struct
> zero-initializations. Will send v2, just waiting in case you have more
> comments.

Nope, I didn't see anything else.  Thanks,

Alex



--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 0e2f8e6..af8a5aa 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1045,7 +1045,6 @@  static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint16_t entries_nr = 0;
     int i, r = 0;
-    struct kvm_assigned_msix_entry msix_entry;
     MSIXTableEntry *entry = adev->msix_table;
 
     /* Get the usable entry number for allocating */
@@ -1075,7 +1074,6 @@  static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     adev->irq_entries_nr = adev->msix_max;
     adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
 
-    msix_entry.assigned_dev_id = adev->dev_id;
     entry = adev->msix_table;
     for (i = 0; i < adev->msix_max; i++, entry++) {
         if (msix_masked(entry)) {
@@ -1098,9 +1096,8 @@  static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
 
         kvm_add_routing_entry(kvm_state, &adev->entry[i]);
 
-        msix_entry.gsi = adev->entry[i].gsi;
-        msix_entry.entry = i;
-        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
+        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
+                                       adev->entry[i].gsi);
         if (r) {
             fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
             break;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 1a2a4fd..ec1911f 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -185,14 +185,6 @@  int kvm_get_irq_route_gsi(void)
 #endif
 }
 
-#ifdef KVM_CAP_DEVICE_MSIX
-int kvm_assign_set_msix_entry(KVMState *s,
-                              struct kvm_assigned_msix_entry *entry)
-{
-    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
-}
-#endif
-
 #if !defined(TARGET_I386)
 void kvm_arch_init_irq_routing(KVMState *s)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 3fd6046..ad628d5 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -65,10 +65,6 @@  int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
 int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
                              struct kvm_irq_routing_entry *newentry);
 
-
-int kvm_assign_set_msix_entry(KVMState *s,
-                              struct kvm_assigned_msix_entry *entry);
-
 #endif /* CONFIG_KVM */
 
 #endif
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 676f45b..e9353ed 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2173,6 +2173,19 @@  int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
     return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
 }
 
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+                               int virq)
+{
+    struct kvm_assigned_msix_entry msix_entry = {
+        .assigned_dev_id = dev_id,
+        .gsi = virq,
+        .entry = vector,
+    };
+
+    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
+}
+
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
 {
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index aac14eb..bd3b398 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -30,6 +30,8 @@  int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
 bool kvm_device_msix_supported(KVMState *s);
 int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
                                  uint32_t nr_vectors);
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+                               int virq);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 #endif