* [Qemu-devel] [PULL 01/11] MAINTAINERS: update Xen files
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 02/11] xenfb.c: avoid expensive loops when prod <= out_cons Stefano Stabellini
` (10 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell
Cc: Markus Armbruster, xen-devel, qemu-devel, Stefano Stabellini
Add the PV block backend, the Xen mapcache, and hw/i386/xen to the list
of Xen related files maintained by me.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
---
MAINTAINERS | 3 +++
1 file changed, 3 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index d8b0f36..e3d4513 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -273,9 +273,12 @@ F: */xen*
F: hw/char/xen_console.c
F: hw/display/xenfb.c
F: hw/net/xen_nic.c
+F: hw/block/xen_*
F: hw/xen/
F: hw/xenpv/
+F: hw/i386/xen/
F: include/hw/xen/
+F: include/sysemu/xen-mapcache.h
Hosts:
------
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 02/11] xenfb.c: avoid expensive loops when prod <= out_cons
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 01/11] MAINTAINERS: update Xen files Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 03/11] xen-hvm: Clean up xen_hvm_init() error handling Stefano Stabellini
` (9 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: xen-devel, qemu-devel, Stefano Stabellini
If the frontend sets out_cons to a value higher than out_prod, it will
cause xenfb_handle_events to loop about 2^32 times. Avoid that by using
better checks at the beginning of the function.
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reported-by: Ling Liu <liuling-it@360.cn>
---
hw/display/xenfb.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/display/xenfb.c b/hw/display/xenfb.c
index 4e2a27a..8eb3046 100644
--- a/hw/display/xenfb.c
+++ b/hw/display/xenfb.c
@@ -789,8 +789,9 @@ static void xenfb_handle_events(struct XenFB *xenfb)
prod = page->out_prod;
out_cons = page->out_cons;
- if (prod == out_cons)
- return;
+ if (prod - out_cons >= XENFB_OUT_RING_LEN) {
+ return;
+ }
xen_rmb(); /* ensure we see ring contents up to prod */
for (cons = out_cons; cons != prod; cons++) {
union xenfb_out_event *event = &XENFB_OUT_RING_REF(page, cons);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 03/11] xen-hvm: Clean up xen_hvm_init() error handling
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 01/11] MAINTAINERS: update Xen files Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 02/11] xenfb.c: avoid expensive loops when prod <= out_cons Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 04/11] xen-hvm: Clean up xen_ram_alloc() " Stefano Stabellini
` (8 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell
Cc: Markus Armbruster, xen-devel, qemu-devel, Stefano Stabellini
From: Markus Armbruster <armbru@redhat.com>
xen_hvm_init() returns -1 without cleaning up on some errors (harmless
long as the caller exit()s on error), dies with hw_error() on others.
hw_error() isn't approprate here. Clean up to exit() on all errors.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
hw/i386/pc_piix.c | 5 ++---
hw/i386/pc_q35.c | 5 ++---
include/hw/xen/xen.h | 2 +-
xen-hvm-stub.c | 3 +--
xen-hvm.c | 61 ++++++++++++++++++++++++++------------------------
5 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index df2b824..db0ae9c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -121,9 +121,8 @@ static void pc_init1(MachineState *machine,
pcms->below_4g_mem_size = machine->ram_size;
}
- if (xen_enabled() && xen_hvm_init(pcms, &ram_memory) != 0) {
- fprintf(stderr, "xen hardware virtual machine initialisation failed\n");
- exit(1);
+ if (xen_enabled()) {
+ xen_hvm_init(pcms, &ram_memory);
}
pc_cpus_init(pcms);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 412b3cd..abbcbf9 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -111,9 +111,8 @@ static void pc_q35_init(MachineState *machine)
pcms->below_4g_mem_size = machine->ram_size;
}
- if (xen_enabled() && xen_hvm_init(pcms, &ram_memory) != 0) {
- fprintf(stderr, "xen hardware virtual machine initialisation failed\n");
- exit(1);
+ if (xen_enabled()) {
+ xen_hvm_init(pcms, &ram_memory);
}
pc_cpus_init(pcms);
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index e90931a..d07bc99 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -39,7 +39,7 @@ qemu_irq *xen_interrupt_controller_init(void);
void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
#if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
-int xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory);
+void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory);
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
struct MemoryRegion *mr);
void xen_modified_memory(ram_addr_t start, ram_addr_t length);
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index 6a39425..7ff0602 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -47,9 +47,8 @@ void xen_modified_memory(ram_addr_t start, ram_addr_t length)
{
}
-int xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
+void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
{
- return 0;
}
void qmp_xen_set_global_dirty_log(bool enable, Error **errp)
diff --git a/xen-hvm.c b/xen-hvm.c
index 2a93390..cb7128c 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -17,6 +17,7 @@
#include "qmp-commands.h"
#include "sysemu/char.h"
+#include "qemu/error-report.h"
#include "qemu/range.h"
#include "sysemu/xen-mapcache.h"
#include "trace.h"
@@ -1189,16 +1190,8 @@ static void xen_wakeup_notifier(Notifier *notifier, void *data)
xc_set_hvm_param(xen_xc, xen_domid, HVM_PARAM_ACPI_S_STATE, 0);
}
-/* return 0 means OK, or -1 means critical issue -- will exit(1) */
-int xen_hvm_init(PCMachineState *pcms,
- MemoryRegion **ram_memory)
+void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory)
{
- /*
- * FIXME Returns -1 without cleaning up on some errors (harmless
- * as long as the caller exit()s on error), dies with hw_error()
- * on others. hw_error() isn't approprate here. Should probably
- * simply exit() on all errors.
- */
int i, rc;
xen_pfn_t ioreq_pfn;
xen_pfn_t bufioreq_pfn;
@@ -1210,19 +1203,19 @@ int xen_hvm_init(PCMachineState *pcms,
state->xce_handle = xen_xc_evtchn_open(NULL, 0);
if (state->xce_handle == XC_HANDLER_INITIAL_VALUE) {
perror("xen: event channel open");
- return -1;
+ goto err;
}
state->xenstore = xs_daemon_open();
if (state->xenstore == NULL) {
perror("xen: xenstore open");
- return -1;
+ goto err;
}
rc = xen_create_ioreq_server(xen_xc, xen_domid, &state->ioservid);
if (rc < 0) {
perror("xen: ioreq server create");
- return -1;
+ goto err;
}
state->exit.notify = xen_exit_notifier;
@@ -1238,8 +1231,9 @@ int xen_hvm_init(PCMachineState *pcms,
&ioreq_pfn, &bufioreq_pfn,
&bufioreq_evtchn);
if (rc < 0) {
- hw_error("failed to get ioreq server info: error %d handle=" XC_INTERFACE_FMT,
- errno, xen_xc);
+ error_report("failed to get ioreq server info: error %d handle=" XC_INTERFACE_FMT,
+ errno, xen_xc);
+ goto err;
}
DPRINTF("shared page at pfn %lx\n", ioreq_pfn);
@@ -1249,8 +1243,9 @@ int xen_hvm_init(PCMachineState *pcms,
state->shared_page = xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
PROT_READ|PROT_WRITE, ioreq_pfn);
if (state->shared_page == NULL) {
- hw_error("map shared IO page returned error %d handle=" XC_INTERFACE_FMT,
- errno, xen_xc);
+ error_report("map shared IO page returned error %d handle=" XC_INTERFACE_FMT,
+ errno, xen_xc);
+ goto err;
}
rc = xen_get_vmport_regs_pfn(xen_xc, xen_domid, &ioreq_pfn);
@@ -1260,11 +1255,14 @@ int xen_hvm_init(PCMachineState *pcms,
xc_map_foreign_range(xen_xc, xen_domid, XC_PAGE_SIZE,
PROT_READ|PROT_WRITE, ioreq_pfn);
if (state->shared_vmport_page == NULL) {
- hw_error("map shared vmport IO page returned error %d handle="
- XC_INTERFACE_FMT, errno, xen_xc);
+ error_report("map shared vmport IO page returned error %d handle="
+ XC_INTERFACE_FMT, errno, xen_xc);
+ goto err;
}
} else if (rc != -ENOSYS) {
- hw_error("get vmport regs pfn returned error %d, rc=%d", errno, rc);
+ error_report("get vmport regs pfn returned error %d, rc=%d",
+ errno, rc);
+ goto err;
}
state->buffered_io_page = xc_map_foreign_range(xen_xc, xen_domid,
@@ -1272,7 +1270,8 @@ int xen_hvm_init(PCMachineState *pcms,
PROT_READ|PROT_WRITE,
bufioreq_pfn);
if (state->buffered_io_page == NULL) {
- hw_error("map buffered IO page returned error %d", errno);
+ error_report("map buffered IO page returned error %d", errno);
+ goto err;
}
/* Note: cpus is empty at this point in init */
@@ -1280,8 +1279,9 @@ int xen_hvm_init(PCMachineState *pcms,
rc = xen_set_ioreq_server_state(xen_xc, xen_domid, state->ioservid, true);
if (rc < 0) {
- hw_error("failed to enable ioreq server info: error %d handle=" XC_INTERFACE_FMT,
- errno, xen_xc);
+ error_report("failed to enable ioreq server info: error %d handle=" XC_INTERFACE_FMT,
+ errno, xen_xc);
+ goto err;
}
state->ioreq_local_port = g_malloc0(max_cpus * sizeof (evtchn_port_t));
@@ -1291,8 +1291,8 @@ int xen_hvm_init(PCMachineState *pcms,
rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
xen_vcpu_eport(state->shared_page, i));
if (rc == -1) {
- fprintf(stderr, "shared evtchn %d bind error %d\n", i, errno);
- return -1;
+ error_report("shared evtchn %d bind error %d", i, errno);
+ goto err;
}
state->ioreq_local_port[i] = rc;
}
@@ -1300,8 +1300,8 @@ int xen_hvm_init(PCMachineState *pcms,
rc = xc_evtchn_bind_interdomain(state->xce_handle, xen_domid,
bufioreq_evtchn);
if (rc == -1) {
- fprintf(stderr, "buffered evtchn bind error %d\n", errno);
- return -1;
+ error_report("buffered evtchn bind error %d", errno);
+ goto err;
}
state->bufioreq_local_port = rc;
@@ -1324,15 +1324,18 @@ int xen_hvm_init(PCMachineState *pcms,
/* Initialize backend core & drivers */
if (xen_be_init() != 0) {
- fprintf(stderr, "%s: xen backend core setup failed\n", __FUNCTION__);
- return -1;
+ error_report("xen backend core setup failed");
+ goto err;
}
xen_be_register("console", &xen_console_ops);
xen_be_register("vkbd", &xen_kbdmouse_ops);
xen_be_register("qdisk", &xen_blkdev_ops);
xen_read_physmap(state);
+ return;
- return 0;
+err:
+ error_report("xen hardware virtual machine initialisation failed");
+ exit(1);
}
void destroy_hvm_domain(bool reboot)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 04/11] xen-hvm: Clean up xen_ram_alloc() error handling
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (2 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 03/11] xen-hvm: Clean up xen_hvm_init() error handling Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 05/11] xen-pvdevice: convert to realize() Stefano Stabellini
` (7 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell
Cc: Markus Armbruster, xen-devel, qemu-devel, Stefano Stabellini
From: Markus Armbruster <armbru@redhat.com>
xen_ram_alloc() dies with hw_error() on error, even though its caller
ram_block_add() handles errors just fine. Add an Error **errp
parameter and use it.
Leave case RUN_STATE_INMIGRATE alone, because that looks like some
kind of warning.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
exec.c | 8 +++++++-
include/hw/xen/xen.h | 2 +-
xen-hvm-stub.c | 3 ++-
xen-hvm.c | 7 ++++---
4 files changed, 14 insertions(+), 6 deletions(-)
diff --git a/exec.c b/exec.c
index 7f0ce42..c268c36 100644
--- a/exec.c
+++ b/exec.c
@@ -1474,6 +1474,7 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
RAMBlock *block;
RAMBlock *last_block = NULL;
ram_addr_t old_ram_size, new_ram_size;
+ Error *err = NULL;
old_ram_size = last_ram_offset() >> TARGET_PAGE_BITS;
@@ -1483,7 +1484,12 @@ static ram_addr_t ram_block_add(RAMBlock *new_block, Error **errp)
if (!new_block->host) {
if (xen_enabled()) {
xen_ram_alloc(new_block->offset, new_block->max_length,
- new_block->mr);
+ new_block->mr, &err);
+ if (err) {
+ error_propagate(errp, err);
+ qemu_mutex_unlock_ramlist();
+ return -1;
+ }
} else {
new_block->host = phys_mem_alloc(new_block->max_length,
&new_block->mr->align);
diff --git a/include/hw/xen/xen.h b/include/hw/xen/xen.h
index d07bc99..1b81b4b 100644
--- a/include/hw/xen/xen.h
+++ b/include/hw/xen/xen.h
@@ -41,7 +41,7 @@ void xenstore_store_pv_console_info(int i, struct CharDriverState *chr);
#if defined(NEED_CPU_H) && !defined(CONFIG_USER_ONLY)
void xen_hvm_init(PCMachineState *pcms, MemoryRegion **ram_memory);
void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size,
- struct MemoryRegion *mr);
+ struct MemoryRegion *mr, Error **errp);
void xen_modified_memory(ram_addr_t start, ram_addr_t length);
#endif
diff --git a/xen-hvm-stub.c b/xen-hvm-stub.c
index 7ff0602..b958367 100644
--- a/xen-hvm-stub.c
+++ b/xen-hvm-stub.c
@@ -30,7 +30,8 @@ void xen_hvm_inject_msi(uint64_t addr, uint32_t data)
{
}
-void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
+void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
+ Error **errp)
{
}
diff --git a/xen-hvm.c b/xen-hvm.c
index cb7128c..a9085a8 100644
--- a/xen-hvm.c
+++ b/xen-hvm.c
@@ -239,9 +239,9 @@ static void xen_ram_init(PCMachineState *pcms,
}
}
-void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
+void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr,
+ Error **errp)
{
- /* FIXME caller ram_block_add() wants error_setg() on failure */
unsigned long nr_pfn;
xen_pfn_t *pfn_list;
int i;
@@ -268,7 +268,8 @@ void xen_ram_alloc(ram_addr_t ram_addr, ram_addr_t size, MemoryRegion *mr)
}
if (xc_domain_populate_physmap_exact(xen_xc, xen_domid, nr_pfn, 0, 0, pfn_list)) {
- hw_error("xen: failed to populate ram at " RAM_ADDR_FMT, ram_addr);
+ error_setg(errp, "xen: failed to populate ram at " RAM_ADDR_FMT,
+ ram_addr);
}
g_free(pfn_list);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 05/11] xen-pvdevice: convert to realize()
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (3 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 04/11] xen-hvm: Clean up xen_ram_alloc() " Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 06/11] Change xen_host_pci_sysfs_path() to return void Stefano Stabellini
` (6 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
hw/i386/xen/xen_pvdevice.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/hw/i386/xen/xen_pvdevice.c b/hw/i386/xen/xen_pvdevice.c
index c218947..9abcf25 100644
--- a/hw/i386/xen/xen_pvdevice.c
+++ b/hw/i386/xen/xen_pvdevice.c
@@ -69,14 +69,16 @@ static const MemoryRegionOps xen_pv_mmio_ops = {
.endianness = DEVICE_LITTLE_ENDIAN,
};
-static int xen_pv_init(PCIDevice *pci_dev)
+static void xen_pv_realize(PCIDevice *pci_dev, Error **errp)
{
XenPVDevice *d = XEN_PV_DEVICE(pci_dev);
uint8_t *pci_conf;
/* device-id property must always be supplied */
- if (d->device_id == 0xffff)
- return -1;
+ if (d->device_id == 0xffff) {
+ error_setg(errp, "Device ID invalid, it must always be supplied");
+ return;
+ }
pci_conf = pci_dev->config;
@@ -97,8 +99,6 @@ static int xen_pv_init(PCIDevice *pci_dev)
pci_register_bar(pci_dev, 1, PCI_BASE_ADDRESS_MEM_PREFETCH,
&d->mmio);
-
- return 0;
}
static Property xen_pv_props[] = {
@@ -114,7 +114,7 @@ static void xen_pv_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = xen_pv_init;
+ k->realize = xen_pv_realize;
k->class_id = PCI_CLASS_SYSTEM_OTHER;
dc->desc = "Xen PV Device";
dc->props = xen_pv_props;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 06/11] Change xen_host_pci_sysfs_path() to return void
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (4 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 05/11] xen-pvdevice: convert to realize() Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 07/11] Xen: use qemu_strtoul instead of strtol Stefano Stabellini
` (5 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
And assert the snprintf() error, because user can do nothing in case of
snprintf() fail.
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
hw/xen/xen-host-pci-device.c | 35 +++++++++++------------------------
1 file changed, 11 insertions(+), 24 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 7d8a023..9c342e7 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -31,19 +31,14 @@
#define IORESOURCE_PREFETCH 0x00001000 /* No side effects */
#define IORESOURCE_MEM_64 0x00100000
-static int xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
- const char *name, char *buf, ssize_t size)
+static void xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
+ const char *name, char *buf, ssize_t size)
{
int rc;
rc = snprintf(buf, size, "/sys/bus/pci/devices/%04x:%02x:%02x.%d/%s",
d->domain, d->bus, d->dev, d->func, name);
-
- if (rc >= size || rc < 0) {
- /* The output is truncated, or some other error was encountered */
- return -ENODEV;
- }
- return 0;
+ assert(rc >= 0 && rc < size);
}
@@ -58,10 +53,8 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
char *endptr, *s;
uint8_t type;
- rc = xen_host_pci_sysfs_path(d, "resource", path, sizeof (path));
- if (rc) {
- return rc;
- }
+ xen_host_pci_sysfs_path(d, "resource", path, sizeof(path));
+
fd = open(path, O_RDONLY);
if (fd == -1) {
XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
@@ -150,10 +143,8 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
unsigned long value;
char *endptr;
- rc = xen_host_pci_sysfs_path(d, name, path, sizeof (path));
- if (rc) {
- return rc;
- }
+ xen_host_pci_sysfs_path(d, name, path, sizeof(path));
+
fd = open(path, O_RDONLY);
if (fd == -1) {
XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
@@ -200,21 +191,17 @@ static bool xen_host_pci_dev_is_virtfn(XenHostPCIDevice *d)
char path[PATH_MAX];
struct stat buf;
- if (xen_host_pci_sysfs_path(d, "physfn", path, sizeof (path))) {
- return false;
- }
+ xen_host_pci_sysfs_path(d, "physfn", path, sizeof(path));
+
return !stat(path, &buf);
}
static int xen_host_pci_config_open(XenHostPCIDevice *d)
{
char path[PATH_MAX];
- int rc;
- rc = xen_host_pci_sysfs_path(d, "config", path, sizeof (path));
- if (rc) {
- return rc;
- }
+ xen_host_pci_sysfs_path(d, "config", path, sizeof(path));
+
d->config_fd = open(path, O_RDWR);
if (d->config_fd < 0) {
return -errno;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 07/11] Xen: use qemu_strtoul instead of strtol
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (5 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 06/11] Change xen_host_pci_sysfs_path() to return void Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 08/11] Add Error **errp for xen_host_pci_device_get() Stefano Stabellini
` (4 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
No need to roll our own (with slightly incorrect handling of errno),
when we can use the common version.
Change signed parsing to unsigned, because what it read are values in
PCI config space, which are non-negative.
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
hw/xen/xen-host-pci-device.c | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 9c342e7..83da9c4 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -141,7 +141,7 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
char buf[XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE];
int fd, rc;
unsigned long value;
- char *endptr;
+ const char *endptr;
xen_host_pci_sysfs_path(d, name, path, sizeof(path));
@@ -158,13 +158,9 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
}
} while (rc < 0);
buf[rc] = 0;
- value = strtol(buf, &endptr, base);
- if (endptr == buf || *endptr != '\n') {
- rc = -1;
- } else if ((value == LONG_MIN || value == LONG_MAX) && errno == ERANGE) {
- rc = -errno;
- } else {
- rc = 0;
+ rc = qemu_strtoul(buf, &endptr, base, &value);
+ if (!rc) {
+ assert(value <= UINT_MAX);
*pvalue = value;
}
out:
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 08/11] Add Error **errp for xen_host_pci_device_get()
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (6 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 07/11] Xen: use qemu_strtoul instead of strtol Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 09/11] Add Error **errp for xen_pt_setup_vga() Stefano Stabellini
` (3 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
To catch the error message. Also modify the caller
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
---
hw/xen/xen-host-pci-device.c | 102 +++++++++++++++++++++++-------------------
hw/xen/xen-host-pci-device.h | 5 ++-
hw/xen/xen_pt.c | 13 +++---
3 files changed, 68 insertions(+), 52 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 83da9c4..3827ca7 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -44,7 +44,7 @@ static void xen_host_pci_sysfs_path(const XenHostPCIDevice *d,
/* This size should be enough to read the first 7 lines of a resource file */
#define XEN_HOST_PCI_RESOURCE_BUFFER_SIZE 400
-static int xen_host_pci_get_resource(XenHostPCIDevice *d)
+static void xen_host_pci_get_resource(XenHostPCIDevice *d, Error **errp)
{
int i, rc, fd;
char path[PATH_MAX];
@@ -57,19 +57,18 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
fd = open(path, O_RDONLY);
if (fd == -1) {
- XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
- return -errno;
+ error_setg_file_open(errp, errno, path);
+ return;
}
do {
- rc = read(fd, &buf, sizeof (buf) - 1);
+ rc = read(fd, &buf, sizeof(buf) - 1);
if (rc < 0 && errno != EINTR) {
- rc = -errno;
+ error_setg_errno(errp, errno, "read err");
goto out;
}
} while (rc < 0);
buf[rc] = 0;
- rc = 0;
s = buf;
for (i = 0; i < PCI_NUM_REGIONS; i++) {
@@ -122,20 +121,19 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
d->rom.bus_flags = flags & IORESOURCE_BITS;
}
}
+
if (i != PCI_NUM_REGIONS) {
- /* Invalid format or input to short */
- rc = -ENODEV;
+ error_setg(errp, "Invalid format or input too short: %s", buf);
}
out:
close(fd);
- return rc;
}
/* This size should be enough to read a long from a file */
#define XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE 22
-static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
- unsigned int *pvalue, int base)
+static void xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
+ unsigned int *pvalue, int base, Error **errp)
{
char path[PATH_MAX];
char buf[XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE];
@@ -147,39 +145,45 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d, const char *name,
fd = open(path, O_RDONLY);
if (fd == -1) {
- XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
- return -errno;
+ error_setg_file_open(errp, errno, path);
+ return;
}
+
do {
- rc = read(fd, &buf, sizeof (buf) - 1);
+ rc = read(fd, &buf, sizeof(buf) - 1);
if (rc < 0 && errno != EINTR) {
- rc = -errno;
+ error_setg_errno(errp, errno, "read err");
goto out;
}
} while (rc < 0);
+
buf[rc] = 0;
rc = qemu_strtoul(buf, &endptr, base, &value);
if (!rc) {
assert(value <= UINT_MAX);
*pvalue = value;
+ } else {
+ error_setg_errno(errp, -rc, "failed to parse value '%s'", buf);
}
+
out:
close(fd);
- return rc;
}
-static inline int xen_host_pci_get_hex_value(XenHostPCIDevice *d,
- const char *name,
- unsigned int *pvalue)
+static inline void xen_host_pci_get_hex_value(XenHostPCIDevice *d,
+ const char *name,
+ unsigned int *pvalue,
+ Error **errp)
{
- return xen_host_pci_get_value(d, name, pvalue, 16);
+ xen_host_pci_get_value(d, name, pvalue, 16, errp);
}
-static inline int xen_host_pci_get_dec_value(XenHostPCIDevice *d,
- const char *name,
- unsigned int *pvalue)
+static inline void xen_host_pci_get_dec_value(XenHostPCIDevice *d,
+ const char *name,
+ unsigned int *pvalue,
+ Error **errp)
{
- return xen_host_pci_get_value(d, name, pvalue, 10);
+ xen_host_pci_get_value(d, name, pvalue, 10, errp);
}
static bool xen_host_pci_dev_is_virtfn(XenHostPCIDevice *d)
@@ -192,17 +196,16 @@ static bool xen_host_pci_dev_is_virtfn(XenHostPCIDevice *d)
return !stat(path, &buf);
}
-static int xen_host_pci_config_open(XenHostPCIDevice *d)
+static void xen_host_pci_config_open(XenHostPCIDevice *d, Error **errp)
{
char path[PATH_MAX];
xen_host_pci_sysfs_path(d, "config", path, sizeof(path));
d->config_fd = open(path, O_RDWR);
- if (d->config_fd < 0) {
- return -errno;
+ if (d->config_fd == -1) {
+ error_setg_file_open(errp, errno, path);
}
- return 0;
}
static int xen_host_pci_config_read(XenHostPCIDevice *d,
@@ -324,11 +327,12 @@ int xen_host_pci_find_ext_cap_offset(XenHostPCIDevice *d, uint32_t cap)
return -1;
}
-int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
- uint8_t bus, uint8_t dev, uint8_t func)
+void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
+ uint8_t bus, uint8_t dev, uint8_t func,
+ Error **errp)
{
unsigned int v;
- int rc = 0;
+ Error *err = NULL;
d->config_fd = -1;
d->domain = domain;
@@ -336,43 +340,51 @@ int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
d->dev = dev;
d->func = func;
- rc = xen_host_pci_config_open(d);
- if (rc) {
+ xen_host_pci_config_open(d, &err);
+ if (err) {
goto error;
}
- rc = xen_host_pci_get_resource(d);
- if (rc) {
+
+ xen_host_pci_get_resource(d, &err);
+ if (err) {
goto error;
}
- rc = xen_host_pci_get_hex_value(d, "vendor", &v);
- if (rc) {
+
+ xen_host_pci_get_hex_value(d, "vendor", &v, &err);
+ if (err) {
goto error;
}
d->vendor_id = v;
- rc = xen_host_pci_get_hex_value(d, "device", &v);
- if (rc) {
+
+ xen_host_pci_get_hex_value(d, "device", &v, &err);
+ if (err) {
goto error;
}
d->device_id = v;
- rc = xen_host_pci_get_dec_value(d, "irq", &v);
- if (rc) {
+
+ xen_host_pci_get_dec_value(d, "irq", &v, &err);
+ if (err) {
goto error;
}
d->irq = v;
- rc = xen_host_pci_get_hex_value(d, "class", &v);
- if (rc) {
+
+ xen_host_pci_get_hex_value(d, "class", &v, &err);
+ if (err) {
goto error;
}
d->class_code = v;
+
d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
- return 0;
+ return;
+
error:
+ error_propagate(errp, err);
+
if (d->config_fd >= 0) {
close(d->config_fd);
d->config_fd = -1;
}
- return rc;
}
bool xen_host_pci_device_closed(XenHostPCIDevice *d)
diff --git a/hw/xen/xen-host-pci-device.h b/hw/xen/xen-host-pci-device.h
index 3d44e04..6acf36e 100644
--- a/hw/xen/xen-host-pci-device.h
+++ b/hw/xen/xen-host-pci-device.h
@@ -36,8 +36,9 @@ typedef struct XenHostPCIDevice {
int config_fd;
} XenHostPCIDevice;
-int xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
- uint8_t bus, uint8_t dev, uint8_t func);
+void xen_host_pci_device_get(XenHostPCIDevice *d, uint16_t domain,
+ uint8_t bus, uint8_t dev, uint8_t func,
+ Error **errp);
void xen_host_pci_device_put(XenHostPCIDevice *pci_dev);
bool xen_host_pci_device_closed(XenHostPCIDevice *d);
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index aa96288..53b5bca 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -767,6 +767,7 @@ static int xen_pt_initfn(PCIDevice *d)
uint8_t machine_irq = 0, scratch;
uint16_t cmd = 0;
int pirq = XEN_PT_UNASSIGNED_PIRQ;
+ Error *err = NULL;
/* register real device */
XEN_PT_LOG(d, "Assigning real physical device %02x:%02x.%d"
@@ -774,11 +775,13 @@ static int xen_pt_initfn(PCIDevice *d)
s->hostaddr.bus, s->hostaddr.slot, s->hostaddr.function,
s->dev.devfn);
- rc = xen_host_pci_device_get(&s->real_device,
- s->hostaddr.domain, s->hostaddr.bus,
- s->hostaddr.slot, s->hostaddr.function);
- if (rc) {
- XEN_PT_ERR(d, "Failed to \"open\" the real pci device. rc: %i\n", rc);
+ xen_host_pci_device_get(&s->real_device,
+ s->hostaddr.domain, s->hostaddr.bus,
+ s->hostaddr.slot, s->hostaddr.function,
+ &err);
+ if (err) {
+ error_append_hint(&err, "Failed to \"open\" the real pci device");
+ error_report_err(err);
return -1;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 09/11] Add Error **errp for xen_pt_setup_vga()
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (7 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 08/11] Add Error **errp for xen_host_pci_device_get() Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:01 ` [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init() Stefano Stabellini
` (2 subsequent siblings)
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
To catch the error message. Also modify the caller
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
hw/xen/xen_pt.c | 7 +++++--
hw/xen/xen_pt.h | 3 ++-
hw/xen/xen_pt_graphics.c | 11 ++++++-----
3 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 53b5bca..07bfcec 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -808,8 +808,11 @@ static int xen_pt_initfn(PCIDevice *d)
return -1;
}
- if (xen_pt_setup_vga(s, &s->real_device) < 0) {
- XEN_PT_ERR(d, "Setup VGA BIOS of passthrough GFX failed!\n");
+ xen_pt_setup_vga(s, &s->real_device, &err);
+ if (err) {
+ error_append_hint(&err, "Setup VGA BIOS of passthrough"
+ " GFX failed");
+ error_report_err(err);
xen_host_pci_device_put(&s->real_device);
return -1;
}
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 3749711..26f74f8 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -330,5 +330,6 @@ static inline bool is_igd_vga_passthrough(XenHostPCIDevice *dev)
}
int xen_pt_register_vga_regions(XenHostPCIDevice *dev);
int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev);
-int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev);
+void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
+ Error **errp);
#endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index df6069b..e7a7c7e 100644
--- a/hw/xen/xen_pt_graphics.c
+++ b/hw/xen/xen_pt_graphics.c
@@ -161,7 +161,8 @@ struct pci_data {
uint16_t reserved;
} __attribute__((packed));
-int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
+void xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
+ Error **errp)
{
unsigned char *bios = NULL;
struct rom_header *rom;
@@ -172,13 +173,14 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
struct pci_data *pd = NULL;
if (!is_igd_vga_passthrough(dev)) {
- return -1;
+ error_setg(errp, "Need to enable igd-passthrough");
+ return;
}
bios = get_vgabios(s, &bios_size, dev);
if (!bios) {
- XEN_PT_ERR(&s->dev, "VGA: Can't getting VBIOS!\n");
- return -1;
+ error_setg(errp, "VGA: Can't get VBIOS");
+ return;
}
/* Currently we fixed this address as a primary. */
@@ -203,7 +205,6 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev)
/* Currently we fixed this address as a primary for legacy BIOS. */
cpu_physical_memory_rw(0xc0000, bios, bios_size, 1);
- return 0;
}
uint32_t igd_read_opregion(XenPCIPassthroughState *s)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init()
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (8 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 09/11] Add Error **errp for xen_pt_setup_vga() Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-22 11:21 ` Paolo Bonzini
2016-01-21 17:01 ` [Qemu-devel] [PULL 11/11] Xen PCI passthru: convert to realize() Stefano Stabellini
2016-01-21 17:45 ` [Qemu-devel] [PULL 0/11] xen-20160121 Peter Maydell
11 siblings, 1 reply; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
To catch the error message. Also modify the caller
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
hw/xen/xen_pt.c | 8 ++++---
hw/xen/xen_pt.h | 2 +-
hw/xen/xen_pt_config_init.c | 51 +++++++++++++++++++++++--------------------
3 files changed, 33 insertions(+), 28 deletions(-)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 07bfcec..9eef3df 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -825,9 +825,11 @@ static int xen_pt_initfn(PCIDevice *d)
xen_pt_register_regions(s, &cmd);
/* reinitialize each config register to be emulated */
- rc = xen_pt_config_init(s);
- if (rc) {
- XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
+ xen_pt_config_init(s, &err);
+ if (err) {
+ error_append_hint(&err, "PCI Config space initialisation failed");
+ error_report_err(err);
+ rc = -1;
goto err_out;
}
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index 26f74f8..c2f8e1f 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -230,7 +230,7 @@ struct XenPCIPassthroughState {
bool listener_set;
};
-int xen_pt_config_init(XenPCIPassthroughState *s);
+void xen_pt_config_init(XenPCIPassthroughState *s, Error **errp);
void xen_pt_config_delete(XenPCIPassthroughState *s);
XenPTRegGroup *xen_pt_find_reg_grp(XenPCIPassthroughState *s, uint32_t address);
XenPTReg *xen_pt_find_reg(XenPTRegGroup *reg_grp, uint32_t address);
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 185a698..81c6721 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1887,8 +1887,9 @@ static uint8_t find_cap_offset(XenPCIPassthroughState *s, uint8_t cap)
return 0;
}
-static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
- XenPTRegGroup *reg_grp, XenPTRegInfo *reg)
+static void xen_pt_config_reg_init(XenPCIPassthroughState *s,
+ XenPTRegGroup *reg_grp, XenPTRegInfo *reg,
+ Error **errp)
{
XenPTReg *reg_entry;
uint32_t data = 0;
@@ -1907,12 +1908,13 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
reg_grp->base_offset + reg->offset, &data);
if (rc < 0) {
g_free(reg_entry);
- return rc;
+ error_setg(errp, "Init emulate register fail");
+ return;
}
if (data == XEN_PT_INVALID_REG) {
/* free unused BAR register entry */
g_free(reg_entry);
- return 0;
+ return;
}
/* Sync up the data to dev.config */
offset = reg_grp->base_offset + reg->offset;
@@ -1930,7 +1932,8 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
if (rc) {
/* Serious issues when we cannot read the host values! */
g_free(reg_entry);
- return rc;
+ error_setg(errp, "Cannot read host values");
+ return;
}
/* Set bits in emu_mask are the ones we emulate. The dev.config shall
* contain the emulated view of the guest - therefore we flip the mask
@@ -1955,10 +1958,10 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
val = data;
if (val & ~size_mask) {
- XEN_PT_ERR(&s->dev,"Offset 0x%04x:0x%04x expands past register size(%d)!\n",
- offset, val, reg->size);
+ error_setg(errp, "Offset 0x%04x:0x%04x expands past"
+ " register size (%d)", offset, val, reg->size);
g_free(reg_entry);
- return -ENXIO;
+ return;
}
/* This could be just pci_set_long as we don't modify the bits
* past reg->size, but in case this routine is run in parallel or the
@@ -1978,13 +1981,12 @@ static int xen_pt_config_reg_init(XenPCIPassthroughState *s,
}
/* list add register entry */
QLIST_INSERT_HEAD(®_grp->reg_tbl_list, reg_entry, entries);
-
- return 0;
}
-int xen_pt_config_init(XenPCIPassthroughState *s)
+void xen_pt_config_init(XenPCIPassthroughState *s, Error **errp)
{
int i, rc;
+ Error *err = NULL;
QLIST_INIT(&s->reg_grps);
@@ -2027,11 +2029,12 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
reg_grp_offset,
®_grp_entry->size);
if (rc < 0) {
- XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld, type=0x%x, rc:%d\n",
- i, ARRAY_SIZE(xen_pt_emu_reg_grps),
+ error_setg(&err, "Failed to initialize %d/%zu, type = 0x%x,"
+ " rc: %d", i, ARRAY_SIZE(xen_pt_emu_reg_grps),
xen_pt_emu_reg_grps[i].grp_type, rc);
+ error_propagate(errp, err);
xen_pt_config_delete(s);
- return rc;
+ return;
}
}
@@ -2039,24 +2042,24 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
if (xen_pt_emu_reg_grps[i].emu_regs) {
int j = 0;
XenPTRegInfo *regs = xen_pt_emu_reg_grps[i].emu_regs;
+
/* initialize capability register */
for (j = 0; regs->size != 0; j++, regs++) {
- /* initialize capability register */
- rc = xen_pt_config_reg_init(s, reg_grp_entry, regs);
- if (rc < 0) {
- XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld reg 0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
- j, ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
- regs->offset, xen_pt_emu_reg_grps[i].grp_type,
- i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
+ xen_pt_config_reg_init(s, reg_grp_entry, regs, &err);
+ if (err) {
+ error_append_hint(&err, "Failed to initialize %d/%zu"
+ " reg 0x%x in grp_type = 0x%x (%d/%zu)",
+ j, ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
+ regs->offset, xen_pt_emu_reg_grps[i].grp_type,
+ i, ARRAY_SIZE(xen_pt_emu_reg_grps));
+ error_propagate(errp, err);
xen_pt_config_delete(s);
- return rc;
+ return;
}
}
}
}
}
-
- return 0;
}
/* delete all emulate register */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init()
2016-01-21 17:01 ` [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init() Stefano Stabellini
@ 2016-01-22 11:21 ` Paolo Bonzini
2016-01-23 12:23 ` Cao jin
0 siblings, 1 reply; 16+ messages in thread
From: Paolo Bonzini @ 2016-01-22 11:21 UTC (permalink / raw)
To: Stefano Stabellini, peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel
On 21/01/2016 18:01, Stefano Stabellini wrote:
> - XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld reg 0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
> - j, ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
> - regs->offset, xen_pt_emu_reg_grps[i].grp_type,
> - i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
> + xen_pt_config_reg_init(s, reg_grp_entry, regs, &err);
> + if (err) {
> + error_append_hint(&err, "Failed to initialize %d/%zu"
> + " reg 0x%x in grp_type = 0x%x (%d/%zu)",
> + j, ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
Coverity noticed a preexisting problem here. emu_regs is a pointer,
thus ARRAY_SIZE doesn't return what you expect.
Paolo
> + regs->offset, xen_pt_emu_reg_grps[i].grp_type,
> + i, ARRAY_SIZE(xen_pt_emu_reg_grps));
> + error_propagate(errp, err);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init()
2016-01-22 11:21 ` Paolo Bonzini
@ 2016-01-23 12:23 ` Cao jin
2016-01-25 10:53 ` Stefano Stabellini
0 siblings, 1 reply; 16+ messages in thread
From: Cao jin @ 2016-01-23 12:23 UTC (permalink / raw)
To: Paolo Bonzini, Stefano Stabellini, peter.maydell; +Cc: xen-devel, qemu-devel
On 01/22/2016 07:21 PM, Paolo Bonzini wrote:
>
>
> On 21/01/2016 18:01, Stefano Stabellini wrote:
>> - XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld reg 0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
>> - j, ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
>> - regs->offset, xen_pt_emu_reg_grps[i].grp_type,
>> - i, ARRAY_SIZE(xen_pt_emu_reg_grps), rc);
>> + xen_pt_config_reg_init(s, reg_grp_entry, regs, &err);
>> + if (err) {
>> + error_append_hint(&err, "Failed to initialize %d/%zu"
>> + " reg 0x%x in grp_type = 0x%x (%d/%zu)",
>> + j, ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
>
> Coverity noticed a preexisting problem here. emu_regs is a pointer,
> thus ARRAY_SIZE doesn't return what you expect.
>
Hi stefano,
Seems ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs) is not important err
message to regular users, and I guess it still can help developer to
debug even without it. So, do you think it is ok to remove it? Or any
better idea?
> Paolo
>
--
Yours Sincerely,
Cao jin
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init()
2016-01-23 12:23 ` Cao jin
@ 2016-01-25 10:53 ` Stefano Stabellini
0 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-25 10:53 UTC (permalink / raw)
To: Cao jin
Cc: peter.maydell, xen-devel, Stefano Stabellini, qemu-devel,
Paolo Bonzini
On Sat, 23 Jan 2016, Cao jin wrote:
> On 01/22/2016 07:21 PM, Paolo Bonzini wrote:
> >
> >
> > On 21/01/2016 18:01, Stefano Stabellini wrote:
> > > - XEN_PT_LOG(&s->dev, "Failed to initialize %d/%ld
> > > reg 0x%x in grp_type=0x%x (%d/%ld), rc=%d\n",
> > > - j,
> > > ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
> > > - regs->offset,
> > > xen_pt_emu_reg_grps[i].grp_type,
> > > - i, ARRAY_SIZE(xen_pt_emu_reg_grps),
> > > rc);
> > > + xen_pt_config_reg_init(s, reg_grp_entry, regs, &err);
> > > + if (err) {
> > > + error_append_hint(&err, "Failed to initialize
> > > %d/%zu"
> > > + " reg 0x%x in grp_type = 0x%x (%d/%zu)",
> > > + j,
> > > ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs),
> >
> > Coverity noticed a preexisting problem here. emu_regs is a pointer,
> > thus ARRAY_SIZE doesn't return what you expect.
> >
> Hi stefano,
>
> Seems ARRAY_SIZE(xen_pt_emu_reg_grps[i].emu_regs) is not important err message
> to regular users, and I guess it still can help developer to debug even
> without it. So, do you think it is ok to remove it? Or any better idea?
Yes, it is OK to remove it.
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PULL 11/11] Xen PCI passthru: convert to realize()
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (9 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 10/11] Add Error **errp for xen_pt_config_init() Stefano Stabellini
@ 2016-01-21 17:01 ` Stefano Stabellini
2016-01-21 17:45 ` [Qemu-devel] [PULL 0/11] xen-20160121 Peter Maydell
11 siblings, 0 replies; 16+ messages in thread
From: Stefano Stabellini @ 2016-01-21 17:01 UTC (permalink / raw)
To: peter.maydell; +Cc: Cao jin, xen-devel, qemu-devel, Stefano Stabellini
From: Cao jin <caoj.fnst@cn.fujitsu.com>
Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
hw/xen/xen_pt.c | 53 ++++++++++++++++++++++++++++-------------------------
1 file changed, 28 insertions(+), 25 deletions(-)
diff --git a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c
index 9eef3df..d33221b 100644
--- a/hw/xen/xen_pt.c
+++ b/hw/xen/xen_pt.c
@@ -760,10 +760,10 @@ static void xen_pt_destroy(PCIDevice *d) {
}
/* init */
-static int xen_pt_initfn(PCIDevice *d)
+static void xen_pt_realize(PCIDevice *d, Error **errp)
{
XenPCIPassthroughState *s = XEN_PT_DEVICE(d);
- int rc = 0;
+ int i, rc = 0;
uint8_t machine_irq = 0, scratch;
uint16_t cmd = 0;
int pirq = XEN_PT_UNASSIGNED_PIRQ;
@@ -781,8 +781,8 @@ static int xen_pt_initfn(PCIDevice *d)
&err);
if (err) {
error_append_hint(&err, "Failed to \"open\" the real pci device");
- error_report_err(err);
- return -1;
+ error_propagate(errp, err);
+ return;
}
s->is_virtfn = s->real_device.is_virtfn;
@@ -802,19 +802,19 @@ static int xen_pt_initfn(PCIDevice *d)
if ((s->real_device.domain == 0) && (s->real_device.bus == 0) &&
(s->real_device.dev == 2) && (s->real_device.func == 0)) {
if (!is_igd_vga_passthrough(&s->real_device)) {
- XEN_PT_ERR(d, "Need to enable igd-passthru if you're trying"
- " to passthrough IGD GFX.\n");
+ error_setg(errp, "Need to enable igd-passthru if you're trying"
+ " to passthrough IGD GFX");
xen_host_pci_device_put(&s->real_device);
- return -1;
+ return;
}
xen_pt_setup_vga(s, &s->real_device, &err);
if (err) {
error_append_hint(&err, "Setup VGA BIOS of passthrough"
" GFX failed");
- error_report_err(err);
+ error_propagate(errp, err);
xen_host_pci_device_put(&s->real_device);
- return -1;
+ return;
}
/* Register ISA bridge for passthrough GFX. */
@@ -836,20 +836,19 @@ static int xen_pt_initfn(PCIDevice *d)
/* Bind interrupt */
rc = xen_host_pci_get_byte(&s->real_device, PCI_INTERRUPT_PIN, &scratch);
if (rc) {
- XEN_PT_ERR(d, "Failed to read PCI_INTERRUPT_PIN! (rc:%d)\n", rc);
+ error_setg_errno(errp, errno, "Failed to read PCI_INTERRUPT_PIN");
goto err_out;
}
if (!scratch) {
- XEN_PT_LOG(d, "no pin interrupt\n");
+ error_setg(errp, "no pin interrupt");
goto out;
}
machine_irq = s->real_device.irq;
rc = xc_physdev_map_pirq(xen_xc, xen_domid, machine_irq, &pirq);
-
if (rc < 0) {
- XEN_PT_ERR(d, "Mapping machine irq %u to pirq %i failed, (err: %d)\n",
- machine_irq, pirq, errno);
+ error_setg_errno(errp, errno, "Mapping machine irq %u to"
+ " pirq %i failed", machine_irq, pirq);
/* Disable PCI intx assertion (turn on bit10 of devctl) */
cmd |= PCI_COMMAND_INTX_DISABLE;
@@ -870,8 +869,8 @@ static int xen_pt_initfn(PCIDevice *d)
PCI_SLOT(d->devfn),
e_intx);
if (rc < 0) {
- XEN_PT_ERR(d, "Binding of interrupt %i failed! (err: %d)\n",
- e_intx, errno);
+ error_setg_errno(errp, errno, "Binding of interrupt %u failed",
+ e_intx);
/* Disable PCI intx assertion (turn on bit10 of devctl) */
cmd |= PCI_COMMAND_INTX_DISABLE;
@@ -879,8 +878,8 @@ static int xen_pt_initfn(PCIDevice *d)
if (xen_pt_mapped_machine_irq[machine_irq] == 0) {
if (xc_physdev_unmap_pirq(xen_xc, xen_domid, machine_irq)) {
- XEN_PT_ERR(d, "Unmapping of machine interrupt %i failed!"
- " (err: %d)\n", machine_irq, errno);
+ error_setg_errno(errp, errno, "Unmapping of machine"
+ " interrupt %u failed", machine_irq);
}
}
s->machine_irq = 0;
@@ -893,14 +892,14 @@ out:
rc = xen_host_pci_get_word(&s->real_device, PCI_COMMAND, &val);
if (rc) {
- XEN_PT_ERR(d, "Failed to read PCI_COMMAND! (rc: %d)\n", rc);
+ error_setg_errno(errp, errno, "Failed to read PCI_COMMAND");
goto err_out;
} else {
val |= cmd;
rc = xen_host_pci_set_word(&s->real_device, PCI_COMMAND, val);
if (rc) {
- XEN_PT_ERR(d, "Failed to write PCI_COMMAND val=0x%x!(rc: %d)\n",
- val, rc);
+ error_setg_errno(errp, errno, "Failed to write PCI_COMMAND"
+ " val = 0x%x", val);
goto err_out;
}
}
@@ -910,15 +909,19 @@ out:
memory_listener_register(&s->io_listener, &address_space_io);
s->listener_set = true;
XEN_PT_LOG(d,
- "Real physical device %02x:%02x.%d registered successfully!\n",
+ "Real physical device %02x:%02x.%d registered successfully\n",
s->hostaddr.bus, s->hostaddr.slot, s->hostaddr.function);
- return 0;
+ return;
err_out:
+ for (i = 0; i < PCI_ROM_SLOT; i++) {
+ object_unparent(OBJECT(&s->bar[i]));
+ }
+ object_unparent(OBJECT(&s->rom));
+
xen_pt_destroy(d);
assert(rc);
- return rc;
}
static void xen_pt_unregister_device(PCIDevice *d)
@@ -937,7 +940,7 @@ static void xen_pci_passthrough_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->init = xen_pt_initfn;
+ k->realize = xen_pt_realize;
k->exit = xen_pt_unregister_device;
k->config_read = xen_pt_pci_read_config;
k->config_write = xen_pt_pci_write_config;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [Qemu-devel] [PULL 0/11] xen-20160121
2016-01-21 17:01 [Qemu-devel] [PULL 0/11] xen-20160121 Stefano Stabellini
` (10 preceding siblings ...)
2016-01-21 17:01 ` [Qemu-devel] [PULL 11/11] Xen PCI passthru: convert to realize() Stefano Stabellini
@ 2016-01-21 17:45 ` Peter Maydell
11 siblings, 0 replies; 16+ messages in thread
From: Peter Maydell @ 2016-01-21 17:45 UTC (permalink / raw)
To: Stefano Stabellini; +Cc: xen-devel@lists.xensource.com Devel, QEMU Developers
On 21 January 2016 at 17:01, Stefano Stabellini
<stefano.stabellini@eu.citrix.com> wrote:
> The following changes since commit 17c8a2197888bac8ec0256b16919b721c76c5e62:
>
> Merge remote-tracking branch 'remotes/armbru/tags/pull-error-2016-01-13' into staging (2016-01-14 13:07:38 +0000)
>
> are available in the git repository at:
>
>
> git://xenbits.xen.org/people/sstabellini/qemu-dm.git tags/xen-20160121
>
> for you to fetch changes up to 5a11d0f7549e24a10e178a9dc8ff5e698031d9a6:
>
> Xen PCI passthru: convert to realize() (2016-01-21 16:45:54 +0000)
>
> ----------------------------------------------------------------
> Xen 2016/01/21
Applied, thanks.
-- PMM
^ permalink raw reply [flat|nested] 16+ messages in thread