From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60289) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGod7-0001bQ-O2 for qemu-devel@nongnu.org; Thu, 29 Jan 2015 07:55:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGod0-0001lO-5B for qemu-devel@nongnu.org; Thu, 29 Jan 2015 07:55:29 -0500 Message-ID: <54CA2DB7.2060901@suse.de> Date: Thu, 29 Jan 2015 13:55:19 +0100 From: Alexander Graf MIME-Version: 1.0 References: <1422053517-23781-1-git-send-email-dvaleev@suse.de> <1422053517-23781-2-git-send-email-dvaleev@suse.de> <54C2D399.3080307@suse.de> <54C61BA2.5000609@suse.de> <54C60A51.7030705@suse.de> <54C98171.6050101@huawei.com> <54C981E0.3080709@suse.de> <54C9AD0E.4050803@huawei.com> In-Reply-To: <54C9AD0E.4050803@huawei.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] hw/ppc/spapr Add qemu_register_boot_set for SPAPR List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gonglei Cc: Dinar Valeev , Dinar Valeev , "qemu-ppc@nongnu.org" , "qemu-devel@nongnu.org" On 29.01.15 04:46, Gonglei wrote: > On 2015/1/29 8:42, Alexander Graf wrote: > >> >> >> On 29.01.15 01:40, Gonglei wrote: >>> On 2015/1/26 17:35, Alexander Graf wrote: >>> >>>> On 01/26/2015 11:49 AM, Dinar Valeev wrote: >>>>> On 01/24/2015 12:04 AM, Alexander Graf wrote: >>>>>> >>>>>> >>>>>> On 23.01.15 23:51, dvaleev@suse.de wrote: >>>>>>> From: Dinar Valeev >>>>>>> >>>>>>> In order to have -boot once=d functioning, it is required to have >>>>>>> qemu_register_boot_set >>>>>>> >>>>>>> qemu-system-ppc64 -enable-kvm -boot once=d >>>>>>> >>>>>>> Ready! >>>>>>> 0 > dev /chosen ok >>>>>>> 0 > .properties >>>>>>> ... >>>>>>> qemu,boot-device d >>>>>>> ... >>>>>>> 0 > reset-all >>>>>>> >>>>>>> Ready! >>>>>>> 0 > dev /chosen ok >>>>>>> 0 > .properties >>>>>>> ... >>>>>>> qemu,boot-device cdn >>>>>>> ... >>>>>>> >>>>>>> Signed-off-by: Dinar Valeev >>>>>>> --- >>>>>>> hw/ppc/spapr.c | 12 ++++++++++++ >>>>>>> 1 file changed, 12 insertions(+) >>>>>>> >>>>>>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >>>>>>> index 3d2cfa3..38b03fc 100644 >>>>>>> --- a/hw/ppc/spapr.c >>>>>>> +++ b/hw/ppc/spapr.c >>>>>>> @@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1) >>>>>>> g_string_append_len(s, s1, strlen(s1) + 1); >>>>>>> } >>>>>>> >>>>>>> +static void spapr_boot_set(void *opaque, const char *boot_device, >>>>>>> + Error **errp) >>>>>>> +{ >>>>>>> + int offset; >>>>>>> + offset = fdt_path_offset(opaque, "/chosen"); >>>>>>> + fdt_setprop_string(opaque, offset, "qemu,boot-device", boot_device); >>>>>>> + >>>>>>> +} >>>>>>> + >>>>>>> + >>>>>>> static void *spapr_create_fdt_skel(hwaddr initrd_base, >>>>>>> hwaddr initrd_size, >>>>>>> hwaddr kernel_size, >>>>>>> @@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base, >>>>>>> if (boot_device) { >>>>>>> _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device))); >>>>>>> } >>>>>>> + qemu_register_boot_set(spapr_boot_set, fdt); >>>>>> >>>>>> If you simply move the code above (the _FDT() one) from create_fdt_skel >>>>>> to spapr_finalize_fdt() you should have the same net effect and much >>>>>> cleaner code :). >>>>> I've tried your proposal, on reset boot-device property stays "d" >>>> >>>> Ugh, the machine field doesn't change on reset. I think it'd be a lot more intuitive if it did. Can you try with the patch below applied as well? >>>> >>> >>> This approach is not good because boot_set_handler is NULL and return error directly. >>> Please using qemu_register_boot_set for this purpose. >> >> I'd personally prefer if we get rid of qemu_register_boot_set >> completely. It duplicates the reset logic as well as information holder >> locality (machine struct vs parameter). >> > > Maybe yes. But lots of other machines do not register > reset callback. So those machines using qemu_register_boot_set() > register a handler callback achieve this purpose. I think we're better off just registering reset handlers then. Alex