From: Greg Kurz <groug@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>
Cc: lvivier@redhat.com, aik@ozlabs.ru, qemu-devel@nongnu.org,
qemu-ppc@nongnu.org, clg@kaod.org, philmd@redhat.com
Subject: Re: [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append
Date: Wed, 11 Sep 2019 10:46:50 +0200 [thread overview]
Message-ID: <20190911104650.7c64009f@bahia.lan> (raw)
In-Reply-To: <20190911040452.8341-6-david@gibson.dropbear.id.au>
On Wed, 11 Sep 2019 14:04:50 +1000
David Gibson <david@gibson.dropbear.id.au> wrote:
> From: Alexey Kardashevskiy <aik@ozlabs.ru>
>
> We are going to use spapr_build_fdt() for the boot time FDT and as an
> update for SLOF during handling of H_CAS. SLOF will apply all properties
> from the QEMU's FDT which is usually ok unless there are properties
> changed by grub or guest kernel. The properties are:
> bootargs, linux,initrd-start, linux,initrd-end, linux,stdout-path,
> linux,rtas-base, linux,rtas-entry. Resetting those during CAS will most
> likely cause grub failure.
>
s/Resetting/Clearing ? They still get reset to the initial setup if "-kernel"
and "-initrd" were passed, but it is okay since neither grub, nor the guest
kernel is supposed to change them in this case, correct ?
> This only creates such properties if we are booting with "-kernel" and
> "-initrd" so they won't get included into the DT update blob and
so they won't get included {if we're not booting with "-kernel" ...}
> therefore the guest is more likely to boot successfully.
>
Maybe rephrase like:
Don't create such properties if we're booting without "-kernel" and
"-initrd" ...
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> hw/ppc/spapr.c | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d072c2aa3d..d18744268f 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1177,11 +1177,16 @@ static void spapr_dt_chosen(SpaprMachineState *spapr, void *fdt)
>
> _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen"));
>
> - _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline));
> - _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
> - spapr->initrd_base));
> - _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
> - spapr->initrd_base + spapr->initrd_size));
> + if (machine->kernel_cmdline && machine->kernel_cmdline[0]) {
machine->kernel_cmdline cannot be NULL.
From vl.c:
if (!kernel_cmdline) {
kernel_cmdline = "";
current_machine->kernel_cmdline = (char *)kernel_cmdline;
}
Also this doesn't check if we're booting with -kernel but rather
that we're booting with -append ${some_not_empty_string}... what
about checking spapr->kernel_size, pretty much like you do for
the initrd ?
> + _FDT(fdt_setprop_string(fdt, chosen, "bootargs",
> + machine->kernel_cmdline));
> + }
> + if (spapr->initrd_size) {
> + _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start",
> + spapr->initrd_base));
> + _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end",
> + spapr->initrd_base + spapr->initrd_size));
> + }
>
> if (spapr->kernel_size) {
> uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR),
next prev parent reply other threads:[~2019-09-11 9:17 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-11 4:04 [Qemu-devel] [PATCH 0/7] spapr: CAS and reset cleanup preliminaries David Gibson
2019-09-11 4:04 ` [Qemu-devel] [PATCH 1/7] spapr: Simplify handling of pre ISA 3.0 guest workaround handling David Gibson
2019-09-11 7:09 ` Cédric Le Goater
2019-09-11 7:26 ` Greg Kurz
2019-09-11 7:37 ` Alexey Kardashevskiy
2019-09-11 4:04 ` [Qemu-devel] [PATCH 2/7] spapr: Move handling of special NVLink numa node from reset to init David Gibson
2019-09-11 7:28 ` Cédric Le Goater
2019-09-11 7:33 ` Greg Kurz
2019-09-11 7:41 ` Alexey Kardashevskiy
2019-09-11 4:04 ` [Qemu-devel] [PATCH 3/7] spapr: Fixes a leak in CAS David Gibson
2019-09-11 7:28 ` Cédric Le Goater
2019-09-11 7:36 ` Greg Kurz
2019-09-11 4:04 ` [Qemu-devel] [PATCH 4/7] spapr: Skip leading zeroes from memory@ DT node names David Gibson
2019-09-11 8:01 ` Greg Kurz
2019-09-11 4:04 ` [Qemu-devel] [PATCH 5/7] spapr: Do not put empty properties for -kernel/-initrd/-append David Gibson
2019-09-11 8:46 ` Greg Kurz [this message]
2019-09-12 1:59 ` Alexey Kardashevskiy
2019-09-11 4:04 ` [Qemu-devel] [PATCH 6/7] spapr: Stop providing RTAS blob David Gibson
2019-09-11 9:16 ` Greg Kurz
2019-09-12 1:50 ` Alexey Kardashevskiy
2019-09-12 7:20 ` Greg Kurz
2019-09-11 4:04 ` [Qemu-devel] [PATCH 7/7] spapr: Perform machine reset in a more sensible order David Gibson
2019-09-11 7:40 ` Alexey Kardashevskiy
2019-09-11 7:51 ` David Gibson
2019-09-11 7:54 ` Cédric Le Goater
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190911104650.7c64009f@bahia.lan \
--to=groug@kaod.org \
--cc=aik@ozlabs.ru \
--cc=clg@kaod.org \
--cc=david@gibson.dropbear.id.au \
--cc=lvivier@redhat.com \
--cc=philmd@redhat.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).