[PATCH v4] hw/ppc: Implement -dtb support for PowerNV

[PATCH v4] hw/ppc: Implement -dtb support for PowerNV

[Aditya Gupta]

Currently any device tree passed with -dtb option in QEMU, was ignored
by the PowerNV code.

Read and pass the passed -dtb to the kernel, thus enabling easier
debugging with custom DTBs.

The existing behaviour when -dtb is 'not' passed, is preserved as-is.

But when a '-dtb' is passed, it completely overrides any dtb nodes or
changes QEMU might have done, such as '-append' arguments to the kernel
(which are mentioned in /chosen/bootargs in the dtb), hence add warning
when -dtb is being used

Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>

---
Changelog
===========
v4:
 + use 'MachineState::fdt' instead of 'PnvMachineState::fdt'
 + added an 'if' check at end of pnv_reset, so we don't free the fdt we
   are using
v3:
 + use 'load_device_tree' to read the device tree, instead of g_file_get_contents
 + tested that passed dtb does NOT get ignored on system_reset

v2:
 + move reading dtb and warning to pnv_init

v1:
 + use 'g_file_get_contents' and add check for -append & -dtb as suggested by Daniel
---
---
 hw/ppc/pnv.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
 1 file changed, 40 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 3526852685b4..5be15f748e45 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -736,21 +736,27 @@ static void pnv_reset(MachineState *machine, ShutdownCause reason)
         }
     }
 
-    fdt = pnv_dt_create(machine);
-
-    /* Pack resulting tree */
-    _FDT((fdt_pack(fdt)));
+    if (machine->fdt) {
+        fdt = machine->fdt;
+    } else {
+        fdt = pnv_dt_create(machine);
+        /* Pack resulting tree */
+        _FDT((fdt_pack(fdt)));
+    }
 
     qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
     cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
 
-    /*
-     * Set machine->fdt for 'dumpdtb' QMP/HMP command. Free
-     * the existing machine->fdt to avoid leaking it during
-     * a reset.
-     */
-    g_free(machine->fdt);
-    machine->fdt = fdt;
+    /* Update machine->fdt with latest fdt */
+    if (machine->fdt != fdt) {
+        /*
+         * Set machine->fdt for 'dumpdtb' QMP/HMP command. Free
+         * the existing machine->fdt to avoid leaking it during
+         * a reset.
+         */
+        g_free(machine->fdt);
+        machine->fdt = fdt;
+    }
 }
 
 static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
@@ -952,6 +958,14 @@ static void pnv_init(MachineState *machine)
         g_free(sz);
         exit(EXIT_FAILURE);
     }
+
+    /* checks for invalid option combinations */
+    if (machine->dtb && (strlen(machine->kernel_cmdline) != 0)) {
+        error_report("-append and -dtb cannot be used together, as passed"
+                " command line is ignored in case of custom dtb");
+        exit(EXIT_FAILURE);
+    }
+
     memory_region_add_subregion(get_system_memory(), 0, machine->ram);
 
     /*
@@ -1003,6 +1017,21 @@ static void pnv_init(MachineState *machine)
         }
     }
 
+    /* load dtb if passed */
+    if (machine->dtb) {
+        int fdt_size;
+
+        warn_report("with manually passed dtb, some options like '-append'"
+                " will get ignored and the dtb passed will be used as-is");
+
+        /* read the file 'machine->dtb', and load it into 'fdt' buffer */
+        machine->fdt = load_device_tree(machine->dtb, &fdt_size);
+        if (!machine->fdt) {
+            error_report("Could not load dtb '%s'", machine->dtb);
+            exit(1);
+        }
+    }
+
     /* MSIs are supported on this platform */
     msi_nonbroken = true;
 
-- 
2.46.0

Re: [PATCH v4] hw/ppc: Implement -dtb support for PowerNV

[Aditya Gupta]

Hi all,

Just a ping for this. Any comments ?


Thanks,

Aditya Gupta

