[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize()
|
From: |
Cao jin |
|
Subject: |
[Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize() |
|
Date: |
Wed, 23 Dec 2015 18:23:12 +0800 |
Signed-off-by: Cao jin <address@hidden>
---
Since the callchain is pretty deep & error path is very much too, so I made the
patch based on the principal: catch/report the most necessary error msg with
smallest modification.(So you can see I don`t change some functions to void,
despite they coule be)
hw/xen/xen-host-pci-device.c | 79 +++++++++++++++++++++++++++-----------------
hw/xen/xen-host-pci-device.h | 5 +--
hw/xen/xen_pt.c | 67 +++++++++++++++++++------------------
hw/xen/xen_pt.h | 5 +--
hw/xen/xen_pt_config_init.c | 47 +++++++++++++-------------
hw/xen/xen_pt_graphics.c | 6 ++--
6 files changed, 118 insertions(+), 91 deletions(-)
diff --git a/hw/xen/xen-host-pci-device.c b/hw/xen/xen-host-pci-device.c
index 7d8a023..1ab6d97 100644
--- a/hw/xen/xen-host-pci-device.c
+++ b/hw/xen/xen-host-pci-device.c
@@ -43,13 +43,14 @@ static int xen_host_pci_sysfs_path(const XenHostPCIDevice
*d,
/* The output is truncated, or some other error was encountered */
return -ENODEV;
}
+
return 0;
}
/* 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];
@@ -60,23 +61,24 @@ static int xen_host_pci_get_resource(XenHostPCIDevice *d)
rc = xen_host_pci_sysfs_path(d, "resource", path, sizeof (path));
if (rc) {
- return rc;
+ error_setg_errno(errp, errno, "snprintf err");
+ return;
}
+
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_errno(errp, errno, "open err");
+ return;
}
do {
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++) {
@@ -129,20 +131,20 @@ 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 to short");
}
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)
+ unsigned int *pvalue, int base, Error **errp)
{
char path[PATH_MAX];
char buf[XEN_HOST_PCI_GET_VALUE_BUFFER_SIZE];
@@ -152,30 +154,38 @@ static int xen_host_pci_get_value(XenHostPCIDevice *d,
const char *name,
rc = xen_host_pci_sysfs_path(d, name, path, sizeof (path));
if (rc) {
+ error_setg_errno(errp, errno, "snprintf err");
return rc;
}
+
fd = open(path, O_RDONLY);
if (fd == -1) {
- XEN_HOST_PCI_LOG("Error: Can't open %s: %s\n", path, strerror(errno));
+ error_setg_errno(errp, errno, "open err");
return -errno;
}
+
do {
rc = read(fd, &buf, sizeof (buf) - 1);
if (rc < 0 && errno != EINTR) {
+ error_setg_errno(errp, errno, "read err");
rc = -errno;
goto out;
}
} while (rc < 0);
+
buf[rc] = 0;
value = strtol(buf, &endptr, base);
if (endptr == buf || *endptr != '\n') {
+ error_setg(errp, "format invalid");
rc = -1;
} else if ((value == LONG_MIN || value == LONG_MAX) && errno == ERANGE) {
+ error_setg_errno(errp, errno, "strtol err");
rc = -errno;
} else {
rc = 0;
*pvalue = value;
}
+
out:
close(fd);
return rc;
@@ -183,16 +193,18 @@ out:
static inline int xen_host_pci_get_hex_value(XenHostPCIDevice *d,
const char *name,
- unsigned int *pvalue)
+ unsigned int *pvalue,
+ Error **errp)
{
- return xen_host_pci_get_value(d, name, pvalue, 16);
+ return 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)
+ unsigned int *pvalue,
+ Error **errp)
{
- return xen_host_pci_get_value(d, name, pvalue, 10);
+ return xen_host_pci_get_value(d, name, pvalue, 10, errp);
}
static bool xen_host_pci_dev_is_virtfn(XenHostPCIDevice *d)
@@ -206,20 +218,21 @@ 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];
int rc;
rc = xen_host_pci_sysfs_path(d, "config", path, sizeof (path));
if (rc) {
- return rc;
+ error_setg_errno(errp, errno, "snprintf err");
+ return;
}
+
d->config_fd = open(path, O_RDWR);
if (d->config_fd < 0) {
- return -errno;
+ error_setg_errno(errp, errno, "open err");
}
- return 0;
}
static int xen_host_pci_config_read(XenHostPCIDevice *d,
@@ -341,8 +354,9 @@ 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;
@@ -353,43 +367,48 @@ 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, errp);
+ if (*errp) {
goto error;
}
- rc = xen_host_pci_get_resource(d);
- if (rc) {
+
+ xen_host_pci_get_resource(d, errp);
+ if (*errp) {
goto error;
}
- rc = xen_host_pci_get_hex_value(d, "vendor", &v);
+
+ rc = xen_host_pci_get_hex_value(d, "vendor", &v, errp);
if (rc) {
goto error;
}
d->vendor_id = v;
- rc = xen_host_pci_get_hex_value(d, "device", &v);
+
+ rc = xen_host_pci_get_hex_value(d, "device", &v, errp);
if (rc) {
goto error;
}
d->device_id = v;
- rc = xen_host_pci_get_dec_value(d, "irq", &v);
+
+ rc = xen_host_pci_get_dec_value(d, "irq", &v, errp);
if (rc) {
goto error;
}
d->irq = v;
- rc = xen_host_pci_get_hex_value(d, "class", &v);
+
+ rc = xen_host_pci_get_hex_value(d, "class", &v, errp);
if (rc) {
goto error;
}
d->class_code = v;
+
d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
- return 0;
+ return;
error:
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..42b9138 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..5f4cbb7 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;
@@ -774,12 +774,11 @@ 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);
- return -1;
+ xen_host_pci_device_get(&s->real_device,
+ s->hostaddr.domain, s->hostaddr.bus,
+ s->hostaddr.slot, s->hostaddr.function, errp);
+ if (*errp) {
+ return;
}
s->is_virtfn = s->real_device.is_virtfn;
@@ -799,16 +798,17 @@ 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;
}
- if (xen_pt_setup_vga(s, &s->real_device) < 0) {
- XEN_PT_ERR(d, "Setup VGA BIOS of passthrough GFX failed!\n");
+ if (xen_pt_setup_vga(s, &s->real_device, errp) < 0) {
+ error_append_hint(errp, "Setup VGA BIOS of passthrough"
+ " GFX failed!");
xen_host_pci_device_put(&s->real_device);
- return -1;
+ return;
}
/* Register ISA bridge for passthrough GFX. */
@@ -819,29 +819,28 @@ 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, errp);
+ if (*errp) {
+ error_append_hint(errp, "PCI Config space initialisation failed.");
goto err_out;
}
/* 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;
@@ -862,8 +861,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 %i failed!",
+ e_intx);
/* Disable PCI intx assertion (turn on bit10 of devctl) */
cmd |= PCI_COMMAND_INTX_DISABLE;
@@ -871,8 +870,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 %i failed!", machine_irq);
}
}
s->machine_irq = 0;
@@ -885,14 +884,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;
}
}
@@ -905,12 +904,16 @@ out:
"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)
@@ -929,7 +932,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;
diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h
index c545280..b4b0c19 100644
--- a/hw/xen/xen_pt.h
+++ b/hw/xen/xen_pt.h
@@ -228,7 +228,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);
@@ -328,5 +328,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);
+int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
+ Error **errp);
#endif /* !XEN_PT_H */
diff --git a/hw/xen/xen_pt_config_init.c b/hw/xen/xen_pt_config_init.c
index 3d8686d..dad2032 100644
--- a/hw/xen/xen_pt_config_init.c
+++ b/hw/xen/xen_pt_config_init.c
@@ -1899,8 +1899,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;
@@ -1919,12 +1920,14 @@ 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;
+ error_setg(errp, "Invalid register value");
+ return;
}
/* Sync up the data to dev.config */
offset = reg_grp->base_offset + reg->offset;
@@ -1942,7 +1945,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
@@ -1967,10 +1971,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
@@ -1990,11 +1994,9 @@ 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;
@@ -2039,11 +2041,11 @@ 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(errp, "Failed to initialize %d/%ld, type=0x%x,"
+ " rc:%d\n", i, ARRAY_SIZE(xen_pt_emu_reg_grps),
xen_pt_emu_reg_grps[i].grp_type, rc);
xen_pt_config_delete(s);
- return rc;
+ return;
}
}
@@ -2054,21 +2056,20 @@ int xen_pt_config_init(XenPCIPassthroughState *s)
/* 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, errp);
+ if (*errp) {
+ error_append_hint(errp, "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_delete(s);
- return rc;
+ return;
}
}
}
}
}
-
- return 0;
}
/* delete all emulate register */
diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c
index df6069b..17e7a59 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)
+int xen_pt_setup_vga(XenPCIPassthroughState *s, XenHostPCIDevice *dev,
+ Error **errp)
{
unsigned char *bios = NULL;
struct rom_header *rom;
@@ -172,12 +173,13 @@ int xen_pt_setup_vga(XenPCIPassthroughState *s,
XenHostPCIDevice *dev)
struct pci_data *pd = NULL;
if (!is_igd_vga_passthrough(dev)) {
+ error_setg(errp, "Need to enable igd-passthrough");
return -1;
}
bios = get_vgabios(s, &bios_size, dev);
if (!bios) {
- XEN_PT_ERR(&s->dev, "VGA: Can't getting VBIOS!\n");
+ error_setg(errp, "VGA: Can't getting VBIOS!");
return -1;
}
--
2.1.0
- [Qemu-devel] [PATCH] Xen PCI passthrough: convert to realize(),
Cao jin <=