[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <8d61e7a2-5eba-43c8-a38d-ca6ae59172b5@zytor.com>
Date: Thu, 15 May 2025 13:24:44 -0700
From: "H. Peter Anvin" <hpa@...or.com>
To: Xin Li <xin@...or.com>,
Jürgen Groß
<jgross@...e.com>,
linux-kernel@...r.kernel.org, x86@...nel.org,
virtualization@...ts.linux.dev, Peter Zijlstra <peterz@...radead.org>
Cc: Ajay Kaher <ajay.kaher@...adcom.com>,
Alexey Makhalov <alexey.amakhalov@...adcom.com>,
Broadcom internal kernel review list
<bcm-kernel-feedback-list@...adcom.com>,
Thomas Gleixner
<tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Boris Ostrovsky <boris.ostrovsky@...cle.com>,
xen-devel@...ts.xenproject.org
Subject: Re: [PATCH 5/6] x86/paravirt: Switch MSR access pv_ops functions to
instruction interfaces
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
Powered by blists - more mailing lists