Re: [Qemu-ppc] [PATCH 2/5] machine: introduce get_fw_dev_path() callback

[Andreas Färber]

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 <address@hidden>
>>> ---
>>>  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