* [Qemu-devel] [PATCH v6 1/9] pci: Clean up error checking in pci_add_capability()
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 2/9] pci: Add comment for pci_add_capability2() Mao Zhongyi
` (7 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 2/9] pci: Add comment for pci_add_capability2()
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 1/9] pci: Clean up error checking in pci_add_capability() Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 3/9] pci: Fix the wrong assertion Mao Zhongyi
` (6 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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
Suggested-by: Markus Armbruster <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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 3/9] pci: Fix the wrong assertion.
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 1/9] pci: Clean up error checking in pci_add_capability() Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 2/9] pci: Add comment for pci_add_capability2() Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-21 7:40 ` Marcel Apfelbaum
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 4/9] pci: Make errp the last parameter of pci_add_capability() Mao Zhongyi
` (5 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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>
---
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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v6 3/9] pci: Fix the wrong assertion.
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 3/9] pci: Fix the wrong assertion Mao Zhongyi
@ 2017-06-21 7:40 ` Marcel Apfelbaum
0 siblings, 0 replies; 17+ messages in thread
From: Marcel Apfelbaum @ 2017-06-21 7:40 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: dmitry, jasowang, kraxel, alex.williamson, armbru
On 20/06/2017 14:57, Mao Zhongyi wrote:
> 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>
> ---
> 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;
> }
>
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v6 4/9] pci: Make errp the last parameter of pci_add_capability()
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
` (2 preceding siblings ...)
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 3/9] pci: Fix the wrong assertion Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 5/9] pci: Replace pci_add_capability2() with pci_add_capability() Mao Zhongyi
` (4 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 5/9] pci: Replace pci_add_capability2() with pci_add_capability()
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
` (3 preceding siblings ...)
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 4/9] pci: Make errp the last parameter of pci_add_capability() Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 6/9] pci: Convert to realize Mao Zhongyi
` (3 subsequent siblings)
8 siblings, 0 replies; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 UTC (permalink / raw)
To: qemu-devel
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.
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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v6 6/9] pci: Convert to realize
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
` (4 preceding siblings ...)
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 5/9] pci: Replace pci_add_capability2() with pci_add_capability() Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-21 7:43 ` Marcel Apfelbaum
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 7/9] pci: Convert shpc_init() to Error Mao Zhongyi
` (2 subsequent siblings)
8 siblings, 1 reply; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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>
---
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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v6 6/9] pci: Convert to realize
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 6/9] pci: Convert to realize Mao Zhongyi
@ 2017-06-21 7:43 ` Marcel Apfelbaum
0 siblings, 0 replies; 17+ messages in thread
From: Marcel Apfelbaum @ 2017-06-21 7:43 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: mst, armbru
On 20/06/2017 14:57, Mao Zhongyi wrote:
> 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>
> ---
> 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);
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v6 7/9] pci: Convert shpc_init() to Error
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
` (5 preceding siblings ...)
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 6/9] pci: Convert to realize Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-22 8:14 ` [Qemu-devel] [PATCH v7 " Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel() Mao Zhongyi
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err Mao Zhongyi
8 siblings, 1 reply; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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>
---
hw/pci-bridge/pci_bridge_dev.c | 21 ++++++++-------------
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, 22 insertions(+), 27 deletions(-)
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 5dbd933..30c4186 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -49,12 +49,11 @@ 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);
int err;
- Error *local_err = NULL;
pci_bridge_initfn(dev, TYPE_PCI_BUS);
@@ -62,7 +61,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 +70,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;
}
@@ -79,20 +78,18 @@ static int pci_bridge_dev_initfn(PCIDevice *dev)
if (bridge_dev->msi != ON_OFF_AUTO_OFF) {
/* it means SHPC exists, because MSI is needed by SHPC */
- err = msi_init(dev, 0, 1, true, true, &local_err);
+ err = msi_init(dev, 0, 1, true, true, errp);
/* Any error other than -ENOTSUP(board's MSI support is broken)
* is a programming error */
assert(!err || err == -ENOTSUP);
if (err && bridge_dev->msi == ON_OFF_AUTO_ON) {
/* Can't satisfy user's explicit msi=on request, fail */
- error_append_hint(&local_err, "You have to use msi=auto (default) "
+ error_append_hint(errp, "You have to use msi=auto (default) "
"or msi=off with this machine type.\n");
- error_report_err(local_err);
goto msi_error;
}
- assert(!local_err || bridge_dev->msi == ON_OFF_AUTO_AUTO);
+ assert(bridge_dev->msi == ON_OFF_AUTO_AUTO);
/* With msi=auto, we fall back to MSI off silently */
- error_free(local_err);
}
if (shpc_present(dev)) {
@@ -101,7 +98,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 +108,6 @@ slotid_error:
}
shpc_error:
pci_bridge_exitfn(dev);
-
- return err;
}
static void pci_bridge_dev_exitfn(PCIDevice *dev)
@@ -216,7 +211,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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v7 7/9] pci: Convert shpc_init() to Error
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 7/9] pci: Convert shpc_init() to Error Mao Zhongyi
@ 2017-06-22 8:14 ` Mao Zhongyi
2017-06-23 9:56 ` Marcel Apfelbaum
0 siblings, 1 reply; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-22 8:14 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>
---
v7:
* drop the !local_err assert is really not appropriate,
now revert it. [Marcel Apfelbaum]
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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v7 7/9] pci: Convert shpc_init() to Error
2017-06-22 8:14 ` [Qemu-devel] [PATCH v7 " Mao Zhongyi
@ 2017-06-23 9:56 ` Marcel Apfelbaum
2017-06-26 1:30 ` Mao Zhongyi
0 siblings, 1 reply; 17+ messages in thread
From: Marcel Apfelbaum @ 2017-06-23 9:56 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: mst, armbru
On 22/06/2017 11:14, Mao Zhongyi wrote:
> 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>
> ---
> v7:
> * drop the !local_err assert is really not appropriate,
> now revert it. [Marcel Apfelbaum]
>
> 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
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Michael might ask you to re-post the whole series.
Thanks,
Marcel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v7 7/9] pci: Convert shpc_init() to Error
2017-06-23 9:56 ` Marcel Apfelbaum
@ 2017-06-26 1:30 ` Mao Zhongyi
0 siblings, 0 replies; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-26 1:30 UTC (permalink / raw)
To: Marcel Apfelbaum, qemu-devel; +Cc: mst, armbru
On 06/23/2017 05:56 PM, Marcel Apfelbaum wrote:
> On 22/06/2017 11:14, Mao Zhongyi wrote:
>> 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>
>> ---
>> v7:
>> * drop the !local_err assert is really not appropriate,
>> now revert it. [Marcel Apfelbaum]
>>
>> 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
>>
>
>
> Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
>
> Michael might ask you to re-post the whole series.
OK, I will. :)
Thanks,
Mao
> Thanks,
> Marcel
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v6 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel()
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
` (6 preceding siblings ...)
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 7/9] pci: Convert shpc_init() to Error Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-21 8:02 ` Marcel Apfelbaum
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err Mao Zhongyi
8 siblings, 1 reply; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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>
---
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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v6 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel()
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel() Mao Zhongyi
@ 2017-06-21 8:02 ` Marcel Apfelbaum
0 siblings, 0 replies; 17+ messages in thread
From: Marcel Apfelbaum @ 2017-06-21 8:02 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: pbonzini, rth, ehabkost, mst, armbru
On 20/06/2017 14:57, Mao Zhongyi wrote:
> 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>
> ---
> 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;
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v6 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err
2017-06-20 11:57 [Qemu-devel] [PATCH v6 0/9] Convert to realize and cleanup Mao Zhongyi
` (7 preceding siblings ...)
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 8/9] i386/kvm/pci-assign: Fix return type of verify_irqchip_kernel() Mao Zhongyi
@ 2017-06-20 11:57 ` Mao Zhongyi
2017-06-21 8:04 ` Marcel Apfelbaum
8 siblings, 1 reply; 17+ messages in thread
From: Mao Zhongyi @ 2017-06-20 11:57 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>
---
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.3
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v6 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err
2017-06-20 11:57 ` [Qemu-devel] [PATCH v6 9/9] i386/kvm/pci-assign: Use errp directly rather than local_err Mao Zhongyi
@ 2017-06-21 8:04 ` Marcel Apfelbaum
0 siblings, 0 replies; 17+ messages in thread
From: Marcel Apfelbaum @ 2017-06-21 8:04 UTC (permalink / raw)
To: Mao Zhongyi, qemu-devel; +Cc: pbonzini, rth, ehabkost, mst, armbru
On 20/06/2017 14:57, Mao Zhongyi wrote:
> 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>
> ---
> 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;
> }
>
>
Reviewed-by: Marcel Apfelbaum <marcel@redhat.com>
Thanks,
Marcel
^ permalink raw reply [flat|nested] 17+ messages in thread