[PATCH for-8.2 0/3] pnv/lpc: Hook up xscoms for LPC

[PATCH for-8.2 0/3] pnv/lpc: Hook up xscoms for LPC

[Joel Stanley]

P8 used xscoms for accessing the LPC bus. In P9 the LPC bus was memory
mapped, simplifying access, however the xscom registers remained. Some
firmwares use this method on P9 and P10, so wire it up in qemu.

The third patch is a hack that shows how to test this access method with
skiboot. It should not be applied.

Joel Stanley (3):
  pnv/lpc: Place mmio regs in their own memory region
  pnv/lpc: Hook up xscom region for P9/P10
  HACK: pnv/lpc: Set up XSCOM dt for P9

 include/hw/ppc/pnv_lpc.h   |  3 ++-
 include/hw/ppc/pnv_xscom.h |  6 ++++++
 hw/ppc/pnv.c               |  8 ++++++--
 hw/ppc/pnv_lpc.c           | 13 ++++++++++++-
 4 files changed, 26 insertions(+), 4 deletions(-)

-- 
2.40.1

[PATCH for-8.2 1/3] pnv/lpc: Place mmio regs in their own memory region

[Joel Stanley]

The P9 and P10 models re-used the xscom_regs memory region for the mmio
access, which is confusing.

Add a separate memory region in preparation for enabling both xscom and
mmio access.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/hw/ppc/pnv_lpc.h | 3 ++-
 hw/ppc/pnv.c             | 4 ++--
 hw/ppc/pnv_lpc.c         | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
index 5d22c4557041..3000964f8999 100644
--- a/include/hw/ppc/pnv_lpc.h
+++ b/include/hw/ppc/pnv_lpc.h
@@ -81,8 +81,9 @@ struct PnvLpcController {
     uint32_t lpc_hc_irqstat;
     uint32_t lpc_hc_error_addr;
 
-    /* XSCOM registers */
+    /* Registers */
     MemoryRegion xscom_regs;
+    MemoryRegion mmio_regs;
 
     /* PSI to generate interrupts */
     qemu_irq psi_irq;
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index eb54f93986df..afdaa25c2b26 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1565,7 +1565,7 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
         return;
     }
     memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
-                                &chip9->lpc.xscom_regs);
+                                &chip9->lpc.mmio_regs);
 
     chip->fw_mr = &chip9->lpc.isa_fw;
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
@@ -1784,7 +1784,7 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
         return;
     }
     memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
-                                &chip10->lpc.xscom_regs);
+                                &chip10->lpc.mmio_regs);
 
     chip->fw_mr = &chip10->lpc.isa_fw;
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index d692858bee78..caf5e10a5f96 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -664,7 +664,7 @@ static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
     }
 
     /* P9 uses a MMIO region */
-    memory_region_init_io(&lpc->xscom_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
+    memory_region_init_io(&lpc->mmio_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
                           lpc, "lpcm", PNV9_LPCM_SIZE);
 }
 
-- 
2.40.1

[PATCH for-8.2 2/3] pnv/lpc: Hook up xscom region for P9/P10

[Joel Stanley]

From P9 on the LPC bus is memory mapped. However the xscom access still
is possible, so add it too.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/hw/ppc/pnv_xscom.h | 6 ++++++
 hw/ppc/pnv.c               | 4 ++++
 hw/ppc/pnv_lpc.c           | 6 ++++++
 3 files changed, 16 insertions(+)

diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index 9bc64635471e..42601bdf419d 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -96,6 +96,9 @@ struct PnvXScomInterfaceClass {
 #define PNV9_XSCOM_SBE_CTRL_BASE  0x00050008
 #define PNV9_XSCOM_SBE_CTRL_SIZE  0x1
 
+#define PNV9_XSCOM_LPC_BASE       0x00090040
+#define PNV9_XSCOM_LPC_SIZE       PNV_XSCOM_LPC_SIZE
+
 #define PNV9_XSCOM_SBE_MBOX_BASE  0x000D0050
 #define PNV9_XSCOM_SBE_MBOX_SIZE  0x16
 
@@ -155,6 +158,9 @@ struct PnvXScomInterfaceClass {
 #define PNV10_XSCOM_SBE_CTRL_BASE  PNV9_XSCOM_SBE_CTRL_BASE
 #define PNV10_XSCOM_SBE_CTRL_SIZE  PNV9_XSCOM_SBE_CTRL_SIZE
 
+#define PNV10_XSCOM_LPC_BASE       PNV9_XSCOM_LPC_BASE
+#define PNV10_XSCOM_LPC_SIZE       PNV9_XSCOM_LPC_SIZE
+
 #define PNV10_XSCOM_SBE_MBOX_BASE  PNV9_XSCOM_SBE_MBOX_BASE
 #define PNV10_XSCOM_SBE_MBOX_SIZE  PNV9_XSCOM_SBE_MBOX_SIZE
 
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index afdaa25c2b26..a5db655b41b6 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1566,6 +1566,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     }
     memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
                                 &chip9->lpc.mmio_regs);
+    pnv_xscom_add_subregion(chip, PNV9_XSCOM_LPC_BASE,
+                            &chip9->lpc.xscom_regs);
 
     chip->fw_mr = &chip9->lpc.isa_fw;
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
@@ -1785,6 +1787,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     }
     memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
                                 &chip10->lpc.mmio_regs);
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_LPC_BASE,
+                            &chip10->lpc.xscom_regs);
 
     chip->fw_mr = &chip10->lpc.isa_fw;
     chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index caf5e10a5f96..6c6a3134087f 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -666,6 +666,12 @@ static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
     /* P9 uses a MMIO region */
     memory_region_init_io(&lpc->mmio_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
                           lpc, "lpcm", PNV9_LPCM_SIZE);
+
+    /* but the XSCOM region still exists */
+    pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(lpc),
+                          &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
+                          PNV_XSCOM_LPC_SIZE);
+
 }
 
 static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)
-- 
2.40.1

[PATCH for-8.2 3/3] HACK: pnv/lpc: Set up XSCOM dt for P9

[Joel Stanley]

To test qemu's model of the xscom interface, apply this patch to qemu
and the following change to skiboot:

  --- a/hw/lpc.c
  +++ b/hw/lpc.c
  @@ -1266,7 +1266,7 @@ static void lpc_init_chip_p9(struct dt_node *opb_node)
          lpc = zalloc(sizeof(struct lpcm));
          assert(lpc);
          lpc->chip_id = gcid;
  -       lpc->mbase = (void *)addr;
  +       lpc->xbase = dt_get_address(lpc_node, 0, NULL);
          lpc->fw_idsel = 0xff;
          lpc->fw_rdsz = 0xff;
          lpc->node = lpc_node;

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 hw/ppc/pnv_lpc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index 6c6a3134087f..62ab688407a3 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -218,6 +218,11 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset, uint64_t lpcm_addr,
     offset = fdt_add_subnode(fdt, lpcm_offset, name);
     _FDT(offset);
     g_free(name);
