diff mbox series

platforms/qemu: update phandle of "interrupt-parent"

Message ID 20190719125916.6554-1-clg@kaod.org
State Not Applicable
Headers show
Series platforms/qemu: update phandle of "interrupt-parent" | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (dab352eecb1dac78112c67d322655e0eae0a16ba)
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present

Commit Message

Cédric Le Goater July 19, 2019, 12:59 p.m. UTC 
QEMU provides a DT populated with the serial devices but the
"interrupt-parent" property is empty (0x0). It was not a problem until
now. But since OpenFirmare started using a recent libdft (>= 1.4.7),
petitboot fails to boot the system image with error :

   dtc_resize: fdt_open_into returned FDT_ERR_BADMAGIC

Provide a DT fixup for "interrupt-parent" properties of the LPC bus.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 platforms/astbmc/astbmc.h |  1 +
 platforms/astbmc/common.c |  6 +++---
 platforms/qemu/qemu.c     | 22 ++++++++++++++++++++++
 3 files changed, 26 insertions(+), 3 deletions(-)

Comments

Greg Kurz July 19, 2019, 5:36 p.m. UTC | #1 
On Fri, 19 Jul 2019 14:59:16 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> QEMU provides a DT populated with the serial devices but the
> "interrupt-parent" property is empty (0x0). It was not a problem until

The "interrupt-parent" property comes from:

    _FDT((fdt_setprop_cell(fdt, node, "interrupt-parent",
                           fdt_get_phandle(fdt, lpc_off))));

Since we don't generate phandles in QEMU, fdt_get_phandle() returns 0.

As an alternative to fixing this in skiboot, what about generating the
phandles in QEMU ?

Something like:
===============
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index a9f150c3cab0..dd2c2a75fcf5 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "sysemu/sysemu.h"
+#include "sysemu/device_tree.h"
 #include "target/ppc/cpu.h"
 #include "qapi/error.h"
 #include "qemu/log.h"
@@ -108,6 +109,7 @@ static int pnv_lpc_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
         cpu_to_be32(lpc_pcba),
         cpu_to_be32(PNV_XSCOM_LPC_SIZE)
     };
+    uint32_t phandle;
 
     name = g_strdup_printf("isa@%x", lpc_pcba);
     offset = fdt_add_subnode(fdt, xscom_offset, name);
@@ -118,6 +120,12 @@ static int pnv_lpc_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
     _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
     _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
     _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
+
+    phandle = qemu_fdt_alloc_phandle(fdt);
+    assert(phandle > 0);
+
+    _FDT((fdt_setprop_cell(fdt, offset, "phandle", phandle)));
+
     return 0;
 }
 
@@ -144,6 +152,7 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset)
                             cpu_to_be32((uint32_t)PNV9_LPCM_SIZE),
     };
     uint32_t reg[2];
