On Fri, Jan 15, 2021 at 05:22:08PM +0800, Zhang Rui wrote: > There are several things special for the RAPL Psys energy counter, on > Intel Sapphire Rapids platform. > 1. it contains one Psys master package, and only CPUs on the master > package can read valid value of the Psys energy counter, reading the > MSR on CPUs in the slave package returns 0. > 2. The master package does not have to be Physical package 0. And when > all the CPUs on the Psys master package are offlined, we lose the Psys > energy counter, at runtime. > 3. The Psys energy counter can be disabled by BIOS, while all the other > energy counters are not affected. > > It is not easy to handle all of these in the current RAPL PMU design > because > a) perf_msr_probe() validates the MSR on some random CPU, which may either > be in the Psys master package or in the Psys slave package. > b) all the RAPL events share the same PMU, and there is not API to remove > the psys-energy event cleanly, without affecting the other events in > the same PMU. > > This patch addresses the problems in a simple way. > > First, by setting .no_check bit for RAPL Psys MSR, the psys-energy event > is always added, so we don't have to check the Psys ENERGY_STATUS MSR on > master package. > > Then, rapl_not_visible() is removed because > 1. it is useless for RAPL MSRs with .no_check cleared, because the > .is_visible() callbacks is always overridden in perf_msr_probe(). > 2. it is useless for RAPL MSRs with .no_check set, because we actually > want the sysfs attributes always be visible for those MSRs. > > With the above changes, we always probe the psys-energy event on Intel SPR > platform. Difference is that the event counter returns 0 when the Psys > RAPL Domain is disabled by BIOS, or the Psys master package is offlined.
Maybe I'm too tired, but I cannot follow. How does this cure the fact that the rapl_cpu_mask might not include that master thing. And how can software detect what the master thing is to begin with?