Re: [PATCH qemu] spapr: Add /choses to FDT only at reset time to preserve kernel and initramdisk

[David Gibson]

On Thu, Oct 24, 2019 at 03:13:08PM +1100, Alexey Kardashevskiy wrote:
65;5603;1c> Since "spapr: Render full FDT on ibm,client-architecture-support" 
we build
> the entire flatten device tree (FDT) twice - at the reset time and
> when "ibm,client-architecture-support" (CAS) is called. The full FDT from
> CAS is then applied on top of the SLOF internal device tree.
> 
> This is mostly ok, however there is a case when the QEMU is started with
> -initrd and for some reason the guest decided to move/unpack the init RAM
> disk image - the guest correctly notifies SLOF about the change but
> at CAS it is overridden with the QEMU initial location addresses and
> the guest may fail to boot if the original initrd memory was changed.
> 
> This fixes the problem by only adding the /chosen node at the reset time
> to prevent the original QEMU's linux,initrd-start/linux,initrd-end to
> override the updated addresses.
> 
> This only treats /chosen differently as we know there is a special case
> already and it is unlikely anything else will need to change /chosen at CAS
> we are better off not touching /chosen after we handed it over to SLOF.
> 
> Signed-off-by: Alexey Kardashevskiy <address@hidden>

Applied to ppc-for-4.2 (with the typo in the subject line corrected
/choses -> /chosen).

> ---
>  hw/ppc/spapr.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d4c07a9b1bab..0580789a1509 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -925,7 +925,7 @@ static bool spapr_hotplugged_dev_before_cas(void)
>      return false;
>  }
>  
> -static void *spapr_build_fdt(SpaprMachineState *spapr);
> +static void *spapr_build_fdt(SpaprMachineState *spapr, bool reset);
>  
>  int spapr_h_cas_compose_response(SpaprMachineState *spapr,
>                                   target_ulong addr, target_ulong size,
> @@ -947,7 +947,7 @@ int spapr_h_cas_compose_response(SpaprMachineState *spapr,
>  
>      size -= sizeof(hdr);
>  
> -    fdt = spapr_build_fdt(spapr);
> +    fdt = spapr_build_fdt(spapr, false);
>      _FDT((fdt_pack(fdt)));
>  
>      if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
> @@ -1205,7 +1205,7 @@ static void spapr_dt_hypervisor(SpaprMachineState 
> *spapr, void *fdt)
>      }
>  }
>  
> -static void *spapr_build_fdt(SpaprMachineState *spapr)
> +static void *spapr_build_fdt(SpaprMachineState *spapr, bool reset)
>  {
>      MachineState *machine = MACHINE(spapr);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> @@ -1305,7 +1305,9 @@ static void *spapr_build_fdt(SpaprMachineState *spapr)
>      spapr_dt_rtas(spapr, fdt);
>  
>      /* /chosen */
> -    spapr_dt_chosen(spapr, fdt);
> +    if (reset) {
> +        spapr_dt_chosen(spapr, fdt);
> +    }
>  
>      /* /hypervisor */
>      if (kvm_enabled()) {
> @@ -1313,11 +1315,14 @@ static void *spapr_build_fdt(SpaprMachineState *spapr)
>      }
>  
>      /* Build memory reserve map */
> -    if (spapr->kernel_size) {
> -        _FDT((fdt_add_mem_rsv(fdt, KERNEL_LOAD_ADDR, spapr->kernel_size)));
> -    }
> -    if (spapr->initrd_size) {
> -        _FDT((fdt_add_mem_rsv(fdt, spapr->initrd_base, spapr->initrd_size)));
> +    if (reset) {
> +        if (spapr->kernel_size) {
> +            _FDT((fdt_add_mem_rsv(fdt, KERNEL_LOAD_ADDR, 
> spapr->kernel_size)));
> +        }
> +        if (spapr->initrd_size) {
> +            _FDT((fdt_add_mem_rsv(fdt, spapr->initrd_base,
> +                                  spapr->initrd_size)));
> +        }
>      }
>  
>      /* ibm,client-architecture-support updates */
> @@ -1726,7 +1731,7 @@ static void spapr_machine_reset(MachineState *machine)
>       */
>      fdt_addr = MIN(spapr->rma_size, RTAS_MAX_ADDR) - FDT_MAX_SIZE;
>  
> -    fdt = spapr_build_fdt(spapr);
> +    fdt = spapr_build_fdt(spapr, true);
>  
>      rc = fdt_pack(fdt);
>  

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature