On 10/02/26 7:16 pm, Caleb Schlossin wrote:
This commit adds the read/write functionality for few core and
quad registers for Power Hypervisor development.

Reviewed-by: Chalapathi V <[email protected]
Reviewed-by: Glenn Miles <[email protected]>
Reviewed-by: Aditya Gupta <[email protected]>
Signed-off-by: Chalapathi V <[email protected]>
Signed-off-by: Caleb Schlossin <[email protected]>
---
  hw/ppc/pnv_core.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 87 insertions(+)

diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 8939515c2c..a95934710f 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -183,12 +183,22 @@ static const MemoryRegionOps pnv_core_power9_xscom_ops = {
/*
   * POWER10 core controls
+ *   Adding additional SCOM register support needed by the
+ *   Power Hypervisor team for their development

.. needed for Power Hypervisor development ?

   */
+#define PNV10_XSCOM_EC_IMA_EVENT_MASK 0x400
  #define PNV10_XSCOM_EC_CORE_THREAD_STATE    0x412
  #define PNV10_XSCOM_EC_CORE_THREAD_INFO     0x413
+#define PNV10_XSCOM_EC_CORE_FIRMASK         0x443
+#define PNV10_XSCOM_EC_CORE_FIRMASK_AND     0x444
+#define PNV10_XSCOM_EC_CORE_FIRMASK_OR      0x445
  #define PNV10_XSCOM_EC_CORE_DIRECT_CONTROLS 0x449
  #define PNV10_XSCOM_EC_CORE_RAS_STATUS      0x454
+#define PNV10_XSCOM_EC_SPATTN_OR            0x497
+#define PNV10_XSCOM_EC_SPATTN_AND           0x498
+#define PNV10_XSCOM_EC_SPATTN               0x499
+#define PNV10_XSCOM_EC_SPATTN_MASK          0x49A
static uint64_t pnv_core_power10_xscom_read(void *opaque, hwaddr addr,
                                             unsigned int width)
@@ -224,6 +234,19 @@ static uint64_t pnv_core_power10_xscom_read(void *opaque, 
hwaddr addr,
              }
          }
          break;
+    case PNV10_XSCOM_EC_IMA_EVENT_MASK:
+    case PNV10_XSCOM_EC_CORE_FIRMASK:
+        return 0;
+    case PNV10_XSCOM_EC_CORE_FIRMASK_OR:
+    case PNV10_XSCOM_EC_CORE_FIRMASK_AND:
+    case PNV10_XSCOM_EC_SPATTN_OR:
+    case PNV10_XSCOM_EC_SPATTN_AND:
+        qemu_log_mask(LOG_GUEST_ERROR, "%s: Write only register, ignoring "
+                              "xscom read at 0x%08x\n", __func__, offset);
+        break;
+    case PNV10_XSCOM_EC_SPATTN:
+    case PNV10_XSCOM_EC_SPATTN_MASK:
+        return 0;

These two registers could be clubbed alongwith previous two regs case
returning 0. A single line comment with the reasoning for same would be
appropriate.


      default:
          qemu_log_mask(LOG_UNIMP, "%s: unimp read 0x%08x\n", __func__,
                        offset);
@@ -284,6 +307,19 @@ static void pnv_core_power10_xscom_write(void *opaque, 
hwaddr addr,
          }
          break;
+ /*
+     * Allow writes to these SCOMs for Power Hypervisor development.
+     * Behavior is not needed, just to allow writes to these SCOMs.
+     */
+    case PNV10_XSCOM_EC_IMA_EVENT_MASK:
+    case PNV10_XSCOM_EC_CORE_FIRMASK:
+    case PNV10_XSCOM_EC_CORE_FIRMASK_OR:
+    case PNV10_XSCOM_EC_CORE_FIRMASK_AND:
+    case PNV10_XSCOM_EC_SPATTN_OR:
+    case PNV10_XSCOM_EC_SPATTN_AND:
+    case PNV10_XSCOM_EC_SPATTN:
+    case PNV10_XSCOM_EC_SPATTN_MASK:
+        break;
      default:
          qemu_log_mask(LOG_UNIMP, "%s: unimp write 0x%08x\n", __func__,
                        offset);
