Re: [Qemu-devel] [PATCH v3 11/12] pc: Support firmware configuration with -blockdev

[Laszlo Ersek]

On 03/12/19 07:52, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <address@hidden> writes:
> 
>> Le lun. 11 mars 2019 17:02, Markus Armbruster <address@hidden> a écrit :
>>
>>> Paolo Bonzini <address@hidden> writes:
> [...]
>>>> Does it work if you add the device yourself as a child of /machine,
>>>> instead of relying on /machine/unattached?
>>>
>>> I figure you're suggesting something like this incremental patch:
>>>
>>>    diff --git a/hw/i386/pc_sysfw.c b/hw/i386/pc_sysfw.c
>>>    index ccf2221acb..9d3a80c51a 100644
>>>    --- a/hw/i386/pc_sysfw.c
>>>    +++ b/hw/i386/pc_sysfw.c
>>>    @@ -99,6 +99,10 @@ void pc_system_flash_create(PCMachineState *pcms)
>>>         if (pcmc->pci_enabled) {
>>>             pcms->flash[0] = pc_pflash_create("system.flash0");
>>>             pcms->flash[1] = pc_pflash_create("system.flash1");
>>>    +        object_property_add_child(OBJECT(pcms), "pfl0",
>>>    +                                  OBJECT(pcms->flash[0]),
>>> &error_abort);
>>>    +        object_property_add_child(OBJECT(pcms), "pfl1",
>>>    +                                  OBJECT(pcms->flash[1]),
>>> &error_abort);
>>>             object_property_add_alias(OBJECT(pcms), "pflash0",
>>>                                       OBJECT(pcms->flash[0]), "drive",
>>>                                       &error_abort);
>>>    @@ -122,19 +126,7 @@ static void
>>> pc_system_flash_cleanup_unused(PCMachineState *pcms)
>>>                 prop_name = g_strdup_printf("pflash%d", i);
>>>                 object_property_del(OBJECT(pcms), prop_name, &error_abort);
>>>                 g_free(prop_name);
>>>    -            /*
>>>    -             * Delete @dev_obj.  Straight object_unref() is wrong,
>>>    -             * since it leaves dangling references in the parent bus
>>>    -             * behind.  object_unparent() doesn't work, either: since
>>>    -             * @dev_obj hasn't been realized, dev_obj->parent is null,
>>>    -             * and object_unparent() does nothing.  DeviceClass method
>>>    -             * device_unparent() works, but we have to take a
>>>    -             * temporary reference, or else device_unparent() commits
>>>    -             * a use-after-free error.
>>>    -             */
>>>    -            object_ref(dev_obj);
>>>    -            object_get_class(dev_obj)->unparent(dev_obj);
>>>    -            object_unref(dev_obj);
>>>    +            object_unparent(dev_obj);
>>>                 pcms->flash[i] = NULL;
>>>             }
>>>         }
>>>
>>> Passes "make check" and "info qtree" looks good both with and without
>>> -machine pflash0,...
>>>
>>> I'm not exactly happy with "pfl0" and "pfl1".  Got a favorite color for
>>> my bikeshed?
>>>
>>
>> ovmf_code & ovmf_vars?
> 
> Tempting, since we use these guys only for OVMF so far.  However, we
> could also use them for SeaBIOS[*], or whatever other firmware comes up.
> 
> [...]
> 
> 
> [*] See also
> Subject: Re: Configuring pflash devices for OVMF firmware
> Message-ID: <address@hidden>
> https://lists.nongnu.org/archive/html/qemu-devel/2019-01/msg08145.html
> 

I think I'd slightly prefer if we didn't use the term "OVMF" in this
context.

Thanks
Laszlo