All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5
@ 2024-03-21 10:04 Saif Abrar
  2024-03-21 10:04 ` [PATCH 01/10] qtest/phb4: Add testbench for PHB4 Saif Abrar
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Hello,

This series updates the existing PHB4 model to the latest spec:
"Power Systems Host Bridge 5 (PHB5) Functional Specification Version 0.5_00".

Updates include the following:
- implemented sticky reset logic
- implemented read-only, write-only, W1C and WxC logic
- return all 1's on read to unimplemented registers
- update PCIE registers for link status, speed and width
- implement IODA PCT debug table without any functionality
- set write-mask bits for PCIE Link-Control-2 register that is read/written by PHB4
- update LSI Source-ID register based on small/big PHB number of interrupts

Also, a new testbench for PHB4 model is added that does XSCOM read/writes
to various registers of interest and verifies the values.

Regards.

Saif Abrar (10):
  qtest/phb4: Add testbench for PHB4
  pnv/phb4: Add reset logic to PHB4
  pnv/phb4: Implement sticky reset logic in PHB4
  pnv/phb4: Implement read-only and write-only bits of registers
  pnv/phb4: Implement write-clear and return 1's on unimplemented reg read
  pnv/phb4: Set link-active status in HPSTAT and LMR registers
  pnv/phb4: Set link speed and width in the DLP training control register
  pnv/phb4: Implement IODA PCT table
  hw/pci: Set write-mask bits for PCIE Link-Control-2 register
  pnv/phb4: Mask off LSI Source-ID based on number of interrupts

 hw/pci-host/pnv_phb4.c                    | 601 ++++++++++++++++++++--
 hw/pci/pcie.c                             |   6 +
 include/hw/pci-host/pnv_phb4.h            |   9 +
 include/hw/pci-host/pnv_phb4_regs.h       |  66 ++-
 include/standard-headers/linux/pci_regs.h |   3 +
 tests/qtest/meson.build                   |   1 +
 tests/qtest/pnv-phb4-test.c               | 177 +++++++
 7 files changed, 805 insertions(+), 58 deletions(-)
 create mode 100644 tests/qtest/pnv-phb4-test.c

-- 
2.39.3



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

* [PATCH 01/10] qtest/phb4: Add testbench for PHB4
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25  9:39   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 02/10] pnv/phb4: Add reset logic to PHB4 Saif Abrar
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

New qtest TB added for PHB4.
TB reads PHB Version register and asserts that
bits[24:31] have value 0xA5.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 tests/qtest/meson.build     |  1 +
 tests/qtest/pnv-phb4-test.c | 74 +++++++++++++++++++++++++++++++++++++
 2 files changed, 75 insertions(+)
 create mode 100644 tests/qtest/pnv-phb4-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 36c5c13a7b..4795e51c17 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -168,6 +168,7 @@ qtests_ppc64 = \
   (config_all_devices.has_key('CONFIG_PSERIES') ? ['device-plug-test'] : []) +               \
   (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-xscom-test'] : []) +                 \
   (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-host-i2c-test'] : []) +              \
+  (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-phb4-test'] : []) +                  \
   (config_all_devices.has_key('CONFIG_PSERIES') ? ['rtas-test'] : []) +                      \
   (slirp.found() ? ['pxe-test'] : []) +              \
   (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) +             \
diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
new file mode 100644
index 0000000000..e3b809e9c4
--- /dev/null
+++ b/tests/qtest/pnv-phb4-test.c
@@ -0,0 +1,74 @@
+/*
+ * QTest testcase for PowerNV PHB4
+ *
+ * Copyright (c) 2024, IBM Corporation.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest.h"
+#include "hw/pci-host/pnv_phb4_regs.h"
+
+#define P10_XSCOM_BASE          0x000603fc00000000ull
+#define PHB4_MMIO               0x000600c3c0000000ull
+#define PHB4_XSCOM              0x8010900ull
+
+#define PPC_BIT(bit)            (0x8000000000000000ULL >> (bit))
+#define PPC_BITMASK(bs, be)     ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
+
+static uint64_t pnv_xscom_addr(uint32_t pcba)
+{
+    return P10_XSCOM_BASE | ((uint64_t) pcba << 3);
+}
+
+static uint64_t pnv_phb4_xscom_addr(uint32_t reg)
+{
+    return pnv_xscom_addr(PHB4_XSCOM + reg);
+}
+
+/*
+ * XSCOM read/write is indirect in PHB4:
+ * Write 'SCOM - HV Indirect Address Register'
+ * with register-offset to read/write.
+   - bit[0]: Valid Bit
+   - bit[51:61]: Indirect Address(00:10)
+ * Read/write 'SCOM - HV Indirect Data Register' to get/set the value.
+ */
+
+static uint64_t pnv_phb4_xscom_read(QTestState *qts, uint32_t reg)
+{
+    qtest_writeq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_ADDR),
+            PPC_BIT(0) | reg);
+    return qtest_readq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_DATA));
+}
+
+/* Assert that 'PHB - Version Register Offset 0x0800' bits-[24:31] are 0xA5 */
+static void phb4_version_test(QTestState *qts)
+{
+    uint64_t ver = pnv_phb4_xscom_read(qts, PHB_VERSION);
+
+    /* PHB Version register [24:31]: Major Revision ID 0xA5 */
+    ver = ver >> (63 - 31);
+    g_assert_cmpuint(ver, ==, 0xA5);
+}
+
+static void test_phb4(void)
+{
+    QTestState *qts = NULL;
+
+    qts = qtest_initf("-machine powernv10 -accel tcg -nographic -d unimp");
+
+    /* Make sure test is running on PHB */
+    phb4_version_test(qts);
+
+    qtest_quit(qts);
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("phb4", test_phb4);
+    return g_test_run();
+}
-- 
2.39.3



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

* [PATCH 02/10] pnv/phb4: Add reset logic to PHB4
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
  2024-03-21 10:04 ` [PATCH 01/10] qtest/phb4: Add testbench for PHB4 Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 13:32   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 03/10] pnv/phb4: Implement sticky reset logic in PHB4 Saif Abrar
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Add a method to be invoked on QEMU reset.
Also add CFG and PBL core-blocks reset logic using
appropriate bits of PHB_PCIE_CRESET register.

Tested by reading the reset value of a register.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c              | 104 +++++++++++++++++++++++++++-
 include/hw/pci-host/pnv_phb4_regs.h |  16 ++++-
 tests/qtest/pnv-phb4-test.c         |  10 +++
 3 files changed, 127 insertions(+), 3 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 075499d36d..d2e7403b37 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -1,7 +1,7 @@
 /*
- * QEMU PowerPC PowerNV (POWER9) PHB4 model
+ * QEMU PowerPC PowerNV (POWER10) PHB4 model
  *
- * Copyright (c) 2018-2020, IBM Corporation.
+ * Copyright (c) 2018-2024, IBM Corporation.
  *
  * This code is licensed under the GPL version 2 or later. See the
  * COPYING file in the top-level directory.
@@ -22,6 +22,7 @@
 #include "hw/qdev-properties.h"
 #include "qom/object.h"
 #include "trace.h"
+#include "sysemu/reset.h"
 
 #define phb_error(phb, fmt, ...)                                        \
     qemu_log_mask(LOG_GUEST_ERROR, "phb4[%d:%d]: " fmt "\n",            \
@@ -499,6 +500,86 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
     }
 }
 
+/*
+ * Get the PCI-E capability offset from the root-port
+ */
+static uint32_t get_exp_offset(PnvPHB4 *phb)
+{
+    PCIHostState *pci = PCI_HOST_BRIDGE(phb->phb_base);
+    PCIDevice *pdev;
+    pdev = pci_find_device(pci->bus, 0, 0);
+    if (!pdev) {
+        phb_error(phb, "PCI device not found");
+        return ~0;
+    }
+    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(pdev);
+    return rpc->exp_offset;
+}
+
+#define RC_CONFIG_WRITE(a, v) pnv_phb4_rc_config_write(phb, a, 4, v);
+
+static void pnv_phb4_cfg_core_reset(PnvPHB4 *phb)
+{
+    /* Zero all registers initially */
+    int i;
+    for (i = PCI_COMMAND ; i < PHB_RC_CONFIG_SIZE ; i += 4) {
+            RC_CONFIG_WRITE(i, 0)
+    }
+
+    RC_CONFIG_WRITE(PCI_COMMAND,          0x100100);
+    RC_CONFIG_WRITE(PCI_CLASS_REVISION,   0x6040000);
+    RC_CONFIG_WRITE(PCI_CACHE_LINE_SIZE,  0x10000);
+    RC_CONFIG_WRITE(PCI_MEMORY_BASE,      0x10);
+    RC_CONFIG_WRITE(PCI_PREF_MEMORY_BASE, 0x10011);
+    RC_CONFIG_WRITE(PCI_CAPABILITY_LIST,  0x40);
+    RC_CONFIG_WRITE(PCI_INTERRUPT_LINE,   0x20000);
+    /* PM Capabilities Register */
+    RC_CONFIG_WRITE(PCI_BRIDGE_CONTROL + PCI_PM_PMC, 0xC8034801);
+
+    uint32_t exp_offset = get_exp_offset(phb);
+    RC_CONFIG_WRITE(exp_offset, 0x420010);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCAP,  0x8022);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCTL,  0x140);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCAP,  0x300105);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCTL,  0x2010008);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_SLTCTL,  0x2000);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCAP2, 0x1003F);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCTL2, 0x20);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCAP2, 0x80003E);
+    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCTL2, 0x5);
+
+    RC_CONFIG_WRITE(PHB_AER_ECAP,    0x14810001);
+    RC_CONFIG_WRITE(PHB_AER_CAPCTRL, 0xA0);
+    RC_CONFIG_WRITE(PHB_SEC_ECAP,    0x1A010019);
+
+    RC_CONFIG_WRITE(PHB_LMR_ECAP, 0x1E810027);
+    /* LMR - Margining Lane Control / Status Register # 2 to 16 */
+    for (i = PHB_LMR_CTLSTA_2 ; i <= PHB_LMR_CTLSTA_16 ; i += 4) {
+        RC_CONFIG_WRITE(i, 0x9C38);
+    }
+
+    RC_CONFIG_WRITE(PHB_DLF_ECAP, 0x1F410025);
+    RC_CONFIG_WRITE(PHB_DLF_CAP,  0x80000001);
+    RC_CONFIG_WRITE(P16_ECAP,     0x22410026);
+    RC_CONFIG_WRITE(P32_ECAP,     0x1002A);
+    RC_CONFIG_WRITE(P32_CAP,      0x103);
+}
+
+static void pnv_phb4_pbl_core_reset(PnvPHB4 *phb)
+{
+    /* Zero all registers initially */
+    int i;
+    for (i = PHB_PBL_CONTROL ; i <= PHB_PBL_ERR1_STATUS_MASK ; i += 8) {
+        phb->regs[i >> 3] = 0x0;
+    }
+
+    /* Set specific register values */
+    phb->regs[PHB_PBL_CONTROL       >> 3] = 0xC009000000000000;
+    phb->regs[PHB_PBL_TIMEOUT_CTRL  >> 3] = 0x2020000000000000;
+    phb->regs[PHB_PBL_NPTAG_ENABLE  >> 3] = 0xFFFFFFFF00000000;
+    phb->regs[PHB_PBL_SYS_LINK_INIT >> 3] = 0x80088B4642473000;
+}
+
 static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
                                unsigned size)
 {
@@ -612,6 +693,16 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
         pnv_phb4_update_xsrc(phb);
         break;
 
+    /* Reset core blocks */
+    case PHB_PCIE_CRESET:
+        if (val & PHB_PCIE_CRESET_CFG_CORE) {
+            pnv_phb4_cfg_core_reset(phb);
+        }
+        if (val & PHB_PCIE_CRESET_PBL) {
+            pnv_phb4_pbl_core_reset(phb);
+        }
+        break;
+
     /* Silent simple writes */
     case PHB_ASN_CMPM:
     case PHB_CONFIG_ADDRESS:
@@ -1531,6 +1622,13 @@ static void pnv_phb4_xscom_realize(PnvPHB4 *phb)
 static PCIIOMMUOps pnv_phb4_iommu_ops = {
     .get_address_space = pnv_phb4_dma_iommu,
 };
+static void pnv_phb4_reset(void *dev)
+{
+    PnvPHB4 *phb = PNV_PHB4(dev);
+    pnv_phb4_cfg_core_reset(phb);
+    pnv_phb4_pbl_core_reset(phb);
+    phb->regs[PHB_PCIE_CRESET >> 3] = 0xE000000000000000;
+}
 
 static void pnv_phb4_instance_init(Object *obj)
 {
@@ -1608,6 +1706,8 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
     phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, xsrc->nr_irqs);
 
     pnv_phb4_xscom_realize(phb);
+
+    qemu_register_reset(pnv_phb4_reset, dev);
 }
 
 /*
diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
index bea96f4d91..6892e21cc9 100644
--- a/include/hw/pci-host/pnv_phb4_regs.h
+++ b/include/hw/pci-host/pnv_phb4_regs.h
@@ -343,6 +343,18 @@
 #define PHB_RC_CONFIG_BASE                      0x1000
 #define   PHB_RC_CONFIG_SIZE                    0x800
 
+#define PHB_AER_ECAP                            0x100
+#define PHB_AER_CAPCTRL                         0x118
+#define PHB_SEC_ECAP                            0x148
+#define PHB_LMR_ECAP                            0x1A0
+#define PHB_LMR_CTLSTA_2                        0x1AC
+#define PHB_LMR_CTLSTA_16                       0x1E4
+#define PHB_DLF_ECAP                            0x1E8
+#define PHB_DLF_CAP                             0x1EC
+#define P16_ECAP                                0x1F4
+#define P32_ECAP                                0x224
+#define P32_CAP                                 0x228
+
 /* PHB4 REGB registers */
 
 /* PBL core */
@@ -368,7 +380,7 @@
 #define PHB_PCIE_SCR                    0x1A00
 #define   PHB_PCIE_SCR_SLOT_CAP         PPC_BIT(15)
 #define   PHB_PCIE_SCR_MAXLINKSPEED     PPC_BITMASK(32, 35)
-
+#define PHB_PCIE_BNR                    0x1A08
 
 #define PHB_PCIE_CRESET                 0x1A10
 #define   PHB_PCIE_CRESET_CFG_CORE      PPC_BIT(0)
@@ -423,6 +435,8 @@
 #define PHB_PCIE_LANE_EQ_CNTL23         0x1B08 /* DD1 only */
 #define PHB_PCIE_TRACE_CTRL             0x1B20
 #define PHB_PCIE_MISC_STRAP             0x1B30
+#define PHB_PCIE_PHY_RXEQ_STAT_G3_00_03 0x1B40
+#define PHB_PCIE_PHY_RXEQ_STAT_G5_12_15 0x1B98
 
 /* Error */
 #define PHB_REGB_ERR_STATUS             0x1C00
diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
index e3b809e9c4..44141462f6 100644
--- a/tests/qtest/pnv-phb4-test.c
+++ b/tests/qtest/pnv-phb4-test.c
@@ -54,6 +54,13 @@ static void phb4_version_test(QTestState *qts)
     g_assert_cmpuint(ver, ==, 0xA5);
 }
 
+/* Assert that 'PHB PBL Control' register has correct reset value */
+static void phb4_reset_test(QTestState *qts)
+{
+    g_assert_cmpuint(pnv_phb4_xscom_read(qts, PHB_PBL_CONTROL),
+                     ==, 0xC009000000000000);
+}
+
 static void test_phb4(void)
 {
     QTestState *qts = NULL;
@@ -63,6 +70,9 @@ static void test_phb4(void)
     /* Make sure test is running on PHB */
     phb4_version_test(qts);
 
+    /* Check reset value of a register */
+    phb4_reset_test(qts);
+
     qtest_quit(qts);
 }
 
-- 
2.39.3



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

