On Fri, 16 Oct 2026 17:47:10 +0800 Jinjie Ruan <[email protected]> wrote:
> From: Jinjie Ruan <[email protected]> > > The MPAMCFG_INTPARTID register (0x600) was defined but never handled, Never handled in original set? I can't remember but it seems an odd comment to have in this series. Or is intent to say it's there earlier in the series. If so I'd say "has been define but not yet handled" I'm guessing it was buggy in the RFC. Not sure I tested it so I'm certainly not surprised it was! > so a guest could not program the request PARTID to internal PARTID > mapping that PARTID narrowing is based on. Add read/write support: > > - Writing MPAMCFG_INTPARTID stores the mapping for the request PARTID > selected by MPAMCFG_PART_SEL; reads return the stored mapping. > - At realize, initialise the map to partid % num_int_partid so the > indirection is always in range before a driver programs it, > mirroring the Linux driver's fallback mapping. MBW_MAX also resets > to 0xffff (full bandwidth) for every internal PARTID, matching > hardware: an unprogrammed PARTID must not read as zero bandwidth. > - Clamp the narrowing indirection lookup to num_int_partid, and wrap > MPAMCFG_PART_SEL writes into the supported PARTID space (the > narrowed space for INTERNAL selectors) so a guest cannot trigger > out-of-bounds accesses on the emulated arrays. > > A guest driver can now exercise PARTID narrowing: several request > PARTIDs share one internal PARTID's configuration, and the guest can > re-map them at runtime. > > Signed-off-by: Jinjie Ruan <[email protected]> Just trivial stuff. Looks good to me and having looked again, the RFC clearly failed to do half of what was needed :( Reviewed-by: Jonathan Cameron <[email protected]> > @@ -482,12 +497,38 @@ static void mpam_msc_write_reg(void *opaque, hwaddr > offset, uint64_t value, > s->num_ris); > return; > } > + { > + /* > + * IHI 0099 7.2.1: the MSC does not check PARTID_SEL on write > + * (the first IMPLEMENTATION DEFINED behaviour); the selector > + * wraps into the supported PARTID space instead. > + */ > + uint32_t limit = FIELD_EX32(value, MPAMF_CFG_PART_SEL, INTERNAL) > ? > + s->num_int_partid : s->num_partid; > + > + s->part_sel = FIELD_EX32(value, MPAMF_CFG_PART_SEL, > + PARTID_SEL) % limit; I'd rewrap that. I think slightly over 80 chars is fine here. > + } > case A_MPAMF_MPAMCFG_CMAX: > if (value & ~MPAMF_MPAMCFG_CMAX_WRITE_MASK) { > qemu_log_mask(LOG_UNIMP, > @@ -619,8 +660,25 @@ static void mpam_msc_realize(DeviceState *dev, Error > **errp) > > s->mpamf = g_new0(Mpamf, s->num_ris); > for (i = 0; i < s->num_ris; i++) { > + int p; > + > s->mpamf[i].per_nrw_id = g_new0(MpamfPerNrwId, s->num_int_partid); > s->mpamf[i].cfg_intpartid = g_new0(uint32_t, s->num_partid); > + /* > + * Until resctrl programs the map, wrap each request PARTID onto an > + * internal PARTID so the narrowing indirection is always in range. > + */ > + for (p = 0; p < s->num_partid; p++) { for (int p = 0; > + s->mpamf[i].cfg_intpartid[p] = p % s->num_int_partid; > + } > + /* > + * per_nrw_id is indexed by INTERNAL PARTID: it has num_int_partid > + * entries, not num_partid. > + */ > + for (p = 0; p < s->num_int_partid; p++) { for (int p = 0; particularly given it has a different use to previous p! > + /* MBW_MAX resets to full bandwidth on real hardware */ > + s->mpamf[i].per_nrw_id[p].cfg_mbw_max = 0xffff; > + } > } > > memory_region_init_io(&s->mr, OBJECT(s), &mpam_msc_ops, s, "mpam_msc",
