On Thursday 04 November 2010 17:47:16 Michael S. Tsirkin wrote: > On Thu, Nov 04, 2010 at 02:18:20PM +0800, Sheng Yang wrote: > > Signed-off-by: Sheng Yang <sheng@xxxxxxxxxxxxxxx> > > If you have the energy for cleanups, how about > removing most of msi/msix code in device-assignment and using msi.c and > msix.c instead? Ah, this one is only used to reduce the review difficulty. I won't mind if you want to merge it with the last one... Use msi.c/msix.c totally another big clean up work to do. Let put it later... -- regards Yang, Sheng > > > --- > > > > hw/device-assignment.c | 77 > > ++++++++++++++++++++++++++++++++--------------- 1 files changed, 52 > > insertions(+), 25 deletions(-) > > > > diff --git a/hw/device-assignment.c b/hw/device-assignment.c > > index 2605bd1..8a98876 100644 > > --- a/hw/device-assignment.c > > +++ b/hw/device-assignment.c > > @@ -1106,15 +1106,10 @@ static void assigned_dev_update_msi(PCIDevice > > *pci_dev, unsigned int ctrl_pos) > > > > #endif > > > > #ifdef KVM_CAP_DEVICE_MSIX > > > > -static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev) > > +static int get_msix_entries_max_nr(AssignedDevice *adev) > > > > { > > > > - AssignedDevice *adev = container_of(pci_dev, AssignedDevice, dev); > > - uint16_t entries_nr = 0, entries_max_nr; > > - int pos = 0, i, r = 0; > > - uint32_t msg_addr, msg_upper_addr, msg_data, msg_ctrl; > > - struct kvm_assigned_msix_nr msix_nr; > > - struct kvm_assigned_msix_entry msix_entry; > > - void *va = adev->msix_table_page; > > + int pos, entries_max_nr; > > + PCIDevice *pci_dev = &adev->dev; > > > > if (adev->cap.available & ASSIGNED_DEVICE_CAP_MSI) > > > > pos = pci_dev->cap.start + PCI_CAPABILITY_CONFIG_MSI_LENGTH; > > > > @@ -1125,6 +1120,17 @@ static int assigned_dev_update_msix_mmio(PCIDevice > > *pci_dev) > > > > entries_max_nr &= PCI_MSIX_TABSIZE; > > entries_max_nr += 1; > > > > + return entries_max_nr; > > +} > > + > > +static int get_msix_valid_entries_nr(AssignedDevice *adev, > > + uint16_t entries_max_nr) > > +{ > > + void *va = adev->msix_table_page; > > + uint32_t msg_data, msg_ctrl; > > + uint16_t entries_nr = 0; > > + int i; > > + > > > > /* Get the usable entry number for allocating */ > > for (i = 0; i < entries_max_nr; i++) { > > > > memcpy(&msg_ctrl, va + i * 16 + 12, 4); > > > > @@ -1134,11 +1140,20 @@ static int > > assigned_dev_update_msix_mmio(PCIDevice *pci_dev) > > > > continue; > > > > entries_nr ++; > > > > } > > > > + return entries_nr; > > +} > > + > > +static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev, > > + uint16_t entries_nr, > > + uint16_t entries_max_nr) > > +{ > > + AssignedDevice *adev = container_of(pci_dev, AssignedDevice, dev); > > + int i, r = 0; > > + uint32_t msg_addr, msg_upper_addr, msg_data, msg_ctrl; > > + struct kvm_assigned_msix_nr msix_nr; > > + struct kvm_assigned_msix_entry msix_entry; > > + void *va = adev->msix_table_page; > > > > - if (entries_nr == 0) { > > - fprintf(stderr, "MSI-X entry number is zero!\n"); > > - return -EINVAL; > > - } > > > > msix_nr.assigned_dev_id = calc_assigned_dev_id(adev->h_segnr, > > adev->h_busnr, > > > > (uint8_t)adev->h_devfn); > > > > msix_nr.entry_nr = entries_nr; > > > > @@ -1180,8 +1195,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice > > *pci_dev) > > > > adev->entry[entries_nr].u.msi.address_lo = msg_addr; > > adev->entry[entries_nr].u.msi.address_hi = msg_upper_addr; > > adev->entry[entries_nr].u.msi.data = msg_data; > > > > - DEBUG("MSI-X data 0x%x, MSI-X addr_lo 0x%x\n!", msg_data, > > msg_addr); - kvm_add_routing_entry(&adev->entry[entries_nr]); > > + DEBUG("MSI-X data 0x%x, MSI-X addr_lo 0x%x!\n", msg_data, > > msg_addr); + kvm_add_routing_entry(&adev->entry[entries_nr]); > > > > msix_entry.gsi = adev->entry[entries_nr].gsi; > > msix_entry.entry = i; > > > > @@ -1190,7 +1205,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice > > *pci_dev) > > > > fprintf(stderr, "fail to set MSI-X entry! %s\n", > > strerror(-r)); break; > > > > } > > > > - DEBUG("MSI-X entry gsi 0x%x, entry %d\n!", > > + DEBUG("MSI-X entry gsi 0x%x, entry %d!\n", > > > > msix_entry.gsi, msix_entry.entry); > > > > entries_nr ++; > > > > } > > > > @@ -1203,12 +1218,12 @@ static int > > assigned_dev_update_msix_mmio(PCIDevice *pci_dev) > > > > return r; > > > > } > > > > -static void assigned_dev_update_msix(PCIDevice *pci_dev, unsigned int > > ctrl_pos) +static void assigned_dev_update_msix(PCIDevice *pci_dev, int > > enable_msix) > > > > { > > > > struct kvm_assigned_irq assigned_irq_data; > > AssignedDevice *assigned_dev = container_of(pci_dev, AssignedDevice, > > dev); > > > > - uint16_t *ctrl_word = (uint16_t *)(pci_dev->config + ctrl_pos); > > > > int r; > > > > + uint16_t entries_nr, entries_max_nr; > > > > memset(&assigned_irq_data, 0, sizeof assigned_irq_data); > > assigned_irq_data.assigned_dev_id = > > > > @@ -1219,8 +1234,7 @@ static void assigned_dev_update_msix(PCIDevice > > *pci_dev, unsigned int ctrl_pos) > > > > * try to catch this by only deassigning irqs if the guest is using > > * MSIX or intends to start. */ > > > > if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) || > > > > - (*ctrl_word & PCI_MSIX_ENABLE)) { > > - > > + enable_msix) { > > > > assigned_irq_data.flags = assigned_dev->irq_requested_type; > > free_dev_irq_entries(assigned_dev); > > r = kvm_deassign_irq(kvm_context, &assigned_irq_data); > > > > @@ -1231,14 +1245,25 @@ static void assigned_dev_update_msix(PCIDevice > > *pci_dev, unsigned int ctrl_pos) > > > > assigned_dev->irq_requested_type = 0; > > > > } > > > > - if (*ctrl_word & PCI_MSIX_ENABLE) { > > - assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX | > > - KVM_DEV_IRQ_GUEST_MSIX; > > - > > - if (assigned_dev_update_msix_mmio(pci_dev) < 0) { > > + entries_max_nr = get_msix_entries_max_nr(assigned_dev); > > + if (entries_max_nr == 0) { > > + fprintf(stderr, "assigned_dev_update_msix: MSI-X entries_max_nr > > == 0"); + return; > > + } > > + entries_nr = get_msix_valid_entries_nr(assigned_dev, > > entries_max_nr); + if (entries_nr == 0) { > > + if (enable_msix) > > + fprintf(stderr, "MSI-X entry number is zero!\n"); > > + return; > > + } > > + if (enable_msix) { > > + if (assigned_dev_update_msix_mmio(pci_dev, > > + entries_nr, entries_max_nr) < 0) { > > > > perror("assigned_dev_update_msix_mmio"); > > return; > > > > } > > > > + assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX | > > + KVM_DEV_IRQ_GUEST_MSIX; > > > > if (kvm_assign_irq(kvm_context, &assigned_irq_data) < 0) { > > > > perror("assigned_dev_enable_msix: assign irq"); > > return; > > > > @@ -1254,6 +1279,7 @@ static void > > assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t ad > > > > { > > > > AssignedDevice *assigned_dev = container_of(pci_dev, AssignedDevice, > > dev); unsigned int pos = pci_dev->cap.start, ctrl_pos; > > > > + uint16_t *ctrl_word; > > > > pci_default_cap_write_config(pci_dev, address, val, len); > > > > #ifdef KVM_CAP_IRQ_ROUTING > > > > @@ -1270,7 +1296,8 @@ static void > > assigned_device_pci_cap_write_config(PCIDevice *pci_dev, uint32_t ad > > > > ctrl_pos = pos + 3; > > if (address <= ctrl_pos && address + len > ctrl_pos) { > > > > ctrl_pos--; /* control is word long */ > > > > - assigned_dev_update_msix(pci_dev, ctrl_pos); > > + ctrl_word = (uint16_t *)(pci_dev->config + ctrl_pos); > > + assigned_dev_update_msix(pci_dev, (*ctrl_word & > > PCI_MSIX_ENABLE)); > > > > } > > > > pos += PCI_CAPABILITY_CONFIG_MSIX_LENGTH; > > > > } -- 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