* [PATCH 03/10] pnv/phb4: Implement sticky reset logic in PHB4
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
  2024-03-21 10:04 ` [PATCH 01/10] qtest/phb4: Add testbench for PHB4 Saif Abrar
  2024-03-21 10:04 ` [PATCH 02/10] pnv/phb4: Add reset logic to PHB4 Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-21 10:04 ` [PATCH 04/10] pnv/phb4: Implement read-only and write-only bits of registers Saif Abrar
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Sticky bits retain their values on reset and are not overwritten with the reset value.
Added sticky reset logic for all required registers,
i.e. CFG core, PBL core, PHB error registers, PCIE stack registers and REGB error registers.

Tested by writing all 1's to the reg PHB_PBL_ERR_INJECT.
This will set the bits in the reg PHB_PBL_ERR_STATUS.
Reset the PBL core by setting PHB_PCIE_CRESET_PBL in reg PHB_PCIE_CRESET.
Verify that the sticky bits in the PHB_PBL_ERR_STATUS reg are still set.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c              | 156 ++++++++++++++++++++++++++--
 include/hw/pci-host/pnv_phb4_regs.h |  20 +++-
 tests/qtest/pnv-phb4-test.c         |  30 +++++-
 3 files changed, 196 insertions(+), 10 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index d2e7403b37..b3a83837f8 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -516,14 +516,52 @@ static uint32_t get_exp_offset(PnvPHB4 *phb)
     return rpc->exp_offset;
 }
 
-#define RC_CONFIG_WRITE(a, v) pnv_phb4_rc_config_write(phb, a, 4, v);
+#define RC_CONFIG_WRITE(a, v) pnv_phb4_rc_config_write(phb, a, 4, v)
+
+/*
+ * Apply sticky-mask 's' to the reset-value 'v' and write to the address 'a'.
+ * RC-config space values and masks are LE.
+ * Method pnv_phb4_rc_config_read() returns BE, hence convert to LE.
+ * Compute new value in LE domain.
+ * New value computation using sticky-mask is in LE.
+ * Convert the computed value from LE to BE before writing back.
+ */
+#define RC_CONFIG_STICKY_RESET(a, v, s) \
+    (RC_CONFIG_WRITE(a, bswap32( \
+                     (bswap32(pnv_phb4_rc_config_read(phb, a, 4)) & s) \
+                      | (v & ~s) \
+                     )))
 
 static void pnv_phb4_cfg_core_reset(PnvPHB4 *phb)
 {
-    /* Zero all registers initially */
+    /*
+     * Zero all registers initially,
+     * except those that have sticky reset.
+     */
     int i;
     for (i = PCI_COMMAND ; i < PHB_RC_CONFIG_SIZE ; i += 4) {
-            RC_CONFIG_WRITE(i, 0)
+        switch (i) {
+        case PCI_EXP_LNKCTL2:
+        case PHB_AER_UERR:
+        case PHB_AER_UERR_MASK:
+        case PHB_AER_CERR:
+        case PHB_AER_CAPCTRL:
+        case PHB_AER_HLOG_1:
+        case PHB_AER_HLOG_2:
+        case PHB_AER_HLOG_3:
+        case PHB_AER_HLOG_4:
+        case PHB_AER_RERR:
+        case PHB_AER_ESID:
+        case PHB_DLF_STAT:
+        case P16_STAT:
+        case P16_LDPM:
+        case P16_FRDPM:
+        case P16_SRDPM:
+        case P32_CTL:
+            break;
+        default:
+            RC_CONFIG_WRITE(i, 0);
+        }
     }
 
     RC_CONFIG_WRITE(PCI_COMMAND,          0x100100);
@@ -563,15 +601,55 @@ static void pnv_phb4_cfg_core_reset(PnvPHB4 *phb)
     RC_CONFIG_WRITE(P16_ECAP,     0x22410026);
     RC_CONFIG_WRITE(P32_ECAP,     0x1002A);
     RC_CONFIG_WRITE(P32_CAP,      0x103);
+
+    /* Sticky reset */
+    RC_CONFIG_STICKY_RESET(exp_offset + PCI_EXP_LNKCTL2,   0x5,  0xFEFFBF);
+    RC_CONFIG_STICKY_RESET(PHB_AER_UERR,      0,    0x1FF030);
+    RC_CONFIG_STICKY_RESET(PHB_AER_UERR_MASK, 0,    0x1FF030);
+    RC_CONFIG_STICKY_RESET(PHB_AER_CERR,      0,    0x11C1);
+    RC_CONFIG_STICKY_RESET(PHB_AER_CAPCTRL,   0xA0, 0x15F);
+    RC_CONFIG_STICKY_RESET(PHB_AER_HLOG_1,    0,    0xFFFFFFFF);
+    RC_CONFIG_STICKY_RESET(PHB_AER_HLOG_2,    0,    0xFFFFFFFF);
+    RC_CONFIG_STICKY_RESET(PHB_AER_HLOG_3,    0,    0xFFFFFFFF);
+    RC_CONFIG_STICKY_RESET(PHB_AER_HLOG_4,    0,    0xFFFFFFFF);
+    RC_CONFIG_STICKY_RESET(PHB_AER_RERR,      0,    0x7F);
+    RC_CONFIG_STICKY_RESET(PHB_AER_ESID,      0,    0xFFFFFFFF);
+    RC_CONFIG_STICKY_RESET(PHB_DLF_STAT,      0,    0x807FFFFF);
+    RC_CONFIG_STICKY_RESET(P16_STAT,          0,    0x1F);
+    RC_CONFIG_STICKY_RESET(P16_LDPM,          0,    0xFFFF);
+    RC_CONFIG_STICKY_RESET(P16_FRDPM,         0,    0xFFFF);
+    RC_CONFIG_STICKY_RESET(P16_SRDPM,         0,    0xFFFF);
+    RC_CONFIG_STICKY_RESET(P32_CTL,           0,    0x3);
 }
 
+/* Apply sticky-mask to the reset-value and write to the reg-address */
+#define STICKY_RST(addr, rst_val, sticky_mask) (phb->regs[addr >> 3] = \
+            ((phb->regs[addr >> 3] & sticky_mask) | (rst_val & ~sticky_mask)))
+
 static void pnv_phb4_pbl_core_reset(PnvPHB4 *phb)
 {
-    /* Zero all registers initially */
+    /*
+     * Zero all registers initially,
+     * with sticky reset of certain registers.
+     */
     int i;
     for (i = PHB_PBL_CONTROL ; i <= PHB_PBL_ERR1_STATUS_MASK ; i += 8) {
-        phb->regs[i >> 3] = 0x0;
+        switch (i) {
+        case PHB_PBL_ERR_STATUS:
+            break;
+        case PHB_PBL_ERR1_STATUS:
+        case PHB_PBL_ERR_LOG_0:
+        case PHB_PBL_ERR_LOG_1:
+        case PHB_PBL_ERR_STATUS_MASK:
+        case PHB_PBL_ERR1_STATUS_MASK:
+            STICKY_RST(i, 0, PPC_BITMASK(0, 63));
+            break;
+        default:
+            phb->regs[i >> 3] = 0x0;
+        }
     }
+    STICKY_RST(PHB_PBL_ERR_STATUS, 0, \
+            (PPC_BITMASK(0, 9) | PPC_BITMASK(12, 63)));
 
     /* Set specific register values */
     phb->regs[PHB_PBL_CONTROL       >> 3] = 0xC009000000000000;
@@ -703,6 +781,17 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
         }
         break;
 
+    /*
+     * Writing bits to a 1 in this register will inject the error corresponding
+     * to the bit that is written. The bits will automatically clear to 0 after
+     * the error is injected. The corresponding bit in the Error Status Reg
+     * should also be set automatically when the error occurs.
+     */
+    case PHB_PBL_ERR_INJECT:
+        phb->regs[PHB_PBL_ERR_STATUS >> 3] = phb->regs[off >> 3];
+        phb->regs[off >> 3] = 0;
+        break;
+
     /* Silent simple writes */
     case PHB_ASN_CMPM:
     case PHB_CONFIG_ADDRESS:
@@ -1622,12 +1711,67 @@ static void pnv_phb4_xscom_realize(PnvPHB4 *phb)
 static PCIIOMMUOps pnv_phb4_iommu_ops = {
     .get_address_space = pnv_phb4_dma_iommu,
 };
+
+static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
+{
+    STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
+    STICKY_RST(PHB_ERR1_STATUS,      0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_ERR_STATUS_MASK,  0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_ERR1_STATUS_MASK, 0, PPC_BITMASK(0, 63));
+
+    STICKY_RST(PHB_TXE_ERR_STATUS,       0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_TXE_ERR1_STATUS,      0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_TXE_ERR_STATUS_MASK,  0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_TXE_ERR1_STATUS_MASK, 0, PPC_BITMASK(0, 63));
+
+    STICKY_RST(PHB_RXE_ARB_ERR_STATUS,       0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_ARB_ERR1_STATUS,      0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_ARB_ERR_LOG_0,        0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_ARB_ERR_LOG_1,        0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_ARB_ERR_STATUS_MASK,  0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_ARB_ERR1_STATUS_MASK, 0, PPC_BITMASK(0, 63));
+
+    STICKY_RST(PHB_RXE_MRG_ERR_STATUS,       0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_MRG_ERR1_STATUS,      0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_MRG_ERR_STATUS_MASK,  0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_MRG_ERR1_STATUS_MASK, 0, PPC_BITMASK(0, 63));
+
+    STICKY_RST(PHB_RXE_TCE_ERR_STATUS,       0, PPC_BITMASK(0, 35));
+    STICKY_RST(PHB_RXE_TCE_ERR1_STATUS,      0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_TCE_ERR_LOG_0,        0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_TCE_ERR_LOG_1,        0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_TCE_ERR_STATUS_MASK,  0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_RXE_TCE_ERR1_STATUS_MASK, 0, PPC_BITMASK(0, 63));
+}
+
+static void pnv_phb4_pcie_stack_reg_reset(PnvPHB4 *phb)
+{
+    STICKY_RST(PHB_PCIE_CRESET, 0xE000000000000000, \
+                        (PHB_PCIE_CRESET_PERST_N | PHB_PCIE_CRESET_REFCLK_N));
+    STICKY_RST(PHB_PCIE_DLP_ERRLOG1,             0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_PCIE_DLP_ERRLOG2,             0, PPC_BITMASK(0, 31));
+    STICKY_RST(PHB_PCIE_DLP_ERR_STATUS,          0, PPC_BITMASK(0, 15));
+}
+
+static void pnv_phb4_regb_err_reg_reset(PnvPHB4 *phb)
+{
+    STICKY_RST(PHB_REGB_ERR_STATUS,       0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_REGB_ERR1_STATUS,      0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_REGB_ERR_LOG_0,        0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_REGB_ERR_LOG_1,        0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_REGB_ERR_STATUS_MASK,  0, PPC_BITMASK(0, 63));
+    STICKY_RST(PHB_REGB_ERR1_STATUS_MASK, 0, PPC_BITMASK(0, 63));
+}
+
 static void pnv_phb4_reset(void *dev)
 {
     PnvPHB4 *phb = PNV_PHB4(dev);
     pnv_phb4_cfg_core_reset(phb);
     pnv_phb4_pbl_core_reset(phb);
-    phb->regs[PHB_PCIE_CRESET >> 3] = 0xE000000000000000;
+
+    pnv_phb4_err_reg_reset(phb);
+    pnv_phb4_pcie_stack_reg_reset(phb);
+    pnv_phb4_regb_err_reg_reset(phb);
 }
 
 static void pnv_phb4_instance_init(Object *obj)
diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
index 6892e21cc9..df5e86d29a 100644
--- a/include/hw/pci-host/pnv_phb4_regs.h
+++ b/include/hw/pci-host/pnv_phb4_regs.h
@@ -344,17 +344,32 @@
 #define   PHB_RC_CONFIG_SIZE                    0x800
 
 #define PHB_AER_ECAP                            0x100
+#define PHB_AER_UERR                            0x104
+#define PHB_AER_UERR_MASK                       0x108
+#define PHB_AER_CERR                            0x110
 #define PHB_AER_CAPCTRL                         0x118
+#define PHB_AER_HLOG_1                          0x11C
+#define PHB_AER_HLOG_2                          0x120
+#define PHB_AER_HLOG_3                          0x124
+#define PHB_AER_HLOG_4                          0x128
+#define PHB_AER_RERR                            0x130
+#define PHB_AER_ESID                            0x134
 #define PHB_SEC_ECAP                            0x148
 #define PHB_LMR_ECAP                            0x1A0
 #define PHB_LMR_CTLSTA_2                        0x1AC
 #define PHB_LMR_CTLSTA_16                       0x1E4
 #define PHB_DLF_ECAP                            0x1E8
 #define PHB_DLF_CAP                             0x1EC
+#define PHB_DLF_STAT                            0x1F0
 #define P16_ECAP                                0x1F4
+#define P16_STAT                                0x200
+#define P16_LDPM                                0x204
+#define P16_FRDPM                               0x208
+#define P16_SRDPM                               0x20C
 #define P32_ECAP                                0x224
 #define P32_CAP                                 0x228
-
+#define P32_CTL                                 0x22C
+#define P32_STAT                                0x230
 /* PHB4 REGB registers */
 
 /* PBL core */
@@ -388,8 +403,7 @@
 #define   PHB_PCIE_CRESET_PBL           PPC_BIT(2)
 #define   PHB_PCIE_CRESET_PERST_N       PPC_BIT(3)
 #define   PHB_PCIE_CRESET_PIPE_N        PPC_BIT(4)
-
-
+#define   PHB_PCIE_CRESET_REFCLK_N      PPC_BIT(8)
 #define PHB_PCIE_HOTPLUG_STATUS         0x1A20
 #define   PHB_PCIE_HPSTAT_PRESENCE      PPC_BIT(10)
 
diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
index 44141462f6..708df3867c 100644
--- a/tests/qtest/pnv-phb4-test.c
+++ b/tests/qtest/pnv-phb4-test.c
@@ -36,7 +36,12 @@ static uint64_t pnv_phb4_xscom_addr(uint32_t reg)
    - bit[51:61]: Indirect Address(00:10)
  * Read/write 'SCOM - HV Indirect Data Register' to get/set the value.
  */
-
+static void pnv_phb4_xscom_write(QTestState *qts, uint32_t reg, uint64_t val)
+{
+    qtest_writeq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_ADDR),
+            PPC_BIT(0) | reg);
+    qtest_writeq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_DATA), val);
+}
 static uint64_t pnv_phb4_xscom_read(QTestState *qts, uint32_t reg)
 {
     qtest_writeq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_ADDR),
@@ -61,6 +66,26 @@ static void phb4_reset_test(QTestState *qts)
                      ==, 0xC009000000000000);
 }
 
+/* Check sticky-reset */
+static void phb4_sticky_rst_test(QTestState *qts)
+{
+    uint64_t val;
+
+    /*
+     * Sticky reset test of PHB_PBL_ERR_STATUS.
+     *
+     * Write all 1's to reg PHB_PBL_ERR_INJECT.
+     * Updated value will be copied to reg PHB_PBL_ERR_STATUS.
+     *
+     * Reset PBL core by setting PHB_PCIE_CRESET_PBL in reg PHB_PCIE_CRESET.
+     * Verify the sticky bits are still set.
+     */
+    pnv_phb4_xscom_write(qts, PHB_PBL_ERR_INJECT, PPC_BITMASK(0, 63));
+    pnv_phb4_xscom_write(qts, PHB_PCIE_CRESET, PHB_PCIE_CRESET_PBL); /*Reset*/
+    val = pnv_phb4_xscom_read(qts, PHB_PBL_ERR_STATUS);
+    g_assert_cmpuint(val, ==, (PPC_BITMASK(0, 9) | PPC_BITMASK(12, 63)));
+}
+
 static void test_phb4(void)
 {
     QTestState *qts = NULL;
@@ -73,6 +98,9 @@ static void test_phb4(void)
     /* Check reset value of a register */
     phb4_reset_test(qts);
 
+    /* Check sticky reset of a register */
+    phb4_sticky_rst_test(qts);
+
     qtest_quit(qts);
 }
 
-- 
2.39.3



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

* [PATCH 04/10] pnv/phb4: Implement read-only and write-only bits of registers
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (2 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 03/10] pnv/phb4: Implement sticky reset logic in PHB4 Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 14:15   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 05/10] pnv/phb4: Implement write-clear and return 1's on unimplemented reg read Saif Abrar
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

SW cannot write the read-only(RO) bits of a register
and write-only(WO) bits of a register return 0 when read.

Added ro_mask[] for each register that defines which
bits in that register are RO.
When writing to a register, the RO-bits are not updated.

When reading a register, clear the WO bits and return the updated value.

Tested the registers PHB_DMA_SYNC, PHB_PCIE_HOTPLUG_STATUS, PHB_PCIE_LMR,
PHB_PCIE_DLP_TRWCTL, PHB_LEM_ERROR_AND_MASK and PHB_LEM_ERROR_OR_MASK
by writing all 1's and reading back the value.
The WO bits in these registers should read back as 0.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c              | 77 ++++++++++++++++++++++++++---
 include/hw/pci-host/pnv_phb4.h      |  7 +++
 include/hw/pci-host/pnv_phb4_regs.h | 19 +++++--
 tests/qtest/pnv-phb4-test.c         | 60 +++++++++++++++++++++-
 4 files changed, 150 insertions(+), 13 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index b3a83837f8..a81763f34c 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -735,6 +735,10 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
         return;
     }
 
+    /* Update 'val' according to the register's RO-mask */
+    val = (phb->regs[off >> 3] & phb->ro_mask[off >> 3]) |
+          (val & ~(phb->ro_mask[off >> 3]));
+
     /* Record whether it changed */
     changed = phb->regs[off >> 3] != val;
 
@@ -808,7 +812,7 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
     case PHB_TCE_TAG_ENABLE:
     case PHB_INT_NOTIFY_ADDR:
     case PHB_INT_NOTIFY_INDEX:
-    case PHB_DMARD_SYNC:
+    case PHB_DMA_SYNC:
        break;
 
     /* Noise on anything else */
@@ -846,7 +850,7 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
     case PHB_VERSION:
         return PNV_PHB4_PEC_GET_CLASS(phb->pec)->version;
 
-        /* Read-only */
+    /* Read-only */
     case PHB_PHB4_GEN_CAP:
         return 0xe4b8000000000000ull;
     case PHB_PHB4_TCE_CAP:
@@ -856,18 +860,49 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
     case PHB_PHB4_EEH_CAP:
         return phb->big_phb ? 0x2000000000000000ull : 0x1000000000000000ull;
 
+    /* Write-only, read will return zeros */
+    case PHB_LEM_ERROR_AND_MASK:
+    case PHB_LEM_ERROR_OR_MASK:
+        return 0;
+    case PHB_PCIE_DLP_TRWCTL:
+        val &= ~PHB_PCIE_DLP_TRWCTL_WREN;
+        return val;
     /* IODA table accesses */
     case PHB_IODA_DATA0:
         return pnv_phb4_ioda_read(phb);
 
+    /*
+     * DMA sync: make it look like it's complete,
+     *           clear write-only read/write start sync bits.
+     */
+    case PHB_DMA_SYNC:
+        val = PHB_DMA_SYNC_RD_COMPLETE |
+            ~(PHB_DMA_SYNC_RD_START | PHB_DMA_SYNC_WR_START);
+        return val;
+
+    /*
+     * PCI-E Stack registers
+     */
+    case PHB_PCIE_SCR:
+        val |= PHB_PCIE_SCR_PLW_X16; /* RO bit */
+        break;
+
     /* Link training always appears trained */
     case PHB_PCIE_DLP_TRAIN_CTL:
         /* TODO: Do something sensible with speed ? */
-        return PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
+        val |= PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
+        return val;
+
+    case PHB_PCIE_HOTPLUG_STATUS:
+        /* Clear write-only bit */
+        val &= ~PHB_PCIE_HPSTAT_RESAMPLE;
+        return val;
 
-    /* DMA read sync: make it look like it's complete */
-    case PHB_DMARD_SYNC:
-        return PHB_DMARD_SYNC_COMPLETE;
+    /* Link Management Register */
+    case PHB_PCIE_LMR:
+        /* These write-only bits always read as 0 */
+        val &= ~(PHB_PCIE_LMR_CHANGELW | PHB_PCIE_LMR_RETRAINLINK);
+        return val;
 
     /* Silent simple reads */
     case PHB_LSI_SOURCE_ID:
@@ -1712,6 +1747,33 @@ static PCIIOMMUOps pnv_phb4_iommu_ops = {
     .get_address_space = pnv_phb4_dma_iommu,
 };
 
+static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
+{
+    /* Clear RO-mask to make all regs as R/W by default */
+    memset(phb->ro_mask, 0x0, PNV_PHB4_NUM_REGS * sizeof(uint64_t));
+
+    /*
+     * Set register specific RO-masks
+     */
+
+    /* PBL - Error Injection Register (0x1910) */
+    phb->ro_mask[PHB_PBL_ERR_INJECT >> 3] =
+        PPC_BITMASK(0, 23) | PPC_BITMASK(28, 35) | PPC_BIT(38) | PPC_BIT(46) |
+        PPC_BITMASK(49, 51) | PPC_BITMASK(55, 63);
+
+    /* Reserved bits[60:63] */
+    phb->ro_mask[PHB_TXE_ERR_LEM_ENABLE >> 3] =
+    phb->ro_mask[PHB_TXE_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(60, 63);
+    /* Reserved bits[36:63] */
+    phb->ro_mask[PHB_RXE_TCE_ERR_LEM_ENABLE >> 3] =
+    phb->ro_mask[PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(36, 63);
+    /* Reserved bits[40:63] */
+    phb->ro_mask[PHB_ERR_LEM_ENABLE >> 3] =
+    phb->ro_mask[PHB_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(40, 63);
+
+    /* TODO: Add more RO-masks as regs are implemented in the model */
+}
+
 static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
 {
     STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
@@ -1782,6 +1844,9 @@ static void pnv_phb4_instance_init(Object *obj)
 
     /* XIVE interrupt source object */
     object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE);
+
+    /* Initialize RO-mask of registers */
+    pnv_phb4_ro_mask_init(phb);
 }
 
 void pnv_phb4_bus_init(DeviceState *dev, PnvPHB4 *phb)
diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 3212e68160..91e81eee0e 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -94,6 +94,13 @@ struct PnvPHB4 {
     uint64_t regs[PNV_PHB4_NUM_REGS];
     MemoryRegion mr_regs;
 
+    /*
+     * Read-only bitmask for registers
+     * Bit value: 1 => RO bit
+     *            0 => RW bit
+     */
+    uint64_t ro_mask[PNV_PHB4_NUM_REGS];
+
     /* Extra SCOM-only register */
     uint64_t scom_hv_ind_addr_reg;
 
diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
index df5e86d29a..391d6a89ea 100644
--- a/include/hw/pci-host/pnv_phb4_regs.h
+++ b/include/hw/pci-host/pnv_phb4_regs.h
@@ -180,9 +180,11 @@
 #define PHB_M64_AOMASK                  0x1d0
 #define PHB_M64_UPPER_BITS              0x1f0
 #define PHB_NXLATE_PREFIX               0x1f8
-#define PHB_DMARD_SYNC                  0x200
-#define   PHB_DMARD_SYNC_START          PPC_BIT(0)
-#define   PHB_DMARD_SYNC_COMPLETE       PPC_BIT(1)
+#define PHB_DMA_SYNC                    0x200
+#define   PHB_DMA_SYNC_RD_START         PPC_BIT(0)
+#define   PHB_DMA_SYNC_RD_COMPLETE      PPC_BIT(1)
+#define   PHB_DMA_SYNC_WR_START         PPC_BIT(2)
+#define   PHB_DMA_SYNC_WR_COMPLETE      PPC_BIT(3)
 #define PHB_RTC_INVALIDATE              0x208
 #define   PHB_RTC_INVALIDATE_ALL        PPC_BIT(0)
 #define   PHB_RTC_INVALIDATE_RID        PPC_BITMASK(16, 31)
@@ -395,8 +397,8 @@
 #define PHB_PCIE_SCR                    0x1A00
 #define   PHB_PCIE_SCR_SLOT_CAP         PPC_BIT(15)
 #define   PHB_PCIE_SCR_MAXLINKSPEED     PPC_BITMASK(32, 35)
+#define   PHB_PCIE_SCR_PLW_X16          PPC_BIT(41) /* x16 */
 #define PHB_PCIE_BNR                    0x1A08
-
 #define PHB_PCIE_CRESET                 0x1A10
 #define   PHB_PCIE_CRESET_CFG_CORE      PPC_BIT(0)
 #define   PHB_PCIE_CRESET_TLDLP         PPC_BIT(1)
@@ -405,7 +407,14 @@
 #define   PHB_PCIE_CRESET_PIPE_N        PPC_BIT(4)
 #define   PHB_PCIE_CRESET_REFCLK_N      PPC_BIT(8)
 #define PHB_PCIE_HOTPLUG_STATUS         0x1A20
+#define   PHB_PCIE_HPSTAT_SIMDIAG       PPC_BIT(3)
+#define   PHB_PCIE_HPSTAT_RESAMPLE      PPC_BIT(9)
 #define   PHB_PCIE_HPSTAT_PRESENCE      PPC_BIT(10)
+#define   PHB_PCIE_HPSTAT_LINKACTIVE    PPC_BIT(12)
+#define PHB_PCIE_LMR                    0x1A30
+#define   PHB_PCIE_LMR_CHANGELW         PPC_BIT(0)
+#define   PHB_PCIE_LMR_RETRAINLINK      PPC_BIT(1)
+#define   PHB_PCIE_LMR_LINKACTIVE       PPC_BIT(8)
 
 #define PHB_PCIE_DLP_TRAIN_CTL          0x1A40
 #define   PHB_PCIE_DLP_LINK_WIDTH       PPC_BITMASK(30, 35)
@@ -433,7 +442,7 @@
 
 #define PHB_PCIE_DLP_TRWCTL             0x1A80
 #define   PHB_PCIE_DLP_TRWCTL_EN        PPC_BIT(0)
-
+#define   PHB_PCIE_DLP_TRWCTL_WREN      PPC_BIT(1)
 #define PHB_PCIE_DLP_ERRLOG1            0x1AA0
 #define PHB_PCIE_DLP_ERRLOG2            0x1AA8
 #define PHB_PCIE_DLP_ERR_STATUS         0x1AB0
diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
index 708df3867c..0c8e58dd5f 100644
--- a/tests/qtest/pnv-phb4-test.c
+++ b/tests/qtest/pnv-phb4-test.c
@@ -75,7 +75,8 @@ static void phb4_sticky_rst_test(QTestState *qts)
      * Sticky reset test of PHB_PBL_ERR_STATUS.
      *
      * Write all 1's to reg PHB_PBL_ERR_INJECT.
-     * Updated value will be copied to reg PHB_PBL_ERR_STATUS.
+     * RO-only bits will not be written and
+     * updated value will be copied to reg PHB_PBL_ERR_STATUS.
      *
      * Reset PBL core by setting PHB_PCIE_CRESET_PBL in reg PHB_PCIE_CRESET.
      * Verify the sticky bits are still set.
@@ -83,7 +84,59 @@ static void phb4_sticky_rst_test(QTestState *qts)
     pnv_phb4_xscom_write(qts, PHB_PBL_ERR_INJECT, PPC_BITMASK(0, 63));
     pnv_phb4_xscom_write(qts, PHB_PCIE_CRESET, PHB_PCIE_CRESET_PBL); /*Reset*/
     val = pnv_phb4_xscom_read(qts, PHB_PBL_ERR_STATUS);
-    g_assert_cmpuint(val, ==, (PPC_BITMASK(0, 9) | PPC_BITMASK(12, 63)));
+    g_assert_cmpuint(val, ==, 0xF00DFD8E00);
+}
+
+/* Check that write-only bits/regs return 0 when read */
+static void phb4_writeonly_read_test(QTestState *qts)
+{
+    uint64_t val;
+
+    /*
+     * Set all bits of PHB_DMA_SYNC,
+     * bits 0 and 2 are write-only and should be read as 0.
+     */
+    pnv_phb4_xscom_write(qts, PHB_DMA_SYNC, PPC_BITMASK(0, 63));
+    val = pnv_phb4_xscom_read(qts, PHB_DMA_SYNC);
+    g_assert_cmpuint(val & PPC_BIT(0), ==, 0x0);
+    g_assert_cmpuint(val & PPC_BIT(2), ==, 0x0);
+
+    /*
+     * Set all bits of PHB_PCIE_HOTPLUG_STATUS,
+     * bit 9 is write-only and should be read as 0.
+     */
+    pnv_phb4_xscom_write(qts, PHB_PCIE_HOTPLUG_STATUS, PPC_BITMASK(0, 63));
+    val = pnv_phb4_xscom_read(qts, PHB_PCIE_HOTPLUG_STATUS);
+    g_assert_cmpuint(val & PPC_BIT(9), ==, 0x0);
+
+    /*
+     * Set all bits of PHB_PCIE_LMR,
+     * bits 0 and 1 are write-only and should be read as 0.
+     */
+    pnv_phb4_xscom_write(qts, PHB_PCIE_LMR, PPC_BITMASK(0, 63));
+    val = pnv_phb4_xscom_read(qts, PHB_PCIE_LMR);
+    g_assert_cmpuint(val & PPC_BIT(0), ==, 0x0);
+    g_assert_cmpuint(val & PPC_BIT(1), ==, 0x0);
+
+    /*
+     * Set all bits of PHB_PCIE_DLP_TRWCTL,
+     * write-only bit-1 should be read as 0.
+     */
+    pnv_phb4_xscom_write(qts, PHB_PCIE_DLP_TRWCTL, PPC_BITMASK(0, 63));
+    val = pnv_phb4_xscom_read(qts, PHB_PCIE_DLP_TRWCTL);
+    g_assert_cmpuint(val & PPC_BIT(1), ==, 0x0);
+
+    /*
+     * Set all bits of PHB_LEM_ERROR_AND_MASK, PHB_LEM_ERROR_OR_MASK,
+     * both regs are write-only and should be read as 0.
+     */
+    pnv_phb4_xscom_write(qts, PHB_LEM_ERROR_AND_MASK, PPC_BITMASK(0, 63));
+    val = pnv_phb4_xscom_read(qts, PHB_LEM_ERROR_AND_MASK);
+    g_assert_cmpuint(val, ==, 0x0);
+
+    pnv_phb4_xscom_write(qts, PHB_LEM_ERROR_OR_MASK, PPC_BITMASK(0, 63));
+    val = pnv_phb4_xscom_read(qts, PHB_LEM_ERROR_OR_MASK);
+    g_assert_cmpuint(val, ==, 0x0);
 }
 
 static void test_phb4(void)
@@ -101,6 +154,9 @@ static void test_phb4(void)
     /* Check sticky reset of a register */
     phb4_sticky_rst_test(qts);
 
+    /* Check write-only logic */
+    phb4_writeonly_read_test(qts);
+
     qtest_quit(qts);
 }
 
-- 
2.39.3



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

* [PATCH 05/10] pnv/phb4: Implement write-clear and return 1's on unimplemented reg read
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (3 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 04/10] pnv/phb4: Implement read-only and write-only bits of registers Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 13:58   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 06/10] pnv/phb4: Set link-active status in HPSTAT and LMR registers Saif Abrar
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Implement write-1-to-clear and write-X-to-clear logic.
Update registers with silent simple read and write.
Return all 1's when an unimplemented/reserved register is read.

Test that reading address 0x0 returns all 1's (i.e. -1).

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c              | 190 ++++++++++++++++++++++------
 include/hw/pci-host/pnv_phb4_regs.h |  12 +-
 tests/qtest/pnv-phb4-test.c         |   9 ++
 3 files changed, 170 insertions(+), 41 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index a81763f34c..4e3a6b37f9 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -683,8 +683,41 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
         return;
     }
 
-    /* Handle masking */
+    /* Handle RO, W1C, WxC and masking */
     switch (off) {
+    /* W1C: Write-1-to-Clear registers */
+    case PHB_TXE_ERR_STATUS:
+    case PHB_RXE_ARB_ERR_STATUS:
+    case PHB_RXE_MRG_ERR_STATUS:
+    case PHB_RXE_TCE_ERR_STATUS:
+    case PHB_ERR_STATUS:
+    case PHB_REGB_ERR_STATUS:
+    case PHB_PCIE_DLP_ERRLOG1:
+    case PHB_PCIE_DLP_ERRLOG2:
+    case PHB_PCIE_DLP_ERR_STATUS:
+    case PHB_PBL_ERR_STATUS:
+        phb->regs[off >> 3] &= ~val;
+        return;
+
+    /* WxC: Clear register on any write */
+    case PHB_PBL_ERR1_STATUS:
+    case PHB_PBL_ERR_LOG_0 ... PHB_PBL_ERR_LOG_1:
+    case PHB_REGB_ERR1_STATUS:
+    case PHB_REGB_ERR_LOG_0 ... PHB_REGB_ERR_LOG_1:
+    case PHB_TXE_ERR1_STATUS:
+    case PHB_TXE_ERR_LOG_0 ... PHB_TXE_ERR_LOG_1:
+    case PHB_RXE_ARB_ERR1_STATUS:
+    case PHB_RXE_ARB_ERR_LOG_0 ... PHB_RXE_ARB_ERR_LOG_1:
+    case PHB_RXE_MRG_ERR1_STATUS:
+    case PHB_RXE_MRG_ERR_LOG_0 ... PHB_RXE_MRG_ERR_LOG_1:
+    case PHB_RXE_TCE_ERR1_STATUS:
+    case PHB_RXE_TCE_ERR_LOG_0 ... PHB_RXE_TCE_ERR_LOG_1:
+    case PHB_ERR1_STATUS:
+    case PHB_ERR_LOG_0 ... PHB_ERR_LOG_1:
+        phb->regs[off >> 3] = 0;
+        return;
+
+    /* Write value updated by masks */
     case PHB_LSI_SOURCE_ID:
         val &= PHB_LSI_SRC_ID;
         break;
@@ -723,7 +756,6 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
     case PHB_LEM_WOF:
         val = 0;
         break;
-    /* TODO: More regs ..., maybe create a table with masks... */
 
     /* Read only registers */
     case PHB_CPU_LOADSTORE_STATUS:
@@ -732,6 +764,12 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
     case PHB_PHB4_TCE_CAP:
     case PHB_PHB4_IRQ_CAP:
     case PHB_PHB4_EEH_CAP:
+    case PHB_VERSION:
+    case PHB_DMA_CHAN_STATUS:
+    case PHB_TCE_TAG_STATUS:
+    case PHB_PBL_BUF_STATUS:
+    case PHB_PCIE_BNR:
+    case PHB_PCIE_PHY_RXEQ_STAT_G3_00_03 ... PHB_PCIE_PHY_RXEQ_STAT_G5_12_15:
         return;
     }
 
@@ -752,6 +790,7 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
             pnv_phb4_update_all_msi_regions(phb);
         }
         break;
+
     case PHB_M32_START_ADDR:
     case PHB_M64_UPPER_BITS:
         if (changed) {
@@ -797,27 +836,63 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
         break;
 
     /* Silent simple writes */
-    case PHB_ASN_CMPM:
-    case PHB_CONFIG_ADDRESS:
-    case PHB_IODA_ADDR:
-    case PHB_TCE_KILL:
-    case PHB_TCE_SPEC_CTL:
-    case PHB_PEST_BAR:
-    case PHB_PELTV_BAR:
+    /* PHB Fundamental register set A */
+    case PHB_CONFIG_DATA ... PHB_LOCK1:
     case PHB_RTT_BAR:
-    case PHB_LEM_FIR_ACCUM:
-    case PHB_LEM_ERROR_MASK:
-    case PHB_LEM_ACTION0:
-    case PHB_LEM_ACTION1:
-    case PHB_TCE_TAG_ENABLE:
+    case PHB_PELTV_BAR:
+    case PHB_PEST_BAR:
+    case PHB_CAPI_CMPM ... PHB_M64_AOMASK:
+    case PHB_NXLATE_PREFIX ... PHB_DMA_SYNC:
+    case PHB_TCE_KILL ... PHB_IODA_ADDR:
+    case PHB_PAPR_ERR_INJ_CTL ... PHB_PAPR_ERR_INJ_MASK:
     case PHB_INT_NOTIFY_ADDR:
     case PHB_INT_NOTIFY_INDEX:
-    case PHB_DMA_SYNC:
-       break;
+    /* Fundamental register set B */
+    case PHB_AIB_FENCE_CTRL ... PHB_Q_DMA_R:
+    /* FIR & Error registers */
+    case PHB_LEM_FIR_ACCUM:
+    case PHB_LEM_ERROR_MASK:
+    case PHB_LEM_ACTION0 ... PHB_LEM_WOF:
+    case PHB_ERR_INJECT ... PHB_ERR_AIB_FENCE_ENABLE:
+    case PHB_ERR_STATUS_MASK ... PHB_ERR1_STATUS_MASK:
+    case PHB_TXE_ERR_INJECT ... PHB_TXE_ERR_AIB_FENCE_ENABLE:
+    case PHB_TXE_ERR_STATUS_MASK ... PHB_TXE_ERR1_STATUS_MASK:
+    case PHB_RXE_ARB_ERR_INJECT ... PHB_RXE_ARB_ERR_AIB_FENCE_ENABLE:
+    case PHB_RXE_ARB_ERR_STATUS_MASK ... PHB_RXE_ARB_ERR1_STATUS_MASK:
+    case PHB_RXE_MRG_ERR_INJECT ... PHB_RXE_MRG_ERR_AIB_FENCE_ENABLE:
+    case PHB_RXE_MRG_ERR_STATUS_MASK ... PHB_RXE_MRG_ERR1_STATUS_MASK:
+    case PHB_RXE_TCE_ERR_INJECT ... PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE:
+    case PHB_RXE_TCE_ERR_STATUS_MASK ... PHB_RXE_TCE_ERR1_STATUS_MASK:
+    /* Performance monitor & Debug registers */
+    case PHB_TRACE_CONTROL ... PHB_PERFMON_CTR1:
+    /* REGB Registers */
+    /* PBL core */
+    case PHB_PBL_CONTROL:
+    case PHB_PBL_TIMEOUT_CTRL:
+    case PHB_PBL_NPTAG_ENABLE:
+    case PHB_PBL_SYS_LINK_INIT:
+    case PHB_PBL_ERR_INF_ENABLE ... PHB_PBL_ERR_FAT_ENABLE:
+    case PHB_PBL_ERR_STATUS_MASK ... PHB_PBL_ERR1_STATUS_MASK:
+    /* PCI-E stack */
+    case PHB_PCIE_SCR:
+    case PHB_PCIE_DLP_STR ... PHB_PCIE_HOTPLUG_STATUS:
+    case PHB_PCIE_LMR ... PHB_PCIE_DLP_LSR:
+    case PHB_PCIE_DLP_RXMGN:
+    case PHB_PCIE_DLP_LANEZEROCTL ... PHB_PCIE_DLP_TRCRDDATA:
+    case PHB_PCIE_DLP_ERR_COUNTERS:
+    case PHB_PCIE_DLP_EIC ...   PHB_PCIE_LANE_EQ_CNTL23:
+    case PHB_PCIE_TRACE_CTRL:
+    case PHB_PCIE_MISC_STRAP ... PHB_PCIE_PHY_EQ_CTL:
+    /* Error registers */
+    case PHB_REGB_ERR_INJECT:
+    case PHB_REGB_ERR_INF_ENABLE ... PHB_REGB_ERR_FAT_ENABLE:
+    case PHB_REGB_ERR_STATUS_MASK ... PHB_REGB_ERR1_STATUS_MASK:
+        break;
 
     /* Noise on anything else */
     default:
-        qemu_log_mask(LOG_UNIMP, "phb4: reg_write 0x%"PRIx64"=%"PRIx64"\n",
+        qemu_log_mask(LOG_UNIMP,
+                      "phb4: unimplemented reg_write 0x%"PRIx64"=%"PRIx64"\n",
                       off, val);
     }
 }
@@ -905,36 +980,75 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
         return val;
 
     /* Silent simple reads */
+    /* PHB Fundamental register set A */
     case PHB_LSI_SOURCE_ID:
+    case PHB_DMA_CHAN_STATUS:
     case PHB_CPU_LOADSTORE_STATUS:
-    case PHB_ASN_CMPM:
+    case PHB_CONFIG_DATA ... PHB_LOCK1:
     case PHB_PHB4_CONFIG:
+    case PHB_RTT_BAR:
+    case PHB_PELTV_BAR:
     case PHB_M32_START_ADDR:
-    case PHB_CONFIG_ADDRESS:
-    case PHB_IODA_ADDR:
-    case PHB_RTC_INVALIDATE:
-    case PHB_TCE_KILL:
-    case PHB_TCE_SPEC_CTL:
     case PHB_PEST_BAR:
-    case PHB_PELTV_BAR:
-    case PHB_RTT_BAR:
+    case PHB_CAPI_CMPM:
+    case PHB_M64_AOMASK:
     case PHB_M64_UPPER_BITS:
-    case PHB_CTRLR:
-    case PHB_LEM_FIR_ACCUM:
-    case PHB_LEM_ERROR_MASK:
-    case PHB_LEM_ACTION0:
-    case PHB_LEM_ACTION1:
-    case PHB_TCE_TAG_ENABLE:
+    case PHB_NXLATE_PREFIX:
+    case PHB_RTC_INVALIDATE ... PHB_IODA_ADDR:
+    case PHB_PAPR_ERR_INJ_CTL ... PHB_ETU_ERR_SUMMARY:
     case PHB_INT_NOTIFY_ADDR:
     case PHB_INT_NOTIFY_INDEX:
-    case PHB_Q_DMA_R:
-    case PHB_ETU_ERR_SUMMARY:
-        break;
-
-    /* Noise on anything else */
+    /* Fundamental register set B */
+    case PHB_CTRLR:
+    case PHB_AIB_FENCE_CTRL ... PHB_Q_DMA_R:
+    case PHB_TCE_TAG_STATUS:
+    /* FIR & Error registers */
+    case PHB_LEM_FIR_ACCUM ... PHB_LEM_ERROR_MASK:
+    case PHB_LEM_ACTION0 ... PHB_LEM_WOF:
+    case PHB_ERR_STATUS ... PHB_ERR_AIB_FENCE_ENABLE:
+    case PHB_ERR_LOG_0 ... PHB_ERR1_STATUS_MASK:
+    case PHB_TXE_ERR_STATUS ... PHB_TXE_ERR_AIB_FENCE_ENABLE:
+    case PHB_TXE_ERR_LOG_0 ... PHB_TXE_ERR1_STATUS_MASK:
+    case PHB_RXE_ARB_ERR_STATUS ... PHB_RXE_ARB_ERR_AIB_FENCE_ENABLE:
+    case PHB_RXE_ARB_ERR_LOG_0 ... PHB_RXE_ARB_ERR1_STATUS_MASK:
+    case PHB_RXE_MRG_ERR_STATUS ... PHB_RXE_MRG_ERR_AIB_FENCE_ENABLE:
+    case PHB_RXE_MRG_ERR_LOG_0 ... PHB_RXE_MRG_ERR1_STATUS_MASK:
+    case PHB_RXE_TCE_ERR_STATUS ... PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE:
+    case PHB_RXE_TCE_ERR_LOG_0 ... PHB_RXE_TCE_ERR1_STATUS_MASK:
+    /* Performance monitor & Debug registers */
+    case PHB_TRACE_CONTROL ... PHB_PERFMON_CTR1:
+    /* REGB Registers */
+    /* PBL core */
+    case PHB_PBL_CONTROL:
+    case PHB_PBL_TIMEOUT_CTRL:
+    case PHB_PBL_NPTAG_ENABLE:
+    case PHB_PBL_SYS_LINK_INIT:
+    case PHB_PBL_BUF_STATUS:
+    case PHB_PBL_ERR_STATUS ... PHB_PBL_ERR_INJECT:
+    case PHB_PBL_ERR_INF_ENABLE ... PHB_PBL_ERR_FAT_ENABLE:
+    case PHB_PBL_ERR_LOG_0 ... PHB_PBL_ERR1_STATUS_MASK:
+    /* PCI-E stack */
+    case PHB_PCIE_BNR ... PHB_PCIE_DLP_STR:
+    case PHB_PCIE_DLP_LANE_PWR:
+    case PHB_PCIE_DLP_LSR:
+    case PHB_PCIE_DLP_RXMGN:
+    case PHB_PCIE_DLP_LANEZEROCTL ... PHB_PCIE_DLP_CTL:
+    case PHB_PCIE_DLP_TRCRDDATA:
+    case PHB_PCIE_DLP_ERRLOG1 ... PHB_PCIE_DLP_ERR_COUNTERS:
+    case PHB_PCIE_DLP_EIC ...   PHB_PCIE_LANE_EQ_CNTL23:
+    case PHB_PCIE_TRACE_CTRL:
+    case PHB_PCIE_MISC_STRAP ... PHB_PCIE_PHY_RXEQ_STAT_G5_12_15:
+    /* Error registers */
+    case PHB_REGB_ERR_STATUS ... PHB_REGB_ERR_INJECT:
+    case PHB_REGB_ERR_INF_ENABLE ... PHB_REGB_ERR_FAT_ENABLE:
+    case PHB_REGB_ERR_LOG_0 ... PHB_REGB_ERR1_STATUS_MASK:
+        break;
+
+    /* Noise on unimplemented read, return all 1's */
     default:
-        qemu_log_mask(LOG_UNIMP, "phb4: reg_read 0x%"PRIx64"=%"PRIx64"\n",
-                      off, val);
+        qemu_log_mask(LOG_UNIMP, "phb4: unimplemented reg_read 0x%"PRIx64"\n",
+                      off);
+        val = ~0ull;
     }
     return val;
 }
diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
index 391d6a89ea..c1d5a83271 100644
--- a/include/hw/pci-host/pnv_phb4_regs.h
+++ b/include/hw/pci-host/pnv_phb4_regs.h
@@ -372,6 +372,7 @@
 #define P32_CAP                                 0x228
 #define P32_CTL                                 0x22C
 #define P32_STAT                                0x230
+
 /* PHB4 REGB registers */
 
 /* PBL core */
@@ -406,6 +407,7 @@
 #define   PHB_PCIE_CRESET_PERST_N       PPC_BIT(3)
 #define   PHB_PCIE_CRESET_PIPE_N        PPC_BIT(4)
 #define   PHB_PCIE_CRESET_REFCLK_N      PPC_BIT(8)
+#define PHB_PCIE_DLP_STR                0x1A18
 #define PHB_PCIE_HOTPLUG_STATUS         0x1A20
 #define   PHB_PCIE_HPSTAT_SIMDIAG       PPC_BIT(3)
 #define   PHB_PCIE_HPSTAT_RESAMPLE      PPC_BIT(9)
@@ -416,6 +418,7 @@
 #define   PHB_PCIE_LMR_RETRAINLINK      PPC_BIT(1)
 #define   PHB_PCIE_LMR_LINKACTIVE       PPC_BIT(8)
 
+#define PHB_PCIE_DLP_LANE_PWR           0x1A38
 #define PHB_PCIE_DLP_TRAIN_CTL          0x1A40
 #define   PHB_PCIE_DLP_LINK_WIDTH       PPC_BITMASK(30, 35)
 #define   PHB_PCIE_DLP_LINK_SPEED       PPC_BITMASK(36, 39)
@@ -435,18 +438,21 @@
 #define   PHB_PCIE_DLP_DL_PGRESET       PPC_BIT(22)
 #define   PHB_PCIE_DLP_TRAINING         PPC_BIT(20)
 #define   PHB_PCIE_DLP_INBAND_PRESENCE  PPC_BIT(19)
-
+#define PHB_PCIE_DLP_LSR                0x1A48
+#define PHB_PCIE_DLP_RXMGN              0x1A50
+#define PHB_PCIE_DLP_LANEZEROCTL        0x1A70
 #define PHB_PCIE_DLP_CTL                0x1A78
 #define   PHB_PCIE_DLP_CTL_BYPASS_PH2   PPC_BIT(4)
 #define   PHB_PCIE_DLP_CTL_BYPASS_PH3   PPC_BIT(5)
-
 #define PHB_PCIE_DLP_TRWCTL             0x1A80
 #define   PHB_PCIE_DLP_TRWCTL_EN        PPC_BIT(0)
 #define   PHB_PCIE_DLP_TRWCTL_WREN      PPC_BIT(1)
+#define PHB_PCIE_DLP_TRCRDDATA          0x1A88
 #define PHB_PCIE_DLP_ERRLOG1            0x1AA0
 #define PHB_PCIE_DLP_ERRLOG2            0x1AA8
 #define PHB_PCIE_DLP_ERR_STATUS         0x1AB0
 #define PHB_PCIE_DLP_ERR_COUNTERS       0x1AB8
+#define PHB_PCIE_DLP_EIC                0x1AC8
 
 #define PHB_PCIE_LANE_EQ_CNTL0          0x1AD0
 #define PHB_PCIE_LANE_EQ_CNTL1          0x1AD8
@@ -458,6 +464,7 @@
 #define PHB_PCIE_LANE_EQ_CNTL23         0x1B08 /* DD1 only */
 #define PHB_PCIE_TRACE_CTRL             0x1B20
 #define PHB_PCIE_MISC_STRAP             0x1B30
+#define PHB_PCIE_PHY_EQ_CTL             0x1B38
 #define PHB_PCIE_PHY_RXEQ_STAT_G3_00_03 0x1B40
 #define PHB_PCIE_PHY_RXEQ_STAT_G5_12_15 0x1B98
 
@@ -591,5 +598,4 @@
 
 #define IODA3_PEST1_FAIL_ADDR           PPC_BITMASK(3, 63)
 
-
 #endif /* PCI_HOST_PNV_PHB4_REGS_H */
diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
index 0c8e58dd5f..96d1bd6724 100644
--- a/tests/qtest/pnv-phb4-test.c
+++ b/tests/qtest/pnv-phb4-test.c
@@ -139,6 +139,12 @@ static void phb4_writeonly_read_test(QTestState *qts)
     g_assert_cmpuint(val, ==, 0x0);
 }
 
+/* Check that reading an unimplemented address 0x0 returns -1 */
+static void phb4_unimplemented_read_test(QTestState *qts)
+{
+    g_assert_cmpint(pnv_phb4_xscom_read(qts, 0x0), ==, -1);
+}
+
 static void test_phb4(void)
 {
     QTestState *qts = NULL;
@@ -157,6 +163,9 @@ static void test_phb4(void)
     /* Check write-only logic */
     phb4_writeonly_read_test(qts);
 
+    /* Check unimplemented register read */
+    phb4_unimplemented_read_test(qts);
+
     qtest_quit(qts);
 }
 
-- 
2.39.3



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

* [PATCH 06/10] pnv/phb4: Set link-active status in HPSTAT and LMR registers
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (4 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 05/10] pnv/phb4: Implement write-clear and return 1's on unimplemented reg read Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-21 10:04 ` [PATCH 07/10] pnv/phb4: Set link speed and width in the DLP training control register Saif Abrar
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Config-read the link-status register in the PCI-E macro,
Depending on the link-active bit, set the link-active status
in the HOTPLUG_STATUS and LINK_MANAGEMENT registers
Also, clear the Presence-status active low bit in HOTPLUG_STATUS reg
after config-reading the slot-status in the PCI-E macro.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c | 57 +++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 56 insertions(+), 1 deletion(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 4e3a6b37f9..7b3d75bae6 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -516,6 +516,19 @@ static uint32_t get_exp_offset(PnvPHB4 *phb)
     return rpc->exp_offset;
 }
 
