* [Qemu-devel] [PATCH 0/4] QOM'ify PCIDevices
@ 2017-12-17 20:49 Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 1/4] hw/block/nvme: QOM'ify PCI NVME Philippe Mathieu-Daudé
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-17 20:49 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
Paul Burton, Yongbok Kim, Edgar E . Iglesias, Alistair Francis,
Hervé Poussineau, Keith Busch
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-block, James Hogan
Hi,
In this series we QOM'ify the last few PCI devices still using
PCIDeviceClass->init() by converting this init() to realize(),
Then we finally remove the then PCIDeviceClass->init() method for good.
Regards,
Phil.
Philippe Mathieu-Daudé (4):
hw/block/nvme: QOM'ify PCI NVME
hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge
hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge
hw/pci: remove obsolete PCIDevice->init()
include/hw/pci/pci.h | 1 -
hw/block/nvme.c | 32 +++++++++++++++++---------------
hw/pci-host/piix.c | 31 +++++++++++++++----------------
hw/pci-host/xilinx-pcie.c | 19 +++++++++----------
hw/pci/pci.c | 14 --------------
5 files changed, 41 insertions(+), 56 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/4] hw/block/nvme: QOM'ify PCI NVME
2017-12-17 20:49 [Qemu-devel] [PATCH 0/4] QOM'ify PCIDevices Philippe Mathieu-Daudé
@ 2017-12-17 20:49 ` Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge Philippe Mathieu-Daudé
` (2 subsequent siblings)
3 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-17 20:49 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost, Keith Busch
Cc: Philippe Mathieu-Daudé,
qemu-devel, qemu-block, Kevin Wolf, Max Reitz
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/block/nvme.c | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 441e21ed1f..9c5f898da0 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -920,9 +920,9 @@ static const MemoryRegionOps nvme_cmb_ops = {
},
};
-static int nvme_init(PCIDevice *pci_dev)
+static void nvme_realize(PCIDevice *pci, Error **errp)
{
- NvmeCtrl *n = NVME(pci_dev);
+ NvmeCtrl *n = NVME(pci);
NvmeIdCtrl *id = &n->id_ctrl;
int i;
@@ -931,30 +931,33 @@ static int nvme_init(PCIDevice *pci_dev)
Error *local_err = NULL;
if (!n->conf.blk) {
- return -1;
+ error_setg(errp, "Block device missing");
+ return;
}
bs_size = blk_getlength(n->conf.blk);
if (bs_size < 0) {
- return -1;
+ error_setg_errno(errp, -bs_size, "Could not get length of device");
+ return;
}
blkconf_serial(&n->conf, &n->serial);
if (!n->serial) {
- return -1;
+ error_setg(errp, "Could not get device serial number");
+ return;
}
blkconf_blocksizes(&n->conf);
blkconf_apply_backend_options(&n->conf, blk_is_read_only(n->conf.blk),
false, &local_err);
if (local_err) {
- error_report_err(local_err);
- return -1;
+ error_propagate(errp, local_err);
+ return;
}
- pci_conf = pci_dev->config;
+ pci_conf = pci->config;
pci_conf[PCI_INTERRUPT_PIN] = 1;
- pci_config_set_prog_interface(pci_dev->config, 0x2);
- pci_config_set_class(pci_dev->config, PCI_CLASS_STORAGE_EXPRESS);
+ pci_config_set_prog_interface(pci->config, 0x2);
+ pci_config_set_class(pci->config, PCI_CLASS_STORAGE_EXPRESS);
pcie_endpoint_cap_init(&n->parent_obj, 0x80);
n->num_namespaces = 1;
@@ -1046,12 +1049,11 @@ static int nvme_init(PCIDevice *pci_dev)
cpu_to_le64(n->ns_size >>
id_ns->lbaf[NVME_ID_NS_FLBAS_INDEX(ns->id_ns.flbas)].ds);
}
- return 0;
}
-static void nvme_exit(PCIDevice *pci_dev)
+static void nvme_exit(PCIDevice *pci)
{
- NvmeCtrl *n = NVME(pci_dev);
+ NvmeCtrl *n = NVME(pci);
nvme_clear_ctrl(n);
g_free(n->namespaces);
@@ -1061,7 +1063,7 @@ static void nvme_exit(PCIDevice *pci_dev)
memory_region_unref(&n->ctrl_mem);
}
- msix_uninit_exclusive_bar(pci_dev);
+ msix_uninit_exclusive_bar(pci);
}
static Property nvme_props[] = {
@@ -1081,7 +1083,7 @@ static void nvme_class_init(ObjectClass *oc, void *data)
DeviceClass *dc = DEVICE_CLASS(oc);
PCIDeviceClass *pc = PCI_DEVICE_CLASS(oc);
- pc->init = nvme_init;
+ pc->realize = nvme_realize;
pc->exit = nvme_exit;
pc->class_id = PCI_CLASS_STORAGE_EXPRESS;
pc->vendor_id = PCI_VENDOR_ID_INTEL;
--
2.15.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge
2017-12-17 20:49 [Qemu-devel] [PATCH 0/4] QOM'ify PCIDevices Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 1/4] hw/block/nvme: QOM'ify PCI NVME Philippe Mathieu-Daudé
@ 2017-12-17 20:49 ` Philippe Mathieu-Daudé
2017-12-18 7:05 ` Marcel Apfelbaum
2017-12-17 20:49 ` [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe " Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init() Philippe Mathieu-Daudé
3 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-17 20:49 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
Hervé Poussineau
Cc: Philippe Mathieu-Daudé, qemu-devel, qemu-block
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/pci-host/piix.c | 31 +++++++++++++++----------------
1 file changed, 15 insertions(+), 16 deletions(-)
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index a684a7cca9..0153f32a32 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -804,7 +804,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
{0xa8, 4}, /* SNB: base of GTT stolen memory */
};
-static int host_pci_config_read(int pos, int len, uint32_t *val)
+static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp)
{
char path[PATH_MAX];
int config_fd;
@@ -812,19 +812,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val)
/* Access real host bridge. */
int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
0, 0, 0, 0, "config");
- int ret = 0;
if (rc >= size || rc < 0) {
- return -ENODEV;
+ error_setg(&error_abort, "Invalid PCI device");
}
config_fd = open(path, O_RDWR);
if (config_fd < 0) {
- return -ENODEV;
+ error_setg_errno(errp, errno, "Failed to open: %s", path);
+ return;
}
if (lseek(config_fd, pos, SEEK_SET) != pos) {
- ret = -errno;
+ error_setg_errno(errp, errno, "Failed to seek: %s", path);
goto out;
}
@@ -832,32 +832,31 @@ static int host_pci_config_read(int pos, int len, uint32_t *val)
rc = read(config_fd, (uint8_t *)val, len);
} while (rc < 0 && (errno == EINTR || errno == EAGAIN));
if (rc != len) {
- ret = -errno;
+ error_setg_errno(errp, errno, "Failed to read: %s", path);
}
out:
close(config_fd);
- return ret;
}
-static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
+static void igd_pt_i440fx_realize(PCIDevice *pci, Error **errp)
{
uint32_t val = 0;
- int rc, i, num;
+ int i, num;
int pos, len;
+ Error *local_err = NULL;
num = ARRAY_SIZE(igd_host_bridge_infos);
for (i = 0; i < num; i++) {
pos = igd_host_bridge_infos[i].offset;
len = igd_host_bridge_infos[i].len;
- rc = host_pci_config_read(pos, len, &val);
- if (rc) {
- return -ENODEV;
+ host_pci_config_read(pos, len, &val, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
}
- pci_default_write_config(pci_dev, pos, val, len);
+ pci_default_write_config(pci, pos, val, len);
}
-
- return 0;
}
static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
@@ -865,7 +864,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = igd_pt_i440fx_initfn;
+ k->realize = igd_pt_i440fx_realize;
dc->desc = "IGD Passthrough Host bridge";
}
--
2.15.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge
2017-12-17 20:49 [Qemu-devel] [PATCH 0/4] QOM'ify PCIDevices Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 1/4] hw/block/nvme: QOM'ify PCI NVME Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge Philippe Mathieu-Daudé
@ 2017-12-17 20:49 ` Philippe Mathieu-Daudé
2017-12-18 7:15 ` Marcel Apfelbaum
2017-12-17 20:49 ` [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init() Philippe Mathieu-Daudé
3 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-17 20:49 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost,
Paul Burton, Yongbok Kim, Edgar E . Iglesias, Alistair Francis
Cc: Philippe Mathieu-Daudé, qemu-devel, James Hogan
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
hw/pci-host/xilinx-pcie.c | 19 +++++++++----------
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
index 7659253090..756db39fd5 100644
--- a/hw/pci-host/xilinx-pcie.c
+++ b/hw/pci-host/xilinx-pcie.c
@@ -267,24 +267,23 @@ static void xilinx_pcie_root_config_write(PCIDevice *d, uint32_t address,
}
}
-static int xilinx_pcie_root_init(PCIDevice *dev)
+static void xilinx_pcie_root_realize(PCIDevice *pci, Error **errp)
{
- BusState *bus = qdev_get_parent_bus(DEVICE(dev));
+ DeviceState *dev = DEVICE(pci);
+ BusState *bus = qdev_get_parent_bus(dev);
XilinxPCIEHost *s = XILINX_PCIE_HOST(bus->parent);
- pci_set_word(dev->config + PCI_COMMAND,
+ pci_set_word(pci->config + PCI_COMMAND,
PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
- pci_set_word(dev->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
- pci_set_word(dev->config + PCI_MEMORY_LIMIT,
+ pci_set_word(pci->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
+ pci_set_word(pci->config + PCI_MEMORY_LIMIT,
((s->mmio_base + s->mmio_size - 1) >> 16) & 0xfff0);
- pci_bridge_initfn(dev, TYPE_PCI_BUS);
+ pci_bridge_initfn(pci, TYPE_PCI_BUS);
- if (pcie_endpoint_cap_v1_init(dev, 0x80) < 0) {
+ if (pcie_endpoint_cap_v1_init(pci, 0x80) < 0) {
hw_error("Failed to initialize PCIe capability");
}
-
- return 0;
}
static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
@@ -300,7 +299,7 @@ static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_BRIDGE_HOST;
k->is_express = true;
k->is_bridge = true;
- k->init = xilinx_pcie_root_init;
+ k->realize = xilinx_pcie_root_realize;
k->exit = pci_bridge_exitfn;
dc->reset = pci_bridge_reset;
k->config_read = xilinx_pcie_root_config_read;
--
2.15.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init()
2017-12-17 20:49 [Qemu-devel] [PATCH 0/4] QOM'ify PCIDevices Philippe Mathieu-Daudé
` (2 preceding siblings ...)
2017-12-17 20:49 ` [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe " Philippe Mathieu-Daudé
@ 2017-12-17 20:49 ` Philippe Mathieu-Daudé
2017-12-18 7:07 ` Marcel Apfelbaum
2018-03-11 15:46 ` Philippe Mathieu-Daudé
3 siblings, 2 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-17 20:49 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum, Eduardo Habkost
Cc: Philippe Mathieu-Daudé, qemu-devel
All PCI devices are now QOM'ified.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/pci/pci.h | 1 -
hw/pci/pci.c | 14 --------------
2 files changed, 15 deletions(-)
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 8d02a0a383..0f1ed64c2f 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -217,7 +217,6 @@ typedef struct PCIDeviceClass {
DeviceClass parent_class;
void (*realize)(PCIDevice *dev, Error **errp);
- int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
PCIUnregisterFunc *exit;
PCIConfigReadFunc *config_read;
PCIConfigWriteFunc *config_write;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b2d139bd9a..cd25ab6f6b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2054,18 +2054,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
}
}
-static void pci_default_realize(PCIDevice *dev, Error **errp)
-{
- PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
-
- if (pc->init) {
- if (pc->init(dev) < 0) {
- error_setg(errp, "Device initialization failed");
- return;
- }
- }
-}
-
PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
const char *name)
{
@@ -2538,13 +2526,11 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
static void pci_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
- PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
k->realize = pci_qdev_realize;
k->unrealize = pci_qdev_unrealize;
k->bus_type = TYPE_PCI_BUS;
k->props = pci_props;
- pc->realize = pci_default_realize;
}
static void pci_device_class_base_init(ObjectClass *klass, void *data)
--
2.15.1
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge
2017-12-17 20:49 ` [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge Philippe Mathieu-Daudé
@ 2017-12-18 7:05 ` Marcel Apfelbaum
2017-12-18 14:54 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-12-18 7:05 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Michael S. Tsirkin, Eduardo Habkost, Hervé Poussineau
Cc: qemu-devel, qemu-block
Hi Philippe,
Thanks for the patch.
On 17/12/2017 22:49, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/pci-host/piix.c | 31 +++++++++++++++----------------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
> index a684a7cca9..0153f32a32 100644
> --- a/hw/pci-host/piix.c
> +++ b/hw/pci-host/piix.c
> @@ -804,7 +804,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
> {0xa8, 4}, /* SNB: base of GTT stolen memory */
> };
>
> -static int host_pci_config_read(int pos, int len, uint32_t *val)
> +static void host_pci_config_read(int pos, int len, uint32_t *val, Error **errp)
> {
> char path[PATH_MAX];
> int config_fd;
> @@ -812,19 +812,19 @@ static int host_pci_config_read(int pos, int len, uint32_t *val)
> /* Access real host bridge. */
> int rc = snprintf(path, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
> 0, 0, 0, 0, "config");
> - int ret = 0;
>
> if (rc >= size || rc < 0) {
> - return -ENODEV;
> + error_setg(&error_abort, "Invalid PCI device");
I think a return statement is missing here.
> }
>
> config_fd = open(path, O_RDWR);
> if (config_fd < 0) {
> - return -ENODEV;
> + error_setg_errno(errp, errno, "Failed to open: %s", path);
> + return;
> }
>
> if (lseek(config_fd, pos, SEEK_SET) != pos) {
> - ret = -errno;
> + error_setg_errno(errp, errno, "Failed to seek: %s", path);
> goto out;
> }
>
> @@ -832,32 +832,31 @@ static int host_pci_config_read(int pos, int len, uint32_t *val)
> rc = read(config_fd, (uint8_t *)val, len);
> } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
> if (rc != len) {
> - ret = -errno;
> + error_setg_errno(errp, errno, "Failed to read: %s", path);
> }
>
> out:
> close(config_fd);
> - return ret;
> }
>
> -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
> +static void igd_pt_i440fx_realize(PCIDevice *pci, Error **errp)
I would keep pci_dev, pci is too "broad".
Also is kind of convention, try to grep the source code.
Other than that the patch looks good to me.
Thanks,
Marcel
> {
> uint32_t val = 0;
> - int rc, i, num;
> + int i, num;
> int pos, len;
> + Error *local_err = NULL;
>
> num = ARRAY_SIZE(igd_host_bridge_infos);
> for (i = 0; i < num; i++) {
> pos = igd_host_bridge_infos[i].offset;
> len = igd_host_bridge_infos[i].len;
> - rc = host_pci_config_read(pos, len, &val);
> - if (rc) {
> - return -ENODEV;
> + host_pci_config_read(pos, len, &val, &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> }
> - pci_default_write_config(pci_dev, pos, val, len);
> + pci_default_write_config(pci, pos, val, len);
> }
> -
> - return 0;
> }
>
> static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> @@ -865,7 +864,7 @@ static void igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
> DeviceClass *dc = DEVICE_CLASS(klass);
> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>
> - k->init = igd_pt_i440fx_initfn;
> + k->realize = igd_pt_i440fx_realize;
> dc->desc = "IGD Passthrough Host bridge";
> }
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init()
2017-12-17 20:49 ` [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init() Philippe Mathieu-Daudé
@ 2017-12-18 7:07 ` Marcel Apfelbaum
2018-03-11 15:46 ` Philippe Mathieu-Daudé
1 sibling, 0 replies; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-12-18 7:07 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Michael S. Tsirkin, Eduardo Habkost
Cc: qemu-devel
On 17/12/2017 22:49, Philippe Mathieu-Daudé wrote:
> All PCI devices are now QOM'ified.
>
Finally!
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/pci/pci.h | 1 -
> hw/pci/pci.c | 14 --------------
> 2 files changed, 15 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 8d02a0a383..0f1ed64c2f 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -217,7 +217,6 @@ typedef struct PCIDeviceClass {
> DeviceClass parent_class;
>
> void (*realize)(PCIDevice *dev, Error **errp);
> - int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
> PCIUnregisterFunc *exit;
> PCIConfigReadFunc *config_read;
> PCIConfigWriteFunc *config_write;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b2d139bd9a..cd25ab6f6b 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2054,18 +2054,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> }
> }
>
> -static void pci_default_realize(PCIDevice *dev, Error **errp)
> -{
> - PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> -
> - if (pc->init) {
> - if (pc->init(dev) < 0) {
> - error_setg(errp, "Device initialization failed");
> - return;
> - }
> - }
> -}
> -
> PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> const char *name)
> {
> @@ -2538,13 +2526,11 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
> static void pci_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *k = DEVICE_CLASS(klass);
> - PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
>
> k->realize = pci_qdev_realize;
> k->unrealize = pci_qdev_unrealize;
> k->bus_type = TYPE_PCI_BUS;
> k->props = pci_props;
> - pc->realize = pci_default_realize;
> }
>
> static void pci_device_class_base_init(ObjectClass *klass, void *data)
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge
2017-12-17 20:49 ` [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe " Philippe Mathieu-Daudé
@ 2017-12-18 7:15 ` Marcel Apfelbaum
2017-12-18 15:11 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Marcel Apfelbaum @ 2017-12-18 7:15 UTC (permalink / raw)
To: Philippe Mathieu-Daudé,
Michael S. Tsirkin, Eduardo Habkost, Paul Burton, Yongbok Kim,
Edgar E . Iglesias, Alistair Francis
Cc: qemu-devel, James Hogan
On 17/12/2017 22:49, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> hw/pci-host/xilinx-pcie.c | 19 +++++++++----------
> 1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
> index 7659253090..756db39fd5 100644
> --- a/hw/pci-host/xilinx-pcie.c
> +++ b/hw/pci-host/xilinx-pcie.c
> @@ -267,24 +267,23 @@ static void xilinx_pcie_root_config_write(PCIDevice *d, uint32_t address,
> }
> }
>
> -static int xilinx_pcie_root_init(PCIDevice *dev)
> +static void xilinx_pcie_root_realize(PCIDevice *pci, Error **errp)
> {
Same comment here, if "dev" seems not good enough maybe
you can use the pci_dev convention.
> - BusState *bus = qdev_get_parent_bus(DEVICE(dev));
> + DeviceState *dev = DEVICE(pci);
> + BusState *bus = qdev_get_parent_bus(dev);
Not sure the above worth the effort :)
> XilinxPCIEHost *s = XILINX_PCIE_HOST(bus->parent);
>
> - pci_set_word(dev->config + PCI_COMMAND,
> + pci_set_word(pci->config + PCI_COMMAND,
> PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
> - pci_set_word(dev->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
> - pci_set_word(dev->config + PCI_MEMORY_LIMIT,
> + pci_set_word(pci->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
> + pci_set_word(pci->config + PCI_MEMORY_LIMIT,
> ((s->mmio_base + s->mmio_size - 1) >> 16) & 0xfff0);
>
> - pci_bridge_initfn(dev, TYPE_PCI_BUS);
> + pci_bridge_initfn(pci, TYPE_PCI_BUS);
>
> - if (pcie_endpoint_cap_v1_init(dev, 0x80) < 0) {
> + if (pcie_endpoint_cap_v1_init(pci, 0x80) < 0) {
> hw_error("Failed to initialize PCIe capability");
I think here you need to use the error_setg (after/instead hw_error).
It is supposed to return -1 if pcie capability was not added.
Thanks,
Marcel
> }
> -
> - return 0;
> }
>
> static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
> @@ -300,7 +299,7 @@ static void xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
> k->class_id = PCI_CLASS_BRIDGE_HOST;
> k->is_express = true;
> k->is_bridge = true;
> - k->init = xilinx_pcie_root_init;
> + k->realize = xilinx_pcie_root_realize;
> k->exit = pci_bridge_exitfn;
> dc->reset = pci_bridge_reset;
> k->config_read = xilinx_pcie_root_config_read;
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge
2017-12-18 7:05 ` Marcel Apfelbaum
@ 2017-12-18 14:54 ` Philippe Mathieu-Daudé
2017-12-18 14:59 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-18 14:54 UTC (permalink / raw)
To: Marcel Apfelbaum, Michael S. Tsirkin, Eduardo Habkost,
Hervé Poussineau
Cc: qemu-devel, qemu-block
Hi Marcel,
On 12/18/2017 04:05 AM, Marcel Apfelbaum wrote:
> Hi Philippe,
>
> Thanks for the patch.
>
> On 17/12/2017 22:49, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/pci-host/piix.c | 31 +++++++++++++++----------------
>> 1 file changed, 15 insertions(+), 16 deletions(-)
>>
>> diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
>> index a684a7cca9..0153f32a32 100644
>> --- a/hw/pci-host/piix.c
>> +++ b/hw/pci-host/piix.c
>> @@ -804,7 +804,7 @@ static const IGDHostInfo igd_host_bridge_infos[] = {
>> {0xa8, 4}, /* SNB: base of GTT stolen memory */
>> };
>> -static int host_pci_config_read(int pos, int len, uint32_t *val)
>> +static void host_pci_config_read(int pos, int len, uint32_t *val,
>> Error **errp)
>> {
>> char path[PATH_MAX];
>> int config_fd;
>> @@ -812,19 +812,19 @@ static int host_pci_config_read(int pos, int
>> len, uint32_t *val)
>> /* Access real host bridge. */
>> int rc = snprintf(path, size,
>> "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
>> 0, 0, 0, 0, "config");
>> - int ret = 0;
>> if (rc >= size || rc < 0) {
>> - return -ENODEV;
>> + error_setg(&error_abort, "Invalid PCI device");
>
> I think a return statement is missing here.
Well, error_setg(&error_abort) will never return, however checking the
documentation again I noticed I forgot these recommendations:
* Please don't error_setg(&error_fatal, ...), use error_report() and
* exit(), because that's more obvious.
* Likewise, don't error_setg(&error_abort, ...), use assert().
abort() might be OK but a bit harsh, so I'll use error_report() + exit()
here.
>> }
>> config_fd = open(path, O_RDWR);
>> if (config_fd < 0) {
>> - return -ENODEV;
>> + error_setg_errno(errp, errno, "Failed to open: %s", path);
>> + return;
>> }
>> if (lseek(config_fd, pos, SEEK_SET) != pos) {
>> - ret = -errno;
>> + error_setg_errno(errp, errno, "Failed to seek: %s", path);
>> goto out;
>> }
>> @@ -832,32 +832,31 @@ static int host_pci_config_read(int pos, int
>> len, uint32_t *val)
>> rc = read(config_fd, (uint8_t *)val, len);
>> } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>> if (rc != len) {
>> - ret = -errno;
>> + error_setg_errno(errp, errno, "Failed to read: %s", path);
>> }
>> out:
>> close(config_fd);
>> - return ret;
>> }
>> -static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev)
>> +static void igd_pt_i440fx_realize(PCIDevice *pci, Error **errp)
>
> I would keep pci_dev, pci is too "broad".
> Also is kind of convention, try to grep the source code.
Ok.
> Other than that the patch looks good to me.
Thanks!
> Thanks,
> Marcel
>
>> {
>> uint32_t val = 0;
>> - int rc, i, num;
>> + int i, num;
>> int pos, len;
>> + Error *local_err = NULL;
>> num = ARRAY_SIZE(igd_host_bridge_infos);
>> for (i = 0; i < num; i++) {
>> pos = igd_host_bridge_infos[i].offset;
>> len = igd_host_bridge_infos[i].len;
>> - rc = host_pci_config_read(pos, len, &val);
>> - if (rc) {
>> - return -ENODEV;
>> + host_pci_config_read(pos, len, &val, &local_err);
>> + if (local_err) {
>> + error_propagate(errp, local_err);
>> + return;
>> }
>> - pci_default_write_config(pci_dev, pos, val, len);
>> + pci_default_write_config(pci, pos, val, len);
>> }
>> -
>> - return 0;
>> }
>> static void igd_passthrough_i440fx_class_init(ObjectClass *klass,
>> void *data)
>> @@ -865,7 +864,7 @@ static void
>> igd_passthrough_i440fx_class_init(ObjectClass *klass, void *data)
>> DeviceClass *dc = DEVICE_CLASS(klass);
>> PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
>> - k->init = igd_pt_i440fx_initfn;
>> + k->realize = igd_pt_i440fx_realize;
>> dc->desc = "IGD Passthrough Host bridge";
>> }
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge
2017-12-18 14:54 ` Philippe Mathieu-Daudé
@ 2017-12-18 14:59 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-18 14:59 UTC (permalink / raw)
To: Marcel Apfelbaum, Michael S. Tsirkin, Eduardo Habkost,
Hervé Poussineau
Cc: qemu-devel@nongnu.org Developers, open list:Block layer core
[...]
>>> -static int host_pci_config_read(int pos, int len, uint32_t *val)
>>> +static void host_pci_config_read(int pos, int len, uint32_t *val,
>>> Error **errp)
>>> {
>>> char path[PATH_MAX];
>>> int config_fd;
>>> @@ -812,19 +812,19 @@ static int host_pci_config_read(int pos, int
>>> len, uint32_t *val)
>>> /* Access real host bridge. */
>>> int rc = snprintf(path, size,
>>> "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
>>> 0, 0, 0, 0, "config");
>>> - int ret = 0;
>>> if (rc >= size || rc < 0) {
>>> - return -ENODEV;
>>> + error_setg(&error_abort, "Invalid PCI device");
>>
>> I think a return statement is missing here.
>
> Well, error_setg(&error_abort) will never return, however checking the
> documentation again I noticed I forgot these recommendations:
>
> * Please don't error_setg(&error_fatal, ...), use error_report() and
> * exit(), because that's more obvious.
> * Likewise, don't error_setg(&error_abort, ...), use assert().
>
> abort() might be OK but a bit harsh, so I'll use error_report() + exit()
> here.
Actually g_strdup_printf() is cleaner.
>>> }
>>> config_fd = open(path, O_RDWR);
>>> if (config_fd < 0) {
>>> - return -ENODEV;
>>> + error_setg_errno(errp, errno, "Failed to open: %s", path);
>>> + return;
>>> }
>>> if (lseek(config_fd, pos, SEEK_SET) != pos) {
>>> - ret = -errno;
>>> + error_setg_errno(errp, errno, "Failed to seek: %s", path);
>>> goto out;
>>> }
>>> @@ -832,32 +832,31 @@ static int host_pci_config_read(int pos, int
>>> len, uint32_t *val)
>>> rc = read(config_fd, (uint8_t *)val, len);
>>> } while (rc < 0 && (errno == EINTR || errno == EAGAIN));
>>> if (rc != len) {
>>> - ret = -errno;
>>> + error_setg_errno(errp, errno, "Failed to read: %s", path);
>>> }
>>> out:
>>> close(config_fd);
>>> - return ret;
>>> }
[...]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe host bridge
2017-12-18 7:15 ` Marcel Apfelbaum
@ 2017-12-18 15:11 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2017-12-18 15:11 UTC (permalink / raw)
To: Marcel Apfelbaum, Michael S. Tsirkin, Eduardo Habkost,
Paul Burton, Yongbok Kim, Edgar E . Iglesias, Alistair Francis
Cc: qemu-devel, James Hogan
On 12/18/2017 04:15 AM, Marcel Apfelbaum wrote:
> On 17/12/2017 22:49, Philippe Mathieu-Daudé wrote:
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>> hw/pci-host/xilinx-pcie.c | 19 +++++++++----------
>> 1 file changed, 9 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/pci-host/xilinx-pcie.c b/hw/pci-host/xilinx-pcie.c
>> index 7659253090..756db39fd5 100644
>> --- a/hw/pci-host/xilinx-pcie.c
>> +++ b/hw/pci-host/xilinx-pcie.c
>> @@ -267,24 +267,23 @@ static void
>> xilinx_pcie_root_config_write(PCIDevice *d, uint32_t address,
>> }
>> }
>> -static int xilinx_pcie_root_init(PCIDevice *dev)
>> +static void xilinx_pcie_root_realize(PCIDevice *pci, Error **errp)
>> {
>
> Same comment here, if "dev" seems not good enough maybe
> you can use the pci_dev convention.
>
>> - BusState *bus = qdev_get_parent_bus(DEVICE(dev));
>> + DeviceState *dev = DEVICE(pci);
>> + BusState *bus = qdev_get_parent_bus(dev);
>
> Not sure the above worth the effort :)
>
>> XilinxPCIEHost *s = XILINX_PCIE_HOST(bus->parent);
>> - pci_set_word(dev->config + PCI_COMMAND,
>> + pci_set_word(pci->config + PCI_COMMAND,
>> PCI_COMMAND_MEMORY | PCI_COMMAND_MASTER);
>> - pci_set_word(dev->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
>> - pci_set_word(dev->config + PCI_MEMORY_LIMIT,
>> + pci_set_word(pci->config + PCI_MEMORY_BASE, s->mmio_base >> 16);
>> + pci_set_word(pci->config + PCI_MEMORY_LIMIT,
>> ((s->mmio_base + s->mmio_size - 1) >> 16) & 0xfff0);
>> - pci_bridge_initfn(dev, TYPE_PCI_BUS);
>> + pci_bridge_initfn(pci, TYPE_PCI_BUS);
>> - if (pcie_endpoint_cap_v1_init(dev, 0x80) < 0) {
>> + if (pcie_endpoint_cap_v1_init(pci, 0x80) < 0) {
>> hw_error("Failed to initialize PCIe capability");
>
> I think here you need to use the error_setg (after/instead hw_error).
> It is supposed to return -1 if pcie capability was not added.
Oops I missed that, I'll fix in v2.
> Thanks,
> Marcel
>
>> }
>> -
>> - return 0;
>> }
>> static void xilinx_pcie_root_class_init(ObjectClass *klass, void
>> *data)
>> @@ -300,7 +299,7 @@ static void
>> xilinx_pcie_root_class_init(ObjectClass *klass, void *data)
>> k->class_id = PCI_CLASS_BRIDGE_HOST;
>> k->is_express = true;
>> k->is_bridge = true;
>> - k->init = xilinx_pcie_root_init;
>> + k->realize = xilinx_pcie_root_realize;
>> k->exit = pci_bridge_exitfn;
>> dc->reset = pci_bridge_reset;
>> k->config_read = xilinx_pcie_root_config_read;
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init()
2017-12-17 20:49 ` [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init() Philippe Mathieu-Daudé
2017-12-18 7:07 ` Marcel Apfelbaum
@ 2018-03-11 15:46 ` Philippe Mathieu-Daudé
2018-03-12 0:03 ` Michael S. Tsirkin
1 sibling, 1 reply; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-11 15:46 UTC (permalink / raw)
To: Michael S. Tsirkin, Paolo Bonzini; +Cc: Marcel Apfelbaum, qemu-devel
Hi Michael, Paolo.
Now than all other PCI QOM'ification patches got applied, can you take
this patch for 2.12?
Thanks,
Phil.
On 12/17/2017 09:49 PM, Philippe Mathieu-Daudé wrote:
> All PCI devices are now QOM'ified.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/pci/pci.h | 1 -
> hw/pci/pci.c | 14 --------------
> 2 files changed, 15 deletions(-)
>
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 8d02a0a383..0f1ed64c2f 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -217,7 +217,6 @@ typedef struct PCIDeviceClass {
> DeviceClass parent_class;
>
> void (*realize)(PCIDevice *dev, Error **errp);
> - int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
> PCIUnregisterFunc *exit;
> PCIConfigReadFunc *config_read;
> PCIConfigWriteFunc *config_write;
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b2d139bd9a..cd25ab6f6b 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -2054,18 +2054,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> }
> }
>
> -static void pci_default_realize(PCIDevice *dev, Error **errp)
> -{
> - PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> -
> - if (pc->init) {
> - if (pc->init(dev) < 0) {
> - error_setg(errp, "Device initialization failed");
> - return;
> - }
> - }
> -}
> -
> PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> const char *name)
> {
> @@ -2538,13 +2526,11 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
> static void pci_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *k = DEVICE_CLASS(klass);
> - PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
>
> k->realize = pci_qdev_realize;
> k->unrealize = pci_qdev_unrealize;
> k->bus_type = TYPE_PCI_BUS;
> k->props = pci_props;
> - pc->realize = pci_default_realize;
> }
>
> static void pci_device_class_base_init(ObjectClass *klass, void *data)
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init()
2018-03-11 15:46 ` Philippe Mathieu-Daudé
@ 2018-03-12 0:03 ` Michael S. Tsirkin
2018-03-12 0:04 ` Michael S. Tsirkin
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2018-03-12 0:03 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Paolo Bonzini, Marcel Apfelbaum, qemu-devel
On Sun, Mar 11, 2018 at 04:46:52PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Michael, Paolo.
>
> Now than all other PCI QOM'ification patches got applied, can you take
> this patch for 2.12?
>
> Thanks,
>
> Phil.
>
> On 12/17/2017 09:49 PM, Philippe Mathieu-Daudé wrote:
> > All PCI devices are now QOM'ified.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > ---
> > include/hw/pci/pci.h | 1 -
> > hw/pci/pci.c | 14 --------------
> > 2 files changed, 15 deletions(-)
> >
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 8d02a0a383..0f1ed64c2f 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -217,7 +217,6 @@ typedef struct PCIDeviceClass {
> > DeviceClass parent_class;
> >
> > void (*realize)(PCIDevice *dev, Error **errp);
> > - int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
> > PCIUnregisterFunc *exit;
> > PCIConfigReadFunc *config_read;
> > PCIConfigWriteFunc *config_write;
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index b2d139bd9a..cd25ab6f6b 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -2054,18 +2054,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> > }
> > }
> >
> > -static void pci_default_realize(PCIDevice *dev, Error **errp)
> > -{
> > - PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> > -
> > - if (pc->init) {
> > - if (pc->init(dev) < 0) {
> > - error_setg(errp, "Device initialization failed");
> > - return;
> > - }
> > - }
> > -}
> > -
> > PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> > const char *name)
> > {
> > @@ -2538,13 +2526,11 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
> > static void pci_device_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *k = DEVICE_CLASS(klass);
> > - PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
> >
> > k->realize = pci_qdev_realize;
> > k->unrealize = pci_qdev_unrealize;
> > k->bus_type = TYPE_PCI_BUS;
> > k->props = pci_props;
> > - pc->realize = pci_default_realize;
How about we assert !init here?
+ assert(!pc->init);
> > }
> >
> > static void pci_device_class_base_init(ObjectClass *klass, void *data)
> >
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init()
2018-03-12 0:03 ` Michael S. Tsirkin
@ 2018-03-12 0:04 ` Michael S. Tsirkin
2018-03-12 10:57 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2018-03-12 0:04 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: Paolo Bonzini, Marcel Apfelbaum, qemu-devel
On Mon, Mar 12, 2018 at 02:03:17AM +0200, Michael S. Tsirkin wrote:
> On Sun, Mar 11, 2018 at 04:46:52PM +0100, Philippe Mathieu-Daudé wrote:
> > Hi Michael, Paolo.
> >
> > Now than all other PCI QOM'ification patches got applied, can you take
> > this patch for 2.12?
> >
> > Thanks,
> >
> > Phil.
> >
> > On 12/17/2017 09:49 PM, Philippe Mathieu-Daudé wrote:
> > > All PCI devices are now QOM'ified.
> > >
> > > Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > > ---
> > > include/hw/pci/pci.h | 1 -
> > > hw/pci/pci.c | 14 --------------
> > > 2 files changed, 15 deletions(-)
> > >
> > > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > > index 8d02a0a383..0f1ed64c2f 100644
> > > --- a/include/hw/pci/pci.h
> > > +++ b/include/hw/pci/pci.h
> > > @@ -217,7 +217,6 @@ typedef struct PCIDeviceClass {
> > > DeviceClass parent_class;
> > >
> > > void (*realize)(PCIDevice *dev, Error **errp);
> > > - int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
> > > PCIUnregisterFunc *exit;
> > > PCIConfigReadFunc *config_read;
> > > PCIConfigWriteFunc *config_write;
> > > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > > index b2d139bd9a..cd25ab6f6b 100644
> > > --- a/hw/pci/pci.c
> > > +++ b/hw/pci/pci.c
> > > @@ -2054,18 +2054,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
> > > }
> > > }
> > >
> > > -static void pci_default_realize(PCIDevice *dev, Error **errp)
> > > -{
> > > - PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
> > > -
> > > - if (pc->init) {
> > > - if (pc->init(dev) < 0) {
> > > - error_setg(errp, "Device initialization failed");
> > > - return;
> > > - }
> > > - }
> > > -}
> > > -
> > > PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> > > const char *name)
> > > {
> > > @@ -2538,13 +2526,11 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
> > > static void pci_device_class_init(ObjectClass *klass, void *data)
> > > {
> > > DeviceClass *k = DEVICE_CLASS(klass);
> > > - PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
> > >
> > > k->realize = pci_qdev_realize;
> > > k->unrealize = pci_qdev_unrealize;
> > > k->bus_type = TYPE_PCI_BUS;
> > > k->props = pci_props;
> > > - pc->realize = pci_default_realize;
>
> How about we assert !init here?
>
> + assert(!pc->init);
Oh sorry I missed the chunk that drops it completely.
Will apply.
> > > }
> > >
> > > static void pci_device_class_base_init(ObjectClass *klass, void *data)
> > >
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init()
2018-03-12 0:04 ` Michael S. Tsirkin
@ 2018-03-12 10:57 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 15+ messages in thread
From: Philippe Mathieu-Daudé @ 2018-03-12 10:57 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Paolo Bonzini, Marcel Apfelbaum, qemu-devel
On 03/12/2018 01:04 AM, Michael S. Tsirkin wrote:
> On Mon, Mar 12, 2018 at 02:03:17AM +0200, Michael S. Tsirkin wrote:
>> On Sun, Mar 11, 2018 at 04:46:52PM +0100, Philippe Mathieu-Daudé wrote:
>>> Hi Michael, Paolo.
>>>
>>> Now than all other PCI QOM'ification patches got applied, can you take
>>> this patch for 2.12?
>>>
>>> Thanks,
>>>
>>> Phil.
>>>
>>> On 12/17/2017 09:49 PM, Philippe Mathieu-Daudé wrote:
>>>> All PCI devices are now QOM'ified.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>> include/hw/pci/pci.h | 1 -
>>>> hw/pci/pci.c | 14 --------------
>>>> 2 files changed, 15 deletions(-)
>>>>
>>>> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
>>>> index 8d02a0a383..0f1ed64c2f 100644
>>>> --- a/include/hw/pci/pci.h
>>>> +++ b/include/hw/pci/pci.h
>>>> @@ -217,7 +217,6 @@ typedef struct PCIDeviceClass {
>>>> DeviceClass parent_class;
>>>>
>>>> void (*realize)(PCIDevice *dev, Error **errp);
>>>> - int (*init)(PCIDevice *dev);/* TODO convert to realize() and remove */
>>>> PCIUnregisterFunc *exit;
>>>> PCIConfigReadFunc *config_read;
>>>> PCIConfigWriteFunc *config_write;
>>>> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
>>>> index b2d139bd9a..cd25ab6f6b 100644
>>>> --- a/hw/pci/pci.c
>>>> +++ b/hw/pci/pci.c
>>>> @@ -2054,18 +2054,6 @@ static void pci_qdev_realize(DeviceState *qdev, Error **errp)
>>>> }
>>>> }
>>>>
>>>> -static void pci_default_realize(PCIDevice *dev, Error **errp)
>>>> -{
>>>> - PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
>>>> -
>>>> - if (pc->init) {
>>>> - if (pc->init(dev) < 0) {
>>>> - error_setg(errp, "Device initialization failed");
>>>> - return;
>>>> - }
>>>> - }
>>>> -}
>>>> -
>>>> PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
>>>> const char *name)
>>>> {
>>>> @@ -2538,13 +2526,11 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
>>>> static void pci_device_class_init(ObjectClass *klass, void *data)
>>>> {
>>>> DeviceClass *k = DEVICE_CLASS(klass);
>>>> - PCIDeviceClass *pc = PCI_DEVICE_CLASS(klass);
>>>>
>>>> k->realize = pci_qdev_realize;
>>>> k->unrealize = pci_qdev_unrealize;
>>>> k->bus_type = TYPE_PCI_BUS;
>>>> k->props = pci_props;
>>>> - pc->realize = pci_default_realize;
>>
>> How about we assert !init here?
>>
>> + assert(!pc->init);
>
> Oh sorry I missed the chunk that drops it completely.
> Will apply.
Thanks!
>
>>>> }
>>>>
>>>> static void pci_device_class_base_init(ObjectClass *klass, void *data)
>>>>
>>
>>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-03-12 10:57 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-17 20:49 [Qemu-devel] [PATCH 0/4] QOM'ify PCIDevices Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 1/4] hw/block/nvme: QOM'ify PCI NVME Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 2/4] hw/pci-host/piix: QOM'ify the IGD Passthrough host bridge Philippe Mathieu-Daudé
2017-12-18 7:05 ` Marcel Apfelbaum
2017-12-18 14:54 ` Philippe Mathieu-Daudé
2017-12-18 14:59 ` Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 3/4] hw/pci-host/xilinx: QOM'ify the AXI-PCIe " Philippe Mathieu-Daudé
2017-12-18 7:15 ` Marcel Apfelbaum
2017-12-18 15:11 ` Philippe Mathieu-Daudé
2017-12-17 20:49 ` [Qemu-devel] [PATCH 4/4] hw/pci: remove obsolete PCIDevice->init() Philippe Mathieu-Daudé
2017-12-18 7:07 ` Marcel Apfelbaum
2018-03-11 15:46 ` Philippe Mathieu-Daudé
2018-03-12 0:03 ` Michael S. Tsirkin
2018-03-12 0:04 ` Michael S. Tsirkin
2018-03-12 10:57 ` Philippe Mathieu-Daudé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.