Hi Babu, On 12/6/2023 9:17 AM, Moger, Babu wrote: > On 12/5/23 17:21, Reinette Chatre wrote: >> On 11/30/2023 4:57 PM, Babu Moger wrote: ... >> Comparing with supported bits would be an additional check, but what does >> that imply? Would it be possible for hardware to have a bit set that is >> not supported? Would that mean it is actually supported or a hardware bug? > > No. Hardware supports all the bits reported here. Like i said before > wanted to remove the hard-coded value. The size of the field in the register is different information from what the value of that field may be. > >> >>> } >>> >>> static void mondata_config_read(struct rdt_domain *d, struct mon_config_info *mon_info) >>> @@ -1621,7 +1621,7 @@ static int mbm_config_write_domain(struct rdt_resource *r, >>> int ret = 0; >>> >>> /* mon_config cannot be more than the supported set of events */ >>> - if (val > MAX_EVT_CONFIG_BITS) { >>> + if (val > resctrl_max_evt_bitmask) { >>> rdt_last_cmd_puts("Invalid event configuration\n"); >>> return -EINVAL; >>> } >> >> This does not look right. resctrl_max_evt_bitmask contains the supported >> types. A user may set a value that is less than resctrl_max_evt_bitmask but >> yet have an unsupported bit set, no? > > I think I have to make this clear in the patch. There is no difference in > the definition. Hardware supports all the events reported by the cpuid. I'll try to elaborate using an example. Let's say AMD decides to make hardware with hypothetical support mask of: resctrl_max_evt_bitmask = 0x4F (no support for Slow Mem). What if user attempts to set config that enables monitoring of Slow Mem: val = 0x30 In the above example, val is not larger than resctrl_max_evt_bitmask but it is an invalid config, no? Reinette