On 20/08/24 16:00, Aditya Gupta wrote:
> Currently any device tree passed with -dtb option in QEMU, was ignored
> by the PowerNV code.
>
> Read and pass the passed -dtb to the kernel, thus enabling easier
> debugging with custom DTBs.
>
> The existing behaviour when -dtb is 'not' passed, is preserved as-is.
>
> But when a '-dtb' is passed, it completely overrides any dtb nodes or
> changes QEMU might have done, such as '-append' arguments to the kernel
> (which are mentioned in /chosen/bootargs in the dtb), hence add warning
> when -dtb is being used
>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
>
> ---
> Changelog
> ===========
> v4:
>   + use 'MachineState::fdt' instead of 'PnvMachineState::fdt'
>   + added an 'if' check at end of pnv_reset, so we don't free the fdt we
>     are using
> v3:
>   + use 'load_device_tree' to read the device tree, instead of g_file_get_contents
>   + tested that passed dtb does NOT get ignored on system_reset
>
> v2:
>   + move reading dtb and warning to pnv_init
>
> v1:
>   + use 'g_file_get_contents' and add check for -append & -dtb as suggested by Daniel
> ---
> ---
>   hw/ppc/pnv.c | 51 ++++++++++++++++++++++++++++++++++++++++-----------
>   1 file changed, 40 insertions(+), 11 deletions(-)
>
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 3526852685b4..5be15f748e45 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -736,21 +736,27 @@ static void pnv_reset(MachineState *machine, ShutdownCause reason)
>           }
>       }
>   
> -    fdt = pnv_dt_create(machine);
> -
> -    /* Pack resulting tree */
> -    _FDT((fdt_pack(fdt)));
> +    if (machine->fdt) {
> +        fdt = machine->fdt;
> +    } else {
> +        fdt = pnv_dt_create(machine);
> +        /* Pack resulting tree */
> +        _FDT((fdt_pack(fdt)));
> +    }
>   
>       qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt));
>       cpu_physical_memory_write(PNV_FDT_ADDR, fdt, fdt_totalsize(fdt));
>   
> -    /*
> -     * Set machine->fdt for 'dumpdtb' QMP/HMP command. Free
> -     * the existing machine->fdt to avoid leaking it during
> -     * a reset.
> -     */
> -    g_free(machine->fdt);
> -    machine->fdt = fdt;
> +    /* Update machine->fdt with latest fdt */
> +    if (machine->fdt != fdt) {
> +        /*
> +         * Set machine->fdt for 'dumpdtb' QMP/HMP command. Free
> +         * the existing machine->fdt to avoid leaking it during
> +         * a reset.
> +         */
> +        g_free(machine->fdt);
> +        machine->fdt = fdt;
> +    }
>   }
>   
>   static ISABus *pnv_chip_power8_isa_create(PnvChip *chip, Error **errp)
> @@ -952,6 +958,14 @@ static void pnv_init(MachineState *machine)
>           g_free(sz);
>           exit(EXIT_FAILURE);
>       }
> +
> +    /* checks for invalid option combinations */
> +    if (machine->dtb && (strlen(machine->kernel_cmdline) != 0)) {
> +        error_report("-append and -dtb cannot be used together, as passed"
> +                " command line is ignored in case of custom dtb");
> +        exit(EXIT_FAILURE);
> +    }
> +
>       memory_region_add_subregion(get_system_memory(), 0, machine->ram);
>   
>       /*
> @@ -1003,6 +1017,21 @@ static void pnv_init(MachineState *machine)
>           }
>       }
>   
> +    /* load dtb if passed */
> +    if (machine->dtb) {
> +        int fdt_size;
> +
> +        warn_report("with manually passed dtb, some options like '-append'"
> +                " will get ignored and the dtb passed will be used as-is");
> +
> +        /* read the file 'machine->dtb', and load it into 'fdt' buffer */
> +        machine->fdt = load_device_tree(machine->dtb, &fdt_size);
> +        if (!machine->fdt) {
> +            error_report("Could not load dtb '%s'", machine->dtb);
> +            exit(1);
> +        }
> +    }
> +
>       /* MSIs are supported on this platform */
>       msi_nonbroken = true;
>   

Re: [PATCH v4] hw/ppc: Implement -dtb support for PowerNV

[Nicholas Piggin]

On Tue Aug 20, 2024 at 8:30 PM AEST, Aditya Gupta wrote:
> Currently any device tree passed with -dtb option in QEMU, was ignored
> by the PowerNV code.
>
> Read and pass the passed -dtb to the kernel, thus enabling easier
> debugging with custom DTBs.
>
> The existing behaviour when -dtb is 'not' passed, is preserved as-is.
>
> But when a '-dtb' is passed, it completely overrides any dtb nodes or
> changes QEMU might have done, such as '-append' arguments to the kernel
> (which are mentioned in /chosen/bootargs in the dtb), hence add warning
> when -dtb is being used
>
> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>

Sorry for getting to it late,

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

Re: [PATCH v4] hw/ppc: Implement -dtb support for PowerNV

[Aditya Gupta]

On 03/11/24 17:04, Nicholas Piggin wrote:

> On Tue Aug 20, 2024 at 8:30 PM AEST, Aditya Gupta wrote:
>> Currently any device tree passed with -dtb option in QEMU, was ignored
>> by the PowerNV code.
>>
>> Read and pass the passed -dtb to the kernel, thus enabling easier
>> debugging with custom DTBs.
>>
>> The existing behaviour when -dtb is 'not' passed, is preserved as-is.
>>
>> But when a '-dtb' is passed, it completely overrides any dtb nodes or
>> changes QEMU might have done, such as '-append' arguments to the kernel
>> (which are mentioned in /chosen/bootargs in the dtb), hence add warning
>> when -dtb is being used
>>
>> Signed-off-by: Aditya Gupta <adityag@linux.ibm.com>
> Sorry for getting to it late,
>
> Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

That's okay. Thanks for the tag Nick !


- Aditya Gupta