[Qemu-trivial] [PATCH] Remove PCI class code from virtio balloon device

[Qemu-trivial] [PATCH] Remove PCI class code from virtio balloon device

[David Gibson]

Currently the virtio balloon device, when using the virtio-pci interface
advertises itself with PCI class code MEMORY_RAM.  This is wrong; the
balloon is vaguely related to memory, but is nothing like a PCI memory
device in the meaning of the class code, and this code is not required or
suggested by the virtio PCI specification.

Worse, this patch causes problems on the pseries machine, because the
firmware, seeing this class code, advertises the device as memory in the
device tree, and then a guest kernel bug causes it to see this "memory"
before the real system memory, leading to a crash in early boot.

This patch fixes the problem by removing the bogus PCI class code on the
balloon device.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/virtio-pci.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index a0fb7c1..3c3907a 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -919,7 +919,6 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
     k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
     k->revision = VIRTIO_PCI_ABI_VERSION;
-    k->class_id = PCI_CLASS_MEMORY_RAM;
     dc->reset = virtio_pci_reset;
     dc->props = virtio_balloon_properties;
 }
-- 
1.7.9.1

Re: [Qemu-trivial] [PATCH] Remove PCI class code from virtio balloon device

[Michael S. Tsirkin]

On Fri, Mar 16, 2012 at 12:03:08PM +1100, David Gibson wrote:
> Currently the virtio balloon device, when using the virtio-pci interface
> advertises itself with PCI class code MEMORY_RAM.  This is wrong; the
> balloon is vaguely related to memory, but is nothing like a PCI memory
> device in the meaning of the class code, and this code is not required or
> suggested by the virtio PCI specification.
> 
> Worse, this patch causes problems on the pseries machine, because the
> firmware, seeing this class code, advertises the device as memory in the
> device tree, and then a guest kernel bug causes it to see this "memory"
> before the real system memory, leading to a crash in early boot.
> 
> This patch fixes the problem by removing the bogus PCI class code on the
> balloon device.
> 
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Rusty Russell <rusty@rustcorp.com.au>
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Well, this gives the class a legacy value 00,
and the PCI spec says we should not use this:

D.1.
Base Class 00h
This base class is defined to provide backward compatibility for devices
that were built
before the Class Code field was defined. No new devices should use this
value and existing
devices should switch to a more appropriate value if possible.
For class codes with this base class value, there are two defined values
for the remaining
fields as shown in the table below. All other values are reserved.
Base Class
00h
Sub-Class
Interface
00h
01h
VGA-compatible device
00h
00h
All currently implemented devices
except VGA-compatible devices

You probably want this instead:

#define PCI_CLASS_OTHERS		0xff


> ---
>  hw/virtio-pci.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
> index a0fb7c1..3c3907a 100644
> --- a/hw/virtio-pci.c
> +++ b/hw/virtio-pci.c
> @@ -919,7 +919,6 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
>      k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
>      k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
>      k->revision = VIRTIO_PCI_ABI_VERSION;
> -    k->class_id = PCI_CLASS_MEMORY_RAM;
>      dc->reset = virtio_pci_reset;
>      dc->props = virtio_balloon_properties;
>  }
> -- 
> 1.7.9.1

Re: [Qemu-trivial] [PATCH] Remove PCI class code from virtio balloon device

[David Gibson]

On Sun, Mar 18, 2012 at 02:38:42PM +0200, Michael S. Tsirkin wrote:
> On Fri, Mar 16, 2012 at 12:03:08PM +1100, David Gibson wrote:
> > Currently the virtio balloon device, when using the virtio-pci interface
> > advertises itself with PCI class code MEMORY_RAM.  This is wrong; the
> > balloon is vaguely related to memory, but is nothing like a PCI memory
> > device in the meaning of the class code, and this code is not required or
> > suggested by the virtio PCI specification.
> > 
> > Worse, this patch causes problems on the pseries machine, because the
> > firmware, seeing this class code, advertises the device as memory in the
> > device tree, and then a guest kernel bug causes it to see this "memory"
> > before the real system memory, leading to a crash in early boot.
> > 
> > This patch fixes the problem by removing the bogus PCI class code on the
> > balloon device.
> > 
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Rusty Russell <rusty@rustcorp.com.au>
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> 
> Well, this gives the class a legacy value 00,
> and the PCI spec says we should not use this:
> 
> D.1.
> Base Class 00h
> This base class is defined to provide backward compatibility for devices
> that were built
> before the Class Code field was defined. No new devices should use this
> value and existing
> devices should switch to a more appropriate value if possible.
> For class codes with this base class value, there are two defined values
> for the remaining
> fields as shown in the table below. All other values are reserved.
> Base Class
> 00h
> Sub-Class
> Interface
> 00h
> 01h
> VGA-compatible device
> 00h
> 00h
> All currently implemented devices
> except VGA-compatible devices
> 
> You probably want this instead:
> 
> #define PCI_CLASS_OTHERS		0xff

Ah, thanks.  I had mistakenly assumed that 0 meant "no class
specified".  I'll respin changing to PCI_CLASS_OTHERS.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[Qemu-trivial] [PATCH] Remove PCI class code from virtio balloon device

[David Gibson]

Currently the virtio balloon device, when using the virtio-pci interface
advertises itself with PCI class code MEMORY_RAM.  This is wrong; the
balloon is vaguely related to memory, but is nothing like a PCI memory
device in the meaning of the class code, and this code is not required or
suggested by the virtio PCI specification.

Worse, this patch causes problems on the pseries machine, because the
firmware, seeing this class code, advertises the device as memory in the
device tree, and then a guest kernel bug causes it to see this "memory"
before the real system memory, leading to a crash in early boot.

This patch fixes the problem by removing the bogus PCI class code on the
balloon device.

Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 hw/virtio-pci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index a0fb7c1..da8a382 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -919,7 +919,7 @@ static void virtio_balloon_class_init(ObjectClass *klass, void *data)
     k->vendor_id = PCI_VENDOR_ID_REDHAT_QUMRANET;
     k->device_id = PCI_DEVICE_ID_VIRTIO_BALLOON;
     k->revision = VIRTIO_PCI_ABI_VERSION;
-    k->class_id = PCI_CLASS_MEMORY_RAM;
+    k->class_id = PCI_CLASS_OTHERS;
     dc->reset = virtio_pci_reset;
     dc->props = virtio_balloon_properties;
 }
-- 
1.7.9.1