+/*
+ * Config-read the link-status register in the PCI-E macro,
+ * convert to LE and check the link-active bit.
+ */
+static uint32_t is_link_active(PnvPHB4 *phb)
+{
+    uint32_t exp_offset = get_exp_offset(phb);
+
+    return (bswap32(pnv_phb4_rc_config_read(phb,
+                        exp_offset + PCI_EXP_LNKSTA, 4))
+            & PCI_EXP_LNKSTA_DLLLA);
+}
+
 #define RC_CONFIG_WRITE(a, v) pnv_phb4_rc_config_write(phb, a, 4, v)
 
 /*
@@ -757,6 +770,11 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
         val = 0;
         break;
 
+    case PHB_PCIE_HOTPLUG_STATUS:
+        /* For normal operations, Simspeed diagnostic bit is always zero */
+        val &= PHB_PCIE_HPSTAT_SIMDIAG;
+        break;
+
     /* Read only registers */
     case PHB_CPU_LOADSTORE_STATUS:
     case PHB_ETU_ERR_SUMMARY:
@@ -968,8 +986,40 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
         val |= PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
         return val;
 
+    /*
+     * Read PCI-E registers and set status for:
+     * - Card present (active low bit 10)
+     * - Link active  (bit 12)
+     */
     case PHB_PCIE_HOTPLUG_STATUS:
