Re: [Qemu-devel] [PATCH v5 6/9] pci: Convert to realize

2017-06-19 Thread Mao Zhongyi 

Hi, Marcel

On 06/19/2017 07:42 PM, Marcel Apfelbaum wrote:

On 12/06/2017 16:48, Mao Zhongyi wrote:

The pci-birdge device i82801b11


It is a dmi-to-pci brigde


and io3130_upstream/downstream


You forgot to mention the pcie_root_port.


still implements the old PCIDeviceClass .init() through *_init()
instead of the new .realize(). All devices need to be converted
to .realize(). So convert it and rename it to *_realize().




I would change the message to something more concise like:

"Convert i82801b11, xio3130_downstream, xio3130_upstream
 and pcie_root_port devices to realize."

I am sure it worth a re-spin though.


Thanks for your kind suggestion, I will. :)




Cc: m...@redhat.com
Cc: mar...@redhat.com
Cc: arm...@redhat.com
Signed-off-by: Mao Zhongyi 
---
  hw/pci-bridge/i82801b11.c  | 11 +--
  hw/pci-bridge/pcie_root_port.c | 15 ++-
  hw/pci-bridge/xio3130_downstream.c | 20 +---
  hw/pci-bridge/xio3130_upstream.c   | 20 +---
  hw/pci/pci_bridge.c|  7 +++
  hw/pci/pcie.c  | 11 ++-
  include/hw/pci/pci_bridge.h|  3 ++-
  include/hw/pci/pcie.h  |  3 ++-
  8 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 2c065c3..2c1b747 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -59,24 +59,23 @@ typedef struct I82801b11Bridge {
  /*< public >*/
  } I82801b11Bridge;
  -static int i82801b11_bridge_initfn(PCIDevice *d)
+static void i82801b11_bridge_realize(PCIDevice *d, Error **errp)
  {
  int rc;
pci_bridge_initfn(d, TYPE_PCI_BUS);
rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,
-   I82801ba_SSVID_SVID, I82801ba_SSVID_SSID);
+   I82801ba_SSVID_SVID, I82801ba_SSVID_SSID,
+   errp);
  if (rc < 0) {
  goto err_bridge;
  }
  pci_config_set_prog_interface(d->config, PCI_CLASS_BRIDGE_PCI_INF_SUB);
-return 0;
+return;
err_bridge:
  pci_bridge_exitfn(d);
-
-return rc;
  }
static const VMStateDescription i82801b11_bridge_dev_vmstate = {
@@ -96,7 +95,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, 
void *data)
  k->vendor_id = PCI_VENDOR_ID_INTEL;
  k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11;
  k->revision = ICH9_D2P_A2_REVISION;
-k->init = i82801b11_bridge_initfn;
+k->realize = i82801b11_bridge_realize;
  k->config_write = pci_bridge_write_config;
  dc->vmsd = _bridge_dev_vmstate;
  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index cf36318..00f0b1f 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -59,29 +59,27 @@ static void rp_realize(PCIDevice *d, Error **errp)
  PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
  PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
  int rc;
-Error *local_err = NULL;
pci_config_set_interrupt_pin(d->config, 1);
  pci_bridge_initfn(d, TYPE_PCIE_BUS);
  pcie_port_init_reg(d);
  -rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, 
rpc->ssid);
+rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id,
+   rpc->ssid, errp);
  if (rc < 0) {
-error_setg(errp, "Can't init SSV ID, error %d", rc);



I suggest using 'error_append_hint' instead of removing
the message; even if it does not add a lot of information,
maybe someone is expecting it to be in the logs.


OK, I see.




  goto err_bridge;
  }
if (rpc->interrupts_init) {
-rc = rpc->interrupts_init(d, _err);
+rc = rpc->interrupts_init(d, errp);
  if (rc < 0) {
-error_propagate(errp, local_err);
  goto err_bridge;
  }
  }
  -rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
+rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT,
+   p->port, errp);
  if (rc < 0) {
-error_setg(errp, "Can't add Root Port capability, error %d", rc);
  goto err_int;
  }
  @@ -98,9 +96,8 @@ static void rp_realize(PCIDevice *d, Error **errp)
  }
rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
-   PCI_ERR_SIZEOF, _err);
+   PCI_ERR_SIZEOF, errp);
  if (rc < 0) {
-error_propagate(errp, local_err);
  goto err;
  }
  pcie_aer_root_init(d);
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index cfe8a36..e706f36 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -56,33 +56,33 @@ static void xio3130_downstream_reset(DeviceState *qdev)
  pci_bridge_reset(qdev);
  }
  -static

Re: [Qemu-devel] [PATCH v5 6/9] pci: Convert to realize

2017-06-19 Thread Marcel Apfelbaum 

On 12/06/2017 16:48, Mao Zhongyi wrote:

The pci-birdge device i82801b11


It is a dmi-to-pci brigde


and io3130_upstream/downstream


You forgot to mention the pcie_root_port.


still implements the old PCIDeviceClass .init() through *_init()
instead of the new .realize(). All devices need to be converted
to .realize(). So convert it and rename it to *_realize().




I would change the message to something more concise like:

"Convert i82801b11, xio3130_downstream, xio3130_upstream
 and pcie_root_port devices to realize."

I am sure it worth a re-spin though.


Cc: m...@redhat.com
Cc: mar...@redhat.com
Cc: arm...@redhat.com
Signed-off-by: Mao Zhongyi 
---
  hw/pci-bridge/i82801b11.c  | 11 +--
  hw/pci-bridge/pcie_root_port.c | 15 ++-
  hw/pci-bridge/xio3130_downstream.c | 20 +---
  hw/pci-bridge/xio3130_upstream.c   | 20 +---
  hw/pci/pci_bridge.c|  7 +++
  hw/pci/pcie.c  | 11 ++-
  include/hw/pci/pci_bridge.h|  3 ++-
  include/hw/pci/pcie.h  |  3 ++-
  8 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 2c065c3..2c1b747 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -59,24 +59,23 @@ typedef struct I82801b11Bridge {
  /*< public >*/
  } I82801b11Bridge;
  
-static int i82801b11_bridge_initfn(PCIDevice *d)

+static void i82801b11_bridge_realize(PCIDevice *d, Error **errp)
  {
  int rc;
  
  pci_bridge_initfn(d, TYPE_PCI_BUS);
  
  rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,

-   I82801ba_SSVID_SVID, I82801ba_SSVID_SSID);
+   I82801ba_SSVID_SVID, I82801ba_SSVID_SSID,
+   errp);
  if (rc < 0) {
  goto err_bridge;
  }
  pci_config_set_prog_interface(d->config, PCI_CLASS_BRIDGE_PCI_INF_SUB);
-return 0;
+return;
  
  err_bridge:

  pci_bridge_exitfn(d);
-
-return rc;
  }
  
  static const VMStateDescription i82801b11_bridge_dev_vmstate = {

@@ -96,7 +95,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, 
void *data)
  k->vendor_id = PCI_VENDOR_ID_INTEL;
  k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11;
  k->revision = ICH9_D2P_A2_REVISION;
-k->init = i82801b11_bridge_initfn;
+k->realize = i82801b11_bridge_realize;
  k->config_write = pci_bridge_write_config;
  dc->vmsd = _bridge_dev_vmstate;
  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index cf36318..00f0b1f 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -59,29 +59,27 @@ static void rp_realize(PCIDevice *d, Error **errp)
  PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
  PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
  int rc;
-Error *local_err = NULL;
  
  pci_config_set_interrupt_pin(d->config, 1);

  pci_bridge_initfn(d, TYPE_PCIE_BUS);
  pcie_port_init_reg(d);
  
-rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);

+rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id,
+   rpc->ssid, errp);
  if (rc < 0) {
-error_setg(errp, "Can't init SSV ID, error %d", rc);



I suggest using 'error_append_hint' instead of removing
the message; even if it does not add a lot of information,
maybe someone is expecting it to be in the logs.


  goto err_bridge;
  }
  
  if (rpc->interrupts_init) {

-rc = rpc->interrupts_init(d, _err);
+rc = rpc->interrupts_init(d, errp);
  if (rc < 0) {
-error_propagate(errp, local_err);
  goto err_bridge;
  }
  }
  
-rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);

+rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT,
+   p->port, errp);
  if (rc < 0) {
-error_setg(errp, "Can't add Root Port capability, error %d", rc);
  goto err_int;
  }
  
@@ -98,9 +96,8 @@ static void rp_realize(PCIDevice *d, Error **errp)

  }
  
  rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,

-   PCI_ERR_SIZEOF, _err);
+   PCI_ERR_SIZEOF, errp);
  if (rc < 0) {
-error_propagate(errp, local_err);
  goto err;
  }
  pcie_aer_root_init(d);
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index cfe8a36..e706f36 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -56,33 +56,33 @@ static void xio3130_downstream_reset(DeviceState *qdev)
  pci_bridge_reset(qdev);
  }
  
-static int xio3130_downstream_initfn(PCIDevice *d)

+static void xio3130_downstream_realize(PCIDevice *d, Error

[Qemu-devel] [PATCH v5 6/9] pci: Convert to realize

2017-06-12 Thread Mao Zhongyi 
The pci-birdge device i82801b11 and io3130_upstream/downstream
still implements the old PCIDeviceClass .init() through *_init()
instead of the new .realize(). All devices need to be converted
to .realize(). So convert it and rename it to *_realize().

Cc: m...@redhat.com
Cc: mar...@redhat.com
Cc: arm...@redhat.com
Signed-off-by: Mao Zhongyi 
---
 hw/pci-bridge/i82801b11.c  | 11 +--
 hw/pci-bridge/pcie_root_port.c | 15 ++-
 hw/pci-bridge/xio3130_downstream.c | 20 +---
 hw/pci-bridge/xio3130_upstream.c   | 20 +---
 hw/pci/pci_bridge.c|  7 +++
 hw/pci/pcie.c  | 11 ++-
 include/hw/pci/pci_bridge.h|  3 ++-
 include/hw/pci/pcie.h  |  3 ++-
 8 files changed, 42 insertions(+), 48 deletions(-)

diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 2c065c3..2c1b747 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -59,24 +59,23 @@ typedef struct I82801b11Bridge {
 /*< public >*/
 } I82801b11Bridge;
 
-static int i82801b11_bridge_initfn(PCIDevice *d)
+static void i82801b11_bridge_realize(PCIDevice *d, Error **errp)
 {
 int rc;
 
 pci_bridge_initfn(d, TYPE_PCI_BUS);
 
 rc = pci_bridge_ssvid_init(d, I82801ba_SSVID_OFFSET,
-   I82801ba_SSVID_SVID, I82801ba_SSVID_SSID);
+   I82801ba_SSVID_SVID, I82801ba_SSVID_SSID,
+   errp);
 if (rc < 0) {
 goto err_bridge;
 }
 pci_config_set_prog_interface(d->config, PCI_CLASS_BRIDGE_PCI_INF_SUB);
-return 0;
+return;
 
 err_bridge:
 pci_bridge_exitfn(d);
-
-return rc;
 }
 
 static const VMStateDescription i82801b11_bridge_dev_vmstate = {
@@ -96,7 +95,7 @@ static void i82801b11_bridge_class_init(ObjectClass *klass, 
void *data)
 k->vendor_id = PCI_VENDOR_ID_INTEL;
 k->device_id = PCI_DEVICE_ID_INTEL_82801BA_11;
 k->revision = ICH9_D2P_A2_REVISION;
-k->init = i82801b11_bridge_initfn;
+k->realize = i82801b11_bridge_realize;
 k->config_write = pci_bridge_write_config;
 dc->vmsd = _bridge_dev_vmstate;
 set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
diff --git a/hw/pci-bridge/pcie_root_port.c b/hw/pci-bridge/pcie_root_port.c
index cf36318..00f0b1f 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -59,29 +59,27 @@ static void rp_realize(PCIDevice *d, Error **errp)
 PCIDeviceClass *dc = PCI_DEVICE_GET_CLASS(d);
 PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(d);
 int rc;
-Error *local_err = NULL;
 
 pci_config_set_interrupt_pin(d->config, 1);
 pci_bridge_initfn(d, TYPE_PCIE_BUS);
 pcie_port_init_reg(d);
 
-rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id, rpc->ssid);
+rc = pci_bridge_ssvid_init(d, rpc->ssvid_offset, dc->vendor_id,
+   rpc->ssid, errp);
 if (rc < 0) {
-error_setg(errp, "Can't init SSV ID, error %d", rc);
 goto err_bridge;
 }
 
 if (rpc->interrupts_init) {
-rc = rpc->interrupts_init(d, _err);
+rc = rpc->interrupts_init(d, errp);
 if (rc < 0) {
-error_propagate(errp, local_err);
 goto err_bridge;
 }
 }
 
-rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT, p->port);
+rc = pcie_cap_init(d, rpc->exp_offset, PCI_EXP_TYPE_ROOT_PORT,
+   p->port, errp);
 if (rc < 0) {
-error_setg(errp, "Can't add Root Port capability, error %d", rc);
 goto err_int;
 }
 
@@ -98,9 +96,8 @@ static void rp_realize(PCIDevice *d, Error **errp)
 }
 
 rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
-   PCI_ERR_SIZEOF, _err);
+   PCI_ERR_SIZEOF, errp);
 if (rc < 0) {
-error_propagate(errp, local_err);
 goto err;
 }
 pcie_aer_root_init(d);
diff --git a/hw/pci-bridge/xio3130_downstream.c 
b/hw/pci-bridge/xio3130_downstream.c
index cfe8a36..e706f36 100644
--- a/hw/pci-bridge/xio3130_downstream.c
+++ b/hw/pci-bridge/xio3130_downstream.c
@@ -56,33 +56,33 @@ static void xio3130_downstream_reset(DeviceState *qdev)
 pci_bridge_reset(qdev);
 }
 
-static int xio3130_downstream_initfn(PCIDevice *d)
+static void xio3130_downstream_realize(PCIDevice *d, Error **errp)
 {
 PCIEPort *p = PCIE_PORT(d);
 PCIESlot *s = PCIE_SLOT(d);
 int rc;
-Error *err = NULL;
 
 pci_bridge_initfn(d, TYPE_PCIE_BUS);
 pcie_port_init_reg(d);
 
 rc = msi_init(d, XIO3130_MSI_OFFSET, XIO3130_MSI_NR_VECTOR,
   XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_64BIT,
-  XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT, );
+  XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
+  errp);
 if (rc < 0) {
 assert(rc == -ENOTSUP);
-