All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] ppc/pnv: ADU model for POWER9/10
@ 2024-04-17 11:02 Nicholas Piggin
  2024-04-17 11:02 ` [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC " Nicholas Piggin
  2024-04-17 11:02 ` [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space Nicholas Piggin
  0 siblings, 2 replies; 13+ messages in thread
From: Nicholas Piggin @ 2024-04-17 11:02 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Cédric Le Goater,
	Frédéric Barrat, Saif Abrar

These patches adds the framework for a proper ADU model rather than
putting registers into the xscom default ops, and implements ADU's
indirect LPC access functionality which IBM's proprietary firmware
uses to provide consoles on UARTs.

Patch 1 should be quite a simple hooking up the xscom address space.

Patch 2 implements one of the memory access functions of the ADU that
drives access to LPC address space from XSCOM register operations which
is non-trivial but there are similar examples already in tree.

Thanks,
Nick

Nicholas Piggin (2):
  ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  ppc/pnv: Implement ADU access to LPC space

 include/hw/ppc/pnv_adu.h   |  41 ++++++++
 include/hw/ppc/pnv_chip.h  |   3 +
 include/hw/ppc/pnv_lpc.h   |   5 +
 include/hw/ppc/pnv_xscom.h |   6 ++
 hw/ppc/pnv.c               |  20 ++++
 hw/ppc/pnv_adu.c           | 202 +++++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_lpc.c           |  12 +--
 hw/ppc/pnv_xscom.c         |   9 --
 hw/ppc/meson.build         |   1 +
 hw/ppc/trace-events        |   4 +
 10 files changed, 288 insertions(+), 15 deletions(-)
 create mode 100644 include/hw/ppc/pnv_adu.h
 create mode 100644 hw/ppc/pnv_adu.c

-- 
2.43.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-04-17 11:02 [PATCH 0/2] ppc/pnv: ADU model for POWER9/10 Nicholas Piggin
@ 2024-04-17 11:02 ` Nicholas Piggin
  2024-04-17 11:25   ` Cédric Le Goater
  2024-04-17 11:02 ` [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space Nicholas Piggin
  1 sibling, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-04-17 11:02 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Cédric Le Goater,
	Frédéric Barrat, Saif Abrar

This implements a framework for an ADU unit model.

The ADU unit actually implements XSCOM, which is the bridge between MMIO
and PIB. However it also includes control and status registers and other
functions that are exposed as PIB (xscom) registers.

To keep things simple, pnv_xscom.c remains the XSCOM bridge
implementation, and pnv_adu.c implements the ADU registers and other
functions.

So far, just the ADU no-op registers in the pnv_xscom.c default handler
are moved over to the adu model.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
 include/hw/ppc/pnv_chip.h  |   3 +
 include/hw/ppc/pnv_xscom.h |   6 ++
 hw/ppc/pnv.c               |  16 ++++++
 hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_xscom.c         |   9 ---
 hw/ppc/meson.build         |   1 +
 hw/ppc/trace-events        |   4 ++
 8 files changed, 175 insertions(+), 9 deletions(-)
 create mode 100644 include/hw/ppc/pnv_adu.h
 create mode 100644 hw/ppc/pnv_adu.c

diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
new file mode 100644
index 0000000000..9dc91857a9
--- /dev/null
+++ b/include/hw/ppc/pnv_adu.h
@@ -0,0 +1,34 @@
+/*
+ * QEMU PowerPC PowerNV Emulation of some ADU behaviour
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#ifndef PPC_PNV_ADU_H
+#define PPC_PNV_ADU_H
+
+#include "hw/ppc/pnv.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_PNV_ADU "pnv-adu"
+
+OBJECT_DECLARE_TYPE(PnvADU, PnvADUClass, PNV_ADU)
+
+struct PnvADU {
+    DeviceState xd;
+
+    MemoryRegion xscom_regs;
+};
+
+struct PnvADUClass {
+    DeviceClass parent_class;
+
+    int xscom_ctrl_size;
+    int xscom_mbox_size;
+    const MemoryRegionOps *xscom_ctrl_ops;
+    const MemoryRegionOps *xscom_mbox_ops;
+};
+
+#endif /* PPC_PNV_ADU_H */
diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
index 8589f3291e..96e50a2983 100644
--- a/include/hw/ppc/pnv_chip.h
+++ b/include/hw/ppc/pnv_chip.h
@@ -2,6 +2,7 @@
 #define PPC_PNV_CHIP_H
 
 #include "hw/pci-host/pnv_phb4.h"
+#include "hw/ppc/pnv_adu.h"
 #include "hw/ppc/pnv_chiptod.h"
 #include "hw/ppc/pnv_core.h"
 #include "hw/ppc/pnv_homer.h"
@@ -77,6 +78,7 @@ struct Pnv9Chip {
     PnvChip      parent_obj;
 
     /*< public >*/
+    PnvADU       adu;
     PnvXive      xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
@@ -110,6 +112,7 @@ struct Pnv10Chip {
     PnvChip      parent_obj;
 
     /*< public >*/
+    PnvADU       adu;
     PnvXive2     xive;
     Pnv9Psi      psi;
     PnvLpcController lpc;
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index 6209e18492..e93d310e79 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -82,6 +82,9 @@ struct PnvXScomInterfaceClass {
 #define PNV_XSCOM_PBCQ_SPCI_BASE  0x9013c00
 #define PNV_XSCOM_PBCQ_SPCI_SIZE  0x5
 
+#define PNV9_XSCOM_ADU_BASE       0x0090000
+#define PNV9_XSCOM_ADU_SIZE       0x55
+
 /*
  * Layout of the XSCOM PCB addresses (POWER 9)
  */
@@ -128,6 +131,9 @@ struct PnvXScomInterfaceClass {
 #define PNV9_XSCOM_PEC_PCI_STK1   0x140
 #define PNV9_XSCOM_PEC_PCI_STK2   0x180
 
+#define PNV10_XSCOM_ADU_BASE      0x0090000
+#define PNV10_XSCOM_ADU_SIZE      0x55
+
 /*
  * Layout of the XSCOM PCB addresses (POWER 10)
  */
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 6e3a5ccdec..5869aac89a 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1530,6 +1530,7 @@ static void pnv_chip_power9_instance_init(Object *obj)
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
     int i;
 
+    object_initialize_child(obj, "adu",  &chip9->adu, TYPE_PNV_ADU);
     object_initialize_child(obj, "xive", &chip9->xive, TYPE_PNV_XIVE);
     object_property_add_alias(obj, "xive-fabric", OBJECT(&chip9->xive),
                               "xive-fabric");
@@ -1640,6 +1641,13 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* ADU */
+    if (!qdev_realize(DEVICE(&chip9->adu), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV9_XSCOM_ADU_BASE,
+                            &chip9->adu.xscom_regs);
+
     pnv_chip_quad_realize(chip9, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
@@ -1806,6 +1814,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
     PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
     int i;
 
+    object_initialize_child(obj, "adu",  &chip10->adu, TYPE_PNV_ADU);
     object_initialize_child(obj, "xive", &chip10->xive, TYPE_PNV_XIVE2);
     object_property_add_alias(obj, "xive-fabric", OBJECT(&chip10->xive),
                               "xive-fabric");
@@ -1898,6 +1907,13 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
         return;
     }
 
+    /* ADU */
+    if (!qdev_realize(DEVICE(&chip10->adu), NULL, errp)) {
+        return;
+    }
+    pnv_xscom_add_subregion(chip, PNV10_XSCOM_ADU_BASE,
+                            &chip10->adu.xscom_regs);
+
     pnv_chip_power10_quad_realize(chip10, &local_err);
     if (local_err) {
         error_propagate(errp, local_err);
diff --git a/hw/ppc/pnv_adu.c b/hw/ppc/pnv_adu.c
new file mode 100644
index 0000000000..5bd33a3841
--- /dev/null
+++ b/hw/ppc/pnv_adu.c
@@ -0,0 +1,111 @@
+/*
+ * QEMU PowerPC PowerNV ADU unit
+ *
+ * The ADU unit actually implements XSCOM, which is the bridge between MMIO
+ * and PIB. However it also includes control and status registers and other
+ * functions that are exposed as PIB (xscom) registers.
+ *
+ * To keep things simple, pnv_xscom.c remains the XSCOM bridge
+ * implementation, and pnv_adu.c implements the ADU registers and other
+ * functions.
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * SPDX-License-Identifier: LGPL-2.1-or-later
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+
+#include "hw/qdev-properties.h"
+#include "hw/ppc/pnv.h"
+#include "hw/ppc/pnv_adu.h"
+#include "hw/ppc/pnv_chip.h"
+#include "hw/ppc/pnv_xscom.h"
+#include "trace.h"
+
+static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
+{
+    PnvADU *adu = PNV_ADU(opaque);
+    uint32_t offset = addr >> 3;
+    uint64_t val = 0;
+
+    switch (offset) {
+    case 0x18:     /* Receive status reg */
+    case 0x12:     /* log register */
+    case 0x13:     /* error register */
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP, "ADU Unimplemented read register: Ox%08x\n",
+                                                                     offset);
+    }
+
+    trace_pnv_adu_xscom_read(addr, val);
+
+    return val;
+}
+
+static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
+                                unsigned width)
+{
+    PnvADU *adu = PNV_ADU(opaque);
+    uint32_t offset = addr >> 3;
+
+    trace_pnv_adu_xscom_write(addr, val);
+
+    switch (offset) {
+    case 0x18:     /* Receive status reg */
+    case 0x12:     /* log register */
+    case 0x13:     /* error register */
+        break;
+
+    default:
+        qemu_log_mask(LOG_UNIMP, "ADU Unimplemented write register: Ox%08x\n",
+                                                                     offset);
+    }
+}
+
+const MemoryRegionOps pnv_adu_xscom_ops = {
+    .read = pnv_adu_xscom_read,
+    .write = pnv_adu_xscom_write,
+    .valid.min_access_size = 8,
+    .valid.max_access_size = 8,
+    .impl.min_access_size = 8,
+    .impl.max_access_size = 8,
+    .endianness = DEVICE_BIG_ENDIAN,
+};
+
+static void pnv_adu_realize(DeviceState *dev, Error **errp)
+{
+    PnvADU *adu = PNV_ADU(dev);
+
+    /* XScom regions for ADU registers */
+    pnv_xscom_region_init(&adu->xscom_regs, OBJECT(dev),
+                          &pnv_adu_xscom_ops, adu, "xscom-adu",
+                          PNV9_XSCOM_ADU_SIZE);
+}
+
+static void pnv_adu_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->realize = pnv_adu_realize;
+    dc->desc = "PowerNV ADU";
+    dc->user_creatable = false;
+}
+
+static const TypeInfo pnv_adu_type_info = {
+    .name          = TYPE_PNV_ADU,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(PnvADU),
+    .class_init    = pnv_adu_class_init,
+    .class_size    = sizeof(PnvADUClass),
+};
+
+static void pnv_adu_register_types(void)
+{
+    type_register_static(&pnv_adu_type_info);
+}
+
+type_init(pnv_adu_register_types);
diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
index a17816d072..d192bbe2c2 100644
--- a/hw/ppc/pnv_xscom.c
+++ b/hw/ppc/pnv_xscom.c
@@ -75,11 +75,6 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
     case PRD_P9_IPOLL_REG_MASK:
     case PRD_P9_IPOLL_REG_STATUS:
 
-        /* P9 xscom reset */
-    case 0x0090018:     /* Receive status reg */
-    case 0x0090012:     /* log register */
-    case 0x0090013:     /* error register */
-
         /* P8 xscom reset */
     case 0x2020007:     /* ADU stuff, log register */
     case 0x2020009:     /* ADU stuff, error register */
@@ -119,10 +114,6 @@ static bool xscom_write_default(PnvChip *chip, uint32_t pcba, uint64_t val)
     case 0x1010c03:     /* PIBAM FIR MASK */
     case 0x1010c04:     /* PIBAM FIR MASK */
     case 0x1010c05:     /* PIBAM FIR MASK */
-        /* P9 xscom reset */
-    case 0x0090018:     /* Receive status reg */
-    case 0x0090012:     /* log register */
-    case 0x0090013:     /* error register */
 
         /* P8 xscom reset */
     case 0x2020007:     /* ADU stuff, log register */
diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
index d096636ee7..932ade7b21 100644
--- a/hw/ppc/meson.build
+++ b/hw/ppc/meson.build
@@ -44,6 +44,7 @@ endif
 ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
   'pnv.c',
   'pnv_xscom.c',
+  'pnv_adu.c',
   'pnv_core.c',
   'pnv_i2c.c',
   'pnv_lpc.c',
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index bf29bbfd4b..1f125ce841 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -95,6 +95,10 @@ vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
 vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
 vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
 
+# pnv_adu.c
+pnv_adu_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
+pnv_adu_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
+
 # pnv_chiptod.c
 pnv_chiptod_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
 pnv_chiptod_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space
  2024-04-17 11:02 [PATCH 0/2] ppc/pnv: ADU model for POWER9/10 Nicholas Piggin
  2024-04-17 11:02 ` [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC " Nicholas Piggin
@ 2024-04-17 11:02 ` Nicholas Piggin
  2024-04-17 12:25   ` Cédric Le Goater
  1 sibling, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-04-17 11:02 UTC (permalink / raw)
  To: qemu-ppc
  Cc: Nicholas Piggin, qemu-devel, Cédric Le Goater,
	Frédéric Barrat, Saif Abrar

One of the functions of the ADU is indirect memory access engines that
send and receive data via ADU registers.

This implements the ADU LPC memory access functionality sufficiently
for IBM proprietary firmware to access the UART and print characters
to the serial port as it does on real hardware.

This requires a linkage between adu and lpc, which allows adu to
perform memory access in the lpc space.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 include/hw/ppc/pnv_adu.h |  7 ++++
 include/hw/ppc/pnv_lpc.h |  5 +++
 hw/ppc/pnv.c             |  4 ++
 hw/ppc/pnv_adu.c         | 91 ++++++++++++++++++++++++++++++++++++++++
 hw/ppc/pnv_lpc.c         | 12 +++---
 5 files changed, 113 insertions(+), 6 deletions(-)

diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
index 9dc91857a9..b7b5d1bb21 100644
--- a/include/hw/ppc/pnv_adu.h
+++ b/include/hw/ppc/pnv_adu.h
@@ -10,6 +10,7 @@
 #define PPC_PNV_ADU_H
 
 #include "hw/ppc/pnv.h"
+#include "hw/ppc/pnv_lpc.h"
 #include "hw/qdev-core.h"
 
 #define TYPE_PNV_ADU "pnv-adu"
@@ -19,6 +20,12 @@ OBJECT_DECLARE_TYPE(PnvADU, PnvADUClass, PNV_ADU)
 struct PnvADU {
     DeviceState xd;
 
+    /* LPCMC (LPC Master Controller) access engine */
+    PnvLpcController *lpc;
+    uint64_t     lpc_base_reg;
+    uint64_t     lpc_cmd_reg;
+    uint64_t     lpc_data_reg;
+
     MemoryRegion xscom_regs;
 };
 
diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
index 5d22c45570..016e2998a8 100644
--- a/include/hw/ppc/pnv_lpc.h
+++ b/include/hw/ppc/pnv_lpc.h
@@ -94,6 +94,11 @@ struct PnvLpcClass {
     DeviceRealize parent_realize;
 };
 
+bool pnv_opb_lpc_read(PnvLpcController *lpc, uint32_t addr,
+                      uint8_t *data, int sz);
+bool pnv_opb_lpc_write(PnvLpcController *lpc, uint32_t addr,
+                       uint8_t *data, int sz);
+
 ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error **errp);
 int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset,
                uint64_t lpcm_addr, uint64_t lpcm_size);
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
index 5869aac89a..eb9dbc62dd 100644
--- a/hw/ppc/pnv.c
+++ b/hw/ppc/pnv.c
@@ -1642,6 +1642,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
     }
 
     /* ADU */
+    object_property_set_link(OBJECT(&chip9->adu), "lpc", OBJECT(&chip9->lpc),
+                             &error_abort);
     if (!qdev_realize(DEVICE(&chip9->adu), NULL, errp)) {
         return;
     }
@@ -1908,6 +1910,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
     }
 
     /* ADU */
+    object_property_set_link(OBJECT(&chip10->adu), "lpc", OBJECT(&chip10->lpc),
+                             &error_abort);
     if (!qdev_realize(DEVICE(&chip10->adu), NULL, errp)) {
         return;
     }
diff --git a/hw/ppc/pnv_adu.c b/hw/ppc/pnv_adu.c
index 5bd33a3841..d5570c23e2 100644
--- a/hw/ppc/pnv_adu.c
+++ b/hw/ppc/pnv_adu.c
@@ -21,9 +21,15 @@
 #include "hw/ppc/pnv.h"
 #include "hw/ppc/pnv_adu.h"
 #include "hw/ppc/pnv_chip.h"
+#include "hw/ppc/pnv_lpc.h"
 #include "hw/ppc/pnv_xscom.h"
 #include "trace.h"
 
+#define ADU_LPC_BASE_REG     0x40
+#define ADU_LPC_CMD_REG      0x41
+#define ADU_LPC_DATA_REG     0x42
+#define ADU_LPC_STATUS_REG   0x43
+
 static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
 {
     PnvADU *adu = PNV_ADU(opaque);
@@ -35,6 +41,24 @@ static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
     case 0x12:     /* log register */
     case 0x13:     /* error register */
         break;
+    case ADU_LPC_BASE_REG:
+        /*
+         * LPC Address Map in Pervasive ADU Workbook
+         *
+         * return PNV10_LPCM_BASE(chip) & PPC_BITMASK(8, 31);
+         * XXX: implement as class property, or get from LPC?
+         */
+        qemu_log_mask(LOG_UNIMP, "ADU: LPC_BASE_REG is not implemented\n");
+        break;
+    case ADU_LPC_CMD_REG:
+        val = adu->lpc_cmd_reg;
+        break;
+    case ADU_LPC_DATA_REG:
+        val = adu->lpc_data_reg;
+        break;
+    case ADU_LPC_STATUS_REG:
+        val = PPC_BIT(0); /* ack / done */
+        break;
 
     default:
         qemu_log_mask(LOG_UNIMP, "ADU Unimplemented read register: Ox%08x\n",
@@ -46,6 +70,26 @@ static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
     return val;
 }
 
+static bool lpc_cmd_read(PnvADU *adu)
+{
+    return !!(adu->lpc_cmd_reg & PPC_BIT(0));
+}
+
+static bool lpc_cmd_write(PnvADU *adu)
+{
+    return !lpc_cmd_read(adu);
+}
+
+static uint32_t lpc_cmd_addr(PnvADU *adu)
+{
+    return (adu->lpc_cmd_reg & PPC_BITMASK(32, 63)) >> PPC_BIT_NR(63);
+}
+
+static uint32_t lpc_cmd_size(PnvADU *adu)
+{
+    return (adu->lpc_cmd_reg & PPC_BITMASK(5, 11)) >> PPC_BIT_NR(11);
+}
+
 static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
                                 unsigned width)
 {
@@ -60,6 +104,47 @@ static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
     case 0x13:     /* error register */
         break;
 
+    case ADU_LPC_BASE_REG:
+        qemu_log_mask(LOG_UNIMP,
+                      "ADU: Changing LPC_BASE_REG is not implemented\n");
+        break;
+
+    case ADU_LPC_CMD_REG:
+        adu->lpc_cmd_reg = val;
+        if (lpc_cmd_read(adu)) {
+            uint32_t lpc_addr = lpc_cmd_addr(adu);
+            uint32_t lpc_size = lpc_cmd_size(adu);
+            uint64_t data = 0;
+
+            pnv_opb_lpc_read(adu->lpc, lpc_addr, (void *)&data, lpc_size);
+
+            /*
+             * ADU access is performed within 8-byte aligned sectors. Smaller
+             * access sizes don't get formatted to the least significant byte,
+             * but rather appear in the data reg at the same offset as the
+             * address in memory. This shifts them into that position.
+             */
+            adu->lpc_data_reg = be64_to_cpu(data) >> ((lpc_addr & 7) * 8);
+        }
+        break;
+
+    case ADU_LPC_DATA_REG:
+        adu->lpc_data_reg = val;
+        if (lpc_cmd_write(adu)) {
+            uint32_t lpc_addr = lpc_cmd_addr(adu);
+            uint32_t lpc_size = lpc_cmd_size(adu);
+            uint64_t data;
+
+            data = cpu_to_be64(val) >> ((lpc_addr & 7) * 8); /* See above */
+            pnv_opb_lpc_write(adu->lpc, lpc_addr, (void *)&data, lpc_size);
+        }
+        break;
+
+    case ADU_LPC_STATUS_REG:
+        qemu_log_mask(LOG_UNIMP,
+                      "ADU: Changing LPC_STATUS_REG is not implemented\n");
+        break;
+
     default:
         qemu_log_mask(LOG_UNIMP, "ADU Unimplemented write register: Ox%08x\n",
                                                                      offset);
@@ -86,12 +171,18 @@ static void pnv_adu_realize(DeviceState *dev, Error **errp)
                           PNV9_XSCOM_ADU_SIZE);
 }
 
+static Property pnv_adu_properties[] = {
+    DEFINE_PROP_LINK("lpc", PnvADU, lpc, TYPE_PNV_LPC, PnvLpcController *),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
 static void pnv_adu_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
     dc->realize = pnv_adu_realize;
     dc->desc = "PowerNV ADU";
+    device_class_set_props(dc, pnv_adu_properties);
     dc->user_creatable = false;
 }
 
diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
index d692858bee..743bd66fc0 100644
--- a/hw/ppc/pnv_lpc.c
+++ b/hw/ppc/pnv_lpc.c
@@ -235,16 +235,16 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset, uint64_t lpcm_addr,
  * TODO: rework to use address_space_stq() and address_space_ldq()
  * instead.
  */
-static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
-                     int sz)
+bool pnv_opb_lpc_read(PnvLpcController *lpc, uint32_t addr,
+                      uint8_t *data, int sz)
 {
     /* XXX Handle access size limits and FW read caching here */
     return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
                                data, sz);
 }
 
-static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
-                      int sz)
+bool pnv_opb_lpc_write(PnvLpcController *lpc, uint32_t addr,
+                       uint8_t *data, int sz)
 {
     /* XXX Handle access size limits here */
     return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
@@ -276,7 +276,7 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, uint64_t cmd)
     }
 
     if (cmd & ECCB_CTL_READ) {
-        success = opb_read(lpc, opb_addr, data, sz);
+        success = pnv_opb_lpc_read(lpc, opb_addr, data, sz);
         if (success) {
             lpc->eccb_stat_reg = ECCB_STAT_OP_DONE |
                     (((uint64_t)data[0]) << 24 |
@@ -293,7 +293,7 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, uint64_t cmd)
         data[2] = lpc->eccb_data_reg >>  8;
         data[3] = lpc->eccb_data_reg;
 
-        success = opb_write(lpc, opb_addr, data, sz);
+        success = pnv_opb_lpc_write(lpc, opb_addr, data, sz);
         lpc->eccb_stat_reg = ECCB_STAT_OP_DONE;
     }
     /* XXX Which error bit (if any) to signal OPB error ? */
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-04-17 11:02 ` [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC " Nicholas Piggin
@ 2024-04-17 11:25   ` Cédric Le Goater
  2024-05-01 12:39     ` Nicholas Piggin
  0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2024-04-17 11:25 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

Hello Nick,

On 4/17/24 13:02, Nicholas Piggin wrote:
> This implements a framework for an ADU unit model.
> 
> The ADU unit actually implements XSCOM, which is the bridge between MMIO
> and PIB. However it also includes control and status registers and other
> functions that are exposed as PIB (xscom) registers.
> 
> To keep things simple, pnv_xscom.c remains the XSCOM bridge
> implementation, and pnv_adu.c implements the ADU registers and other
> functions.
> 
> So far, just the ADU no-op registers in the pnv_xscom.c default handler
> are moved over to the adu model.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
>   include/hw/ppc/pnv_chip.h  |   3 +
>   include/hw/ppc/pnv_xscom.h |   6 ++
>   hw/ppc/pnv.c               |  16 ++++++
>   hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
>   hw/ppc/pnv_xscom.c         |   9 ---
>   hw/ppc/meson.build         |   1 +
>   hw/ppc/trace-events        |   4 ++
>   8 files changed, 175 insertions(+), 9 deletions(-)
>   create mode 100644 include/hw/ppc/pnv_adu.h
>   create mode 100644 hw/ppc/pnv_adu.c
> 
> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> new file mode 100644
> index 0000000000..9dc91857a9
> --- /dev/null
> +++ b/include/hw/ppc/pnv_adu.h
> @@ -0,0 +1,34 @@
> +/*
> + * QEMU PowerPC PowerNV Emulation of some ADU behaviour
> + *
> + * Copyright (c) 2024, IBM Corporation.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later


Did you mean GPL-2.0-or-later ?

The rest looks good.

Thanks,

C.




> + */
> +
> +#ifndef PPC_PNV_ADU_H
> +#define PPC_PNV_ADU_H
> +
> +#include "hw/ppc/pnv.h"
> +#include "hw/qdev-core.h"
> +
> +#define TYPE_PNV_ADU "pnv-adu"
> +
> +OBJECT_DECLARE_TYPE(PnvADU, PnvADUClass, PNV_ADU)
> +
> +struct PnvADU {
> +    DeviceState xd;
> +
> +    MemoryRegion xscom_regs;
> +};
> +
> +struct PnvADUClass {
> +    DeviceClass parent_class;
> +
> +    int xscom_ctrl_size;
> +    int xscom_mbox_size;
> +    const MemoryRegionOps *xscom_ctrl_ops;
> +    const MemoryRegionOps *xscom_mbox_ops;
> +};
> +
> +#endif /* PPC_PNV_ADU_H */
> diff --git a/include/hw/ppc/pnv_chip.h b/include/hw/ppc/pnv_chip.h
> index 8589f3291e..96e50a2983 100644
> --- a/include/hw/ppc/pnv_chip.h
> +++ b/include/hw/ppc/pnv_chip.h
> @@ -2,6 +2,7 @@
>   #define PPC_PNV_CHIP_H
>   
>   #include "hw/pci-host/pnv_phb4.h"
> +#include "hw/ppc/pnv_adu.h"
>   #include "hw/ppc/pnv_chiptod.h"
>   #include "hw/ppc/pnv_core.h"
>   #include "hw/ppc/pnv_homer.h"
> @@ -77,6 +78,7 @@ struct Pnv9Chip {
>       PnvChip      parent_obj;
>   
>       /*< public >*/
> +    PnvADU       adu;
>       PnvXive      xive;
>       Pnv9Psi      psi;
>       PnvLpcController lpc;
> @@ -110,6 +112,7 @@ struct Pnv10Chip {
>       PnvChip      parent_obj;
>   
>       /*< public >*/
> +    PnvADU       adu;
>       PnvXive2     xive;
>       Pnv9Psi      psi;
>       PnvLpcController lpc;
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index 6209e18492..e93d310e79 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -82,6 +82,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV_XSCOM_PBCQ_SPCI_BASE  0x9013c00
>   #define PNV_XSCOM_PBCQ_SPCI_SIZE  0x5
>   
> +#define PNV9_XSCOM_ADU_BASE       0x0090000
> +#define PNV9_XSCOM_ADU_SIZE       0x55
> +
>   /*
>    * Layout of the XSCOM PCB addresses (POWER 9)
>    */
> @@ -128,6 +131,9 @@ struct PnvXScomInterfaceClass {
>   #define PNV9_XSCOM_PEC_PCI_STK1   0x140
>   #define PNV9_XSCOM_PEC_PCI_STK2   0x180
>   
> +#define PNV10_XSCOM_ADU_BASE      0x0090000
> +#define PNV10_XSCOM_ADU_SIZE      0x55
> +
>   /*
>    * Layout of the XSCOM PCB addresses (POWER 10)
>    */
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 6e3a5ccdec..5869aac89a 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1530,6 +1530,7 @@ static void pnv_chip_power9_instance_init(Object *obj)
>       PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
>       int i;
>   
> +    object_initialize_child(obj, "adu",  &chip9->adu, TYPE_PNV_ADU);
>       object_initialize_child(obj, "xive", &chip9->xive, TYPE_PNV_XIVE);
>       object_property_add_alias(obj, "xive-fabric", OBJECT(&chip9->xive),
>                                 "xive-fabric");
> @@ -1640,6 +1641,13 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> +    /* ADU */
> +    if (!qdev_realize(DEVICE(&chip9->adu), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV9_XSCOM_ADU_BASE,
> +                            &chip9->adu.xscom_regs);
> +
>       pnv_chip_quad_realize(chip9, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
> @@ -1806,6 +1814,7 @@ static void pnv_chip_power10_instance_init(Object *obj)
>       PnvChipClass *pcc = PNV_CHIP_GET_CLASS(obj);
>       int i;
>   
> +    object_initialize_child(obj, "adu",  &chip10->adu, TYPE_PNV_ADU);
>       object_initialize_child(obj, "xive", &chip10->xive, TYPE_PNV_XIVE2);
>       object_property_add_alias(obj, "xive-fabric", OBJECT(&chip10->xive),
>                                 "xive-fabric");
> @@ -1898,6 +1907,13 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> +    /* ADU */
> +    if (!qdev_realize(DEVICE(&chip10->adu), NULL, errp)) {
> +        return;
> +    }
> +    pnv_xscom_add_subregion(chip, PNV10_XSCOM_ADU_BASE,
> +                            &chip10->adu.xscom_regs);
> +
>       pnv_chip_power10_quad_realize(chip10, &local_err);
>       if (local_err) {
>           error_propagate(errp, local_err);
> diff --git a/hw/ppc/pnv_adu.c b/hw/ppc/pnv_adu.c
> new file mode 100644
> index 0000000000..5bd33a3841
> --- /dev/null
> +++ b/hw/ppc/pnv_adu.c
> @@ -0,0 +1,111 @@
> +/*
> + * QEMU PowerPC PowerNV ADU unit
> + *
> + * The ADU unit actually implements XSCOM, which is the bridge between MMIO
> + * and PIB. However it also includes control and status registers and other
> + * functions that are exposed as PIB (xscom) registers.
> + *
> + * To keep things simple, pnv_xscom.c remains the XSCOM bridge
> + * implementation, and pnv_adu.c implements the ADU registers and other
> + * functions.
> + *
> + * Copyright (c) 2024, IBM Corporation.
> + *
> + * SPDX-License-Identifier: LGPL-2.1-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/log.h"
> +
> +#include "hw/qdev-properties.h"
> +#include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_adu.h"
> +#include "hw/ppc/pnv_chip.h"
> +#include "hw/ppc/pnv_xscom.h"
> +#include "trace.h"
> +
> +static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
> +{
> +    PnvADU *adu = PNV_ADU(opaque);
> +    uint32_t offset = addr >> 3;
> +    uint64_t val = 0;
> +
> +    switch (offset) {
> +    case 0x18:     /* Receive status reg */
> +    case 0x12:     /* log register */
> +    case 0x13:     /* error register */
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "ADU Unimplemented read register: Ox%08x\n",
> +                                                                     offset);
> +    }
> +
> +    trace_pnv_adu_xscom_read(addr, val);
> +
> +    return val;
> +}
> +
> +static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
> +                                unsigned width)
> +{
> +    PnvADU *adu = PNV_ADU(opaque);
> +    uint32_t offset = addr >> 3;
> +
> +    trace_pnv_adu_xscom_write(addr, val);
> +
> +    switch (offset) {
> +    case 0x18:     /* Receive status reg */
> +    case 0x12:     /* log register */
> +    case 0x13:     /* error register */
> +        break;
> +
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "ADU Unimplemented write register: Ox%08x\n",
> +                                                                     offset);
> +    }
> +}
> +
> +const MemoryRegionOps pnv_adu_xscom_ops = {
> +    .read = pnv_adu_xscom_read,
> +    .write = pnv_adu_xscom_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
> +static void pnv_adu_realize(DeviceState *dev, Error **errp)
> +{
> +    PnvADU *adu = PNV_ADU(dev);
> +
> +    /* XScom regions for ADU registers */
> +    pnv_xscom_region_init(&adu->xscom_regs, OBJECT(dev),
> +                          &pnv_adu_xscom_ops, adu, "xscom-adu",
> +                          PNV9_XSCOM_ADU_SIZE);
> +}
> +
> +static void pnv_adu_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->realize = pnv_adu_realize;
> +    dc->desc = "PowerNV ADU";
> +    dc->user_creatable = false;
> +}
> +
> +static const TypeInfo pnv_adu_type_info = {
> +    .name          = TYPE_PNV_ADU,
> +    .parent        = TYPE_DEVICE,
> +    .instance_size = sizeof(PnvADU),
> +    .class_init    = pnv_adu_class_init,
> +    .class_size    = sizeof(PnvADUClass),
> +};
> +
> +static void pnv_adu_register_types(void)
> +{
> +    type_register_static(&pnv_adu_type_info);
> +}
> +
> +type_init(pnv_adu_register_types);
> diff --git a/hw/ppc/pnv_xscom.c b/hw/ppc/pnv_xscom.c
> index a17816d072..d192bbe2c2 100644
> --- a/hw/ppc/pnv_xscom.c
> +++ b/hw/ppc/pnv_xscom.c
> @@ -75,11 +75,6 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba)
>       case PRD_P9_IPOLL_REG_MASK:
>       case PRD_P9_IPOLL_REG_STATUS:
>   
> -        /* P9 xscom reset */
> -    case 0x0090018:     /* Receive status reg */
> -    case 0x0090012:     /* log register */
> -    case 0x0090013:     /* error register */
> -
>           /* P8 xscom reset */
>       case 0x2020007:     /* ADU stuff, log register */
>       case 0x2020009:     /* ADU stuff, error register */
> @@ -119,10 +114,6 @@ static bool xscom_write_default(PnvChip *chip, uint32_t pcba, uint64_t val)
>       case 0x1010c03:     /* PIBAM FIR MASK */
>       case 0x1010c04:     /* PIBAM FIR MASK */
>       case 0x1010c05:     /* PIBAM FIR MASK */
> -        /* P9 xscom reset */
> -    case 0x0090018:     /* Receive status reg */
> -    case 0x0090012:     /* log register */
> -    case 0x0090013:     /* error register */
>   
>           /* P8 xscom reset */
>       case 0x2020007:     /* ADU stuff, log register */
> diff --git a/hw/ppc/meson.build b/hw/ppc/meson.build
> index d096636ee7..932ade7b21 100644
> --- a/hw/ppc/meson.build
> +++ b/hw/ppc/meson.build
> @@ -44,6 +44,7 @@ endif
>   ppc_ss.add(when: 'CONFIG_POWERNV', if_true: files(
>     'pnv.c',
>     'pnv_xscom.c',
> +  'pnv_adu.c',
>     'pnv_core.c',
>     'pnv_i2c.c',
>     'pnv_lpc.c',
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index bf29bbfd4b..1f125ce841 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -95,6 +95,10 @@ vof_write(uint32_t ih, unsigned cb, const char *msg) "ih=0x%x [%u] \"%s\""
>   vof_avail(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>   vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx64" size=0x%"PRIx64
>   
> +# pnv_adu.c
> +pnv_adu_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> +pnv_adu_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
> +
>   # pnv_chiptod.c
>   pnv_chiptod_xscom_read(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64
>   pnv_chiptod_xscom_write(uint64_t addr, uint64_t val) "addr 0x%" PRIx64 " val 0x%" PRIx64



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space
  2024-04-17 11:02 ` [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space Nicholas Piggin
@ 2024-04-17 12:25   ` Cédric Le Goater
  2024-05-01 12:43     ` Nicholas Piggin
  0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2024-04-17 12:25 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On 4/17/24 13:02, Nicholas Piggin wrote:
> One of the functions of the ADU is indirect memory access engines that
> send and receive data via ADU registers.
> 
> This implements the ADU LPC memory access functionality sufficiently
> for IBM proprietary firmware to access the UART and print characters
> to the serial port as it does on real hardware.
> 
> This requires a linkage between adu and lpc, which allows adu to
> perform memory access in the lpc space.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   include/hw/ppc/pnv_adu.h |  7 ++++
>   include/hw/ppc/pnv_lpc.h |  5 +++
>   hw/ppc/pnv.c             |  4 ++
>   hw/ppc/pnv_adu.c         | 91 ++++++++++++++++++++++++++++++++++++++++
>   hw/ppc/pnv_lpc.c         | 12 +++---
>   5 files changed, 113 insertions(+), 6 deletions(-)
> 
> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> index 9dc91857a9..b7b5d1bb21 100644
> --- a/include/hw/ppc/pnv_adu.h
> +++ b/include/hw/ppc/pnv_adu.h
> @@ -10,6 +10,7 @@
>   #define PPC_PNV_ADU_H
>   
>   #include "hw/ppc/pnv.h"
> +#include "hw/ppc/pnv_lpc.h"
>   #include "hw/qdev-core.h"
>   
>   #define TYPE_PNV_ADU "pnv-adu"
> @@ -19,6 +20,12 @@ OBJECT_DECLARE_TYPE(PnvADU, PnvADUClass, PNV_ADU)
>   struct PnvADU {
>       DeviceState xd;
>   
> +    /* LPCMC (LPC Master Controller) access engine */
> +    PnvLpcController *lpc;
> +    uint64_t     lpc_base_reg;
> +    uint64_t     lpc_cmd_reg;
> +    uint64_t     lpc_data_reg;

I don't see reset values for these registers. Is that ok ?

>       MemoryRegion xscom_regs;
>   };
>   
> diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> index 5d22c45570..016e2998a8 100644
> --- a/include/hw/ppc/pnv_lpc.h
> +++ b/include/hw/ppc/pnv_lpc.h
> @@ -94,6 +94,11 @@ struct PnvLpcClass {
>       DeviceRealize parent_realize;
>   };
>   
> +bool pnv_opb_lpc_read(PnvLpcController *lpc, uint32_t addr,
> +                      uint8_t *data, int sz);
> +bool pnv_opb_lpc_write(PnvLpcController *lpc, uint32_t addr,
> +                       uint8_t *data, int sz);

May be rename to pnv_lpc_opb_read/write ?

>   ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error **errp);
>   int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset,
>                  uint64_t lpcm_addr, uint64_t lpcm_size);
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 5869aac89a..eb9dbc62dd 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -1642,6 +1642,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>       }
>   
>       /* ADU */
> +    object_property_set_link(OBJECT(&chip9->adu), "lpc", OBJECT(&chip9->lpc),
> +                             &error_abort);

