On Wed, 11 Sep 2019 14:04:50 +1000
David Gibson <da...@gibson.dropbear.id.au> wrote:
> From: Alexey Kardashevskiy <a...@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 <a...@ozlabs.ru>
> Signed-off-by: David Gibson <da...@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),
