Re: [PATCH 5/6] x86/paravirt: Switch MSR access pv_ops functions to instruction interfaces

[Xin Li <xin@zytor.com>]

On 6/11/2025 5:58 AM, Juergen Gross wrote:
>> Here is a patch I cooked.  I added an ALTERNATIVE() hack because the 
>> new instructions can't be more than 6 bytes long.  But with the patch you
>> just sent, it shouldn't be needed.
> 
> I have meanwhile dropped the patch copying the original indirect call.
> 
> Reason is that I'm seeing a potential risk with current alternative
> patching when using ALTERNATIVE_[23](): depending on the tested features
> it might happen that an instruction sequence not suitable for the current
> runtime environment is patched in as an intermediate step. In case there
> is an interrupt happening just then AND the handling of the interrupt is
> using the patch site, this could result in crashes or undefined behavior.

Oh, I had assumed that Linux disables interrupts during the patching
process. Just out of curiosity, why are interrupts allowed in this case?

> 
> I have meanwhile a set of 3 patches fixing that problem by merging all
> alternative patching of a patch site in the local buffer and only then
> patching the code at the target site with the final result.
> 
> The same problem arises with your code below, but this time it isn't
> fixed by my patches: the two ALTERNATIVE() instances in the asm() construct
> would need to be patched in a single atomic operation to be consistent.
> Otherwise you could end up e.g. on bare metal with paravirt_read_msr()
> having replaced the indirect call with "rdmsr", but not yet having added
> the code to merge %rdx into %rax.
> 
> I'm just doing a V2 of my series, but this time including the additional
> support of the non-serializing and immediate forms. Lets see how this will
> look like. I will drop using the EAX_EDX_* macros, but due to the reason
> mentioned above I won't switch to your variant completely.

Great!

Thanks!
     Xin