I would add an assert on the lpc pointer in the ADU realize routine.

Thanks,

C.

>       if (!qdev_realize(DEVICE(&chip9->adu), NULL, errp)) {
>           return;
>       }
> @@ -1908,6 +1910,8 @@ static void pnv_chip_power10_realize(DeviceState *dev, Error **errp)
>       }
>   
>       /* ADU */
> +    object_property_set_link(OBJECT(&chip10->adu), "lpc", OBJECT(&chip10->lpc),
> +                             &error_abort);
>       if (!qdev_realize(DEVICE(&chip10->adu), NULL, errp)) {
>           return;
>       }
> diff --git a/hw/ppc/pnv_adu.c b/hw/ppc/pnv_adu.c
> index 5bd33a3841..d5570c23e2 100644
> --- a/hw/ppc/pnv_adu.c
> +++ b/hw/ppc/pnv_adu.c
> @@ -21,9 +21,15 @@
>   #include "hw/ppc/pnv.h"
>   #include "hw/ppc/pnv_adu.h"
>   #include "hw/ppc/pnv_chip.h"
> +#include "hw/ppc/pnv_lpc.h"
>   #include "hw/ppc/pnv_xscom.h"
>   #include "trace.h"
>   
> +#define ADU_LPC_BASE_REG     0x40
> +#define ADU_LPC_CMD_REG      0x41
> +#define ADU_LPC_DATA_REG     0x42
> +#define ADU_LPC_STATUS_REG   0x43
> +
>   static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
>   {
>       PnvADU *adu = PNV_ADU(opaque);
> @@ -35,6 +41,24 @@ static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
>       case 0x12:     /* log register */
>       case 0x13:     /* error register */
>           break;
> +    case ADU_LPC_BASE_REG:
> +        /*
> +         * LPC Address Map in Pervasive ADU Workbook
> +         *
> +         * return PNV10_LPCM_BASE(chip) & PPC_BITMASK(8, 31);
> +         * XXX: implement as class property, or get from LPC?
> +         */
> +        qemu_log_mask(LOG_UNIMP, "ADU: LPC_BASE_REG is not implemented\n");
> +        break;
> +    case ADU_LPC_CMD_REG:
> +        val = adu->lpc_cmd_reg;
> +        break;
> +    case ADU_LPC_DATA_REG:
> +        val = adu->lpc_data_reg;
> +        break;
> +    case ADU_LPC_STATUS_REG:
> +        val = PPC_BIT(0); /* ack / done */
> +        break;
>   
>       default:
>           qemu_log_mask(LOG_UNIMP, "ADU Unimplemented read register: Ox%08x\n",
> @@ -46,6 +70,26 @@ static uint64_t pnv_adu_xscom_read(void *opaque, hwaddr addr, unsigned width)
>       return val;
>   }
>   
> +static bool lpc_cmd_read(PnvADU *adu)
> +{
> +    return !!(adu->lpc_cmd_reg & PPC_BIT(0));
> +}
> +
> +static bool lpc_cmd_write(PnvADU *adu)
> +{
> +    return !lpc_cmd_read(adu);
> +}
> +
> +static uint32_t lpc_cmd_addr(PnvADU *adu)
> +{
> +    return (adu->lpc_cmd_reg & PPC_BITMASK(32, 63)) >> PPC_BIT_NR(63);
> +}
> +
> +static uint32_t lpc_cmd_size(PnvADU *adu)
> +{
> +    return (adu->lpc_cmd_reg & PPC_BITMASK(5, 11)) >> PPC_BIT_NR(11);
> +}
> +
>   static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
>                                   unsigned width)
>   {
> @@ -60,6 +104,47 @@ static void pnv_adu_xscom_write(void *opaque, hwaddr addr, uint64_t val,
>       case 0x13:     /* error register */
>           break;
>   
> +    case ADU_LPC_BASE_REG:
> +        qemu_log_mask(LOG_UNIMP,
> +                      "ADU: Changing LPC_BASE_REG is not implemented\n");
> +        break;
> +
> +    case ADU_LPC_CMD_REG:
> +        adu->lpc_cmd_reg = val;
> +        if (lpc_cmd_read(adu)) {
> +            uint32_t lpc_addr = lpc_cmd_addr(adu);
> +            uint32_t lpc_size = lpc_cmd_size(adu);
> +            uint64_t data = 0;
> +
> +            pnv_opb_lpc_read(adu->lpc, lpc_addr, (void *)&data, lpc_size);
> +
> +            /*
> +             * ADU access is performed within 8-byte aligned sectors. Smaller
> +             * access sizes don't get formatted to the least significant byte,
> +             * but rather appear in the data reg at the same offset as the
> +             * address in memory. This shifts them into that position.
> +             */
> +            adu->lpc_data_reg = be64_to_cpu(data) >> ((lpc_addr & 7) * 8);
> +        }
> +        break;
> +
> +    case ADU_LPC_DATA_REG:
> +        adu->lpc_data_reg = val;
> +        if (lpc_cmd_write(adu)) {
> +            uint32_t lpc_addr = lpc_cmd_addr(adu);
> +            uint32_t lpc_size = lpc_cmd_size(adu);
> +            uint64_t data;
> +
> +            data = cpu_to_be64(val) >> ((lpc_addr & 7) * 8); /* See above */
> +            pnv_opb_lpc_write(adu->lpc, lpc_addr, (void *)&data, lpc_size);
> +        }
> +        break;
> +
> +    case ADU_LPC_STATUS_REG:
> +        qemu_log_mask(LOG_UNIMP,
> +                      "ADU: Changing LPC_STATUS_REG is not implemented\n");
> +        break;
> +
>       default:
>           qemu_log_mask(LOG_UNIMP, "ADU Unimplemented write register: Ox%08x\n",
>                                                                        offset);
> @@ -86,12 +171,18 @@ static void pnv_adu_realize(DeviceState *dev, Error **errp)
>                             PNV9_XSCOM_ADU_SIZE);
>   }
>   
> +static Property pnv_adu_properties[] = {
> +    DEFINE_PROP_LINK("lpc", PnvADU, lpc, TYPE_PNV_LPC, PnvLpcController *),
> +    DEFINE_PROP_END_OF_LIST(),
> +};
> +
>   static void pnv_adu_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
>       dc->realize = pnv_adu_realize;
>       dc->desc = "PowerNV ADU";
> +    device_class_set_props(dc, pnv_adu_properties);
>       dc->user_creatable = false;
>   }
>   
> diff --git a/hw/ppc/pnv_lpc.c b/hw/ppc/pnv_lpc.c
> index d692858bee..743bd66fc0 100644
> --- a/hw/ppc/pnv_lpc.c
> +++ b/hw/ppc/pnv_lpc.c
> @@ -235,16 +235,16 @@ int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset, uint64_t lpcm_addr,
>    * TODO: rework to use address_space_stq() and address_space_ldq()
>    * instead.
>    */
> -static bool opb_read(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
> -                     int sz)
> +bool pnv_opb_lpc_read(PnvLpcController *lpc, uint32_t addr,
> +                      uint8_t *data, int sz)
>   {
>       /* XXX Handle access size limits and FW read caching here */
>       return !address_space_read(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
>                                  data, sz);
>   }
>   
> -static bool opb_write(PnvLpcController *lpc, uint32_t addr, uint8_t *data,
> -                      int sz)
> +bool pnv_opb_lpc_write(PnvLpcController *lpc, uint32_t addr,
> +                       uint8_t *data, int sz)
>   {
>       /* XXX Handle access size limits here */
>       return !address_space_write(&lpc->opb_as, addr, MEMTXATTRS_UNSPECIFIED,
> @@ -276,7 +276,7 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, uint64_t cmd)
>       }
>   
>       if (cmd & ECCB_CTL_READ) {
> -        success = opb_read(lpc, opb_addr, data, sz);
> +        success = pnv_opb_lpc_read(lpc, opb_addr, data, sz);
>           if (success) {
>               lpc->eccb_stat_reg = ECCB_STAT_OP_DONE |
>                       (((uint64_t)data[0]) << 24 |
> @@ -293,7 +293,7 @@ static void pnv_lpc_do_eccb(PnvLpcController *lpc, uint64_t cmd)
>           data[2] = lpc->eccb_data_reg >>  8;
>           data[3] = lpc->eccb_data_reg;
>   
> -        success = opb_write(lpc, opb_addr, data, sz);
> +        success = pnv_opb_lpc_write(lpc, opb_addr, data, sz);
>           lpc->eccb_stat_reg = ECCB_STAT_OP_DONE;
>       }
>       /* XXX Which error bit (if any) to signal OPB error ? */



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-04-17 11:25   ` Cédric Le Goater
@ 2024-05-01 12:39     ` Nicholas Piggin
  2024-05-02  8:47       ` Cédric Le Goater
  0 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-05-01 12:39 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On Wed Apr 17, 2024 at 9:25 PM AEST, Cédric Le Goater wrote:
> Hello Nick,
>
> On 4/17/24 13:02, Nicholas Piggin wrote:
> > This implements a framework for an ADU unit model.
> > 
> > The ADU unit actually implements XSCOM, which is the bridge between MMIO
> > and PIB. However it also includes control and status registers and other
> > functions that are exposed as PIB (xscom) registers.
> > 
> > To keep things simple, pnv_xscom.c remains the XSCOM bridge
> > implementation, and pnv_adu.c implements the ADU registers and other
> > functions.
> > 
> > So far, just the ADU no-op registers in the pnv_xscom.c default handler
> > are moved over to the adu model.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
> >   include/hw/ppc/pnv_chip.h  |   3 +
> >   include/hw/ppc/pnv_xscom.h |   6 ++
> >   hw/ppc/pnv.c               |  16 ++++++
> >   hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
> >   hw/ppc/pnv_xscom.c         |   9 ---
> >   hw/ppc/meson.build         |   1 +
> >   hw/ppc/trace-events        |   4 ++
> >   8 files changed, 175 insertions(+), 9 deletions(-)
> >   create mode 100644 include/hw/ppc/pnv_adu.h
> >   create mode 100644 hw/ppc/pnv_adu.c
> > 
> > diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> > new file mode 100644
> > index 0000000000..9dc91857a9
> > --- /dev/null
> > +++ b/include/hw/ppc/pnv_adu.h
> > @@ -0,0 +1,34 @@
> > +/*
> > + * QEMU PowerPC PowerNV Emulation of some ADU behaviour
> > + *
> > + * Copyright (c) 2024, IBM Corporation.
> > + *
> > + * SPDX-License-Identifier: LGPL-2.1-or-later
>
>
> Did you mean GPL-2.0-or-later ?

Hey Cedric,

Thanks for reviewing, I've been away so sorry for the late reply.

It just came from one of the headers I copied which was LGPL. But
there's really nothing much in it and could find a GPL header to
copy. Is GPL-2.0-or-later preferred?

Thanks,
Nick


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space
  2024-04-17 12:25   ` Cédric Le Goater
@ 2024-05-01 12:43     ` Nicholas Piggin
  2024-05-02  8:32       ` Cédric Le Goater
  0 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-05-01 12:43 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On Wed Apr 17, 2024 at 10:25 PM AEST, Cédric Le Goater wrote:
> On 4/17/24 13:02, Nicholas Piggin wrote:
> > One of the functions of the ADU is indirect memory access engines that
> > send and receive data via ADU registers.
> > 
> > This implements the ADU LPC memory access functionality sufficiently
> > for IBM proprietary firmware to access the UART and print characters
> > to the serial port as it does on real hardware.
> > 
> > This requires a linkage between adu and lpc, which allows adu to
> > perform memory access in the lpc space.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >   include/hw/ppc/pnv_adu.h |  7 ++++
> >   include/hw/ppc/pnv_lpc.h |  5 +++
> >   hw/ppc/pnv.c             |  4 ++
> >   hw/ppc/pnv_adu.c         | 91 ++++++++++++++++++++++++++++++++++++++++
> >   hw/ppc/pnv_lpc.c         | 12 +++---
> >   5 files changed, 113 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> > index 9dc91857a9..b7b5d1bb21 100644
> > --- a/include/hw/ppc/pnv_adu.h
> > +++ b/include/hw/ppc/pnv_adu.h
> > @@ -10,6 +10,7 @@
> >   #define PPC_PNV_ADU_H
> >   
> >   #include "hw/ppc/pnv.h"
> > +#include "hw/ppc/pnv_lpc.h"
> >   #include "hw/qdev-core.h"
> >   
> >   #define TYPE_PNV_ADU "pnv-adu"
> > @@ -19,6 +20,12 @@ OBJECT_DECLARE_TYPE(PnvADU, PnvADUClass, PNV_ADU)
> >   struct PnvADU {
> >       DeviceState xd;
> >   
> > +    /* LPCMC (LPC Master Controller) access engine */
> > +    PnvLpcController *lpc;
> > +    uint64_t     lpc_base_reg;
> > +    uint64_t     lpc_cmd_reg;
> > +    uint64_t     lpc_data_reg;
>
> I don't see reset values for these registers. Is that ok ?
>
> >       MemoryRegion xscom_regs;
> >   };
> >   
> > diff --git a/include/hw/ppc/pnv_lpc.h b/include/hw/ppc/pnv_lpc.h
> > index 5d22c45570..016e2998a8 100644
> > --- a/include/hw/ppc/pnv_lpc.h
> > +++ b/include/hw/ppc/pnv_lpc.h
> > @@ -94,6 +94,11 @@ struct PnvLpcClass {
> >       DeviceRealize parent_realize;
> >   };
> >   
> > +bool pnv_opb_lpc_read(PnvLpcController *lpc, uint32_t addr,
> > +                      uint8_t *data, int sz);
> > +bool pnv_opb_lpc_write(PnvLpcController *lpc, uint32_t addr,
> > +                       uint8_t *data, int sz);
>
> May be rename to pnv_lpc_opb_read/write ?

Yes that's more appropriate.

> >   ISABus *pnv_lpc_isa_create(PnvLpcController *lpc, bool use_cpld, Error **errp);
> >   int pnv_dt_lpc(PnvChip *chip, void *fdt, int root_offset,
> >                  uint64_t lpcm_addr, uint64_t lpcm_size);
> > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> > index 5869aac89a..eb9dbc62dd 100644
> > --- a/hw/ppc/pnv.c
> > +++ b/hw/ppc/pnv.c
> > @@ -1642,6 +1642,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >       }
> >   
> >       /* ADU */
> > +    object_property_set_link(OBJECT(&chip9->adu), "lpc", OBJECT(&chip9->lpc),
> > +                             &error_abort);
>
> I would add an assert on the lpc pointer in the ADU realize routine.

A assert != NULL, in case this failed to link correctly? (Maybe if it
was called before lpc object was realized). I will do.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space
  2024-05-01 12:43     ` Nicholas Piggin
@ 2024-05-02  8:32       ` Cédric Le Goater
  2024-05-03  4:47         ` Nicholas Piggin
  0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2024-05-02  8:32 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

Hello Nick,


>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
>>> index 5869aac89a..eb9dbc62dd 100644
>>> --- a/hw/ppc/pnv.c
>>> +++ b/hw/ppc/pnv.c
>>> @@ -1642,6 +1642,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
>>>        }
>>>    
>>>        /* ADU */
>>> +    object_property_set_link(OBJECT(&chip9->adu), "lpc", OBJECT(&chip9->lpc),
>>> +                             &error_abort);
>>
>> I would add an assert on the lpc pointer in the ADU realize routine.
> 
> A assert != NULL, in case this failed to link correctly? (Maybe if it
> was called before lpc object was realized). I will do.

It is to make sure that an ADU object is not "realized" without
the pointer '->lpc' being set before, since it is a must-have for
the implementation to operate (and do LPC transactions).

There are several :

    assert(s->chip);

in the pnv models for the same kind of purpose.

Thanks,

C.





^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-05-01 12:39     ` Nicholas Piggin
@ 2024-05-02  8:47       ` Cédric Le Goater
  2024-05-03  4:51         ` Nicholas Piggin
  0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2024-05-02  8:47 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On 5/1/24 14:39, Nicholas Piggin wrote:
> On Wed Apr 17, 2024 at 9:25 PM AEST, Cédric Le Goater wrote:
>> Hello Nick,
>>
>> On 4/17/24 13:02, Nicholas Piggin wrote:
>>> This implements a framework for an ADU unit model.
>>>
>>> The ADU unit actually implements XSCOM, which is the bridge between MMIO
>>> and PIB. However it also includes control and status registers and other
>>> functions that are exposed as PIB (xscom) registers.
>>>
>>> To keep things simple, pnv_xscom.c remains the XSCOM bridge
>>> implementation, and pnv_adu.c implements the ADU registers and other
>>> functions.
>>>
>>> So far, just the ADU no-op registers in the pnv_xscom.c default handler
>>> are moved over to the adu model.
>>>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>>    include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
>>>    include/hw/ppc/pnv_chip.h  |   3 +
>>>    include/hw/ppc/pnv_xscom.h |   6 ++
>>>    hw/ppc/pnv.c               |  16 ++++++
>>>    hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
>>>    hw/ppc/pnv_xscom.c         |   9 ---
>>>    hw/ppc/meson.build         |   1 +
>>>    hw/ppc/trace-events        |   4 ++
>>>    8 files changed, 175 insertions(+), 9 deletions(-)
>>>    create mode 100644 include/hw/ppc/pnv_adu.h
>>>    create mode 100644 hw/ppc/pnv_adu.c
>>>
>>> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
>>> new file mode 100644
>>> index 0000000000..9dc91857a9
>>> --- /dev/null
>>> +++ b/include/hw/ppc/pnv_adu.h
>>> @@ -0,0 +1,34 @@
>>> +/*
>>> + * QEMU PowerPC PowerNV Emulation of some ADU behaviour
>>> + *
>>> + * Copyright (c) 2024, IBM Corporation.
>>> + *
>>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>>
>>
>> Did you mean GPL-2.0-or-later ?
> 
> Hey Cedric,
> 
> Thanks for reviewing, I've been away so sorry for the late reply.
> 
> It just came from one of the headers I copied which was LGPL. But
> there's really nothing much in it and could find a GPL header to
> copy. Is GPL-2.0-or-later preferred?

I would since all pnv models are GPL.

I think some parts of QEMU were initially LGPL (there used to be
a library, may be that's the reason ?) and other parts are relaxed
to LGPL because they are reused in libraries.

Thanks,

C.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space
  2024-05-02  8:32       ` Cédric Le Goater
@ 2024-05-03  4:47         ` Nicholas Piggin
  0 siblings, 0 replies; 13+ messages in thread
From: Nicholas Piggin @ 2024-05-03  4:47 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On Thu May 2, 2024 at 6:32 PM AEST, Cédric Le Goater wrote:
> Hello Nick,
>
>
> >>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >>> index 5869aac89a..eb9dbc62dd 100644
> >>> --- a/hw/ppc/pnv.c
> >>> +++ b/hw/ppc/pnv.c
> >>> @@ -1642,6 +1642,8 @@ static void pnv_chip_power9_realize(DeviceState *dev, Error **errp)
> >>>        }
> >>>    
> >>>        /* ADU */
> >>> +    object_property_set_link(OBJECT(&chip9->adu), "lpc", OBJECT(&chip9->lpc),
> >>> +                             &error_abort);
> >>
> >> I would add an assert on the lpc pointer in the ADU realize routine.
> > 
> > A assert != NULL, in case this failed to link correctly? (Maybe if it
> > was called before lpc object was realized). I will do.
>
> It is to make sure that an ADU object is not "realized" without
> the pointer '->lpc' being set before, since it is a must-have for
> the implementation to operate (and do LPC transactions).
>
> There are several :
>
>     assert(s->chip);
>
> in the pnv models for the same kind of purpose.

Makes sense.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-05-02  8:47       ` Cédric Le Goater
@ 2024-05-03  4:51         ` Nicholas Piggin
  2024-05-03  5:44           ` Cédric Le Goater
  0 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2024-05-03  4:51 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On Thu May 2, 2024 at 6:47 PM AEST, Cédric Le Goater wrote:
> On 5/1/24 14:39, Nicholas Piggin wrote:
> > On Wed Apr 17, 2024 at 9:25 PM AEST, Cédric Le Goater wrote:
> >> Hello Nick,
> >>
> >> On 4/17/24 13:02, Nicholas Piggin wrote:
> >>> This implements a framework for an ADU unit model.
> >>>
> >>> The ADU unit actually implements XSCOM, which is the bridge between MMIO
> >>> and PIB. However it also includes control and status registers and other
> >>> functions that are exposed as PIB (xscom) registers.
> >>>
> >>> To keep things simple, pnv_xscom.c remains the XSCOM bridge
> >>> implementation, and pnv_adu.c implements the ADU registers and other
> >>> functions.
> >>>
> >>> So far, just the ADU no-op registers in the pnv_xscom.c default handler
> >>> are moved over to the adu model.
> >>>
> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>> ---
> >>>    include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
> >>>    include/hw/ppc/pnv_chip.h  |   3 +
> >>>    include/hw/ppc/pnv_xscom.h |   6 ++
> >>>    hw/ppc/pnv.c               |  16 ++++++
> >>>    hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
> >>>    hw/ppc/pnv_xscom.c         |   9 ---
> >>>    hw/ppc/meson.build         |   1 +
> >>>    hw/ppc/trace-events        |   4 ++
> >>>    8 files changed, 175 insertions(+), 9 deletions(-)
> >>>    create mode 100644 include/hw/ppc/pnv_adu.h
> >>>    create mode 100644 hw/ppc/pnv_adu.c
> >>>
> >>> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> >>> new file mode 100644
> >>> index 0000000000..9dc91857a9
> >>> --- /dev/null
> >>> +++ b/include/hw/ppc/pnv_adu.h
> >>> @@ -0,0 +1,34 @@
> >>> +/*
> >>> + * QEMU PowerPC PowerNV Emulation of some ADU behaviour
> >>> + *
> >>> + * Copyright (c) 2024, IBM Corporation.
> >>> + *
> >>> + * SPDX-License-Identifier: LGPL-2.1-or-later
> >>
> >>
> >> Did you mean GPL-2.0-or-later ?
> > 
> > Hey Cedric,
> > 
> > Thanks for reviewing, I've been away so sorry for the late reply.
> > 
> > It just came from one of the headers I copied which was LGPL. But
> > there's really nothing much in it and could find a GPL header to
> > copy. Is GPL-2.0-or-later preferred?
>
> I would since all pnv models are GPL.

Some of pnv is actually LGPL. That's okay I'll change to GPL.

> I think some parts of QEMU were initially LGPL (there used to be
> a library, may be that's the reason ?) and other parts are relaxed
> to LGPL because they are reused in libraries.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-05-03  4:51         ` Nicholas Piggin
@ 2024-05-03  5:44           ` Cédric Le Goater
  2024-05-07  4:32             ` Nicholas Piggin
  0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2024-05-03  5:44 UTC (permalink / raw)
  To: Nicholas Piggin, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On 5/3/24 06:51, Nicholas Piggin wrote:
> On Thu May 2, 2024 at 6:47 PM AEST, Cédric Le Goater wrote:
>> On 5/1/24 14:39, Nicholas Piggin wrote:
>>> On Wed Apr 17, 2024 at 9:25 PM AEST, Cédric Le Goater wrote:
>>>> Hello Nick,
>>>>
>>>> On 4/17/24 13:02, Nicholas Piggin wrote:
>>>>> This implements a framework for an ADU unit model.
>>>>>
>>>>> The ADU unit actually implements XSCOM, which is the bridge between MMIO
>>>>> and PIB. However it also includes control and status registers and other
>>>>> functions that are exposed as PIB (xscom) registers.
>>>>>
>>>>> To keep things simple, pnv_xscom.c remains the XSCOM bridge
>>>>> implementation, and pnv_adu.c implements the ADU registers and other
>>>>> functions.
>>>>>
>>>>> So far, just the ADU no-op registers in the pnv_xscom.c default handler
>>>>> are moved over to the adu model.
>>>>>
>>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>>>> ---
>>>>>     include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
>>>>>     include/hw/ppc/pnv_chip.h  |   3 +
>>>>>     include/hw/ppc/pnv_xscom.h |   6 ++
>>>>>     hw/ppc/pnv.c               |  16 ++++++
>>>>>     hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
>>>>>     hw/ppc/pnv_xscom.c         |   9 ---
>>>>>     hw/ppc/meson.build         |   1 +
>>>>>     hw/ppc/trace-events        |   4 ++
>>>>>     8 files changed, 175 insertions(+), 9 deletions(-)
>>>>>     create mode 100644 include/hw/ppc/pnv_adu.h
>>>>>     create mode 100644 hw/ppc/pnv_adu.c
>>>>>
>>>>> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
>>>>> new file mode 100644
>>>>> index 0000000000..9dc91857a9
>>>>> --- /dev/null
>>>>> +++ b/include/hw/ppc/pnv_adu.h
>>>>> @@ -0,0 +1,34 @@
>>>>> +/*
>>>>> + * QEMU PowerPC PowerNV Emulation of some ADU behaviour
>>>>> + *
>>>>> + * Copyright (c) 2024, IBM Corporation.
>>>>> + *
>>>>> + * SPDX-License-Identifier: LGPL-2.1-or-later
>>>>
>>>>
>>>> Did you mean GPL-2.0-or-later ?
>>>
>>> Hey Cedric,
>>>
>>> Thanks for reviewing, I've been away so sorry for the late reply.
>>>
>>> It just came from one of the headers I copied which was LGPL. But
>>> there's really nothing much in it and could find a GPL header to
>>> copy. Is GPL-2.0-or-later preferred?
>>
>> I would since all pnv models are GPL.
> 
> Some of pnv is actually LGPL. 

I was grepping for 'LGPL' and not 'Lesser' ... Indeed you are right.
Most files miss an SPDX-License-Identifier tag also.

> That's okay I'll change to GPL.

LGPL is more relaxed if the code needs to be used in libraries, but
I am not sure it applies to the PNV models. What would you prefer ?

C.

  
>> I think some parts of QEMU were initially LGPL (there used to be
>> a library, may be that's the reason ?) and other parts are relaxed
>> to LGPL because they are reused in libraries.
> 
> Thanks,
> Nick



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC model for POWER9/10
  2024-05-03  5:44           ` Cédric Le Goater
@ 2024-05-07  4:32             ` Nicholas Piggin
  0 siblings, 0 replies; 13+ messages in thread
From: Nicholas Piggin @ 2024-05-07  4:32 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc
  Cc: qemu-devel, Frédéric Barrat, Saif Abrar

On Fri May 3, 2024 at 3:44 PM AEST, Cédric Le Goater wrote:
> On 5/3/24 06:51, Nicholas Piggin wrote:
> > On Thu May 2, 2024 at 6:47 PM AEST, Cédric Le Goater wrote:
> >> On 5/1/24 14:39, Nicholas Piggin wrote:
> >>> On Wed Apr 17, 2024 at 9:25 PM AEST, Cédric Le Goater wrote:
> >>>> Hello Nick,
> >>>>
> >>>> On 4/17/24 13:02, Nicholas Piggin wrote:
> >>>>> This implements a framework for an ADU unit model.
> >>>>>
> >>>>> The ADU unit actually implements XSCOM, which is the bridge between MMIO
> >>>>> and PIB. However it also includes control and status registers and other
> >>>>> functions that are exposed as PIB (xscom) registers.
> >>>>>
> >>>>> To keep things simple, pnv_xscom.c remains the XSCOM bridge
> >>>>> implementation, and pnv_adu.c implements the ADU registers and other
> >>>>> functions.
> >>>>>
> >>>>> So far, just the ADU no-op registers in the pnv_xscom.c default handler
> >>>>> are moved over to the adu model.
> >>>>>
> >>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> >>>>> ---
> >>>>>     include/hw/ppc/pnv_adu.h   |  34 ++++++++++++
> >>>>>     include/hw/ppc/pnv_chip.h  |   3 +
> >>>>>     include/hw/ppc/pnv_xscom.h |   6 ++
> >>>>>     hw/ppc/pnv.c               |  16 ++++++
> >>>>>     hw/ppc/pnv_adu.c           | 111 +++++++++++++++++++++++++++++++++++++
> >>>>>     hw/ppc/pnv_xscom.c         |   9 ---
> >>>>>     hw/ppc/meson.build         |   1 +
> >>>>>     hw/ppc/trace-events        |   4 ++
> >>>>>     8 files changed, 175 insertions(+), 9 deletions(-)
> >>>>>     create mode 100644 include/hw/ppc/pnv_adu.h
> >>>>>     create mode 100644 hw/ppc/pnv_adu.c
> >>>>>
> >>>>> diff --git a/include/hw/ppc/pnv_adu.h b/include/hw/ppc/pnv_adu.h
> >>>>> new file mode 100644
> >>>>> index 0000000000..9dc91857a9
> >>>>> --- /dev/null
> >>>>> +++ b/include/hw/ppc/pnv_adu.h
> >>>>> @@ -0,0 +1,34 @@
> >>>>> +/*
> >>>>> + * QEMU PowerPC PowerNV Emulation of some ADU behaviour
> >>>>> + *
> >>>>> + * Copyright (c) 2024, IBM Corporation.
> >>>>> + *
> >>>>> + * SPDX-License-Identifier: LGPL-2.1-or-later
> >>>>
> >>>>
> >>>> Did you mean GPL-2.0-or-later ?
> >>>
> >>> Hey Cedric,
> >>>
> >>> Thanks for reviewing, I've been away so sorry for the late reply.
> >>>
> >>> It just came from one of the headers I copied which was LGPL. But
> >>> there's really nothing much in it and could find a GPL header to
> >>> copy. Is GPL-2.0-or-later preferred?
> >>
> >> I would since all pnv models are GPL.
> > 
> > Some of pnv is actually LGPL. 
>
> I was grepping for 'LGPL' and not 'Lesser' ... Indeed you are right.
> Most files miss an SPDX-License-Identifier tag also.
>
> > That's okay I'll change to GPL.
>
> LGPL is more relaxed if the code needs to be used in libraries, but
> I am not sure it applies to the PNV models. What would you prefer ?

GPL seems to be more common and I don't see a need for LGPL here,
so maybe GPL?

We could probably switch all LGPL pnv over to GPL if we wanted to.
I think LGPL permits such relicensing. Will leave this discussion
for another time though.

Thanks,
Nick


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2024-05-07  4:33 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-17 11:02 [PATCH 0/2] ppc/pnv: ADU model for POWER9/10 Nicholas Piggin
2024-04-17 11:02 ` [PATCH 1/2] ppc/pnv: Begin a more complete ADU LPC " Nicholas Piggin
2024-04-17 11:25   ` Cédric Le Goater
2024-05-01 12:39     ` Nicholas Piggin
2024-05-02  8:47       ` Cédric Le Goater
2024-05-03  4:51         ` Nicholas Piggin
2024-05-03  5:44           ` Cédric Le Goater
2024-05-07  4:32             ` Nicholas Piggin
2024-04-17 11:02 ` [PATCH 2/2] ppc/pnv: Implement ADU access to LPC space Nicholas Piggin
2024-04-17 12:25   ` Cédric Le Goater
2024-05-01 12:43     ` Nicholas Piggin
2024-05-02  8:32       ` Cédric Le Goater
2024-05-03  4:47         ` Nicholas Piggin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.