Fix r283120

[fabient]

Summary

Fix r283120 which use more than 8bits for class id.
The bug is manifested by invalid class put in the log record recorded to file.
Software events will be more impacted as the logtype will be invalid.

[jhb]

I guess we are using non-char types to store these values in other places? Otherwise I imagine the compiler would have complained about storing too-large values. Would be nice if that's the case to find the places that are storing this in a larger value and fix those as well to catch this in the future.

sys/sys/pmc.h

150

Please set this one to something large like 0xFE or 0x80. One of the main problems before was that people kept adding new entries before SOFT to keep it last.

[fabient]

In D3783#78030, @jhb wrote:

I guess we are using non-char types to store these values in other places? Otherwise I imagine the compiler would have complained about storing too-large values. Would be nice if that's the case to find the places that are storing this in a larger value and fix those as well to catch this in the future.

Just below patch there is the description so the macro will not warn:

/*

• PMC IDs have the following format: *
• +--------+----------+-----------+-----------+
• | CPU | PMC MODE | PMC CLASS | ROW INDEX |
• +--------+----------+-----------+-----------+ *
• where each field is 8 bits wide. Field 'CPU' is set to the
• requested CPU for system-wide PMCs or PMC_CPU_ANY for process-mode
• PMCs. Field 'PMC MODE' is the allocated PMC mode. Field 'PMC
• CLASS' is the class of the PMC. Field 'ROW INDEX' is the row index
• for the PMC. *
• The 'ROW INDEX' ranges over 0..NWPMCS where NHWPMCS is the total
• number of hardware PMCs on this cpu. */

#define PMC_ID_TO_ROWINDEX(ID) ((ID) & 0xFF)
#define PMC_ID_TO_CLASS(ID) (((ID) & 0xFF00) >> 8)
#define PMC_ID_TO_MODE(ID) (((ID) & 0xFF0000) >> 16)
#define PMC_ID_TO_CPU(ID) (((ID) & 0xFF000000) >> 24)
#define PMC_ID_MAKE_ID(CPU,MODE,CLASS,ROWINDEX) \

((((CPU) & 0xFF) << 24) | (((MODE) & 0xFF) << 16) |     \
(((CLASS) & 0xFF) << 8) | ((ROWINDEX) & 0xFF))

[fabient]

sys/sys/pmc.h

150

Fine; I will change to 0xFF but from my POV if I remember well there is no requirement to set SOFT the last one. So I can move SOFT to be the first one instead ? this was originally added at the end to not broke compat. The only requirement is on CLASS_INDEX where i've reserved 0 for SOFT index.

[jhb]

sys/sys/pmc.h

150

I'm fine with moving SOFT to the beginning. We've already destroyed compat in HEAD as it is. Alternatively, we could pick the values used in 10 and fix the classes at those values for 11 and later (I think SOFT would then end up in the middle). You can choose which you think is best.

[emaste]

ping?

[fabient]

Commited as r289025