From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:56413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YGdBa-0000ZZ-I5 for qemu-devel@nongnu.org; Wed, 28 Jan 2015 19:42:19 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YGdBU-0002lW-2Z for qemu-devel@nongnu.org; Wed, 28 Jan 2015 19:42:18 -0500 Message-ID: <54C981E0.3080709@suse.de> Date: Thu, 29 Jan 2015 01:42:08 +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> In-Reply-To: <54C98171.6050101@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 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). Alex