* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Ville Syrjälä @ 2026-03-28 0:49 UTC (permalink / raw)
To: Nicolas Frattaroli
Cc: Maxime Ripard, Harry Wentland, Leo Li, Rodrigo Siqueira,
Alex Deucher, Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Thomas Zimmermann, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Sandy Huang, Heiko Stübner, Andy Yan,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Dmitry Baryshkov, Sascha Hauer, Rob Herring, Jonathan Corbet,
Shuah Khan, kernel, amd-gfx, dri-devel, linux-kernel,
linux-arm-kernel, linux-rockchip, intel-gfx, intel-xe, linux-doc,
Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <4153041.tdWV9SEqCh@workhorse>
On Fri, Mar 27, 2026 at 01:56:06PM +0100, Nicolas Frattaroli wrote:
> On Thursday, 26 March 2026 18:58:25 Central European Standard Time Ville Syrjälä wrote:
> > On Thu, Mar 26, 2026 at 06:02:47PM +0100, Maxime Ripard wrote:
> > > On Wed, Mar 25, 2026 at 08:43:15PM +0200, Ville Syrjälä wrote:
> > > > On Wed, Mar 25, 2026 at 03:56:58PM +0100, Maxime Ripard wrote:
> > > > > On Wed, Mar 25, 2026 at 01:03:07PM +0200, Ville Syrjälä wrote:
> > > > > > On Wed, Mar 25, 2026 at 09:24:27AM +0100, Maxime Ripard wrote:
> > > > > > > On Tue, Mar 24, 2026 at 09:53:35PM +0200, Ville Syrjälä wrote:
> > > > > > > > On Tue, Mar 24, 2026 at 08:10:11PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > > On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> > > > > > > > > > On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > > > > +enum drm_connector_color_format {
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > > > > > > > + * helpers should pick a suitable color format. All implementations of a
> > > > > > > > > > > + * specific display protocol must behave the same way with "AUTO", but
> > > > > > > > > > > + * different display protocols do not necessarily have the same "AUTO"
> > > > > > > > > > > + * semantics.
> > > > > > > > > > > + *
> > > > > > > > > > > + * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > > > > > > > + * bandwidth required for full-scale RGB is not available, or the mode
> > > > > > > > > > > + * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > > > > > > > + * YCbCr 4:2:0.
> > > > > > > > > > > + *
> > > > > > > > > > > + * For display protocols other than HDMI, the recursive bridge chain
> > > > > > > > > > > + * format selection picks the first chain of bridge formats that works,
> > > > > > > > > > > + * as has already been the case before the introduction of the "color
> > > > > > > > > > > + * format" property. Non-HDMI bridges should therefore either sort their
> > > > > > > > > > > + * bus output formats by preference, or agree on a unified auto format
> > > > > > > > > > > + * selection logic that's implemented in a common state helper (like
> > > > > > > > > > > + * how HDMI does it).
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > > > > > > > > > > + * not subsampled)
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > > > > > > > > > > + * with horizontal subsampling)
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > > > > > > > > > > +
> > > > > > > > > > > + /**
> > > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > > > > > > > > > > + * with horizontal and vertical subsampling)
> > > > > > > > > > > + */
> > > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> > > > > > > > > >
> > > > > > > > > > Seems like this should document what the quantization range
> > > > > > > > > > should be for each format.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I don't think so? If you want per-component bit depth values,
> > > > > > > > > DRM_FORMAT_* defines would be the appropriate values to use. This
> > > > > > > > > enum is more abstract than that, and is there to communicate
> > > > > > > > > YUV vs. RGB and chroma subsampling, with bit depth being handled
> > > > > > > > > by other properties.
> > > > > > > > >
> > > > > > > > > If you mean the factor used for subsampling, then that'd only be
> > > > > > > > > relevant if YCBCR410 was supported where one chroma plane isn't
> > > > > > > > > halved but quartered in resolution. I suspect 4:1:0 will never
> > > > > > > > > be added; no digital display protocol standard supports it to my
> > > > > > > > > knowledge, and hopefully none ever will.
> > > > > > > >
> > > > > > > > No, I mean the quantization range (16-235 vs. 0-255 etc).
> > > > > > > >
> > > > > > > > The i915 behaviour is that YCbCr is always limited range,
> > > > > > > > RGB can either be full or limited range depending on the
> > > > > > > > "Broadcast RGB" property and other related factors.
> > > > > > >
> > > > > > > So far the HDMI state has both the format and quantization range as
> > > > > > > different fields. I'm not sure we need to document the range in the
> > > > > > > format field, maybe only mention it's not part of the format but has a
> > > > > > > field of its own?
> > > > > >
> > > > > > I think we only have it for RGB (on some drivers only?). For YCbCr
> > > > > > I think the assumption is limited range everywhere.
> > > > > >
> > > > > > But I'm not really concerned about documenting struct members.
> > > > > > What I'm talking about is the *uapi* docs. Surely userspace
> > > > > > will want to know what the new property actually does so the
> > > > > > uapi needs to be documented properly. And down the line some
> > > > > > new driver might also implement the wrong behaviour if there
> > > > > > is no clear specification.
> > > > >
> > > > > Ack
> > > > >
> > > > > > So I'm thinking (or perhaps hoping) the rule might be something like:
> > > > > > - YCbCr limited range
> > > > > > - RGB full range if "Broadcast RGB" property is not present
> > > > >
> > > > > Isn't it much more complicated than that for HDMI though? My
> > > > > recollection was that any VIC but VIC1 would be limited range, and
> > > > > anything else full range?
> > > >
> > > > Do we have some driver that implements the CTA-861 CE vs. IT mode
> > > > logic but doesn't expose the "Broadcast RGB" property? I was hoping
> > > > those would always go hand in hand now.
> > >
> > > I'm not sure. i915 and the HDMI state helpers handle it properly (I
> > > think?) but it looks like only vc4 registers the Broadcast RGB property
> > > and uses the HDMI state helpers.
> > >
> > > And it looks like amdgpu registers Broadcast RGB but doesn't use
> > > drm_default_rgb_quant_range() which seems suspicious?
> >
> > If they want just manual full vs. limited then they should
> > limit the property to not expose the "auto" option at all.
> >
> > amdgpu also ties this in with the "colorspace" property, which
> > originally in i915 only controlled the infoframes/etc. But on
> > amdgpu it now controls various aspects of output color
> > transformation. The end result is that the property is a complete
> > mess with most of the values making no sense. And for whatever
> > reason everyone involved refused to remove/deprecate the
> > nonsensical values :/
> >
> > Looks like this series should make sure the documentation for
> > the "colorspace" property is in sync with the new property
> > as well. Currently now it's giving conflicting information.
> >
>
> I take it the problematic information is in
>
> * DOC: standard connector properties
> *
> * Colorspace:
>
> and probably specifically BT2020_YCC's (and BT2020_RGB's?) insistence
> that they "produce RGB content".
>
> I think we probably just have to change the statement "The variants
> BT2020_RGB and BT2020_YCC are equivalent and the driver chooses between
> RGB and YCbCr on its own."
>
> The "on its own" here would get turned into "based on the color format
> property".
>
> Speaking of i915, that patch is one of the very few (5) patches in
> this series still lacking a review (hint hint nudge nudge). I'd like
> to get some more feedback on the remaining patches before I send out
> another revision, so that it's hopefully not just docs changes (I
> know better than to think those patches must be perfect and won't
> need revision.)
The i915 code around this is already a big mess, and I don't really
adding to that mess. So I think we'll need to do some refactoring before
we add anything there. I already started typing something and so far
it looks fairly straightforward, so I should have something soon.
While doing that several questions came to my mind though:
* More interactions with the colorspace property, but I sent
a separate mail already about that
* Which conversion matrix to use, and the answer I suspect
should be "ask the colorspace property", as mentioned in the
other mail
* Should we flat out reject color formats (and I suppose also
colorspace prop values) the sink doesn't claim to support?
If yes, then I think we'll have to forget about adding anything
to i915 MST code. The way the MST stuff works is that if one
stream needs a modeset then all the related streams get modeset
as well. Thus if the user replaces a monitor getting fed with a
YCbCr stream just as another stream is being modeset, then the
entire atomic commit could fail due to the YCbCr stream getting
rejected.
I think eventually we might have to invent some mechanism where
all the input into the modeset computation is cached somehow,
and said cache updated only on explicit userspace modesets.
Either that or we have to come up with a way to skip some of
the calculations that depend on external factors. Either way
it's going to be a pain.
OTOH if we don't mind feeding the sink with stuff it can't
understand, then I suppose we might add YCbCr 4:4:4 support
for MST. It shouldn't be any different from RGB apart from
the RGB->YCbCr conversion, which is handled elsewhere. But
YCbCr 4:2:0 is definitely out either way, the MST code has
no support for that currently.
--
Ville Syrjälä
Intel
^ permalink raw reply
* Re: [PATCH v8 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Pawan Gupta @ 2026-03-28 0:42 UTC (permalink / raw)
To: David Laight
Cc: Borislav Petkov, x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin,
Josh Poimboeuf, David Kaplan, Sean Christopherson, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
Andy Lutomirski, Thomas Gleixner, Ingo Molnar, David Ahern,
Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
John Fastabend, Stanislav Fomichev, Hao Luo, Paolo Bonzini,
Jonathan Corbet, linux-kernel, kvm, Asit Mallick, Tao Zhang, bpf,
netdev, linux-doc
In-Reply-To: <20260326202931.wlggnd3nfj6hngpb@desk>
On Thu, Mar 26, 2026 at 01:29:31PM -0700, Pawan Gupta wrote:
> On Thu, Mar 26, 2026 at 10:45:57AM +0000, David Laight wrote:
> > On Thu, 26 Mar 2026 11:01:20 +0100
> > Borislav Petkov <bp@alien8.de> wrote:
> >
> > > On Thu, Mar 26, 2026 at 01:39:34AM -0700, Pawan Gupta wrote:
> > > > I believe the equivalent for cpu_feature_enabled() in asm is the
> > > > ALTERNATIVE. Please let me know if I am missing something.
> > >
> > > Yes, you are.
> > >
> > > The point is that you don't want to stick those alternative calls inside some
> > > magic bhb_loop function but hand them in from the outside, as function
> > > arguments.
> > >
> > > Basically what I did.
> > >
> > > Then you were worried about this being C code and it had to be noinstr... So
> > > that outer function can be rewritten in asm, I think, and still keep it well
> > > separate.
> > >
> > > I'll try to rewrite it once I get a free minute, and see how it looks.
> > >
> >
> > I think someone tried getting C code to write the values to global data
> > and getting the asm to read them.
> > That got discounted because it spilt things between two largely unrelated files.
>
>
> The implementation with global variables wasn't that bad, let me revive it.
>
> This part which ties sequence to BHI mitigation, which is not ideal,
> (because VMSCAPE also uses it) it does seems a cleaner option.
>
> --- a/arch/x86/kernel/cpu/bugs.c
> +++ b/arch/x86/kernel/cpu/bugs.c
> @@ -2095,6 +2095,11 @@ static void __init bhi_select_mitigation(void)
>
> static void __init bhi_update_mitigation(void)
> {
> + if (!cpu_feature_enabled(X86_FEATURE_BHI_CTRL)) {
> + bhi_seq_outer_loop = 5;
> + bhi_seq_inner_loop = 5;
> + }
> +
>
> I believe this can be moved to somewhere common to all mitigations.
>
> > I think the BPF code would need significant refactoring to call a C function.
>
> Ya, true. Will use globals and keep clear_bhb_loop() in asm.
While testing this approach, I noticed that syscalls were suffering an 8%
regression on ICX for Native BHI mitigation:
$ perf bench syscall basic -l 100000000
Bisection pointed to the change for using 8-bit registers (al/ah replacing
eax/ecx) as the main contributor to the regression. (Global variables added
a bit, but within noise).
Further digging revealed a strange behavior, using %ah for the inner loop
was causing the regression, interchanging %al and %ah in the loops
(for movb and sub) eliminated the regression.
<clear_bhb_loop_nofence>:
movb bhb_seq_outer_loop(%rip), %al
call 1f
jmp 5f
1: call 2f
.Lret1: RET
2: movb bhb_seq_inner_loop(%rip), %ah
3: jmp 4f
nop
4: sub $1, %ah <---- No regression with %al here
jnz 3b
sub $1, %al
jnz 1b
My guess is, "sub $1, %al" is faster than "sub $1, %ah". Using %al in the
inner loop, which is executed more number of times is likely making the
difference. A perf profile is needed to confirm this.
Never imagined a register selection can make an 8% difference in
performance! Anyways, will update the patch with this finding.
^ permalink raw reply
* Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"
From: Ville Syrjälä @ 2026-03-28 0:22 UTC (permalink / raw)
To: Maxime Ripard
Cc: Nicolas Frattaroli, Harry Wentland, Leo Li, Rodrigo Siqueira,
Alex Deucher, Christian König, David Airlie, Simona Vetter,
Maarten Lankhorst, Thomas Zimmermann, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Sandy Huang, Heiko Stübner, Andy Yan,
Jani Nikula, Rodrigo Vivi, Joonas Lahtinen, Tvrtko Ursulin,
Dmitry Baryshkov, Sascha Hauer, Rob Herring, Jonathan Corbet,
Shuah Khan, kernel, amd-gfx, dri-devel, linux-kernel,
linux-arm-kernel, linux-rockchip, intel-gfx, intel-xe, linux-doc,
Werner Sembach, Andri Yngvason, Marius Vlad
In-Reply-To: <acVzwRyk_J24GrJ4@intel.com>
On Thu, Mar 26, 2026 at 07:58:25PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 26, 2026 at 06:02:47PM +0100, Maxime Ripard wrote:
> > On Wed, Mar 25, 2026 at 08:43:15PM +0200, Ville Syrjälä wrote:
> > > On Wed, Mar 25, 2026 at 03:56:58PM +0100, Maxime Ripard wrote:
> > > > On Wed, Mar 25, 2026 at 01:03:07PM +0200, Ville Syrjälä wrote:
> > > > > On Wed, Mar 25, 2026 at 09:24:27AM +0100, Maxime Ripard wrote:
> > > > > > On Tue, Mar 24, 2026 at 09:53:35PM +0200, Ville Syrjälä wrote:
> > > > > > > On Tue, Mar 24, 2026 at 08:10:11PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> > > > > > > > > On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > > > > > > > > > +enum drm_connector_color_format {
> > > > > > > > > > + /**
> > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display protocol
> > > > > > > > > > + * helpers should pick a suitable color format. All implementations of a
> > > > > > > > > > + * specific display protocol must behave the same way with "AUTO", but
> > > > > > > > > > + * different display protocols do not necessarily have the same "AUTO"
> > > > > > > > > > + * semantics.
> > > > > > > > > > + *
> > > > > > > > > > + * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if the
> > > > > > > > > > + * bandwidth required for full-scale RGB is not available, or the mode
> > > > > > > > > > + * is YCbCr 4:2:0-only, as long as the mode and output both support
> > > > > > > > > > + * YCbCr 4:2:0.
> > > > > > > > > > + *
> > > > > > > > > > + * For display protocols other than HDMI, the recursive bridge chain
> > > > > > > > > > + * format selection picks the first chain of bridge formats that works,
> > > > > > > > > > + * as has already been the case before the introduction of the "color
> > > > > > > > > > + * format" property. Non-HDMI bridges should therefore either sort their
> > > > > > > > > > + * bus output formats by preference, or agree on a unified auto format
> > > > > > > > > > + * selection logic that's implemented in a common state helper (like
> > > > > > > > > > + * how HDMI does it).
> > > > > > > > > > + */
> > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > > > > > > > > > +
> > > > > > > > > > + /**
> > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > > > > > > > > > + */
> > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > > > > > > > > > +
> > > > > > > > > > + /**
> > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > > > > > > > > > + * not subsampled)
> > > > > > > > > > + */
> > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > > > > > > > > > +
> > > > > > > > > > + /**
> > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > > > > > > > > > + * with horizontal subsampling)
> > > > > > > > > > + */
> > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > > > > > > > > > +
> > > > > > > > > > + /**
> > > > > > > > > > + * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > > > > > > > > > + * with horizontal and vertical subsampling)
> > > > > > > > > > + */
> > > > > > > > > > + DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> > > > > > > > >
> > > > > > > > > Seems like this should document what the quantization range
> > > > > > > > > should be for each format.
> > > > > > > > >
> > > > > > > >
> > > > > > > > I don't think so? If you want per-component bit depth values,
> > > > > > > > DRM_FORMAT_* defines would be the appropriate values to use. This
> > > > > > > > enum is more abstract than that, and is there to communicate
> > > > > > > > YUV vs. RGB and chroma subsampling, with bit depth being handled
> > > > > > > > by other properties.
> > > > > > > >
> > > > > > > > If you mean the factor used for subsampling, then that'd only be
> > > > > > > > relevant if YCBCR410 was supported where one chroma plane isn't
> > > > > > > > halved but quartered in resolution. I suspect 4:1:0 will never
> > > > > > > > be added; no digital display protocol standard supports it to my
> > > > > > > > knowledge, and hopefully none ever will.
> > > > > > >
> > > > > > > No, I mean the quantization range (16-235 vs. 0-255 etc).
> > > > > > >
> > > > > > > The i915 behaviour is that YCbCr is always limited range,
> > > > > > > RGB can either be full or limited range depending on the
> > > > > > > "Broadcast RGB" property and other related factors.
> > > > > >
> > > > > > So far the HDMI state has both the format and quantization range as
> > > > > > different fields. I'm not sure we need to document the range in the
> > > > > > format field, maybe only mention it's not part of the format but has a
> > > > > > field of its own?
> > > > >
> > > > > I think we only have it for RGB (on some drivers only?). For YCbCr
> > > > > I think the assumption is limited range everywhere.
> > > > >
> > > > > But I'm not really concerned about documenting struct members.
> > > > > What I'm talking about is the *uapi* docs. Surely userspace
> > > > > will want to know what the new property actually does so the
> > > > > uapi needs to be documented properly. And down the line some
> > > > > new driver might also implement the wrong behaviour if there
> > > > > is no clear specification.
> > > >
> > > > Ack
> > > >
> > > > > So I'm thinking (or perhaps hoping) the rule might be something like:
> > > > > - YCbCr limited range
> > > > > - RGB full range if "Broadcast RGB" property is not present
> > > >
> > > > Isn't it much more complicated than that for HDMI though? My
> > > > recollection was that any VIC but VIC1 would be limited range, and
> > > > anything else full range?
> > >
> > > Do we have some driver that implements the CTA-861 CE vs. IT mode
> > > logic but doesn't expose the "Broadcast RGB" property? I was hoping
> > > those would always go hand in hand now.
> >
> > I'm not sure. i915 and the HDMI state helpers handle it properly (I
> > think?) but it looks like only vc4 registers the Broadcast RGB property
> > and uses the HDMI state helpers.
> >
> > And it looks like amdgpu registers Broadcast RGB but doesn't use
> > drm_default_rgb_quant_range() which seems suspicious?
>
> If they want just manual full vs. limited then they should
> limit the property to not expose the "auto" option at all.
>
> amdgpu also ties this in with the "colorspace" property, which
> originally in i915 only controlled the infoframes/etc. But on
> amdgpu it now controls various aspects of output color
> transformation. The end result is that the property is a complete
> mess with most of the values making no sense. And for whatever
> reason everyone involved refused to remove/deprecate the
> nonsensical values :/
>
> Looks like this series should make sure the documentation for
> the "colorspace" property is in sync with the new property
> as well. Currently now it's giving conflicting information.
After pondering about this a bit more I guess we could actually
use this to make all the values of the colorspace property make
some sense.
Since we won't have to worry about that RGB->YCbCr 4:2:0
fallback when using and explicit color format, all we'd have
to do is explicitly reject the nonsensical combinations:
color_format_and_colorspace_ok()
{
switch (color_format) {
case DRM_CONNECTOR_COLOR_FORMAT_YCBCR444:
case DRM_CONNECTOR_COLOR_FORMAT_YCBCR422:
case DRM_CONNECTOR_COLOR_FORMAT_YCBCR420:
switch (colorspace) {
case DRM_MODE_COLORIMETRY_NO_DATA:
case DRM_MODE_COLORIMETRY_SMPTE_170M_YCC:
case DRM_MODE_COLORIMETRY_BT601_YCC:
case DRM_MODE_COLORIMETRY_BT709_YCC:
case DRM_MODE_COLORIMETRY_XVYCC_601:
case DRM_MODE_COLORIMETRY_XVYCC_709:
case DRM_MODE_COLORIMETRY_SYCC_601:
case DRM_MODE_COLORIMETRY_OPYCC_601:
case DRM_MODE_COLORIMETRY_BT2020_CYCC:
case DRM_MODE_COLORIMETRY_BT2020_YCC:
case DRM_MODE_COLORIMETRY_BT709_YCC:
return true;
default:
return false;
}
break;
case DRM_CONNECTOR_COLOR_FORMAT_RGB444:
switch (colorspace) {
case DRM_MODE_COLORIMETRY_NO_DATA:
case DRM_MODE_COLORIMETRY_OPRGB:
case DRM_MODE_COLORIMETRY_BT2020_RGB:
case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
case DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER:
case DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED:
case DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT:
case DRM_MODE_COLORIMETRY_OPRGB:
case DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65:
case DRM_MODE_COLORIMETRY_BT2020_RGB:
return true;
default
return false;
}
break;
case DRM_CONNECTOR_COLOR_FORMAT_AUTO:
switch (colorspace) {
case DRM_MODE_COLORIMETRY_NO_DATA:
case DRM_MODE_COLORIMETRY_BT2020_RGB:
case DRM_MODE_COLORIMETRY_BT2020_YCC:
return true;
default:
return false;
}
default:
bad;
}
}
And then presumably the colorspace property is the thing that should
dictate which conversion matrix to use. So something like this:
csc_matrix()
{
switch (colorspace) {
case DRM_MODE_COLORIMETRY_SMPTE_170M_YCC:
case DRM_MODE_COLORIMETRY_BT601_YCC:
case DRM_MODE_COLORIMETRY_XVYCC_601:
case DRM_MODE_COLORIMETRY_SYCC_601:
case DRM_MODE_COLORIMETRY_OPYCC_601:
return 601;
case DRM_MODE_COLORIMETRY_BT709_YCC:
case DRM_MODE_COLORIMETRY_XVYCC_709:
return 709;
case DRM_MODE_COLORIMETRY_BT2020_YCC:
case DRM_MODE_COLORIMETRY_BT2020_RGB:
return 2020;
case DRM_MODE_COLORIMETRY_BT2020_CYCC:
return 2020_const;
case DRM_MODE_COLORIMETRY_NO_DATA:
return vdisplay >= 720 ? 709 : 601;
default:
bad;
}
}
--
Ville Syrjälä
Intel
^ permalink raw reply
* Re: [PATCH] doc tools: better handle KBUILD_VERBOSE
From: Mauro Carvalho Chehab @ 2026-03-28 0:09 UTC (permalink / raw)
To: Jacob Keller
Cc: Jonathan Corbet, Linux Doc Mailing List, linux-kernel,
Mauro Carvalho Chehab, Randy Dunlap, Shuah Khan
In-Reply-To: <540ceb4a-fd54-4e95-9431-8f233252259c@intel.com>
On Fri, 27 Mar 2026 11:35:39 -0700
Jacob Keller <jacob.e.keller@intel.com> wrote:
> On 3/26/2026 10:57 PM, Mauro Carvalho Chehab wrote:
> > As reported by Jacob, there are troubles when KBUILD_VERBOSE is
> > set at the environment.
> >
> > Fix it on both kernel-doc and sphinx-build-wrapper.
> >
> > Reported-by: Jacob Keller <jacob.e.keller@intel.com>
> > Closes: https://lore.kernel.org/linux-doc/9367d899-53af-4d9c-9320-22fc4dbadca5@intel.com/
> > Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
> > ---
>
> I loaded this on my system and tested the build works as expected both
> with V=0 and when I export KBUILD_VERBOSE manually.
>
> Thanks for fixing this quickly!
>
> Tested-by: Jacob Keller <jacob.e.keller@intel.com>
>
> > tools/docs/sphinx-build-wrapper | 7 ++++++-
> > tools/lib/python/kdoc/kdoc_files.py | 7 ++++++-
> > 2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/docs/sphinx-build-wrapper b/tools/docs/sphinx-build-wrapper
> > index 2c63d28f639d..1bb962202784 100755
> > --- a/tools/docs/sphinx-build-wrapper
> > +++ b/tools/docs/sphinx-build-wrapper
> > @@ -238,7 +238,12 @@ class SphinxBuilder:
> > self.latexopts = os.environ.get("LATEXOPTS", "")
> >
> > if not verbose:
> > - verbose = bool(os.environ.get("KBUILD_VERBOSE", "") != "")
> > + try:
> > + verbose = bool(int(os.environ.get("KBUILD_VERBOSE", 0)))
> > + except ValueError:
> > + # Handles an eventual case where verbosity is not a number
> > + # like KBUILD_VERBOSE=""
>
> Strictly speaking I think os.environ.get() will handle the case of an
> empty KBUILD_VERBOSE by converting to the default value (in this case 0).
It won't. See:
$ FOO="" python3
Python 3.14.3 (main, Feb 4 2026, 00:00:00) [GCC 15.2.1 20260123 (Red Hat 15.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.environ.get("FOO", 0)
''
$ FOO="0" python3
Python 3.14.3 (main, Feb 4 2026, 00:00:00) [GCC 15.2.1 20260123 (Red Hat 15.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.environ.get("FOO", 0)
...
'0'
$ unset FOO; python3
Python 3.14.3 (main, Feb 4 2026, 00:00:00) [GCC 15.2.1 20260123 (Red Hat 15.2.1-7)] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import os
>>> os.environ.get("FOO", 0)
...
0
it will only get an integer 0 if the env var (at the above example, FOO)
is not on env.
That's basically why we need to first convert it to integer than to
bool.
> The intent of the comment and code is pretty clear though, so I don't
> know that deserves a re-roll.
I opted to add a comment there because having two conversions,
first to int then to bool is not that obvious ;-)
Thanks,
Mauro
^ permalink raw reply
* [PATCH v7 9/9] KVM: selftests: nSVM: Add svm_nested_pat test
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled, verify that KVM
correctly virtualizes the host PAT MSR and the guest PAT register for
nested SVM guests.
With nested NPT disabled:
* L1 and L2 share the same PAT
* The vmcb12.g_pat is ignored
With nested NPT enabled:
* An invalid g_pat in vmcb12 causes VMEXIT_INVALID
* RDMSR(IA32_PAT) from L2 returns the value of the guest PAT register
* WRMSR(IA32_PAT) from L2 is reflected in vmcb12's g_pat on VMEXIT
* RDMSR(IA32_PAT) from L1 returns the value of the host PAT MSR
* Save/restore with the vCPU in guest mode preserves both hPAT and gPAT
Signed-off-by: Jim Mattson <jmattson@google.com>
---
tools/arch/x86/include/uapi/asm/kvm.h | 2 +
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../selftests/kvm/x86/svm_nested_pat_test.c | 304 ++++++++++++++++++
3 files changed, 307 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
diff --git a/tools/arch/x86/include/uapi/asm/kvm.h b/tools/arch/x86/include/uapi/asm/kvm.h
index 7ceff6583652..be6f428a79aa 100644
--- a/tools/arch/x86/include/uapi/asm/kvm.h
+++ b/tools/arch/x86/include/uapi/asm/kvm.h
@@ -476,6 +476,7 @@ struct kvm_sync_regs {
#define KVM_X86_QUIRK_SLOT_ZAP_ALL (1 << 7)
#define KVM_X86_QUIRK_STUFF_FEATURE_MSRS (1 << 8)
#define KVM_X86_QUIRK_IGNORE_GUEST_PAT (1 << 9)
+#define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT (1 << 11)
#define KVM_STATE_NESTED_FORMAT_VMX 0
#define KVM_STATE_NESTED_FORMAT_SVM 1
@@ -530,6 +531,7 @@ struct kvm_svm_nested_state_data {
struct kvm_svm_nested_state_hdr {
__u64 vmcb_pa;
+ __u64 gpat;
};
/* for KVM_CAP_NESTED_STATE */
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 3d372d78a275..88871572ee9d 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -113,6 +113,7 @@ TEST_GEN_PROGS_x86 += x86/svm_vmcall_test
TEST_GEN_PROGS_x86 += x86/svm_int_ctl_test
TEST_GEN_PROGS_x86 += x86/svm_nested_clear_efer_svme
TEST_GEN_PROGS_x86 += x86/svm_nested_invalid_vmcb12_gpa
+TEST_GEN_PROGS_x86 += x86/svm_nested_pat_test
TEST_GEN_PROGS_x86 += x86/svm_nested_shutdown_test
TEST_GEN_PROGS_x86 += x86/svm_nested_soft_inject_test
TEST_GEN_PROGS_x86 += x86/svm_lbr_nested_state
diff --git a/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c b/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
new file mode 100644
index 000000000000..704f31a079a9
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KVM nested SVM PAT test
+ *
+ * Copyright (C) 2026, Google LLC.
+ *
+ * Test that KVM correctly virtualizes the PAT MSR and VMCB g_pat field
+ * for nested SVM guests:
+ *
+ * o With nested NPT disabled:
+ * - L1 and L2 share the same PAT
+ * - The vmcb12.g_pat is ignored
+ * o With nested NPT enabled:
+ * - Invalid g_pat in vmcb12 should cause VMEXIT_INVALID
+ * - L2 should see vmcb12.g_pat via RDMSR, not L1's PAT
+ * - L2's writes to PAT should be saved to vmcb12 on exit
+ * - L1's PAT should be restored after #VMEXIT from L2
+ * - State save/restore should preserve both L1's and L2's PAT values
+ */
+#include <fcntl.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "svm_util.h"
+
+#define L2_GUEST_STACK_SIZE 256
+
+#define PAT_DEFAULT 0x0007040600070406ULL
+#define L1_PAT_VALUE 0x0007040600070404ULL /* Change PA0 to WT */
+#define L2_VMCB12_PAT 0x0606060606060606ULL /* All WB */
+#define L2_PAT_MODIFIED 0x0606060606060604ULL /* Change PA0 to WT */
+#define INVALID_PAT_VALUE 0x0808080808080808ULL /* 8 is reserved */
+
+/*
+ * Shared state between L1 and L2 for verification.
+ */
+struct pat_test_data {
+ uint64_t l2_pat_read;
+ uint64_t l2_pat_after_write;
+ uint64_t l1_pat_after_vmexit;
+ uint64_t vmcb12_gpat_after_exit;
+ bool l2_done;
+};
+
+static struct pat_test_data *pat_data;
+
+static void l2_guest_code(void)
+{
+ pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
+ wrmsr(MSR_IA32_CR_PAT, L2_PAT_MODIFIED);
+ pat_data->l2_pat_after_write = rdmsr(MSR_IA32_CR_PAT);
+ pat_data->l2_done = true;
+ vmmcall();
+}
+
+static void l2_guest_code_saverestoretest(void)
+{
+ pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
+
+ GUEST_SYNC(1);
+ GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), pat_data->l2_pat_read);
+
+ wrmsr(MSR_IA32_CR_PAT, L2_PAT_MODIFIED);
+ pat_data->l2_pat_after_write = rdmsr(MSR_IA32_CR_PAT);
+
+ GUEST_SYNC(2);
+ GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L2_PAT_MODIFIED);
+
+ pat_data->l2_done = true;
+ vmmcall();
+}
+
+static void l2_guest_code_multi_vmentry(void)
+{
+ pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
+ wrmsr(MSR_IA32_CR_PAT, L2_PAT_MODIFIED);
+ pat_data->l2_pat_after_write = rdmsr(MSR_IA32_CR_PAT);
+ vmmcall();
+
+ pat_data->l2_pat_read = rdmsr(MSR_IA32_CR_PAT);
+ pat_data->l2_done = true;
+ vmmcall();
+}
+
+static struct vmcb *l1_common_setup(struct svm_test_data *svm,
+ struct pat_test_data *data,
+ void *l2_guest_code,
+ void *l2_guest_stack)
+{
+ struct vmcb *vmcb = svm->vmcb;
+
+ pat_data = data;
+
+ wrmsr(MSR_IA32_CR_PAT, L1_PAT_VALUE);
+ GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+ generic_svm_setup(svm, l2_guest_code, l2_guest_stack);
+
+ vmcb->save.g_pat = L2_VMCB12_PAT;
+ vmcb->control.intercept &= ~(1ULL << INTERCEPT_MSR_PROT);
+
+ return vmcb;
+}
+
+static void l1_assert_l2_state(struct pat_test_data *data, uint64_t expected_pat_read)
+{
+ GUEST_ASSERT(data->l2_done);
+ GUEST_ASSERT_EQ(data->l2_pat_read, expected_pat_read);
+ GUEST_ASSERT_EQ(data->l2_pat_after_write, L2_PAT_MODIFIED);
+}
+
+static void l1_svm_code_npt_disabled(struct svm_test_data *svm,
+ struct pat_test_data *data)
+{
+ unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+ struct vmcb *vmcb;
+
+ vmcb = l1_common_setup(svm, data, l2_guest_code,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+ run_guest(vmcb, svm->vmcb_gpa);
+
+ GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+ l1_assert_l2_state(data, L1_PAT_VALUE);
+
+ data->l1_pat_after_vmexit = rdmsr(MSR_IA32_CR_PAT);
+ GUEST_ASSERT_EQ(data->l1_pat_after_vmexit, L2_PAT_MODIFIED);
+
+ GUEST_DONE();
+}
+
+static void l1_svm_code_invalid_gpat(struct svm_test_data *svm,
+ struct pat_test_data *data)
+{
+ unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+ struct vmcb *vmcb;
+
+ vmcb = l1_common_setup(svm, data, l2_guest_code,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+ vmcb->save.g_pat = INVALID_PAT_VALUE;
+
+ run_guest(vmcb, svm->vmcb_gpa);
+
+ GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_ERR);
+ GUEST_ASSERT(!data->l2_done);
+
+ GUEST_DONE();
+}
+
+static void l1_svm_code_npt_enabled(struct svm_test_data *svm,
+ struct pat_test_data *data)
+{
+ unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+ struct vmcb *vmcb;
+
+ vmcb = l1_common_setup(svm, data, l2_guest_code,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+ run_guest(vmcb, svm->vmcb_gpa);
+
+ GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+ l1_assert_l2_state(data, L2_VMCB12_PAT);
+
+ data->vmcb12_gpat_after_exit = vmcb->save.g_pat;
+ GUEST_ASSERT_EQ(data->vmcb12_gpat_after_exit, L2_PAT_MODIFIED);
+
+ data->l1_pat_after_vmexit = rdmsr(MSR_IA32_CR_PAT);
+ GUEST_ASSERT_EQ(data->l1_pat_after_vmexit, L1_PAT_VALUE);
+
+ GUEST_DONE();
+}
+
+static void l1_svm_code_saverestore(struct svm_test_data *svm,
+ struct pat_test_data *data)
+{
+ unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+ struct vmcb *vmcb;
+
+ vmcb = l1_common_setup(svm, data, l2_guest_code_saverestoretest,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+ run_guest(vmcb, svm->vmcb_gpa);
+
+ GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+ GUEST_ASSERT(data->l2_done);
+
+ GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+ GUEST_ASSERT_EQ(vmcb->save.g_pat, L2_PAT_MODIFIED);
+
+ GUEST_DONE();
+}
+
+static void l1_svm_code_multi_vmentry(struct svm_test_data *svm,
+ struct pat_test_data *data)
+{
+ unsigned long l2_guest_stack[L2_GUEST_STACK_SIZE];
+ struct vmcb *vmcb;
+
+ vmcb = l1_common_setup(svm, data, l2_guest_code_multi_vmentry,
+ &l2_guest_stack[L2_GUEST_STACK_SIZE]);
+
+ run_guest(vmcb, svm->vmcb_gpa);
+ GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+
+ GUEST_ASSERT_EQ(data->l2_pat_after_write, L2_PAT_MODIFIED);
+ GUEST_ASSERT_EQ(vmcb->save.g_pat, L2_PAT_MODIFIED);
+ GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+ vmcb->save.rip += 3; /* vmmcall */
+ run_guest(vmcb, svm->vmcb_gpa);
+
+ GUEST_ASSERT_EQ(vmcb->control.exit_code, SVM_EXIT_VMMCALL);
+ GUEST_ASSERT(data->l2_done);
+ GUEST_ASSERT_EQ(data->l2_pat_read, L2_PAT_MODIFIED);
+ GUEST_ASSERT_EQ(rdmsr(MSR_IA32_CR_PAT), L1_PAT_VALUE);
+
+ GUEST_DONE();
+}
+
+static void run_test(void *l1_code, const char *test_name, bool npt_enabled,
+ bool do_save_restore)
+{
+ struct pat_test_data *data_hva;
+ vm_vaddr_t svm_gva, data_gva;
+ struct kvm_x86_state *state;
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+ struct ucall uc;
+
+ pr_info("Testing: %s\n", test_name);
+
+ vm = vm_create_with_one_vcpu(&vcpu, l1_code);
+ vm_enable_cap(vm, KVM_CAP_DISABLE_QUIRKS2,
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+ if (npt_enabled)
+ vm_enable_npt(vm);
+
+ vcpu_alloc_svm(vm, &svm_gva);
+
+ data_gva = vm_vaddr_alloc_page(vm);
+ data_hva = addr_gva2hva(vm, data_gva);
+ memset(data_hva, 0, sizeof(*data_hva));
+
+ if (npt_enabled)
+ tdp_identity_map_default_memslots(vm);
+
+ vcpu_args_set(vcpu, 2, svm_gva, data_gva);
+
+ for (;;) {
+ vcpu_run(vcpu);
+ TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ /* NOT REACHED */
+ case UCALL_SYNC:
+ if (do_save_restore) {
+ pr_info(" Save/restore at sync point %ld\n",
+ uc.args[1]);
+ state = vcpu_save_state(vcpu);
+ kvm_vm_release(vm);
+ vcpu = vm_recreate_with_one_vcpu(vm);
+ vm_enable_cap(vm, KVM_CAP_DISABLE_QUIRKS2,
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+ vcpu_load_state(vcpu, state);
+ kvm_x86_state_cleanup(state);
+ }
+ break;
+ case UCALL_DONE:
+ pr_info(" PASSED\n");
+ kvm_vm_free(vm);
+ return;
+ default:
+ TEST_FAIL("Unknown ucall %lu", uc.cmd);
+ }
+ }
+}
+
+int main(int argc, char *argv[])
+{
+ TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_SVM));
+ TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_NPT));
+ TEST_REQUIRE(kvm_has_cap(KVM_CAP_NESTED_STATE));
+ TEST_REQUIRE(kvm_check_cap(KVM_CAP_DISABLE_QUIRKS2) &
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+
+ run_test(l1_svm_code_npt_disabled, "nested NPT disabled", false, false);
+
+ run_test(l1_svm_code_invalid_gpat, "invalid g_pat", true, false);
+
+ run_test(l1_svm_code_npt_enabled, "nested NPT enabled", true, false);
+
+ run_test(l1_svm_code_saverestore, "save/restore", true, true);
+
+ run_test(l1_svm_code_multi_vmentry, "multiple entries", true, false);
+
+ return 0;
+}
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 8/9] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
Add a 'gpat' field to kvm_svm_nested_state_hdr to carry L2's guest PAT
value across save and restore.
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and the vCPU is in
guest mode with nested NPT enabled, save vmcb02's g_pat into the header on
KVM_GET_NESTED_STATE, and restore it on KVM_SET_NESTED_STATE.
Host-initiated accesses to IA32_PAT (via KVM_GET/SET_MSRS) always target
L1's hPAT, so they cannot be used to save or restore gPAT. The separate
header field ensures that KVM_GET/SET_MSRS and KVM_GET/SET_NESTED_STATE are
independent and can be ordered arbitrarily during save and restore.
Note that struct kvm_svm_nested_state_hdr is included in a union padded to
120 bytes, so there is room to add the gpat field without changing any
offsets.
Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
Documentation/virt/kvm/api.rst | 1 +
arch/x86/include/uapi/asm/kvm.h | 1 +
arch/x86/kvm/svm/nested.c | 20 ++++++++++++++++++++
3 files changed, 22 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 0a2d873ca5a3..d6bbb7bad173 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4965,6 +4965,7 @@ Errors:
struct kvm_svm_nested_state_hdr {
__u64 vmcb_pa;
+ __u64 gpat;
};
struct kvm_vmx_nested_state_data {
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 3ada2fa9ca86..1585ec804066 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -533,6 +533,7 @@ struct kvm_svm_nested_state_data {
struct kvm_svm_nested_state_hdr {
__u64 vmcb_pa;
+ __u64 gpat;
};
/* for KVM_CAP_NESTED_STATE */
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index add57f2a2d9f..54399e642745 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1865,6 +1865,9 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
/* First fill in the header and copy it out. */
if (is_guest_mode(vcpu)) {
kvm_state.hdr.svm.vmcb_pa = svm->nested.vmcb12_gpa;
+ kvm_state.hdr.svm.gpat = 0;
+ if (l2_has_separate_pat(svm))
+ kvm_state.hdr.svm.gpat = svm->vmcb->save.g_pat;
kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
@@ -1918,6 +1921,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
struct vmcb_save_area_cached save_cached;
struct vmcb_ctrl_area_cached ctl_cached;
unsigned long cr0;
+ bool use_separate_l2_pat;
int ret;
BUILD_BUG_ON(sizeof(struct vmcb_control_area) + sizeof(struct vmcb_save_area) >
@@ -1993,6 +1997,18 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
!nested_vmcb_check_save(vcpu, &save_cached, false))
goto out_free;
+ /*
+ * Validate gPAT when the shared PAT quirk is disabled (i.e. L2
+ * has its own gPAT). This is done separately from the
+ * vmcb_save_area_cached validation above, because gPAT is L2
+ * state, but the vmcb_save_area_cached is populated with L1 state.
+ */
+ use_separate_l2_pat =
+ (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP) &&
+ !kvm_check_has_quirk(vcpu->kvm,
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+ if (use_separate_l2_pat && !kvm_pat_valid(kvm_state->hdr.svm.gpat))
+ goto out_free;
/*
* All checks done, we can enter guest mode. Userspace provides
@@ -2017,6 +2033,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
nested_copy_vmcb_control_to_cache(svm, ctl);
svm_switch_vmcb(svm, &svm->nested.vmcb02);
+
+ if (use_separate_l2_pat)
+ vmcb_set_gpat(svm->vmcb, kvm_state->hdr.svm.gpat);
+
nested_vmcb02_prepare_control(svm);
/*
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 7/9] KVM: Documentation: document KVM_{GET,SET}_NESTED_STATE for SVM
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
Document the nested state constants and structures for SVM that were
added by commit cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE
and KVM_SET_NESTED_STATE").
Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
Documentation/virt/kvm/api.rst | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 2d56f17e3760..0a2d873ca5a3 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4942,10 +4942,13 @@ Errors:
#define KVM_STATE_NESTED_FORMAT_SVM 1
#define KVM_STATE_NESTED_VMX_VMCS_SIZE 0x1000
+ #define KVM_STATE_NESTED_SVM_VMCB_SIZE 0x1000
#define KVM_STATE_NESTED_VMX_SMM_GUEST_MODE 0x00000001
#define KVM_STATE_NESTED_VMX_SMM_VMXON 0x00000002
+ #define KVM_STATE_NESTED_GIF_SET 0x00000100
+
#define KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE 0x00000001
struct kvm_vmx_nested_state_hdr {
@@ -4960,11 +4963,19 @@ Errors:
__u64 preemption_timer_deadline;
};
+ struct kvm_svm_nested_state_hdr {
+ __u64 vmcb_pa;
+ };
+
struct kvm_vmx_nested_state_data {
__u8 vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
__u8 shadow_vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
};
+ struct kvm_svm_nested_state_data {
+ __u8 vmcb12[KVM_STATE_NESTED_SVM_VMCB_SIZE];
+ };
+
This ioctl copies the vcpu's nested virtualization state from the kernel to
userspace.
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 6/9] KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
According to the APM volume 3 pseudo-code for "VMRUN," when nested paging
is enabled in the vmcb, the guest PAT register (gPAT) is saved to the vmcb
on emulated VMEXIT.
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and the vCPU is in
guest mode with nested NPT enabled, save the vmcb02 g_pat field to the
vmcb12 g_pat field on emulated VMEXIT.
Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index ccc556eb4d2f..add57f2a2d9f 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1248,6 +1248,9 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
vmcb12->save.dr6 = svm->vcpu.arch.dr6;
vmcb12->save.cpl = vmcb02->save.cpl;
+ if (l2_has_separate_pat(svm))
+ vmcb12->save.g_pat = vmcb02->save.g_pat;
+
if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK)) {
vmcb12->save.s_cet = vmcb02->save.s_cet;
vmcb12->save.isst_addr = vmcb02->save.isst_addr;
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 5/9] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and the vCPU is in
guest mode with nested NPT enabled, guest accesses to IA32_PAT are
redirected to the gPAT register, which is stored in VMCB02's g_pat field.
Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
to hPAT, which is stored in vcpu->arch.pat.
Directing host-initiated accesses to hPAT ensures that KVM_GET/SET_MSRS and
KVM_GET/SET_NESTED_STATE are independent of each other and can be ordered
arbitrarily during save and restore. gPAT is saved and restored separately
via KVM_GET/SET_NESTED_STATE.
Use WARN_ON_ONCE to flag any host-initiated accesses originating from KVM
itself rather than userspace.
Use pr_warn_once to flag any use of the common MSR-handling code (now
shared by VMX and TDX) for IA32_PAT by a vCPU that is SVM-capable.
Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/nested.c | 9 -------
arch/x86/kvm/svm/svm.c | 53 ++++++++++++++++++++++++++++++++++-----
arch/x86/kvm/svm/svm.h | 1 -
arch/x86/kvm/x86.c | 6 +++++
4 files changed, 53 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 8170042d5fb3..ccc556eb4d2f 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -703,15 +703,6 @@ static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
return 0;
}
-void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
-{
- if (!svm->nested.vmcb02.ptr)
- return;
-
- /* FIXME: merge g_pat from vmcb01 and vmcb12. */
- vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
-}
-
static bool nested_vmcb12_has_lbrv(struct kvm_vcpu *vcpu)
{
return guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index af808e83173e..34fd07d6ad4d 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2776,6 +2776,47 @@ static bool sev_es_prevent_msr_access(struct kvm_vcpu *vcpu,
!msr_write_intercepted(vcpu, msr_info->index);
}
+static bool svm_pat_accesses_gpat(struct kvm_vcpu *vcpu, bool from_host)
+{
+ struct vcpu_svm *svm = to_svm(vcpu);
+
+ /*
+ * When nested NPT is enabled, L2 has a separate PAT from L1. Guest
+ * accesses to IA32_PAT while running L2 target L2's gPAT;
+ * host-initiated accesses always target L1's hPAT so that
+ * KVM_GET/SET_MSRS and KVM_GET/SET_NESTED_STATE are independent of
+ * each other and can be ordered arbitrarily during save and restore.
+ */
+ WARN_ON_ONCE(from_host && vcpu->wants_to_run);
+ return !from_host && is_guest_mode(vcpu) && l2_has_separate_pat(svm);
+}
+
+static u64 svm_get_pat(struct kvm_vcpu *vcpu, bool from_host)
+{
+ if (svm_pat_accesses_gpat(vcpu, from_host))
+ return to_svm(vcpu)->vmcb->save.g_pat;
+ else
+ return vcpu->arch.pat;
+}
+
+static void svm_set_pat(struct kvm_vcpu *vcpu, bool from_host, u64 data)
+{
+ struct vcpu_svm *svm = to_svm(vcpu);
+
+ if (svm_pat_accesses_gpat(vcpu, from_host)) {
+ vmcb_set_gpat(svm->vmcb, data);
+ return;
+ }
+
+ svm->vcpu.arch.pat = data;
+
+ if (npt_enabled) {
+ vmcb_set_gpat(svm->vmcb01.ptr, data);
+ if (is_guest_mode(&svm->vcpu) && !l2_has_separate_pat(svm))
+ vmcb_set_gpat(svm->vmcb, data);
+ }
+}
+
static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
{
struct vcpu_svm *svm = to_svm(vcpu);
@@ -2892,6 +2933,9 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_AMD64_DE_CFG:
msr_info->data = svm->msr_decfg;
break;
+ case MSR_IA32_CR_PAT:
+ msr_info->data = svm_get_pat(vcpu, msr_info->host_initiated);
+ break;
default:
return kvm_get_msr_common(vcpu, msr_info);
}
@@ -2975,13 +3019,10 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
break;
case MSR_IA32_CR_PAT:
- ret = kvm_set_msr_common(vcpu, msr);
- if (ret)
- break;
+ if (!kvm_pat_valid(data))
+ return 1;
- vmcb_set_gpat(svm->vmcb01.ptr, data);
- if (is_guest_mode(vcpu))
- nested_vmcb02_compute_g_pat(svm);
+ svm_set_pat(vcpu, msr->host_initiated, data);
break;
case MSR_IA32_SPEC_CTRL:
if (!msr->host_initiated &&
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index b43e37b0448c..0b0279734486 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -868,7 +868,6 @@ void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
struct vmcb_save_area *save);
void nested_sync_control_from_vmcb02(struct vcpu_svm *svm);
-void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm);
void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb);
extern struct kvm_x86_nested_ops svm_nested_ops;
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0b5d48e75b65..cfb2517f692a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4025,6 +4025,9 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
}
break;
case MSR_IA32_CR_PAT:
+ if (!(efer_reserved_bits & EFER_SVME))
+ pr_warn_once("%s: MSR_IA32_CR_PAT should be handled by AMD vendor-specific code\n", __func__);
+
if (!kvm_pat_valid(data))
return 1;
@@ -4436,6 +4439,9 @@ int kvm_get_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
break;
}
case MSR_IA32_CR_PAT:
+ if (!(efer_reserved_bits & EFER_SVME))
+ pr_warn_once("%s: MSR_IA32_CR_PAT should be handled by AMD vendor-specific code\n", __func__);
+
msr_info->data = vcpu->arch.pat;
break;
case MSR_MTRRcap:
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 4/9] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and nested NPT is
enabled in vmcb12, copy the (cached and validated) vmcb12 g_pat field to
vmcb02's g_pat, giving L2 its own independent guest PAT register.
When the quirk is enabled (default), or when NPT is enabled but nested NPT
is disabled, copy L1's IA32_PAT MSR to the vmcb02 g_pat field, since L2
shares the IA32_PAT MSR with L1.
When NPT is disabled, the g_pat field is ignored by hardware.
Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index cb837842f2c3..8170042d5fb3 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -727,9 +727,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
struct kvm_vcpu *vcpu = &svm->vcpu;
- nested_vmcb02_compute_g_pat(svm);
- vmcb_mark_dirty(vmcb02, VMCB_NPT);
-
/* Load the nested guest state */
if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
new_vmcb12 = true;
@@ -760,6 +757,13 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
vmcb_mark_dirty(vmcb02, VMCB_CET);
}
+ if (l2_has_separate_pat(svm)) {
+ if (unlikely(new_vmcb12 || vmcb12_is_dirty(control, VMCB_NPT)))
+ vmcb_set_gpat(vmcb02, svm->nested.save.g_pat);
+ } else if (npt_enabled) {
+ vmcb_set_gpat(vmcb02, vcpu->arch.pat);
+ }
+
kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
svm_set_efer(vcpu, svm->nested.save.efer);
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 3/9] KVM: x86: nSVM: Cache and validate vmcb12 g_pat
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and nested paging is
enabled in vmcb12, validate g_pat at emulated VMRUN and cause an immediate
VMEXIT with exit code VMEXIT_INVALID if it is invalid, as specified in the
APM, volume 2: "Nested Paging and VMRUN/VMEXIT."
Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 23 +++++++++++++++++++----
arch/x86/kvm/svm/svm.h | 1 +
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 32fa8e688c00..cb837842f2c3 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -410,7 +410,8 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
/* Common checks that apply to both L1 and L2 state. */
static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
- struct vmcb_save_area_cached *save)
+ struct vmcb_save_area_cached *save,
+ bool check_gpat)
{
if (CC(!(save->efer & EFER_SVME)))
return false;
@@ -445,6 +446,15 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
if (CC(!kvm_valid_efer(vcpu, save->efer)))
return false;
+ /*
+ * If userspace contrives to get an invalid g_pat into vmcb02 by
+ * disabling KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT in a race with
+ * this check, it should be prepared for the KVM_EXIT_FAIL_ENTRY
+ * that will follow.
+ */
+ if (check_gpat && CC(!kvm_pat_valid(save->g_pat)))
+ return false;
+
return true;
}
@@ -452,7 +462,8 @@ int nested_svm_check_cached_vmcb12(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
- if (!nested_vmcb_check_save(vcpu, &svm->nested.save) ||
+ if (!nested_vmcb_check_save(vcpu, &svm->nested.save,
+ l2_has_separate_pat(svm)) ||
!nested_vmcb_check_controls(vcpu, &svm->nested.ctl))
return -EINVAL;
@@ -562,6 +573,7 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
to->rax = from->rax;
to->cr2 = from->cr2;
+ to->g_pat = from->g_pat;
svm_copy_lbrs(to, from);
}
@@ -1971,13 +1983,16 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
/*
* Validate host state saved from before VMRUN (see
- * nested_svm_check_permissions).
+ * nested_svm_check_permissions). Note that the g_pat field is not
+ * validated, because (a) it may have been clobbered by SMM before
+ * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
+ * #VMEXIT.
*/
__nested_copy_vmcb_save_to_cache(&save_cached, save);
if (!(save->cr0 & X86_CR0_PG) ||
!(save->cr0 & X86_CR0_PE) ||
(save->rflags & X86_EFLAGS_VM) ||
- !nested_vmcb_check_save(vcpu, &save_cached))
+ !nested_vmcb_check_save(vcpu, &save_cached, false))
goto out_free;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index a1d62c3cc3d6..b43e37b0448c 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -161,6 +161,7 @@ struct vmcb_save_area_cached {
u64 isst_addr;
u64 rax;
u64 cr2;
+ u64 g_pat;
u64 dbgctl;
u64 br_from;
u64 br_to;
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 2/9] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
When running an L2 guest and writing to MSR_IA32_CR_PAT, the host PAT value
is stored in both vmcb01's g_pat field and vmcb02's g_pat field, but the
clean bit was only being cleared for vmcb02.
Introduce the helper vmcb_set_gpat() which sets vmcb->save.g_pat and marks
the VMCB dirty for VMCB_NPT. Use this helper in both svm_set_msr() for
updating vmcb01 and in nested_vmcb02_compute_g_pat() for updating vmcb02,
ensuring both VMCBs' NPT fields are properly marked dirty.
Fixes: 4995a3685f1b ("KVM: SVM: Use a separate vmcb for the nested L2 guest")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 2 +-
arch/x86/kvm/svm/svm.c | 3 +--
arch/x86/kvm/svm/svm.h | 6 ++++++
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 5ff01d2ac85e..32fa8e688c00 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -697,7 +697,7 @@ void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
return;
/* FIXME: merge g_pat from vmcb01 and vmcb12. */
- svm->nested.vmcb02.ptr->save.g_pat = svm->vmcb01.ptr->save.g_pat;
+ vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
}
static bool nested_vmcb12_has_lbrv(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d2ca226871c2..af808e83173e 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2979,10 +2979,9 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
if (ret)
break;
- svm->vmcb01.ptr->save.g_pat = data;
+ vmcb_set_gpat(svm->vmcb01.ptr, data);
if (is_guest_mode(vcpu))
nested_vmcb02_compute_g_pat(svm);
- vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
break;
case MSR_IA32_SPEC_CTRL:
if (!msr->host_initiated &&
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 74014110b550..a1d62c3cc3d6 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -439,6 +439,12 @@ static inline bool vmcb12_is_dirty(struct vmcb_ctrl_area_cached *control, int bi
return !test_bit(bit, (unsigned long *)&control->clean);
}
+static inline void vmcb_set_gpat(struct vmcb *vmcb, u64 data)
+{
+ vmcb->save.g_pat = data;
+ vmcb_mark_dirty(vmcb, VMCB_NPT);
+}
+
static __always_inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
{
return container_of(vcpu, struct vcpu_svm, vcpu);
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 1/9] KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260327234023.2659476-1-jmattson@google.com>
Define a quirk to control whether nested SVM shares L1's PAT with L2
(legacy behavior) or gives L2 its own independent gPAT (correct behavior
per the APM).
When the quirk is enabled (default), L2 shares L1's PAT, preserving the
legacy KVM behavior. When userspace disables the quirk, KVM correctly
virtualizes the PAT for nested SVM guests, giving L2 a separate gPAT as
specified in the AMD architecture.
Signed-off-by: Jim Mattson <jmattson@google.com>
---
Documentation/virt/kvm/api.rst | 14 ++++++++++++++
arch/x86/include/asm/kvm_host.h | 3 ++-
arch/x86/include/uapi/asm/kvm.h | 1 +
arch/x86/kvm/svm/svm.h | 11 +++++++++++
4 files changed, 28 insertions(+), 1 deletion(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 032516783e96..2d56f17e3760 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8551,6 +8551,20 @@ KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM By default, KVM relaxes the consisten
bit to be cleared. Note that the vmcs02
bit is still completely controlled by the
host, regardless of the quirk setting.
+
+KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT By default, KVM for nested SVM guests
+ shares the IA32_PAT MSR between L1 and
+ L2. This is legacy behavior and does
+ not match the AMD architecture
+ specification. When this quirk is
+ disabled and nested paging (NPT) is
+ enabled for L2, KVM correctly
+ virtualizes a separate guest PAT
+ register for L2, using the g_pat
+ field in the VMCB. When NPT is
+ disabled for L2, L1 and L2 continue
+ to share the IA32_PAT MSR regardless
+ of the quirk setting.
======================================== ================================================
7.32 KVM_CAP_MAX_VCPU_ID
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d3bdc9828133..0b4ab141feae 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2511,7 +2511,8 @@ int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages);
KVM_X86_QUIRK_SLOT_ZAP_ALL | \
KVM_X86_QUIRK_STUFF_FEATURE_MSRS | \
KVM_X86_QUIRK_IGNORE_GUEST_PAT | \
- KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM)
+ KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM | \
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT)
#define KVM_X86_CONDITIONAL_QUIRKS \
(KVM_X86_QUIRK_CD_NW_CLEARED | \
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 5f2b30d0405c..3ada2fa9ca86 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -477,6 +477,7 @@ struct kvm_sync_regs {
#define KVM_X86_QUIRK_STUFF_FEATURE_MSRS (1 << 8)
#define KVM_X86_QUIRK_IGNORE_GUEST_PAT (1 << 9)
#define KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM (1 << 10)
+#define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT (1 << 11)
#define KVM_STATE_NESTED_FORMAT_VMX 0
#define KVM_STATE_NESTED_FORMAT_SVM 1
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index ff1e4b4dc998..74014110b550 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -616,6 +616,17 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP;
}
+static inline bool l2_has_separate_pat(struct vcpu_svm *svm)
+{
+ /*
+ * If KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled while a vCPU
+ * is running, the L2 IA32_PAT semantics for that vCPU are undefined.
+ */
+ return nested_npt_enabled(svm) &&
+ !kvm_check_has_quirk(svm->vcpu.kvm,
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+}
+
static inline bool nested_vnmi_enabled(struct vcpu_svm *svm)
{
return guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_VNMI) &&
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply related
* [PATCH v7 0/9] KVM: x86: nSVM: Improve PAT virtualization
From: Jim Mattson @ 2026-03-27 23:40 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, linux-kselftest,
Yosry Ahmed
Cc: Jim Mattson
Currently, KVM's implementation of nested SVM treats the PAT MSR the same
way whether or not nested NPT is enabled: L1 and L2 share a single
PAT. However, the AMD APM specifies that when nested NPT is enabled, the host
(L1) and the guest (L2) should have independent PATs: hPAT for L1 and gPAT
for L2.
This patch series implements independent PATs for L1 and L2 when nested NPT
is enabled, but only when a new quirk, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT,
is disabled. By default, the quirk is enabled, preserving KVM's legacy
behavior. When the quirk is disabled, KVM correctly virtualizes a separate
PAT register for L2, using the g_pat field in the VMCB.
Guest accesses to the IA32_PAT MSR are redirected to either hPAT or gPAT
depending on the current mode and whether nested NPT is enabled. All other
accesses, including userspace accesses via KVM_{GET,SET}_MSRS, continue to
reference hPAT. L2's gPAT is saved and restored via a new 'gpat' field in
kvm_svm_nested_state_hdr, which is within the existing padding of the header
to maintain ABI compatibility.
v1: https://lore.kernel.org/kvm/20260113003016.3511895-1-jmattson@google.com/
v2: https://lore.kernel.org/kvm/20260115232154.3021475-1-jmattson@google.com/
v3: https://lore.kernel.org/kvm/20260205214326.1029278-1-jmattson@google.com/
v4: https://lore.kernel.org/kvm/20260212155905.3448571-1-jmattson@google.com/
v5: https://lore.kernel.org/kvm/20260224005500.1471972-1-jmattson@google.com/
v6: https://lore.kernel.org/kvm/20260326174944.3820245-1-jmattson@google.com/
v6 -> v7:
* Drop the patch "KVM: x86: Remove common handling of MSR_IA32_CR_PAT,"
because TDX still calls the common handler.
* Instead, add a warning to the common PAT handling code if it is
called by an SVM-capable vCPU
* Add comments about userspace disabling the quirk while a vCPU is
running.
* Cache the value of use_separate_l2_pat in KVM_SET_NESTED_STATE, in
case the quirk is disabled concurrently with the execution of that
function.
Jim Mattson (9):
KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT
KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest
mode
KVM: x86: nSVM: Cache and validate vmcb12 g_pat
KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT
KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT
KVM: Documentation: document KVM_{GET,SET}_NESTED_STATE for SVM
KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
KVM: selftests: nSVM: Add svm_nested_pat test
Documentation/virt/kvm/api.rst | 26 ++
arch/x86/include/asm/kvm_host.h | 3 +-
arch/x86/include/uapi/asm/kvm.h | 2 +
arch/x86/kvm/svm/nested.c | 65 +++-
arch/x86/kvm/svm/svm.c | 54 +++-
arch/x86/kvm/svm/svm.h | 19 +-
arch/x86/kvm/x86.c | 6 +
tools/arch/x86/include/uapi/asm/kvm.h | 2 +
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../selftests/kvm/x86/svm_nested_pat_test.c | 304 ++++++++++++++++++
10 files changed, 457 insertions(+), 25 deletions(-)
create mode 100644 tools/testing/selftests/kvm/x86/svm_nested_pat_test.c
base-commit: 3d6cdcc8883b5726513d245eef0e91cabfc397f7
--
2.53.0.1018.g2bb0e51243-goog
^ permalink raw reply
* Re: [PATCH v3 07/24] vfio/pci: Preserve vfio-pci device files across Live Update
From: Samiullah Khawaja @ 2026-03-27 23:39 UTC (permalink / raw)
To: David Matlack
Cc: Alex Williamson, Bjorn Helgaas, Adithya Jayachandran,
Alexander Graf, Alex Mastro, Andrew Morton, Ankit Agrawal,
Arnd Bergmann, Askar Safin, Borislav Petkov (AMD), Chris Li,
Dapeng Mi, David Rientjes, Feng Tang, Jacob Pan, Jason Gunthorpe,
Jason Gunthorpe, Jonathan Corbet, Josh Hilke, Kees Cook,
Kevin Tian, kexec, kvm, Leon Romanovsky, Leon Romanovsky,
linux-doc, linux-kernel, linux-kselftest, linux-mm, linux-pci,
Li RongQing, Lukas Wunner, Marco Elver, Michał Winiarski,
Mike Rapoport, Parav Pandit, Pasha Tatashin, Paul E. McKenney,
Pawan Gupta, Peter Zijlstra (Intel), Pranjal Shrivastava,
Pratyush Yadav, Raghavendra Rao Ananta, Randy Dunlap,
Rodrigo Vivi, Saeed Mahameed, Shuah Khan, Vipin Sharma,
Vivek Kasireddy, William Tu, Yi Liu, Zhu Yanjun
In-Reply-To: <20260323235817.1960573-8-dmatlack@google.com>
On Mon, Mar 23, 2026 at 11:57:59PM +0000, David Matlack wrote:
>From: Vipin Sharma <vipinsh@google.com>
>
>Implement the live update file handler callbacks to preserve a vfio-pci
>device across a Live Update. Subsequent commits will enable userspace to
>then retrieve this file after the Live Update.
>
>Live Update support is scoped only to cdev files (i.e. not
>VFIO_GROUP_GET_DEVICE_FD files).
>
>State about each device is serialized into a new ABI struct
>vfio_pci_core_device_ser. The contents of this struct are preserved
>across the Live Update to the next kernel using a combination of
>Kexec-Handover (KHO) to preserve the page(s) holding the struct and the
>Live Update Orchestrator (LUO) to preserve the physical address of the
>struct.
>
>For now the only contents of struct vfio_pci_core_device_ser the
>device's PCI segment number and BDF, so that the device can be uniquely
>identified after the Live Update.
>
>Require that userspace disables interrupts on the device prior to
>freeze() so that the device does not send any interrupts until new
>interrupt handlers have been set up by the next kernel.
>
>Reset the device and restore its state in the freeze() callback. This
>ensures the device can be received by the next kernel in a consistent
>state. Eventually this will be dropped and the device can be preserved
>across in a running state, but that requires further work in VFIO and
>the core PCI layer.
>
>Note that LUO holds a reference to this file when it is preserved. So
>VFIO is guaranteed that vfio_df_device_last_close() will not be called
>on this device no matter what userspace does.
LUO session holding the reference is also in userspace. Just to be
accurate, maybe say something like that the LUO will hold a reference
until the FD is preserved and vfio_df_device_last_close() will not be
called until file is unpreserved.
>
>Signed-off-by: Vipin Sharma <vipinsh@google.com>
>Co-developed-by: David Matlack <dmatlack@google.com>
>Signed-off-by: David Matlack <dmatlack@google.com>
>---
> drivers/vfio/pci/vfio_pci.c | 2 +-
> drivers/vfio/pci/vfio_pci_core.c | 57 +++++----
> drivers/vfio/pci/vfio_pci_liveupdate.c | 156 ++++++++++++++++++++++++-
> drivers/vfio/pci/vfio_pci_priv.h | 4 +
> drivers/vfio/vfio_main.c | 3 +-
> include/linux/kho/abi/vfio_pci.h | 15 +++
> include/linux/vfio.h | 2 +
> 7 files changed, 213 insertions(+), 26 deletions(-)
>
>diff --git a/drivers/vfio/pci/vfio_pci.c b/drivers/vfio/pci/vfio_pci.c
>index 41dcbe4ace67..351480d13f6e 100644
>--- a/drivers/vfio/pci/vfio_pci.c
>+++ b/drivers/vfio/pci/vfio_pci.c
>@@ -125,7 +125,7 @@ static int vfio_pci_open_device(struct vfio_device *core_vdev)
> return 0;
> }
>
>-static const struct vfio_device_ops vfio_pci_ops = {
>+const struct vfio_device_ops vfio_pci_ops = {
> .name = "vfio-pci",
> .init = vfio_pci_core_init_dev,
> .release = vfio_pci_core_release_dev,
>diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>index d43745fe4c84..81f941323641 100644
>--- a/drivers/vfio/pci/vfio_pci_core.c
>+++ b/drivers/vfio/pci/vfio_pci_core.c
>@@ -585,9 +585,42 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
> }
> EXPORT_SYMBOL_GPL(vfio_pci_core_enable);
>
>+void vfio_pci_core_try_reset(struct vfio_pci_core_device *vdev)
>+{
>+ struct pci_dev *pdev = vdev->pdev;
>+ struct pci_dev *bridge = pci_upstream_bridge(pdev);
>+
>+ lockdep_assert_held(&vdev->vdev.dev_set->lock);
>+
>+ if (!vdev->reset_works)
>+ return;
>+
>+ /*
>+ * Try to get the locks ourselves to prevent a deadlock. The
>+ * success of this is dependent on being able to lock the device,
>+ * which is not always possible.
>+ *
>+ * We cannot use the "try" reset interface here, since that will
>+ * overwrite the previously restored configuration information.
>+ */
>+ if (bridge && !pci_dev_trylock(bridge))
>+ return;
>+
>+ if (!pci_dev_trylock(pdev))
>+ goto out;
>+
>+ if (!__pci_reset_function_locked(pdev))
>+ vdev->needs_reset = false;
>+
>+ pci_dev_unlock(pdev);
>+out:
>+ if (bridge)
>+ pci_dev_unlock(bridge);
>+}
>+EXPORT_SYMBOL_GPL(vfio_pci_core_try_reset);
>+
> void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> {
>- struct pci_dev *bridge;
> struct pci_dev *pdev = vdev->pdev;
> struct vfio_pci_dummy_resource *dummy_res, *tmp;
> struct vfio_pci_ioeventfd *ioeventfd, *ioeventfd_tmp;
>@@ -687,27 +720,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
> */
> pci_write_config_word(pdev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
>
>- /*
>- * Try to get the locks ourselves to prevent a deadlock. The
>- * success of this is dependent on being able to lock the device,
>- * which is not always possible.
>- * We can not use the "try" reset interface here, which will
>- * overwrite the previously restored configuration information.
>- */
>- if (vdev->reset_works) {
>- bridge = pci_upstream_bridge(pdev);
>- if (bridge && !pci_dev_trylock(bridge))
>- goto out_restore_state;
>- if (pci_dev_trylock(pdev)) {
>- if (!__pci_reset_function_locked(pdev))
>- vdev->needs_reset = false;
>- pci_dev_unlock(pdev);
>- }
>- if (bridge)
>- pci_dev_unlock(bridge);
>- }
>-
>-out_restore_state:
>+ vfio_pci_core_try_reset(vdev);
> pci_restore_state(pdev);
> out:
> pci_disable_device(pdev);
>diff --git a/drivers/vfio/pci/vfio_pci_liveupdate.c b/drivers/vfio/pci/vfio_pci_liveupdate.c
>index 5ea5af46b159..c4ebc7c486e5 100644
>--- a/drivers/vfio/pci/vfio_pci_liveupdate.c
>+++ b/drivers/vfio/pci/vfio_pci_liveupdate.c
>@@ -6,27 +6,178 @@
> * David Matlack <dmatlack@google.com>
> */
>
>+/**
>+ * DOC: VFIO PCI Preservation via LUO
>+ *
>+ * VFIO PCI devices can be preserved over a kexec using the Live Update
>+ * Orchestrator (LUO) file preservation. This allows userspace (such as a VMM)
>+ * to transfer an in-use device to the next kernel.
>+ *
>+ * .. note::
>+ * The support for preserving VFIO PCI devices is currently *partial* and
>+ * should be considered *experimental*. It should only be used by developers
>+ * working on expanding the support for the time being.
>+ *
>+ * To avoid accidental usage while the support is still experimental, this
>+ * support is hidden behind a default-disable config option
>+ * ``CONFIG_VFIO_PCI_LIVEUPDATE``. Once the kernel support has stabilized and
>+ * become complete, this option will be enabled by default when
>+ * ``CONFIG_VFIO_PCI`` and ``CONFIG_LIVEUPDATE`` are enabled.
>+ *
>+ * Usage Example
>+ * =============
>+ *
>+ * VFIO PCI devices can be preserved across a kexec by preserving the file
>+ * associated with the device in a LUO session::
>+ *
>+ * device_fd = open("/dev/vfio/devices/X");
>+ * ...
>+ * ioctl(session_fd, LIVEUPDATE_SESSION_PRESERVE_FD, { ..., device_fd, ...});
>+ *
>+ * .. note::
>+ * LUO will hold an extra reference to the device file for as long as it is
>+ * preserved, so there is no way for the file to be destroyed or the device
>+ * to be unbound from the vfio-pci driver while it is preserved.
>+ *
>+ * Retrieving the file after kexec is not yet supported.
>+ *
>+ * Restrictions
>+ * ============
>+ *
>+ * The kernel imposes the following restrictions when preserving VFIO devices:
>+ *
>+ * * The device must be bound to the ``vfio-pci`` driver.
>+ *
>+ * * ``CONFIG_VFIO_PCI_ZDEV_KVM`` must not be enabled. This may be relaxed in
>+ * the future.
>+ *
>+ * * The device not be an Intel display device. This may be relaxed in the
>+ * future.
Seems there is a typo. The device "may"/"must" not be?
>+ *
>+ * * The device file must have been acquired from the VFIO character device,
>+ * not ``VFIO_GROUP_GET_DEVICE_FD``.
>+ *
>+ * * The device must have interrupt disable prior to kexec. Failure to disable
>+ * interrupts on the device will cause the ``reboot(LINUX_REBOOT_CMD_KEXEC)``
>+ * syscall (to initiate the kexec) to fail.
>+ *
>+ * Preservation Behavior
>+ * =====================
>+ *
>+ * The eventual goal of this support is to avoid disrupting the workload, state,
>+ * or configuration of each preserved device during a Live Update. This would
>+ * include allowing the device to perform DMA to preserved memory buffers and
>+ * perform P2P DMA to other preserved devices. However, there are many pieces
>+ * that still need to land in the kernel.
>+ *
>+ * For now, VFIO only preserves the following state for for devices:
>+ *
>+ * * The PCI Segment, Bus, Device, and Function numbers of the device. The
>+ * kernel guarantees the these will not change across a kexec when a device
>+ * is preserved.
>+ *
>+ * Since the kernel is not yet prepared to preserve all parts of the device and
>+ * its dependencies (such as DMA mappings), VFIO currently resets and restores
>+ * preserved devices back into an idle state during kexec, before handing off
>+ * control to the next kernel. This will be relaxed in future versions of the
>+ * kernel once it is safe to allow the device to keep running across kexec.
>+ */
>+
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
>+#include <linux/kexec_handover.h>
> #include <linux/kho/abi/vfio_pci.h>
> #include <linux/liveupdate.h>
> #include <linux/errno.h>
>+#include <linux/vfio.h>
>
> #include "vfio_pci_priv.h"
>
> static bool vfio_pci_liveupdate_can_preserve(struct liveupdate_file_handler *handler,
> struct file *file)
> {
>- return false;
>+ struct vfio_device *device = vfio_device_from_file(file);
>+ struct vfio_pci_core_device *vdev;
>+ struct pci_dev *pdev;
>+
>+ if (!device)
>+ return false;
>+
>+ /* Live Update support is limited to cdev files. */
>+ if (!vfio_device_cdev_opened(device))
>+ return false;
>+
>+ if (device->ops != &vfio_pci_ops)
>+ return false;
>+
>+ vdev = container_of(device, struct vfio_pci_core_device, vdev);
>+ pdev = vdev->pdev;
>+
>+ /*
>+ * Don't support specialized vfio-pci devices for now since they haven't
>+ * been tested.
>+ */
>+ if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM) || vfio_pci_is_intel_display(pdev))
>+ return false;
>+
>+ return true;
> }
>
> static int vfio_pci_liveupdate_preserve(struct liveupdate_file_op_args *args)
> {
>- return -EOPNOTSUPP;
>+ struct vfio_device *device = vfio_device_from_file(args->file);
>+ struct vfio_pci_core_device_ser *ser;
>+ struct vfio_pci_core_device *vdev;
>+ struct pci_dev *pdev;
>+
>+ vdev = container_of(device, struct vfio_pci_core_device, vdev);
>+ pdev = vdev->pdev;
>+
>+ ser = kho_alloc_preserve(sizeof(*ser));
>+ if (IS_ERR(ser))
>+ return PTR_ERR(ser);
>+
>+ ser->bdf = pci_dev_id(pdev);
>+ ser->domain = pci_domain_nr(pdev->bus);
>+
>+ args->serialized_data = virt_to_phys(ser);
>+ return 0;
> }
>
> static void vfio_pci_liveupdate_unpreserve(struct liveupdate_file_op_args *args)
> {
>+ kho_unpreserve_free(phys_to_virt(args->serialized_data));
>+}
>+
>+static int vfio_pci_liveupdate_freeze(struct liveupdate_file_op_args *args)
>+{
>+ struct vfio_device *device = vfio_device_from_file(args->file);
>+ struct vfio_pci_core_device *vdev;
>+ struct pci_dev *pdev;
>+ int ret;
>+
>+ vdev = container_of(device, struct vfio_pci_core_device, vdev);
>+ pdev = vdev->pdev;
>+
>+ guard(mutex)(&device->dev_set->lock);
>+
>+ /*
>+ * Userspace must disable interrupts on the device prior to freeze so
>+ * that the device does not send any interrupts until new interrupt
>+ * handlers have been established by the next kernel.
>+ */
>+ if (vdev->irq_type != VFIO_PCI_NUM_IRQS) {
>+ pci_err(pdev, "Freeze failed! Interrupts are still enabled.\n");
>+ return -EINVAL;
>+ }
>+
>+ ret = pci_load_saved_state(pdev, vdev->pci_saved_state);
>+ if (ret)
>+ return ret;
>+
>+ vfio_pci_core_try_reset(vdev);
nit: I see you have already added a comment earlier saying that device will
be reset to a clean state as all the of the dependencies to preserve the
state for continous DMA are not there. Maybe we can add a comment here
to specify that this will go away once we have full preservation
support.
>+ pci_restore_state(pdev);
>+ return 0;
> }
>
> static int vfio_pci_liveupdate_retrieve(struct liveupdate_file_op_args *args)
>@@ -42,6 +193,7 @@ static const struct liveupdate_file_ops vfio_pci_liveupdate_file_ops = {
> .can_preserve = vfio_pci_liveupdate_can_preserve,
> .preserve = vfio_pci_liveupdate_preserve,
> .unpreserve = vfio_pci_liveupdate_unpreserve,
>+ .freeze = vfio_pci_liveupdate_freeze,
> .retrieve = vfio_pci_liveupdate_retrieve,
> .finish = vfio_pci_liveupdate_finish,
> .owner = THIS_MODULE,
>diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
>index cbf46e09da30..fa5c7f544f8a 100644
>--- a/drivers/vfio/pci/vfio_pci_priv.h
>+++ b/drivers/vfio/pci/vfio_pci_priv.h
>@@ -11,6 +11,10 @@
> /* Cap maximum number of ioeventfds per device (arbitrary) */
> #define VFIO_PCI_IOEVENTFD_MAX 1000
>
>+extern const struct vfio_device_ops vfio_pci_ops;
>+
>+void vfio_pci_core_try_reset(struct vfio_pci_core_device *vdev);
>+
> struct vfio_pci_ioeventfd {
> struct list_head next;
> struct vfio_pci_core_device *vdev;
>diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
>index 742477546b15..8b222f71bbab 100644
>--- a/drivers/vfio/vfio_main.c
>+++ b/drivers/vfio/vfio_main.c
>@@ -1436,7 +1436,7 @@ const struct file_operations vfio_device_fops = {
> #endif
> };
>
>-static struct vfio_device *vfio_device_from_file(struct file *file)
>+struct vfio_device *vfio_device_from_file(struct file *file)
> {
> struct vfio_device_file *df = file->private_data;
>
>@@ -1444,6 +1444,7 @@ static struct vfio_device *vfio_device_from_file(struct file *file)
> return NULL;
> return df->device;
> }
>+EXPORT_SYMBOL_GPL(vfio_device_from_file);
>
> /**
> * vfio_file_is_valid - True if the file is valid vfio file
>diff --git a/include/linux/kho/abi/vfio_pci.h b/include/linux/kho/abi/vfio_pci.h
>index e2412b455e61..876aaf81dd92 100644
>--- a/include/linux/kho/abi/vfio_pci.h
>+++ b/include/linux/kho/abi/vfio_pci.h
>@@ -9,6 +9,9 @@
> #ifndef _LINUX_LIVEUPDATE_ABI_VFIO_PCI_H
> #define _LINUX_LIVEUPDATE_ABI_VFIO_PCI_H
>
>+#include <linux/compiler.h>
>+#include <linux/types.h>
>+
> /**
> * DOC: VFIO PCI Live Update ABI
> *
>@@ -25,4 +28,16 @@
>
> #define VFIO_PCI_LUO_FH_COMPATIBLE "vfio-pci-v1"
>
>+/**
>+ * struct vfio_pci_core_device_ser - Serialized state of a single VFIO PCI
>+ * device.
>+ *
>+ * @domain: The device's PCI domain number (segment).
>+ * @bdf: The device's PCI bus, device, and function number.
>+ */
>+struct vfio_pci_core_device_ser {
>+ u32 domain;
>+ u16 bdf;
>+} __packed;
>+
> #endif /* _LINUX_LIVEUPDATE_ABI_VFIO_PCI_H */
>diff --git a/include/linux/vfio.h b/include/linux/vfio.h
>index e90859956514..e9d3ddb715c5 100644
>--- a/include/linux/vfio.h
>+++ b/include/linux/vfio.h
>@@ -81,6 +81,8 @@ struct vfio_device {
> #endif
> };
>
>+struct vfio_device *vfio_device_from_file(struct file *file);
>+
> /**
> * struct vfio_device_ops - VFIO bus driver device callbacks
> *
>--
>2.53.0.983.g0bb29b3bc5-goog
>
^ permalink raw reply
* Re: [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Reinette Chatre @ 2026-03-27 22:11 UTC (permalink / raw)
To: Babu Moger, corbet, tony.luck, Dave.Martin, james.morse, tglx,
mingo, bp, dave.hansen
Cc: skhan, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <47c0db32-d0e0-4c53-90bd-b74863d233dc@amd.com>
Hi Babu,
On 3/26/26 10:12 AM, Babu Moger wrote:
> Hi Reinette,
>
> Thanks for the review comments. Will address one by one.
>
> On 3/24/26 17:51, Reinette Chatre wrote:
>> Hi Babu,
>>
>> On 3/12/26 1:36 PM, Babu Moger wrote:
>>> This series adds support for Privilege-Level Zero Association (PLZA) to the
>>> resctrl subsystem. PLZA is an AMD feature that allows specifying a CLOSID
>>> and/or RMID for execution in kernel mode (privilege level zero), so that
>>> kernel work is not subject to the same resource constrains as the current
>>> user-space task. This avoids kernel operations being aggressively throttled
>>> when a task's memory bandwidth is heavily limited.
>>>
>>> The feature documentation is not yet publicly available, but it is expected
>>> to be released in the next few weeks. In the meantime, a brief description
>>> of the features is provided below.
>>>
>>> Privilege Level Zero Association (PLZA)
>>>
>>> Privilege Level Zero Association (PLZA) allows the hardware to
>>> automatically associate execution in Privilege Level Zero (CPL=0) with a
>>> specific COS (Class of Service) and/or RMID (Resource Monitoring
>>> Identifier). The QoS feature set already has a mechanism to associate
>>> execution on each logical processor with an RMID or COS. PLZA allows the
>>> system to override this per-thread association for a thread that is
>>> executing with CPL=0.
>>> ------------------------------------------------------------------------
>>>
>>> The series introduces the feature in a way that supports the interface in
>>> a generic manner to accomodate MPAM or other vendor specific implimentation.
>>>
>>> Below is the detailed requirements provided by Reinette:
>>> https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
>> Our discussion considered how resctrl could support PLZA in a generic way while
>> also preparing to support MPAM's variants and how PLZA may evolve to have similar
>> capabilities when considering the capabilities of its registers.
>>
>> This does not mean that your work needs to implement everything that was discussed.
>> Instead, this work is expected to just support what PLZA is capable of today but
>> do so in a way that the future enhancements could be added to.
>>
>> This series is quite difficult to follow since it appears to implement a full
>> featured generic interface while PLZA cannot take advantage of it.
>>
>> Could you please simplify this work to focus on just enabling PLZA and only
>> add interfaces needed to do so?
> Sure. Will try. Lets continue the discussion.
>>
>>> Summary:
>>> 1. Kernel-mode/PLZA controls and status should be exposed under the resctrl
>>> info directory:/sys/fs/resctrl/info/, not as a separate or arch-specific path.
>>>
>>> 2. Add two info files
>>>
>>> a. kernel_mode
>>> Purpose: Control how resource allocation and monitoring apply in kernel mode
>>> (e.g. inherit from task vs global assign).
>>>
>>> Read: List supported modes and show current one (e.g. with [brackets]).
>>> Write: Set current mode by name (e.g. inherit_ctrl_and_mon, global_assign_ctrl_assign_mon).
>>>
>>> b. kernel_mode_assignment
>>>
>>> Purpose: When a “global assign” kernel mode is active, specify which resctrl group
>>> (CLOSID/RMID) is used for kernel work.
>>>
>>> Read: Show the assigned group in a path-like form (e.g. //, ctrl1//, ctrl1/mon1/).
>>> Write: Assign or clear the group used for kernel mode (and optionally clear with an empty write).
>>>
>>> The patches are based on top of commit (v7.0.0-rc3)
>>> 839e91ce3f41b (tip/master) Merge branch into tip/master: 'x86/tdx'
>>> ------------------------------------------------------------------------
>>>
>>> Examples: kernel_mode and kernel_mode_assignment
>>>
>>> All paths below are under /sys/fs/resctrl/ (e.g. info/kernel_mode means
>>> /sys/fs/resctrl/info/kernel_mode). Resctrl must be mounted and the platform
>>> must support the relevant modes (e.g. AMD with PLZA).
>>>
>>> 1) kernel_mode — show and set the current kernel mode
>>>
>>> Read supported modes and which one is active (current in brackets):
>>>
>>> $ cat info/kernel_mode
>>> [inherit_ctrl_and_mon]
>>> global_assign_ctrl_inherit_mon
>>> global_assign_ctrl_assign_mon
>>>
>>> Set the active mode (e.g. use one CLOSID+RMID for all kernel work):
>>>
>>> $ echo "global_assign_ctrl_assign_mon" > info/kernel_mode
>>> $ cat info/kernel_mode
>>> inherit_ctrl_and_mon
>>> global_assign_ctrl_inherit_mon
>>> [global_assign_ctrl_assign_mon]
>>>
>>> Mode meanings:
>>> - inherit_ctrl_and_mon: kernel uses same CLOSID/RMID as the current task (default).
>>> - global_assign_ctrl_inherit_mon: one CLOSID for all kernel work; RMID inherited from user.
>>> - global_assign_ctrl_assign_mon: one resource group (CLOSID+RMID) for all kernel work.
>>>
>>> 2) kernel_mode_assignment — show and set which group is used for kernel work
>>>
>>> Only relevant when kernel_mode is not "inherit_ctrl_and_mon". Read the
>> To help with future usages please connect visibility of this file with the mode in
>> info/kernel_mode. This helps us to support future modes with other resctrl files, possible
>> within each resource group.
>> Specifically, kernel_mode_assignment is not visible to user space if mode is "inherit_ctrl_and_mon",
>> while it is visible when mode is global_assign_ctrl_inherit_mon or global_assign_ctrl_assign_mon.
>
> Sure. Will do.
>
>>
>>> currently assigned group (path format is "CTRL_MON/MON/"):
>> The format depends on the mode, right? If the mode is "global_assign_ctrl_inherit_mon"
>> then it should only contain a control group, alternatively, if the mode is
>> "global_assign_ctrl_assign_mon" then it contains control and mon group. This gives
>> resctrl future flexibility to change format for future modes.
>
> This can be done both ways. Whole purpose of these groups is to get CLOSID and RMID to enable PLZA. User can echo CTRL_MON or MON group to kernel_mode_assignment in any of the modes. We can decide what needs to be updated in MSR (PQR_PLZA_ASSOC) based on what kernel mode is selected.
The "both ways" are specific to one of the two active modes though.
PLZA only needs the RMID when the mode is "global_assign_ctrl_assign_mon".
Displaying and parsing monitor group when the mode is
"global_assign_ctrl_inherit_mon" creates an inconsistent interface since the mode
only uses a control group. The interface to user space should match the mode otherwise
it becomes confusing.
...
>>>
>>> Tony suggested using global variables to store the kernel mode
>>> CLOSID and RMID. However, the kernel mode CLOSID and RMID are
>>> coming from rdtgroup structure with the new interface. Accessing
>>> them requires holding the associated lock, which would make the
>>> context switch path unnecessarily expensive. So, dropped the idea.
>>> https://lore.kernel.org/lkml/aXuxVSbk1GR2ttzF@agluck-desk3/
>>> Let me know if there are other ways to optimize this.
>> I do not see why the context switch path needs to be touched at all with this
>> implementation. Since PLZA only supports global assignment does it not mean that resctrl
>> only needs to update PQR_PLZA_ASSOC when user writes to info/kernel_mode and
>> info/kernel_mode_assignment?
>
> Each thread has an MSR to configure whether to associate privilege level zero execution with a separate COS and/or RMID, and the value of the COS and/or RMID. PLZA may be enabled or disabled on a per-thread basis. However, the COS and RMID association and configuration must be the same for all threads in the QOS Domain.
Based on previous comment in https://lore.kernel.org/lkml/abb049fa-3a3d-4601-9ae3-61eeb7fd8fcf@amd.com/
and this implementation all fields of PQR_PLZA_ASSOC except PQR_PLZA_ASSOC.plza_en must be the
same for all CPUs on the system, not just per QoS domain. Could you please confirm?
>
> So, PQR_PLZA_ASSOC is a per thread MSR just like PQR_ASSOC.
>
> Privilege-Level Zero Association (PLZA) allows the user to specify a COS and/or RMID associated with execution in Privilege-Level Zero. When enabled on a HW thread, when that thread enters Privilige-Level Zero, transactions associated with that thread will be associated with the PLZA COS and/or RMID. Otherwise, the HW thread will be associated with the COS and RMID identified by PQR_ASSOC.
>
> More below.
>
>>
>> Consider some of the scenarios:
>>
>> resctrl mount with default state:
>>
>> # cat info/kernel_mode
>> [inherit_ctrl_and_mon]
>> global_assign_ctrl_inherit_mon
>> global_assign_ctrl_assign_mon
>> # ls info/kernel_mode_assignment
>> ls: cannot access 'info/kernel_mode_assignment': No such file or directory
>>
>> enable global_assign_ctrl_assign_mon mode:
>> # echo "global_assign_ctrl_assign_mon" > info/kernel_mode
>>
>> Expectation here is that when user space sets this mode as above then resctrl would
>> in turn program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
>> MSR_IA32_PQR_PLZA_ASSOC.rmid=0
>> MSR_IA32_PQR_PLZA_ASSOC.rmid_en=1
>> MSR_IA32_PQR_PLZA_ASSOC.closid=0
>> MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
>> MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>>
>> I do not see why it is necessary to maintain any per-CPU or per-task state or needing
>> to touch the context switch code. Since PLZA only supports global could it not
>> just set MSR_IA32_PQR_PLZA_ASSOC on all online CPUs and be done with it?
>> Only caveat is that if a CPU is offline then this setting needs to be stashed
>> so that MSR_IA32_PQR_PLZA_ASSOC can be set when new CPU comes online.
>>
>> The way that rdtgroup_config_kmode() introduced in patch #11 assumes it is dealing
>> with RDT_RESOURCE_L3 and traverses the resource domain list and resource group
>> CPU mask seems unnecessary to me as well as error prone since the system may only
>> have, for example, RDT_RESOURCE_MBA enabled or even just monitoring. Why not just set
>> MSR_IA32_PQR_PLZA_ASSOC on all CPUs and be done?
>>
>> To continue the scenarios ...
>>
>> After user's setting above related files read:
>> # cat info/kernel_mode
>> inherit_ctrl_and_mon
>> global_assign_ctrl_inherit_mon
>> [global_assign_ctrl_assign_mon]
>> # cat info/kernel_mode_assignment
>> //
>>
>> Modify group used by global_assign_ctrl_assign_mon mode:
>> # echo 'ctrl1/mon1/' > info/kernel_mode_assignment
>>
>> Expectation here is that when user space sets this then resctrl would
>> program MSR_IA32_PQR_PLZA_ASSOC on all CPUs to be:
>> MSR_IA32_PQR_PLZA_ASSOC.rmid=<rmid of mon1>
>> MSR_IA32_PQR_PLZA_ASSOC.rmid_en=1
>> MSR_IA32_PQR_PLZA_ASSOC.closid=<closid of ctrl1>
>> MSR_IA32_PQR_PLZA_ASSOC.closid_en=1
>> MSR_IA32_PQR_PLZA_ASSOC.plza_en=1
>
>
> This works correctly when PLZA associations are defined by per CPU. For example, lets assume that *ctrl1* is assigned *CLOSID 1*.
>
> In this scenario, every task in the system running on a any CPU will use the limits associated with *CLOSID 1* whenever it enters Privilege-Level Zero, because the CPU's *PQR_PLZA_ASSOC* register has PLZA enabled and CLOSID is 1.
>
> Now consider task-based association:
>
> We have two resctrl groups:
>
> * *ctrl1 -> CLOSID 1 -> task1.plza = 1 : *User wants PLZA be enabled
> for this task.
> * *ctrl2 -> CLOSID 2 -> task2.plza = 0 : *User wants PLZA
> disabled for this task.
>
> Suppose *task1* is first scheduled on *CPU 0*. This behaves as expected: since CPU 0 's *PQR_PLZA_ASSOC* contains *CLOSID 1, plza_en =1*, task1 will use the limits from CLOSID 1 when it enters Privilege-Level Zero.
>
> However, if *task2* later runs on *CPU 0*, we expect it to use *CLOSID 2* in both user mode and kernel mode, because user has PLZA disabled for this task. But CPU 0 still has *CLOSID 1, **plza_en =1* in its PQR_PLZA_ASSOC register.
>
> As a result, task2 will incorrectly run with *CLOSID 1* when entering Privilege-Level Zero something we explicitly want to avoid.
>
> At that point, PLZA must be disabled on CPU 0 to prevent the unintended association. Hope this explanation makes the issue clear.
>
A couple of points:
- Looks like we still need to come to agreement what is meant by "global" when it
comes to kernel mode.
In your description there is a "global" configuration, but the assignment is "per-task".
To me this sounds like a new and distinct kernel_mode from the "global" modes
considered so far. This seems to move to the "per_task" mode mentioned in but
the implementation does not take into account any of the earlier discussions
surrounding it:
https://lore.kernel.org/lkml/2ab556af-095b-422b-9396-f845c6fd0342@intel.com/
We only learned about one use case in https://lore.kernel.org/lkml/CABPqkBSq=cgn-am4qorA_VN0vsbpbfDePSi7gubicpROB1=djw@mail.gmail.com/
As I understand this use case requires PLZA globally enabled for all tasks. Thus
I consider task assignment to be "global" when in the "global_*" kernel modes.
If this is indeed a common use case then supporting only global configuration
but then requiring user space to manually assign all tasks afterwards sounds
cumbersome for user space and also detrimental to system performance with all
the churn to modify all the task_structs involved. The accompanying documentation
does not mention all this additional user space interactions required by user
space to use this implementation.
I find this implementation difficult and inefficient to use in the one use case
we know of. I would suggest that resctrl optimizes for the one known use case.
- This implementation ignores discussion on how existing resctrl files should
not be repurposed.
This implementation allows user space to set a resource group in
kernel_mode_assignment with the consequence that this resource group's
"tasks" file changes behavior. I consider this a break of resctrl interface.
We did briefly consider per-task configuration/assignment in previous discussion
and the proposal was for it to use a new file (only when and if needed!).
- Now a user is required to write the task id of every task that participates
in PLZA. Apart from the churn already mentioned this also breaks existing
usage since it is no longer possible for new tasks to be added to this
resource group. This creates an awkward interface where all tasks belonging
to a resource group inherits the allocations/monitoring for their user space
work and will get PLZA enabled whether user requested it or not while
tasks from other resource groups need to be explicitly enabled. This creates
an inconsistency when it comes to task assignment. The only way to "remove"
PLZA from such a task would be to assign it to another resource group which
may not have the user space allocations ... and once this is done the task
cannot be moved back.
There is no requirement that CLOSID/RMID should be dedicated to kernel work
but this implementation does so in an inconsistent way.
- Apart from the same issues as with repurposing of tasks file, why should same
CPU allocation be used for kernel and user space?
Reinette
^ permalink raw reply
* [PATCH V10 8/8] dax: export dax_dev_get()
From: John Groves @ 2026-03-27 21:05 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves, Jonathan Cameron
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <john@groves.net>
famfs needs to look up a dax_device by dev_t when resolving fmap
entries that reference character dax devices.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
drivers/dax/super.c | 3 ++-
include/linux/dax.h | 1 +
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index d4ab60c406bf..25cf99dd9360 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -521,7 +521,7 @@ static int dax_set(struct inode *inode, void *data)
return 0;
}
-static struct dax_device *dax_dev_get(dev_t devt)
+struct dax_device *dax_dev_get(dev_t devt)
{
struct dax_device *dax_dev;
struct inode *inode;
@@ -544,6 +544,7 @@ static struct dax_device *dax_dev_get(dev_t devt)
return dax_dev;
}
+EXPORT_SYMBOL_GPL(dax_dev_get);
struct dax_device *alloc_dax(void *private, const struct dax_operations *ops)
{
diff --git a/include/linux/dax.h b/include/linux/dax.h
index a85e270bfb3c..9ef95b136bb8 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -54,6 +54,7 @@ struct dax_device *alloc_dax(void *private, const struct dax_operations *ops);
void *dax_holder(struct dax_device *dax_dev);
void put_dax(struct dax_device *dax_dev);
void kill_dax(struct dax_device *dax_dev);
+struct dax_device *dax_dev_get(dev_t devt);
void dax_write_cache(struct dax_device *dax_dev, bool wc);
bool dax_write_cache_enabled(struct dax_device *dax_dev);
bool dax_synchronous(struct dax_device *dax_dev);
--
2.53.0
^ permalink raw reply related
* [PATCH V10 7/8] dax: Add fs_dax_get() func to prepare dax for fs-dax usage
From: John Groves @ 2026-03-27 21:05 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves, Jonathan Cameron
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <john@groves.net>
The fs_dax_get() function should be called by fs-dax file systems after
opening a fsdev dax device. This adds holder_operations, which provides
a memory failure callback path and effects exclusivity between callers
of fs_dax_get().
fs_dax_get() is specific to fsdev_dax, so it checks the driver type
(which required touching bus.[ch]). fs_dax_get() fails if fsdev_dax is
not bound to the memory.
This function serves the same role as fs_dax_get_by_bdev(), which dax
file systems call after opening the pmem block device.
This can't be located in fsdev.c because struct dax_device is opaque
there.
This will be called by fs/fuse/famfs.c in a subsequent commit.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Signed-off-by: John Groves <john@groves.net>
---
drivers/dax/bus.c | 2 --
drivers/dax/bus.h | 2 ++
drivers/dax/super.c | 66 ++++++++++++++++++++++++++++++++++++++++++++-
include/linux/dax.h | 16 ++++++++---
4 files changed, 79 insertions(+), 7 deletions(-)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index 1b412264bb36..32f7b7702c28 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -39,8 +39,6 @@ static int dax_bus_uevent(const struct device *dev, struct kobj_uevent_env *env)
return add_uevent_var(env, "MODALIAS=" DAX_DEVICE_MODALIAS_FMT, 0);
}
-#define to_dax_drv(__drv) container_of_const(__drv, struct dax_device_driver, drv)
-
static struct dax_id *__dax_match_id(const struct dax_device_driver *dax_drv,
const char *dev_name)
{
diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
index 880bdf7e72d7..dc6f112ac4a4 100644
--- a/drivers/dax/bus.h
+++ b/drivers/dax/bus.h
@@ -42,6 +42,8 @@ struct dax_device_driver {
void (*remove)(struct dev_dax *dev);
};
+#define to_dax_drv(__drv) container_of_const(__drv, struct dax_device_driver, drv)
+
int __dax_driver_register(struct dax_device_driver *dax_drv,
struct module *module, const char *mod_name);
#define dax_driver_register(driver) \
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index ba0b4cd18a77..d4ab60c406bf 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -14,6 +14,7 @@
#include <linux/fs.h>
#include <linux/cacheinfo.h>
#include "dax-private.h"
+#include "bus.h"
/**
* struct dax_device - anchor object for dax services
@@ -111,6 +112,10 @@ struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev, u64 *start_off,
}
EXPORT_SYMBOL_GPL(fs_dax_get_by_bdev);
+#endif /* CONFIG_BLOCK && CONFIG_FS_DAX */
+
+#if IS_ENABLED(CONFIG_FS_DAX)
+
void fs_put_dax(struct dax_device *dax_dev, void *holder)
{
if (dax_dev && holder &&
@@ -119,7 +124,66 @@ void fs_put_dax(struct dax_device *dax_dev, void *holder)
put_dax(dax_dev);
}
EXPORT_SYMBOL_GPL(fs_put_dax);
-#endif /* CONFIG_BLOCK && CONFIG_FS_DAX */
+
+/**
+ * fs_dax_get() - get ownership of a devdax via holder/holder_ops
+ *
+ * fs-dax file systems call this function to prepare to use a devdax device for
+ * fsdax. This is like fs_dax_get_by_bdev(), but the caller already has struct
+ * dev_dax (and there is no bdev). The holder makes this exclusive.
+ *
+ * @dax_dev: dev to be prepared for fs-dax usage
+ * @holder: filesystem or mapped device inside the dax_device
+ * @hops: operations for the inner holder
+ *
+ * Returns: 0 on success, <0 on failure
+ */
+int fs_dax_get(struct dax_device *dax_dev, void *holder,
+ const struct dax_holder_operations *hops)
+{
+ struct dev_dax *dev_dax;
+ struct dax_device_driver *dax_drv;
+ int id;
+
+ id = dax_read_lock();
+ if (!dax_dev || !dax_alive(dax_dev) || !igrab(&dax_dev->inode)) {
+ dax_read_unlock(id);
+ return -ENODEV;
+ }
+ dax_read_unlock(id);
+
+ /* Verify the device is bound to fsdev_dax driver */
+ dev_dax = dax_get_private(dax_dev);
+ if (!dev_dax) {
+ iput(&dax_dev->inode);
+ return -ENODEV;
+ }
+
+ device_lock(&dev_dax->dev);
+ if (!dev_dax->dev.driver) {
+ device_unlock(&dev_dax->dev);
+ iput(&dax_dev->inode);
+ return -ENODEV;
+ }
+ dax_drv = to_dax_drv(dev_dax->dev.driver);
+ if (dax_drv->type != DAXDRV_FSDEV_TYPE) {
+ device_unlock(&dev_dax->dev);
+ iput(&dax_dev->inode);
+ return -EOPNOTSUPP;
+ }
+ device_unlock(&dev_dax->dev);
+
+ if (cmpxchg(&dax_dev->holder_data, NULL, holder)) {
+ iput(&dax_dev->inode);
+ return -EBUSY;
+ }
+
+ dax_dev->holder_ops = hops;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(fs_dax_get);
+#endif /* CONFIG_FS_DAX */
enum dax_device_flags {
/* !alive + rcu grace period == no new operations / mappings */
diff --git a/include/linux/dax.h b/include/linux/dax.h
index b19bfe0c2fd1..a85e270bfb3c 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -130,7 +130,6 @@ int dax_add_host(struct dax_device *dax_dev, struct gendisk *disk);
void dax_remove_host(struct gendisk *disk);
struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev, u64 *start_off,
void *holder, const struct dax_holder_operations *ops);
-void fs_put_dax(struct dax_device *dax_dev, void *holder);
#else
static inline int dax_add_host(struct dax_device *dax_dev, struct gendisk *disk)
{
@@ -145,12 +144,12 @@ static inline struct dax_device *fs_dax_get_by_bdev(struct block_device *bdev,
{
return NULL;
}
-static inline void fs_put_dax(struct dax_device *dax_dev, void *holder)
-{
-}
#endif /* CONFIG_BLOCK && CONFIG_FS_DAX */
#if IS_ENABLED(CONFIG_FS_DAX)
+void fs_put_dax(struct dax_device *dax_dev, void *holder);
+int fs_dax_get(struct dax_device *dax_dev, void *holder,
+ const struct dax_holder_operations *hops);
int dax_writeback_mapping_range(struct address_space *mapping,
struct dax_device *dax_dev, struct writeback_control *wbc);
int dax_folio_reset_order(struct folio *folio);
@@ -164,6 +163,15 @@ dax_entry_t dax_lock_mapping_entry(struct address_space *mapping,
void dax_unlock_mapping_entry(struct address_space *mapping,
unsigned long index, dax_entry_t cookie);
#else
+static inline void fs_put_dax(struct dax_device *dax_dev, void *holder)
+{
+}
+
+static inline int fs_dax_get(struct dax_device *dax_dev, void *holder,
+ const struct dax_holder_operations *hops)
+{
+ return -EOPNOTSUPP;
+}
static inline struct page *dax_layout_busy_page(struct address_space *mapping)
{
return NULL;
--
2.53.0
^ permalink raw reply related
* [PATCH V10 6/8] dax: Add dax_set_ops() for setting dax_operations at bind time
From: John Groves @ 2026-03-27 21:05 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Jonathan Cameron, John Groves
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <John@Groves.net>
Add a new dax_set_ops() function that allows drivers to set the
dax_operations after the dax_device has been allocated. This is needed
for fsdev_dax where the operations need to be set during probe and
cleared during unbind.
The fsdev driver uses devm_add_action_or_reset() for cleanup consistency,
avoiding the complexity of mixing devm-managed resources with manual
cleanup in a remove() callback. This ensures cleanup happens automatically
in the correct reverse order when the device is unbound.
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
drivers/dax/fsdev.c | 16 ++++++++++++++++
drivers/dax/super.c | 38 +++++++++++++++++++++++++++++++++++++-
include/linux/dax.h | 1 +
3 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
index 30f57c74c979..4499d9621f33 100644
--- a/drivers/dax/fsdev.c
+++ b/drivers/dax/fsdev.c
@@ -117,6 +117,13 @@ static void fsdev_kill(void *dev_dax)
kill_dev_dax(dev_dax);
}
+static void fsdev_clear_ops(void *data)
+{
+ struct dev_dax *dev_dax = data;
+
+ dax_set_ops(dev_dax->dax_dev, NULL);
+}
+
/*
* Page map operations for FS-DAX mode
* Similar to fsdax_pagemap_ops in drivers/nvdimm/pmem.c
@@ -303,6 +310,15 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
if (rc)
return rc;
+ /* Set the dax operations for fs-dax access path */
+ rc = dax_set_ops(dax_dev, &dev_dax_ops);
+ if (rc)
+ return rc;
+
+ rc = devm_add_action_or_reset(dev, fsdev_clear_ops, dev_dax);
+ if (rc)
+ return rc;
+
run_dax(dax_dev);
return devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
}
diff --git a/drivers/dax/super.c b/drivers/dax/super.c
index c00b9dff4a06..ba0b4cd18a77 100644
--- a/drivers/dax/super.c
+++ b/drivers/dax/super.c
@@ -157,6 +157,9 @@ long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
if (!dax_alive(dax_dev))
return -ENXIO;
+ if (!dax_dev->ops)
+ return -EOPNOTSUPP;
+
if (nr_pages < 0)
return -EINVAL;
@@ -207,6 +210,10 @@ int dax_zero_page_range(struct dax_device *dax_dev, pgoff_t pgoff,
if (!dax_alive(dax_dev))
return -ENXIO;
+
+ if (!dax_dev->ops)
+ return -EOPNOTSUPP;
+
/*
* There are no callers that want to zero more than one page as of now.
* Once users are there, this check can be removed after the
@@ -223,7 +230,7 @@ EXPORT_SYMBOL_GPL(dax_zero_page_range);
size_t dax_recovery_write(struct dax_device *dax_dev, pgoff_t pgoff,
void *addr, size_t bytes, struct iov_iter *iter)
{
- if (!dax_dev->ops->recovery_write)
+ if (!dax_dev->ops || !dax_dev->ops->recovery_write)
return 0;
return dax_dev->ops->recovery_write(dax_dev, pgoff, addr, bytes, iter);
}
@@ -307,6 +314,35 @@ void set_dax_nomc(struct dax_device *dax_dev)
}
EXPORT_SYMBOL_GPL(set_dax_nomc);
+/**
+ * dax_set_ops - set the dax_operations for a dax_device
+ * @dax_dev: the dax_device to configure
+ * @ops: the operations to set (may be NULL to clear)
+ *
+ * This allows drivers to set the dax_operations after the dax_device
+ * has been allocated. This is needed when the device is created before
+ * the driver that needs specific ops is bound (e.g., fsdev_dax binding
+ * to a dev_dax created by hmem).
+ *
+ * When setting non-NULL ops, fails if ops are already set (returns -EBUSY).
+ * When clearing ops (NULL), always succeeds.
+ *
+ * Return: 0 on success, -EBUSY if ops already set
+ */
+int dax_set_ops(struct dax_device *dax_dev, const struct dax_operations *ops)
+{
+ if (ops) {
+ /* Setting ops: fail if already set */
+ if (cmpxchg(&dax_dev->ops, NULL, ops) != NULL)
+ return -EBUSY;
+ } else {
+ /* Clearing ops: always allowed */
+ dax_dev->ops = NULL;
+ }
+ return 0;
+}
+EXPORT_SYMBOL_GPL(dax_set_ops);
+
bool dax_alive(struct dax_device *dax_dev)
{
lockdep_assert_held(&dax_srcu);
diff --git a/include/linux/dax.h b/include/linux/dax.h
index 73cfc1a7c8f1..b19bfe0c2fd1 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -243,6 +243,7 @@ static inline void dax_break_layout_final(struct inode *inode)
bool dax_alive(struct dax_device *dax_dev);
void *dax_get_private(struct dax_device *dax_dev);
+int dax_set_ops(struct dax_device *dax_dev, const struct dax_operations *ops);
long dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff, long nr_pages,
enum dax_access_mode mode, void **kaddr, unsigned long *pfn);
size_t dax_copy_from_iter(struct dax_device *dax_dev, pgoff_t pgoff, void *addr,
--
2.53.0
^ permalink raw reply related
* [PATCH V10 5/8] dax: Add dax_operations for use by fs-dax on fsdev dax
From: John Groves @ 2026-03-27 21:04 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <John@Groves.net>
fsdev: Add dax_operations for use by famfs.
This replicates the functionality from drivers/nvdimm/pmem.c that
conventional fs-dax file systems (e.g. xfs) use to support dax
read/write/mmap to a daxdev - without which famfs can't sit atop a
daxdev.
- These methods are based on pmem_dax_ops from drivers/nvdimm/pmem.c
- fsdev_dax_direct_access() returns the hpa, pfn and kva. The kva was
newly stored as dev_dax->virt_addr by dev_dax_probe().
- The hpa/pfn are used for mmap (dax_iomap_fault()), and the kva is used
for read/write (dax_iomap_rw())
- fsdev_dax_recovery_write() and dev_dax_zero_page_range() have not been
tested yet. I'm looking for suggestions as to how to test those.
- dax-private.h: add dev_dax->cached_size, which fsdev needs to
remember. The dev_dax size cannot change while a driver is bound
(dev_dax_resize returns -EBUSY if dev->driver is set). Caching the size
at probe time allows fsdev's direct_access path can use it without
acquiring dax_dev_rwsem (which isn't exported anyway).
Signed-off-by: John Groves <john@groves.net>
---
drivers/dax/dax-private.h | 2 +
drivers/dax/fsdev.c | 84 +++++++++++++++++++++++++++++++++++++++
2 files changed, 86 insertions(+)
diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
index 7a3727d76a68..81e4af49e39c 100644
--- a/drivers/dax/dax-private.h
+++ b/drivers/dax/dax-private.h
@@ -70,6 +70,7 @@ struct dev_dax_range {
* @region: parent region
* @dax_dev: core dax functionality
* @virt_addr: kva from memremap; used by fsdev_dax
+ * @cached_size: size of daxdev cached by fsdev_dax
* @align: alignment of this instance
* @target_node: effective numa node if dev_dax memory range is onlined
* @dyn_id: is this a dynamic or statically created instance
@@ -85,6 +86,7 @@ struct dev_dax {
struct dax_region *region;
struct dax_device *dax_dev;
void *virt_addr;
+ u64 cached_size;
unsigned int align;
int target_node;
bool dyn_id;
diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
index c75478d3d548..30f57c74c979 100644
--- a/drivers/dax/fsdev.c
+++ b/drivers/dax/fsdev.c
@@ -28,6 +28,85 @@
* - No mmap support - all access is through fs-dax/iomap
*/
+static void fsdev_write_dax(void *addr, struct page *page,
+ unsigned int off, unsigned int len)
+{
+ while (len) {
+ void *mem = kmap_local_page(page);
+ unsigned int chunk = min_t(unsigned int, len, PAGE_SIZE - off);
+
+ memcpy_flushcache(addr, mem + off, chunk);
+ kunmap_local(mem);
+ len -= chunk;
+ off = 0;
+ page++;
+ addr += chunk;
+ }
+}
+
+static long __fsdev_dax_direct_access(struct dax_device *dax_dev, pgoff_t pgoff,
+ long nr_pages, enum dax_access_mode mode, void **kaddr,
+ unsigned long *pfn)
+{
+ struct dev_dax *dev_dax = dax_get_private(dax_dev);
+ size_t size = nr_pages << PAGE_SHIFT;
+ size_t offset = pgoff << PAGE_SHIFT;
+ void *virt_addr = dev_dax->virt_addr + offset;
+ phys_addr_t phys;
+ unsigned long local_pfn;
+
+ phys = dax_pgoff_to_phys(dev_dax, pgoff, size);
+ if (phys == -1) {
+ dev_dbg(&dev_dax->dev,
+ "pgoff (%#lx) out of range\n", pgoff);
+ return -EFAULT;
+ }
+
+ if (kaddr)
+ *kaddr = virt_addr;
+
+ local_pfn = PHYS_PFN(phys);
+ if (pfn)
+ *pfn = local_pfn;
+
+ /*
+ * Use cached_size which was computed at probe time. The size cannot
+ * change while the driver is bound (resize returns -EBUSY).
+ */
+ return PHYS_PFN(min(size, dev_dax->cached_size - offset));
+}
+
+static int fsdev_dax_zero_page_range(struct dax_device *dax_dev,
+ pgoff_t pgoff, size_t nr_pages)
+{
+ void *kaddr;
+
+ WARN_ONCE(nr_pages > 1, "%s: nr_pages > 1\n", __func__);
+ __fsdev_dax_direct_access(dax_dev, pgoff, 1, DAX_ACCESS, &kaddr, NULL);
+ fsdev_write_dax(kaddr, ZERO_PAGE(0), 0, PAGE_SIZE);
+ return 0;
+}
+
+static long fsdev_dax_direct_access(struct dax_device *dax_dev,
+ pgoff_t pgoff, long nr_pages, enum dax_access_mode mode,
+ void **kaddr, unsigned long *pfn)
+{
+ return __fsdev_dax_direct_access(dax_dev, pgoff, nr_pages, mode,
+ kaddr, pfn);
+}
+
+static size_t fsdev_dax_recovery_write(struct dax_device *dax_dev, pgoff_t pgoff,
+ void *addr, size_t bytes, struct iov_iter *i)
+{
+ return _copy_from_iter_flushcache(addr, bytes, i);
+}
+
+static const struct dax_operations dev_dax_ops = {
+ .direct_access = fsdev_dax_direct_access,
+ .zero_page_range = fsdev_dax_zero_page_range,
+ .recovery_write = fsdev_dax_recovery_write,
+};
+
static void fsdev_cdev_del(void *cdev)
{
cdev_del(cdev);
@@ -167,6 +246,11 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
}
}
+ /* Cache size now; it cannot change while driver is bound */
+ dev_dax->cached_size = 0;
+ for (i = 0; i < dev_dax->nr_range; i++)
+ dev_dax->cached_size += range_len(&dev_dax->ranges[i].range);
+
/*
* Use MEMORY_DEVICE_FS_DAX without setting vmemmap_shift, leaving
* folios at order-0. Unlike device.c (MEMORY_DEVICE_GENERIC), this
--
2.53.0
^ permalink raw reply related
* [PATCH V10 4/8] dax: Save the kva from memremap
From: John Groves @ 2026-03-27 21:04 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves, Ira Weiny, Jonathan Cameron
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <john@groves.net>
Save the kva from memremap because we need it for iomap rw support.
Prior to famfs, there were no iomap users of /dev/dax - so the virtual
address from memremap was not needed.
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
drivers/dax/dax-private.h | 2 ++
drivers/dax/fsdev.c | 3 ++-
2 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/dax/dax-private.h b/drivers/dax/dax-private.h
index c6ae27c982f4..7a3727d76a68 100644
--- a/drivers/dax/dax-private.h
+++ b/drivers/dax/dax-private.h
@@ -69,6 +69,7 @@ struct dev_dax_range {
* data while the device is activated in the driver.
* @region: parent region
* @dax_dev: core dax functionality
+ * @virt_addr: kva from memremap; used by fsdev_dax
* @align: alignment of this instance
* @target_node: effective numa node if dev_dax memory range is onlined
* @dyn_id: is this a dynamic or statically created instance
@@ -83,6 +84,7 @@ struct dev_dax_range {
struct dev_dax {
struct dax_region *region;
struct dax_device *dax_dev;
+ void *virt_addr;
unsigned int align;
int target_node;
bool dyn_id;
diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
index 8b5c6976ad17..c75478d3d548 100644
--- a/drivers/dax/fsdev.c
+++ b/drivers/dax/fsdev.c
@@ -121,6 +121,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
struct device *dev = &dev_dax->dev;
struct dev_pagemap *pgmap;
struct inode *inode;
+ u64 data_offset = 0;
struct cdev *cdev;
void *addr;
int rc, i;
@@ -196,7 +197,6 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
if (pgmap->range.start != dev_dax->ranges[0].range.start) {
u64 phys = dev_dax->ranges[0].range.start;
u64 pgmap_phys = dev_dax->pgmap[0].range.start;
- u64 data_offset = 0;
if (!WARN_ON(pgmap_phys > phys))
data_offset = phys - pgmap_phys;
@@ -204,6 +204,7 @@ static int fsdev_dax_probe(struct dev_dax *dev_dax)
pr_debug("%s: offset detected phys=%llx pgmap_phys=%llx offset=%llx\n",
__func__, phys, pgmap_phys, data_offset);
}
+ dev_dax->virt_addr = addr + data_offset;
inode = dax_inode(dax_dev);
cdev = inode->i_cdev;
--
2.53.0
^ permalink raw reply related
* [PATCH V10 3/8] dax: add fsdev.c driver for fs-dax on character dax
From: John Groves @ 2026-03-27 21:04 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves, Jonathan Cameron
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <john@groves.net>
The new fsdev driver provides pages/folios initialized compatibly with
fsdax - normal rather than devdax-style refcounting, and starting out
with order-0 folios.
When fsdev binds to a daxdev, it is usually (always?) switching from the
devdax mode (device.c), which pre-initializes compound folios according
to its alignment. Fsdev uses fsdev_clear_folio_state() to switch the
folios into a fsdax-compatible state.
A side effect of this is that raw mmap doesn't (can't?) work on an fsdev
dax instance. Accordingly, The fsdev driver does not provide raw mmap -
devices must be put in 'devdax' mode (drivers/dax/device.c) to get raw
mmap capability.
In this commit is just the framework, which remaps pages/folios compatibly
with fsdax.
Enabling dax changes:
- bus.h: add DAXDRV_FSDEV_TYPE driver type
- bus.c: allow DAXDRV_FSDEV_TYPE drivers to bind to daxdevs
- dax.h: prototype inode_dax(), which fsdev needs
Suggested-by: Dan Williams <dan.j.williams@intel.com>
Suggested-by: Gregory Price <gourry@gourry.net>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
MAINTAINERS | 8 ++
drivers/dax/Kconfig | 5 +
drivers/dax/Makefile | 2 +
drivers/dax/bus.h | 1 +
drivers/dax/fsdev.c | 245 +++++++++++++++++++++++++++++++++++++++++++
fs/dax.c | 1 +
6 files changed, 262 insertions(+)
create mode 100644 drivers/dax/fsdev.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 7d10988cbc62..eedf4cce56ed 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7298,6 +7298,14 @@ L: linux-cxl@vger.kernel.org
S: Supported
F: drivers/dax/
+DEVICE DIRECT ACCESS (DAX) [fsdev_dax]
+M: John Groves <jgroves@micron.com>
+M: John Groves <John@Groves.net>
+L: nvdimm@lists.linux.dev
+L: linux-cxl@vger.kernel.org
+S: Supported
+F: drivers/dax/fsdev.c
+
DEVICE FREQUENCY (DEVFREQ)
M: MyungJoo Ham <myungjoo.ham@samsung.com>
M: Kyungmin Park <kyungmin.park@samsung.com>
diff --git a/drivers/dax/Kconfig b/drivers/dax/Kconfig
index d656e4c0eb84..6d8493cc540c 100644
--- a/drivers/dax/Kconfig
+++ b/drivers/dax/Kconfig
@@ -61,6 +61,11 @@ config DEV_DAX_HMEM_DEVICES
depends on DEV_DAX_HMEM && DAX
def_bool y
+config DEV_DAX_FSDEV
+ tristate
+ depends on DEV_DAX && FS_DAX
+ default DEV_DAX
+
config DEV_DAX_KMEM
tristate "KMEM DAX: map dax-devices as System-RAM"
default DEV_DAX
diff --git a/drivers/dax/Makefile b/drivers/dax/Makefile
index 5ed5c39857c8..ba35bda7abef 100644
--- a/drivers/dax/Makefile
+++ b/drivers/dax/Makefile
@@ -4,11 +4,13 @@ obj-$(CONFIG_DEV_DAX) += device_dax.o
obj-$(CONFIG_DEV_DAX_KMEM) += kmem.o
obj-$(CONFIG_DEV_DAX_PMEM) += dax_pmem.o
obj-$(CONFIG_DEV_DAX_CXL) += dax_cxl.o
+obj-$(CONFIG_DEV_DAX_FSDEV) += fsdev_dax.o
dax-y := super.o
dax-y += bus.o
device_dax-y := device.o
dax_pmem-y := pmem.o
dax_cxl-y := cxl.o
+fsdev_dax-y := fsdev.o
obj-y += hmem/
diff --git a/drivers/dax/bus.h b/drivers/dax/bus.h
index cbbf64443098..880bdf7e72d7 100644
--- a/drivers/dax/bus.h
+++ b/drivers/dax/bus.h
@@ -31,6 +31,7 @@ struct dev_dax *devm_create_dev_dax(struct dev_dax_data *data);
enum dax_driver_type {
DAXDRV_KMEM_TYPE,
DAXDRV_DEVICE_TYPE,
+ DAXDRV_FSDEV_TYPE,
};
struct dax_device_driver {
diff --git a/drivers/dax/fsdev.c b/drivers/dax/fsdev.c
new file mode 100644
index 000000000000..8b5c6976ad17
--- /dev/null
+++ b/drivers/dax/fsdev.c
@@ -0,0 +1,245 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2026 Micron Technology, Inc. */
+#include <linux/memremap.h>
+#include <linux/pagemap.h>
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/cdev.h>
+#include <linux/slab.h>
+#include <linux/dax.h>
+#include <linux/uio.h>
+#include <linux/fs.h>
+#include <linux/mm.h>
+#include "dax-private.h"
+#include "bus.h"
+
+/*
+ * FS-DAX compatible devdax driver
+ *
+ * Unlike drivers/dax/device.c which pre-initializes compound folios based
+ * on device alignment (via vmemmap_shift), this driver leaves folios
+ * uninitialized similar to pmem. This allows fs-dax filesystems like famfs
+ * to work without needing special handling for pre-initialized folios.
+ *
+ * Key differences from device.c:
+ * - pgmap type is MEMORY_DEVICE_FS_DAX (not MEMORY_DEVICE_GENERIC)
+ * - vmemmap_shift is NOT set (folios remain order-0)
+ * - fs-dax can dynamically create compound folios as needed
+ * - No mmap support - all access is through fs-dax/iomap
+ */
+
+static void fsdev_cdev_del(void *cdev)
+{
+ cdev_del(cdev);
+}
+
+static void fsdev_kill(void *dev_dax)
+{
+ kill_dev_dax(dev_dax);
+}
+
+/*
+ * Page map operations for FS-DAX mode
+ * Similar to fsdax_pagemap_ops in drivers/nvdimm/pmem.c
+ *
+ * Note: folio_free callback is not needed for MEMORY_DEVICE_FS_DAX.
+ * The core mm code in free_zone_device_folio() handles the wake_up_var()
+ * directly for this memory type.
+ */
+static int fsdev_pagemap_memory_failure(struct dev_pagemap *pgmap,
+ unsigned long pfn, unsigned long nr_pages, int mf_flags)
+{
+ struct dev_dax *dev_dax = pgmap->owner;
+ u64 offset = PFN_PHYS(pfn) - dev_dax->ranges[0].range.start;
+ u64 len = nr_pages << PAGE_SHIFT;
+
+ return dax_holder_notify_failure(dev_dax->dax_dev, offset,
+ len, mf_flags);
+}
+
+static const struct dev_pagemap_ops fsdev_pagemap_ops = {
+ .memory_failure = fsdev_pagemap_memory_failure,
+};
+
+/*
+ * Clear any stale folio state from pages in the given range.
+ * This is necessary because device_dax pre-initializes compound folios
+ * based on vmemmap_shift, and that state may persist after driver unbind.
+ * Since fsdev_dax uses MEMORY_DEVICE_FS_DAX without vmemmap_shift, fs-dax
+ * expects to find clean order-0 folios that it can build into compound
+ * folios on demand.
+ *
+ * At probe time, no filesystem should be mounted yet, so all mappings
+ * are stale and must be cleared along with compound state.
+ */
+static void fsdev_clear_folio_state(struct dev_dax *dev_dax)
+{
+ for (int i = 0; i < dev_dax->nr_range; i++) {
+ struct range *range = &dev_dax->ranges[i].range;
+ unsigned long pfn = PHYS_PFN(range->start);
+ unsigned long end_pfn = PHYS_PFN(range->end) + 1;
+
+ while (pfn < end_pfn) {
+ struct folio *folio = pfn_folio(pfn);
+ int order = dax_folio_reset_order(folio);
+
+ pfn += 1UL << order;
+ }
+ }
+}
+
+static void fsdev_clear_folio_state_action(void *data)
+{
+ fsdev_clear_folio_state(data);
+}
+
+static int fsdev_open(struct inode *inode, struct file *filp)
+{
+ struct dax_device *dax_dev = inode_dax(inode);
+ struct dev_dax *dev_dax = dax_get_private(dax_dev);
+
+ filp->private_data = dev_dax;
+
+ return 0;
+}
+
+static int fsdev_release(struct inode *inode, struct file *filp)
+{
+ return 0;
+}
+
+static const struct file_operations fsdev_fops = {
+ .llseek = noop_llseek,
+ .owner = THIS_MODULE,
+ .open = fsdev_open,
+ .release = fsdev_release,
+};
+
+static int fsdev_dax_probe(struct dev_dax *dev_dax)
+{
+ struct dax_device *dax_dev = dev_dax->dax_dev;
+ struct device *dev = &dev_dax->dev;
+ struct dev_pagemap *pgmap;
+ struct inode *inode;
+ struct cdev *cdev;
+ void *addr;
+ int rc, i;
+
+ if (static_dev_dax(dev_dax)) {
+ if (dev_dax->nr_range > 1) {
+ dev_warn(dev, "static pgmap / multi-range device conflict\n");
+ return -EINVAL;
+ }
+
+ pgmap = dev_dax->pgmap;
+ } else {
+ size_t pgmap_size;
+
+ if (dev_dax->pgmap) {
+ dev_warn(dev, "dynamic-dax with pre-populated page map\n");
+ return -EINVAL;
+ }
+
+ pgmap_size = struct_size(pgmap, ranges, dev_dax->nr_range - 1);
+ pgmap = devm_kzalloc(dev, pgmap_size, GFP_KERNEL);
+ if (!pgmap)
+ return -ENOMEM;
+
+ pgmap->nr_range = dev_dax->nr_range;
+ dev_dax->pgmap = pgmap;
+
+ for (i = 0; i < dev_dax->nr_range; i++) {
+ struct range *range = &dev_dax->ranges[i].range;
+
+ pgmap->ranges[i] = *range;
+ }
+ }
+
+ for (i = 0; i < dev_dax->nr_range; i++) {
+ struct range *range = &dev_dax->ranges[i].range;
+
+ if (!devm_request_mem_region(dev, range->start,
+ range_len(range), dev_name(dev))) {
+ dev_warn(dev, "mapping%d: %#llx-%#llx could not reserve range\n",
+ i, range->start, range->end);
+ return -EBUSY;
+ }
+ }
+
+ /*
+ * Use MEMORY_DEVICE_FS_DAX without setting vmemmap_shift, leaving
+ * folios at order-0. Unlike device.c (MEMORY_DEVICE_GENERIC), this
+ * lets fs-dax dynamically build compound folios as needed, similar
+ * to pmem behavior.
+ */
+ pgmap->type = MEMORY_DEVICE_FS_DAX;
+ pgmap->ops = &fsdev_pagemap_ops;
+ pgmap->owner = dev_dax;
+
+ addr = devm_memremap_pages(dev, pgmap);
+ if (IS_ERR(addr))
+ return PTR_ERR(addr);
+
+ /*
+ * Clear any stale compound folio state left over from a previous
+ * driver (e.g., device_dax with vmemmap_shift). Also register this
+ * as a devm action so folio state is cleared on unbind, ensuring
+ * clean pages for subsequent drivers (e.g., kmem for system-ram).
+ */
+ fsdev_clear_folio_state(dev_dax);
+ rc = devm_add_action_or_reset(dev, fsdev_clear_folio_state_action,
+ dev_dax);
+ if (rc)
+ return rc;
+
+ /* Detect whether the data is at a non-zero offset into the memory */
+ if (pgmap->range.start != dev_dax->ranges[0].range.start) {
+ u64 phys = dev_dax->ranges[0].range.start;
+ u64 pgmap_phys = dev_dax->pgmap[0].range.start;
+ u64 data_offset = 0;
+
+ if (!WARN_ON(pgmap_phys > phys))
+ data_offset = phys - pgmap_phys;
+
+ pr_debug("%s: offset detected phys=%llx pgmap_phys=%llx offset=%llx\n",
+ __func__, phys, pgmap_phys, data_offset);
+ }
+
+ inode = dax_inode(dax_dev);
+ cdev = inode->i_cdev;
+ cdev_init(cdev, &fsdev_fops);
+ cdev->owner = dev->driver->owner;
+ cdev_set_parent(cdev, &dev->kobj);
+ rc = cdev_add(cdev, dev->devt, 1);
+ if (rc)
+ return rc;
+
+ rc = devm_add_action_or_reset(dev, fsdev_cdev_del, cdev);
+ if (rc)
+ return rc;
+
+ run_dax(dax_dev);
+ return devm_add_action_or_reset(dev, fsdev_kill, dev_dax);
+}
+
+static struct dax_device_driver fsdev_dax_driver = {
+ .probe = fsdev_dax_probe,
+ .type = DAXDRV_FSDEV_TYPE,
+};
+
+static int __init dax_init(void)
+{
+ return dax_driver_register(&fsdev_dax_driver);
+}
+
+static void __exit dax_exit(void)
+{
+ dax_driver_unregister(&fsdev_dax_driver);
+}
+
+MODULE_AUTHOR("John Groves");
+MODULE_DESCRIPTION("FS-DAX Device: fs-dax compatible devdax driver");
+MODULE_LICENSE("GPL");
+module_init(dax_init);
+module_exit(dax_exit);
+MODULE_ALIAS_DAX_DEVICE(0);
diff --git a/fs/dax.c b/fs/dax.c
index 87bed6de920d..cb5c87e43bc3 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -429,6 +429,7 @@ int dax_folio_reset_order(struct folio *folio)
return order;
}
+EXPORT_SYMBOL_GPL(dax_folio_reset_order);
static inline unsigned long dax_folio_put(struct folio *folio)
{
--
2.53.0
^ permalink raw reply related
* [PATCH V10 2/8] dax: Factor out dax_folio_reset_order() helper
From: John Groves @ 2026-03-27 21:04 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Jonathan Cameron, Ira Weiny, John Groves
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <John@Groves.net>
Both fs/dax.c:dax_folio_put() and drivers/dax/fsdev.c:
fsdev_clear_folio_state() (the latter coming in the next commit after this
one) contain nearly identical code to reset a compound DAX folio back to
order-0 pages. Factor this out into a shared helper function.
The new dax_folio_reset_order() function:
- Clears the folio's mapping and share count
- Resets compound folio state via folio_reset_order()
- Clears PageHead and compound_head for each sub-page
- Restores the pgmap pointer for each resulting order-0 folio
- Returns the original folio order (for callers that need to advance by
that many pages)
Two intentional differences from the original dax_folio_put() logic:
1. folio->share is cleared unconditionally. This is correct because the DAX
subsystem maintains the invariant that share != 0 only when
mapping == NULL (enforced by dax_folio_make_shared()). dax_folio_put()
ensures share has reached zero before calling this helper, so the
unconditional clear is safe.
2. folio->pgmap is now explicitly restored for order-0 folios. For the
dax_folio_put() caller this is a no-op (reads and writes back the same
field). It is intentional for the upcoming fsdev_clear_folio_state()
caller, which converts previously-compound folios and needs pgmap
re-established for all pages regardless of order.
This simplifies fsdev_clear_folio_state() from ~50 lines to ~15 lines.
Suggested-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
fs/dax.c | 73 ++++++++++++++++++++++++++++++++++-----------
include/linux/dax.h | 1 +
2 files changed, 56 insertions(+), 18 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index 289e6254aa30..87bed6de920d 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -378,6 +378,58 @@ static void dax_folio_make_shared(struct folio *folio)
folio->share = 1;
}
+/**
+ * dax_folio_reset_order - Reset a compound DAX folio to order-0 pages
+ * @folio: The folio to reset
+ *
+ * Splits a compound folio back into individual order-0 pages,
+ * clearing compound state and restoring pgmap pointers.
+ *
+ * Returns: the original folio order (0 if already order-0)
+ */
+int dax_folio_reset_order(struct folio *folio)
+{
+ struct dev_pagemap *pgmap = page_pgmap(&folio->page);
+ int order = folio_order(folio);
+
+ /*
+ * DAX maintains the invariant that folio->share != 0 only when
+ * folio->mapping == NULL (enforced by dax_folio_make_shared()).
+ * Equivalently: folio->mapping != NULL implies folio->share == 0.
+ * Callers ensure share has been decremented to zero before
+ * calling here, so unconditionally clearing both fields is
+ * correct.
+ */
+ folio->mapping = NULL;
+ folio->share = 0;
+
+ if (!order) {
+ /*
+ * Restore pgmap explicitly even for order-0 folios. For the
+ * dax_folio_put() caller this is a no-op (same value), but
+ * fsdev_clear_folio_state() may call this on folios that
+ * were previously compound and need pgmap re-established.
+ */
+ folio->pgmap = pgmap;
+ return 0;
+ }
+
+ folio_reset_order(folio);
+
+ for (int i = 0; i < (1UL << order); i++) {
+ struct page *page = folio_page(folio, i);
+ struct folio *f = (struct folio *)page;
+
+ ClearPageHead(page);
+ clear_compound_head(page);
+ f->mapping = NULL;
+ f->share = 0;
+ f->pgmap = pgmap;
+ }
+
+ return order;
+}
+
static inline unsigned long dax_folio_put(struct folio *folio)
{
unsigned long ref;
@@ -391,28 +443,13 @@ static inline unsigned long dax_folio_put(struct folio *folio)
if (ref)
return ref;
- folio->mapping = NULL;
- order = folio_order(folio);
- if (!order)
- return 0;
- folio_reset_order(folio);
+ order = dax_folio_reset_order(folio);
+ /* Debug check: verify refcounts are zero for all sub-folios */
for (i = 0; i < (1UL << order); i++) {
- struct dev_pagemap *pgmap = page_pgmap(&folio->page);
struct page *page = folio_page(folio, i);
- struct folio *new_folio = (struct folio *)page;
-
- ClearPageHead(page);
- clear_compound_head(page);
- new_folio->mapping = NULL;
- /*
- * Reset pgmap which was over-written by
- * prep_compound_page().
- */
- new_folio->pgmap = pgmap;
- new_folio->share = 0;
- WARN_ON_ONCE(folio_ref_count(new_folio));
+ WARN_ON_ONCE(folio_ref_count((struct folio *)page));
}
return ref;
diff --git a/include/linux/dax.h b/include/linux/dax.h
index bf103f317cac..73cfc1a7c8f1 100644
--- a/include/linux/dax.h
+++ b/include/linux/dax.h
@@ -153,6 +153,7 @@ static inline void fs_put_dax(struct dax_device *dax_dev, void *holder)
#if IS_ENABLED(CONFIG_FS_DAX)
int dax_writeback_mapping_range(struct address_space *mapping,
struct dax_device *dax_dev, struct writeback_control *wbc);
+int dax_folio_reset_order(struct folio *folio);
struct page *dax_layout_busy_page(struct address_space *mapping);
struct page *dax_layout_busy_page_range(struct address_space *mapping, loff_t start, loff_t end);
--
2.53.0
^ permalink raw reply related
* [PATCH V10 1/8] dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c
From: John Groves @ 2026-03-27 21:04 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves, Ira Weiny, Jonathan Cameron
In-Reply-To: <0100019d311bed04-dbb67b48-c55d-4e6a-962a-a0f8b714f2e7-000000@email.amazonses.com>
From: John Groves <john@groves.net>
This function will be used by both device.c and fsdev.c, but both are
loadable modules. Moving to bus.c puts it in core and makes it available
to both.
No code changes - just relocated.
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Signed-off-by: John Groves <john@groves.net>
---
drivers/dax/bus.c | 20 ++++++++++++++++++++
drivers/dax/device.c | 23 -----------------------
2 files changed, 20 insertions(+), 23 deletions(-)
diff --git a/drivers/dax/bus.c b/drivers/dax/bus.c
index c94c09622516..1b412264bb36 100644
--- a/drivers/dax/bus.c
+++ b/drivers/dax/bus.c
@@ -1417,6 +1417,26 @@ static const struct device_type dev_dax_type = {
.groups = dax_attribute_groups,
};
+/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c */
+__weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff,
+ unsigned long size)
+{
+ for (int i = 0; i < dev_dax->nr_range; i++) {
+ struct dev_dax_range *dax_range = &dev_dax->ranges[i];
+ struct range *range = &dax_range->range;
+ phys_addr_t phys;
+
+ if (!in_range(pgoff, dax_range->pgoff, PHYS_PFN(range_len(range))))
+ continue;
+ phys = PFN_PHYS(pgoff - dax_range->pgoff) + range->start;
+ if (phys + size - 1 <= range->end)
+ return phys;
+ break;
+ }
+ return -1;
+}
+EXPORT_SYMBOL_GPL(dax_pgoff_to_phys);
+
static struct dev_dax *__devm_create_dev_dax(struct dev_dax_data *data)
{
struct dax_region *dax_region = data->dax_region;
diff --git a/drivers/dax/device.c b/drivers/dax/device.c
index 528e81240c4d..2d2dbfd35e94 100644
--- a/drivers/dax/device.c
+++ b/drivers/dax/device.c
@@ -57,29 +57,6 @@ static int check_vma(struct dev_dax *dev_dax, struct vm_area_struct *vma,
vma->vm_file, func);
}
-/* see "strong" declaration in tools/testing/nvdimm/dax-dev.c */
-__weak phys_addr_t dax_pgoff_to_phys(struct dev_dax *dev_dax, pgoff_t pgoff,
- unsigned long size)
-{
- int i;
-
- for (i = 0; i < dev_dax->nr_range; i++) {
- struct dev_dax_range *dax_range = &dev_dax->ranges[i];
- struct range *range = &dax_range->range;
- unsigned long long pgoff_end;
- phys_addr_t phys;
-
- pgoff_end = dax_range->pgoff + PHYS_PFN(range_len(range)) - 1;
- if (pgoff < dax_range->pgoff || pgoff > pgoff_end)
- continue;
- phys = PFN_PHYS(pgoff - dax_range->pgoff) + range->start;
- if (phys + size - 1 <= range->end)
- return phys;
- break;
- }
- return -1;
-}
-
static void dax_set_mapping(struct vm_fault *vmf, unsigned long pfn,
unsigned long fault_size)
{
--
2.53.0
^ permalink raw reply related
* [PATCH V10 0/8] dax: prepare for famfs
From: John Groves @ 2026-03-27 21:03 UTC (permalink / raw)
To: John Groves, Miklos Szeredi, Dan Williams, Bernd Schubert,
Alison Schofield
Cc: John Groves, Jonathan Corbet, Shuah Khan, Vishal Verma,
Dave Jiang, Matthew Wilcox, Jan Kara, Alexander Viro,
David Hildenbrand, Christian Brauner, Darrick J . Wong,
Randy Dunlap, Jeff Layton, Amir Goldstein, Jonathan Cameron,
Stefan Hajnoczi, Joanne Koong, Josef Bacik, Bagas Sanjaya,
Chen Linxuan, James Morse, Fuad Tabba, Sean Christopherson,
Shivank Garg, Ackerley Tng, Gregory Price, Aravind Ramesh,
Ajay Joshi, venkataravis@micron.com, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, nvdimm@lists.linux.dev,
linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org,
John Groves
In-Reply-To: <20260327210311.79099-1-john@jagalactic.com>
From: John Groves <john@groves.net>
This patch series along with the bundled patches to fuse are available
as a git tag at [0].
Dropped the "bundle" thread. If this submission goes smoothly, I'll update
the fuse patches to v10 (very little change there as yet).
Changes v9 -> v10
- Minor modernizations per comments from (mostly) Jonathan
- Minor Kconfig simplification
- bus.c:dax_match_type(): don't make fsdev_dax eligible for automatic binding
where devdax would otherwise bind
- dax-private.h: add missing kerneldoc comment for field cached_size in
struct dev_dax_range (thanks Dave)
- fsdev_write_dax(): s/pmem_addr/addr/ (thanks Dave)
- include/linux/dax.h: remove a spuriously-added declaration of inode_dax()
(thanks Jonathan)
Description:
This patch series introduces the required dax support for famfs.
Previous versions of the famfs series included both dax and fuse patches.
This series separates them into separate patch series' (and the fuse
series dependends on this dax series).
The famfs user space code can be found at [1]
Dax Overview:
This series introduces a new "famfs mode" of devdax, whose driver is
drivers/dax/fsdev.c. This driver supports dax_iomap_rw() and
dax_iomap_fault() calls against a character dax instance. A dax device
now can be converted among three modes: 'system-ram', 'devdax' and
'famfs' via daxctl or sysfs (e.g. unbind devdax and bind famfs instead).
In famfs mode, a dax device initializes its pages consistent with the
fsdaxmode of pmem. Raw read/write/mmap are not supported in this mode,
but famfs is happy in this mode - using dax_iomap_rw() for read/write and
dax_iomap_fault() for mmap faults.
Changes v8 -> v9
- Big clarifying comments in fs/dax.c:dax_folio_reset_order() in response
to Jonathan's comments/questions
- Added drivers/dax/Kconfig:CONFIG_DEV_DAX_FSDEV to control inclusion of the
new famfs dax mode (which is in drivers/dax/fsdev.c - bind that to a daxdev
and you have famfs mode)
- Some trivial refactoring, mostly per Jonathan's comments
Changes v7 -> v8
- dax: Added a devm action to clear folio state when unbinding fsdev.c
(thanks Allison)
- Added a missing device_lock() in fs_dax_get() (thanks Dave)
- Re-factored some __free blocks for inline declaration
- Used FIELD_PREP where appropriate
- Minor doc edits
Changes v6 -> v7
- Fixed a regression in famfs_interleave_fileofs_to_daxofs() that
was reported by Intel's kernel test robot
- Added a check in __fsdev_dax_direct_access() for negative return
from pgoff_to_phys(), which would indicate an out-of-range offset
- Fixed a bug in __famfs_meta_free(), where not all interleaved
extents were freed
- Added chunksize alignment checks in famfs_fuse_meta_alloc() and
famfs_interleave_fileofs_to_daxofs() as interleaved chunks must
be PTE or PMD aligned
- Simplified famfs_file_init_dax() a bit
- Re-ran CM's kernel code review prompts on the entire series and
fixed several minor issues
Changes v4 -> v5 -> v6
- None. Re-sending due to technical difficulties
Changes v3 [9] -> v4
- The patch "dax: prevent driver unbind while filesystem holds device"
has been dropped. Dan Williams indicated that the favored behavior is
for a file system to stop working if an underlying driver is unbound,
rather than preventing the unbind.
- The patch "famfs_fuse: Famfs mount opt: -o shadow=<shadowpath>" has
been dropped. Found a way for the famfs user space to do without the
-o opt (via getxattr).
- Squashed the fs/fuse/Kconfig patch into the first subsequent patch
that needed the change
("famfs_fuse: Basic fuse kernel ABI enablement for famfs")
- Many review comments addressed.
- Addressed minor kerneldoc infractions reported by test robot.
Changes v2 [7] -> v3
- Dax: Completely new fsdev driver (drivers/dax/fsdev.c) replaces the
dev_dax_iomap modifications to bus.c/device.c. Devdax devices can now
be switched among 'devdax', 'famfs' and 'system-ram' modes via daxctl
or sysfs.
- Dax: fsdev uses MEMORY_DEVICE_FS_DAX type and leaves folios at order-0
(no vmemmap_shift), allowing fs-dax to manage folio lifecycles
dynamically like pmem does.
- Dax: The "poisoned page" problem is properly fixed via
fsdev_clear_folio_state(), which clears stale mapping/compound state
when fsdev binds. The temporary WARN_ON_ONCE workaround in fs/dax.c
has been removed.
- Dax: Added dax_set_ops() so fsdev can set dax_operations at bind time
(and clear them on unbind), since the dax_device is created before we
know which driver will bind.
- Dax: Added custom bind/unbind sysfs handlers; unbind return -EBUSY if a
filesystem holds the device, preventing unbind while famfs is mounted.
- Fuse: Famfs mounts now require that the fuse server/daemon has
CAP_SYS_RAWIO because they expose raw memory devices.
- Fuse: Added DAX address_space_operations with noop_dirty_folio since
famfs is memory-backed with no writeback required.
- Rebased to latest kernels, fully compatible with Alistair Popple
et. al's recent dax refactoring.
- Ran this series through Chris Mason's code review AI prompts to check
for issues - several subtle problems found and fixed.
- Dropped RFC status - this version is intended to be mergeable.
Changes v1 [8] -> v2:
- The GET_FMAP message/response has been moved from LOOKUP to OPEN, as
was the pretty much unanimous consensus.
- Made the response payload to GET_FMAP variable sized (patch 12)
- Dodgy kerneldoc comments cleaned up or removed.
- Fixed memory leak of fc->shadow in patch 11 (thanks Joanne)
- Dropped many pr_debug and pr_notice calls
References
[0] - https://github.com/jagalactic/linux/tree/famfs-v10 (this patch set)
[1] - https://famfs.org (famfs user space)
[2] - https://lore.kernel.org/linux-cxl/cover.1708709155.git.john@groves.net/
[3] - https://lore.kernel.org/linux-cxl/cover.1714409084.git.john@groves.net/
[4] - https://lwn.net/Articles/983105/ (lsfmm 2024)
[5] - https://lwn.net/Articles/1020170/ (lsfmm 2025)
[6] - https://lore.kernel.org/linux-cxl/cover.8068ad144a7eea4a813670301f4d2a86a8e68ec4.1740713401.git-series.apopple@nvidia.com/
[7] - https://lore.kernel.org/linux-fsdevel/20250703185032.46568-1-john@groves.net/ (famfs fuse v2)
[8] - https://lore.kernel.org/linux-fsdevel/20250421013346.32530-1-john@groves.net/ (famfs fuse v1)
[9] - https://lore.kernel.org/linux-fsdevel/20260107153244.64703-1-john@groves.net/T/#mb2c868801be16eca82dab239a1d201628534aea7 (famfs fuse v3)
John Groves (8):
dax: move dax_pgoff_to_phys from [drivers/dax/] device.c to bus.c
dax: Factor out dax_folio_reset_order() helper
dax: add fsdev.c driver for fs-dax on character dax
dax: Save the kva from memremap
dax: Add dax_operations for use by fs-dax on fsdev dax
dax: Add dax_set_ops() for setting dax_operations at bind time
dax: Add fs_dax_get() func to prepare dax for fs-dax usage
dax: export dax_dev_get()
MAINTAINERS | 8 +
drivers/dax/Kconfig | 5 +
drivers/dax/Makefile | 2 +
drivers/dax/bus.c | 22 ++-
drivers/dax/bus.h | 3 +
drivers/dax/dax-private.h | 4 +
drivers/dax/device.c | 23 ---
drivers/dax/fsdev.c | 346 ++++++++++++++++++++++++++++++++++++++
drivers/dax/super.c | 107 +++++++++++-
fs/dax.c | 74 ++++++--
include/linux/dax.h | 19 ++-
11 files changed, 563 insertions(+), 50 deletions(-)
create mode 100644 drivers/dax/fsdev.c
base-commit: c369299895a591d96745d6492d4888259b004a9e
--
2.53.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox