Re: [Qemu-devel] [PATCH 2/2] fw_cfg: set the get_boot_devices_list() ignore_suffixes parameter from machine property

[Mark Cave-Ayland]

On 06/08/18 21:11, Eduardo Habkost wrote:

> On Sun, Aug 05, 2018 at 12:28:50PM +0100, Mark Cave-Ayland wrote:
> > For the older machines (such as Mac and SPARC) the DT nodes representing
> > bootdevices for disk nodes are irregular for mainly historical reasons, and
> > should be handled on an individual basis via a custom FWPathProvider.
> >
> > Since the majority of bootdevice nodes for these machines either do not have a
> > separate disk node or require different (custom) names then it is much easier
> > to allow the ignore_suffixes parameter to be set on a per-machine basis via
> > a machine property.
> >
> > The default value for this new fwcfg_bootdevice_ignore_suffixes machine
> > property is false to preserve compatibility for existing machines.
> >
> > Signed-off-by: Mark Cave-Ayland <address@hidden>
> > ---
> >   hw/core/machine.c   | 3 +++
> >   hw/nvram/fw_cfg.c   | 5 ++++-
> >   include/hw/boards.h | 1 +
> >   3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/core/machine.c b/hw/core/machine.c
> > index a9aeb22f03..fbadb35865 100644
> > --- a/hw/core/machine.c
> > +++ b/hw/core/machine.c
> > @@ -525,6 +525,9 @@ static void machine_class_init(ObjectClass *oc, void *data)
> >       mc->default_ram_size = 128 * MiB;
> >       mc->rom_file_has_mr = true;
> >   
> > +    /* Default to using fwcfg bootdevice suffixes */
> > +    mc->fwcfg_bootdevice_ignore_suffixes = false;
> > +
> >       /* numa node memory size aligned on 8MB by default.
> >        * On Linux, each node's border has to be 8MB aligned
> >        */
> > diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> > index b23e7f64a8..ec6b8113ab 100644
> > --- a/hw/nvram/fw_cfg.c
> > +++ b/hw/nvram/fw_cfg.c
> > @@ -861,7 +861,10 @@ static void fw_cfg_machine_reset(void *opaque)
> >       void *ptr;
> >       size_t len;
> >       FWCfgState *s = opaque;
> > -    char *bootindex = get_boot_devices_list(&len, false);
> > +    MachineClass *mc = MACHINE_GET_CLASS(qdev_get_machine());
> > +
> > +    char *bootindex = get_boot_devices_list(&len,
> > +                          mc->fwcfg_bootdevice_ignore_suffixes);
> >   
> >       ptr = fw_cfg_modify_file(s, "bootorder", (uint8_t *)bootindex, len);
> >       g_free(ptr);
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index d139a431a6..2cf76d82a6 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -204,6 +204,7 @@ struct MachineClass {
> >       const char **valid_cpu_types;
> >       strList *allowed_dynamic_sysbus_devices;
> >       bool auto_enable_numa_with_memhp;
> > +    bool fwcfg_bootdevice_ignore_suffixes;
>
> We add MachineClass field when there's no obvious place for a
> device property (that we could set using compat_props).
>
> In this case you are controlling behavior of TYPE_FW_CFG, so I
> suggest adding a compat property to TYPE_FW_CFG, and setting it
> on MachineClass::compat_props.  This way we avoid adding a
> fw_cfg-specific field to MachineClass.

Ah I see, thanks for the pointer! Just out of curiosity, is there any 
documentation anywhere regarding compat_props?

I've managed to get something working using a fw_cfg property (patch for 
follow shortly) and the relevant section of the machine code looks like 
this:


#define HEATHROW_COMPAT \
    {\
        .driver = "fw_cfg",\
        .property = "bootdevice_ignore_suffixes",\
        .value = "on",\
    },

static void heathrow_class_init(ObjectClass *oc, void *data)
{
    MachineClass *mc = MACHINE_CLASS(oc);

    ....
    ....
    SET_MACHINE_COMPAT(mc, HEATHROW_COMPAT);
}


Is this sufficient, or are the compat properties supposed to be 
versioned according to the QEMU machine version?


ATB,

Mark.