| Message ID | 20240715183332.27287-5-kowal@linux.vnet.ibm.com |
|---|---|
| State | New |
| Headers | show |
| Series | XIVE changes for Cache Watch, VSTs, STT and info pic | expand |
On 7/15/24 20:33, Michael Kowal wrote: > From: Frederic Barrat <fbarrat@linux.ibm.com> > > The cache watch facility uses the same register interface to handle > entries in the NVP, NVG and NVC tables. A bit-field in the 'watchX > specification' register tells the table type. So far, that bit-field > was not read and the code assumed a read/write to the NVP table. > > This patch allows to read/write entries in the NVG and NVC table as > well. > > Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> > Signed-off-by: Michael Kowal <kowal@linux.vnet.ibm.com> > --- > hw/intc/pnv_xive2.c | 59 ++++++++++++++++++++++++++++++++------------- > 1 file changed, 42 insertions(+), 17 deletions(-) > > diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c > index d62ac20d98..c72c66dd6a 100644 > --- a/hw/intc/pnv_xive2.c > +++ b/hw/intc/pnv_xive2.c > @@ -462,46 +462,71 @@ static int pnv_xive2_write_nvp(Xive2Router *xrtr, uint8_t blk, uint32_t idx, > word_number); > } > > -static int pnv_xive2_nvp_update(PnvXive2 *xive, uint8_t watch_engine) > +static int pnv_xive2_nxc_to_table_type(uint8_t nxc_type, uint32_t *table_type) > { > - uint8_t blk; > - uint32_t idx; > + switch (nxc_type) { > + case PC_NXC_WATCH_NXC_NVP: > + *table_type = VST_NVP; > + break; > + case PC_NXC_WATCH_NXC_NVG: > + *table_type = VST_NVG; > + break; > + case PC_NXC_WATCH_NXC_NVC: > + *table_type = VST_NVC; > + break; > + default: > + qemu_log_mask(LOG_GUEST_ERROR, > + "XIVE: invalid table type for nxc operation\n"); > + return -1; > + } > + return 0; > +} > + > +static int pnv_xive2_nxc_update(PnvXive2 *xive, uint8_t watch_engine) > +{ > + uint8_t blk, nxc_type; > + uint32_t idx, table_type = -1; > int i, spec_reg, data_reg; > uint64_t nxc_watch[4]; > > - if (watch_engine > 3) { > - return -1; > - } > + assert(watch_engine < ARRAY_SIZE(nxc_watch)); > + This belongs to patch 1. > spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; > data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; > + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, xive->pc_regs[spec_reg]); > blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); > idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); > > + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); > + > for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { > nxc_watch[i] = cpu_to_be64(xive->pc_regs[data_reg + i]); > } > > - return pnv_xive2_vst_write(xive, VST_NVP, blk, idx, nxc_watch, > + return pnv_xive2_vst_write(xive, table_type, blk, idx, nxc_watch, > XIVE_VST_WORD_ALL); > } > > -static void pnv_xive2_nvp_cache_load(PnvXive2 *xive, uint8_t watch_engine) > +static void pnv_xive2_nxc_cache_load(PnvXive2 *xive, uint8_t watch_engine) > { > - uint8_t blk; > - uint32_t idx; > + uint8_t blk, nxc_type; > + uint32_t idx, table_type = -1; > uint64_t nxc_watch[4] = { 0 }; > int i, spec_reg, data_reg; > > - if (watch_engine > 3) { > - return; > - } > + assert(watch_engine < ARRAY_SIZE(nxc_watch)); > + > spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; > data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; > + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, xive->pc_regs[spec_reg]); > blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); > idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); > > - if (pnv_xive2_vst_read(xive, VST_NVP, blk, idx, nxc_watch)) { > - xive2_error(xive, "VST: no NVP entry %x/%x !?", blk, idx); > + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); > + > + if (pnv_xive2_vst_read(xive, table_type, blk, idx, nxc_watch)) { > + xive2_error(xive, "VST: no NXC entry %x/%x in %s table!?", > + blk, idx, vst_infos[table_type].name); > } > > for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { > @@ -1431,7 +1456,7 @@ static uint64_t pnv_xive2_ic_pc_read(void *opaque, hwaddr offset, > * SPEC register > */ > watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; > - pnv_xive2_nvp_cache_load(xive, watch_engine); > + pnv_xive2_nxc_cache_load(xive, watch_engine); > val = xive->pc_regs[reg]; > break; > > @@ -1505,7 +1530,7 @@ static void pnv_xive2_ic_pc_write(void *opaque, hwaddr offset, > /* writing to DATA0 triggers the cache write */ > watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; > xive->pc_regs[reg] = val; > - pnv_xive2_nvp_update(xive, watch_engine); > + pnv_xive2_nxc_update(xive, watch_engine); > break; > > /* case PC_NXC_FLUSH_CTRL: */
On 7/16/2024 2:29 AM, Cédric Le Goater wrote: > On 7/15/24 20:33, Michael Kowal wrote: >> From: Frederic Barrat <fbarrat@linux.ibm.com> >> >> The cache watch facility uses the same register interface to handle >> entries in the NVP, NVG and NVC tables. A bit-field in the 'watchX >> specification' register tells the table type. So far, that bit-field >> was not read and the code assumed a read/write to the NVP table. >> >> This patch allows to read/write entries in the NVG and NVC table as >> well. >> >> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> >> Signed-off-by: Michael Kowal <kowal@linux.vnet.ibm.com> >> --- >> hw/intc/pnv_xive2.c | 59 ++++++++++++++++++++++++++++++++------------- >> 1 file changed, 42 insertions(+), 17 deletions(-) >> >> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c >> index d62ac20d98..c72c66dd6a 100644 >> --- a/hw/intc/pnv_xive2.c >> +++ b/hw/intc/pnv_xive2.c >> @@ -462,46 +462,71 @@ static int pnv_xive2_write_nvp(Xive2Router >> *xrtr, uint8_t blk, uint32_t idx, >> word_number); >> } >> -static int pnv_xive2_nvp_update(PnvXive2 *xive, uint8_t watch_engine) >> +static int pnv_xive2_nxc_to_table_type(uint8_t nxc_type, uint32_t >> *table_type) >> { >> - uint8_t blk; >> - uint32_t idx; >> + switch (nxc_type) { >> + case PC_NXC_WATCH_NXC_NVP: >> + *table_type = VST_NVP; >> + break; >> + case PC_NXC_WATCH_NXC_NVG: >> + *table_type = VST_NVG; >> + break; >> + case PC_NXC_WATCH_NXC_NVC: >> + *table_type = VST_NVC; >> + break; >> + default: >> + qemu_log_mask(LOG_GUEST_ERROR, >> + "XIVE: invalid table type for nxc operation\n"); >> + return -1; >> + } >> + return 0; >> +} >> + >> +static int pnv_xive2_nxc_update(PnvXive2 *xive, uint8_t watch_engine) >> +{ >> + uint8_t blk, nxc_type; >> + uint32_t idx, table_type = -1; >> int i, spec_reg, data_reg; >> uint64_t nxc_watch[4]; >> - if (watch_engine > 3) { >> - return -1; >> - } >> + assert(watch_engine < ARRAY_SIZE(nxc_watch)); >> + > > This belongs to patch 1. I was trying to reduce the size of the first patch set and this could be isolated to separate patch set. I can move it back to patch set 1. Also note above, this is where I originally added the assert... > >> spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; >> data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; >> + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, >> xive->pc_regs[spec_reg]); >> blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); >> idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); >> + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); >> + >> for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { >> nxc_watch[i] = cpu_to_be64(xive->pc_regs[data_reg + i]); >> } >> - return pnv_xive2_vst_write(xive, VST_NVP, blk, idx, nxc_watch, >> + return pnv_xive2_vst_write(xive, table_type, blk, idx, nxc_watch, >> XIVE_VST_WORD_ALL); >> } >> -static void pnv_xive2_nvp_cache_load(PnvXive2 *xive, uint8_t >> watch_engine) >> +static void pnv_xive2_nxc_cache_load(PnvXive2 *xive, uint8_t >> watch_engine) >> { >> - uint8_t blk; >> - uint32_t idx; >> + uint8_t blk, nxc_type; >> + uint32_t idx, table_type = -1; >> uint64_t nxc_watch[4] = { 0 }; >> int i, spec_reg, data_reg; >> - if (watch_engine > 3) { >> - return; >> - } >> + assert(watch_engine < ARRAY_SIZE(nxc_watch)); >> + >> spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; >> data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; >> + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, >> xive->pc_regs[spec_reg]); >> blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); >> idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); >> - if (pnv_xive2_vst_read(xive, VST_NVP, blk, idx, nxc_watch)) { >> - xive2_error(xive, "VST: no NVP entry %x/%x !?", blk, idx); >> + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); >> + >> + if (pnv_xive2_vst_read(xive, table_type, blk, idx, nxc_watch)) { >> + xive2_error(xive, "VST: no NXC entry %x/%x in %s table!?", >> + blk, idx, vst_infos[table_type].name); >> } >> for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { >> @@ -1431,7 +1456,7 @@ static uint64_t pnv_xive2_ic_pc_read(void >> *opaque, hwaddr offset, >> * SPEC register >> */ >> watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; >> - pnv_xive2_nvp_cache_load(xive, watch_engine); >> + pnv_xive2_nxc_cache_load(xive, watch_engine); >> val = xive->pc_regs[reg]; >> break; >> @@ -1505,7 +1530,7 @@ static void pnv_xive2_ic_pc_write(void >> *opaque, hwaddr offset, >> /* writing to DATA0 triggers the cache write */ >> watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; >> xive->pc_regs[reg] = val; >> - pnv_xive2_nvp_update(xive, watch_engine); >> + pnv_xive2_nxc_update(xive, watch_engine); >> break; >> /* case PC_NXC_FLUSH_CTRL: */ >
On 7/16/2024 10:46 AM, Mike Kowal wrote: > > On 7/16/2024 2:29 AM, Cédric Le Goater wrote: >> On 7/15/24 20:33, Michael Kowal wrote: >>> From: Frederic Barrat <fbarrat@linux.ibm.com> >>> >>> The cache watch facility uses the same register interface to handle >>> entries in the NVP, NVG and NVC tables. A bit-field in the 'watchX >>> specification' register tells the table type. So far, that bit-field >>> was not read and the code assumed a read/write to the NVP table. >>> >>> This patch allows to read/write entries in the NVG and NVC table as >>> well. >>> >>> Signed-off-by: Frederic Barrat <fbarrat@linux.ibm.com> >>> Signed-off-by: Michael Kowal <kowal@linux.vnet.ibm.com> >>> --- >>> hw/intc/pnv_xive2.c | 59 >>> ++++++++++++++++++++++++++++++++------------- >>> 1 file changed, 42 insertions(+), 17 deletions(-) >>> >>> diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c >>> index d62ac20d98..c72c66dd6a 100644 >>> --- a/hw/intc/pnv_xive2.c >>> +++ b/hw/intc/pnv_xive2.c >>> @@ -462,46 +462,71 @@ static int pnv_xive2_write_nvp(Xive2Router >>> *xrtr, uint8_t blk, uint32_t idx, >>> word_number); >>> } >>> -static int pnv_xive2_nvp_update(PnvXive2 *xive, uint8_t >>> watch_engine) >>> +static int pnv_xive2_nxc_to_table_type(uint8_t nxc_type, uint32_t >>> *table_type) >>> { >>> - uint8_t blk; >>> - uint32_t idx; >>> + switch (nxc_type) { >>> + case PC_NXC_WATCH_NXC_NVP: >>> + *table_type = VST_NVP; >>> + break; >>> + case PC_NXC_WATCH_NXC_NVG: >>> + *table_type = VST_NVG; >>> + break; >>> + case PC_NXC_WATCH_NXC_NVC: >>> + *table_type = VST_NVC; >>> + break; >>> + default: >>> + qemu_log_mask(LOG_GUEST_ERROR, >>> + "XIVE: invalid table type for nxc operation\n"); >>> + return -1; >>> + } >>> + return 0; >>> +} >>> + >>> +static int pnv_xive2_nxc_update(PnvXive2 *xive, uint8_t watch_engine) >>> +{ >>> + uint8_t blk, nxc_type; >>> + uint32_t idx, table_type = -1; >>> int i, spec_reg, data_reg; >>> uint64_t nxc_watch[4]; >>> - if (watch_engine > 3) { >>> - return -1; >>> - } >>> + assert(watch_engine < ARRAY_SIZE(nxc_watch)); >>> + >> >> This belongs to patch 1. > > I was trying to reduce the size of the first patch set and this could > be isolated to separate patch set. I can move it back to patch set 1. > > Also note above, this is where I originally added the assert... Disregard these comments. After rereading I understand. I thought you meant that this entire patch set should be in 1... > >> >>> spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; >>> data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; >>> + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, >>> xive->pc_regs[spec_reg]); >>> blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); >>> idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); >>> + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); >>> + >>> for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { >>> nxc_watch[i] = cpu_to_be64(xive->pc_regs[data_reg + i]); >>> } >>> - return pnv_xive2_vst_write(xive, VST_NVP, blk, idx, nxc_watch, >>> + return pnv_xive2_vst_write(xive, table_type, blk, idx, nxc_watch, >>> XIVE_VST_WORD_ALL); >>> } >>> -static void pnv_xive2_nvp_cache_load(PnvXive2 *xive, uint8_t >>> watch_engine) >>> +static void pnv_xive2_nxc_cache_load(PnvXive2 *xive, uint8_t >>> watch_engine) >>> { >>> - uint8_t blk; >>> - uint32_t idx; >>> + uint8_t blk, nxc_type; >>> + uint32_t idx, table_type = -1; >>> uint64_t nxc_watch[4] = { 0 }; >>> int i, spec_reg, data_reg; >>> - if (watch_engine > 3) { >>> - return; >>> - } >>> + assert(watch_engine < ARRAY_SIZE(nxc_watch)); >>> + >>> spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; >>> data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; >>> + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, >>> xive->pc_regs[spec_reg]); >>> blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); >>> idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); >>> - if (pnv_xive2_vst_read(xive, VST_NVP, blk, idx, nxc_watch)) { >>> - xive2_error(xive, "VST: no NVP entry %x/%x !?", blk, idx); >>> + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); >>> + >>> + if (pnv_xive2_vst_read(xive, table_type, blk, idx, nxc_watch)) { >>> + xive2_error(xive, "VST: no NXC entry %x/%x in %s table!?", >>> + blk, idx, vst_infos[table_type].name); >>> } >>> for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { >>> @@ -1431,7 +1456,7 @@ static uint64_t pnv_xive2_ic_pc_read(void >>> *opaque, hwaddr offset, >>> * SPEC register >>> */ >>> watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; >>> - pnv_xive2_nvp_cache_load(xive, watch_engine); >>> + pnv_xive2_nxc_cache_load(xive, watch_engine); >>> val = xive->pc_regs[reg]; >>> break; >>> @@ -1505,7 +1530,7 @@ static void pnv_xive2_ic_pc_write(void >>> *opaque, hwaddr offset, >>> /* writing to DATA0 triggers the cache write */ >>> watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; >>> xive->pc_regs[reg] = val; >>> - pnv_xive2_nvp_update(xive, watch_engine); >>> + pnv_xive2_nxc_update(xive, watch_engine); >>> break; >>> /* case PC_NXC_FLUSH_CTRL: */ >>
diff --git a/hw/intc/pnv_xive2.c b/hw/intc/pnv_xive2.c index d62ac20d98..c72c66dd6a 100644 --- a/hw/intc/pnv_xive2.c +++ b/hw/intc/pnv_xive2.c @@ -462,46 +462,71 @@ static int pnv_xive2_write_nvp(Xive2Router *xrtr, uint8_t blk, uint32_t idx, word_number); } -static int pnv_xive2_nvp_update(PnvXive2 *xive, uint8_t watch_engine) +static int pnv_xive2_nxc_to_table_type(uint8_t nxc_type, uint32_t *table_type) { - uint8_t blk; - uint32_t idx; + switch (nxc_type) { + case PC_NXC_WATCH_NXC_NVP: + *table_type = VST_NVP; + break; + case PC_NXC_WATCH_NXC_NVG: + *table_type = VST_NVG; + break; + case PC_NXC_WATCH_NXC_NVC: + *table_type = VST_NVC; + break; + default: + qemu_log_mask(LOG_GUEST_ERROR, + "XIVE: invalid table type for nxc operation\n"); + return -1; + } + return 0; +} + +static int pnv_xive2_nxc_update(PnvXive2 *xive, uint8_t watch_engine) +{ + uint8_t blk, nxc_type; + uint32_t idx, table_type = -1; int i, spec_reg, data_reg; uint64_t nxc_watch[4]; - if (watch_engine > 3) { - return -1; - } + assert(watch_engine < ARRAY_SIZE(nxc_watch)); + spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, xive->pc_regs[spec_reg]); blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); + for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { nxc_watch[i] = cpu_to_be64(xive->pc_regs[data_reg + i]); } - return pnv_xive2_vst_write(xive, VST_NVP, blk, idx, nxc_watch, + return pnv_xive2_vst_write(xive, table_type, blk, idx, nxc_watch, XIVE_VST_WORD_ALL); } -static void pnv_xive2_nvp_cache_load(PnvXive2 *xive, uint8_t watch_engine) +static void pnv_xive2_nxc_cache_load(PnvXive2 *xive, uint8_t watch_engine) { - uint8_t blk; - uint32_t idx; + uint8_t blk, nxc_type; + uint32_t idx, table_type = -1; uint64_t nxc_watch[4] = { 0 }; int i, spec_reg, data_reg; - if (watch_engine > 3) { - return; - } + assert(watch_engine < ARRAY_SIZE(nxc_watch)); + spec_reg = (PC_NXC_WATCH0_SPEC + watch_engine * 0x40) >> 3; data_reg = (PC_NXC_WATCH0_DATA0 + watch_engine * 0x40) >> 3; + nxc_type = GETFIELD(PC_NXC_WATCH_NXC_TYPE, xive->pc_regs[spec_reg]); blk = GETFIELD(PC_NXC_WATCH_BLOCK_ID, xive->pc_regs[spec_reg]); idx = GETFIELD(PC_NXC_WATCH_INDEX, xive->pc_regs[spec_reg]); - if (pnv_xive2_vst_read(xive, VST_NVP, blk, idx, nxc_watch)) { - xive2_error(xive, "VST: no NVP entry %x/%x !?", blk, idx); + assert(pnv_xive2_nxc_to_table_type(nxc_type, &table_type)); + + if (pnv_xive2_vst_read(xive, table_type, blk, idx, nxc_watch)) { + xive2_error(xive, "VST: no NXC entry %x/%x in %s table!?", + blk, idx, vst_infos[table_type].name); } for (i = 0; i < ARRAY_SIZE(nxc_watch); i++) { @@ -1431,7 +1456,7 @@ static uint64_t pnv_xive2_ic_pc_read(void *opaque, hwaddr offset, * SPEC register */ watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; - pnv_xive2_nvp_cache_load(xive, watch_engine); + pnv_xive2_nxc_cache_load(xive, watch_engine); val = xive->pc_regs[reg]; break; @@ -1505,7 +1530,7 @@ static void pnv_xive2_ic_pc_write(void *opaque, hwaddr offset, /* writing to DATA0 triggers the cache write */ watch_engine = (offset - PC_NXC_WATCH0_DATA0) >> 6; xive->pc_regs[reg] = val; - pnv_xive2_nvp_update(xive, watch_engine); + pnv_xive2_nxc_update(xive, watch_engine); break; /* case PC_NXC_FLUSH_CTRL: */