-        /* Clear write-only bit */
+        /*
+         * Presence-status bit hpi_present_n is active-low, with reset value 1.
+         * Start by setting this bit to 1, indicating the card is not present.
+         * Then check the PCI-E register and clear the bit if card is present.
+         */
+        val |= PHB_PCIE_HPSTAT_PRESENCE;
+
+        /* Get the PCI-E capability offset from the root-port */
+        uint32_t exp_base = get_exp_offset(phb);
+
+        /*
+         * Config-read the PCI-E macro register for slot-status.
+         * Method for config-read converts to BE value.
+         * To check actual bit in the PCI-E register,
+         * convert the value back to LE using bswap32().
+         * Clear the Presence-status active low bit.
+         */
+        if (bswap32(pnv_phb4_rc_config_read(phb, exp_base + PCI_EXP_SLTSTA, 4))
+                    & PCI_EXP_SLTSTA_PDS) {
+            val &= ~PHB_PCIE_HPSTAT_PRESENCE;
+        }
+
+        /* Check if link is active and set the bit */
+        if (is_link_active(phb)) {
+            val |= PHB_PCIE_HPSTAT_LINKACTIVE;
+        }
+
+        /* Clear write-only resample-bit */
         val &= ~PHB_PCIE_HPSTAT_RESAMPLE;
         return val;
 
@@ -977,6 +1027,11 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
     case PHB_PCIE_LMR:
         /* These write-only bits always read as 0 */
         val &= ~(PHB_PCIE_LMR_CHANGELW | PHB_PCIE_LMR_RETRAINLINK);
+
+        /* Check if link is active and set the bit */
+        if (is_link_active(phb)) {
+            val |= PHB_PCIE_LMR_LINKACTIVE;
+        }
         return val;
 
     /* Silent simple reads */
-- 
2.39.3



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

* [PATCH 07/10] pnv/phb4: Set link speed and width in the DLP training control register
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (5 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 06/10] pnv/phb4: Set link-active status in HPSTAT and LMR registers Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 13:36   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 08/10] pnv/phb4: Implement IODA PCT table Saif Abrar
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Get the current link-status from PCIE macro.
Extract link-speed and link-width from the link-status
and set in the DLP training control (PCIE_DLP_TCR) register.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c | 21 +++++++++++++++++++--
 1 file changed, 19 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 7b3d75bae6..6823ffab54 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -980,10 +980,27 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
         val |= PHB_PCIE_SCR_PLW_X16; /* RO bit */
         break;
 
-    /* Link training always appears trained */
     case PHB_PCIE_DLP_TRAIN_CTL:
-        /* TODO: Do something sensible with speed ? */
+        /* Link training always appears trained */
         val |= PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