+    uint32_t lpc_pcba = PNV9_XSCOM_LPC_BASE;
+    reg[0] = cpu_to_be32(lpc_pcba);
+    reg[1] = cpu_to_be32(PNV_XSCOM_LPC_SIZE);
+
+    _FDT((fdt_setprop(fdt, offset, "reg", reg, sizeof(reg))));
     _FDT((fdt_setprop_cell(fdt, offset, "#address-cells", 2)));
     _FDT((fdt_setprop_cell(fdt, offset, "#size-cells", 1)));
     _FDT((fdt_setprop(fdt, offset, "compatible", lpc_compat,
-- 
2.40.1

Re: [PATCH for-8.2 2/3] pnv/lpc: Hook up xscom region for P9/P10

[Frederic Barrat]

Hello Joel,

So we're re-using the same xscom ops as on P8. A quick look at the 
definition of those 4 registers on P8 (0xb0020) and on P9/P10 
(0x00090040) seem to show they are not the same though. Am i missing 
something?

   Fred


On 08/08/2023 10:34, Joel Stanley wrote:
>  From P9 on the LPC bus is memory mapped. However the xscom access still
> is possible, so add it too.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   include/hw/ppc/pnv_xscom.h | 6 ++++++
>   hw/ppc/pnv.c               | 4 ++++
>   hw/ppc/pnv_lpc.c           | 6 ++++++
>   3 files changed, 16 insertions(+)
> 
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index 9bc64635471e..42601bdf419d 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -96,6 +96,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV9_XSCOM_SBE_CTRL_BASE  0x00050008
>   #define PNV9_XSCOM_SBE_CTRL_SIZE  0x1
>   
> +#define PNV9_XSCOM_LPC_BASE       0x00090040
> +#define PNV9_XSCOM_LPC_SIZE       PNV_XSCOM_LPC_SIZE
> +
>   #define PNV9_XSCOM_SBE_MBOX_BASE  0x000D0050
>   #define PNV9_XSCOM_SBE_MBOX_SIZE  0x16
>   
> @@ -155,6 +158,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV10_XSCOM_SBE_CTRL_BASE  PNV9_XSCOM_SBE_CTRL_BASE
>   #define PNV10_XSCOM_SBE_CTRL_SIZE  PNV9_XSCOM_SBE_CTRL_SIZE
>   
> +#define PNV10_XSCOM_LPC_BASE       PNV9_XSCOM_LPC_BASE
> +#define PNV10_XSCOM_LPC_SIZE       PNV9_XSCOM_LPC_SIZE
> +
>   #define PNV10_XSCOM_SBE_MBOX_BASE  PNV9_XSCOM_SBE_MBOX_BASE
>   #define PNV10_XSCOM_SBE_MBOX_SIZE  PNV9_XSCOM_SBE_MBOX_SIZE
>   
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index afdaa25c2b26..a5db655b41b6 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1566,6 +1566,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>       }
>       memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
>                                   &chip9->lpc.mmio_regs);
> +    pnv_xscom_add_subregion(chip, PNV9_XSCOM_LPC_BASE,
> +                            &chip9->lpc.xscom_regs);
>   
>       chip->fw_mr = &chip9->lpc.isa_fw;
>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> @@ -1785,6 +1787,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>       }
>       memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
>                                   &chip10->lpc.mmio_regs);
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_LPC_BASE,
> +                            &chip10->lpc.xscom_regs);
>   
>       chip->fw_mr = &chip10->lpc.isa_fw;
>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index caf5e10a5f96..6c6a3134087f 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -666,6 +666,12 @@ static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
>       /* P9 uses a MMIO region */
>       memory_region_init_io(&lpc->mmio_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
>                             lpc, "lpcm", PNV9_LPCM_SIZE);
> +
> +    /* but the XSCOM region still exists */
> +    pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(lpc),
> +                          &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
> +                          PNV_XSCOM_LPC_SIZE);
> +
>   }
>   
>   static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)

Re: [PATCH for-8.2 2/3] pnv/lpc: Hook up xscom region for P9/P10

[Cédric Le Goater]

On 8/9/23 16:56, Frederic Barrat wrote:
> Hello Joel,
> 
> So we're re-using the same xscom ops as on P8. A quick look at the definition of those 4 registers on P8 (0xb0020) and on P9/P10 (0x00090040) seem to show they are not the same though. Am i missing something?

Joel, are we ok ? Should we grab this patch ? or not.

Thanks,

C.


