* [Qemu-devel] [PATCH v7 1/9] pci: Clean up error checking in pci_add_capability()
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 2/9] pci: Add comment for pci_add_capability2() Mao Zhongyi
` (8 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel
On success, pci_add_capability2() returns a positive value. On
failure, it sets an error and return a negative value.
pci_add_capability() laboriously checks this behavior. No other
caller does. Drop the checks from pci_add_capability().
Cc: mst@redhat.com
Cc: marcel@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci/pci.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 98ccc27..53566b8 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2270,12 +2270,8 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
Error *local_err = NULL;
ret = pci_add_capability2(pdev, cap_id, offset, size, &local_err);
- if (local_err) {
- assert(ret < 0);
+ if (ret < 0) {
error_report_err(local_err);
- } else {
- /* success implies a positive offset in config space */
- assert(ret > 0);
}
return ret;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 2/9] pci: Add comment for pci_add_capability2()
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 1/9] pci: Clean up error checking in pci_add_capability() Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 3/9] pci: Fix the wrong assertion Mao Zhongyi
` (7 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel, armbru
Comments for pci_add_capability2() to explain the return
value. This may help to make a correct return value check
for its callers.
Cc: mst@redhat.com
Cc: marcel@redhat.com
Cc: armbru@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci/pci.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 53566b8..b73bfea 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2276,6 +2276,12 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
return ret;
}
+/*
+ * On success, pci_add_capability2() returns a positive value
+ * that the offset of the pci capability.
+ * On failure, it sets an error and returns a negative error
+ * code.
+ */
int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
Error **errp)
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 3/9] pci: Fix the wrong assertion.
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 1/9] pci: Clean up error checking in pci_add_capability() Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 2/9] pci: Add comment for pci_add_capability2() Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 4/9] pci: Make errp the last parameter of pci_add_capability() Mao Zhongyi
` (6 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: dmitry, jasowang, kraxel, alex.williamson, armbru, marcel
pci_add_capability returns a strictly positive value on success,
correct asserts.
Cc: dmitry@daynix.com
Cc: jasowang@redhat.com
Cc: kraxel@redhat.com
Cc: alex.williamson@redhat.com
Cc: armbru@redhat.com
Cc: marcel@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/net/e1000e.c | 2 +-
hw/net/eepro100.c | 2 +-
hw/usb/hcd-xhci.c | 2 +-
hw/vfio/pci.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 6e23493..8259d67 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -374,7 +374,7 @@ e1000e_add_pm_capability(PCIDevice *pdev, uint8_t offset, uint16_t pmc)
{
int ret = pci_add_capability(pdev, PCI_CAP_ID_PM, offset, PCI_PM_SIZEOF);
- if (ret >= 0) {
+ if (ret > 0) {
pci_set_word(pdev->config + offset + PCI_PM_PMC,
PCI_PM_CAP_VER_1_1 |
pmc);
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index 4bf71f2..da36816 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -571,7 +571,7 @@ static void e100_pci_reset(EEPRO100State * s)
int cfg_offset = 0xdc;
int r = pci_add_capability(&s->dev, PCI_CAP_ID_PM,
cfg_offset, PCI_PM_SIZEOF);
- assert(r >= 0);
+ assert(r > 0);
pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21);
#if 0 /* TODO: replace dummy code for power management emulation. */
/* TODO: Power Management Control / Status. */
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index a0c7960..ab42f86 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3417,7 +3417,7 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
if (pci_bus_is_express(dev->bus) ||
xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
ret = pcie_endpoint_cap_init(dev, 0xa0);
- assert(ret >= 0);
+ assert(ret > 0);
}
if (xhci->msix != ON_OFF_AUTO_OFF) {
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 32aca77..5881968 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1744,7 +1744,7 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
}
pos = pci_add_capability(&vdev->pdev, PCI_CAP_ID_EXP, pos, size);
- if (pos >= 0) {
+ if (pos > 0) {
vdev->pdev.exp.exp_cap = pos;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 4/9] pci: Make errp the last parameter of pci_add_capability()
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (2 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 3/9] pci: Fix the wrong assertion Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 5/9] pci: Replace pci_add_capability2() with pci_add_capability() Mao Zhongyi
` (5 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, rth, ehabkost, mst, jasowang, marcel, alex.williamson,
armbru
Add Error argument for pci_add_capability() to leverage the errp
to pass info on errors. This way is helpful for its callers to
make a better error handling when moving to 'realize'.
Cc: pbonzini@redhat.com
Cc: rth@twiddle.net
Cc: ehabkost@redhat.com
Cc: mst@redhat.com
Cc: jasowang@redhat.com
Cc: marcel@redhat.com
Cc: alex.williamson@redhat.com
Cc: armbru@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/amd_iommu.c | 24 +++++++++++++++++-------
hw/net/e1000e.c | 30 ++++++++++++++++++------------
hw/net/eepro100.c | 18 ++++++++++++++----
hw/pci-bridge/i82801b11.c | 1 +
hw/pci/pci.c | 10 ++++------
hw/pci/pci_bridge.c | 7 ++++++-
hw/pci/pcie.c | 10 ++++++++--
hw/pci/shpc.c | 5 ++++-
hw/pci/slotid_cap.c | 7 ++++++-
hw/vfio/pci.c | 9 ++++++---
hw/virtio/virtio-pci.c | 12 ++++++++----
include/hw/pci/pci.h | 3 ++-
12 files changed, 94 insertions(+), 42 deletions(-)
diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c
index 7b6d4ea..d93ffc2 100644
--- a/hw/i386/amd_iommu.c
+++ b/hw/i386/amd_iommu.c
@@ -1158,13 +1158,23 @@ static void amdvi_realize(DeviceState *dev, Error **err)
x86_iommu->type = TYPE_AMD;
qdev_set_parent_bus(DEVICE(&s->pci), &bus->qbus);
object_property_set_bool(OBJECT(&s->pci), true, "realized", err);
- s->capab_offset = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
- AMDVI_CAPAB_SIZE);
- assert(s->capab_offset > 0);
- ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0, AMDVI_CAPAB_REG_SIZE);
- assert(ret > 0);
- ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_HT, 0, AMDVI_CAPAB_REG_SIZE);
- assert(ret > 0);
+ ret = pci_add_capability(&s->pci.dev, AMDVI_CAPAB_ID_SEC, 0,
+ AMDVI_CAPAB_SIZE, err);
+ if (ret < 0) {
+ return;
+ }
+ s->capab_offset = ret;
+
+ ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_MSI, 0,
+ AMDVI_CAPAB_REG_SIZE, err);
+ if (ret < 0) {
+ return;
+ }
+ ret = pci_add_capability(&s->pci.dev, PCI_CAP_ID_HT, 0,
+ AMDVI_CAPAB_REG_SIZE, err);
+ if (ret < 0) {
+ return;
+ }
/* set up MMIO */
memory_region_init_io(&s->mmio, OBJECT(s), &mmio_mem_ops, s, "amdvi-mmio",
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 8259d67..d1b1a97 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -47,6 +47,7 @@
#include "e1000e_core.h"
#include "trace.h"
+#include "qapi/error.h"
#define TYPE_E1000E "e1000e"
#define E1000E(obj) OBJECT_CHECK(E1000EState, (obj), TYPE_E1000E)
@@ -372,21 +373,26 @@ e1000e_gen_dsn(uint8_t *mac)
static int
e1000e_add_pm_capability(PCIDevice *pdev, uint8_t offset, uint16_t pmc)
{
- int ret = pci_add_capability(pdev, PCI_CAP_ID_PM, offset, PCI_PM_SIZEOF);
+ Error *local_err = NULL;
+ int ret = pci_add_capability(pdev, PCI_CAP_ID_PM, offset,
+ PCI_PM_SIZEOF, &local_err);
- if (ret > 0) {
- pci_set_word(pdev->config + offset + PCI_PM_PMC,
- PCI_PM_CAP_VER_1_1 |
- pmc);
+ if (local_err) {
+ error_report_err(local_err);
+ return ret;
+ }
- pci_set_word(pdev->wmask + offset + PCI_PM_CTRL,
- PCI_PM_CTRL_STATE_MASK |
- PCI_PM_CTRL_PME_ENABLE |
- PCI_PM_CTRL_DATA_SEL_MASK);
+ pci_set_word(pdev->config + offset + PCI_PM_PMC,
+ PCI_PM_CAP_VER_1_1 |
+ pmc);
- pci_set_word(pdev->w1cmask + offset + PCI_PM_CTRL,
- PCI_PM_CTRL_PME_STATUS);
- }
+ pci_set_word(pdev->wmask + offset + PCI_PM_CTRL,
+ PCI_PM_CTRL_STATE_MASK |
+ PCI_PM_CTRL_PME_ENABLE |
+ PCI_PM_CTRL_DATA_SEL_MASK);
+
+ pci_set_word(pdev->w1cmask + offset + PCI_PM_CTRL,
+ PCI_PM_CTRL_PME_STATUS);
return ret;
}
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index da36816..5a4774a 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -48,6 +48,7 @@
#include "sysemu/sysemu.h"
#include "sysemu/dma.h"
#include "qemu/bitops.h"
+#include "qapi/error.h"
/* QEMU sends frames smaller than 60 bytes to ethernet nics.
* Such frames are rejected by real nics and their emulations.
@@ -494,7 +495,7 @@ static void eepro100_fcp_interrupt(EEPRO100State * s)
}
#endif
-static void e100_pci_reset(EEPRO100State * s)
+static void e100_pci_reset(EEPRO100State *s, Error **errp)
{
E100PCIDeviceInfo *info = eepro100_get_class(s);
uint32_t device = s->device;
@@ -570,8 +571,12 @@ static void e100_pci_reset(EEPRO100State * s)
/* Power Management Capabilities */
int cfg_offset = 0xdc;
int r = pci_add_capability(&s->dev, PCI_CAP_ID_PM,
- cfg_offset, PCI_PM_SIZEOF);
- assert(r > 0);
+ cfg_offset, PCI_PM_SIZEOF,
+ errp);
+ if (r < 0) {
+ return;
+ }
+
pci_set_word(pci_conf + cfg_offset + PCI_PM_PMC, 0x7e21);
#if 0 /* TODO: replace dummy code for power management emulation. */
/* TODO: Power Management Control / Status. */
@@ -1858,12 +1863,17 @@ static void e100_nic_realize(PCIDevice *pci_dev, Error **errp)
{
EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
E100PCIDeviceInfo *info = eepro100_get_class(s);
+ Error *local_err = NULL;
TRACE(OTHER, logout("\n"));
s->device = info->device;
- e100_pci_reset(s);
+ e100_pci_reset(s, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
/* Add 64 * 2 EEPROM. i82557 and i82558 support a 64 word EEPROM,
* i82559 and later support 64 or 256 word EEPROM. */
diff --git a/hw/pci-bridge/i82801b11.c b/hw/pci-bridge/i82801b11.c
index 2404e7e..2c065c3 100644
--- a/hw/pci-bridge/i82801b11.c
+++ b/hw/pci-bridge/i82801b11.c
@@ -44,6 +44,7 @@
#include "qemu/osdep.h"
#include "hw/pci/pci.h"
#include "hw/i386/ich9.h"
+#include "qapi/error.h"
/*****************************************************************************/
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b73bfea..2bba37a 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2264,15 +2264,13 @@ static void pci_del_option_rom(PCIDevice *pdev)
* in pci config space
*/
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
- uint8_t offset, uint8_t size)
+ uint8_t offset, uint8_t size,
+ Error **errp)
{
int ret;
- Error *local_err = NULL;
- ret = pci_add_capability2(pdev, cap_id, offset, size, &local_err);
- if (ret < 0) {
- error_report_err(local_err);
- }
+ ret = pci_add_capability2(pdev, cap_id, offset, size, errp);
+
return ret;
}
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index 5118ef4..bb0f3a3 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -33,6 +33,7 @@
#include "hw/pci/pci_bridge.h"
#include "hw/pci/pci_bus.h"
#include "qemu/range.h"
+#include "qapi/error.h"
/* PCI bridge subsystem vendor ID helper functions */
#define PCI_SSVID_SIZEOF 8
@@ -43,8 +44,12 @@ int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
uint16_t svid, uint16_t ssid)
{
int pos;
- pos = pci_add_capability(dev, PCI_CAP_ID_SSVID, offset, PCI_SSVID_SIZEOF);
+ Error *local_err = NULL;
+
+ pos = pci_add_capability(dev, PCI_CAP_ID_SSVID, offset,
+ PCI_SSVID_SIZEOF, &local_err);
if (pos < 0) {
+ error_report_err(local_err);
return pos;
}
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 18e634f..f187512 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -91,11 +91,14 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
/* PCIe cap v2 init */
int pos;
uint8_t *exp_cap;
+ Error *local_err = NULL;
assert(pci_is_express(dev));
- pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, PCI_EXP_VER2_SIZEOF);
+ pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
+ PCI_EXP_VER2_SIZEOF, &local_err);
if (pos < 0) {
+ error_report_err(local_err);
return pos;
}
dev->exp.exp_cap = pos;
@@ -123,11 +126,14 @@ int pcie_cap_v1_init(PCIDevice *dev, uint8_t offset, uint8_t type,
{
/* PCIe cap v1 init */
int pos;
+ Error *local_err = NULL;
assert(pci_is_express(dev));
- pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset, PCI_EXP_VER1_SIZEOF);
+ pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
+ PCI_EXP_VER1_SIZEOF, &local_err);
if (pos < 0) {
+ error_report_err(local_err);
return pos;
}
dev->exp.exp_cap = pos;
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 42fafac..d72d5e4 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -450,9 +450,12 @@ static int shpc_cap_add_config(PCIDevice *d)
{
uint8_t *config;
int config_offset;
+ Error *local_err = NULL;
config_offset = pci_add_capability(d, PCI_CAP_ID_SHPC,
- 0, SHPC_CAP_LENGTH);
+ 0, SHPC_CAP_LENGTH,
+ &local_err);
if (config_offset < 0) {
+ error_report_err(local_err);
return config_offset;
}
config = d->config + config_offset;
diff --git a/hw/pci/slotid_cap.c b/hw/pci/slotid_cap.c
index aec1e91..bdca205 100644
--- a/hw/pci/slotid_cap.c
+++ b/hw/pci/slotid_cap.c
@@ -2,6 +2,7 @@
#include "hw/pci/slotid_cap.h"
#include "hw/pci/pci.h"
#include "qemu/error-report.h"
+#include "qapi/error.h"
#define SLOTID_CAP_LENGTH 4
#define SLOTID_NSLOTS_SHIFT ctz32(PCI_SID_ESR_NSLOTS)
@@ -11,6 +12,8 @@ int slotid_cap_init(PCIDevice *d, int nslots,
unsigned offset)
{
int cap;
+ Error *local_err = NULL;
+
if (!chassis) {
error_report("Bridge chassis not specified. Each bridge is required "
"to be assigned a unique chassis id > 0.");
@@ -21,8 +24,10 @@ int slotid_cap_init(PCIDevice *d, int nslots,
return -EINVAL;
}
- cap = pci_add_capability(d, PCI_CAP_ID_SLOTID, offset, SLOTID_CAP_LENGTH);
+ cap = pci_add_capability(d, PCI_CAP_ID_SLOTID, offset,
+ SLOTID_CAP_LENGTH, &local_err);
if (cap < 0) {
+ error_report_err(local_err);
return cap;
}
/* We make each chassis unique, this way each bridge is First in Chassis */
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 5881968..70bfb59 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1743,11 +1743,14 @@ static int vfio_setup_pcie_cap(VFIOPCIDevice *vdev, int pos, uint8_t size,
PCI_EXP_LNKCAP_MLW | PCI_EXP_LNKCAP_SLS);
}
- pos = pci_add_capability(&vdev->pdev, PCI_CAP_ID_EXP, pos, size);
- if (pos > 0) {
- vdev->pdev.exp.exp_cap = pos;
+ pos = pci_add_capability(&vdev->pdev, PCI_CAP_ID_EXP, pos, size,
+ errp);
+ if (pos < 0) {
+ return pos;
}
+ vdev->pdev.exp.exp_cap = pos;
+
return pos;
}
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index f9b7244..1fc5059 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1162,8 +1162,8 @@ static int virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
PCIDevice *dev = &proxy->pci_dev;
int offset;
- offset = pci_add_capability(dev, PCI_CAP_ID_VNDR, 0, cap->cap_len);
- assert(offset > 0);
+ offset = pci_add_capability(dev, PCI_CAP_ID_VNDR, 0,
+ cap->cap_len, &error_abort);
assert(cap->cap_len >= sizeof *cap);
memcpy(dev->config + offset + PCI_CAP_FLAGS, &cap->cap_len,
@@ -1810,8 +1810,12 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error **errp)
pos = pcie_endpoint_cap_init(pci_dev, 0);
assert(pos > 0);
- pos = pci_add_capability(pci_dev, PCI_CAP_ID_PM, 0, PCI_PM_SIZEOF);
- assert(pos > 0);
+ pos = pci_add_capability(pci_dev, PCI_CAP_ID_PM, 0,
+ PCI_PM_SIZEOF, errp);
+ if (pos < 0) {
+ return;
+ }
+
pci_dev->exp.pm_cap = pos;
/*
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index a37a2d5..fe52aa8 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -356,7 +356,8 @@ void pci_unregister_vga(PCIDevice *pci_dev);
pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
- uint8_t offset, uint8_t size);
+ uint8_t offset, uint8_t size,
+ Error **errp);
int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
Error **errp);
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 5/9] pci: Replace pci_add_capability2() with pci_add_capability()
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (3 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 4/9] pci: Make errp the last parameter of pci_add_capability() Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 6/9] pci: Convert to realize Mao Zhongyi
` (4 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel
Cc: pbonzini, rth, ehabkost, mst, dmitry, jasowang, marcel,
alex.williamson, armbru
After the patch 'Make errp the last parameter of pci_add_capability()',
pci_add_capability() and pci_add_capability2() now do exactly the same.
So drop the wrapper pci_add_capability() of pci_add_capability2(), then
replace the pci_add_capability2() with pci_add_capability() everywhere.
Cc: pbonzini@redhat.com
Cc: rth@twiddle.net
Cc: ehabkost@redhat.com
Cc: mst@redhat.com
Cc: dmitry@daynix.com
Cc: jasowang@redhat.com
Cc: marcel@redhat.com
Cc: alex.williamson@redhat.com
Cc: armbru@redhat.com
Suggested-by: Eduardo Habkost <ehabkost@redhat.com>
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/kvm/pci-assign.c | 14 +++++++-------
hw/ide/ich.c | 2 +-
hw/pci/msi.c | 2 +-
hw/pci/msix.c | 2 +-
hw/pci/pci.c | 20 ++------------------
hw/vfio/pci.c | 6 +++---
include/hw/pci/pci.h | 3 ---
7 files changed, 15 insertions(+), 34 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 87dcbdd..3d60455 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1254,7 +1254,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
dev->dev.cap_present |= QEMU_PCI_CAP_MSI;
dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
/* Only 32-bit/no-mask currently supported */
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSI, pos, 10,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1288,7 +1288,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
}
dev->dev.cap_present |= QEMU_PCI_CAP_MSIX;
dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1318,7 +1318,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
if (pos) {
uint16_t pmc;
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1386,7 +1386,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
return -EINVAL;
}
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_EXP, pos, size,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, size,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1462,7 +1462,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
uint32_t status;
/* Only expose the minimum, 8 byte capability */
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_PCIX, pos, 8,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1490,7 +1490,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD, 0);
if (pos) {
/* Direct R/W passthrough */
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VPD, pos, 8,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
@@ -1508,7 +1508,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
pos += PCI_CAP_LIST_NEXT) {
uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS);
/* Direct R/W passthrough */
- ret = pci_add_capability2(pci_dev, PCI_CAP_ID_VNDR, pos, len,
+ ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len,
&local_err);
if (ret < 0) {
error_propagate(errp, local_err);
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 4599169..989fca5 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -130,7 +130,7 @@ static void pci_ich9_ahci_realize(PCIDevice *dev, Error **errp)
pci_register_bar(dev, ICH9_MEM_BAR, PCI_BASE_ADDRESS_SPACE_MEMORY,
&d->ahci.mem);
- sata_cap_offset = pci_add_capability2(dev, PCI_CAP_ID_SATA,
+ sata_cap_offset = pci_add_capability(dev, PCI_CAP_ID_SATA,
ICH9_SATA_CAP_OFFSET, SATA_CAP_SIZE,
errp);
if (sata_cap_offset < 0) {
diff --git a/hw/pci/msi.c b/hw/pci/msi.c
index a87b227..5e05ce5 100644
--- a/hw/pci/msi.c
+++ b/hw/pci/msi.c
@@ -216,7 +216,7 @@ int msi_init(struct PCIDevice *dev, uint8_t offset,
}
cap_size = msi_cap_sizeof(flags);
- config_offset = pci_add_capability2(dev, PCI_CAP_ID_MSI, offset,
+ config_offset = pci_add_capability(dev, PCI_CAP_ID_MSI, offset,
cap_size, errp);
if (config_offset < 0) {
return config_offset;
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index bb54e8b..d634326 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -294,7 +294,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
return -EINVAL;
}
- cap = pci_add_capability2(dev, PCI_CAP_ID_MSIX,
+ cap = pci_add_capability(dev, PCI_CAP_ID_MSIX,
cap_pos, MSIX_CAP_LENGTH, errp);
if (cap < 0) {
return cap;
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 2bba37a..283ca5e 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -2259,28 +2259,12 @@ static void pci_del_option_rom(PCIDevice *pdev)
}
/*
- * if offset = 0,
- * Find and reserve space and add capability to the linked list
- * in pci config space
- */
-int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
- uint8_t offset, uint8_t size,
- Error **errp)
-{
- int ret;
-
- ret = pci_add_capability2(pdev, cap_id, offset, size, errp);
-
- return ret;
-}
-
-/*
- * On success, pci_add_capability2() returns a positive value
+ * On success, pci_add_capability() returns a positive value
* that the offset of the pci capability.
* On failure, it sets an error and returns a negative error
* code.
*/
-int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
+int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
Error **errp)
{
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 70bfb59..8de8272 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -1837,14 +1837,14 @@ static int vfio_add_std_cap(VFIOPCIDevice *vdev, uint8_t pos, Error **errp)
case PCI_CAP_ID_PM:
vfio_check_pm_reset(vdev, pos);
vdev->pm_cap = pos;
- ret = pci_add_capability2(pdev, cap_id, pos, size, errp);
+ ret = pci_add_capability(pdev, cap_id, pos, size, errp);
break;
case PCI_CAP_ID_AF:
vfio_check_af_flr(vdev, pos);
- ret = pci_add_capability2(pdev, cap_id, pos, size, errp);
+ ret = pci_add_capability(pdev, cap_id, pos, size, errp);
break;
default:
- ret = pci_add_capability2(pdev, cap_id, pos, size, errp);
+ ret = pci_add_capability(pdev, cap_id, pos, size, errp);
break;
}
out:
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index fe52aa8..e598b09 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -358,9 +358,6 @@ pcibus_t pci_get_bar_addr(PCIDevice *pci_dev, int region_num);
int pci_add_capability(PCIDevice *pdev, uint8_t cap_id,
uint8_t offset, uint8_t size,
Error **errp);
-int pci_add_capability2(PCIDevice *pdev, uint8_t cap_id,
- uint8_t offset, uint8_t size,
- Error **errp);
void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 6/9] pci: Convert to realize
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (4 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 5/9] pci: Replace pci_add_capability2() with pci_add_capability() Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 7/9] pci: Convert shpc_init() to Error Mao Zhongyi
` (3 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel, armbru
Convert i82801b11, io3130_upstream, io3130_downstream and
pcie_root_port devices to realize.
Cc: mst@redhat.com
Cc: marcel@redhat.com
Cc: armbru@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci-bridge/i82801b11.c | 11 +++++------
hw/pci-bridge/pcie_root_port.c | 18 +++++++++---------
hw/pci-bridge/xio3130_downstream.c | 20 +++++++++-----------
hw/pci-bridge/xio3130_upstream.c | 20 +++++++++-----------
hw/pci/pci_bridge.c | 7 +++----
hw/pci/pcie.c | 24 +++++++++++++++++-------
include/hw/pci/pci_bridge.h | 3 ++-
include/hw/pci/pcie.h | 3 ++-
8 files changed, 56 insertions(+), 50 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 = &i82801b11_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..4d588cb 100644
--- a/hw/pci-bridge/pcie_root_port.c
+++ b/hw/pci-bridge/pcie_root_port.c
@@ -59,29 +59,30 @@ 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);
+ error_append_hint(errp, "Can't init SSV ID, error %d\n", rc);
goto err_bridge;
}
if (rpc->interrupts_init) {
- rc = rpc->interrupts_init(d, &local_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);
+ error_append_hint(errp, "Can't add Root Port capability, "
+ "error %d\n", rc);
goto err_int;
}
@@ -98,9 +99,8 @@ static void rp_realize(PCIDevice *d, Error **errp)
}
rc = pcie_aer_init(d, PCI_ERR_VER, rpc->aer_offset,
- PCI_ERR_SIZEOF, &local_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, &err);
+ XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
+ errp);
if (rc < 0) {
assert(rc == -ENOTSUP);
- error_report_err(err);
goto err_bridge;
}
rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
- XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
+ XIO3130_SSVID_SVID, XIO3130_SSVID_SSID,
+ errp);
if (rc < 0) {
goto err_bridge;
}
rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_DOWNSTREAM,
- p->port);
+ p->port, errp);
if (rc < 0) {
goto err_msi;
}
@@ -98,13 +98,12 @@ static int xio3130_downstream_initfn(PCIDevice *d)
}
rc = pcie_aer_init(d, PCI_ERR_VER, XIO3130_AER_OFFSET,
- PCI_ERR_SIZEOF, &err);
+ PCI_ERR_SIZEOF, errp);
if (rc < 0) {
- error_report_err(err);
goto err;
}
- return 0;
+ return;
err:
pcie_chassis_del_slot(s);
@@ -114,7 +113,6 @@ err_msi:
msi_uninit(d);
err_bridge:
pci_bridge_exitfn(d);
- return rc;
}
static void xio3130_downstream_exitfn(PCIDevice *d)
@@ -181,7 +179,7 @@ static void xio3130_downstream_class_init(ObjectClass *klass, void *data)
k->is_express = 1;
k->is_bridge = 1;
k->config_write = xio3130_downstream_write_config;
- k->init = xio3130_downstream_initfn;
+ k->realize = xio3130_downstream_realize;
k->exit = xio3130_downstream_exitfn;
k->vendor_id = PCI_VENDOR_ID_TI;
k->device_id = PCI_DEVICE_ID_TI_XIO3130D;
diff --git a/hw/pci-bridge/xio3130_upstream.c b/hw/pci-bridge/xio3130_upstream.c
index 401c784..a052224 100644
--- a/hw/pci-bridge/xio3130_upstream.c
+++ b/hw/pci-bridge/xio3130_upstream.c
@@ -53,32 +53,32 @@ static void xio3130_upstream_reset(DeviceState *qdev)
pcie_cap_deverr_reset(d);
}
-static int xio3130_upstream_initfn(PCIDevice *d)
+static void xio3130_upstream_realize(PCIDevice *d, Error **errp)
{
PCIEPort *p = PCIE_PORT(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, &err);
+ XIO3130_MSI_SUPPORTED_FLAGS & PCI_MSI_FLAGS_MASKBIT,
+ errp);
if (rc < 0) {
assert(rc == -ENOTSUP);
- error_report_err(err);
goto err_bridge;
}
rc = pci_bridge_ssvid_init(d, XIO3130_SSVID_OFFSET,
- XIO3130_SSVID_SVID, XIO3130_SSVID_SSID);
+ XIO3130_SSVID_SVID, XIO3130_SSVID_SSID,
+ errp);
if (rc < 0) {
goto err_bridge;
}
rc = pcie_cap_init(d, XIO3130_EXP_OFFSET, PCI_EXP_TYPE_UPSTREAM,
- p->port);
+ p->port, errp);
if (rc < 0) {
goto err_msi;
}
@@ -86,13 +86,12 @@ static int xio3130_upstream_initfn(PCIDevice *d)
pcie_cap_deverr_init(d);
rc = pcie_aer_init(d, PCI_ERR_VER, XIO3130_AER_OFFSET,
- PCI_ERR_SIZEOF, &err);
+ PCI_ERR_SIZEOF, errp);
if (rc < 0) {
- error_report_err(err);
goto err;
}
- return 0;
+ return;
err:
pcie_cap_exit(d);
@@ -100,7 +99,6 @@ err_msi:
msi_uninit(d);
err_bridge:
pci_bridge_exitfn(d);
- return rc;
}
static void xio3130_upstream_exitfn(PCIDevice *d)
@@ -153,7 +151,7 @@ static void xio3130_upstream_class_init(ObjectClass *klass, void *data)
k->is_express = 1;
k->is_bridge = 1;
k->config_write = xio3130_upstream_write_config;
- k->init = xio3130_upstream_initfn;
+ k->realize = xio3130_upstream_realize;
k->exit = xio3130_upstream_exitfn;
k->vendor_id = PCI_VENDOR_ID_TI;
k->device_id = PCI_DEVICE_ID_TI_XIO3130U;
diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c
index bb0f3a3..720119b 100644
--- a/hw/pci/pci_bridge.c
+++ b/hw/pci/pci_bridge.c
@@ -41,15 +41,14 @@
#define PCI_SSVID_SSID 6
int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
- uint16_t svid, uint16_t ssid)
+ uint16_t svid, uint16_t ssid,
+ Error **errp)
{
int pos;
- Error *local_err = NULL;
pos = pci_add_capability(dev, PCI_CAP_ID_SSVID, offset,
- PCI_SSVID_SIZEOF, &local_err);
+ PCI_SSVID_SIZEOF, errp);
if (pos < 0) {
- error_report_err(local_err);
return pos;
}
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index f187512..32191f2 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -86,19 +86,19 @@ pcie_cap_v1_fill(PCIDevice *dev, uint8_t port, uint8_t type, uint8_t version)
pci_set_word(cmask + PCI_EXP_LNKSTA, 0);
}
-int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port)
+int pcie_cap_init(PCIDevice *dev, uint8_t offset,
+ uint8_t type, uint8_t port,
+ Error **errp)
{
/* PCIe cap v2 init */
int pos;
uint8_t *exp_cap;
- Error *local_err = NULL;
assert(pci_is_express(dev));
pos = pci_add_capability(dev, PCI_CAP_ID_EXP, offset,
- PCI_EXP_VER2_SIZEOF, &local_err);
+ PCI_EXP_VER2_SIZEOF, errp);
if (pos < 0) {
- error_report_err(local_err);
return pos;
}
dev->exp.exp_cap = pos;
@@ -147,6 +147,8 @@ static int
pcie_endpoint_cap_common_init(PCIDevice *dev, uint8_t offset, uint8_t cap_size)
{
uint8_t type = PCI_EXP_TYPE_ENDPOINT;
+ Error *local_err = NULL;
+ int ret;
/*
* Windows guests will report Code 10, device cannot start, if
@@ -157,9 +159,17 @@ pcie_endpoint_cap_common_init(PCIDevice *dev, uint8_t offset, uint8_t cap_size)
type = PCI_EXP_TYPE_RC_END;
}
- return (cap_size == PCI_EXP_VER1_SIZEOF)
- ? pcie_cap_v1_init(dev, offset, type, 0)
- : pcie_cap_init(dev, offset, type, 0);
+ if (cap_size == PCI_EXP_VER1_SIZEOF) {
+ return pcie_cap_v1_init(dev, offset, type, 0);
+ } else {
+ ret = pcie_cap_init(dev, offset, type, 0, &local_err);
+
+ if (ret < 0) {
+ error_report_err(local_err);
+ }
+
+ return ret;
+ }
}
int pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset)
diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h
index d5891cd..ff7cbaa 100644
--- a/include/hw/pci/pci_bridge.h
+++ b/include/hw/pci/pci_bridge.h
@@ -33,7 +33,8 @@
#define PCI_BRIDGE_DEV_PROP_SHPC "shpc"
int pci_bridge_ssvid_init(PCIDevice *dev, uint8_t offset,
- uint16_t svid, uint16_t ssid);
+ uint16_t svid, uint16_t ssid,
+ Error **errp);
PCIDevice *pci_bridge_get_device(PCIBus *bus);
PCIBus *pci_bridge_get_sec_bus(PCIBridge *br);
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 3d8f24b..b71e369 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -84,7 +84,8 @@ struct PCIExpressDevice {
#define COMPAT_PROP_PCP "power_controller_present"
/* PCI express capability helper functions */
-int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type, uint8_t port);
+int pcie_cap_init(PCIDevice *dev, uint8_t offset, uint8_t type,
+ uint8_t port, Error **errp);
int pcie_cap_v1_init(PCIDevice *dev, uint8_t offset,
uint8_t type, uint8_t port);
int pcie_endpoint_cap_init(PCIDevice *dev, uint8_t offset);
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 7/9] pci: Convert shpc_init() to Error
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (5 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 6/9] pci: Convert to realize Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel() Mao Zhongyi
` (2 subsequent siblings)
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: mst, marcel, armbru
In order to propagate error message better, convert shpc_init() to
Error also convert the pci_bridge_dev_initfn() to realize.
Cc: mst@redhat.com
Cc: marcel@redhat.com
Cc: armbru@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/pci-bridge/pci_bridge_dev.c | 14 ++++++--------
hw/pci/shpc.c | 11 +++++------
hw/pci/slotid_cap.c | 11 +++++------
include/hw/pci/shpc.h | 3 ++-
include/hw/pci/slotid_cap.h | 3 ++-
5 files changed, 20 insertions(+), 22 deletions(-)
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 5dbd933..4373f1d 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -49,7 +49,7 @@ struct PCIBridgeDev {
};
typedef struct PCIBridgeDev PCIBridgeDev;
-static int pci_bridge_dev_initfn(PCIDevice *dev)
+static void pci_bridge_dev_realize(PCIDevice *dev, Error **errp)
{
PCIBridge *br = PCI_BRIDGE(dev);
PCIBridgeDev *bridge_dev = PCI_BRIDGE_DEV(dev);
@@ -62,7 +62,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
dev->config[PCI_INTERRUPT_PIN] = 0x1;
memory_region_init(&bridge_dev->bar, OBJECT(dev), "shpc-bar",
shpc_bar_size(dev));
- err = shpc_init(dev, &br->sec_bus, &bridge_dev->bar, 0);
+ err = shpc_init(dev, &br->sec_bus, &bridge_dev->bar, 0, errp);
if (err) {
goto shpc_error;
}
@@ -71,7 +71,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
bridge_dev->msi = ON_OFF_AUTO_OFF;
}
- err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0);
+ err = slotid_cap_init(dev, 0, bridge_dev->chassis_nr, 0, errp);
if (err) {
goto slotid_error;
}
@@ -87,7 +87,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
/* Can't satisfy user's explicit msi=on request, fail */
error_append_hint(&local_err, "You have to use msi=auto (default) "
"or msi=off with this machine type.\n");
- error_report_err(local_err);
+ error_propagate(errp, local_err);
goto msi_error;
}
assert(!local_err || bridge_dev->msi == ON_OFF_AUTO_AUTO);
@@ -101,7 +101,7 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
pci_register_bar(dev, 0, PCI_BASE_ADDRESS_SPACE_MEMORY |
PCI_BASE_ADDRESS_MEM_TYPE_64, &bridge_dev->bar);
}
- return 0;
+ return;
msi_error:
slotid_cap_cleanup(dev);
@@ -111,8 +111,6 @@ slotid_error:
}
shpc_error:
pci_bridge_exitfn(dev);
-
- return err;
}
static void pci_bridge_dev_exitfn(PCIDevice *dev)
@@ -216,7 +214,7 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
- k->init = pci_bridge_dev_initfn;
+ k->realize = pci_bridge_dev_realize;
k->exit = pci_bridge_dev_exitfn;
k->config_write = pci_bridge_dev_write_config;
k->vendor_id = PCI_VENDOR_ID_REDHAT;
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index d72d5e4..69fc14b 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -446,16 +446,14 @@ static void shpc_cap_update_dword(PCIDevice *d)
}
/* Add SHPC capability to the config space for the device. */
-static int shpc_cap_add_config(PCIDevice *d)
+static int shpc_cap_add_config(PCIDevice *d, Error **errp)
{
uint8_t *config;
int config_offset;
- Error *local_err = NULL;
config_offset = pci_add_capability(d, PCI_CAP_ID_SHPC,
0, SHPC_CAP_LENGTH,
- &local_err);
+ errp);
if (config_offset < 0) {
- error_report_err(local_err);
return config_offset;
}
config = d->config + config_offset;
@@ -584,13 +582,14 @@ void shpc_device_hot_unplug_request_cb(HotplugHandler *hotplug_dev,
}
/* Initialize the SHPC structure in bridge's BAR. */
-int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar, unsigned offset)
+int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar,
+ unsigned offset, Error **errp)
{
int i, ret;
int nslots = SHPC_MAX_SLOTS; /* TODO: qdev property? */
SHPCDevice *shpc = d->shpc = g_malloc0(sizeof(*d->shpc));
shpc->sec_bus = sec_bus;
- ret = shpc_cap_add_config(d);
+ ret = shpc_cap_add_config(d, errp);
if (ret) {
g_free(d->shpc);
return ret;
diff --git a/hw/pci/slotid_cap.c b/hw/pci/slotid_cap.c
index bdca205..36d021b 100644
--- a/hw/pci/slotid_cap.c
+++ b/hw/pci/slotid_cap.c
@@ -9,14 +9,14 @@
int slotid_cap_init(PCIDevice *d, int nslots,
uint8_t chassis,
- unsigned offset)
+ unsigned offset,
+ Error **errp)
{
int cap;
- Error *local_err = NULL;
if (!chassis) {
- error_report("Bridge chassis not specified. Each bridge is required "
- "to be assigned a unique chassis id > 0.");
+ error_setg(errp, "Bridge chassis not specified. Each bridge is required"
+ " to be assigned a unique chassis id > 0.");
return -EINVAL;
}
if (nslots < 0 || nslots > (PCI_SID_ESR_NSLOTS >> SLOTID_NSLOTS_SHIFT)) {
@@ -25,9 +25,8 @@ int slotid_cap_init(PCIDevice *d, int nslots,
}
cap = pci_add_capability(d, PCI_CAP_ID_SLOTID, offset,
- SLOTID_CAP_LENGTH, &local_err);
+ SLOTID_CAP_LENGTH, errp);
if (cap < 0) {
- error_report_err(local_err);
return cap;
}
/* We make each chassis unique, this way each bridge is First in Chassis */
diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h
index 71e836b..ee19fec 100644
--- a/include/hw/pci/shpc.h
+++ b/include/hw/pci/shpc.h
@@ -38,7 +38,8 @@ struct SHPCDevice {
void shpc_reset(PCIDevice *d);
int shpc_bar_size(PCIDevice *dev);
-int shpc_init(PCIDevice *dev, PCIBus *sec_bus, MemoryRegion *bar, unsigned off);
+int shpc_init(PCIDevice *dev, PCIBus *sec_bus, MemoryRegion *bar,
+ unsigned off, Error **errp);
void shpc_cleanup(PCIDevice *dev, MemoryRegion *bar);
void shpc_free(PCIDevice *dev);
void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len);
diff --git a/include/hw/pci/slotid_cap.h b/include/hw/pci/slotid_cap.h
index 70db047..a777ea0 100644
--- a/include/hw/pci/slotid_cap.h
+++ b/include/hw/pci/slotid_cap.h
@@ -5,7 +5,8 @@
int slotid_cap_init(PCIDevice *dev, int nslots,
uint8_t chassis,
- unsigned offset);
+ unsigned offset,
+ Error **errp);
void slotid_cap_cleanup(PCIDevice *dev);
#endif
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel()
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (6 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 7/9] pci: Convert shpc_init() to Error Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err Mao Zhongyi
2017-07-03 17:41 ` [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Michael S. Tsirkin
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, rth, ehabkost, mst, armbru, marcel
When the function no success value to transmit, it usually make the
function return void. It has turned out not to be a success, because
it means that the extra local_err variable and error_propagate() will
be needed. It leads to cumbersome code, therefore, transmit success/
failure in the return value is worth. So fix the return type to avoid
it.
Cc: pbonzini@redhat.com
Cc: rth@twiddle.net
Cc: ehabkost@redhat.com
Cc: mst@redhat.com
Cc: armbru@redhat.com
Cc: marcel@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/kvm/pci-assign.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index 3d60455..b7fdb47 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -824,12 +824,13 @@ static void assign_device(AssignedDevice *dev, Error **errp)
}
}
-static void verify_irqchip_in_kernel(Error **errp)
+static int verify_irqchip_in_kernel(Error **errp)
{
if (kvm_irqchip_in_kernel()) {
- return;
+ return -1;
}
error_setg(errp, "pci-assign requires KVM with in-kernel irqchip enabled");
+ return 0;
}
static int assign_intx(AssignedDevice *dev, Error **errp)
@@ -838,7 +839,6 @@ static int assign_intx(AssignedDevice *dev, Error **errp)
PCIINTxRoute intx_route;
bool intx_host_msi;
int r;
- Error *local_err = NULL;
/* Interrupt PIN 0 means don't use INTx */
if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0) {
@@ -846,9 +846,7 @@ static int assign_intx(AssignedDevice *dev, Error **errp)
return 0;
}
- verify_irqchip_in_kernel(&local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ if (verify_irqchip_in_kernel(errp) < 0) {
return -ENOTSUP;
}
@@ -1246,9 +1244,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
* MSI capability is the 1st capability in capability config */
pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
- verify_irqchip_in_kernel(&local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ if (verify_irqchip_in_kernel(errp) < 0) {
return -ENOTSUP;
}
dev->dev.cap_present |= QEMU_PCI_CAP_MSI;
@@ -1281,9 +1277,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
uint32_t msix_table_entry;
uint16_t msix_max;
- verify_irqchip_in_kernel(&local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ if (verify_irqchip_in_kernel(errp) < 0) {
return -ENOTSUP;
}
dev->dev.cap_present |= QEMU_PCI_CAP_MSIX;
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [Qemu-devel] [PATCH v7 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (7 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel() Mao Zhongyi
@ 2017-06-27 6:16 ` Mao Zhongyi
2017-07-03 17:41 ` [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Michael S. Tsirkin
9 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-06-27 6:16 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, rth, ehabkost, mst, armbru, marcel
In assigned_device_pci_cap_init(), first, error messages are filled
to a local_err variable, then through error_propagate() pass to
the parameter of errp. It leads to cumbersome code. In order to
avoid the extra local_err and error_propagate(), drop it and use
errp instead.
Cc: pbonzini@redhat.com
Cc: rth@twiddle.net
Cc: ehabkost@redhat.com
Cc: mst@redhat.com
Cc: armbru@redhat.com
Cc: marcel@redhat.com
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/kvm/pci-assign.c | 22 +++++++---------------
1 file changed, 7 insertions(+), 15 deletions(-)
diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
index b7fdb47..9f2615c 100644
--- a/hw/i386/kvm/pci-assign.c
+++ b/hw/i386/kvm/pci-assign.c
@@ -1232,7 +1232,6 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
AssignedDevice *dev = PCI_ASSIGN(pci_dev);
PCIRegion *pci_region = dev->real_device.regions;
int ret, pos;
- Error *local_err = NULL;
/* Clear initial capabilities pointer and status copied from hw */
pci_set_byte(pci_dev->config + PCI_CAPABILITY_LIST, 0);
@@ -1251,9 +1250,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
/* Only 32-bit/no-mask currently supported */
ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
pci_dev->msi_cap = pos;
@@ -1283,9 +1281,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
dev->dev.cap_present |= QEMU_PCI_CAP_MSIX;
dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
pci_dev->msix_cap = pos;
@@ -1313,9 +1310,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
uint16_t pmc;
ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
@@ -1381,9 +1377,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
}
ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, size,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
@@ -1457,9 +1452,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
/* Only expose the minimum, 8 byte capability */
ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
@@ -1485,9 +1479,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
if (pos) {
/* Direct R/W passthrough */
ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
@@ -1503,9 +1496,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev, Error **errp)
uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS);
/* Direct R/W passthrough */
ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len,
- &local_err);
+ errp);
if (ret < 0) {
- error_propagate(errp, local_err);
return ret;
}
--
2.9.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup
2017-06-27 6:16 [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Mao Zhongyi
` (8 preceding siblings ...)
2017-06-27 6:16 ` [Qemu-devel] [PATCH v7 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err Mao Zhongyi
@ 2017-07-03 17:41 ` Michael S. Tsirkin
2017-07-04 1:58 ` Mao Zhongyi
9 siblings, 1 reply; 12+ messages in thread
From: Michael S. Tsirkin @ 2017-07-03 17:41 UTC (permalink / raw)
To: Mao Zhongyi
Cc: qemu-devel, marcel, armbru, dmitry, jasowang, kraxel,
alex.williamson, pbonzini, rth, ehabkost
On Tue, Jun 27, 2017 at 02:16:46PM +0800, Mao Zhongyi wrote:
> This series mainly implements the conversions of pci-bridge devices
> i82801b11, io3130_upstream/downstream and so on to realize(). Naturally
> part of error messages need to be converted to Error, then propagate
> to its callers via the argument errp, bonus clean related minor flaw
> up. In short, the former patches are prerequisites for latter ones.
Applied, thanks!
Please remember to Cc maintainers on all patches, this was
missing on patch 3.
> v7:
> * patch7: -drop the !local_err assert is really not appropriate,
> now revert it. [Marcel Apfelbaum]
>
> v6:
> * patch3: -rename the subject.
> * patch6: -simplify the commit message.
> -use error_append_hint replace original error_setg rather
> than remove it directly. [Marcel Apfelbaum]
> -report the error message from local_err. [Marcel Apfelbaum]
> v5:
> * patch5: replace pci_add_capability2() with pci_add_capability(), because
> it's confusing to have a function named pci_add_capability2() if
> pci_add_capability() doesn't exist anymore. [Eduardo Habkost]
> * patch8: a new patch that fix the return type of verify_irqchip_kernel().
> * patch9: a new patch that use the errp directly instead of the local_err to
> propagate the error messages.
>
> v4:
> * patch4: changed from patch 5 in v3. use a elegant way to check
> the error, like
>
> function(...);
> if (function succeeded) {
> /* non-error code path here */
> foo = bar;
> }
>
> or
>
> function(...);
> if (function succeeded) {
> /* non-error code path here */
> foo = bar;
> } else {
> /* error path here */
> return ret;
> }
>
> for readability, instead of this:
>
> function(...)
> if (function failed) {
> return ...; /* or: "goto out" */
> }
>
> /* non-error code path here */
> foo = bar; [Eduardo Habkost]
>
> meanwhile, split previous patch4 out. [Michael S. Tsirkin]
> * patch5: a new patch that replace pci_add_capability() with
> pci_add_capability2(). [Eduardo Habkost]
>
> v3:
> * patch2: explain the specified means of the return value, also
> improve the commit message. [Marcel Apfelbaum]
> * patch3: simplify the subject and commit message, fix another
> wrong assert. [Marcel Apfelbaum]
> * patch4: adjust the subject.
> * patch5: fix a wrong optimization for errp. [Eduardo Habkost]
> * patch7: a new patch that converts shpc_init() to Error in order
> to propagate the error better.
> v2:
> * patch1: subject and commit message was rewrited by markus.
> * patch2: comment was added to pci_add_capability2().
> * patch3: a new patch that fix the wrong return value judgment condition.
> * patch4: a new patch that fix code style problems.
> * patch5: add an errp argument for pci_add_capability to pass
> error for its callers.
> * patch6: convert part of pci-bridge device to realize.
>
> v1:
> * patch1: fix unreasonable return value check
>
> Cc: mst@redhat.com
> Cc: marcel@redhat.com
> Cc: armbru@redhat.com
> Cc: dmitry@daynix.com
> Cc: jasowang@redhat.com
> Cc: kraxel@redhat.com
> Cc: alex.williamson@redhat.com
> Cc: pbonzini@redhat.com
> Cc: rth@twiddle.net
> Cc: ehabkost@redhat.com
>
> Mao Zhongyi (9):
> pci: Clean up error checking in pci_add_capability()
> pci: Add comment for pci_add_capability2()
> pci: Fix the wrong assertion.
> pci: Make errp the last parameter of pci_add_capability()
> pci: Replace pci_add_capability2() with pci_add_capability()
> pci: Convert to realize
> pci: Convert shpc_init() to Error
> i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel()
> i386/kvm/pci-assign: Use errp directly rather than local_err
>
> hw/i386/amd_iommu.c | 24 ++++++++++++-----
> hw/i386/kvm/pci-assign.c | 54 ++++++++++++++------------------------
> hw/ide/ich.c | 2 +-
> hw/net/e1000e.c | 30 ++++++++++++---------
> hw/net/eepro100.c | 18 ++++++++++---
> hw/pci-bridge/i82801b11.c | 12 ++++-----
> hw/pci-bridge/pci_bridge_dev.c | 14 +++++-----
> hw/pci-bridge/pcie_root_port.c | 18 ++++++-------
> hw/pci-bridge/xio3130_downstream.c | 20 +++++++-------
> hw/pci-bridge/xio3130_upstream.c | 20 +++++++-------
> hw/pci/msi.c | 2 +-
> hw/pci/msix.c | 2 +-
> hw/pci/pci.c | 24 +++--------------
> hw/pci/pci_bridge.c | 8 ++++--
> hw/pci/pcie.c | 28 +++++++++++++++-----
> hw/pci/shpc.c | 10 ++++---
> hw/pci/slotid_cap.c | 12 ++++++---
> hw/usb/hcd-xhci.c | 2 +-
> hw/vfio/pci.c | 15 ++++++-----
> hw/virtio/virtio-pci.c | 12 ++++++---
> include/hw/pci/pci.h | 2 --
> include/hw/pci/pci_bridge.h | 3 ++-
> include/hw/pci/pcie.h | 3 ++-
> include/hw/pci/shpc.h | 3 ++-
> include/hw/pci/slotid_cap.h | 3 ++-
> 25 files changed, 183 insertions(+), 158 deletions(-)
>
> --
> 2.9.4
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup
2017-07-03 17:41 ` [Qemu-devel] [PATCH v7 0/9] Convert to realize and cleanup Michael S. Tsirkin
@ 2017-07-04 1:58 ` Mao Zhongyi
0 siblings, 0 replies; 12+ messages in thread
From: Mao Zhongyi @ 2017-07-04 1:58 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: qemu-devel, marcel, armbru, dmitry, jasowang, kraxel,
alex.williamson, pbonzini, rth, ehabkost
Hi, Michael
On 07/04/2017 01:41 AM, Michael S. Tsirkin wrote:
> On Tue, Jun 27, 2017 at 02:16:46PM +0800, Mao Zhongyi wrote:
>> This series mainly implements the conversions of pci-bridge devices
>> i82801b11, io3130_upstream/downstream and so on to realize(). Naturally
>> part of error messages need to be converted to Error, then propagate
>> to its callers via the argument errp, bonus clean related minor flaw
>> up. In short, the former patches are prerequisites for latter ones.
>
>
> Applied, thanks!
>
> Please remember to Cc maintainers on all patches, this was
> missing on patch 3.
Thank you for your reminding. I remember.
Thanks,
Mao
>> v7:
>> * patch7: -drop the !local_err assert is really not appropriate,
>> now revert it. [Marcel Apfelbaum]
>>
>> v6:
>> * patch3: -rename the subject.
>> * patch6: -simplify the commit message.
>> -use error_append_hint replace original error_setg rather
>> than remove it directly. [Marcel Apfelbaum]
>> -report the error message from local_err. [Marcel Apfelbaum]
>> v5:
>> * patch5: replace pci_add_capability2() with pci_add_capability(), because
>> it's confusing to have a function named pci_add_capability2() if
>> pci_add_capability() doesn't exist anymore. [Eduardo Habkost]
>> * patch8: a new patch that fix the return type of verify_irqchip_kernel().
>> * patch9: a new patch that use the errp directly instead of the local_err to
>> propagate the error messages.
>>
>> v4:
>> * patch4: changed from patch 5 in v3. use a elegant way to check
>> the error, like
>>
>> function(...);
>> if (function succeeded) {
>> /* non-error code path here */
>> foo = bar;
>> }
>>
>> or
>>
>> function(...);
>> if (function succeeded) {
>> /* non-error code path here */
>> foo = bar;
>> } else {
>> /* error path here */
>> return ret;
>> }
>>
>> for readability, instead of this:
>>
>> function(...)
>> if (function failed) {
>> return ...; /* or: "goto out" */
>> }
>>
>> /* non-error code path here */
>> foo = bar; [Eduardo Habkost]
>>
>> meanwhile, split previous patch4 out. [Michael S. Tsirkin]
>> * patch5: a new patch that replace pci_add_capability() with
>> pci_add_capability2(). [Eduardo Habkost]
>>
>> v3:
>> * patch2: explain the specified means of the return value, also
>> improve the commit message. [Marcel Apfelbaum]
>> * patch3: simplify the subject and commit message, fix another
>> wrong assert. [Marcel Apfelbaum]
>> * patch4: adjust the subject.
>> * patch5: fix a wrong optimization for errp. [Eduardo Habkost]
>> * patch7: a new patch that converts shpc_init() to Error in order
>> to propagate the error better.
>> v2:
>> * patch1: subject and commit message was rewrited by markus.
>> * patch2: comment was added to pci_add_capability2().
>> * patch3: a new patch that fix the wrong return value judgment condition.
>> * patch4: a new patch that fix code style problems.
>> * patch5: add an errp argument for pci_add_capability to pass
>> error for its callers.
>> * patch6: convert part of pci-bridge device to realize.
>>
>> v1:
>> * patch1: fix unreasonable return value check
>>
>> Cc: mst@redhat.com
>> Cc: marcel@redhat.com
>> Cc: armbru@redhat.com
>> Cc: dmitry@daynix.com
>> Cc: jasowang@redhat.com
>> Cc: kraxel@redhat.com
>> Cc: alex.williamson@redhat.com
>> Cc: pbonzini@redhat.com
>> Cc: rth@twiddle.net
>> Cc: ehabkost@redhat.com
>>
>> Mao Zhongyi (9):
>> pci: Clean up error checking in pci_add_capability()
>> pci: Add comment for pci_add_capability2()
>> pci: Fix the wrong assertion.
>> pci: Make errp the last parameter of pci_add_capability()
>> pci: Replace pci_add_capability2() with pci_add_capability()
>> pci: Convert to realize
>> pci: Convert shpc_init() to Error
>> i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel()
>> i386/kvm/pci-assign: Use errp directly rather than local_err
>>
>> hw/i386/amd_iommu.c | 24 ++++++++++++-----
>> hw/i386/kvm/pci-assign.c | 54 ++++++++++++++------------------------
>> hw/ide/ich.c | 2 +-
>> hw/net/e1000e.c | 30 ++++++++++++---------
>> hw/net/eepro100.c | 18 ++++++++++---
>> hw/pci-bridge/i82801b11.c | 12 ++++-----
>> hw/pci-bridge/pci_bridge_dev.c | 14 +++++-----
>> hw/pci-bridge/pcie_root_port.c | 18 ++++++-------
>> hw/pci-bridge/xio3130_downstream.c | 20 +++++++-------
>> hw/pci-bridge/xio3130_upstream.c | 20 +++++++-------
>> hw/pci/msi.c | 2 +-
>> hw/pci/msix.c | 2 +-
>> hw/pci/pci.c | 24 +++--------------
>> hw/pci/pci_bridge.c | 8 ++++--
>> hw/pci/pcie.c | 28 +++++++++++++++-----
>> hw/pci/shpc.c | 10 ++++---
>> hw/pci/slotid_cap.c | 12 ++++++---
>> hw/usb/hcd-xhci.c | 2 +-
>> hw/vfio/pci.c | 15 ++++++-----
>> hw/virtio/virtio-pci.c | 12 ++++++---
>> include/hw/pci/pci.h | 2 --
>> include/hw/pci/pci_bridge.h | 3 ++-
>> include/hw/pci/pcie.h | 3 ++-
>> include/hw/pci/shpc.h | 3 ++-
>> include/hw/pci/slotid_cap.h | 3 ++-
>> 25 files changed, 183 insertions(+), 158 deletions(-)
>>
>> --
>> 2.9.4
>>
>>
>
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread