hw/ppc/pnv_core.c | 3 +-- include/hw/ppc/pnv_xscom.h | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-)
The P10 core xscom memory regions overlap because the size is wrong.
The P10 core+L2 xscom region size is allocated as 0x1000 (with some
unused ranges). "EC" is used as a closer match, as "EX" includes L3
which has a disjoint xscom range that would require a different
region if it were implemented.
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
hw/ppc/pnv_core.c | 3 +--
include/hw/ppc/pnv_xscom.h | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index b7223bb445..ffbc29cbf4 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -299,9 +299,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
}
snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id);
- /* TODO: check PNV_XSCOM_EX_SIZE for p10 */
pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops,
- pc, name, PNV_XSCOM_EX_SIZE);
+ pc, name, PNV10_XSCOM_EC_SIZE);
qemu_register_reset(pnv_core_reset, pc);
return;
diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
index f7da9a1dc6..a4c9d95dc5 100644
--- a/include/hw/ppc/pnv_xscom.h
+++ b/include/hw/ppc/pnv_xscom.h
@@ -133,7 +133,7 @@ struct PnvXScomInterfaceClass {
#define PNV10_XSCOM_EC_BASE(core) \
((uint64_t) PNV10_XSCOM_EQ_BASE(core) | PNV10_XSCOM_EC(core & 0x3))
-#define PNV10_XSCOM_EC_SIZE 0x100000
+#define PNV10_XSCOM_EC_SIZE 0x1000
#define PNV10_XSCOM_PSIHB_BASE 0x3011D00
#define PNV10_XSCOM_PSIHB_SIZE 0x100
--
2.40.1
This patch breaks make check-qtest: $ make -j -C build && make -C build check-qtest (...) 16/44 qemu:qtest+qtest-ppc64 / qtest-ppc64/pnv-xscom-test ERROR 0.89s killed by signal 6 SIGABRT >>> G_TEST_DBUS_DAEMON=/home/danielhb/powerpc/qemu/tests/dbus-vmstate-daemon.sh QTEST_QEMU_IMG=./qemu-img QTEST_QEMU_BINARY=./qemu-system-ppc64 QTEST_QEMU_STORAGE_DAEMON_BINARY=./storage-daemon/qemu-storage-daemon MALLOC_PERTURB_=231 /home/danielhb/powerpc/qemu/build/tests/qtest/pnv-xscom-test --tap -k ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― ✀ ―――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――― stderr: ** ERROR:../tests/qtest/pnv-xscom-test.c:115:test_xscom_core: assertion failed (dts0 == 0x26f024f023f0000ull): (0x00000000 == 0x26f024f023f0000) (test program exited with status code -6) ―――――――――――――――――――――――――――――――――――――――――――――――― I believe you'll need to take a look at tests/qtest/pnv-xscom-test.c as well. Thanks, Daniel On 7/4/23 22:27, Nicholas Piggin wrote: > The P10 core xscom memory regions overlap because the size is wrong. > The P10 core+L2 xscom region size is allocated as 0x1000 (with some > unused ranges). "EC" is used as a closer match, as "EX" includes L3 > which has a disjoint xscom range that would require a different > region if it were implemented. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/ppc/pnv_core.c | 3 +-- > include/hw/ppc/pnv_xscom.h | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c > index b7223bb445..ffbc29cbf4 100644 > --- a/hw/ppc/pnv_core.c > +++ b/hw/ppc/pnv_core.c > @@ -299,9 +299,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) > } > > snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id); > - /* TODO: check PNV_XSCOM_EX_SIZE for p10 */ > pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops, > - pc, name, PNV_XSCOM_EX_SIZE); > + pc, name, PNV10_XSCOM_EC_SIZE); > > qemu_register_reset(pnv_core_reset, pc); > return; > diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h > index f7da9a1dc6..a4c9d95dc5 100644 > --- a/include/hw/ppc/pnv_xscom.h > +++ b/include/hw/ppc/pnv_xscom.h > @@ -133,7 +133,7 @@ struct PnvXScomInterfaceClass { > > #define PNV10_XSCOM_EC_BASE(core) \ > ((uint64_t) PNV10_XSCOM_EQ_BASE(core) | PNV10_XSCOM_EC(core & 0x3)) > -#define PNV10_XSCOM_EC_SIZE 0x100000 > +#define PNV10_XSCOM_EC_SIZE 0x1000 > > #define PNV10_XSCOM_PSIHB_BASE 0x3011D00 > #define PNV10_XSCOM_PSIHB_SIZE 0x100
On Wed, 5 Jul 2023 at 01:27, Nicholas Piggin <npiggin@gmail.com> wrote: > > The P10 core xscom memory regions overlap because the size is wrong. > The P10 core+L2 xscom region size is allocated as 0x1000 (with some > unused ranges). "EC" is used as a closer match, as "EX" includes L3 > which has a disjoint xscom range that would require a different > region if it were implemented. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Nice, that looks better: 0000000100000000-00000001000fffff (prio 0, i/o): xscom-quad.0: 0x100000 0000000100108000-000000010010ffff (prio 0, i/o): xscom-core.3: 0x8000 0000000100110000-0000000100117fff (prio 0, i/o): xscom-core.2: 0x8000 0000000100120000-0000000100127fff (prio 0, i/o): xscom-core.1: 0x8000 0000000100140000-0000000100147fff (prio 0, i/o): xscom-core.0: 0x8000 0000000108000000-00000001080fffff (prio 0, i/o): xscom-quad.4: 0x100000 0000000108108000-000000010810ffff (prio 0, i/o): xscom-core.7: 0x8000 0000000108110000-0000000108117fff (prio 0, i/o): xscom-core.6: 0x8000 0000000108120000-0000000108127fff (prio 0, i/o): xscom-core.5: 0x8000 0000000108140000-0000000108147fff (prio 0, i/o): xscom-core.4: 0x8000 Reviewed-by: Joel Stanley <joel@jms.id.au> > --- > hw/ppc/pnv_core.c | 3 +-- > include/hw/ppc/pnv_xscom.h | 2 +- > 2 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c > index b7223bb445..ffbc29cbf4 100644 > --- a/hw/ppc/pnv_core.c > +++ b/hw/ppc/pnv_core.c > @@ -299,9 +299,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) > } > > snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id); > - /* TODO: check PNV_XSCOM_EX_SIZE for p10 */ > pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops, > - pc, name, PNV_XSCOM_EX_SIZE); > + pc, name, PNV10_XSCOM_EC_SIZE); > > qemu_register_reset(pnv_core_reset, pc); > return; > diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h > index f7da9a1dc6..a4c9d95dc5 100644 > --- a/include/hw/ppc/pnv_xscom.h > +++ b/include/hw/ppc/pnv_xscom.h > @@ -133,7 +133,7 @@ struct PnvXScomInterfaceClass { > > #define PNV10_XSCOM_EC_BASE(core) \ > ((uint64_t) PNV10_XSCOM_EQ_BASE(core) | PNV10_XSCOM_EC(core & 0x3)) > -#define PNV10_XSCOM_EC_SIZE 0x100000 > +#define PNV10_XSCOM_EC_SIZE 0x1000 > > #define PNV10_XSCOM_PSIHB_BASE 0x3011D00 > #define PNV10_XSCOM_PSIHB_SIZE 0x100 > -- > 2.40.1 >
On 7/5/23 04:05, Joel Stanley wrote: > On Wed, 5 Jul 2023 at 01:27, Nicholas Piggin <npiggin@gmail.com> wrote: >> >> The P10 core xscom memory regions overlap because the size is wrong. >> The P10 core+L2 xscom region size is allocated as 0x1000 (with some >> unused ranges). "EC" is used as a closer match, as "EX" includes L3 >> which has a disjoint xscom range that would require a different >> region if it were implemented. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > Nice, that looks better: > > 0000000100000000-00000001000fffff (prio 0, i/o): xscom-quad.0: 0x100000 > 0000000100108000-000000010010ffff (prio 0, i/o): xscom-core.3: 0x8000 > 0000000100110000-0000000100117fff (prio 0, i/o): xscom-core.2: 0x8000 > 0000000100120000-0000000100127fff (prio 0, i/o): xscom-core.1: 0x8000 > 0000000100140000-0000000100147fff (prio 0, i/o): xscom-core.0: 0x8000 > 0000000108000000-00000001080fffff (prio 0, i/o): xscom-quad.4: 0x100000 > 0000000108108000-000000010810ffff (prio 0, i/o): xscom-core.7: 0x8000 > 0000000108110000-0000000108117fff (prio 0, i/o): xscom-core.6: 0x8000 > 0000000108120000-0000000108127fff (prio 0, i/o): xscom-core.5: 0x8000 > 0000000108140000-0000000108147fff (prio 0, i/o): xscom-core.4: 0x8000 > > Reviewed-by: Joel Stanley <joel@jms.id.au> It'd interesting to add some dummy SLW handlers to get rid of the XSCOM errors at boot and shutdown on P10 : [ 4824.393446266,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0 [ 4824.393588777,5] Unable to log error [ 4824.393650582,3] XSCOM: Write failed, ret = -6 [ 4824.394124623,3] Could not set special wakeup on 0:0: Unable to write QME_SPWU_HYP. [ 4824.394368459,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0 [ 4824.394382007,5] Unable to log error [ 4824.394384603,3] XSCOM: Write failed, ret = -6 Thanks, C. > > >> --- >> hw/ppc/pnv_core.c | 3 +-- >> include/hw/ppc/pnv_xscom.h | 2 +- >> 2 files changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c >> index b7223bb445..ffbc29cbf4 100644 >> --- a/hw/ppc/pnv_core.c >> +++ b/hw/ppc/pnv_core.c >> @@ -299,9 +299,8 @@ static void pnv_core_realize(DeviceState *dev, Error **errp) >> } >> >> snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id); >> - /* TODO: check PNV_XSCOM_EX_SIZE for p10 */ >> pnv_xscom_region_init(&pc->xscom_regs, OBJECT(dev), pcc->xscom_ops, >> - pc, name, PNV_XSCOM_EX_SIZE); >> + pc, name, PNV10_XSCOM_EC_SIZE); >> >> qemu_register_reset(pnv_core_reset, pc); >> return; >> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h >> index f7da9a1dc6..a4c9d95dc5 100644 >> --- a/include/hw/ppc/pnv_xscom.h >> +++ b/include/hw/ppc/pnv_xscom.h >> @@ -133,7 +133,7 @@ struct PnvXScomInterfaceClass { >> >> #define PNV10_XSCOM_EC_BASE(core) \ >> ((uint64_t) PNV10_XSCOM_EQ_BASE(core) | PNV10_XSCOM_EC(core & 0x3)) >> -#define PNV10_XSCOM_EC_SIZE 0x100000 >> +#define PNV10_XSCOM_EC_SIZE 0x1000 >> >> #define PNV10_XSCOM_PSIHB_BASE 0x3011D00 >> #define PNV10_XSCOM_PSIHB_SIZE 0x100 >> -- >> 2.40.1 >>
On Wed, 5 Jul 2023 at 10:02, Cédric Le Goater <clg@kaod.org> wrote: > > On 7/5/23 04:05, Joel Stanley wrote: > > On Wed, 5 Jul 2023 at 01:27, Nicholas Piggin <npiggin@gmail.com> wrote: > >> > >> The P10 core xscom memory regions overlap because the size is wrong. > >> The P10 core+L2 xscom region size is allocated as 0x1000 (with some > >> unused ranges). "EC" is used as a closer match, as "EX" includes L3 > >> which has a disjoint xscom range that would require a different > >> region if it were implemented. > >> > >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > > > Nice, that looks better: > > > > 0000000100000000-00000001000fffff (prio 0, i/o): xscom-quad.0: 0x100000 > > 0000000100108000-000000010010ffff (prio 0, i/o): xscom-core.3: 0x8000 > > 0000000100110000-0000000100117fff (prio 0, i/o): xscom-core.2: 0x8000 > > 0000000100120000-0000000100127fff (prio 0, i/o): xscom-core.1: 0x8000 > > 0000000100140000-0000000100147fff (prio 0, i/o): xscom-core.0: 0x8000 > > 0000000108000000-00000001080fffff (prio 0, i/o): xscom-quad.4: 0x100000 > > 0000000108108000-000000010810ffff (prio 0, i/o): xscom-core.7: 0x8000 > > 0000000108110000-0000000108117fff (prio 0, i/o): xscom-core.6: 0x8000 > > 0000000108120000-0000000108127fff (prio 0, i/o): xscom-core.5: 0x8000 > > 0000000108140000-0000000108147fff (prio 0, i/o): xscom-core.4: 0x8000 > > > > Reviewed-by: Joel Stanley <joel@jms.id.au> > > It'd interesting to add some dummy SLW handlers to get rid of the > XSCOM errors at boot and shutdown on P10 : > > [ 4824.393446266,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0 > [ 4824.393588777,5] Unable to log error > [ 4824.393650582,3] XSCOM: Write failed, ret = -6 > [ 4824.394124623,3] Could not set special wakeup on 0:0: Unable to write QME_SPWU_HYP. > [ 4824.394368459,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0 > [ 4824.394382007,5] Unable to log error > [ 4824.394384603,3] XSCOM: Write failed, ret = -6 Yes. I was looking at this yesterday. We need to figure out how to do the xscom addressing for the QME. It sets (different) bits in order to address a given core. For a -smp 4 machine, the P10_QME_SPWU_HYP read comes in on these addresses: case 0x200e883c: case 0x200e483c: case 0x200e283c: case 0x200e183c: ie, the fourth nibble selects the core. For a -smp 8 machine, the address now has bit 24 set to select the second quad, so we need to cover these addresses: case 0x210e883c: case 0x210e483c: case 0x210e283c: case 0x210e183c: I am thinking about how to map this into an address range that a model can claim. Cheers, Joel PS. For reference, this is sufficient to silence xscom errors with skiboot and -M powernv10 -smp4. A different set of hacks is required for p9. --- a/hw/ppc/pnv_xscom.c +++ b/hw/ppc/pnv_xscom.c @@ -106,6 +106,26 @@ static uint64_t xscom_read_default(PnvChip *chip, uint32_t pcba) case 0x401082a: case 0x4010828: return 0; + + /* P10_QME_SPWU_HYP */ + case 0x200e883c: + case 0x200e483c: + case 0x200e283c: + case 0x200e183c: + return 0; + + /* P10_QME_SSH_HYP */ + case 0x200e882c: + case 0x200e482c: + case 0x200e282c: + case 0x200e182c: + return 0; + + /* XPEC_P10_PCI_CPLT_CONF1 */ + case 0x08000009: + case 0x09000009: + return 0; + default: return -1; } @@ -152,6 +172,13 @@ static bool xscom_write_default(PnvChip *chip, uint32_t pcba, uint64_t val) case PRD_P8_IPOLL_REG_STATUS: case PRD_P9_IPOLL_REG_MASK: case PRD_P9_IPOLL_REG_STATUS: + + /* P10_QME_SPWU_HYP */ + case 0x200e883c: + case 0x200e483c: + case 0x200e283c: + case 0x200e183c: + return true; default: return false;
On 7/6/23 04:33, Joel Stanley wrote: > On Wed, 5 Jul 2023 at 10:02, Cédric Le Goater <clg@kaod.org> wrote: >> >> On 7/5/23 04:05, Joel Stanley wrote: >>> On Wed, 5 Jul 2023 at 01:27, Nicholas Piggin <npiggin@gmail.com> wrote: >>>> >>>> The P10 core xscom memory regions overlap because the size is wrong. >>>> The P10 core+L2 xscom region size is allocated as 0x1000 (with some >>>> unused ranges). "EC" is used as a closer match, as "EX" includes L3 >>>> which has a disjoint xscom range that would require a different >>>> region if it were implemented. >>>> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> >>> Nice, that looks better: >>> >>> 0000000100000000-00000001000fffff (prio 0, i/o): xscom-quad.0: 0x100000 >>> 0000000100108000-000000010010ffff (prio 0, i/o): xscom-core.3: 0x8000 >>> 0000000100110000-0000000100117fff (prio 0, i/o): xscom-core.2: 0x8000 >>> 0000000100120000-0000000100127fff (prio 0, i/o): xscom-core.1: 0x8000 >>> 0000000100140000-0000000100147fff (prio 0, i/o): xscom-core.0: 0x8000 >>> 0000000108000000-00000001080fffff (prio 0, i/o): xscom-quad.4: 0x100000 >>> 0000000108108000-000000010810ffff (prio 0, i/o): xscom-core.7: 0x8000 >>> 0000000108110000-0000000108117fff (prio 0, i/o): xscom-core.6: 0x8000 >>> 0000000108120000-0000000108127fff (prio 0, i/o): xscom-core.5: 0x8000 >>> 0000000108140000-0000000108147fff (prio 0, i/o): xscom-core.4: 0x8000 >>> >>> Reviewed-by: Joel Stanley <joel@jms.id.au> >> >> It'd interesting to add some dummy SLW handlers to get rid of the >> XSCOM errors at boot and shutdown on P10 : >> >> [ 4824.393446266,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0 >> [ 4824.393588777,5] Unable to log error >> [ 4824.393650582,3] XSCOM: Write failed, ret = -6 >> [ 4824.394124623,3] Could not set special wakeup on 0:0: Unable to write QME_SPWU_HYP. >> [ 4824.394368459,3] XSCOM: write error gcid=0x0 pcb_addr=0x200e883c stat=0x0 >> [ 4824.394382007,5] Unable to log error >> [ 4824.394384603,3] XSCOM: Write failed, ret = -6 > > Yes. I was looking at this yesterday. We need to figure out how to do > the xscom addressing for the QME. It sets (different) bits in order to > address a given core.> > > For a -smp 4 machine, the P10_QME_SPWU_HYP read comes in on these addresses: > > case 0x200e883c: > case 0x200e483c: > case 0x200e283c: > case 0x200e183c: > > ie, the fourth nibble selects the core. > > For a -smp 8 machine, the address now has bit 24 set to select the > second quad, so we need to cover these addresses: > > case 0x210e883c: > case 0x210e483c: > case 0x210e283c: > case 0x210e183c: > > I am thinking about how to map this into an address range that a model > can claim. OPAL should have the magic macros for XSCOM addresses. I included a few in QEMU. > Cheers, > > Joel > > PS. For reference, this is sufficient to silence xscom errors with > skiboot and -M powernv10 -smp4. A different set of hacks is required > for p9. > Thanks, C. > --- a/hw/ppc/pnv_xscom.c > +++ b/hw/ppc/pnv_xscom.c > @@ -106,6 +106,26 @@ static uint64_t xscom_read_default(PnvChip *chip, > uint32_t pcba) > case 0x401082a: > case 0x4010828: > return 0; > + > + /* P10_QME_SPWU_HYP */ > + case 0x200e883c: > + case 0x200e483c: > + case 0x200e283c: > + case 0x200e183c: > + return 0; > + > + /* P10_QME_SSH_HYP */ > + case 0x200e882c: > + case 0x200e482c: > + case 0x200e282c: > + case 0x200e182c: > + return 0; > + > + /* XPEC_P10_PCI_CPLT_CONF1 */ > + case 0x08000009: > + case 0x09000009: > + return 0; > + > default: > return -1; > } > @@ -152,6 +172,13 @@ static bool xscom_write_default(PnvChip *chip, > uint32_t pcba, uint64_t val) > case PRD_P8_IPOLL_REG_STATUS: > case PRD_P9_IPOLL_REG_MASK: > case PRD_P9_IPOLL_REG_STATUS: > + > + /* P10_QME_SPWU_HYP */ > + case 0x200e883c: > + case 0x200e483c: > + case 0x200e283c: > + case 0x200e183c: > + > return true; > default: > return false;
© 2016 - 2024 Red Hat, Inc.