+
+        /* Get the current link-status from PCIE */
+        uint32_t exp_offset = get_exp_offset(phb);
+        uint32_t lnkstatus = bswap32(pnv_phb4_rc_config_read(phb,
+                                exp_offset + PCI_EXP_LNKSTA, 4));
+
+        /* Extract link-speed from the link-status */
+        uint32_t v = lnkstatus & PCI_EXP_LNKSTA_CLS;
+        /* Set the current link-speed at the LINK_SPEED position */
+        val = SETFIELD(PHB_PCIE_DLP_LINK_SPEED, val, v);
+
+        /*
+         * Extract link-width from the link-status,
+         * after shifting the required bitfields.
+         */
+        v = (lnkstatus & PCI_EXP_LNKSTA_NLW) >> PCI_EXP_LNKSTA_NLW_SHIFT;
+        /* Set the current link-width at the LINK_WIDTH position */
+        val = SETFIELD(PHB_PCIE_DLP_LINK_WIDTH, val, v);
         return val;
 
     /*
-- 
2.39.3



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

* [PATCH 08/10] pnv/phb4: Implement IODA PCT table
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (6 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 07/10] pnv/phb4: Set link speed and width in the DLP training control register Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 13:35   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register Saif Abrar
  2024-03-21 10:04 ` [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts Saif Abrar
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

IODA PCT table (#3) is implemented
without any functionality, being a debug table.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c              | 6 ++++++
 include/hw/pci-host/pnv_phb4.h      | 2 ++
 include/hw/pci-host/pnv_phb4_regs.h | 1 +
 3 files changed, 9 insertions(+)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index 6823ffab54..f48750ee54 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -263,6 +263,10 @@ static uint64_t *pnv_phb4_ioda_access(PnvPHB4 *phb,
         mask = phb->big_phb ? PNV_PHB4_MAX_MIST : (PNV_PHB4_MAX_MIST >> 1);
         mask -= 1;
         break;
+    case IODA3_TBL_PCT:
+        tptr = phb->ioda_PCT;
+        mask = 7;
+        break;
     case IODA3_TBL_RCAM:
         mask = phb->big_phb ? 127 : 63;
         break;
@@ -361,6 +365,8 @@ static void pnv_phb4_ioda_write(PnvPHB4 *phb, uint64_t val)
     /* Handle side effects */
     switch (table) {
     case IODA3_TBL_LIST:
+    case IODA3_TBL_PCT:
+        /* No action for debug tables */
         break;
     case IODA3_TBL_MIST: {
         /* Special mask for MIST partial write */
diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 91e81eee0e..6d83e5616f 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -64,6 +64,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB4, PNV_PHB4)
 #define PNV_PHB4_MAX_LSIs          8
 #define PNV_PHB4_MAX_INTs          4096
 #define PNV_PHB4_MAX_MIST          (PNV_PHB4_MAX_INTs >> 2)
+#define PNV_PHB4_MAX_PCT           128
 #define PNV_PHB4_MAX_MMIO_WINDOWS  32
 #define PNV_PHB4_MIN_MMIO_WINDOWS  16
 #define PNV_PHB4_NUM_REGS          (0x3000 >> 3)
@@ -144,6 +145,7 @@ struct PnvPHB4 {
     /* On-chip IODA tables */
     uint64_t ioda_LIST[PNV_PHB4_MAX_LSIs];
     uint64_t ioda_MIST[PNV_PHB4_MAX_MIST];
+    uint64_t ioda_PCT[PNV_PHB4_MAX_PCT];
     uint64_t ioda_TVT[PNV_PHB4_MAX_TVEs];
     uint64_t ioda_MBT[PNV_PHB4_MAX_MBEs];
     uint64_t ioda_MDT[PNV_PHB4_MAX_PEs];
diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
index c1d5a83271..e30adff7b2 100644
--- a/include/hw/pci-host/pnv_phb4_regs.h
+++ b/include/hw/pci-host/pnv_phb4_regs.h
@@ -486,6 +486,7 @@
 
 #define IODA3_TBL_LIST          1
 #define IODA3_TBL_MIST          2
+#define IODA3_TBL_PCT           3
 #define IODA3_TBL_RCAM          5
 #define IODA3_TBL_MRT           6
 #define IODA3_TBL_PESTA         7
-- 
2.39.3



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

* [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (7 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 08/10] pnv/phb4: Implement IODA PCT table Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 13:35   ` Cédric Le Goater
  2024-03-21 10:04 ` [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts Saif Abrar
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

PHB updates the register PCIE Link-Control-2.
Set the write-mask bits for TLS, ENTER_COMP, TX_MARGIN,
HASD, MOD_COMP, COMP_SOS and COMP_P_DE.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci/pcie.c                             | 6 ++++++
 include/standard-headers/linux/pci_regs.h | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index 4b2f0805c6..e3081f6b84 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -212,6 +212,12 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
 
     pci_set_word(dev->wmask + pos + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_EETLPPB);
 
+    pci_set_word(dev->wmask + pos + PCI_EXP_LNKCTL2,
+            PCI_EXP_LNKCTL2_TLS | PCI_EXP_LNKCTL2_ENTER_COMP |
+            PCI_EXP_LNKCTL2_TX_MARGIN | PCI_EXP_LNKCTL2_HASD |
+            PCI_EXP_LNKCTL2_MOD_COMP | PCI_EXP_LNKCTL2_COMP_SOS |
+            PCI_EXP_LNKCTL2_COMP_P_DE);
+
     if (dev->cap_present & QEMU_PCIE_EXTCAP_INIT) {
         /* read-only to behave like a 'NULL' Extended Capability Header */
         pci_set_long(dev->wmask + PCI_CONFIG_SPACE_SIZE, 0);
diff --git a/include/standard-headers/linux/pci_regs.h b/include/standard-headers/linux/pci_regs.h
index a39193213f..f743defe91 100644
--- a/include/standard-headers/linux/pci_regs.h
+++ b/include/standard-headers/linux/pci_regs.h
@@ -694,6 +694,9 @@
 #define  PCI_EXP_LNKCTL2_ENTER_COMP	0x0010 /* Enter Compliance */
 #define  PCI_EXP_LNKCTL2_TX_MARGIN	0x0380 /* Transmit Margin */
 #define  PCI_EXP_LNKCTL2_HASD		0x0020 /* HW Autonomous Speed Disable */
+#define  PCI_EXP_LNKCTL2_MOD_COMP	0x0400 /* Enter Modified Compliance */
+#define  PCI_EXP_LNKCTL2_COMP_SOS	0x0800 /* Compliance SOS */
+#define  PCI_EXP_LNKCTL2_COMP_P_DE	0xF000 /* Compliance Preset/De-emphasis */
 #define PCI_EXP_LNKSTA2		0x32	/* Link Status 2 */
 #define  PCI_EXP_LNKSTA2_FLIT		0x0400 /* Flit Mode Status */
 #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	0x32	/* end of v2 EPs w/ link */
-- 
2.39.3



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

* [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts
  2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
                   ` (8 preceding siblings ...)
  2024-03-21 10:04 ` [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register Saif Abrar
@ 2024-03-21 10:04 ` Saif Abrar
  2024-03-25 13:34   ` Cédric Le Goater
  9 siblings, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-21 10:04 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel
  Cc: clg, npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini,
	thuth, lvivier, saif.abrar

Add a method to reset the value of LSI Source-ID.
Mask off LSI source-id based on number of interrupts in the big/small PHB.

Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
---
 hw/pci-host/pnv_phb4.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index f48750ee54..8fbaf6512e 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -489,6 +489,7 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
 
     lsi_base = GETFIELD(PHB_LSI_SRC_ID, phb->regs[PHB_LSI_SOURCE_ID >> 3]);
     lsi_base <<= 3;
+    lsi_base &= (xsrc->nr_irqs - 1);
 
     /* TODO: handle reset values of PHB_LSI_SRC_ID */
     if (!lsi_base) {
@@ -1966,6 +1967,12 @@ static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
     /* TODO: Add more RO-masks as regs are implemented in the model */
 }
 
+static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
+{
+    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
+    pnv_phb4_update_xsrc(phb);
+}
+
 static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
 {
     STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
@@ -2023,6 +2030,7 @@ static void pnv_phb4_reset(void *dev)
     pnv_phb4_cfg_core_reset(phb);
     pnv_phb4_pbl_core_reset(phb);
 
+    pnv_phb4_fund_A_reset(phb);
     pnv_phb4_err_reg_reset(phb);
     pnv_phb4_pcie_stack_reg_reset(phb);
     pnv_phb4_regb_err_reg_reset(phb);
@@ -2102,8 +2110,6 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
         return;
     }
 
-    pnv_phb4_update_xsrc(phb);
-
     phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, xsrc->nr_irqs);
 
     pnv_phb4_xscom_realize(phb);
-- 
2.39.3



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

* Re: [PATCH 01/10] qtest/phb4: Add testbench for PHB4
  2024-03-21 10:04 ` [PATCH 01/10] qtest/phb4: Add testbench for PHB4 Saif Abrar
@ 2024-03-25  9:39   ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25  9:39 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

Hello Saif,

On 3/21/24 11:04, Saif Abrar wrote:
> New qtest TB added for PHB4.
> TB reads PHB Version register and asserts that
> bits[24:31] have value 0xA5.
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
> ---
>   tests/qtest/meson.build     |  1 +
>   tests/qtest/pnv-phb4-test.c | 74 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 75 insertions(+)
>   create mode 100644 tests/qtest/pnv-phb4-test.c
> 
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 36c5c13a7b..4795e51c17 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -168,6 +168,7 @@ qtests_ppc64 = \
>     (config_all_devices.has_key('CONFIG_PSERIES') ? ['device-plug-test'] : []) +               \
>     (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-xscom-test'] : []) +                 \
>     (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-host-i2c-test'] : []) +              \
> +  (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-phb4-test'] : []) +                  \
>     (config_all_devices.has_key('CONFIG_PSERIES') ? ['rtas-test'] : []) +                      \
>     (slirp.found() ? ['pxe-test'] : []) +              \
>     (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) +             \
> diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
> new file mode 100644
> index 0000000000..e3b809e9c4
> --- /dev/null
> +++ b/tests/qtest/pnv-phb4-test.c
> @@ -0,0 +1,74 @@
> +/*
> + * QTest testcase for PowerNV PHB4
> + *
> + * Copyright (c) 2024, IBM Corporation.
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "libqtest.h"
> +#include "hw/pci-host/pnv_phb4_regs.h"
> +
> +#define P10_XSCOM_BASE          0x000603fc00000000ull
> +#define PHB4_MMIO               0x000600c3c0000000ull
> +#define PHB4_XSCOM              0x8010900ull
> +
> +#define PPC_BIT(bit)            (0x8000000000000000ULL >> (bit))
> +#define PPC_BITMASK(bs, be)     ((PPC_BIT(bs) - PPC_BIT(be)) | PPC_BIT(bs))
> +
> +static uint64_t pnv_xscom_addr(uint32_t pcba)
> +{
> +    return P10_XSCOM_BASE | ((uint64_t) pcba << 3);
> +}
> +
> +static uint64_t pnv_phb4_xscom_addr(uint32_t reg)
> +{
> +    return pnv_xscom_addr(PHB4_XSCOM + reg);
> +}

Please use tests/qtest/pnv-xscom.h instead.

> +/*
> + * XSCOM read/write is indirect in PHB4:
> + * Write 'SCOM - HV Indirect Address Register'
> + * with register-offset to read/write.
> +   - bit[0]: Valid Bit
> +   - bit[51:61]: Indirect Address(00:10)
> + * Read/write 'SCOM - HV Indirect Data Register' to get/set the value.
> + */
> +
> +static uint64_t pnv_phb4_xscom_read(QTestState *qts, uint32_t reg)
> +{
> +    qtest_writeq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_ADDR),
> +            PPC_BIT(0) | reg);
> +    return qtest_readq(qts, pnv_phb4_xscom_addr(PHB_SCOM_HV_IND_DATA));
> +}

> +/* Assert that 'PHB - Version Register Offset 0x0800' bits-[24:31] are 0xA5 */
> +static void phb4_version_test(QTestState *qts)
> +{
> +    uint64_t ver = pnv_phb4_xscom_read(qts, PHB_VERSION);
> +
> +    /* PHB Version register [24:31]: Major Revision ID 0xA5 */
> +    ver = ver >> (63 - 31);
> +    g_assert_cmpuint(ver, ==, 0xA5);
> +}
> +
> +static void test_phb4(void)
> +{
> +    QTestState *qts = NULL;
> +
> +    qts = qtest_initf("-machine powernv10 -accel tcg -nographic -d unimp");

"-nographic -d unimp" is not needed.

> +
> +    /* Make sure test is running on PHB */
> +    phb4_version_test(qts);

Please add similar tests for phb[345]. See tests/qtest/pnv-xscom-test.c.

Thanks,

C.


> +
> +    qtest_quit(qts);
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    g_test_init(&argc, &argv, NULL);
> +    qtest_add_func("phb4", test_phb4);
> +    return g_test_run();
> +}



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

* Re: [PATCH 02/10] pnv/phb4: Add reset logic to PHB4
  2024-03-21 10:04 ` [PATCH 02/10] pnv/phb4: Add reset logic to PHB4 Saif Abrar
@ 2024-03-25 13:32   ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 13:32 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier, Frederic Barrat, Daniel Henrique Barboza

Cc: +Fred +Daniel

On 3/21/24 11:04, Saif Abrar wrote:
> Add a method to be invoked on QEMU reset.
> Also add CFG and PBL core-blocks reset logic using
> appropriate bits of PHB_PCIE_CRESET register.
> 
> Tested by reading the reset value of a register.
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
> ---
>   hw/pci-host/pnv_phb4.c              | 104 +++++++++++++++++++++++++++-
>   include/hw/pci-host/pnv_phb4_regs.h |  16 ++++-
>   tests/qtest/pnv-phb4-test.c         |  10 +++
>   3 files changed, 127 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 075499d36d..d2e7403b37 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -1,7 +1,7 @@
>   /*
> - * QEMU PowerPC PowerNV (POWER9) PHB4 model
> + * QEMU PowerPC PowerNV (POWER10) PHB4 model

You can add an extra line for POWER10/PHB5 but please keep POWER9/PHB4.
POWER8 and POWER9 are still supported.

>    *
> - * Copyright (c) 2018-2020, IBM Corporation.
> + * Copyright (c) 2018-2024, IBM Corporation.
>    *
>    * This code is licensed under the GPL version 2 or later. See the
>    * COPYING file in the top-level directory.
> @@ -22,6 +22,7 @@
>   #include "hw/qdev-properties.h"
>   #include "qom/object.h"
>   #include "trace.h"
> +#include "sysemu/reset.h"
>   
>   #define phb_error(phb, fmt, ...)                                        \
>       qemu_log_mask(LOG_GUEST_ERROR, "phb4[%d:%d]: " fmt "\n",            \
> @@ -499,6 +500,86 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
>       }
>   }
>   
> +/*
> + * Get the PCI-E capability offset from the root-port
> + */
> +static uint32_t get_exp_offset(PnvPHB4 *phb)
> +{
> +    PCIHostState *pci = PCI_HOST_BRIDGE(phb->phb_base);
> +    PCIDevice *pdev;
> +    pdev = pci_find_device(pci->bus, 0, 0);
> +    if (!pdev) {
> +        phb_error(phb, "PCI device not found");
> +        return ~0;
> +    }
> +    PCIERootPortClass *rpc = PCIE_ROOT_PORT_GET_CLASS(pdev);
> +    return rpc->exp_offset;
> +}
> +
> +#define RC_CONFIG_WRITE(a, v) pnv_phb4_rc_config_write(phb, a, 4, v);

This helper RC_CONFIG_WRITE doesn't look very useful.

> +
> +static void pnv_phb4_cfg_core_reset(PnvPHB4 *phb)
> +{
> +    /* Zero all registers initially */
> +    int i;
> +    for (i = PCI_COMMAND ; i < PHB_RC_CONFIG_SIZE ; i += 4) {
> +            RC_CONFIG_WRITE(i, 0)
> +    }
> +
> +    RC_CONFIG_WRITE(PCI_COMMAND,          0x100100);
> +    RC_CONFIG_WRITE(PCI_CLASS_REVISION,   0x6040000);
> +    RC_CONFIG_WRITE(PCI_CACHE_LINE_SIZE,  0x10000);
> +    RC_CONFIG_WRITE(PCI_MEMORY_BASE,      0x10);
> +    RC_CONFIG_WRITE(PCI_PREF_MEMORY_BASE, 0x10011);
> +    RC_CONFIG_WRITE(PCI_CAPABILITY_LIST,  0x40);
> +    RC_CONFIG_WRITE(PCI_INTERRUPT_LINE,   0x20000);

Can we use literal defined values instead of numerical ones ? It would
help the reader understand what are the default settings.

> +    /* PM Capabilities Register */
> +    RC_CONFIG_WRITE(PCI_BRIDGE_CONTROL + PCI_PM_PMC, 0xC8034801);
> +
> +    uint32_t exp_offset = get_exp_offset(phb);
> +    RC_CONFIG_WRITE(exp_offset, 0x420010);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCAP,  0x8022);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCTL,  0x140);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCAP,  0x300105);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCTL,  0x2010008);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_SLTCTL,  0x2000);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCAP2, 0x1003F);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_DEVCTL2, 0x20);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCAP2, 0x80003E);
> +    RC_CONFIG_WRITE(exp_offset + PCI_EXP_LNKCTL2, 0x5);
> +
> +    RC_CONFIG_WRITE(PHB_AER_ECAP,    0x14810001);
> +    RC_CONFIG_WRITE(PHB_AER_CAPCTRL, 0xA0);
> +    RC_CONFIG_WRITE(PHB_SEC_ECAP,    0x1A010019);
> +
> +    RC_CONFIG_WRITE(PHB_LMR_ECAP, 0x1E810027);
> +    /* LMR - Margining Lane Control / Status Register # 2 to 16 */
> +    for (i = PHB_LMR_CTLSTA_2 ; i <= PHB_LMR_CTLSTA_16 ; i += 4) {
> +        RC_CONFIG_WRITE(i, 0x9C38);
> +    }
> +
> +    RC_CONFIG_WRITE(PHB_DLF_ECAP, 0x1F410025);
> +    RC_CONFIG_WRITE(PHB_DLF_CAP,  0x80000001);
> +    RC_CONFIG_WRITE(P16_ECAP,     0x22410026);
> +    RC_CONFIG_WRITE(P32_ECAP,     0x1002A);
> +    RC_CONFIG_WRITE(P32_CAP,      0x103);
> +}

The reset of the root complex register values should be done in
pnv_phb_root_port_reset_hold().

A lot of changes were done on the PHB4/5 models 2 or 3 years ago to
prepare libvirt support of the PowerNV machines. User created PHB
devices was added and generic models of the root complex and the PHB
were introduced to facilitate the machine definition from a libvirt
POV.

Livirt support was abandoned but the PHB models didn't change. I think
there are possible cleanups if we deprecate the generic models.

> +static void pnv_phb4_pbl_core_reset(PnvPHB4 *phb)
> +{
> +    /* Zero all registers initially */
> +    int i;
> +    for (i = PHB_PBL_CONTROL ; i <= PHB_PBL_ERR1_STATUS_MASK ; i += 8) {
> +        phb->regs[i >> 3] = 0x0;
> +    }
> +
> +    /* Set specific register values */
> +    phb->regs[PHB_PBL_CONTROL       >> 3] = 0xC009000000000000;
> +    phb->regs[PHB_PBL_TIMEOUT_CTRL  >> 3] = 0x2020000000000000;
> +    phb->regs[PHB_PBL_NPTAG_ENABLE  >> 3] = 0xFFFFFFFF00000000;
> +    phb->regs[PHB_PBL_SYS_LINK_INIT >> 3] = 0x80088B4642473000;
> +}
> +
>   static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>                                  unsigned size)
>   {
> @@ -612,6 +693,16 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>           pnv_phb4_update_xsrc(phb);
>           break;
>   
> +    /* Reset core blocks */
> +    case PHB_PCIE_CRESET:
> +        if (val & PHB_PCIE_CRESET_CFG_CORE) {
> +            pnv_phb4_cfg_core_reset(phb);
> +        }
> +        if (val & PHB_PCIE_CRESET_PBL) {
> +            pnv_phb4_pbl_core_reset(phb);
> +        }
> +        break;
> +
>       /* Silent simple writes */
>       case PHB_ASN_CMPM:
>       case PHB_CONFIG_ADDRESS:
> @@ -1531,6 +1622,13 @@ static void pnv_phb4_xscom_realize(PnvPHB4 *phb)
>   static PCIIOMMUOps pnv_phb4_iommu_ops = {
>       .get_address_space = pnv_phb4_dma_iommu,
>   };
> +static void pnv_phb4_reset(void *dev)
> +{
> +    PnvPHB4 *phb = PNV_PHB4(dev);
> +    pnv_phb4_cfg_core_reset(phb);
> +    pnv_phb4_pbl_core_reset(phb);
> +    phb->regs[PHB_PCIE_CRESET >> 3] = 0xE000000000000000;
> +}
>   
>   static void pnv_phb4_instance_init(Object *obj)
>   {
> @@ -1608,6 +1706,8 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>       phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, xsrc->nr_irqs);
>   
>       pnv_phb4_xscom_realize(phb);
> +
> +    qemu_register_reset(pnv_phb4_reset, dev);

That's the old reset style. Please take a look at the ResettableClass :

   https://qemu.readthedocs.io/en/v8.2.1/devel/reset.html

Thanks,

C.



>   }
>   
>   /*
> diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
> index bea96f4d91..6892e21cc9 100644
> --- a/include/hw/pci-host/pnv_phb4_regs.h
> +++ b/include/hw/pci-host/pnv_phb4_regs.h
> @@ -343,6 +343,18 @@
>   #define PHB_RC_CONFIG_BASE                      0x1000
>   #define   PHB_RC_CONFIG_SIZE                    0x800
>   
> +#define PHB_AER_ECAP                            0x100
> +#define PHB_AER_CAPCTRL                         0x118
> +#define PHB_SEC_ECAP                            0x148
> +#define PHB_LMR_ECAP                            0x1A0
> +#define PHB_LMR_CTLSTA_2                        0x1AC
> +#define PHB_LMR_CTLSTA_16                       0x1E4
> +#define PHB_DLF_ECAP                            0x1E8
> +#define PHB_DLF_CAP                             0x1EC
> +#define P16_ECAP                                0x1F4
> +#define P32_ECAP                                0x224
> +#define P32_CAP                                 0x228
> +
>   /* PHB4 REGB registers */
>   
>   /* PBL core */
> @@ -368,7 +380,7 @@
>   #define PHB_PCIE_SCR                    0x1A00
>   #define   PHB_PCIE_SCR_SLOT_CAP         PPC_BIT(15)
>   #define   PHB_PCIE_SCR_MAXLINKSPEED     PPC_BITMASK(32, 35)
> -
> +#define PHB_PCIE_BNR                    0x1A08
>   
>   #define PHB_PCIE_CRESET                 0x1A10
>   #define   PHB_PCIE_CRESET_CFG_CORE      PPC_BIT(0)
> @@ -423,6 +435,8 @@
>   #define PHB_PCIE_LANE_EQ_CNTL23         0x1B08 /* DD1 only */
>   #define PHB_PCIE_TRACE_CTRL             0x1B20
>   #define PHB_PCIE_MISC_STRAP             0x1B30
> +#define PHB_PCIE_PHY_RXEQ_STAT_G3_00_03 0x1B40
> +#define PHB_PCIE_PHY_RXEQ_STAT_G5_12_15 0x1B98
>   
>   /* Error */
>   #define PHB_REGB_ERR_STATUS             0x1C00
> diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
> index e3b809e9c4..44141462f6 100644
> --- a/tests/qtest/pnv-phb4-test.c
> +++ b/tests/qtest/pnv-phb4-test.c
> @@ -54,6 +54,13 @@ static void phb4_version_test(QTestState *qts)
>       g_assert_cmpuint(ver, ==, 0xA5);
>   }
>   
> +/* Assert that 'PHB PBL Control' register has correct reset value */
> +static void phb4_reset_test(QTestState *qts)
> +{
> +    g_assert_cmpuint(pnv_phb4_xscom_read(qts, PHB_PBL_CONTROL),
> +                     ==, 0xC009000000000000);
> +}
> +
>   static void test_phb4(void)
>   {
>       QTestState *qts = NULL;
> @@ -63,6 +70,9 @@ static void test_phb4(void)
>       /* Make sure test is running on PHB */
>       phb4_version_test(qts);
>   
> +    /* Check reset value of a register */
> +    phb4_reset_test(qts);
> +
>       qtest_quit(qts);
>   }
>   



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

* Re: [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts
  2024-03-21 10:04 ` [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts Saif Abrar
@ 2024-03-25 13:34   ` Cédric Le Goater
  2024-03-27  9:59     ` Saif Abrar
  2024-03-28  9:31     ` Saif Abrar
  0 siblings, 2 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 13:34 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/21/24 11:04, Saif Abrar wrote:
> Add a method to reset the value of LSI Source-ID.
> Mask off LSI source-id based on number of interrupts in the big/small PHB.

Looks ok.

  
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
> ---
>   hw/pci-host/pnv_phb4.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index f48750ee54..8fbaf6512e 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -489,6 +489,7 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
>   
>       lsi_base = GETFIELD(PHB_LSI_SRC_ID, phb->regs[PHB_LSI_SOURCE_ID >> 3]);
>       lsi_base <<= 3;
> +    lsi_base &= (xsrc->nr_irqs - 1);
>   
>       /* TODO: handle reset values of PHB_LSI_SRC_ID */
>       if (!lsi_base) {
> @@ -1966,6 +1967,12 @@ static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
>       /* TODO: Add more RO-masks as regs are implemented in the model */
>   }
>   
> +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)

What is fund_A ?

> +{
> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);

Is this mask the default value for HW ?


Thanks,

C.


> +    pnv_phb4_update_xsrc(phb);
> +}
> +
>   static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
>   {
>       STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
> @@ -2023,6 +2030,7 @@ static void pnv_phb4_reset(void *dev)
>       pnv_phb4_cfg_core_reset(phb);
>       pnv_phb4_pbl_core_reset(phb);
>   
> +    pnv_phb4_fund_A_reset(phb);
>       pnv_phb4_err_reg_reset(phb);
>       pnv_phb4_pcie_stack_reg_reset(phb);
>       pnv_phb4_regb_err_reg_reset(phb);
> @@ -2102,8 +2110,6 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>           return;
>       }
>   
> -    pnv_phb4_update_xsrc(phb);
> -
>       phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, xsrc->nr_irqs);
>   
>       pnv_phb4_xscom_realize(phb);



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

