On 30.09.25 10:38, Peter Zijlstra wrote:
On Tue, Sep 30, 2025 at 09:03:55AM +0200, Juergen Gross wrote:+static __always_inline u64 read_msr(u32 msr) +{ + if (cpu_feature_enabled(X86_FEATURE_XENPV)) + return xen_read_msr(msr); + + return native_rdmsrq(msr); +} + +static __always_inline int read_msr_safe(u32 msr, u64 *p) +{ + if (cpu_feature_enabled(X86_FEATURE_XENPV)) + return xen_read_msr_safe(msr, p); + + return native_read_msr_safe(msr, p); +} + +static __always_inline void write_msr(u32 msr, u64 val) +{ + if (cpu_feature_enabled(X86_FEATURE_XENPV)) + xen_write_msr(msr, val); + else + native_wrmsrq(msr, val); +} + +static __always_inline int write_msr_safe(u32 msr, u64 val) +{ + if (cpu_feature_enabled(X86_FEATURE_XENPV)) + return xen_write_msr_safe(msr, val); + + return native_write_msr_safe(msr, val); +} + +static __always_inline u64 rdpmc(int counter) +{ + if (cpu_feature_enabled(X86_FEATURE_XENPV)) + return xen_read_pmc(counter); + + return native_read_pmc(counter); +}Egads, didn't we just construct giant ALTERNATIVE()s for the native_ things? Why wrap that in a cpu_feature_enabled() instead of just adding one more case to the ALTERNATIVE() ?
The problem I encountered with using pv_ops was to implement the *_safe() variants. There is no simple way to do that using ALTERNATIVE_<n>(), as in the Xen PV case the call will remain, and I didn't find a way to specify a sane interface between the call-site and the called Xen function to return the error indicator. Remember that at the call site the main interface is the one of the RDMSR/WRMSR instructions. They lack an error indicator. In Xin's series there was a patch written initially by you to solve such a problem by adding the _ASM_EXTABLE_FUNC_REWIND() exception table method. I think this is a dead end, as it will break when using a shadow stack. Additionally I found a rather ugly hack only to avoid re-iterating most of the bare metal ALTERNATIVE() for the paravirt case. It is possible, but the bare metal case is gaining one additional ALTERNATIVE level, resulting in patching the original instruction with an identical copy first. Another benefit of my approach is the dropping of "#include <asm/paravirt.h>" from msr.h, which is making life a little bit easier. Juergen
OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key
OpenPGP_signature.asc
Description: OpenPGP digital signature
