[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: |
Alexander Graf |
Subject: |
[Qemu-devel] Re: [PATCH] [Also for STABLE-0.12] Don't check for bus master for old guests |
Date: |
Tue, 16 Mar 2010 21:26:53 +0100 |
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