* Re: [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register
  2024-03-21 10:04 ` [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register Saif Abrar
@ 2024-03-25 13:35   ` Cédric Le Goater
  2024-03-25 14:37     ` Cornelia Huck
  0 siblings, 1 reply; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 13:35 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/21/24 11:04, Saif Abrar wrote:
> PHB updates the register PCIE Link-Control-2.
> Set the write-mask bits for TLS, ENTER_COMP, TX_MARGIN,
> HASD, MOD_COMP, COMP_SOS and COMP_P_DE.


You should resend this patch independently of the PowerNV PHB changes.


Thanks,

C.



> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
> ---
>   hw/pci/pcie.c                             | 6 ++++++
>   include/standard-headers/linux/pci_regs.h | 3 +++
>   2 files changed, 9 insertions(+)
> 
> diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
> index 4b2f0805c6..e3081f6b84 100644
> --- a/hw/pci/pcie.c
> +++ b/hw/pci/pcie.c
> @@ -212,6 +212,12 @@ int pcie_cap_init(PCIDevice *dev, uint8_t offset,
>   
>       pci_set_word(dev->wmask + pos + PCI_EXP_DEVCTL2, PCI_EXP_DEVCTL2_EETLPPB);
>   
> +    pci_set_word(dev->wmask + pos + PCI_EXP_LNKCTL2,
> +            PCI_EXP_LNKCTL2_TLS | PCI_EXP_LNKCTL2_ENTER_COMP |
> +            PCI_EXP_LNKCTL2_TX_MARGIN | PCI_EXP_LNKCTL2_HASD |
> +            PCI_EXP_LNKCTL2_MOD_COMP | PCI_EXP_LNKCTL2_COMP_SOS |
> +            PCI_EXP_LNKCTL2_COMP_P_DE);
> +
>       if (dev->cap_present & QEMU_PCIE_EXTCAP_INIT) {
>           /* read-only to behave like a 'NULL' Extended Capability Header */
>           pci_set_long(dev->wmask + PCI_CONFIG_SPACE_SIZE, 0);
> diff --git a/include/standard-headers/linux/pci_regs.h b/include/standard-headers/linux/pci_regs.h
> index a39193213f..f743defe91 100644
> --- a/include/standard-headers/linux/pci_regs.h
> +++ b/include/standard-headers/linux/pci_regs.h
> @@ -694,6 +694,9 @@
>   #define  PCI_EXP_LNKCTL2_ENTER_COMP	0x0010 /* Enter Compliance */
>   #define  PCI_EXP_LNKCTL2_TX_MARGIN	0x0380 /* Transmit Margin */
>   #define  PCI_EXP_LNKCTL2_HASD		0x0020 /* HW Autonomous Speed Disable */
> +#define  PCI_EXP_LNKCTL2_MOD_COMP	0x0400 /* Enter Modified Compliance */
> +#define  PCI_EXP_LNKCTL2_COMP_SOS	0x0800 /* Compliance SOS */
> +#define  PCI_EXP_LNKCTL2_COMP_P_DE	0xF000 /* Compliance Preset/De-emphasis */
>   #define PCI_EXP_LNKSTA2		0x32	/* Link Status 2 */
>   #define  PCI_EXP_LNKSTA2_FLIT		0x0400 /* Flit Mode Status */
>   #define PCI_CAP_EXP_ENDPOINT_SIZEOF_V2	0x32	/* end of v2 EPs w/ link */



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

* Re: [PATCH 08/10] pnv/phb4: Implement IODA PCT table
  2024-03-21 10:04 ` [PATCH 08/10] pnv/phb4: Implement IODA PCT table Saif Abrar
@ 2024-03-25 13:35   ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 13:35 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/21/24 11:04, Saif Abrar wrote:
> IODA PCT table (#3) is implemented
> without any functionality, being a debug table.
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/pci-host/pnv_phb4.c              | 6 ++++++
>   include/hw/pci-host/pnv_phb4.h      | 2 ++
>   include/hw/pci-host/pnv_phb4_regs.h | 1 +
>   3 files changed, 9 insertions(+)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 6823ffab54..f48750ee54 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -263,6 +263,10 @@ static uint64_t *pnv_phb4_ioda_access(PnvPHB4 *phb,
>           mask = phb->big_phb ? PNV_PHB4_MAX_MIST : (PNV_PHB4_MAX_MIST >> 1);
>           mask -= 1;
>           break;
> +    case IODA3_TBL_PCT:
> +        tptr = phb->ioda_PCT;
> +        mask = 7;
> +        break;
>       case IODA3_TBL_RCAM:
>           mask = phb->big_phb ? 127 : 63;
>           break;
> @@ -361,6 +365,8 @@ static void pnv_phb4_ioda_write(PnvPHB4 *phb, uint64_t val)
>       /* Handle side effects */
>       switch (table) {
>       case IODA3_TBL_LIST:
> +    case IODA3_TBL_PCT:
> +        /* No action for debug tables */
>           break;
>       case IODA3_TBL_MIST: {
>           /* Special mask for MIST partial write */
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 91e81eee0e..6d83e5616f 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -64,6 +64,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB4, PNV_PHB4)
>   #define PNV_PHB4_MAX_LSIs          8
>   #define PNV_PHB4_MAX_INTs          4096
>   #define PNV_PHB4_MAX_MIST          (PNV_PHB4_MAX_INTs >> 2)
> +#define PNV_PHB4_MAX_PCT           128
>   #define PNV_PHB4_MAX_MMIO_WINDOWS  32
>   #define PNV_PHB4_MIN_MMIO_WINDOWS  16
>   #define PNV_PHB4_NUM_REGS          (0x3000 >> 3)
> @@ -144,6 +145,7 @@ struct PnvPHB4 {
>       /* On-chip IODA tables */
>       uint64_t ioda_LIST[PNV_PHB4_MAX_LSIs];
>       uint64_t ioda_MIST[PNV_PHB4_MAX_MIST];
> +    uint64_t ioda_PCT[PNV_PHB4_MAX_PCT];
>       uint64_t ioda_TVT[PNV_PHB4_MAX_TVEs];
>       uint64_t ioda_MBT[PNV_PHB4_MAX_MBEs];
>       uint64_t ioda_MDT[PNV_PHB4_MAX_PEs];
> diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
> index c1d5a83271..e30adff7b2 100644
> --- a/include/hw/pci-host/pnv_phb4_regs.h
> +++ b/include/hw/pci-host/pnv_phb4_regs.h
> @@ -486,6 +486,7 @@
>   
>   #define IODA3_TBL_LIST          1
>   #define IODA3_TBL_MIST          2
> +#define IODA3_TBL_PCT           3
>   #define IODA3_TBL_RCAM          5
>   #define IODA3_TBL_MRT           6
>   #define IODA3_TBL_PESTA         7



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

* Re: [PATCH 07/10] pnv/phb4: Set link speed and width in the DLP training control register
  2024-03-21 10:04 ` [PATCH 07/10] pnv/phb4: Set link speed and width in the DLP training control register Saif Abrar
@ 2024-03-25 13:36   ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 13:36 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/21/24 11:04, Saif Abrar wrote:
> Get the current link-status from PCIE macro.
> Extract link-speed and link-width from the link-status
> and set in the DLP training control (PCIE_DLP_TCR) register.
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/pci-host/pnv_phb4.c | 21 +++++++++++++++++++--
>   1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index 7b3d75bae6..6823ffab54 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -980,10 +980,27 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
>           val |= PHB_PCIE_SCR_PLW_X16; /* RO bit */
>           break;
>   
> -    /* Link training always appears trained */
>       case PHB_PCIE_DLP_TRAIN_CTL:
> -        /* TODO: Do something sensible with speed ? */
> +        /* Link training always appears trained */
>           val |= PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
> +
> +        /* Get the current link-status from PCIE */
> +        uint32_t exp_offset = get_exp_offset(phb);
> +        uint32_t lnkstatus = bswap32(pnv_phb4_rc_config_read(phb,
> +                                exp_offset + PCI_EXP_LNKSTA, 4));
> +
> +        /* Extract link-speed from the link-status */
> +        uint32_t v = lnkstatus & PCI_EXP_LNKSTA_CLS;
> +        /* Set the current link-speed at the LINK_SPEED position */
> +        val = SETFIELD(PHB_PCIE_DLP_LINK_SPEED, val, v);
> +
> +        /*
> +         * Extract link-width from the link-status,
> +         * after shifting the required bitfields.
> +         */
> +        v = (lnkstatus & PCI_EXP_LNKSTA_NLW) >> PCI_EXP_LNKSTA_NLW_SHIFT;
> +        /* Set the current link-width at the LINK_WIDTH position */
> +        val = SETFIELD(PHB_PCIE_DLP_LINK_WIDTH, val, v);
>           return val;
>   
>       /*



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

* Re: [PATCH 05/10] pnv/phb4: Implement write-clear and return 1's on unimplemented reg read
  2024-03-21 10:04 ` [PATCH 05/10] pnv/phb4: Implement write-clear and return 1's on unimplemented reg read Saif Abrar
@ 2024-03-25 13:58   ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 13:58 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/21/24 11:04, Saif Abrar wrote:
> Implement write-1-to-clear and write-X-to-clear logic.
> Update registers with silent simple read and write.
> Return all 1's when an unimplemented/reserved register is read.
> 
> Test that reading address 0x0 returns all 1's (i.e. -1).
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>


Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   hw/pci-host/pnv_phb4.c              | 190 ++++++++++++++++++++++------
>   include/hw/pci-host/pnv_phb4_regs.h |  12 +-
>   tests/qtest/pnv-phb4-test.c         |   9 ++
>   3 files changed, 170 insertions(+), 41 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index a81763f34c..4e3a6b37f9 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -683,8 +683,41 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>           return;
>       }
>   
> -    /* Handle masking */
> +    /* Handle RO, W1C, WxC and masking */
>       switch (off) {
> +    /* W1C: Write-1-to-Clear registers */
> +    case PHB_TXE_ERR_STATUS:
> +    case PHB_RXE_ARB_ERR_STATUS:
> +    case PHB_RXE_MRG_ERR_STATUS:
> +    case PHB_RXE_TCE_ERR_STATUS:
> +    case PHB_ERR_STATUS:
> +    case PHB_REGB_ERR_STATUS:
> +    case PHB_PCIE_DLP_ERRLOG1:
> +    case PHB_PCIE_DLP_ERRLOG2:
> +    case PHB_PCIE_DLP_ERR_STATUS:
> +    case PHB_PBL_ERR_STATUS:
> +        phb->regs[off >> 3] &= ~val;
> +        return;
> +
> +    /* WxC: Clear register on any write */
> +    case PHB_PBL_ERR1_STATUS:
> +    case PHB_PBL_ERR_LOG_0 ... PHB_PBL_ERR_LOG_1:
> +    case PHB_REGB_ERR1_STATUS:
> +    case PHB_REGB_ERR_LOG_0 ... PHB_REGB_ERR_LOG_1:
> +    case PHB_TXE_ERR1_STATUS:
> +    case PHB_TXE_ERR_LOG_0 ... PHB_TXE_ERR_LOG_1:
> +    case PHB_RXE_ARB_ERR1_STATUS:
> +    case PHB_RXE_ARB_ERR_LOG_0 ... PHB_RXE_ARB_ERR_LOG_1:
> +    case PHB_RXE_MRG_ERR1_STATUS:
> +    case PHB_RXE_MRG_ERR_LOG_0 ... PHB_RXE_MRG_ERR_LOG_1:
> +    case PHB_RXE_TCE_ERR1_STATUS:
> +    case PHB_RXE_TCE_ERR_LOG_0 ... PHB_RXE_TCE_ERR_LOG_1:
> +    case PHB_ERR1_STATUS:
> +    case PHB_ERR_LOG_0 ... PHB_ERR_LOG_1:
> +        phb->regs[off >> 3] = 0;
> +        return;
> +
> +    /* Write value updated by masks */
>       case PHB_LSI_SOURCE_ID:
>           val &= PHB_LSI_SRC_ID;
>           break;
> @@ -723,7 +756,6 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>       case PHB_LEM_WOF:
>           val = 0;
>           break;
> -    /* TODO: More regs ..., maybe create a table with masks... */
>   
>       /* Read only registers */
>       case PHB_CPU_LOADSTORE_STATUS:
> @@ -732,6 +764,12 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>       case PHB_PHB4_TCE_CAP:
>       case PHB_PHB4_IRQ_CAP:
>       case PHB_PHB4_EEH_CAP:
> +    case PHB_VERSION:
> +    case PHB_DMA_CHAN_STATUS:
> +    case PHB_TCE_TAG_STATUS:
> +    case PHB_PBL_BUF_STATUS:
> +    case PHB_PCIE_BNR:
> +    case PHB_PCIE_PHY_RXEQ_STAT_G3_00_03 ... PHB_PCIE_PHY_RXEQ_STAT_G5_12_15:
>           return;
>       }
>   
> @@ -752,6 +790,7 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>               pnv_phb4_update_all_msi_regions(phb);
>           }
>           break;
> +
>       case PHB_M32_START_ADDR:
>       case PHB_M64_UPPER_BITS:
>           if (changed) {
> @@ -797,27 +836,63 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>           break;
>   
>       /* Silent simple writes */
> -    case PHB_ASN_CMPM:
> -    case PHB_CONFIG_ADDRESS:
> -    case PHB_IODA_ADDR:
> -    case PHB_TCE_KILL:
> -    case PHB_TCE_SPEC_CTL:
> -    case PHB_PEST_BAR:
> -    case PHB_PELTV_BAR:
> +    /* PHB Fundamental register set A */
> +    case PHB_CONFIG_DATA ... PHB_LOCK1:
>       case PHB_RTT_BAR:
> -    case PHB_LEM_FIR_ACCUM:
> -    case PHB_LEM_ERROR_MASK:
> -    case PHB_LEM_ACTION0:
> -    case PHB_LEM_ACTION1:
> -    case PHB_TCE_TAG_ENABLE:
> +    case PHB_PELTV_BAR:
> +    case PHB_PEST_BAR:
> +    case PHB_CAPI_CMPM ... PHB_M64_AOMASK:
> +    case PHB_NXLATE_PREFIX ... PHB_DMA_SYNC:
> +    case PHB_TCE_KILL ... PHB_IODA_ADDR:
> +    case PHB_PAPR_ERR_INJ_CTL ... PHB_PAPR_ERR_INJ_MASK:
>       case PHB_INT_NOTIFY_ADDR:
>       case PHB_INT_NOTIFY_INDEX:
> -    case PHB_DMA_SYNC:
> -       break;
> +    /* Fundamental register set B */
> +    case PHB_AIB_FENCE_CTRL ... PHB_Q_DMA_R:
> +    /* FIR & Error registers */
> +    case PHB_LEM_FIR_ACCUM:
> +    case PHB_LEM_ERROR_MASK:
> +    case PHB_LEM_ACTION0 ... PHB_LEM_WOF:
> +    case PHB_ERR_INJECT ... PHB_ERR_AIB_FENCE_ENABLE:
> +    case PHB_ERR_STATUS_MASK ... PHB_ERR1_STATUS_MASK:
> +    case PHB_TXE_ERR_INJECT ... PHB_TXE_ERR_AIB_FENCE_ENABLE:
> +    case PHB_TXE_ERR_STATUS_MASK ... PHB_TXE_ERR1_STATUS_MASK:
> +    case PHB_RXE_ARB_ERR_INJECT ... PHB_RXE_ARB_ERR_AIB_FENCE_ENABLE:
> +    case PHB_RXE_ARB_ERR_STATUS_MASK ... PHB_RXE_ARB_ERR1_STATUS_MASK:
> +    case PHB_RXE_MRG_ERR_INJECT ... PHB_RXE_MRG_ERR_AIB_FENCE_ENABLE:
> +    case PHB_RXE_MRG_ERR_STATUS_MASK ... PHB_RXE_MRG_ERR1_STATUS_MASK:
> +    case PHB_RXE_TCE_ERR_INJECT ... PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE:
> +    case PHB_RXE_TCE_ERR_STATUS_MASK ... PHB_RXE_TCE_ERR1_STATUS_MASK:
> +    /* Performance monitor & Debug registers */
> +    case PHB_TRACE_CONTROL ... PHB_PERFMON_CTR1:
> +    /* REGB Registers */
> +    /* PBL core */
> +    case PHB_PBL_CONTROL:
> +    case PHB_PBL_TIMEOUT_CTRL:
> +    case PHB_PBL_NPTAG_ENABLE:
> +    case PHB_PBL_SYS_LINK_INIT:
> +    case PHB_PBL_ERR_INF_ENABLE ... PHB_PBL_ERR_FAT_ENABLE:
> +    case PHB_PBL_ERR_STATUS_MASK ... PHB_PBL_ERR1_STATUS_MASK:
> +    /* PCI-E stack */
> +    case PHB_PCIE_SCR:
> +    case PHB_PCIE_DLP_STR ... PHB_PCIE_HOTPLUG_STATUS:
> +    case PHB_PCIE_LMR ... PHB_PCIE_DLP_LSR:
> +    case PHB_PCIE_DLP_RXMGN:
> +    case PHB_PCIE_DLP_LANEZEROCTL ... PHB_PCIE_DLP_TRCRDDATA:
> +    case PHB_PCIE_DLP_ERR_COUNTERS:
> +    case PHB_PCIE_DLP_EIC ...   PHB_PCIE_LANE_EQ_CNTL23:
> +    case PHB_PCIE_TRACE_CTRL:
> +    case PHB_PCIE_MISC_STRAP ... PHB_PCIE_PHY_EQ_CTL:
> +    /* Error registers */
> +    case PHB_REGB_ERR_INJECT:
> +    case PHB_REGB_ERR_INF_ENABLE ... PHB_REGB_ERR_FAT_ENABLE:
> +    case PHB_REGB_ERR_STATUS_MASK ... PHB_REGB_ERR1_STATUS_MASK:
> +        break;
>   
>       /* Noise on anything else */
>       default:
> -        qemu_log_mask(LOG_UNIMP, "phb4: reg_write 0x%"PRIx64"=%"PRIx64"\n",
> +        qemu_log_mask(LOG_UNIMP,
> +                      "phb4: unimplemented reg_write 0x%"PRIx64"=%"PRIx64"\n",
>                         off, val);
>       }
>   }
> @@ -905,36 +980,75 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
>           return val;
>   
>       /* Silent simple reads */
> +    /* PHB Fundamental register set A */
>       case PHB_LSI_SOURCE_ID:
> +    case PHB_DMA_CHAN_STATUS:
>       case PHB_CPU_LOADSTORE_STATUS:
> -    case PHB_ASN_CMPM:
> +    case PHB_CONFIG_DATA ... PHB_LOCK1:
>       case PHB_PHB4_CONFIG:
> +    case PHB_RTT_BAR:
> +    case PHB_PELTV_BAR:
>       case PHB_M32_START_ADDR:
> -    case PHB_CONFIG_ADDRESS:
> -    case PHB_IODA_ADDR:
> -    case PHB_RTC_INVALIDATE:
> -    case PHB_TCE_KILL:
> -    case PHB_TCE_SPEC_CTL:
>       case PHB_PEST_BAR:
> -    case PHB_PELTV_BAR:
> -    case PHB_RTT_BAR:
> +    case PHB_CAPI_CMPM:
> +    case PHB_M64_AOMASK:
>       case PHB_M64_UPPER_BITS:
> -    case PHB_CTRLR:
> -    case PHB_LEM_FIR_ACCUM:
> -    case PHB_LEM_ERROR_MASK:
> -    case PHB_LEM_ACTION0:
> -    case PHB_LEM_ACTION1:
> -    case PHB_TCE_TAG_ENABLE:
> +    case PHB_NXLATE_PREFIX:
> +    case PHB_RTC_INVALIDATE ... PHB_IODA_ADDR:
> +    case PHB_PAPR_ERR_INJ_CTL ... PHB_ETU_ERR_SUMMARY:
>       case PHB_INT_NOTIFY_ADDR:
>       case PHB_INT_NOTIFY_INDEX:
> -    case PHB_Q_DMA_R:
> -    case PHB_ETU_ERR_SUMMARY:
> -        break;
> -
> -    /* Noise on anything else */
> +    /* Fundamental register set B */
> +    case PHB_CTRLR:
> +    case PHB_AIB_FENCE_CTRL ... PHB_Q_DMA_R:
> +    case PHB_TCE_TAG_STATUS:
> +    /* FIR & Error registers */
> +    case PHB_LEM_FIR_ACCUM ... PHB_LEM_ERROR_MASK:
> +    case PHB_LEM_ACTION0 ... PHB_LEM_WOF:
> +    case PHB_ERR_STATUS ... PHB_ERR_AIB_FENCE_ENABLE:
> +    case PHB_ERR_LOG_0 ... PHB_ERR1_STATUS_MASK:
> +    case PHB_TXE_ERR_STATUS ... PHB_TXE_ERR_AIB_FENCE_ENABLE:
> +    case PHB_TXE_ERR_LOG_0 ... PHB_TXE_ERR1_STATUS_MASK:
> +    case PHB_RXE_ARB_ERR_STATUS ... PHB_RXE_ARB_ERR_AIB_FENCE_ENABLE:
> +    case PHB_RXE_ARB_ERR_LOG_0 ... PHB_RXE_ARB_ERR1_STATUS_MASK:
> +    case PHB_RXE_MRG_ERR_STATUS ... PHB_RXE_MRG_ERR_AIB_FENCE_ENABLE:
> +    case PHB_RXE_MRG_ERR_LOG_0 ... PHB_RXE_MRG_ERR1_STATUS_MASK:
> +    case PHB_RXE_TCE_ERR_STATUS ... PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE:
> +    case PHB_RXE_TCE_ERR_LOG_0 ... PHB_RXE_TCE_ERR1_STATUS_MASK:
> +    /* Performance monitor & Debug registers */
> +    case PHB_TRACE_CONTROL ... PHB_PERFMON_CTR1:
> +    /* REGB Registers */
> +    /* PBL core */
> +    case PHB_PBL_CONTROL:
> +    case PHB_PBL_TIMEOUT_CTRL:
> +    case PHB_PBL_NPTAG_ENABLE:
> +    case PHB_PBL_SYS_LINK_INIT:
> +    case PHB_PBL_BUF_STATUS:
> +    case PHB_PBL_ERR_STATUS ... PHB_PBL_ERR_INJECT:
> +    case PHB_PBL_ERR_INF_ENABLE ... PHB_PBL_ERR_FAT_ENABLE:
> +    case PHB_PBL_ERR_LOG_0 ... PHB_PBL_ERR1_STATUS_MASK:
> +    /* PCI-E stack */
> +    case PHB_PCIE_BNR ... PHB_PCIE_DLP_STR:
> +    case PHB_PCIE_DLP_LANE_PWR:
> +    case PHB_PCIE_DLP_LSR:
> +    case PHB_PCIE_DLP_RXMGN:
> +    case PHB_PCIE_DLP_LANEZEROCTL ... PHB_PCIE_DLP_CTL:
> +    case PHB_PCIE_DLP_TRCRDDATA:
> +    case PHB_PCIE_DLP_ERRLOG1 ... PHB_PCIE_DLP_ERR_COUNTERS:
> +    case PHB_PCIE_DLP_EIC ...   PHB_PCIE_LANE_EQ_CNTL23:
> +    case PHB_PCIE_TRACE_CTRL:
> +    case PHB_PCIE_MISC_STRAP ... PHB_PCIE_PHY_RXEQ_STAT_G5_12_15:
> +    /* Error registers */
> +    case PHB_REGB_ERR_STATUS ... PHB_REGB_ERR_INJECT:
> +    case PHB_REGB_ERR_INF_ENABLE ... PHB_REGB_ERR_FAT_ENABLE:
> +    case PHB_REGB_ERR_LOG_0 ... PHB_REGB_ERR1_STATUS_MASK:
> +        break;
> +
> +    /* Noise on unimplemented read, return all 1's */
>       default:
> -        qemu_log_mask(LOG_UNIMP, "phb4: reg_read 0x%"PRIx64"=%"PRIx64"\n",
> -                      off, val);
> +        qemu_log_mask(LOG_UNIMP, "phb4: unimplemented reg_read 0x%"PRIx64"\n",
> +                      off);
> +        val = ~0ull;
>       }
>       return val;
>   }
> diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
> index 391d6a89ea..c1d5a83271 100644
> --- a/include/hw/pci-host/pnv_phb4_regs.h
> +++ b/include/hw/pci-host/pnv_phb4_regs.h
> @@ -372,6 +372,7 @@
>   #define P32_CAP                                 0x228
>   #define P32_CTL                                 0x22C
>   #define P32_STAT                                0x230
> +
>   /* PHB4 REGB registers */
>   
>   /* PBL core */
> @@ -406,6 +407,7 @@
>   #define   PHB_PCIE_CRESET_PERST_N       PPC_BIT(3)
>   #define   PHB_PCIE_CRESET_PIPE_N        PPC_BIT(4)
>   #define   PHB_PCIE_CRESET_REFCLK_N      PPC_BIT(8)
> +#define PHB_PCIE_DLP_STR                0x1A18
>   #define PHB_PCIE_HOTPLUG_STATUS         0x1A20
>   #define   PHB_PCIE_HPSTAT_SIMDIAG       PPC_BIT(3)
>   #define   PHB_PCIE_HPSTAT_RESAMPLE      PPC_BIT(9)
> @@ -416,6 +418,7 @@
>   #define   PHB_PCIE_LMR_RETRAINLINK      PPC_BIT(1)
>   #define   PHB_PCIE_LMR_LINKACTIVE       PPC_BIT(8)
>   
> +#define PHB_PCIE_DLP_LANE_PWR           0x1A38
>   #define PHB_PCIE_DLP_TRAIN_CTL          0x1A40
>   #define   PHB_PCIE_DLP_LINK_WIDTH       PPC_BITMASK(30, 35)
>   #define   PHB_PCIE_DLP_LINK_SPEED       PPC_BITMASK(36, 39)
> @@ -435,18 +438,21 @@
>   #define   PHB_PCIE_DLP_DL_PGRESET       PPC_BIT(22)
>   #define   PHB_PCIE_DLP_TRAINING         PPC_BIT(20)
>   #define   PHB_PCIE_DLP_INBAND_PRESENCE  PPC_BIT(19)
> -
> +#define PHB_PCIE_DLP_LSR                0x1A48
> +#define PHB_PCIE_DLP_RXMGN              0x1A50
> +#define PHB_PCIE_DLP_LANEZEROCTL        0x1A70
>   #define PHB_PCIE_DLP_CTL                0x1A78
>   #define   PHB_PCIE_DLP_CTL_BYPASS_PH2   PPC_BIT(4)
>   #define   PHB_PCIE_DLP_CTL_BYPASS_PH3   PPC_BIT(5)
> -
>   #define PHB_PCIE_DLP_TRWCTL             0x1A80
>   #define   PHB_PCIE_DLP_TRWCTL_EN        PPC_BIT(0)
>   #define   PHB_PCIE_DLP_TRWCTL_WREN      PPC_BIT(1)
> +#define PHB_PCIE_DLP_TRCRDDATA          0x1A88
>   #define PHB_PCIE_DLP_ERRLOG1            0x1AA0
>   #define PHB_PCIE_DLP_ERRLOG2            0x1AA8
>   #define PHB_PCIE_DLP_ERR_STATUS         0x1AB0
>   #define PHB_PCIE_DLP_ERR_COUNTERS       0x1AB8
> +#define PHB_PCIE_DLP_EIC                0x1AC8
>   
>   #define PHB_PCIE_LANE_EQ_CNTL0          0x1AD0
>   #define PHB_PCIE_LANE_EQ_CNTL1          0x1AD8
> @@ -458,6 +464,7 @@
>   #define PHB_PCIE_LANE_EQ_CNTL23         0x1B08 /* DD1 only */
>   #define PHB_PCIE_TRACE_CTRL             0x1B20
>   #define PHB_PCIE_MISC_STRAP             0x1B30
> +#define PHB_PCIE_PHY_EQ_CTL             0x1B38
>   #define PHB_PCIE_PHY_RXEQ_STAT_G3_00_03 0x1B40
>   #define PHB_PCIE_PHY_RXEQ_STAT_G5_12_15 0x1B98
>   
> @@ -591,5 +598,4 @@
>   
>   #define IODA3_PEST1_FAIL_ADDR           PPC_BITMASK(3, 63)
>   
> -
>   #endif /* PCI_HOST_PNV_PHB4_REGS_H */
> diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
> index 0c8e58dd5f..96d1bd6724 100644
> --- a/tests/qtest/pnv-phb4-test.c
> +++ b/tests/qtest/pnv-phb4-test.c
> @@ -139,6 +139,12 @@ static void phb4_writeonly_read_test(QTestState *qts)
>       g_assert_cmpuint(val, ==, 0x0);
>   }
>   
> +/* Check that reading an unimplemented address 0x0 returns -1 */
> +static void phb4_unimplemented_read_test(QTestState *qts)
> +{
> +    g_assert_cmpint(pnv_phb4_xscom_read(qts, 0x0), ==, -1);
> +}
> +
>   static void test_phb4(void)
>   {
>       QTestState *qts = NULL;
> @@ -157,6 +163,9 @@ static void test_phb4(void)
>       /* Check write-only logic */
>       phb4_writeonly_read_test(qts);
>   
> +    /* Check unimplemented register read */
> +    phb4_unimplemented_read_test(qts);
> +
>       qtest_quit(qts);
>   }
>   



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

