* [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex
@ 2013-11-25 7:27 Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 1/5] boot: extend get_boot_devices_list() to ignore suffixes Alexey Kardashevskiy
` (4 more replies)
0 siblings, 5 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-25 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
This adds bootindex support for sPAPR. SLOF does not support this yet.
Alexey Kardashevskiy (5):
boot: extend get_boot_devices_list() to ignore suffixes
machine: introduce get_fw_dev_path() callback
spapr-llan: add to boot device list
spapr-vio: fix firmware names
spapr: define get_fw_dev_path() callback
hw/core/qdev.c | 15 ++++++++++++++-
hw/net/spapr_llan.c | 3 +++
hw/nvram/fw_cfg.c | 2 +-
hw/ppc/spapr.c | 42 +++++++++++++++++++++++++++++++++++++++++-
hw/ppc/spapr_vio.c | 2 ++
include/hw/boards.h | 1 +
include/sysemu/sysemu.h | 2 +-
vl.c | 6 +++---
8 files changed, 66 insertions(+), 7 deletions(-)
--
1.8.4.rc4
^ permalink raw reply [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 1/5] boot: extend get_boot_devices_list() to ignore suffixes
2013-11-25 7:27 [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex Alexey Kardashevskiy
@ 2013-11-25 7:27 ` Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback Alexey Kardashevskiy
` (3 subsequent siblings)
4 siblings, 0 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-25 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
As suffixes do not make sense for sPAPR's device tree and
there is no way to filter them out on the BusState::get_fw_dev_path
level, let's add an ability for the external caller to specify
whether to apply suffixes or not.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/nvram/fw_cfg.c | 2 +-
include/sysemu/sysemu.h | 2 +-
vl.c | 6 +++---
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
index f5dc3ea..4d0c76f 100644
--- a/hw/nvram/fw_cfg.c
+++ b/hw/nvram/fw_cfg.c
@@ -504,7 +504,7 @@ static void fw_cfg_machine_ready(struct Notifier *n, void *data)
{
size_t len;
FWCfgState *s = container_of(n, FWCfgState, machine_ready);
- char *bootindex = get_boot_devices_list(&len);
+ char *bootindex = get_boot_devices_list(&len, false);
fw_cfg_add_file(s, "bootorder", (uint8_t*)bootindex, len);
}
diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h
index 495dae8..2b71a4a 100644
--- a/include/sysemu/sysemu.h
+++ b/include/sysemu/sysemu.h
@@ -185,7 +185,7 @@ void rtc_change_mon_event(struct tm *tm);
void add_boot_device_path(int32_t bootindex, DeviceState *dev,
const char *suffix);
-char *get_boot_devices_list(size_t *size);
+char *get_boot_devices_list(size_t *size, bool ignore_suffixes);
DeviceState *get_boot_device(uint32_t position);
diff --git a/vl.c b/vl.c
index 8d5d874..aefc11a 100644
--- a/vl.c
+++ b/vl.c
@@ -1211,7 +1211,7 @@ DeviceState *get_boot_device(uint32_t position)
* memory pointed by "size" is assigned total length of the array in bytes
*
*/
-char *get_boot_devices_list(size_t *size)
+char *get_boot_devices_list(size_t *size, bool ignore_suffixes)
{
FWBootEntry *i;
size_t total = 0;
@@ -1226,7 +1226,7 @@ char *get_boot_devices_list(size_t *size)
assert(devpath);
}
- if (i->suffix && devpath) {
+ if (i->suffix && !ignore_suffixes && devpath) {
size_t bootpathlen = strlen(devpath) + strlen(i->suffix) + 1;
bootpath = g_malloc(bootpathlen);
@@ -1234,7 +1234,7 @@ char *get_boot_devices_list(size_t *size)
g_free(devpath);
} else if (devpath) {
bootpath = devpath;
- } else {
+ } else if (!ignore_suffixes) {
assert(i->suffix);
bootpath = g_strdup(i->suffix);
}
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-11-25 7:27 [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 1/5] boot: extend get_boot_devices_list() to ignore suffixes Alexey Kardashevskiy
@ 2013-11-25 7:27 ` Alexey Kardashevskiy
2013-11-26 4:55 ` Alexey Kardashevskiy
2013-12-03 9:37 ` Paolo Bonzini
2013-11-25 7:27 ` [Qemu-devel] [PATCH 3/5] spapr-llan: add to boot device list Alexey Kardashevskiy
` (2 subsequent siblings)
4 siblings, 2 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-25 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
QEMU supports firmware names for all devices in the QEMU tree but
sometime the exact format differs from what sPAPR platform uses.
This introduces a callback to let a machine fix device tree path names.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/core/qdev.c | 15 ++++++++++++++-
include/hw/boards.h | 1 +
2 files changed, 15 insertions(+), 1 deletion(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index e374a93..7347483 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -26,6 +26,7 @@
this API directly. */
#include "hw/qdev.h"
+#include "hw/boards.h"
#include "sysemu/sysemu.h"
#include "qapi/error.h"
#include "qapi/qmp/qerror.h"
@@ -497,6 +498,15 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
return NULL;
}
+static char *machine_get_fw_dev_path(BusState *bus, DeviceState *dev)
+{
+ if (current_machine->get_fw_dev_path) {
+ return current_machine->get_fw_dev_path(bus, dev);
+ }
+
+ return NULL;
+}
+
static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
{
int l = 0;
@@ -504,7 +514,10 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
if (dev && dev->parent_bus) {
char *d;
l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
- d = bus_get_fw_dev_path(dev->parent_bus, dev);
+ d = machine_get_fw_dev_path(dev->parent_bus, dev);
+ if (!d) {
+ d = bus_get_fw_dev_path(dev->parent_bus, dev);
+ }
if (d) {
l += snprintf(p + l, size - l, "%s", d);
g_free(d);
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 5a7ae9f..50ff24a 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -43,6 +43,7 @@ typedef struct QEMUMachine {
GlobalProperty *compat_props;
struct QEMUMachine *next;
const char *hw_version;
+ char *(*get_fw_dev_path)(BusState *bus, DeviceState *dev);
} QEMUMachine;
int qemu_register_machine(QEMUMachine *m);
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 3/5] spapr-llan: add to boot device list
2013-11-25 7:27 [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 1/5] boot: extend get_boot_devices_list() to ignore suffixes Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback Alexey Kardashevskiy
@ 2013-11-25 7:27 ` Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 4/5] spapr-vio: fix firmware names Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 5/5] spapr: define get_fw_dev_path() callback Alexey Kardashevskiy
4 siblings, 0 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-25 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/net/spapr_llan.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/hw/net/spapr_llan.c b/hw/net/spapr_llan.c
index 1bd6f50..229de00 100644
--- a/hw/net/spapr_llan.c
+++ b/hw/net/spapr_llan.c
@@ -29,6 +29,7 @@
#include "hw/qdev.h"
#include "hw/ppc/spapr.h"
#include "hw/ppc/spapr_vio.h"
+#include "sysemu/sysemu.h"
#include <libfdt.h>
@@ -213,6 +214,8 @@ static int spapr_vlan_init(VIOsPAPRDevice *sdev)
object_get_typename(OBJECT(sdev)), sdev->qdev.id, dev);
qemu_format_nic_info_str(qemu_get_queue(dev->nic), dev->nicconf.macaddr.a);
+ add_boot_device_path(dev->nicconf.bootindex, DEVICE(dev), "");
+
return 0;
}
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 4/5] spapr-vio: fix firmware names
2013-11-25 7:27 [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex Alexey Kardashevskiy
` (2 preceding siblings ...)
2013-11-25 7:27 ` [Qemu-devel] [PATCH 3/5] spapr-llan: add to boot device list Alexey Kardashevskiy
@ 2013-11-25 7:27 ` Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 5/5] spapr: define get_fw_dev_path() callback Alexey Kardashevskiy
4 siblings, 0 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-25 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
This changes VIO bridge fw name from spapr-vio-bridge to vdevice and
vscsi/veth node names from QEMU object names to VIO specific device tree
names.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr_vio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index fee6195..42b8416 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -68,6 +68,7 @@ static void spapr_vio_bus_class_init(ObjectClass *klass, void *data)
BusClass *k = BUS_CLASS(klass);
k->get_dev_path = spapr_vio_get_dev_name;
+ k->get_fw_dev_path = spapr_vio_get_dev_name;
}
static const TypeInfo spapr_vio_bus_info = {
@@ -531,6 +532,7 @@ static void spapr_vio_bridge_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
SysBusDeviceClass *k = SYS_BUS_DEVICE_CLASS(klass);
+ dc->fw_name = "vdevice";
k->init = spapr_vio_bridge_init;
dc->no_user = 1;
}
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH 5/5] spapr: define get_fw_dev_path() callback
2013-11-25 7:27 [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex Alexey Kardashevskiy
` (3 preceding siblings ...)
2013-11-25 7:27 ` [Qemu-devel] [PATCH 4/5] spapr-vio: fix firmware names Alexey Kardashevskiy
@ 2013-11-25 7:27 ` Alexey Kardashevskiy
2013-11-26 4:05 ` [Qemu-devel] [PATCH v2] " Alexey Kardashevskiy
4 siblings, 1 reply; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-25 7:27 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
This introduces a get_fw_dev_path() callback.
This fixes SCSI disks device node names (which are wildcard nodes in
the device-tree).
This fixes PHB name from "pci" to "pci@XXXX" where XXXX is a BUID as
there is no bus on top of sPAPRPHBState where PHB firmware name could
be fixed using the BusClass::get_fw_dev_path mechanism.
This stores the boot list in the /chosen/qemu,boolist property of
the device tree. SLOF needs an update in order to support the boot list.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/ppc/spapr.c | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7e53a5f..88bb9ce 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -45,6 +45,7 @@
#include "hw/pci/msi.h"
#include "hw/pci/pci.h"
+#include "hw/scsi/scsi.h"
#include "exec/address-spaces.h"
#include "hw/usb.h"
@@ -590,6 +591,8 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
int ret;
void *fdt;
sPAPRPHBState *phb;
+ size_t cb = 0;
+ char *bootlist;
fdt = g_malloc(FDT_MAX_SIZE);
@@ -633,6 +636,15 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
}
+ bootlist = get_boot_devices_list(&cb, true);
+ if (cb && bootlist) {
+ int offset = fdt_path_offset(fdt, "/chosen");
+ if (offset < 0) {
+ exit(1);
+ }
+ ret = fdt_setprop_string(fdt, offset, "qemu,bootlist", bootlist);
+ }
+
_FDT((fdt_pack(fdt)));
if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
@@ -711,7 +723,6 @@ static void ppc_spapr_reset(void)
first_ppc_cpu->env.gpr[5] = 0;
first_cpu->halted = 0;
first_ppc_cpu->env.nip = spapr->entry_point;
-
}
static void spapr_cpu_reset(void *opaque)
@@ -1357,6 +1368,34 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
assert(spapr->fdt_skel != NULL);
}
+#define CAST(type, obj, name) \
+ ((type *)object_dynamic_cast(OBJECT(obj), (name)))
+
+static char *spapr_get_fw_dev_path(BusState *bus, DeviceState *dev)
+{
+ SCSIDevice *d = CAST(SCSIDevice, dev, TYPE_SCSI_DEVICE);
+ sPAPRPHBState *phb = CAST(sPAPRPHBState, dev, TYPE_SPAPR_PCI_HOST_BRIDGE);
+
+ if (d) {
+ /*
+ * Replace "channel@0/disk@0,0" with "disk@8000000000000000":
+ * We use SRP luns of the form 8000 | (bus << 8) | (id << 5) | lun
+ * in the top 16 bits of the 64-bit LUN
+ */
+ unsigned id = 0x8000 | (d->channel << 8) | (d->id << 5) | d->lun;
+
+ return g_strdup_printf("%s@%"PRIX64, qdev_fw_name(dev),
+ (uint64_t)id << 48);
+ }
+
+ if (phb) {
+ /* Replace "pci" with "pci@800000020000000" */
+ return g_strdup_printf("pci@%"PRIX64, phb->buid);
+ }
+
+ return NULL;
+}
+
static QEMUMachine spapr_machine = {
.name = "pseries",
.desc = "pSeries Logical Partition (PAPR compliant)",
@@ -1367,6 +1406,7 @@ static QEMUMachine spapr_machine = {
.max_cpus = MAX_CPUS,
.no_parallel = 1,
.default_boot_order = NULL,
+ .get_fw_dev_path = spapr_get_fw_dev_path
};
static void spapr_machine_init(void)
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* [Qemu-devel] [PATCH v2] spapr: define get_fw_dev_path() callback
2013-11-25 7:27 ` [Qemu-devel] [PATCH 5/5] spapr: define get_fw_dev_path() callback Alexey Kardashevskiy
@ 2013-11-26 4:05 ` Alexey Kardashevskiy
0 siblings, 0 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-26 4:05 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Alexander Graf,
Nikunj A Dadhania
This introduces a get_fw_dev_path() callback.
This fixes SCSI disks device node names (which are wildcard nodes in
the device-tree).
This fixes PHB name from "pci" to "pci@XXXX" where XXXX is a BUID as
there is no bus on top of sPAPRPHBState where PHB firmware name could
be fixed using the BusClass::get_fw_dev_path mechanism.
This stores the boot list in the /chosen/qemu,boot-list property of
the device tree. SLOF needs an update in order to support the boot list.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
Changes:
v2:
* \n replaced with space as this is what SLOF expects there
* qemu,bootlist renamed to qemu,boot-list
---
hw/ppc/spapr.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 48 insertions(+), 1 deletion(-)
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7e53a5f..fe02f23 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -45,6 +45,7 @@
#include "hw/pci/msi.h"
#include "hw/pci/pci.h"
+#include "hw/scsi/scsi.h"
#include "exec/address-spaces.h"
#include "hw/usb.h"
@@ -587,9 +588,11 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
hwaddr rtas_addr,
hwaddr rtas_size)
{
- int ret;
+ int ret, i;
void *fdt;
sPAPRPHBState *phb;
+ size_t cb = 0;
+ char *bootlist;
fdt = g_malloc(FDT_MAX_SIZE);
@@ -633,6 +636,21 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
spapr_populate_chosen_stdout(fdt, spapr->vio_bus);
}
+ bootlist = get_boot_devices_list(&cb, true);
+ if (cb && bootlist) {
+ int offset = fdt_path_offset(fdt, "/chosen");
+ if (offset < 0) {
+ exit(1);
+ }
+ for (i = 0; i < cb; i++) {
+ if (bootlist[i] == '\n') {
+ bootlist[i] = ' ';
+ }
+
+ }
+ ret = fdt_setprop_string(fdt, offset, "qemu,boot-list", bootlist);
+ }
+
_FDT((fdt_pack(fdt)));
if (fdt_totalsize(fdt) > FDT_MAX_SIZE) {
@@ -1357,6 +1375,34 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
assert(spapr->fdt_skel != NULL);
}
+#define CAST(type, obj, name) \
+ ((type *)object_dynamic_cast(OBJECT(obj), (name)))
+
+static char *spapr_get_fw_dev_path(BusState *bus, DeviceState *dev)
+{
+ SCSIDevice *d = CAST(SCSIDevice, dev, TYPE_SCSI_DEVICE);
+ sPAPRPHBState *phb = CAST(sPAPRPHBState, dev, TYPE_SPAPR_PCI_HOST_BRIDGE);
+
+ if (d) {
+ /*
+ * Replace "channel@0/disk@0,0" with "disk@8000000000000000":
+ * We use SRP luns of the form 8000 | (bus << 8) | (id << 5) | lun
+ * in the top 16 bits of the 64-bit LUN
+ */
+ unsigned id = 0x8000 | (d->channel << 8) | (d->id << 5) | d->lun;
+
+ return g_strdup_printf("%s@%"PRIX64, qdev_fw_name(dev),
+ (uint64_t)id << 48);
+ }
+
+ if (phb) {
+ /* Replace "pci" with "pci@800000020000000" */
+ return g_strdup_printf("pci@%"PRIX64, phb->buid);
+ }
+
+ return NULL;
+}
+
static QEMUMachine spapr_machine = {
.name = "pseries",
.desc = "pSeries Logical Partition (PAPR compliant)",
@@ -1367,6 +1413,7 @@ static QEMUMachine spapr_machine = {
.max_cpus = MAX_CPUS,
.no_parallel = 1,
.default_boot_order = NULL,
+ .get_fw_dev_path = spapr_get_fw_dev_path
};
static void spapr_machine_init(void)
--
1.8.4.rc4
^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-11-25 7:27 ` [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback Alexey Kardashevskiy
@ 2013-11-26 4:55 ` Alexey Kardashevskiy
2013-12-03 3:52 ` Alexey Kardashevskiy
2013-12-03 9:37 ` Paolo Bonzini
1 sibling, 1 reply; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-11-26 4:55 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, Nikunj A Dadhania
Hi!
btw there is a problem with this patch - it does not compile for
"linux-user" as there is no current-machine global variable defined in vl.c
which is not compiled for linux-user.
How to solve this problem correctly?
On 11/25/2013 06:27 PM, Alexey Kardashevskiy wrote:
> QEMU supports firmware names for all devices in the QEMU tree but
> sometime the exact format differs from what sPAPR platform uses.
>
> This introduces a callback to let a machine fix device tree path names.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/core/qdev.c | 15 ++++++++++++++-
> include/hw/boards.h | 1 +
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index e374a93..7347483 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -26,6 +26,7 @@
> this API directly. */
>
> #include "hw/qdev.h"
> +#include "hw/boards.h"
> #include "sysemu/sysemu.h"
> #include "qapi/error.h"
> #include "qapi/qmp/qerror.h"
> @@ -497,6 +498,15 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
> return NULL;
> }
>
> +static char *machine_get_fw_dev_path(BusState *bus, DeviceState *dev)
> +{
> + if (current_machine->get_fw_dev_path) {
> + return current_machine->get_fw_dev_path(bus, dev);
> + }
> +
> + return NULL;
> +}
> +
> static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
> {
> int l = 0;
> @@ -504,7 +514,10 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
> if (dev && dev->parent_bus) {
> char *d;
> l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
> - d = bus_get_fw_dev_path(dev->parent_bus, dev);
> + d = machine_get_fw_dev_path(dev->parent_bus, dev);
> + if (!d) {
> + d = bus_get_fw_dev_path(dev->parent_bus, dev);
> + }
> if (d) {
> l += snprintf(p + l, size - l, "%s", d);
> g_free(d);
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 5a7ae9f..50ff24a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -43,6 +43,7 @@ typedef struct QEMUMachine {
> GlobalProperty *compat_props;
> struct QEMUMachine *next;
> const char *hw_version;
> + char *(*get_fw_dev_path)(BusState *bus, DeviceState *dev);
> } QEMUMachine;
>
> int qemu_register_machine(QEMUMachine *m);
>
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-11-26 4:55 ` Alexey Kardashevskiy
@ 2013-12-03 3:52 ` Alexey Kardashevskiy
2013-12-03 9:11 ` Markus Armbruster
2013-12-03 13:41 ` Andreas Färber
0 siblings, 2 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-03 3:52 UTC (permalink / raw)
To: qemu-devel
Cc: Alexey Kardashevskiy, Paolo Bonzini, qemu-ppc,
Andreas Färber
On 11/26/2013 03:55 PM, Alexey Kardashevskiy wrote:
> Hi!
>
> btw there is a problem with this patch - it does not compile for
> "linux-user" as there is no current-machine global variable defined in vl.c
> which is not compiled for linux-user.
>
> How to solve this problem correctly?
>
>
> On 11/25/2013 06:27 PM, Alexey Kardashevskiy wrote:
>> QEMU supports firmware names for all devices in the QEMU tree but
>> sometime the exact format differs from what sPAPR platform uses.
>>
>> This introduces a callback to let a machine fix device tree path names.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> hw/core/qdev.c | 15 ++++++++++++++-
>> include/hw/boards.h | 1 +
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index e374a93..7347483 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -26,6 +26,7 @@
>> this API directly. */
>>
>> #include "hw/qdev.h"
>> +#include "hw/boards.h"
>> #include "sysemu/sysemu.h"
>> #include "qapi/error.h"
>> #include "qapi/qmp/qerror.h"
>> @@ -497,6 +498,15 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
>> return NULL;
>> }
>>
>> +static char *machine_get_fw_dev_path(BusState *bus, DeviceState *dev)
>> +{
>> + if (current_machine->get_fw_dev_path) {
>> + return current_machine->get_fw_dev_path(bus, dev);
>> + }
>> +
>> + return NULL;
>> +}
>> +
Anyone, please?
The only easy fix for this I can think of would be this:
extern QEMUMachine *current_machine __attribute__((weak));
But I suspect this is disgusting? :)
>> static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
>> {
>> int l = 0;
>> @@ -504,7 +514,10 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
>> if (dev && dev->parent_bus) {
>> char *d;
>> l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
>> - d = bus_get_fw_dev_path(dev->parent_bus, dev);
>> + d = machine_get_fw_dev_path(dev->parent_bus, dev);
>> + if (!d) {
>> + d = bus_get_fw_dev_path(dev->parent_bus, dev);
>> + }
>> if (d) {
>> l += snprintf(p + l, size - l, "%s", d);
>> g_free(d);
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 5a7ae9f..50ff24a 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -43,6 +43,7 @@ typedef struct QEMUMachine {
>> GlobalProperty *compat_props;
>> struct QEMUMachine *next;
>> const char *hw_version;
>> + char *(*get_fw_dev_path)(BusState *bus, DeviceState *dev);
>> } QEMUMachine;
>>
>> int qemu_register_machine(QEMUMachine *m);
>>
>
>
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 3:52 ` Alexey Kardashevskiy
@ 2013-12-03 9:11 ` Markus Armbruster
2013-12-03 9:32 ` Alexey Kardashevskiy
2013-12-03 13:41 ` Andreas Färber
1 sibling, 1 reply; 23+ messages in thread
From: Markus Armbruster @ 2013-12-03 9:11 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Paolo Bonzini, qemu-ppc, qemu-devel, Andreas Färber
Alexey Kardashevskiy <aik@ozlabs.ru> writes:
> On 11/26/2013 03:55 PM, Alexey Kardashevskiy wrote:
>> Hi!
>>
>> btw there is a problem with this patch - it does not compile for
>> "linux-user" as there is no current-machine global variable defined in vl.c
>> which is not compiled for linux-user.
>>
>> How to solve this problem correctly?
[...]
> Anyone, please?
>
> The only easy fix for this I can think of would be this:
>
> extern QEMUMachine *current_machine __attribute__((weak));
>
>
> But I suspect this is disgusting? :)
Absolutely not. It's merely not portable to machines with object file
formats and linkers stuck in the 80s. However, we routinely twist
ourselves into knots for portability (observation, not endorsement), and
at least one previous attempt[*] to introduce weak symbols got nowhere.
[*] https://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg03853.html
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 9:11 ` Markus Armbruster
@ 2013-12-03 9:32 ` Alexey Kardashevskiy
2013-12-03 9:41 ` Paolo Bonzini
0 siblings, 1 reply; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-03 9:32 UTC (permalink / raw)
To: Markus Armbruster
Cc: Paolo Bonzini, qemu-ppc, qemu-devel, Andreas Färber
On 12/03/2013 08:11 PM, Markus Armbruster wrote:
> Alexey Kardashevskiy <aik@ozlabs.ru> writes:
>
>> On 11/26/2013 03:55 PM, Alexey Kardashevskiy wrote:
>>> Hi!
>>>
>>> btw there is a problem with this patch - it does not compile for
>>> "linux-user" as there is no current-machine global variable defined in vl.c
>>> which is not compiled for linux-user.
>>>
>>> How to solve this problem correctly?
> [...]
>> Anyone, please?
>>
>> The only easy fix for this I can think of would be this:
>>
>> extern QEMUMachine *current_machine __attribute__((weak));
>>
>>
>> But I suspect this is disgusting? :)
>
> Absolutely not. It's merely not portable to machines with object file
> formats and linkers stuck in the 80s. However, we routinely twist
> ourselves into knots for portability (observation, not endorsement), and
> at least one previous attempt[*] to introduce weak symbols got nowhere.
>
> [*] https://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg03853.html
Since that GCC_WEAK patch did not make it to upstream, there must be
another way of fixing my issue :)
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-11-25 7:27 ` [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback Alexey Kardashevskiy
2013-11-26 4:55 ` Alexey Kardashevskiy
@ 2013-12-03 9:37 ` Paolo Bonzini
2013-12-03 13:44 ` Andreas Färber
1 sibling, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2013-12-03 9:37 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Paul Mackerras, qemu-ppc, qemu-devel, Nikunj A Dadhania
Il 25/11/2013 08:27, Alexey Kardashevskiy ha scritto:
> QEMU supports firmware names for all devices in the QEMU tree but
> sometime the exact format differs from what sPAPR platform uses.
>
> This introduces a callback to let a machine fix device tree path names.
>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/core/qdev.c | 15 ++++++++++++++-
> include/hw/boards.h | 1 +
> 2 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index e374a93..7347483 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -26,6 +26,7 @@
> this API directly. */
>
> #include "hw/qdev.h"
> +#include "hw/boards.h"
> #include "sysemu/sysemu.h"
> #include "qapi/error.h"
> #include "qapi/qmp/qerror.h"
> @@ -497,6 +498,15 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
> return NULL;
> }
>
> +static char *machine_get_fw_dev_path(BusState *bus, DeviceState *dev)
> +{
> + if (current_machine->get_fw_dev_path) {
> + return current_machine->get_fw_dev_path(bus, dev);
> + }
> +
> + return NULL;
> +}
> +
> static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
> {
> int l = 0;
> @@ -504,7 +514,10 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
> if (dev && dev->parent_bus) {
> char *d;
> l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
> - d = bus_get_fw_dev_path(dev->parent_bus, dev);
> + d = machine_get_fw_dev_path(dev->parent_bus, dev);
> + if (!d) {
> + d = bus_get_fw_dev_path(dev->parent_bus, dev);
> + }
> if (d) {
> l += snprintf(p + l, size - l, "%s", d);
> g_free(d);
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 5a7ae9f..50ff24a 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -43,6 +43,7 @@ typedef struct QEMUMachine {
> GlobalProperty *compat_props;
> struct QEMUMachine *next;
> const char *hw_version;
> + char *(*get_fw_dev_path)(BusState *bus, DeviceState *dev);
> } QEMUMachine;
>
> int qemu_register_machine(QEMUMachine *m);
>
You can check "if (current_machine &&
current_machine->get_fw_dev_path)", and move current_machine from vl.c
to hw/qdev/core.c.
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 9:32 ` Alexey Kardashevskiy
@ 2013-12-03 9:41 ` Paolo Bonzini
0 siblings, 0 replies; 23+ messages in thread
From: Paolo Bonzini @ 2013-12-03 9:41 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: qemu-ppc, Markus Armbruster, Andreas Färber, qemu-devel
Il 03/12/2013 10:32, Alexey Kardashevskiy ha scritto:
>> > Absolutely not. It's merely not portable to machines with object file
>> > formats and linkers stuck in the 80s. However, we routinely twist
>> > ourselves into knots for portability (observation, not endorsement), and
>> > at least one previous attempt[*] to introduce weak symbols got nowhere.
>> >
>> > [*] https://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg03853.html
>
> Since that GCC_WEAK patch did not make it to upstream, there must be
> another way of fixing my issue :)
FWIW, the successor to that patch is the libqemustub.a static library,
which is so 1980s but has exactly the same functionality as weak symbols.
In your case, you do not even need a stub, I think.
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 3:52 ` Alexey Kardashevskiy
2013-12-03 9:11 ` Markus Armbruster
@ 2013-12-03 13:41 ` Andreas Färber
1 sibling, 0 replies; 23+ messages in thread
From: Andreas Färber @ 2013-12-03 13:41 UTC (permalink / raw)
To: Alexey Kardashevskiy, qemu-devel; +Cc: Paolo Bonzini, qemu-ppc
Am 03.12.2013 04:52, schrieb Alexey Kardashevskiy:
> On 11/26/2013 03:55 PM, Alexey Kardashevskiy wrote:
>> Hi!
>>
>> btw there is a problem with this patch - it does not compile for
>> "linux-user" as there is no current-machine global variable defined in vl.c
>> which is not compiled for linux-user.
>>
>> How to solve this problem correctly?
>>
>>
>> On 11/25/2013 06:27 PM, Alexey Kardashevskiy wrote:
>>> QEMU supports firmware names for all devices in the QEMU tree but
>>> sometime the exact format differs from what sPAPR platform uses.
>>>
>>> This introduces a callback to let a machine fix device tree path names.
>>>
>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>>> ---
>>> hw/core/qdev.c | 15 ++++++++++++++-
>>> include/hw/boards.h | 1 +
>>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>>> index e374a93..7347483 100644
>>> --- a/hw/core/qdev.c
>>> +++ b/hw/core/qdev.c
>>> @@ -26,6 +26,7 @@
>>> this API directly. */
>>>
>>> #include "hw/qdev.h"
>>> +#include "hw/boards.h"
>>> #include "sysemu/sysemu.h"
>>> #include "qapi/error.h"
>>> #include "qapi/qmp/qerror.h"
>>> @@ -497,6 +498,15 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
>>> return NULL;
>>> }
>>>
>>> +static char *machine_get_fw_dev_path(BusState *bus, DeviceState *dev)
>>> +{
>>> + if (current_machine->get_fw_dev_path) {
>>> + return current_machine->get_fw_dev_path(bus, dev);
>>> + }
>>> +
>>> + return NULL;
>>> +}
>>> +
>
>
> Anyone, please?
>
> The only easy fix for this I can think of would be this:
>
> extern QEMUMachine *current_machine __attribute__((weak));
>
>
> But I suspect this is disgusting? :)
Kind of. ;) It's introducing one concept in ppc code that's different
from the rest of the code. And it has nothing to do with core device
code, so please not in qdev.c. Better place this function close to where
the current_machine variable actually lives (vl.c?) and add a stub
function to stubs/ if needed.
I don't see how a firmware device path might be relevant for linux-user
though - so since the CPU will be the only device in linux-user, you
might as well use an old-fashioned #ifdef around its usage (assuming
that's reasonably contained) and just place the function where it makes
the most sense for softmmus.
Regards,
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 9:37 ` Paolo Bonzini
@ 2013-12-03 13:44 ` Andreas Färber
2013-12-03 14:00 ` Paolo Bonzini
0 siblings, 1 reply; 23+ messages in thread
From: Andreas Färber @ 2013-12-03 13:44 UTC (permalink / raw)
To: Paolo Bonzini, Alexey Kardashevskiy
Cc: qemu-ppc, Paul Mackerras, qemu-devel, Nikunj A Dadhania
Am 03.12.2013 10:37, schrieb Paolo Bonzini:
> Il 25/11/2013 08:27, Alexey Kardashevskiy ha scritto:
>> QEMU supports firmware names for all devices in the QEMU tree but
>> sometime the exact format differs from what sPAPR platform uses.
>>
>> This introduces a callback to let a machine fix device tree path names.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
>> ---
>> hw/core/qdev.c | 15 ++++++++++++++-
>> include/hw/boards.h | 1 +
>> 2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
>> index e374a93..7347483 100644
>> --- a/hw/core/qdev.c
>> +++ b/hw/core/qdev.c
>> @@ -26,6 +26,7 @@
>> this API directly. */
>>
>> #include "hw/qdev.h"
>> +#include "hw/boards.h"
>> #include "sysemu/sysemu.h"
>> #include "qapi/error.h"
>> #include "qapi/qmp/qerror.h"
>> @@ -497,6 +498,15 @@ static char *bus_get_fw_dev_path(BusState *bus, DeviceState *dev)
>> return NULL;
>> }
>>
>> +static char *machine_get_fw_dev_path(BusState *bus, DeviceState *dev)
>> +{
>> + if (current_machine->get_fw_dev_path) {
>> + return current_machine->get_fw_dev_path(bus, dev);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
>> {
>> int l = 0;
>> @@ -504,7 +514,10 @@ static int qdev_get_fw_dev_path_helper(DeviceState *dev, char *p, int size)
>> if (dev && dev->parent_bus) {
>> char *d;
>> l = qdev_get_fw_dev_path_helper(dev->parent_bus->parent, p, size);
>> - d = bus_get_fw_dev_path(dev->parent_bus, dev);
>> + d = machine_get_fw_dev_path(dev->parent_bus, dev);
>> + if (!d) {
>> + d = bus_get_fw_dev_path(dev->parent_bus, dev);
>> + }
>> if (d) {
>> l += snprintf(p + l, size - l, "%s", d);
>> g_free(d);
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 5a7ae9f..50ff24a 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -43,6 +43,7 @@ typedef struct QEMUMachine {
>> GlobalProperty *compat_props;
>> struct QEMUMachine *next;
>> const char *hw_version;
>> + char *(*get_fw_dev_path)(BusState *bus, DeviceState *dev);
>> } QEMUMachine;
>>
>> int qemu_register_machine(QEMUMachine *m);
>>
>
> You can check "if (current_machine &&
> current_machine->get_fw_dev_path)", and move current_machine from vl.c
> to hw/qdev/core.c.
Please don't encourage moving random stuff into "core" device code.
If needed, we can easily add a machine.c, but that should remain
softmmu-only.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 13:44 ` Andreas Färber
@ 2013-12-03 14:00 ` Paolo Bonzini
2013-12-03 14:35 ` Andreas Färber
2013-12-10 7:34 ` Alexey Kardashevskiy
0 siblings, 2 replies; 23+ messages in thread
From: Paolo Bonzini @ 2013-12-03 14:00 UTC (permalink / raw)
To: Andreas Färber
Cc: Alexey Kardashevskiy, qemu-ppc, Paul Mackerras, qemu-devel,
Nikunj A Dadhania
Il 03/12/2013 14:44, Andreas Färber ha scritto:
>> >
>> > You can check "if (current_machine &&
>> > current_machine->get_fw_dev_path)", and move current_machine from vl.c
>> > to hw/qdev/core.c.
> Please don't encourage moving random stuff into "core" device code.
>
> If needed, we can easily add a machine.c, but that should remain
> softmmu-only.
Another solution would be to:
(1) add an interface which contains "get_fw_dev_path". When
qdev_get_fw_dev_path is called, walk the QOM tree until an object that
implements the interface is found. If none is found, call the bus
implementation as usual.
(2) in vl.c, add a way for current_machine to override the /machine
object. A 100%-QOMified machine indeed could put a SOC-like Device there.
(3) for spapr, define the machine object to something that implements
said interface.
It seemed a bit complicated for this particular problem, but I cannot
say it's overengineered.
More aspects of the configuration could be moved to the new interface
over time, for example compat properties.
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 14:00 ` Paolo Bonzini
@ 2013-12-03 14:35 ` Andreas Färber
2013-12-03 14:58 ` Paolo Bonzini
2013-12-10 7:34 ` Alexey Kardashevskiy
1 sibling, 1 reply; 23+ messages in thread
From: Andreas Färber @ 2013-12-03 14:35 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Crosthwaite, Nikunj A Dadhania, Alexey Kardashevskiy,
qemu-devel, qemu-ppc, Anthony Liguori, Paul Mackerras,
Hervé Poussineau
Am 03.12.2013 15:00, schrieb Paolo Bonzini:
> Il 03/12/2013 14:44, Andreas Färber ha scritto:
>>>>
>>>> You can check "if (current_machine &&
>>>> current_machine->get_fw_dev_path)", and move current_machine from vl.c
>>>> to hw/qdev/core.c.
>> Please don't encourage moving random stuff into "core" device code.
>>
>> If needed, we can easily add a machine.c, but that should remain
>> softmmu-only.
>
> Another solution would be to:
>
> (1) add an interface which contains "get_fw_dev_path". When
> qdev_get_fw_dev_path is called, walk the QOM tree until an object that
> implements the interface is found. If none is found, call the bus
> implementation as usual.
>
> (2) in vl.c, add a way for current_machine to override the /machine
> object. A 100%-QOMified machine indeed could put a SOC-like Device there.
>
> (3) for spapr, define the machine object to something that implements
> said interface.
>
> It seemed a bit complicated for this particular problem, but I cannot
> say it's overengineered.
>
> More aspects of the configuration could be moved to the new interface
> over time, for example compat properties.
I remember Hervé running into problems with interfaces and ppc -cpu ?
(or -cpu host?), which does an object_class_foreach(), in the middle of
which QOM interfaces tried registering new types, leading to a GLib
assertion. Anthony never replied to that patch despite repeated pings,
so the issue is likely still unresolved.
http://patchwork.ozlabs.org/patch/241072/ (proposed workaround)
http://patchwork.ozlabs.org/patch/241504/ (test case)
http://patchwork.ozlabs.org/patch/241073/ (actual interface attempt)
I've thus been avoiding interfaces.
They are also kind of odd wrt memory management since even when we
object_initialize() previously allocated memory, interfaces of that type
are not part of the object size and are still dynamically allocated
IIRC. That may well be fixable of course by allocating space for
interfaces after the instance_size and bumping its copy in
TypeImpl::instance_size accordingly or calculate it in my proposed
type_get_instance_size().
http://patchwork.ozlabs.org/patch/269591/
http://patchwork.ozlabs.org/patch/269595/
(Which is BTW where I long wanted to introduce a static error_oom
similar to the error_abort discussed in a different thread now.)
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 14:35 ` Andreas Färber
@ 2013-12-03 14:58 ` Paolo Bonzini
2013-12-11 5:20 ` Alexey Kardashevskiy
0 siblings, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2013-12-03 14:58 UTC (permalink / raw)
To: Andreas Färber
Cc: Peter Crosthwaite, Nikunj A Dadhania, Alexey Kardashevskiy,
qemu-devel, qemu-ppc, Anthony Liguori, Paul Mackerras,
Hervé Poussineau
Il 03/12/2013 15:35, Andreas Färber ha scritto:
> Am 03.12.2013 15:00, schrieb Paolo Bonzini:
>> Il 03/12/2013 14:44, Andreas Färber ha scritto:
>>>>>
>>>>> You can check "if (current_machine &&
>>>>> current_machine->get_fw_dev_path)", and move current_machine from vl.c
>>>>> to hw/qdev/core.c.
>>> Please don't encourage moving random stuff into "core" device code.
>>>
>>> If needed, we can easily add a machine.c, but that should remain
>>> softmmu-only.
>>
>> Another solution would be to:
>>
>> (1) add an interface which contains "get_fw_dev_path". When
>> qdev_get_fw_dev_path is called, walk the QOM tree until an object that
>> implements the interface is found. If none is found, call the bus
>> implementation as usual.
>>
>> (2) in vl.c, add a way for current_machine to override the /machine
>> object. A 100%-QOMified machine indeed could put a SOC-like Device there.
>>
>> (3) for spapr, define the machine object to something that implements
>> said interface.
>>
>> It seemed a bit complicated for this particular problem, but I cannot
>> say it's overengineered.
>>
>> More aspects of the configuration could be moved to the new interface
>> over time, for example compat properties.
>
> I remember Hervé running into problems with interfaces and ppc -cpu ?
> (or -cpu host?), which does an object_class_foreach(), in the middle of
> which QOM interfaces tried registering new types, leading to a GLib
> assertion. Anthony never replied to that patch despite repeated pings,
> so the issue is likely still unresolved.
>
> http://patchwork.ozlabs.org/patch/241072/ (proposed workaround)
> http://patchwork.ozlabs.org/patch/241504/ (test case)
> http://patchwork.ozlabs.org/patch/241073/ (actual interface attempt)
Well, let's fix it. Thanks for the test case.
> They are also kind of odd wrt memory management since even when we
> object_initialize() previously allocated memory, interfaces of that type
> are not part of the object size and are still dynamically allocated
> IIRC. That may well be fixable of course by allocating space for
> interfaces after the instance_size and bumping its copy in
> TypeImpl::instance_size accordingly or calculate it in my proposed
> type_get_instance_size().
> http://patchwork.ozlabs.org/patch/269591/
> http://patchwork.ozlabs.org/patch/269595/
No, that has changed since commit 33e95c6 (qom: Reimplement Interfaces,
2012-08-10).
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 14:00 ` Paolo Bonzini
2013-12-03 14:35 ` Andreas Färber
@ 2013-12-10 7:34 ` Alexey Kardashevskiy
1 sibling, 0 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-10 7:34 UTC (permalink / raw)
To: Paolo Bonzini, Andreas Färber
Cc: qemu-ppc, Paul Mackerras, qemu-devel, Nikunj A Dadhania
On 12/04/2013 01:00 AM, Paolo Bonzini wrote:
> Il 03/12/2013 14:44, Andreas Färber ha scritto:
>>>>
>>>> You can check "if (current_machine &&
>>>> current_machine->get_fw_dev_path)", and move current_machine from vl.c
>>>> to hw/qdev/core.c.
>> Please don't encourage moving random stuff into "core" device code.
>>
>> If needed, we can easily add a machine.c, but that should remain
>> softmmu-only.
>
> Another solution would be to:
>
> (1) add an interface which contains "get_fw_dev_path". When
> qdev_get_fw_dev_path is called, walk the QOM tree until an object that
> implements the interface is found. If none is found, call the bus
> implementation as usual.
>
> (2) in vl.c, add a way for current_machine to override the /machine
> object. A 100%-QOMified machine indeed could put a SOC-like Device there.
Is there any good example of a 100%-QOMified machine? I could not find any.
> (3) for spapr, define the machine object to something that implements
> said interface.
>
> It seemed a bit complicated for this particular problem, but I cannot
> say it's overengineered.
I posted another series, please have a look. I did not find a good example
to copy from so...
> More aspects of the configuration could be moved to the new interface
> over time, for example compat properties.
How would it help?
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-03 14:58 ` Paolo Bonzini
@ 2013-12-11 5:20 ` Alexey Kardashevskiy
2013-12-11 7:47 ` Paolo Bonzini
0 siblings, 1 reply; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-11 5:20 UTC (permalink / raw)
To: Paolo Bonzini, Andreas Färber
Cc: Peter Crosthwaite, Nikunj A Dadhania, qemu-devel, qemu-ppc,
Anthony Liguori, Paul Mackerras, Hervé Poussineau
On 12/04/2013 01:58 AM, Paolo Bonzini wrote:
> Il 03/12/2013 15:35, Andreas Färber ha scritto:
>> Am 03.12.2013 15:00, schrieb Paolo Bonzini:
>>> Il 03/12/2013 14:44, Andreas Färber ha scritto:
>>>>>>
>>>>>> You can check "if (current_machine &&
>>>>>> current_machine->get_fw_dev_path)", and move current_machine from vl.c
>>>>>> to hw/qdev/core.c.
>>>> Please don't encourage moving random stuff into "core" device code.
>>>>
>>>> If needed, we can easily add a machine.c, but that should remain
>>>> softmmu-only.
>>>
>>> Another solution would be to:
>>>
>>> (1) add an interface which contains "get_fw_dev_path". When
>>> qdev_get_fw_dev_path is called, walk the QOM tree until an object that
>>> implements the interface is found. If none is found, call the bus
>>> implementation as usual.
>>>
>>> (2) in vl.c, add a way for current_machine to override the /machine
>>> object. A 100%-QOMified machine indeed could put a SOC-like Device there.
>>>
>>> (3) for spapr, define the machine object to something that implements
>>> said interface.
>>>
>>> It seemed a bit complicated for this particular problem, but I cannot
>>> say it's overengineered.
>>>
>>> More aspects of the configuration could be moved to the new interface
>>> over time, for example compat properties.
>>
>> I remember Hervé running into problems with interfaces and ppc -cpu ?
>> (or -cpu host?), which does an object_class_foreach(), in the middle of
>> which QOM interfaces tried registering new types, leading to a GLib
>> assertion. Anthony never replied to that patch despite repeated pings,
>> so the issue is likely still unresolved.
>>
>> http://patchwork.ozlabs.org/patch/241072/ (proposed workaround)
>> http://patchwork.ozlabs.org/patch/241504/ (test case)
>> http://patchwork.ozlabs.org/patch/241073/ (actual interface attempt)
>
> Well, let's fix it. Thanks for the test case.
Any progress on this?
I am asking since the patchset about bootindex you gave me yesterday prints
"(process:38896): GLib-CRITICAL **: g_hash_table_foreach: assertion
`version == hash_table->version' failed" which I "fixed" by moving the
machine object creation chunk before kvm_init() in vl.c.
btw what do I do with that patchset now? I works for me (except the issue
above), do I have to repost it again? Thanks.
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-11 5:20 ` Alexey Kardashevskiy
@ 2013-12-11 7:47 ` Paolo Bonzini
2013-12-11 7:59 ` Alexey Kardashevskiy
0 siblings, 1 reply; 23+ messages in thread
From: Paolo Bonzini @ 2013-12-11 7:47 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Peter Crosthwaite, Nikunj A Dadhania, qemu-devel, qemu-ppc,
Anthony Liguori, Paul Mackerras, Andreas Färber,
Hervé Poussineau
Il 11/12/2013 06:20, Alexey Kardashevskiy ha scritto:
>
> Any progress on this?
>
> I am asking since the patchset about bootindex you gave me yesterday prints
> "(process:38896): GLib-CRITICAL **: g_hash_table_foreach: assertion
> `version == hash_table->version' failed" which I "fixed" by moving the
> machine object creation chunk before kvm_init() in vl.c.
>
> btw what do I do with that patchset now? I works for me (except the issue
> above), do I have to repost it again? Thanks.
Please do, but we need to sort out the get_fw_dev_path suffixes first.
I'll be on IRC in ~1 hour.
Paolo
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-11 7:47 ` Paolo Bonzini
@ 2013-12-11 7:59 ` Alexey Kardashevskiy
2013-12-11 8:38 ` Alexey Kardashevskiy
0 siblings, 1 reply; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-11 7:59 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Crosthwaite, Nikunj A Dadhania, qemu-devel, qemu-ppc,
Anthony Liguori, Paul Mackerras, Andreas Färber,
Hervé Poussineau
On 12/11/2013 06:47 PM, Paolo Bonzini wrote:
> Il 11/12/2013 06:20, Alexey Kardashevskiy ha scritto:
>>
>> Any progress on this?
>>
>> I am asking since the patchset about bootindex you gave me yesterday prints
>> "(process:38896): GLib-CRITICAL **: g_hash_table_foreach: assertion
>> `version == hash_table->version' failed" which I "fixed" by moving the
>> machine object creation chunk before kvm_init() in vl.c.
>>
>> btw what do I do with that patchset now? I works for me (except the issue
>> above), do I have to repost it again? Thanks.
>
> Please do, but we need to sort out the get_fw_dev_path suffixes first.
> I'll be on IRC in ~1 hour.
And this is not it, "make check" on x86 fails:
GTESTER tests/test-bitops
LINK tests/test-qdev-global-props
hw/core/qdev.o: In function `qdev_get_fw_dev_path_from_handler':
/home/alexey/p/qemu/hw/core/qdev.c:514: undefined reference to
`fw_path_provider_try_get_dev_path'
collect2: error: ld returned 1 exit status
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback
2013-12-11 7:59 ` Alexey Kardashevskiy
@ 2013-12-11 8:38 ` Alexey Kardashevskiy
0 siblings, 0 replies; 23+ messages in thread
From: Alexey Kardashevskiy @ 2013-12-11 8:38 UTC (permalink / raw)
To: Paolo Bonzini
Cc: Peter Crosthwaite, Nikunj A Dadhania, qemu-devel, qemu-ppc,
Anthony Liguori, Paul Mackerras, Andreas Färber,
Hervé Poussineau
On 12/11/2013 06:59 PM, Alexey Kardashevskiy wrote:
> On 12/11/2013 06:47 PM, Paolo Bonzini wrote:
>> Il 11/12/2013 06:20, Alexey Kardashevskiy ha scritto:
>>>
>>> Any progress on this?
>>>
>>> I am asking since the patchset about bootindex you gave me yesterday prints
>>> "(process:38896): GLib-CRITICAL **: g_hash_table_foreach: assertion
>>> `version == hash_table->version' failed" which I "fixed" by moving the
>>> machine object creation chunk before kvm_init() in vl.c.
>>>
>>> btw what do I do with that patchset now? I works for me (except the issue
>>> above), do I have to repost it again? Thanks.
>>
>> Please do, but we need to sort out the get_fw_dev_path suffixes first.
>> I'll be on IRC in ~1 hour.
>
>
> And this is not it, "make check" on x86 fails:
>
> GTESTER tests/test-bitops
> LINK tests/test-qdev-global-props
> hw/core/qdev.o: In function `qdev_get_fw_dev_path_from_handler':
> /home/alexey/p/qemu/hw/core/qdev.c:514: undefined reference to
> `fw_path_provider_try_get_dev_path'
> collect2: error: ld returned 1 exit status
And "make check" on ppc64 fails:
GTESTER check-qtest-ppc64
(process:34077): GLib-CRITICAL **: g_hash_table_foreach: assertion `version
== hash_table->version' failed
Unable to find PowerPC CPU definition
Broken pipe
GTester: last random seed: R02S285a6f9556504cf9918b792d3bbff9f3
(process:34081): GLib-CRITICAL **: g_hash_table_foreach: assertion `version
== hash_table->version' failed
Unable to find PowerPC CPU definition
Broken pipe
GTester: last random seed: R02S4b1c4b660fcbbbf3907b024c4dd96e69
Oh :)
--
Alexey
^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2013-12-11 8:39 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-25 7:27 [Qemu-devel] [RFC PATCH 0/5] spapr: support bootindex Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 1/5] boot: extend get_boot_devices_list() to ignore suffixes Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 2/5] machine: introduce get_fw_dev_path() callback Alexey Kardashevskiy
2013-11-26 4:55 ` Alexey Kardashevskiy
2013-12-03 3:52 ` Alexey Kardashevskiy
2013-12-03 9:11 ` Markus Armbruster
2013-12-03 9:32 ` Alexey Kardashevskiy
2013-12-03 9:41 ` Paolo Bonzini
2013-12-03 13:41 ` Andreas Färber
2013-12-03 9:37 ` Paolo Bonzini
2013-12-03 13:44 ` Andreas Färber
2013-12-03 14:00 ` Paolo Bonzini
2013-12-03 14:35 ` Andreas Färber
2013-12-03 14:58 ` Paolo Bonzini
2013-12-11 5:20 ` Alexey Kardashevskiy
2013-12-11 7:47 ` Paolo Bonzini
2013-12-11 7:59 ` Alexey Kardashevskiy
2013-12-11 8:38 ` Alexey Kardashevskiy
2013-12-10 7:34 ` Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 3/5] spapr-llan: add to boot device list Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 4/5] spapr-vio: fix firmware names Alexey Kardashevskiy
2013-11-25 7:27 ` [Qemu-devel] [PATCH 5/5] spapr: define get_fw_dev_path() callback Alexey Kardashevskiy
2013-11-26 4:05 ` [Qemu-devel] [PATCH v2] " Alexey Kardashevskiy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).