On Fri, 16 Oct 2026 17:47:07 +0800 Jinjie Ruan <[email protected]> wrote:
> From: Jinjie Ruan <[email protected]> > > Add the priority controls: MPAMF_PRI_IDR advertises a 2-bit > INTPRI (0 = low) with no DSPRI, HAS_PRI_PART is set on the cache > subtype, and MPAMCFG_PRI is readable and writable per selected > internal PARTID, logging writes of DSPRI/INTPRI fields the IDR > does not implement. > > Signed-off-by: Jinjie Ruan <[email protected]> I wonder if it is worth maintaining the flexibility of all the rest of the code not assuming HAS_PRI when it is hard coded to 1? Maybe it is worth piping through the ability to turn each feature off? Or simplifying the code by removing the checks. When I wrote that original I was mostly tweaking options and rebuilding to see what bugs I could find in James' kernel code. I think it would be useful to keep the ability to do that, but I have no idea if anyone wants the rich set of control parameters that needs. Maybe x-has_no_pri or something like that to indicate debug only? BTW not sure how you decided which patches to keep my original SoB and which not. I don't really care given this was ancient code I was probably never getting back to, but maybe do another pass for what was more or less straight out of the RFC vs needing a bigger rewrite. Thanks, Jonathan > --- > hw/arm/mpam.c | 36 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 36 insertions(+) > > diff --git a/hw/arm/mpam.c b/hw/arm/mpam.c > index 7c33ec4..94e517b 100644 > --- a/hw/arm/mpam.c > +++ b/hw/arm/mpam.c > @@ -365,6 +365,13 @@ static uint64_t mpam_msc_read_reg(void *opaque, hwaddr > offset, > return mpamf->cpor_idr; > case A_MPAMF_MBW_IDR: > return mpamf->mbw_idr; > + case A_MPAMF_PRI_IDR: > + if (!FIELD_EX32(mpamf->idr, MPAMF_IDR, HAS_PRI_PART)) { > + qemu_log_mask(LOG_UNIMP, > + "MPAM: Unexpected read of PRI_IDR with no PRI PART > support\n"); > + return 0; > + } > + return mpamf->pri_idr; > case A_MPAMF_PARTID_NRW_IDR: { > return mpamf->partid_nrw_idr; > } > @@ -380,6 +387,8 @@ static uint64_t mpam_msc_read_reg(void *opaque, hwaddr > offset, > return mpamf->per_nrw_id[nrw_part_sel].cfg_mbw_min; > case A_MPAMF_MPAMCFG_MBW_MAX: > return mpamf->per_nrw_id[nrw_part_sel].cfg_mbw_max; > + case A_MPAMF_MPAMCFG_PRI: > + return mpamf->per_nrw_id[nrw_part_sel].cfg_pri; > case A_MPAMF_MPAMCFG_MBW_PROP: > return mpamf->per_nrw_id[nrw_part_sel].cfg_mbw_prop; > case A_MPAMF_MPAMCFG_CPBM0... > @@ -467,6 +476,24 @@ static void mpam_msc_write_reg(void *opaque, hwaddr > offset, uint64_t value, > } > mpamf->per_nrw_id[nrw_part_sel].cfg_mbw_max = value; > return; > + case A_MPAMF_MPAMCFG_PRI: > + if (!FIELD_EX32(mpamf->idr, MPAMF_IDR, HAS_PRI_PART)) { > + qemu_log_mask(LOG_UNIMP, > + "MPAM: Unexpected write to CFG_PRI when > !HAS_PRI_PART\n"); > + } else { > + if (!FIELD_EX32(mpamf->pri_idr, MPAMF_PRI_IDR, HAS_DSPRI) && > + FIELD_EX32(value, MPAMF_MPAMCFG_PRI, DSPRI)) { > + qemu_log_mask(LOG_UNIMP, > + "MPAM: Unexpected write to CFGP_PRI DSPRI when > !HAS_DSPRI\n"); > + } > + if (!FIELD_EX32(mpamf->pri_idr, MPAMF_PRI_IDR, HAS_INTPRI) && > + FIELD_EX32(value, MPAMF_MPAMCFG_PRI, INTPRI)) { > + qemu_log_mask(LOG_UNIMP, > + "MPAM: Unexpected write to CFGP_PRI INTPRI > when !HAS_INTPRI\n"); > + } > + } > + mpamf->per_nrw_id[nrw_part_sel].cfg_pri = value; > + return; > case A_MPAMF_MPAMCFG_MBW_PROP: > if (value & ~MPAMF_MPAMCFG_MBW_PROP_WRITE_MASK) { > qemu_log_mask(LOG_UNIMP, > @@ -648,6 +675,7 @@ static void mpam_msc_cache_realize(DeviceState *dev, > Error **errp) > > /* Cache specific bit */ > mpamf->idr = FIELD_DP64(mpamf->idr, MPAMF_IDR, HAS_CPOR_PART, 1); > + mpamf->idr = FIELD_DP64(mpamf->idr, MPAMF_IDR, HAS_PRI_PART, 1); > mpamf->idr = FIELD_DP64(mpamf->idr, MPAMF_IDR, HAS_PARTID_NRW, > s->num_int_partid < s->num_partid); > > @@ -665,6 +693,14 @@ static void mpam_msc_cache_realize(DeviceState *dev, > Error **errp) > mpamf->cpor_idr = FIELD_DP32(mpamf->cpor_idr, MPAMF_CPOR_IDR, > CPBM_WD, > MPAM_CACHE_PART); > > + /* Priority */ > + mpamf->pri_idr = FIELD_DP32(mpamf->pri_idr, MPAMF_PRI_IDR, > + HAS_INTPRI, 1); > + mpamf->pri_idr = FIELD_DP32(mpamf->pri_idr, MPAMF_PRI_IDR, > + INTPRI_0_IS_LOW, 1); > + mpamf->pri_idr = FIELD_DP32(mpamf->pri_idr, MPAMF_PRI_IDR, > + INTPRI_WD, 2); > + > if (s->num_int_partid < s->num_partid) { > mpamf->partid_nrw_idr = FIELD_DP32(mpamf->partid_nrw_idr, > MPAMF_PARTID_NRW_IDR,
