Re: [PATCH v4 8/8] hw/i386: Introduce the microvm machine type

[Sergio Lopez]

Sergio Lopez <address@hidden> writes:

> Michael S. Tsirkin <address@hidden> writes:
>
>> On Tue, Sep 24, 2019 at 02:44:33PM +0200, Sergio Lopez wrote:
>>> +static void microvm_fix_kernel_cmdline(MachineState *machine)
>>> +{
>>> +    X86MachineState *x86ms = X86_MACHINE(machine);
>>> +    BusState *bus;
>>> +    BusChild *kid;
>>> +    char *cmdline;
>>> +
>>> +    /*
>>> +     * Find MMIO transports with attached devices, and add them to the 
>>> kernel
>>> +     * command line.
>>> +     *
>>> +     * Yes, this is a hack, but one that heavily improves the UX without
>>> +     * introducing any significant issues.
>>> +     */
>>> +    cmdline = g_strdup(machine->kernel_cmdline);
>>> +    bus = sysbus_get_default();
>>> +    QTAILQ_FOREACH(kid, &bus->children, sibling) {
>>> +        DeviceState *dev = kid->child;
>>> +        ObjectClass *class = object_get_class(OBJECT(dev));
>>> +
>>> +        if (class == object_class_by_name(TYPE_VIRTIO_MMIO)) {
>>> +            VirtIOMMIOProxy *mmio = VIRTIO_MMIO(OBJECT(dev));
>>> +            VirtioBusState *mmio_virtio_bus = &mmio->bus;
>>> +            BusState *mmio_bus = &mmio_virtio_bus->parent_obj;
>>> +
>>> +            if (!QTAILQ_EMPTY(&mmio_bus->children)) {
>>> +                gchar *mmio_cmdline = 
>>> microvm_get_mmio_cmdline(mmio_bus->name);
>>> +                if (mmio_cmdline) {
>>> +                    char *newcmd = g_strjoin(NULL, cmdline, mmio_cmdline, 
>>> NULL);
>>> +                    g_free(mmio_cmdline);
>>> +                    g_free(cmdline);
>>> +                    cmdline = newcmd;
>>> +                }
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    fw_cfg_modify_i32(x86ms->fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(cmdline) 
>>> + 1);
>>> +    fw_cfg_modify_string(x86ms->fw_cfg, FW_CFG_CMDLINE_DATA, cmdline);
>>> +}
>>
>> Can we rearrange this somewhat? Maybe the mmio constructor
>> would format the device description and add to some list,
>> and then microvm would just get stuff from that list
>> and add it to kernel command line?
>> This way it can also be controlled by a virtio-mmio property, so
>> e.g. you can disable it per device if you like.
>> In particular, this seems like a handy trick for any machine type
>> using mmio.
>
> Disabling it per-device won't be easy, as transport options can't be
> specified using the underlying device properties.
>
> But, otherwise, sounds like a good idea to avoid having to traverse the
> qtree. I'll give it a try.

Hi Michael,

I'm working on this, but can't find an easy way to obtain the actual IRQ
number with the data I have access on virtio_mmio_realizefn(). I there a
way to do that without building a new access interface? If it isn't,
knowing this is an specific hack for microvm, is it really worth
building it or can we just keep it as is in v4?

Thanks,
Sergio.

signature.asc
Description: PGP signature