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

["H. Peter Anvin" <hpa@xxxxxxxxx>]

On 5/15/25 00:32, Xin Li wrote:
> Hi Juergen,
>
> I have some update on this thread while working on it.
>
> If we continue down the path of maintaining pvops MSR APIs as this patch
> series does, it seems we’ll need to duplicate the ALTERNATIVE code in
> three different places.
>
> 1) The MSR access primitives defined in <asm/msr.h>, which is used when
>     CONFIG_PARAVIRT=n.
>
> 2) The pvops native MSR functions pv_native_{rd,wr}msr{,_safe}() defined
>     in arch/x86/kernel/paravirt.c, used when CONFIG_PARAVIRT=y on bare
>     metal.
>
> 3) The pvops Xen MSR functions paravirt_{read,write}_msr{,_safe}()
>     defined in <asm/paravirt.h>, used when CONFIG_PARAVIRT_XXL=y.
>
> hpa had mentioned to me earlier that this would be a maintenance burden
> — something I only truly realized once I got hands-on with it.
>
> Maybe you have something in mind to address it?
>
> Also add PeterZ to the To list because he cares it.
Having the code being duplicated is definitely not a good thing; 
although I'm not one of the x86 maintainers anymore, I would consider it 
a strong reason to NAK such a patchset.
At one point I was considering augmenting the alternatives framework to 
be able to call an ad hoc subroutine to generate the code. It would be 
useful in cases like this, where if PV is enabled it can make a callout 
to the currently-active PV code to query the desired code to be output.
There are 16 unused bits in the alternatives table (not counting the 14 
unused flag bits), which could be used for an enumeration of such 
subroutines, optionally split into 8 bits of function enumeration and 8 
bits of private data. In this case, the "replacement" pointer becomes 
available as a private pointer; possibly to a metadata structure used by 
the subroutine.
This could also be used to significantly enhance the static-immediate 
framework, by being able to have explicit code which handles the 
transformations instead of needing to rely on assembly hacks. That way 
we might even be able to do that kind of transformations for any 
ro_after_init value.
I think the biggest concern is how this would affect objtool, since 
objtool would now not have any kind of direct visibility into the 
possibly generated code. How to best feed the information objtool needs 
to it would be my biggest question (in part because I don't know what 
objtool would actually need.)
        -hpa