On 21.03.26 00:33, Borislav Petkov wrote:
On Fri, Mar 20, 2026 at 12:03:30PM -0700, Dave Hansen wrote:This is old cruft, but it appears that having two copies of these MSR functions is enabling warnings to creep in[1].I know there's also been some work to pare down the XXL code, but it's obviously not merged yet and this is a good baby step. Create helpers that both paravirt and native can use in common code and remove the paravirt implementations of the helpers. This reduces the amount of logic that is duplicated in the paravirt code. The other thing I really like about this is that it puts the raw=>{native,paravirt} switch in one compact place in the code. Conceptually: - native: The bare-metal implementation. Might not be usable under paravirt XXL. - raw: The lowest-level function that is always usable. Might be native or paravirt under the hood.I went through the patchset twice and I kinda get what you're trying to do but the "raw" thing is confusing as hell. To me "raw" means, the lowest level of the functionality - something like __<function_name> with the two underscores. Or three, depending on the indirection levels. And those do *only* *raw* instructions - no more indirections. But then how can "raw" be the lowest level and then still have something else underneath - native_ and paravirt_? I *think* this is only a naming issue and with "raw_" you probably wanna say "native_or_paravirt_" depending on the ifdeffery... but shorter... If so, I wouldn't call it "raw". I'd say xx_read_msr() xx_write_msr() to denote that the "xx" resolves to either of the two types. But a better name. I can't think of a good one now but I know that "raw" isn't it... Hmmm.
I'd like to suggest to do a major cleanup of the MSR interfaces. We have too many of them doing similar things. Some are capable to do tracing, some aren't. Some are paravirt capable, some aren't. And the names of those functions don't reflect that at all. We even have multiple functions or macros doing exactly the same thing, but having different names. And in future it will be even more complicated due to the write MSR interfaces needing serializing and non-serializing variants. My idea would be to have something like: msr_read() msr_read_notrace() msr_write_sync() msr_write_nosync() msr_write_sync_notrace() msr_write_nosync_notrace() All of those should be paravirt capable and they should be the only "official" interfaces. They will depend on low-level primitives, but those should be used only by the official access functions and maybe in some very special places. I think this should be the first step towards a MSR access consolidation, as it allows any internal optimizations and changes without further bothering most of the users. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
