From: Saif Abrar<[email protected]>
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.
Add .class_size in pnv_phb4_type_info.
Signed-off-by: Saif Abrar<[email protected]>
Reviewed-by: Cédric Le Goater<[email protected]>
Reviewed-by: "Michael S. Tsirkin"<[email protected]>
Reviewed-by: Caleb Schlossin<[email protected]>
Reviewed-by: Aditya Gupta<[email protected]>
---
hw/pci-host/pnv_phb4.c | 79 ++++++++++++++++++++++++++---
include/hw/pci-host/pnv_phb4.h | 13 ++++-
include/hw/pci-host/pnv_phb4_regs.h | 19 +++++--
tests/qtest/pnv-phb-test.c | 63 ++++++++++++++++++++++-
4 files changed, 160 insertions(+), 14 deletions(-)
diff --git a/hw/pci-host/pnv_phb4.c b/hw/pci-host/pnv_phb4.c
index a91b8277d6..9730fd1f8a 100644
--- a/hw/pci-host/pnv_phb4.c
+++ b/hw/pci-host/pnv_phb4.c
@@ -654,6 +654,12 @@ static void pnv_phb4_reg_write(void *opaque, hwaddr off,
uint64_t val,
return;
}
+ /* Update 'val' according to the register's RO-mask */
+ PnvPHB4Class *k = PNV_PHB4_GET_CLASS(phb);
+
+ val = (phb->regs[off >> 3] & k->ro_mask[off >> 3]) |
+ (val & ~(k->ro_mask[off >> 3]));
+
/* Record whether it changed */
changed = phb->regs[off >> 3] != val;
@@ -729,7 +735,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 */
@@ -767,7 +773,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:
@@ -777,18 +783,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:
@@ -1633,6 +1670,30 @@ static PCIIOMMUOps pnv_phb4_iommu_ops = {
.get_address_space = pnv_phb4_dma_iommu,
};
+static void pnv_phb4_ro_mask_init(PnvPHB4Class *phb4c)
+{
+ /*
+ * Set register specific RO-masks
+ */
+
+ /* PBL - Error Injection Register (0x1910) */
+ phb4c->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] */
+ phb4c->ro_mask[PHB_TXE_ERR_LEM_ENABLE >> 3] =
+ phb4c->ro_mask[PHB_TXE_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(60, 63);
+ /* Reserved bits[36:63] */
+ phb4c->ro_mask[PHB_RXE_TCE_ERR_LEM_ENABLE >> 3] =
+ phb4c->ro_mask[PHB_RXE_TCE_ERR_AIB_FENCE_ENABLE >> 3] = PPC_BITMASK(36,
63);
+ /* Reserved bits[40:63] */
+ phb4c->ro_mask[PHB_ERR_LEM_ENABLE >> 3] =
+ phb4c->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));
@@ -1692,6 +1753,7 @@ static void pnv_phb4_reset(Object *obj, ResetType type)
pnv_phb4_err_reg_reset(phb);
pnv_phb4_pcie_stack_reg_reset(phb);
pnv_phb4_regb_err_reg_reset(phb);
+ phb->regs[PHB_PCIE_CRESET >> 3] = 0xE000000000000000;
}
static void pnv_phb4_instance_init(Object *obj)
@@ -1866,6 +1928,7 @@ static void pnv_phb4_class_init(ObjectClass *klass, const
void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
XiveNotifierClass *xfc = XIVE_NOTIFIER_CLASS(klass);
ResettableClass *rc = RESETTABLE_CLASS(klass);
+ PnvPHB4Class *phb4c = PNV_PHB4_CLASS(klass);
dc->realize = pnv_phb4_realize;
device_class_set_props(dc, pnv_phb4_properties);
@@ -1874,6 +1937,9 @@ static void pnv_phb4_class_init(ObjectClass *klass, const
void *data)
xfc->notify = pnv_phb4_xive_notify;
rc->phases.enter = pnv_phb4_reset;
+
+ /* Initialize RO-mask of registers */
+ pnv_phb4_ro_mask_init(phb4c);
}
static const TypeInfo pnv_phb4_type_info = {
@@ -1881,6 +1947,7 @@ static const TypeInfo pnv_phb4_type_info = {
.parent = TYPE_DEVICE,
.instance_init = pnv_phb4_instance_init,
.instance_size = sizeof(PnvPHB4),
+ .class_size = sizeof(PnvPHB4Class),
.class_init = pnv_phb4_class_init,
.interfaces = (const InterfaceInfo[]) {
{ TYPE_XIVE_NOTIFIER },
diff --git a/include/hw/pci-host/pnv_phb4.h b/include/hw/pci-host/pnv_phb4.h
index 95ba26db53..567a3e6e95 100644
--- a/include/hw/pci-host/pnv_phb4.h
+++ b/include/hw/pci-host/pnv_phb4.h
@@ -19,7 +19,7 @@
#define TYPE_PNV_PHB4 "pnv-phb4"
-OBJECT_DECLARE_SIMPLE_TYPE(PnvPHB4, PNV_PHB4)
+OBJECT_DECLARE_TYPE(PnvPHB4, PnvPHB4Class, PNV_PHB4)
typedef struct PnvPhb4PecStack PnvPhb4PecStack;
@@ -156,6 +156,17 @@ struct PnvPHB4 {
QLIST_HEAD(, PnvPhb4DMASpace) dma_spaces;
};
+typedef struct PnvPHB4Class {
+ DeviceClass parent_class;
+
+ /*
+ * Read-only bitmask for registers
+ * Bit value: 1 => RO bit
+ * 0 => RW bit
+ */
+ uint64_t ro_mask[PNV_PHB4_NUM_REGS];
+} PnvPHB4Class;
+
void pnv_phb4_pic_print_info(PnvPHB4 *phb, GString *buf);
int pnv_phb4_pec_get_phb_id(PnvPhb4PecState *pec, int stack_index);
PnvPhb4PecState *pnv_pec_add_phb(PnvChip *chip, PnvPHB *phb, Error **errp);
diff --git a/include/hw/pci-host/pnv_phb4_regs.h
b/include/hw/pci-host/pnv_phb4_regs.h
index 59329068d3..d0962c1607 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)
@@ -387,8 +389,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)
@@ -397,7 +399,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)
@@ -425,7 +434,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-phb-test.c b/tests/qtest/pnv-phb-test.c
index 2294cf5525..23238f1582 100644
--- a/tests/qtest/pnv-phb-test.c
+++ b/tests/qtest/pnv-phb-test.c
@@ -76,7 +76,8 @@ static void phb5_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.
@@ -84,7 +85,62 @@ static void phb5_sticky_rst_test(QTestState *qts)
PHB5_XSCOM_WRITE(PHB_PBL_ERR_INJECT, PPC_BITMASK(0, 63));
PHB5_XSCOM_WRITE(PHB_PCIE_CRESET, PHB_PCIE_CRESET_PBL); /*Reset*/
val = PHB5_XSCOM_READ(PHB_PBL_ERR_STATUS);
- g_assert_cmpuint(val, ==, (PPC_BITMASK(0, 9) | PPC_BITMASK(12, 63)));
+ g_assert_cmpuint(val, ==, (PPC_BITMASK(24, 27) | PPC_BITMASK(36, 37) |
+ PPC_BITMASK(39, 45) | PPC_BITMASK(47, 48) |
+ PPC_BITMASK(52, 54)));
+
+}
+
+/* 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.
+ */
+ PHB5_XSCOM_WRITE(PHB_DMA_SYNC, PPC_BITMASK(0, 63));
+ val = PHB5_XSCOM_READ(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.
+ */
+ PHB5_XSCOM_WRITE(PHB_PCIE_HOTPLUG_STATUS, PPC_BITMASK(0, 63));
+ val = PHB5_XSCOM_READ(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.
+ */
+ PHB5_XSCOM_WRITE(PHB_PCIE_LMR, PPC_BITMASK(0, 63));
+ val = PHB5_XSCOM_READ(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.
+ */
+ PHB5_XSCOM_WRITE(PHB_PCIE_DLP_TRWCTL, PPC_BITMASK(0, 63));
+ val = PHB5_XSCOM_READ(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.
+ */
+ PHB5_XSCOM_WRITE(PHB_LEM_ERROR_AND_MASK, PPC_BITMASK(0, 63));
+ val = PHB5_XSCOM_READ(PHB_LEM_ERROR_AND_MASK);
+ g_assert_cmpuint(val, ==, 0x0);
+
+ PHB5_XSCOM_WRITE(PHB_LEM_ERROR_OR_MASK, PPC_BITMASK(0, 63));
+ val = PHB5_XSCOM_READ(PHB_LEM_ERROR_OR_MASK);
+ g_assert_cmpuint(val, ==, 0x0);
}
static void phb5_tests(void)
@@ -99,6 +155,9 @@ static void phb5_tests(void)
/* Check sticky reset of a register */
phb5_sticky_rst_test(qts);
+ /* Check write-only logic */
+ phb4_writeonly_read_test(qts);
+
qtest_quit(qts);
}
