From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector Date: Thu, 16 Aug 2012 19:30:08 +0200 Message-ID: <502D2E20.8060806@siemens.com> References: <1345134065.4683.471.camel@ul30vt.home> <502D202C.2050601@siemens.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Cc: Avi Kivity , Marcelo Tosatti , "kvm@vger.kernel.org" To: Alex Williamson Return-path: Received: from david.siemens.de ([192.35.17.14]:30857 "EHLO david.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755866Ab2HPRaM (ORCPT ); Thu, 16 Aug 2012 13:30:12 -0400 In-Reply-To: <502D202C.2050601@siemens.com> Sender: kvm-owner@vger.kernel.org List-ID: 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 >>> --- >>> 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