+    uint32_t phandle;
 
     /*
      * OPB bus
@@ -212,6 +221,11 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset)
     _FDT((fdt_setprop(fdt, offset, "compatible", lpc_compat,
                       sizeof(lpc_compat))));
 
+    phandle = qemu_fdt_alloc_phandle(fdt);
+    assert(phandle > 0);
+
+    _FDT((fdt_setprop_cell(fdt, offset, "phandle", phandle)));
+
     return 0;
 }
 
===============


> now. But since OpenFirmare started using a recent libdft (>= 1.4.7),
> petitboot fails to boot the system image with error :
> 
>    dtc_resize: fdt_open_into returned FDT_ERR_BADMAGIC
> 
> Provide a DT fixup for "interrupt-parent" properties of the LPC bus.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  platforms/astbmc/astbmc.h |  1 +
>  platforms/astbmc/common.c |  6 +++---
>  platforms/qemu/qemu.c     | 22 ++++++++++++++++++++++
>  3 files changed, 26 insertions(+), 3 deletions(-)
> 
> diff --git a/platforms/astbmc/astbmc.h b/platforms/astbmc/astbmc.h
> index c302b6070d0e..122b0007aabb 100644
> --- a/platforms/astbmc/astbmc.h
> +++ b/platforms/astbmc/astbmc.h
> @@ -96,6 +96,7 @@ extern const struct bmc_platform bmc_plat_ast2500_ami;
>  extern const struct bmc_platform bmc_plat_ast2500_openbmc;
>  
>  extern void astbmc_early_init(void);
> +extern struct dt_node *astbmc_dt_find_primary_lpc(void);
>  extern int64_t astbmc_ipmi_reboot(void);
>  extern int64_t astbmc_ipmi_power_down(uint64_t request);
>  extern void astbmc_init(void);
> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
> index 76fa25f8ab98..7f581bebeb90 100644
> --- a/platforms/astbmc/common.c
> +++ b/platforms/astbmc/common.c
> @@ -381,7 +381,7 @@ static void astbmc_fixup_bmc_sensors(void)
>  	}
>  }
>  
> -static struct dt_node *dt_find_primary_lpc(void)
> +struct dt_node *astbmc_dt_find_primary_lpc(void)
>  {
>  	struct dt_node *n, *primary_lpc = NULL;
>  
> @@ -406,7 +406,7 @@ static void astbmc_fixup_dt(void)
>  {
>  	struct dt_node *primary_lpc;
>  
> -	primary_lpc = dt_find_primary_lpc();
> +	primary_lpc = astbmc_dt_find_primary_lpc();
>  
>  	if (!primary_lpc)
>  		return;
> @@ -496,7 +496,7 @@ void astbmc_early_init(void)
>  		 * fallback.
>  		 */
>  		if (proc_gen == proc_gen_p9) {
> -			astbmc_fixup_dt_mbox(dt_find_primary_lpc());
> +			astbmc_fixup_dt_mbox(astbmc_dt_find_primary_lpc());
>  			ast_setup_sio_mbox(MBOX_IO_BASE, MBOX_LPC_IRQ);
>  		}
>  	} else {
> diff --git a/platforms/qemu/qemu.c b/platforms/qemu/qemu.c
> index b528a826301a..193bec06b195 100644
> --- a/platforms/qemu/qemu.c
> +++ b/platforms/qemu/qemu.c
> @@ -23,6 +23,26 @@
>  
>  static bool bt_device_present;
>  
> +static void qemu_fixup_dt_lpc_interrupt_parent(void)
> +{
> +	struct dt_node *lpc, *node;
> +
> +	lpc = astbmc_dt_find_primary_lpc();
> +
> +	if (!lpc)
> +		return;
> +
> +	dt_for_each_child(lpc, node) {
> +		struct dt_property *prop;
> +
> +		prop = __dt_find_property(node, "interrupt-parent");
> +		if (!prop)
> +			return;
> +		dt_del_property(node, prop);
> +		dt_add_property_cells(node, "interrupt-parent", lpc->phandle);
> +	}
> +}
> +
>  static bool qemu_probe_common(const char *compat)
>  {
>  	struct dt_node *n;
> @@ -37,6 +57,8 @@ static bool qemu_probe_common(const char *compat)
>  		bt_device_present = true;
>  	}
>  
> +	qemu_fixup_dt_lpc_interrupt_parent();
> +
>  	return true;
>  }
>
Cédric Le Goater July 22, 2019, 6:27 a.m. UTC | #2 
On 19/07/2019 19:36, Greg Kurz wrote:
> On Fri, 19 Jul 2019 14:59:16 +0200
> Cédric Le Goater <clg@kaod.org> wrote:
> 
>> QEMU provides a DT populated with the serial devices but the
>> "interrupt-parent" property is empty (0x0). It was not a problem until
> 
> The "interrupt-parent" property comes from:
> 
>     _FDT((fdt_setprop_cell(fdt, node, "interrupt-parent",
>                            fdt_get_phandle(fdt, lpc_off))));
> 
> Since we don't generate phandles in QEMU, fdt_get_phandle() returns 0.
> 
> As an alternative to fixing this in skiboot, what about generating the
> phandles in QEMU ?

This is much better ! Drop that patch. We can handle it at the QEMU
level.

Thanks,