> 
>    Fred
> 
> 
> On 08/08/2023 10:34, Joel Stanley wrote:
>>  From P9 on the LPC bus is memory mapped. However the xscom access still
>> is possible, so add it too.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
>> ---
>>   include/hw/ppc/pnv_xscom.h | 6 ++++++
>>   hw/ppc/pnv.c               | 4 ++++
>>   hw/ppc/pnv_lpc.c           | 6 ++++++
>>   3 files changed, 16 insertions(+)
>>
>> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
>> index 9bc64635471e..42601bdf419d 100644
>> --- a/include/hw/ppc/pnv_xscom.h
>> +++ b/include/hw/ppc/pnv_xscom.h
>> @@ -96,6 +96,9 @@ struct PnvXScomInterfaceClass {
>>   #define PNV9_XSCOM_SBE_CTRL_BASE  0x00050008
>>   #define PNV9_XSCOM_SBE_CTRL_SIZE  0x1
>> +#define PNV9_XSCOM_LPC_BASE       0x00090040
>> +#define PNV9_XSCOM_LPC_SIZE       PNV_XSCOM_LPC_SIZE
>> +
>>   #define PNV9_XSCOM_SBE_MBOX_BASE  0x000D0050
>>   #define PNV9_XSCOM_SBE_MBOX_SIZE  0x16
>> @@ -155,6 +158,9 @@ struct PnvXScomInterfaceClass {
>>   #define PNV10_XSCOM_SBE_CTRL_BASE  PNV9_XSCOM_SBE_CTRL_BASE
>>   #define PNV10_XSCOM_SBE_CTRL_SIZE  PNV9_XSCOM_SBE_CTRL_SIZE
>> +#define PNV10_XSCOM_LPC_BASE       PNV9_XSCOM_LPC_BASE
>> +#define PNV10_XSCOM_LPC_SIZE       PNV9_XSCOM_LPC_SIZE
>> +
>>   #define PNV10_XSCOM_SBE_MBOX_BASE  PNV9_XSCOM_SBE_MBOX_BASE
>>   #define PNV10_XSCOM_SBE_MBOX_SIZE  PNV9_XSCOM_SBE_MBOX_SIZE
>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>> index afdaa25c2b26..a5db655b41b6 100644
>> --- a/hw/ppc/pnv.c
>> +++ b/hw/ppc/pnv.c
>> @@ -1566,6 +1566,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>       }
>>       memory_region_add_subregion(get_system_memory(), PNV9_LPCM_BASE(chip),
>>                                   &chip9->lpc.mmio_regs);
>> +    pnv_xscom_add_subregion(chip, PNV9_XSCOM_LPC_BASE,
>> +                            &chip9->lpc.xscom_regs);
>>       chip->fw_mr = &chip9->lpc.isa_fw;
>>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>> @@ -1785,6 +1787,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>>       }
>>       memory_region_add_subregion(get_system_memory(), PNV10_LPCM_BASE(chip),
>>                                   &chip10->lpc.mmio_regs);
>> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_LPC_BASE,
>> +                            &chip10->lpc.xscom_regs);
>>       chip->fw_mr = &chip10->lpc.isa_fw;
>>       chip->dt_isa_nodename = g_strdup_printf("/lpcm-opb@%" PRIx64 "/lpc@0",
>> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
>> index caf5e10a5f96..6c6a3134087f 100644
>> --- a/hw/ppc/pnv_lpc.c
>> +++ b/hw/ppc/pnv_lpc.c
>> @@ -666,6 +666,12 @@ static void pnv_lpc_power9_realize(DeviceState *dev, Error **errp)
>>       /* P9 uses a MMIO region */
>>       memory_region_init_io(&lpc->mmio_regs, OBJECT(lpc), &pnv_lpc_mmio_ops,
>>                             lpc, "lpcm", PNV9_LPCM_SIZE);
>> +
>> +    /* but the XSCOM region still exists */
>> +    pnv_xscom_region_init(&lpc->xscom_regs, OBJECT(lpc),
>> +                          &pnv_lpc_xscom_ops, lpc, "xscom-lpc",
>> +                          PNV_XSCOM_LPC_SIZE);
>> +
>>   }
>>   static void pnv_lpc_power9_class_init(ObjectClass *klass, void *data)

Re: [PATCH for-8.2 2/3] pnv/lpc: Hook up xscom region for P9/P10

[Joel Stanley]

On Tue, 29 Aug 2023 at 14:45, Cédric Le Goater <clg@kaod.org> wrote:
>
> On 8/9/23 16:56, Frederic Barrat wrote:
> > Hello Joel,
> >
> > So we're re-using the same xscom ops as on P8. A quick look at the definition of those 4 registers on P8 (0xb0020) and on P9/P10 (0x00090040) seem to show they are not the same though. Am i missing something?
>
> Joel, are we ok ? Should we grab this patch ? or not.

Sorry, I chatted to Fred about this one but forgot to reply on the
list. I made a bad assumption about the xscom registers matching
between p8 and p9/10. This patch will need to be reworked, so please
ignore it.

Cheers,

Joel