* Re: [PATCH 04/10] pnv/phb4: Implement read-only and write-only bits of registers
  2024-03-21 10:04 ` [PATCH 04/10] pnv/phb4: Implement read-only and write-only bits of registers Saif Abrar
@ 2024-03-25 14:15   ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-25 14:15 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/21/24 11:04, Saif Abrar wrote:
> SW cannot write the read-only(RO) bits of a register
> and write-only(WO) bits of a register return 0 when read.
> 
> Added ro_mask[] for each register that defines which
> bits in that register are RO.
> When writing to a register, the RO-bits are not updated.
> 
> When reading a register, clear the WO bits and return the updated value.
> 
> Tested the registers PHB_DMA_SYNC, PHB_PCIE_HOTPLUG_STATUS, PHB_PCIE_LMR,
> PHB_PCIE_DLP_TRWCTL, PHB_LEM_ERROR_AND_MASK and PHB_LEM_ERROR_OR_MASK
> by writing all 1's and reading back the value.
> The WO bits in these registers should read back as 0.
> 
> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
> ---
>   hw/pci-host/pnv_phb4.c              | 77 ++++++++++++++++++++++++++---
>   include/hw/pci-host/pnv_phb4.h      |  7 +++
>   include/hw/pci-host/pnv_phb4_regs.h | 19 +++++--
>   tests/qtest/pnv-phb4-test.c         | 60 +++++++++++++++++++++-
>   4 files changed, 150 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
> index b3a83837f8..a81763f34c 100644
> --- a/hw/pci-host/pnv_phb4.c
> +++ b/hw/pci-host/pnv_phb4.c
> @@ -735,6 +735,10 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>           return;
>       }
>   
> +    /* Update 'val' according to the register's RO-mask */
> +    val = (phb->regs[off >> 3] & phb->ro_mask[off >> 3]) |
> +          (val & ~(phb->ro_mask[off >> 3]));
> +
>       /* Record whether it changed */
>       changed = phb->regs[off >> 3] != val;
>   
> @@ -808,7 +812,7 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off, uint64_t val,
>       case PHB_TCE_TAG_ENABLE:
>       case PHB_INT_NOTIFY_ADDR:
>       case PHB_INT_NOTIFY_INDEX:
> -    case PHB_DMARD_SYNC:
> +    case PHB_DMA_SYNC:
>          break;
>   
>       /* Noise on anything else */
> @@ -846,7 +850,7 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
>       case PHB_VERSION:
>           return PNV_PHB4_PEC_GET_CLASS(phb->pec)->version;
>   
> -        /* Read-only */
> +    /* Read-only */
>       case PHB_PHB4_GEN_CAP:
>           return 0xe4b8000000000000ull;
>       case PHB_PHB4_TCE_CAP:
> @@ -856,18 +860,49 @@ static uint64_t pnv_phb4_reg_read(void *opaque, hwaddr off, unsigned size)
>       case PHB_PHB4_EEH_CAP:
>           return phb->big_phb ? 0x2000000000000000ull : 0x1000000000000000ull;
>   
> +    /* Write-only, read will return zeros */
> +    case PHB_LEM_ERROR_AND_MASK:
> +    case PHB_LEM_ERROR_OR_MASK:
> +        return 0;
> +    case PHB_PCIE_DLP_TRWCTL:
> +        val &= ~PHB_PCIE_DLP_TRWCTL_WREN;
> +        return val;
>       /* IODA table accesses */
>       case PHB_IODA_DATA0:
>           return pnv_phb4_ioda_read(phb);
>   
> +    /*
> +     * DMA sync: make it look like it's complete,
> +     *           clear write-only read/write start sync bits.
> +     */
> +    case PHB_DMA_SYNC:
> +        val = PHB_DMA_SYNC_RD_COMPLETE |
> +            ~(PHB_DMA_SYNC_RD_START | PHB_DMA_SYNC_WR_START);
> +        return val;
> +
> +    /*
> +     * PCI-E Stack registers
> +     */
> +    case PHB_PCIE_SCR:
> +        val |= PHB_PCIE_SCR_PLW_X16; /* RO bit */
> +        break;
> +
>       /* Link training always appears trained */
>       case PHB_PCIE_DLP_TRAIN_CTL:
>           /* TODO: Do something sensible with speed ? */
> -        return PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
> +        val |= PHB_PCIE_DLP_INBAND_PRESENCE | PHB_PCIE_DLP_TL_LINKACT;
> +        return val;
> +
> +    case PHB_PCIE_HOTPLUG_STATUS:
> +        /* Clear write-only bit */
> +        val &= ~PHB_PCIE_HPSTAT_RESAMPLE;
> +        return val;
>   
> -    /* DMA read sync: make it look like it's complete */
> -    case PHB_DMARD_SYNC:
> -        return PHB_DMARD_SYNC_COMPLETE;
> +    /* Link Management Register */
> +    case PHB_PCIE_LMR:
> +        /* These write-only bits always read as 0 */
> +        val &= ~(PHB_PCIE_LMR_CHANGELW | PHB_PCIE_LMR_RETRAINLINK);
> +        return val;
>   
>       /* Silent simple reads */
>       case PHB_LSI_SOURCE_ID:
> @@ -1712,6 +1747,33 @@ static PCIIOMMUOps pnv_phb4_iommu_ops = {
>       .get_address_space = pnv_phb4_dma_iommu,
>   };
>   
> +static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
> +{
> +    /* Clear RO-mask to make all regs as R/W by default */
> +    memset(phb->ro_mask, 0x0, PNV_PHB4_NUM_REGS * sizeof(uint64_t));
> +
> +    /*
> +     * Set register specific RO-masks
> +     */
> +
> +    /* PBL - Error Injection Register (0x1910) */
> +    phb->ro_mask[PHB_PBL_ERR_INJECT >> 3] =
> +        PPC_BITMASK(0, 23) | PPC_BITMASK(28, 35) | PPC_BIT(38) | PPC_BIT(46) |
> +        PPC_BITMASK(49, 51) | PPC_BITMASK(55, 63);
> +
> +    /* Reserved bits[60:63] */
> +    phb->ro_mask[PHB_TXE_ERR_LEM_ENABLE >> 3] =
> +    phb->ro_mask[PHB_TXE_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(60, 63);
> +    /* Reserved bits[36:63] */
> +    phb->ro_mask[PHB_RXE_TCE_ERR_LEM_ENABLE >> 3] =
> +    phb->ro_mask[PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(36, 63);
> +    /* Reserved bits[40:63] */
> +    phb->ro_mask[PHB_ERR_LEM_ENABLE >> 3] =
> +    phb->ro_mask[PHB_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(40, 63);


These constant values should be defined in an array under PnvPhb4Class

Thanks,

C.

> +    /* TODO: Add more RO-masks as regs are implemented in the model */
> +}
> +
>   static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
>   {
>       STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
> @@ -1782,6 +1844,9 @@ static void pnv_phb4_instance_init(Object *obj)
>   
>       /* XIVE interrupt source object */
>       object_initialize_child(obj, "source", &phb->xsrc, TYPE_XIVE_SOURCE);
> +
> +    /* Initialize RO-mask of registers */
> +    pnv_phb4_ro_mask_init(phb);
>   }
>   
>   void pnv_phb4_bus_init(DeviceState *dev, PnvPHB4 *phb)
> diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
> index 3212e68160..91e81eee0e 100644
> --- a/include/hw/pci-host/pnv_phb4.h
> +++ b/include/hw/pci-host/pnv_phb4.h
> @@ -94,6 +94,13 @@ struct PnvPHB4 {
>       uint64_t regs[PNV_PHB4_NUM_REGS];
>       MemoryRegion mr_regs;
>   
> +    /*
> +     * Read-only bitmask for registers
> +     * Bit value: 1 => RO bit
> +     *            0 => RW bit
> +     */
> +    uint64_t ro_mask[PNV_PHB4_NUM_REGS];> +
>       /* Extra SCOM-only register */
>       uint64_t scom_hv_ind_addr_reg;
>   
> diff --git a/include/hw/pci-host/pnv_phb4_regs.h b/include/hw/pci-host/pnv_phb4_regs.h
> index df5e86d29a..391d6a89ea 100644
> --- a/include/hw/pci-host/pnv_phb4_regs.h
> +++ b/include/hw/pci-host/pnv_phb4_regs.h
> @@ -180,9 +180,11 @@
>   #define PHB_M64_AOMASK                  0x1d0
>   #define PHB_M64_UPPER_BITS              0x1f0
>   #define PHB_NXLATE_PREFIX               0x1f8
> -#define PHB_DMARD_SYNC                  0x200
> -#define   PHB_DMARD_SYNC_START          PPC_BIT(0)
> -#define   PHB_DMARD_SYNC_COMPLETE       PPC_BIT(1)
> +#define PHB_DMA_SYNC                    0x200
> +#define   PHB_DMA_SYNC_RD_START         PPC_BIT(0)
> +#define   PHB_DMA_SYNC_RD_COMPLETE      PPC_BIT(1)
> +#define   PHB_DMA_SYNC_WR_START         PPC_BIT(2)
> +#define   PHB_DMA_SYNC_WR_COMPLETE      PPC_BIT(3)
>   #define PHB_RTC_INVALIDATE              0x208
>   #define   PHB_RTC_INVALIDATE_ALL        PPC_BIT(0)
>   #define   PHB_RTC_INVALIDATE_RID        PPC_BITMASK(16, 31)
> @@ -395,8 +397,8 @@
>   #define PHB_PCIE_SCR                    0x1A00
>   #define   PHB_PCIE_SCR_SLOT_CAP         PPC_BIT(15)
>   #define   PHB_PCIE_SCR_MAXLINKSPEED     PPC_BITMASK(32, 35)
> +#define   PHB_PCIE_SCR_PLW_X16          PPC_BIT(41) /* x16 */
>   #define PHB_PCIE_BNR                    0x1A08
> -
>   #define PHB_PCIE_CRESET                 0x1A10
>   #define   PHB_PCIE_CRESET_CFG_CORE      PPC_BIT(0)
>   #define   PHB_PCIE_CRESET_TLDLP         PPC_BIT(1)
> @@ -405,7 +407,14 @@
>   #define   PHB_PCIE_CRESET_PIPE_N        PPC_BIT(4)
>   #define   PHB_PCIE_CRESET_REFCLK_N      PPC_BIT(8)
>   #define PHB_PCIE_HOTPLUG_STATUS         0x1A20
> +#define   PHB_PCIE_HPSTAT_SIMDIAG       PPC_BIT(3)
> +#define   PHB_PCIE_HPSTAT_RESAMPLE      PPC_BIT(9)
>   #define   PHB_PCIE_HPSTAT_PRESENCE      PPC_BIT(10)
> +#define   PHB_PCIE_HPSTAT_LINKACTIVE    PPC_BIT(12)
> +#define PHB_PCIE_LMR                    0x1A30
> +#define   PHB_PCIE_LMR_CHANGELW         PPC_BIT(0)
> +#define   PHB_PCIE_LMR_RETRAINLINK      PPC_BIT(1)
> +#define   PHB_PCIE_LMR_LINKACTIVE       PPC_BIT(8)
>   
>   #define PHB_PCIE_DLP_TRAIN_CTL          0x1A40
>   #define   PHB_PCIE_DLP_LINK_WIDTH       PPC_BITMASK(30, 35)
> @@ -433,7 +442,7 @@
>   
>   #define PHB_PCIE_DLP_TRWCTL             0x1A80
>   #define   PHB_PCIE_DLP_TRWCTL_EN        PPC_BIT(0)
> -
> +#define   PHB_PCIE_DLP_TRWCTL_WREN      PPC_BIT(1)
>   #define PHB_PCIE_DLP_ERRLOG1            0x1AA0
>   #define PHB_PCIE_DLP_ERRLOG2            0x1AA8
>   #define PHB_PCIE_DLP_ERR_STATUS         0x1AB0
> diff --git a/tests/qtest/pnv-phb4-test.c b/tests/qtest/pnv-phb4-test.c
> index 708df3867c..0c8e58dd5f 100644
> --- a/tests/qtest/pnv-phb4-test.c
> +++ b/tests/qtest/pnv-phb4-test.c
> @@ -75,7 +75,8 @@ static void phb4_sticky_rst_test(QTestState *qts)
>        * Sticky reset test of PHB_PBL_ERR_STATUS.
>        *
>        * Write all 1's to reg PHB_PBL_ERR_INJECT.
> -     * Updated value will be copied to reg PHB_PBL_ERR_STATUS.
> +     * RO-only bits will not be written and
> +     * updated value will be copied to reg PHB_PBL_ERR_STATUS.
>        *
>        * Reset PBL core by setting PHB_PCIE_CRESET_PBL in reg PHB_PCIE_CRESET.
>        * Verify the sticky bits are still set.
> @@ -83,7 +84,59 @@ static void phb4_sticky_rst_test(QTestState *qts)
>       pnv_phb4_xscom_write(qts, PHB_PBL_ERR_INJECT, PPC_BITMASK(0, 63));
>       pnv_phb4_xscom_write(qts, PHB_PCIE_CRESET, PHB_PCIE_CRESET_PBL); /*Reset*/
>       val = pnv_phb4_xscom_read(qts, PHB_PBL_ERR_STATUS);
> -    g_assert_cmpuint(val, ==, (PPC_BITMASK(0, 9) | PPC_BITMASK(12, 63)));
> +    g_assert_cmpuint(val, ==, 0xF00DFD8E00);
> +}
> +
> +/* Check that write-only bits/regs return 0 when read */
> +static void phb4_writeonly_read_test(QTestState *qts)
> +{
> +    uint64_t val;
> +
> +    /*
> +     * Set all bits of PHB_DMA_SYNC,
> +     * bits 0 and 2 are write-only and should be read as 0.
> +     */
> +    pnv_phb4_xscom_write(qts, PHB_DMA_SYNC, PPC_BITMASK(0, 63));
> +    val = pnv_phb4_xscom_read(qts, PHB_DMA_SYNC);
> +    g_assert_cmpuint(val & PPC_BIT(0), ==, 0x0);
> +    g_assert_cmpuint(val & PPC_BIT(2), ==, 0x0);
> +
> +    /*
> +     * Set all bits of PHB_PCIE_HOTPLUG_STATUS,
> +     * bit 9 is write-only and should be read as 0.
> +     */
> +    pnv_phb4_xscom_write(qts, PHB_PCIE_HOTPLUG_STATUS, PPC_BITMASK(0, 63));
> +    val = pnv_phb4_xscom_read(qts, PHB_PCIE_HOTPLUG_STATUS);
> +    g_assert_cmpuint(val & PPC_BIT(9), ==, 0x0);
> +
> +    /*
> +     * Set all bits of PHB_PCIE_LMR,
> +     * bits 0 and 1 are write-only and should be read as 0.
> +     */
> +    pnv_phb4_xscom_write(qts, PHB_PCIE_LMR, PPC_BITMASK(0, 63));
> +    val = pnv_phb4_xscom_read(qts, PHB_PCIE_LMR);
> +    g_assert_cmpuint(val & PPC_BIT(0), ==, 0x0);
> +    g_assert_cmpuint(val & PPC_BIT(1), ==, 0x0);
> +
> +    /*
> +     * Set all bits of PHB_PCIE_DLP_TRWCTL,
> +     * write-only bit-1 should be read as 0.
> +     */
> +    pnv_phb4_xscom_write(qts, PHB_PCIE_DLP_TRWCTL, PPC_BITMASK(0, 63));
> +    val = pnv_phb4_xscom_read(qts, PHB_PCIE_DLP_TRWCTL);
> +    g_assert_cmpuint(val & PPC_BIT(1), ==, 0x0);
> +
> +    /*
> +     * Set all bits of PHB_LEM_ERROR_AND_MASK, PHB_LEM_ERROR_OR_MASK,
> +     * both regs are write-only and should be read as 0.
> +     */
> +    pnv_phb4_xscom_write(qts, PHB_LEM_ERROR_AND_MASK, PPC_BITMASK(0, 63));
> +    val = pnv_phb4_xscom_read(qts, PHB_LEM_ERROR_AND_MASK);
> +    g_assert_cmpuint(val, ==, 0x0);
> +
> +    pnv_phb4_xscom_write(qts, PHB_LEM_ERROR_OR_MASK, PPC_BITMASK(0, 63));
> +    val = pnv_phb4_xscom_read(qts, PHB_LEM_ERROR_OR_MASK);
> +    g_assert_cmpuint(val, ==, 0x0);
>   }
>   
>   static void test_phb4(void)
> @@ -101,6 +154,9 @@ static void test_phb4(void)
>       /* Check sticky reset of a register */
>       phb4_sticky_rst_test(qts);
>   
> +    /* Check write-only logic */
> +    phb4_writeonly_read_test(qts);
> +
>       qtest_quit(qts);
>   }
>   



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

* Re: [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register
  2024-03-25 13:35   ` Cédric Le Goater
