[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH] [Also for STABLE-0.12] Don't check for bus mast
From: |
Michael S. Tsirkin |
Subject: |
[Qemu-devel] Re: [PATCH] [Also for STABLE-0.12] Don't check for bus master for old guests |
Date: |
Tue, 16 Mar 2010 22:27:18 +0200 |
User-agent: |
Mutt/1.5.19 (2009-01-05) |
On Tue, Mar 16, 2010 at 09:26:53PM +0100, Alexander Graf wrote:
>
> On 16.03.2010, at 21:21, Michael S. Tsirkin wrote:
>
> > On Tue, Mar 16, 2010 at 07:18:07PM +0100, Alexander Graf wrote:
> >> Older Linux guests don't activate the bus master enable bit. So for those
> >> we
> >> can just try to be clever and track if they set the DEVICE_OK bit even
> >> though
> >> bus mastering is still disabled.
> >>
> >> Under that condition we can disable the windows safety check. With that
> >> logic
> >> in place both guests should work just fine. Without PCI hotplug breaks
> >> virtio-net in Linux < 2.6.34 guests.
> >>
> >> Signed-off-by: Alexander Graf <address@hidden>
> >> CC: Michael S. Tsirkin <address@hidden>
> >> ---
> >> hw/virtio-pci.c | 25 ++++++++++++++++++++++++-
> >> 1 files changed, 24 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> >> index 3594152..4fc4b3c 100644
> >> --- a/hw/virtio-pci.c
> >> +++ b/hw/virtio-pci.c
> >> @@ -76,6 +76,10 @@
> >> * 12 is historical, and due to x86 page size. */
> >> #define VIRTIO_PCI_QUEUE_ADDR_SHIFT 12
> >>
> >> +/* We can catch some guest bugs inside here so we continue supporting
> >> older
> >> + guests. */
> >> +#define VIRTIO_PCI_BUG_BUS_MASTER (1 << 0)
> >> +
> >> /* QEMU doesn't strictly need write barriers since everything runs in
> >> * lock-step. We'll leave the calls to wmb() in though to make it obvious
> >> for
> >> * KVM or if kqemu gets SMP support.
> >> @@ -87,6 +91,7 @@
> >> typedef struct {
> >> PCIDevice pci_dev;
> >> VirtIODevice *vdev;
> >> + uint32_t bugs;
> >> uint32_t addr;
> >> uint32_t class_code;
> >> uint32_t nvectors;
> >> @@ -138,6 +143,13 @@ static int virtio_pci_load_config(void * opaque,
> >> QEMUFile *f)
> >> if (proxy->vdev->config_vector != VIRTIO_NO_VECTOR) {
> >> return msix_vector_use(&proxy->pci_dev,
> >> proxy->vdev->config_vector);
> >> }
> >> +
> >> + /* Try to find out if the guest has bus master disabled, but is
> >> + in ready state. Then we have a buggy guest OS. */
> >> + if (!(proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK) &&
> >
> > should not this be (proxy->vdev->status & VIRTIO_CONFIG_S_DRIVER_OK)?
>
> Yikes. Of course.
>
> >
> >> + !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> >> + proxy->bugs |= VIRTIO_PCI_BUG_BUS_MASTER;
> >> + }
> >> return 0;
> >> }
> >>
> >> @@ -162,6 +174,7 @@ static void virtio_pci_reset(DeviceState *d)
> >> VirtIOPCIProxy *proxy = container_of(d, VirtIOPCIProxy, pci_dev.qdev);
> >> virtio_reset(proxy->vdev);
> >> msix_reset(&proxy->pci_dev);
> >> + proxy->bugs = 0;
> >> }
> >>
> >> static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
> >> @@ -205,6 +218,14 @@ static void virtio_ioport_write(void *opaque,
> >> uint32_t addr, uint32_t val)
> >> virtio_reset(proxy->vdev);
> >> msix_unuse_all_vectors(&proxy->pci_dev);
> >> }
> >> +
> >> + /* Linux before 2.6.34 sets the device as OK without enabling
> >> + the PCI device bus master bit. In this case we need to disable
> >> + some safety checks. */
> >> + if ((val & VIRTIO_CONFIG_S_DRIVER_OK) &&
> >> + !(proxy->pci_dev.config[PCI_COMMAND] & PCI_COMMAND_MASTER)) {
> >> + proxy->bugs |= VIRTIO_PCI_BUG_BUS_MASTER;
> >> + }
> >> break;
> >> case VIRTIO_MSI_CONFIG_VECTOR:
> >> msix_vector_unuse(&proxy->pci_dev, vdev->config_vector);
> >> @@ -372,7 +393,9 @@ static void virtio_write_config(PCIDevice *pci_dev,
> >> uint32_t address,
> >>
> >> if (PCI_COMMAND == address) {
> >> if (!(val & PCI_COMMAND_MASTER)) {
> >> - proxy->vdev->status &= ~VIRTIO_CONFIG_S_DRIVER_OK;
> >> + if (!(proxy->bugs & VIRTIO_PCI_BUG_BUS_MASTER)) {
> >
> > nested if statements are confusing
> >
> > if (!(val & PCI_COMMAND_MASTER) &&
> > !(proxy->bugs & VIRTIO_PCI_BUG_BUS_MASTER))
> >
> > would be clearer.
>
> While I agree in general, I figured I'd try to be as little intrusive as
> possible. And I didn't really want to do different patches for -stable and
> master.
>
>
> Alex
Roll the master patch, Anthony or I can do -stable.
--
MST