@@ -579,6 +615,23 @@ static const MemoryRegionOps pnv_quad_power9_xscom_ops = {
   * POWER10 Quads
   */
+#define P10_XSCOM_EQ3_MODE_REG1 0x1160a
+#define P10_XSCOM_EQ3_NCU_SPEC_BAR_REG  0x11650
+#define P10_XSCOM_EQ3_HTM_MODE          0x11680
+#define P10_XSCOM_EQ3_HTM_IMA_PDBAR     0x1168b
+#define P10_XSCOM_EQ2_MODE_REG1         0x1260a
+#define P10_XSCOM_EQ2_NCU_SPEC_BAR_REG  0x12650
+#define P10_XSCOM_EQ2_HTM_MODE          0x12680
+#define P10_XSCOM_EQ2_HTM_IMA_PDBAR     0x1268b
+#define P10_XSCOM_EQ1_MODE_REG1         0x1460a
+#define P10_XSCOM_EQ1_NCU_SPEC_BAR_REG  0x14650
+#define P10_XSCOM_EQ1_HTM_MODE          0x14680
+#define P10_XSCOM_EQ1_HTM_IMA_PDBAR     0x1468b
+#define P10_XSCOM_EQ0_MODE_REG1         0x1860a
+#define P10_XSCOM_EQ0_NCU_SPEC_BAR_REG  0x18650
+#define P10_XSCOM_EQ0_HTM_MODE          0x18680
+#define P10_XSCOM_EQ0_HTM_IMA_PDBAR     0x1868b
+

I think it's time to probably move all macro definitions to include/hw/ppc/pnv/pnv_core.h ? Preferably, a separate patch to move the existing macros first, and then this patch can add new ones there.

  static uint64_t pnv_quad_power10_xscom_read(void *opaque, hwaddr addr,
                                              unsigned int width)
  {
@@ -586,6 +639,23 @@ static uint64_t pnv_quad_power10_xscom_read(void *opaque, 
hwaddr addr,
      uint64_t val = -1;
switch (offset) {
+    case P10_XSCOM_EQ0_MODE_REG1:
+    case P10_XSCOM_EQ0_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ0_HTM_MODE:
+    case P10_XSCOM_EQ0_HTM_IMA_PDBAR:
+    case P10_XSCOM_EQ1_MODE_REG1:
+    case P10_XSCOM_EQ1_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ1_HTM_MODE:
+    case P10_XSCOM_EQ1_HTM_IMA_PDBAR:
+    case P10_XSCOM_EQ2_MODE_REG1:
+    case P10_XSCOM_EQ2_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ2_HTM_MODE:
+    case P10_XSCOM_EQ2_HTM_IMA_PDBAR:
+    case P10_XSCOM_EQ3_MODE_REG1:
+    case P10_XSCOM_EQ3_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ3_HTM_MODE:
+    case P10_XSCOM_EQ3_HTM_IMA_PDBAR:

a comment for the rationale behind this change would be helpful.

+        return 0;
      default:
          qemu_log_mask(LOG_UNIMP, "%s: unimp read 0x%08x\n", __func__,
                        offset);
@@ -600,6 +670,23 @@ static void pnv_quad_power10_xscom_write(void *opaque, 
hwaddr addr,
      uint32_t offset = addr >> 3;
switch (offset) {
+    case P10_XSCOM_EQ0_MODE_REG1:
+    case P10_XSCOM_EQ0_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ0_HTM_MODE:
+    case P10_XSCOM_EQ0_HTM_IMA_PDBAR:
+    case P10_XSCOM_EQ1_MODE_REG1:
+    case P10_XSCOM_EQ1_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ1_HTM_MODE:
+    case P10_XSCOM_EQ1_HTM_IMA_PDBAR:
+    case P10_XSCOM_EQ2_MODE_REG1:
+    case P10_XSCOM_EQ2_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ2_HTM_MODE:
+    case P10_XSCOM_EQ2_HTM_IMA_PDBAR:
+    case P10_XSCOM_EQ3_MODE_REG1:
+    case P10_XSCOM_EQ3_NCU_SPEC_BAR_REG:
+    case P10_XSCOM_EQ3_HTM_MODE:
+    case P10_XSCOM_EQ3_HTM_IMA_PDBAR:
+        break;

ditto for comment providing rationale.

BTW, I am still not sure if we really need this patch upstream at the
moment in case a behaviour change would be needed again for some of
these regs until the full PowerVM support is ready to be upstreamed.
However, since the change appears safe (has been vetted by multiple
reviewers already), we can consider it along with above minor comments
addressed.

regards,
Harsh

      default:
          qemu_log_mask(LOG_UNIMP, "%s: unimp write 0x%08x\n", __func__,
                        offset);


Reply via email to