Skip to contentSkip to conversation

kvm tools: PCI -- Make PCI device numbers being unique

PATCH In review Cyrill Gorcunov 3 participants 6 messages May 8, 2011 – May 9, 2011 KVM

lore
Share & tools
lore

Cyrill Gorcunov

author
#0 lore raw

PATCH 1/1 kvm tools: PCI -- Make PCI device numbers being unique

PCI device numbers must be unique on a bus (as a part of Bus/Device/Function tuple). Make it so. Note the patch is rather a fast fix since we need a bit more smart pci device manager (in particular multiple virtio block devices most probably should lay on a separate pci bus).

Signed-off-by Cyrill Gorcunov <[email protected]>
---
 tools/kvm/include/kvm/virtio-pci-dev.h |    5 +++++
 tools/kvm/virtio/blk.c                 |    2 +-
 tools/kvm/virtio/console.c             |    2 +-
 tools/kvm/virtio/net.c                 |    2 +-
 tools/kvm/virtio/rng.c                 |    2 +-
 5 files changed, 9 insertions(+), 4 deletions(-)
tools/kvm/include/kvm/virtio-pci-dev.h +5 -0
Index: linux-2.6.git/tools/kvm/include/kvm/virtio-pci-dev.h
=====================================================================
@@ -16,4 +16,9 @@
 #define PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE		0x0003
 #define PCI_SUBSYSTEM_ID_VIRTIO_RNG		0x0004
 
+#define PCI_DEVICE_VIRTIO_NET			0x2
+#define PCI_DEVICE_VIRTIO_BLK			0x1
+#define PCI_DEVICE_VIRTIO_CONSOLE		0x3
+#define PCI_DEVICE_VIRTIO_RNG			0x4
+
 #endif /* VIRTIO_PCI_DEV_H_ */
tools/kvm/virtio/blk.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/blk.c
=====================================================================
@@ -295,7 +295,7 @@ void virtio_blk__init(struct kvm *self,
 		},
 	};
 
-	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_BLK, &dev, &pin, &line) < 0)
+	if (irq__register_device(PCI_DEVICE_VIRTIO_BLK, &dev, &pin, &line) < 0)
 		return;
 
 	bdev->pci_hdr.irq_pin	= pin;
tools/kvm/virtio/console.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/console.c
=====================================================================
@@ -238,7 +238,7 @@ void virtio_console__init(struct kvm *se
 {
 	u8 dev, line, pin;
 
-	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_CONSOLE, &dev, &pin, &line) < 0)
+	if (irq__register_device(PCI_DEVICE_VIRTIO_CONSOLE, &dev, &pin, &line) < 0)
 		return;
 
 	virtio_console_pci_device.irq_pin	= pin;
tools/kvm/virtio/net.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/net.c
=====================================================================
@@ -385,7 +385,7 @@ void virtio_net__init(const struct virti
 	if (virtio_net__tap_init(params)) {
 		u8 dev, line, pin;
 
-		if (irq__register_device(PCI_DEVICE_ID_VIRTIO_NET, &dev, &pin, &line) < 0)
+		if (irq__register_device(PCI_DEVICE_VIRTIO_NET, &dev, &pin, &line) < 0)
 			return;
 
 		virtio_net_pci_device.irq_pin	= pin;
tools/kvm/virtio/rng.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/rng.c
=====================================================================
@@ -178,5 +178,5 @@ void virtio_rng__init(struct kvm *kvm)
 	virtio_rng_pci_device.irq_line	= line;
 	pci__register(&virtio_rng_pci_device, dev);
 
-	ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
+	ioport__register(PCI_DEVICE_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
 }
#1 lore raw

tools/kvm/virtio/rng.c void virtio_rng__init(struct kvm *kvm) · line 182

Quoted 11 lines · Cyrill Gorcunov

Index: linux-2.6.git/tools/kvm/virtio/rng.c
=====================================================================

tools/kvm/virtio/rng.c +1 -1
@@ -178,5 +178,5 @@ void virtio_rng__init(struct kvm *kvm)
 	virtio_rng_pci_device.irq_line	= line;
 	pci__register(&virtio_rng_pci_device, dev);
 
-	ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
+	ioport__register(PCI_DEVICE_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
 }

I think you wanted to change irq__register_device, not ioport__register in virtio-rng.

Signature · 1 line
Sasha.
author
#2 lore raw
Quoted 9 lines · Sasha Levin
Quoted 8 lines · Cyrill Gorcunov

Index: linux-2.6.git/tools/kvm/virtio/rng.c
=====================================================================

tools/kvm/virtio/rng.c +0 -0
@@ -178,5 +178,5 @@ void virtio_rng__init(struct kvm *kvm)
 	virtio_rng_pci_device.irq_line	= line;
 	pci__register(&virtio_rng_pci_device, dev);
Quoted 7 lines

tools/kvm/virtio/rng.c void virtio_rng__init(struct kvm *kvm) · line 182

Quoted 3 lines
tools/kvm/virtio/rng.c +1 -1
-	ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
+	ioport__register(PCI_DEVICE_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
 }

I think you wanted to change irq__register_device, not ioport__register in virtio-rng.

Good catch Sasha, thanks!

Signature · 1 line
            Cyrill
author
#3 lore raw
Quoted 18 lines · Cyrill Gorcunov
Quoted 9 lines · Sasha Levin
Quoted 8 lines · Cyrill Gorcunov

Index: linux-2.6.git/tools/kvm/virtio/rng.c
=====================================================================

tools/kvm/virtio/rng.c +0 -0
@@ -178,5 +178,5 @@ void virtio_rng__init(struct kvm *kvm)
 	virtio_rng_pci_device.irq_line	= line;
 	pci__register(&virtio_rng_pci_device, dev);
Quoted 7 lines

tools/kvm/virtio/rng.c void virtio_rng__init(struct kvm *kvm) · line 182

Quoted 3 lines
tools/kvm/virtio/rng.c +1 -1
-	ioport__register(IOPORT_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
+	ioport__register(PCI_DEVICE_VIRTIO_RNG, &virtio_rng_io_ops, IOPORT_VIRTIO_RNG_SIZE);
 }

I think you wanted to change irq__register_device, not ioport__register in virtio-rng.

Quoted 2 lines

Good catch Sasha, thanks!

This one should go better.
---
From: Cyrill Gorcunov <[email protected]>
Subject: [PATCH] kvm tools: PCI -- Make PCI device numbers being unique v2

PCI device numbers must be unique on a bus (as a part of Bus/Device/Function tuple).Make it so. Note the patch is rather a fast fix since we need a bit more smart pci device manager (in particular multiple virtio block devices most probably should lay on a separate pci bus).

v2: Sasha spotted the nit in virtio_rng__init, ioport
function was touched insted of irq__register_device.

Signed-off-by Cyrill Gorcunov <[email protected]>
CC Sasha Levin <[email protected]>
---
 tools/kvm/include/kvm/virtio-pci-dev.h |    5 +++++
 tools/kvm/virtio/blk.c                 |    2 +-
 tools/kvm/virtio/console.c             |    2 +-
 tools/kvm/virtio/net.c                 |    2 +-
 tools/kvm/virtio/rng.c                 |    2 +-
 5 files changed, 9 insertions(+), 4 deletions(-)
tools/kvm/include/kvm/virtio-pci-dev.h +5 -0
Index: linux-2.6.git/tools/kvm/include/kvm/virtio-pci-dev.h
=====================================================================
@@ -16,4 +16,9 @@
 #define PCI_SUBSYSTEM_ID_VIRTIO_CONSOLE		0x0003
 #define PCI_SUBSYSTEM_ID_VIRTIO_RNG		0x0004
 
+#define PCI_DEVICE_VIRTIO_NET			0x2
+#define PCI_DEVICE_VIRTIO_BLK			0x1
+#define PCI_DEVICE_VIRTIO_CONSOLE		0x3
+#define PCI_DEVICE_VIRTIO_RNG			0x4
+
 #endif /* VIRTIO_PCI_DEV_H_ */
tools/kvm/virtio/blk.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/blk.c
=====================================================================
@@ -295,7 +295,7 @@ void virtio_blk__init(struct kvm *self,
 		},
 	};
 
-	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_BLK, &dev, &pin, &line) < 0)
+	if (irq__register_device(PCI_DEVICE_VIRTIO_BLK, &dev, &pin, &line) < 0)
 		return;
 
 	bdev->pci_hdr.irq_pin	= pin;
tools/kvm/virtio/console.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/console.c
=====================================================================
@@ -238,7 +238,7 @@ void virtio_console__init(struct kvm *se
 {
 	u8 dev, line, pin;
 
-	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_CONSOLE, &dev, &pin, &line) < 0)
+	if (irq__register_device(PCI_DEVICE_VIRTIO_CONSOLE, &dev, &pin, &line) < 0)
 		return;
 
 	virtio_console_pci_device.irq_pin	= pin;
tools/kvm/virtio/net.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/net.c
=====================================================================
@@ -385,7 +385,7 @@ void virtio_net__init(const struct virti
 	if (virtio_net__tap_init(params)) {
 		u8 dev, line, pin;
 
-		if (irq__register_device(PCI_DEVICE_ID_VIRTIO_NET, &dev, &pin, &line) < 0)
+		if (irq__register_device(PCI_DEVICE_VIRTIO_NET, &dev, &pin, &line) < 0)
 			return;
 
 		virtio_net_pci_device.irq_pin	= pin;
tools/kvm/virtio/rng.c +1 -1
Index: linux-2.6.git/tools/kvm/virtio/rng.c
=====================================================================
@@ -171,7 +171,7 @@ void virtio_rng__init(struct kvm *kvm)
 	if (rdev.fd < 0)
 		die("Failed initializing RNG");
 
-	if (irq__register_device(PCI_DEVICE_ID_VIRTIO_RNG, &dev, &pin, &line) < 0)
+	if (irq__register_device(PCI_DEVICE_VIRTIO_RNG, &dev, &pin, &line) < 0)
 		return;
 
 	virtio_rng_pci_device.irq_pin	= pin;
#4 lore raw
Quoted 11 lines

From: Cyrill Gorcunov <[email protected]>
Subject: [PATCH] kvm tools: PCI -- Make PCI device numbers being unique v2

PCI device numbers must be unique on a bus (as a part of Bus/Device/Function tuple).Make it so. Note the patch is rather a fast fix since we need a bit more smart pci device manager (in particular multiple virtio block devices most probably should lay on a separate pci bus).

v2: Sasha spotted the nit in virtio_rng__init, ioport
function was touched insted of irq__register_device.

Hey, I don't like the new patch subject trend you're trying to start at all. You can make it

kvm tools,pci: Make PCI device numbers unique

but in this particular case "PCI" already appears in the title so

kmv tools: Make PCI device numbers unique

is the right thing to do.

In addition, the changelog doesn't really tell me much. Does it fix something? Why would we need a "smart pci device manager" and why is that relevant for this patch? Hmmh?

Pekka

author
#5 lore raw
Quoted 14 lines · Pekka Enberg
Quoted 11 lines

From: Cyrill Gorcunov <[email protected]>
Subject: [PATCH] kvm tools: PCI -- Make PCI device numbers being unique v2

PCI device numbers must be unique on a bus (as a part of Bus/Device/Function tuple).Make it so. Note the patch is rather a fast fix since we need a bit more smart pci device manager (in particular multiple virtio block devices most probably should lay on a separate pci bus).

v2: Sasha spotted the nit in virtio_rng__init, ioport
function was touched insted of irq__register_device.

Hey, I don't like the new patch subject trend you're trying to start at all. You can make it

Quoted 7 lines

kvm tools,pci: Make PCI device numbers unique

but in this particular case "PCI" already appears in the title so

kmv tools: Make PCI device numbers unique

is the right thing to do.

PCI stands for kvm tools subsystem, but if you like more the last -- I'm fine with it.

Quoted 6 lines

In addition, the changelog doesn't really tell me much. Does it fix something? Why would we need a "smart pci device manager" and why is that relevant for this patch? Hmmh?

Pekka

The thing is that at moment the id's passed to MP table is incorrect, they are to be 5 bits long (mp spec). The smart manager we need -- it's because there could be multiple virtio block device and they _are_ to be separated pci devices, ie with own numbers and intx# assignents. As result we probably should have such virtio devices to lay on a separate pci bus, or if the number of pci devices exceed the width of address line then we should pass them to another pci bus. That is what I had in mind but I'm not sure all this should come to the changelog.

Signature · 1 line
            Cyrill