C. 

 
> Something like:
> ===============
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index a9f150c3cab0..dd2c2a75fcf5 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -19,6 +19,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "sysemu/sysemu.h"
> +#include "sysemu/device_tree.h"
>  #include "target/ppc/cpu.h"
>  #include "qapi/error.h"
>  #include "qemu/log.h"
> @@ -108,6 +109,7 @@ static int pnv_lpc_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>          cpu_to_be32(lpc_pcba),
>          cpu_to_be32(PNV_XSCOM_LPC_SIZE)
>      };
> +    uint32_t phandle;
>  
>      name = g_strdup_printf("isa@%x", lpc_pcba);
>      offset = fdt_add_subnode(fdt, xscom_offset, name);
> @@ -118,6 +120,12 @@ static int pnv_lpc_dt_xscom(PnvXScomInterface *dev, void *fdt, int xscom_offset)
>      _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
>      _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
>      _FDT((fdt_setprop(fdt, offset, "compatible", compat, sizeof(compat))));
> +
> +    phandle = qemu_fdt_alloc_phandle(fdt);
> +    assert(phandle > 0);
> +
> +    _FDT((fdt_setprop_cell(fdt, offset, "phandle", phandle)));
> +
>      return 0;
>  }
>  
> @@ -144,6 +152,7 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset)
>                              cpu_to_be32((uint32_t)PNV9_LPCM_SIZE),
>      };
>      uint32_t reg[2];
> +    uint32_t phandle;
>  
>      /*
>       * OPB bus
> @@ -212,6 +221,11 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset)
>      _FDT((fdt_setprop(fdt, offset, "compatible", lpc_compat,
>                        sizeof(lpc_compat))));
>  
> +    phandle = qemu_fdt_alloc_phandle(fdt);
> +    assert(phandle > 0);
> +
> +    _FDT((fdt_setprop_cell(fdt, offset, "phandle", phandle)));
> +
>      return 0;
>  }
>  
> ===============
> 
> 
>> now. But since OpenFirmare started using a recent libdft (>= 1.4.7),
>> petitboot fails to boot the system image with error :
>>
>>    dtc_resize: fdt_open_into returned FDT_ERR_BADMAGIC
>>
>> Provide a DT fixup for "interrupt-parent" properties of the LPC bus.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  platforms/astbmc/astbmc.h |  1 +
>>  platforms/astbmc/common.c |  6 +++---
>>  platforms/qemu/qemu.c     | 22 ++++++++++++++++++++++
>>  3 files changed, 26 insertions(+), 3 deletions(-)
>>
>> diff --git a/platforms/astbmc/astbmc.h b/platforms/astbmc/astbmc.h
>> index c302b6070d0e..122b0007aabb 100644
>> --- a/platforms/astbmc/astbmc.h
>> +++ b/platforms/astbmc/astbmc.h
>> @@ -96,6 +96,7 @@ extern const struct bmc_platform bmc_plat_ast2500_ami;
>>  extern const struct bmc_platform bmc_plat_ast2500_openbmc;
>>  
>>  extern void astbmc_early_init(void);
>> +extern struct dt_node *astbmc_dt_find_primary_lpc(void);
>>  extern int64_t astbmc_ipmi_reboot(void);
>>  extern int64_t astbmc_ipmi_power_down(uint64_t request);
>>  extern void astbmc_init(void);
>> diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
>> index 76fa25f8ab98..7f581bebeb90 100644
>> --- a/platforms/astbmc/common.c
>> +++ b/platforms/astbmc/common.c
>> @@ -381,7 +381,7 @@ static void astbmc_fixup_bmc_sensors(void)
>>  	}
>>  }
>>  
>> -static struct dt_node *dt_find_primary_lpc(void)
>> +struct dt_node *astbmc_dt_find_primary_lpc(void)
>>  {
>>  	struct dt_node *n, *primary_lpc = NULL;
>>  
>> @@ -406,7 +406,7 @@ static void astbmc_fixup_dt(void)
>>  {
>>  	struct dt_node *primary_lpc;
>>  
>> -	primary_lpc = dt_find_primary_lpc();
>> +	primary_lpc = astbmc_dt_find_primary_lpc();
>>  
>>  	if (!primary_lpc)
>>  		return;
>> @@ -496,7 +496,7 @@ void astbmc_early_init(void)
>>  		 * fallback.
>>  		 */
>>  		if (proc_gen == proc_gen_p9) {
>> -			astbmc_fixup_dt_mbox(dt_find_primary_lpc());
>> +			astbmc_fixup_dt_mbox(astbmc_dt_find_primary_lpc());
>>  			ast_setup_sio_mbox(MBOX_IO_BASE, MBOX_LPC_IRQ);
>>  		}
>>  	} else {
>> diff --git a/platforms/qemu/qemu.c b/platforms/qemu/qemu.c
>> index b528a826301a..193bec06b195 100644
>> --- a/platforms/qemu/qemu.c
>> +++ b/platforms/qemu/qemu.c
>> @@ -23,6 +23,26 @@
>>  
>>  static bool bt_device_present;
>>  
>> +static void qemu_fixup_dt_lpc_interrupt_parent(void)
>> +{
>> +	struct dt_node *lpc, *node;
>> +
>> +	lpc = astbmc_dt_find_primary_lpc();
>> +
>> +	if (!lpc)
>> +		return;
>> +
>> +	dt_for_each_child(lpc, node) {
>> +		struct dt_property *prop;
>> +
>> +		prop = __dt_find_property(node, "interrupt-parent");
>> +		if (!prop)
>> +			return;
>> +		dt_del_property(node, prop);
>> +		dt_add_property_cells(node, "interrupt-parent", lpc->phandle);
>> +	}
>> +}
>> +
>>  static bool qemu_probe_common(const char *compat)
>>  {
>>  	struct dt_node *n;
>> @@ -37,6 +57,8 @@ static bool qemu_probe_common(const char *compat)
>>  		bt_device_present = true;
>>  	}
>>  
>> +	qemu_fixup_dt_lpc_interrupt_parent();
>> +
>>  	return true;
>>  }
>>  
>
diff mbox series