@ 2024-03-25 14:37     ` Cornelia Huck
  0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2024-03-25 14:37 UTC (permalink / raw)
  To: Cédric Le Goater, Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, pbonzini, thuth, lvivier

On Mon, Mar 25 2024, Cédric Le Goater <clg@kaod.org> wrote:

> On 3/21/24 11:04, Saif Abrar wrote:
>> PHB updates the register PCIE Link-Control-2.
>> Set the write-mask bits for TLS, ENTER_COMP, TX_MARGIN,
>> HASD, MOD_COMP, COMP_SOS and COMP_P_DE.
>
>
> You should resend this patch independently of the PowerNV PHB changes.
>
>
> Thanks,
>
> C.
>
>
>
>> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
>> ---
>>   hw/pci/pcie.c                             | 6 ++++++
>>   include/standard-headers/linux/pci_regs.h | 3 +++
>>   2 files changed, 9 insertions(+)

This patch also needs to be split: the code under standard-headers/ is
updated via a Linux headers update, which should either be a full update
against a released kernel version (can be -rc), or a placeholder patch
if the changes have not yet been merged.



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

* Re: [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts
  2024-03-25 13:34   ` Cédric Le Goater
@ 2024-03-27  9:59     ` Saif Abrar
  2024-03-27 16:19       ` Cédric Le Goater
  2024-03-28  9:31     ` Saif Abrar
  1 sibling, 1 reply; 23+ messages in thread
From: Saif Abrar @ 2024-03-27  9:59 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

Hello Cedric,

>   }
>   +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
>
> What is fund_A ?

I used 'fund_A' as an abbreviation to "Fundamental Register Set A".

Please let know if you suggest another abbreviation to name this method.


>> +{
>> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
>
> Is this mask the default value for HW ?
Yes, the spec defines the bits[04:12] of LSI Source ID having reset 
value: 0x1FF


Regards,

Saif


On 25-03-2024 07:04 pm, Cédric Le Goater wrote:
> On 3/21/24 11:04, Saif Abrar wrote:
>> Add a method to reset the value of LSI Source-ID.
>> Mask off LSI source-id based on number of interrupts in the big/small 
>> PHB.
>
> Looks ok.
>
>
>> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
>> ---
>>   hw/pci-host/pnv_phb4.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index f48750ee54..8fbaf6512e 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -489,6 +489,7 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
>>         lsi_base = GETFIELD(PHB_LSI_SRC_ID, 
>> phb->regs[PHB_LSI_SOURCE_ID >> 3]);
>>       lsi_base <<= 3;
>> +    lsi_base &= (xsrc->nr_irqs - 1);
>>         /* TODO: handle reset values of PHB_LSI_SRC_ID */
>>       if (!lsi_base) {
>> @@ -1966,6 +1967,12 @@ static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
>>       /* TODO: Add more RO-masks as regs are implemented in the model */
>>   }
>>   +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
>
> What is fund_A ?
>
>> +{
>> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
>
> Is this mask the default value for HW ?
>
>
> Thanks,
>
> C.
>
>
>> +    pnv_phb4_update_xsrc(phb);
>> +}
>> +
>>   static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
>>   {
>>       STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
>> @@ -2023,6 +2030,7 @@ static void pnv_phb4_reset(void *dev)
>>       pnv_phb4_cfg_core_reset(phb);
>>       pnv_phb4_pbl_core_reset(phb);
>>   +    pnv_phb4_fund_A_reset(phb);
>>       pnv_phb4_err_reg_reset(phb);
>>       pnv_phb4_pcie_stack_reg_reset(phb);
>>       pnv_phb4_regb_err_reg_reset(phb);
>> @@ -2102,8 +2110,6 @@ static void pnv_phb4_realize(DeviceState *dev, 
>> Error **errp)
>>           return;
>>       }
>>   -    pnv_phb4_update_xsrc(phb);
>> -
>>       phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, 
>> xsrc->nr_irqs);
>>         pnv_phb4_xscom_realize(phb);
>


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

* Re: [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts
  2024-03-27  9:59     ` Saif Abrar
@ 2024-03-27 16:19       ` Cédric Le Goater
  0 siblings, 0 replies; 23+ messages in thread
From: Cédric Le Goater @ 2024-03-27 16:19 UTC (permalink / raw)
  To: Saif Abrar, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

On 3/27/24 10:59, Saif Abrar wrote:
> Hello Cedric,
> 
>>   }
>>   +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
>>
>> What is fund_A ?
> 
> I used 'fund_A' as an abbreviation to "Fundamental Register Set A".
> 
> Please let know if you suggest another abbreviation to name this method.

pnv_phb4_reset_xsrc may be ?


Thanks,

C.

> 
>>> +{
>>> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
>>
>> Is this mask the default value for HW ?
> Yes, the spec defines the bits[04:12] of LSI Source ID having reset value: 0x1FF
> 
> 
> Regards,
> 
> Saif
> 
> 
> On 25-03-2024 07:04 pm, Cédric Le Goater wrote:
>> On 3/21/24 11:04, Saif Abrar wrote:
>>> Add a method to reset the value of LSI Source-ID.
>>> Mask off LSI source-id based on number of interrupts in the big/small PHB.
>>
>> Looks ok.
>>
>>
>>> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
>>> ---
>>>   hw/pci-host/pnv_phb4.c | 10 ++++++++--
>>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>>> index f48750ee54..8fbaf6512e 100644
>>> --- a/hw/pci-host/pnv_phb4.c
>>> +++ b/hw/pci-host/pnv_phb4.c
>>> @@ -489,6 +489,7 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
>>>         lsi_base = GETFIELD(PHB_LSI_SRC_ID, phb->regs[PHB_LSI_SOURCE_ID >> 3]);
>>>       lsi_base <<= 3;
>>> +    lsi_base &= (xsrc->nr_irqs - 1);
>>>         /* TODO: handle reset values of PHB_LSI_SRC_ID */
>>>       if (!lsi_base) {
>>> @@ -1966,6 +1967,12 @@ static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
>>>       /* TODO: Add more RO-masks as regs are implemented in the model */
>>>   }
>>>   +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
>>
>> What is fund_A ?
>>
>>> +{
>>> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
>>
>> Is this mask the default value for HW ?
>>
>>
>> Thanks,
>>
>> C.
>>
>>
>>> +    pnv_phb4_update_xsrc(phb);
>>> +}
>>> +
>>>   static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
>>>   {
>>>       STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
>>> @@ -2023,6 +2030,7 @@ static void pnv_phb4_reset(void *dev)
>>>       pnv_phb4_cfg_core_reset(phb);
>>>       pnv_phb4_pbl_core_reset(phb);
>>>   +    pnv_phb4_fund_A_reset(phb);
>>>       pnv_phb4_err_reg_reset(phb);
>>>       pnv_phb4_pcie_stack_reg_reset(phb);
>>>       pnv_phb4_regb_err_reg_reset(phb);
>>> @@ -2102,8 +2110,6 @@ static void pnv_phb4_realize(DeviceState *dev, Error **errp)
>>>           return;
>>>       }
>>>   -    pnv_phb4_update_xsrc(phb);
>>> -
>>>       phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, xsrc->nr_irqs);
>>>         pnv_phb4_xscom_realize(phb);
>>



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

* Re: [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts
  2024-03-25 13:34   ` Cédric Le Goater
  2024-03-27  9:59     ` Saif Abrar
@ 2024-03-28  9:31     ` Saif Abrar
  1 sibling, 0 replies; 23+ messages in thread
From: Saif Abrar @ 2024-03-28  9:31 UTC (permalink / raw)
  To: Cédric Le Goater, qemu-ppc, qemu-devel
  Cc: npiggin, fbarrat, mst, marcel.apfelbaum, cohuck, pbonzini, thuth,
	lvivier

Hello Cedric,

>   }
>   +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
>
> What is fund_A ?

I used 'fund_A' as an abbreviation to "Fundamental Register Set A".

Please let know if you suggest another abbreviation to name this method.


>> +{
>> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
>
> Is this mask the default value for HW ?
Yes, the spec defines the bits[04:12] of LSI Source ID having reset 
value: 0x1FF


Regards,

Saif


On 25-03-2024 07:04 pm, Cédric Le Goater wrote:
> On 3/21/24 11:04, Saif Abrar wrote:
>> Add a method to reset the value of LSI Source-ID.
>> Mask off LSI source-id based on number of interrupts in the big/small 
>> PHB.
>
> Looks ok.
>
>
>> Signed-off-by: Saif Abrar <saif.abrar@linux.vnet.ibm.com>
>> ---
>>   hw/pci-host/pnv_phb4.c | 10 ++++++++--
>>   1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
>> index f48750ee54..8fbaf6512e 100644
>> --- a/hw/pci-host/pnv_phb4.c
>> +++ b/hw/pci-host/pnv_phb4.c
>> @@ -489,6 +489,7 @@ static void pnv_phb4_update_xsrc(PnvPHB4 *phb)
>>         lsi_base = GETFIELD(PHB_LSI_SRC_ID, 
>> phb->regs[PHB_LSI_SOURCE_ID >> 3]);
>>       lsi_base <<= 3;
>> +    lsi_base &= (xsrc->nr_irqs - 1);
>>         /* TODO: handle reset values of PHB_LSI_SRC_ID */
>>       if (!lsi_base) {
>> @@ -1966,6 +1967,12 @@ static void pnv_phb4_ro_mask_init(PnvPHB4 *phb)
>>       /* TODO: Add more RO-masks as regs are implemented in the model */
>>   }
>>   +static void pnv_phb4_fund_A_reset(PnvPHB4 *phb)
>
> What is fund_A ?
>
>> +{
>> +    phb->regs[PHB_LSI_SOURCE_ID >> 3] = PPC_BITMASK(4, 12);
>
> Is this mask the default value for HW ?
>
>
> Thanks,
>
> C.
>
>
>> +    pnv_phb4_update_xsrc(phb);
>> +}
>> +
>>   static void pnv_phb4_err_reg_reset(PnvPHB4 *phb)
>>   {
>>       STICKY_RST(PHB_ERR_STATUS,       0, PPC_BITMASK(0, 33));
>> @@ -2023,6 +2030,7 @@ static void pnv_phb4_reset(void *dev)
>>       pnv_phb4_cfg_core_reset(phb);
>>       pnv_phb4_pbl_core_reset(phb);
>>   +    pnv_phb4_fund_A_reset(phb);
>>       pnv_phb4_err_reg_reset(phb);
>>       pnv_phb4_pcie_stack_reg_reset(phb);
>>       pnv_phb4_regb_err_reg_reset(phb);
>> @@ -2102,8 +2110,6 @@ static void pnv_phb4_realize(DeviceState *dev, 
>> Error **errp)
>>           return;
>>       }
>>   -    pnv_phb4_update_xsrc(phb);
>> -
>>       phb->qirqs = qemu_allocate_irqs(xive_source_set_irq, xsrc, 
>> xsrc->nr_irqs);
>>         pnv_phb4_xscom_realize(phb);
>


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

end of thread, other threads:[~2024-03-28  9:32 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-21 10:04 [PATCH 00/10] pnv/phb4: Update PHB4 to the latest spec PH5 Saif Abrar
2024-03-21 10:04 ` [PATCH 01/10] qtest/phb4: Add testbench for PHB4 Saif Abrar
2024-03-25  9:39   ` Cédric Le Goater
2024-03-21 10:04 ` [PATCH 02/10] pnv/phb4: Add reset logic to PHB4 Saif Abrar
2024-03-25 13:32   ` Cédric Le Goater
2024-03-21 10:04 ` [PATCH 03/10] pnv/phb4: Implement sticky reset logic in PHB4 Saif Abrar
2024-03-21 10:04 ` [PATCH 04/10] pnv/phb4: Implement read-only and write-only bits of registers Saif Abrar
2024-03-25 14:15   ` Cédric Le Goater
2024-03-21 10:04 ` [PATCH 05/10] pnv/phb4: Implement write-clear and return 1's on unimplemented reg read Saif Abrar
2024-03-25 13:58   ` Cédric Le Goater
2024-03-21 10:04 ` [PATCH 06/10] pnv/phb4: Set link-active status in HPSTAT and LMR registers Saif Abrar
2024-03-21 10:04 ` [PATCH 07/10] pnv/phb4: Set link speed and width in the DLP training control register Saif Abrar
2024-03-25 13:36   ` Cédric Le Goater
2024-03-21 10:04 ` [PATCH 08/10] pnv/phb4: Implement IODA PCT table Saif Abrar
2024-03-25 13:35   ` Cédric Le Goater
2024-03-21 10:04 ` [PATCH 09/10] hw/pci: Set write-mask bits for PCIE Link-Control-2 register Saif Abrar
2024-03-25 13:35   ` Cédric Le Goater
2024-03-25 14:37     ` Cornelia Huck
2024-03-21 10:04 ` [PATCH 10/10] pnv/phb4: Mask off LSI Source-ID based on number of interrupts Saif Abrar
2024-03-25 13:34   ` Cédric Le Goater
2024-03-27  9:59     ` Saif Abrar
2024-03-27 16:19       ` Cédric Le Goater
2024-03-28  9:31     ` Saif Abrar

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.