| Message ID | 1385364460-24332-3-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
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); >
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 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
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 :)
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
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
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
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
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
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
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
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?
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.
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
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
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 :)
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);
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(-)