Patch

diff --git a/platforms/astbmc/astbmc.h b/platforms/astbmc/astbmc.h
index c302b6070d0e..122b0007aabb 100644
--- a/platforms/astbmc/astbmc.h
+++ b/platforms/astbmc/astbmc.h
@@ -96,6 +96,7 @@  extern const struct bmc_platform bmc_plat_ast2500_ami;
 extern const struct bmc_platform bmc_plat_ast2500_openbmc;
 
 extern void astbmc_early_init(void);
+extern struct dt_node *astbmc_dt_find_primary_lpc(void);
 extern int64_t astbmc_ipmi_reboot(void);
 extern int64_t astbmc_ipmi_power_down(uint64_t request);
 extern void astbmc_init(void);
diff --git a/platforms/astbmc/common.c b/platforms/astbmc/common.c
index 76fa25f8ab98..7f581bebeb90 100644
--- a/platforms/astbmc/common.c
+++ b/platforms/astbmc/common.c
@@ -381,7 +381,7 @@  static void astbmc_fixup_bmc_sensors(void)
 	}
 }
 
-static struct dt_node *dt_find_primary_lpc(void)
+struct dt_node *astbmc_dt_find_primary_lpc(void)
 {
 	struct dt_node *n, *primary_lpc = NULL;
 
@@ -406,7 +406,7 @@  static void astbmc_fixup_dt(void)
 {
 	struct dt_node *primary_lpc;
 
-	primary_lpc = dt_find_primary_lpc();
+	primary_lpc = astbmc_dt_find_primary_lpc();
 
 	if (!primary_lpc)
 		return;
@@ -496,7 +496,7 @@  void astbmc_early_init(void)
 		 * fallback.
 		 */
 		if (proc_gen == proc_gen_p9) {
-			astbmc_fixup_dt_mbox(dt_find_primary_lpc());
+			astbmc_fixup_dt_mbox(astbmc_dt_find_primary_lpc());
 			ast_setup_sio_mbox(MBOX_IO_BASE, MBOX_LPC_IRQ);
 		}
 	} else {
diff --git a/platforms/qemu/qemu.c b/platforms/qemu/qemu.c
index b528a826301a..193bec06b195 100644
--- a/platforms/qemu/qemu.c
+++ b/platforms/qemu/qemu.c
@@ -23,6 +23,26 @@ 
 
 static bool bt_device_present;
 
+static void qemu_fixup_dt_lpc_interrupt_parent(void)
+{
+	struct dt_node *lpc, *node;
+
+	lpc = astbmc_dt_find_primary_lpc();
+
+	if (!lpc)
+		return;
+
+	dt_for_each_child(lpc, node) {
+		struct dt_property *prop;
+
+		prop = __dt_find_property(node, "interrupt-parent");
+		if (!prop)
+			return;
+		dt_del_property(node, prop);
+		dt_add_property_cells(node, "interrupt-parent", lpc->phandle);
+	}
+}
+
 static bool qemu_probe_common(const char *compat)
 {
 	struct dt_node *n;
@@ -37,6 +57,8 @@  static bool qemu_probe_common(const char *compat)
 		bt_device_present = true;
 	}
 
+	qemu_fixup_dt_lpc_interrupt_parent();
+
 	return true;
 }