| Message ID | 20130513201840.5430.86331.stgit@bling.home |
|---|---|
| State | New |
| Headers | show |
On Mon, May 13, 2013 at 02:20:02PM -0600, Alex Williamson wrote: > To support guest MSI affinity changes update the MSI message any time > the guest writes to the address or data fields. > > Signed-off-by: Alex Williamson <alex.williamson@redhat.com> Seems the only way we can fix this for 1.5. Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > > v2: Don't cache MSIMessage > > hw/i386/kvm/pci-assign.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c > index c1e08ec..ff85590 100644 > --- a/hw/i386/kvm/pci-assign.c > +++ b/hw/i386/kvm/pci-assign.c > @@ -1026,6 +1026,21 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev) > } > } > > +static void assigned_dev_update_msi_msg(PCIDevice *pci_dev) > +{ > + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); > + uint8_t ctrl_byte = pci_get_byte(pci_dev->config + pci_dev->msi_cap + > + PCI_MSI_FLAGS); > + > + if (assigned_dev->assigned_irq_type != ASSIGNED_IRQ_MSI || > + !(ctrl_byte & PCI_MSI_FLAGS_ENABLE)) { > + return; > + } > + > + kvm_irqchip_update_msi_route(kvm_state, assigned_dev->msi_virq[0], > + msi_get_message(pci_dev, 0)); > +} > + > static bool assigned_dev_msix_masked(MSIXTableEntry *entry) > { > return (entry->ctrl & cpu_to_le32(0x1)) != 0; > @@ -1201,6 +1216,9 @@ static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, > if (range_covers_byte(address, len, > pci_dev->msi_cap + PCI_MSI_FLAGS)) { > assigned_dev_update_msi(pci_dev); > + } else if (ranges_overlap(address, len, /* 32bit MSI only */ > + pci_dev->msi_cap + PCI_MSI_ADDRESS_LO, 6)) { > + assigned_dev_update_msi_msg(pci_dev); > } > } > if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
"Michael S. Tsirkin" <mst@redhat.com> writes: > On Mon, May 13, 2013 at 02:20:02PM -0600, Alex Williamson wrote: >> To support guest MSI affinity changes update the MSI message any time >> the guest writes to the address or data fields. >> >> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > Seems the only way we can fix this for 1.5. > > Acked-by: Michael S. Tsirkin <mst@redhat.com> Is this a bug or a feature? Can someone describe the scenario in which the bug occurs? Regards, Anthony Liguori > >> --- >> >> v2: Don't cache MSIMessage >> >> hw/i386/kvm/pci-assign.c | 18 ++++++++++++++++++ >> 1 file changed, 18 insertions(+) >> >> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c >> index c1e08ec..ff85590 100644 >> --- a/hw/i386/kvm/pci-assign.c >> +++ b/hw/i386/kvm/pci-assign.c >> @@ -1026,6 +1026,21 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev) >> } >> } >> >> +static void assigned_dev_update_msi_msg(PCIDevice *pci_dev) >> +{ >> + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); >> + uint8_t ctrl_byte = pci_get_byte(pci_dev->config + pci_dev->msi_cap + >> + PCI_MSI_FLAGS); >> + >> + if (assigned_dev->assigned_irq_type != ASSIGNED_IRQ_MSI || >> + !(ctrl_byte & PCI_MSI_FLAGS_ENABLE)) { >> + return; >> + } >> + >> + kvm_irqchip_update_msi_route(kvm_state, assigned_dev->msi_virq[0], >> + msi_get_message(pci_dev, 0)); >> +} >> + >> static bool assigned_dev_msix_masked(MSIXTableEntry *entry) >> { >> return (entry->ctrl & cpu_to_le32(0x1)) != 0; >> @@ -1201,6 +1216,9 @@ static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, >> if (range_covers_byte(address, len, >> pci_dev->msi_cap + PCI_MSI_FLAGS)) { >> assigned_dev_update_msi(pci_dev); >> + } else if (ranges_overlap(address, len, /* 32bit MSI only */ >> + pci_dev->msi_cap + PCI_MSI_ADDRESS_LO, 6)) { >> + assigned_dev_update_msi_msg(pci_dev); >> } >> } >> if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
On Mon, May 20, 2013 at 07:51:40AM -0500, Anthony Liguori wrote: > "Michael S. Tsirkin" <mst@redhat.com> writes: > > > On Mon, May 13, 2013 at 02:20:02PM -0600, Alex Williamson wrote: > >> To support guest MSI affinity changes update the MSI message any time > >> the guest writes to the address or data fields. > >> > >> Signed-off-by: Alex Williamson <alex.williamson@redhat.com> > > > > Seems the only way we can fix this for 1.5. > > > > Acked-by: Michael S. Tsirkin <mst@redhat.com> > > Is this a bug or a feature? Can someone describe the scenario in which > the bug occurs? > > Regards, > > Anthony Liguori It's a bug. Here's how it occurs w.g. with a linux guest: - guest kernel enables MSI, interrupts are sent to CPU0 - irqbalance runs in guest and moves MSI to CPU1 - guest kernel updates MSI register with new data without this patch interrupts keep getting sent to CPU0 in violation of the spec. > > > >> --- > >> > >> v2: Don't cache MSIMessage > >> > >> hw/i386/kvm/pci-assign.c | 18 ++++++++++++++++++ > >> 1 file changed, 18 insertions(+) > >> > >> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c > >> index c1e08ec..ff85590 100644 > >> --- a/hw/i386/kvm/pci-assign.c > >> +++ b/hw/i386/kvm/pci-assign.c > >> @@ -1026,6 +1026,21 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev) > >> } > >> } > >> > >> +static void assigned_dev_update_msi_msg(PCIDevice *pci_dev) > >> +{ > >> + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); > >> + uint8_t ctrl_byte = pci_get_byte(pci_dev->config + pci_dev->msi_cap + > >> + PCI_MSI_FLAGS); > >> + > >> + if (assigned_dev->assigned_irq_type != ASSIGNED_IRQ_MSI || > >> + !(ctrl_byte & PCI_MSI_FLAGS_ENABLE)) { > >> + return; > >> + } > >> + > >> + kvm_irqchip_update_msi_route(kvm_state, assigned_dev->msi_virq[0], > >> + msi_get_message(pci_dev, 0)); > >> +} > >> + > >> static bool assigned_dev_msix_masked(MSIXTableEntry *entry) > >> { > >> return (entry->ctrl & cpu_to_le32(0x1)) != 0; > >> @@ -1201,6 +1216,9 @@ static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, > >> if (range_covers_byte(address, len, > >> pci_dev->msi_cap + PCI_MSI_FLAGS)) { > >> assigned_dev_update_msi(pci_dev); > >> + } else if (ranges_overlap(address, len, /* 32bit MSI only */ > >> + pci_dev->msi_cap + PCI_MSI_ADDRESS_LO, 6)) { > >> + assigned_dev_update_msi_msg(pci_dev); > >> } > >> } > >> if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
Applied. Thanks. Regards, Anthony Liguori
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c index c1e08ec..ff85590 100644 --- a/hw/i386/kvm/pci-assign.c +++ b/hw/i386/kvm/pci-assign.c @@ -1026,6 +1026,21 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev) } } +static void assigned_dev_update_msi_msg(PCIDevice *pci_dev) +{ + AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev); + uint8_t ctrl_byte = pci_get_byte(pci_dev->config + pci_dev->msi_cap + + PCI_MSI_FLAGS); + + if (assigned_dev->assigned_irq_type != ASSIGNED_IRQ_MSI || + !(ctrl_byte & PCI_MSI_FLAGS_ENABLE)) { + return; + } + + kvm_irqchip_update_msi_route(kvm_state, assigned_dev->msi_virq[0], + msi_get_message(pci_dev, 0)); +} + static bool assigned_dev_msix_masked(MSIXTableEntry *entry) { return (entry->ctrl & cpu_to_le32(0x1)) != 0; @@ -1201,6 +1216,9 @@ static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address, if (range_covers_byte(address, len, pci_dev->msi_cap + PCI_MSI_FLAGS)) { assigned_dev_update_msi(pci_dev); + } else if (ranges_overlap(address, len, /* 32bit MSI only */ + pci_dev->msi_cap + PCI_MSI_ADDRESS_LO, 6)) { + assigned_dev_update_msi_msg(pci_dev); } } if (assigned_dev->cap.available & ASSIGNED_DEVICE_CAP_MSIX) {
To support guest MSI affinity changes update the MSI message any time the guest writes to the address or data fields. Signed-off-by: Alex Williamson <alex.williamson@redhat.com> --- v2: Don't cache MSIMessage hw/i386/kvm/pci-assign.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)