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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxx>
>>> ---
>>>  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

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux