Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
On Wed, May 18, 2011 at 01:55:17AM +0900, Isaku Yamahata wrote:
vender id/device id... in pci configuration space are read-only registers
which are commonly defined for all pci devices.
So initialize them in common code and it simplifies the initialization a bit.
I didn't converted virtio-pci and qxl because it determines ids dynaically.
So I'll leave those conversion (or not to convert) to the authors.
Hmm, virtio doesn't:
static PCIDeviceInfo virtio_info[] = {
...
}
has the array of devices.
qxl has a property to convert device id to 'development'
and to tweak revision through a qdev property.
Gerd, could you comment on why this is useful?
--
MST
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
Hi, qxl has a property to convert device id to 'development' and to tweak revision through a qdev property. Gerd, could you comment on why this is useful? I think we can zap this, was in use for a while one year ago, but in the end it turned out we can keep qxl compatible to older versions and we've ended up just bumping the pci revision (which needs to stay configurable because there are picky guest drivers in the wild which refuse to work if they find the revision being 2 instead of 1). cheers, Gerd
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
On Wed, May 18, 2011 at 12:17:46PM +0300, Michael S. Tsirkin wrote:
On Wed, May 18, 2011 at 01:55:17AM +0900, Isaku Yamahata wrote:
vender id/device id... in pci configuration space are read-only registers
which are commonly defined for all pci devices.
So initialize them in common code and it simplifies the initialization a
bit.
I didn't converted virtio-pci and qxl because it determines ids dynaically.
So I'll leave those conversion (or not to convert) to the authors.
Hmm, virtio doesn't:
static PCIDeviceInfo virtio_info[] = {
...
}
has the array of devices.
Okay. I missed it somehow. I get the following, And I'll leave
qxl convection to Gerd.
The remaining issue is, should I adopt/drop prog_interface conversion?
From c9834234c73eb03d764a3c999cbd34f4814a5553 Mon Sep 17 00:00:00 2001
Message-Id:
c9834234c73eb03d764a3c999cbd34f4814a5553.1305715603.git.yamah...@valinux.co.jp
In-Reply-To: cover.1305715602.git.yamah...@valinux.co.jp
References: cover.1305715602.git.yamah...@valinux.co.jp
From: Isaku Yamahata yamah...@valinux.co.jp
Date: Wed, 18 May 2011 19:46:04 +0900
Subject: [PATCH] virtio-pci.c: convert to PCIDEviceInfo to initialize ids
use PCIDeviceInfo to initialize ids.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
hw/virtio-pci.c | 69 --
1 files changed, 31 insertions(+), 38 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c19629d..270e2c7 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -669,9 +669,7 @@ static const VirtIOBindings virtio_pci_bindings = {
.vmstate_change = virtio_pci_vmstate_change,
};
-static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
-uint16_t vendor, uint16_t device,
-uint16_t class_code, uint8_t pif)
+static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
{
uint8_t *config;
uint32_t size;
@@ -679,19 +677,12 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy,
VirtIODevice *vdev,
proxy-vdev = vdev;
config = proxy-pci_dev.config;
-pci_config_set_vendor_id(config, vendor);
-pci_config_set_device_id(config, device);
-
-config[0x08] = VIRTIO_PCI_ABI_VERSION;
-
-config[0x09] = pif;
-pci_config_set_class(config, class_code);
-
-config[0x2c] = vendor 0xFF;
-config[0x2d] = (vendor 8) 0xFF;
-config[0x2e] = vdev-device_id 0xFF;
-config[0x2f] = (vdev-device_id 8) 0xFF;
+if (proxy-class_code) {
+pci_config_set_class(config, proxy-class_code);
+}
+pci_set_word(config + 0x2c, pci_get_word(config + PCI_VENDOR_ID));
+pci_set_word(config + 0x2e, vdev-device_id);
config[0x3d] = 1;
if (vdev-nvectors !msix_init(proxy-pci_dev, vdev-nvectors, 1, 0)) {
@@ -735,10 +726,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
return -1;
}
vdev-nvectors = proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-PCI_DEVICE_ID_VIRTIO_BLOCK,
-proxy-class_code, 0x00);
+virtio_init_pci(proxy, vdev);
/* make the actual value visible */
proxy-nvectors = vdev-nvectors;
return 0;
@@ -776,10 +764,7 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
vdev-nvectors = proxy-nvectors == DEV_NVECTORS_UNSPECIFIED
? proxy-serial.max_virtserial_ports +
1
: proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-PCI_DEVICE_ID_VIRTIO_CONSOLE,
-proxy-class_code, 0x00);
+virtio_init_pci(proxy, vdev);
proxy-nvectors = vdev-nvectors;
return 0;
}
@@ -801,11 +786,7 @@ static int virtio_net_init_pci(PCIDevice *pci_dev)
vdev = virtio_net_init(pci_dev-qdev, proxy-nic, proxy-net);
vdev-nvectors = proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-PCI_DEVICE_ID_VIRTIO_NET,
-PCI_CLASS_NETWORK_ETHERNET,
-0x00);
+virtio_init_pci(proxy, vdev);
/* make the actual value visible */
proxy-nvectors = vdev-nvectors;
@@ -827,11 +808,7 @@ static int virtio_balloon_init_pci(PCIDevice *pci_dev)
VirtIODevice *vdev;
vdev = virtio_balloon_init(pci_dev-qdev);
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-PCI_DEVICE_ID_VIRTIO_BALLOON,
-PCI_CLASS_MEMORY_RAM,
-0x00);
+virtio_init_pci(proxy, vdev);
return 0;
}
@@ -843,11 +820,7 @@ static int virtio_9p_init_pci(PCIDevice *pci_dev)
vdev = virtio_9p_init(pci_dev-qdev, proxy-fsconf);
vdev-nvectors = proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
On Wed, May 18, 2011 at 07:55:17PM +0900, Isaku Yamahata wrote:
On Wed, May 18, 2011 at 12:17:46PM +0300, Michael S. Tsirkin wrote:
On Wed, May 18, 2011 at 01:55:17AM +0900, Isaku Yamahata wrote:
vender id/device id... in pci configuration space are read-only registers
which are commonly defined for all pci devices.
So initialize them in common code and it simplifies the initialization a
bit.
I didn't converted virtio-pci and qxl because it determines ids
dynaically.
So I'll leave those conversion (or not to convert) to the authors.
Hmm, virtio doesn't:
static PCIDeviceInfo virtio_info[] = {
...
}
has the array of devices.
Okay. I missed it somehow. I get the following, And I'll leave
qxl convection to Gerd.
The remaining issue is, should I adopt/drop prog_interface conversion?
Drop it I think. I don't see what it buys us.
The point with all this work IMO is to be able
to sort devices by type and
to show device vendor/type/revision in -help as well as
to identify device by vendor/device/revision
instead of the arbitrary qemu names on the monitor.
We can use numeric values, as well as parse pci.ids
if present on system for symbolic ones.
revision is sometimes 0 and initialized later by devices,
so if it's 0 we don't really know what it is, but it's
a bit less important I guess, so while I'm not 100%
we should have it in the table, I'm not sure we shouldn't either.
However none of this seems to apply to prog_interface which is
useful for the OS but not that useful for the user.
From c9834234c73eb03d764a3c999cbd34f4814a5553 Mon Sep 17 00:00:00 2001
Message-Id:
c9834234c73eb03d764a3c999cbd34f4814a5553.1305715603.git.yamah...@valinux.co.jp
In-Reply-To: cover.1305715602.git.yamah...@valinux.co.jp
References: cover.1305715602.git.yamah...@valinux.co.jp
From: Isaku Yamahata yamah...@valinux.co.jp
Date: Wed, 18 May 2011 19:46:04 +0900
Subject: [PATCH] virtio-pci.c: convert to PCIDEviceInfo to initialize ids
use PCIDeviceInfo to initialize ids.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
hw/virtio-pci.c | 69 --
1 files changed, 31 insertions(+), 38 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index c19629d..270e2c7 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -669,9 +669,7 @@ static const VirtIOBindings virtio_pci_bindings = {
.vmstate_change = virtio_pci_vmstate_change,
};
-static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
-uint16_t vendor, uint16_t device,
-uint16_t class_code, uint8_t pif)
+static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev)
{
uint8_t *config;
uint32_t size;
@@ -679,19 +677,12 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy,
VirtIODevice *vdev,
proxy-vdev = vdev;
config = proxy-pci_dev.config;
-pci_config_set_vendor_id(config, vendor);
-pci_config_set_device_id(config, device);
-
-config[0x08] = VIRTIO_PCI_ABI_VERSION;
-
-config[0x09] = pif;
-pci_config_set_class(config, class_code);
-
-config[0x2c] = vendor 0xFF;
-config[0x2d] = (vendor 8) 0xFF;
-config[0x2e] = vdev-device_id 0xFF;
-config[0x2f] = (vdev-device_id 8) 0xFF;
+if (proxy-class_code) {
+pci_config_set_class(config, proxy-class_code);
+}
+pci_set_word(config + 0x2c, pci_get_word(config + PCI_VENDOR_ID));
+pci_set_word(config + 0x2e, vdev-device_id);
config[0x3d] = 1;
if (vdev-nvectors !msix_init(proxy-pci_dev, vdev-nvectors, 1, 0))
{
@@ -735,10 +726,7 @@ static int virtio_blk_init_pci(PCIDevice *pci_dev)
return -1;
}
vdev-nvectors = proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-PCI_DEVICE_ID_VIRTIO_BLOCK,
-proxy-class_code, 0x00);
+virtio_init_pci(proxy, vdev);
/* make the actual value visible */
proxy-nvectors = vdev-nvectors;
return 0;
@@ -776,10 +764,7 @@ static int virtio_serial_init_pci(PCIDevice *pci_dev)
vdev-nvectors = proxy-nvectors == DEV_NVECTORS_UNSPECIFIED
? proxy-serial.max_virtserial_ports
+ 1
: proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-PCI_DEVICE_ID_VIRTIO_CONSOLE,
-proxy-class_code, 0x00);
+virtio_init_pci(proxy, vdev);
proxy-nvectors = vdev-nvectors;
return 0;
}
@@ -801,11 +786,7 @@ static int virtio_net_init_pci(PCIDevice *pci_dev)
vdev = virtio_net_init(pci_dev-qdev, proxy-nic, proxy-net);
vdev-nvectors = proxy-nvectors;
-virtio_init_pci(proxy, vdev,
-PCI_VENDOR_ID_REDHAT_QUMRANET,
-
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
On Wed, May 18, 2011 at 07:55:17PM +0900, Isaku Yamahata wrote:
On Wed, May 18, 2011 at 12:17:46PM +0300, Michael S. Tsirkin wrote:
On Wed, May 18, 2011 at 01:55:17AM +0900, Isaku Yamahata wrote:
vender id/device id... in pci configuration space are read-only registers
which are commonly defined for all pci devices.
So initialize them in common code and it simplifies the initialization a
bit.
I didn't converted virtio-pci and qxl because it determines ids
dynaically.
So I'll leave those conversion (or not to convert) to the authors.
Hmm, virtio doesn't:
static PCIDeviceInfo virtio_info[] = {
...
}
has the array of devices.
Okay. I missed it somehow. I get the following, And I'll leave
qxl convection to Gerd.
Well the devel device id can go it seems, but we
need think what to do with the revision.
Maybe it's not so terrible that revision will
be wrong in -help for qxl - any better ideas?
--
MST
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
Hi, Well the devel device id can go it seems, but we need think what to do with the revision. Maybe it's not so terrible that revision will be wrong in -help for qxl - any better ideas? Fine with me. Default is 2 and it can be set to 1 using rev=1 property if needed. Help should show rev 2. cheers, Gerd
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
On Wed, May 18, 2011 at 03:07:21PM +0200, Gerd Hoffmann wrote: Hi, Well the devel device id can go it seems, but we need think what to do with the revision. Maybe it's not so terrible that revision will be wrong in -help for qxl - any better ideas? Fine with me. Default is 2 and it can be set to 1 using rev=1 property if needed. Help should show rev 2. cheers, Gerd I think it's easier to not show the revision ...
Re: [Qemu-devel] [PATCH v2 00/38] pci: initialize ids in pci common code
Here is the prog_interface part.
You were unsure about prog_interface, so I split it out.
Thus you can determine if prog_interface conversion is wanted or not.
From abaf67175190c2f4d0c222c2ae8010e9de38bf59 Mon Sep 17 00:00:00 2001
Message-Id:
abaf67175190c2f4d0c222c2ae8010e9de38bf59.1305684983.git.yamah...@valinux.co.jp
From: Isaku Yamahata yamah...@valinux.co.jp
Date: Wed, 18 May 2011 09:58:21 +0900
Subject: [PATCH] pci: initialize prog_interface by common code
Add prog_interface to PCIDeviceInfo and initialize
prog_interface register in the common initialization code.
It's read-only register.
Signed-off-by: Isaku Yamahata yamah...@valinux.co.jp
---
hw/ac97.c|2 --
hw/acpi_piix4.c |1 -
hw/bonito.c |3 ---
hw/grackle_pci.c |2 +-
hw/gt64xxx.c |1 -
hw/ide/cmd646.c |3 +--
hw/ide/ich.c |3 +--
hw/ide/piix.c|5 ++---
hw/ide/via.c |2 +-
hw/pci.c |1 +
hw/pci.h |1 +
hw/sun4u.c |1 -
hw/usb-ohci.c|2 +-
hw/usb-uhci.c|1 -
hw/vt82c686.c|4
15 files changed, 9 insertions(+), 23 deletions(-)
diff --git a/hw/ac97.c b/hw/ac97.c
index bf1d1d4..ec2b928 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -1289,8 +1289,6 @@ static int ac97_initfn (PCIDevice *dev)
c[PCI_STATUS] = PCI_STATUS_FAST_BACK; /* pcists pci status rwc, ro */
c[PCI_STATUS + 1] = PCI_STATUS_DEVSEL_MEDIUM 8;
-c[PCI_CLASS_PROG] = 0x00; /* pi programming interface ro */
-
/* TODO set when bar is registered. no need to override. */
/* nabmar native audio mixer base address rw */
c[PCI_BASE_ADDRESS_0] = PCI_BASE_ADDRESS_SPACE_IO;
diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index 03d833a..4e5674f 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -319,7 +319,6 @@ static int piix4_pm_initfn(PCIDevice *dev)
pci_conf = s-dev.config;
pci_conf[0x06] = 0x80;
pci_conf[0x07] = 0x02;
-pci_conf[0x09] = 0x00;
pci_conf[0x3d] = 0x01; // interrupt pin 1
pci_conf[0x40] = 0x01; /* PM io base read only bit */
diff --git a/hw/bonito.c b/hw/bonito.c
index e8c57a3..d1e6d1f 100644
--- a/hw/bonito.c
+++ b/hw/bonito.c
@@ -690,9 +690,6 @@ static int bonito_initfn(PCIDevice *dev)
{
PCIBonitoState *s = DO_UPCAST(PCIBonitoState, dev, dev);
-/* Bonito North Bridge, built on FPGA, VENDOR_ID/DEVICE_ID are undefined
*/
-pci_config_set_prog_interface(dev-config, 0x00);
-
/* set the north bridge register mapping */
s-bonito_reg_handle = cpu_register_io_memory(bonito_read, bonito_write, s,
DEVICE_NATIVE_ENDIAN);
diff --git a/hw/grackle_pci.c b/hw/grackle_pci.c
index 7a5221c..46ead7b 100644
--- a/hw/grackle_pci.c
+++ b/hw/grackle_pci.c
@@ -104,7 +104,6 @@ static int pci_grackle_init_device(SysBusDevice *dev)
static int grackle_pci_host_init(PCIDevice *d)
{
-d-config[0x09] = 0x01;
return 0;
}
@@ -115,6 +114,7 @@ static PCIDeviceInfo grackle_pci_host_info = {
.vendor_id = PCI_VENDOR_ID_MOTOROLA,
.device_id = PCI_DEVICE_ID_MOTOROLA_MPC106,
.revision = 0x00, // revision
+.prog_interface = 0x01,
.class_id = PCI_CLASS_BRIDGE_HOST,
};
diff --git a/hw/gt64xxx.c b/hw/gt64xxx.c
index 8e1f6a0..54ace8e 100644
--- a/hw/gt64xxx.c
+++ b/hw/gt64xxx.c
@@ -1121,7 +1121,6 @@ static int gt64120_pci_init(PCIDevice *d)
pci_set_word(d-config + PCI_COMMAND, 0);
pci_set_word(d-config + PCI_STATUS,
PCI_STATUS_FAST_BACK | PCI_STATUS_DEVSEL_MEDIUM);
-pci_config_set_prog_interface(d-config, 0);
pci_set_long(d-config + PCI_BASE_ADDRESS_0, 0x0008);
pci_set_long(d-config + PCI_BASE_ADDRESS_1, 0x0108);
pci_set_long(d-config + PCI_BASE_ADDRESS_2, 0x1c00);
diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c
index 56302b5..0ca9767 100644
--- a/hw/ide/cmd646.c
+++ b/hw/ide/cmd646.c
@@ -226,8 +226,6 @@ static int pci_cmd646_ide_initfn(PCIDevice *dev)
qemu_irq *irq;
int i;
-pci_conf[PCI_CLASS_PROG] = 0x8f;
-
pci_conf[0x51] = 0x04; // enable IDE0
if (d-secondary) {
/* XXX: if not enabled, really disable the seconday IDE controller */
@@ -279,6 +277,7 @@ static PCIDeviceInfo cmd646_ide_info[] = {
.vendor_id= PCI_VENDOR_ID_CMD,
.device_id= PCI_DEVICE_ID_CMD_646,
.revision = 0x07, // IDE controller revision
+.prog_intarface = 0x8f,
.class_id = PCI_CLASS_STORAGE_IDE,
.qdev.props = (Property[]) {
DEFINE_PROP_UINT32(secondary, PCIIDEState, secondary, 0),
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index cb1c405..784bd96 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -77,8 +77,6 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
struct AHCIPCIState *d;
d = DO_UPCAST(struct AHCIPCIState, card, dev);
-pci_config_set_prog_interface(d-card.config, AHCI_PROGMODE_MAJOR_REV_1);
-
d-card.config[PCI_CACHE_LINE_SIZE] = 0x08; /*
