* Re: [PATCH 7/7] i2c: nomadik: add support for I2C_XFER_V2 - detailed fault reporting
From: Linus Walleij @ 2026-06-24 22:56 UTC (permalink / raw)
To: Dmitry Guzman
Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel
In-Reply-To: <20260623-i2c-fault-reporting-v1-7-6db1a8aabf18@mobileye.com>
On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman
<Dmitry.Guzman@mobileye.com> wrote:
> I2C_XFER_V2 is a new API that allows I2C clients to get the detailed
> report in case of transmission failure. Previously, the only information
> returned by I2C bus controller was the error code; there was no way to
> find out how many messages or bytes in a certain message have been sent
> or received until the fault condition occurred.
>
> This commit introduces support of this feature in i2c-nomadik driver.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
I don't fully understand patch 1 but if that is fine, this is fine:
Acked-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 6/7] i2c: nomadik: add quirks max_len=2047 and no_zero_len_read
From: Linus Walleij @ 2026-06-24 22:55 UTC (permalink / raw)
To: Dmitry Guzman
Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel
In-Reply-To: <20260623-i2c-fault-reporting-v1-6-6db1a8aabf18@mobileye.com>
On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman
<Dmitry.Guzman@mobileye.com> wrote:
> In Nomadik I2c controller, register I2C_MCR has 11-bit wide LENGTH
> field. Its maximum value is 2047, so this is the maximum length of a
> single message. It is less than the common maximum I2C message length in
> I2C subsystem (8192), so define a quirk in order to report the
> unsupported message without any attempt to transfer it.
>
> Zero length reading doesn't work properly on this controller, so add
> `I2C_AQ_NO_ZERO_LEN_READ` quirk flag.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
Excellent improvements, almost a Fixes: patch.
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 5/7] i2c: nomadik: change print level for fault messages to debug
From: Linus Walleij @ 2026-06-24 22:54 UTC (permalink / raw)
To: Dmitry Guzman
Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel
In-Reply-To: <20260623-i2c-fault-reporting-v1-5-6db1a8aabf18@mobileye.com>
On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman
<Dmitry.Guzman@mobileye.com> wrote:
> i2c-nomadik driver prints error message on every faulted message. This
> is not a good practice, because in I2C a fault not always is an error,
> sometimes it is the expected result. For example, scanning bus with
> `i2cdetects` prints over 100 messages in dmesg (two messages per each
> target address).
>
> To avoid excessive prints in the log, change the print level from err to
> debug.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 4/7] i2c: nomadik: return proper fault codes
From: Linus Walleij @ 2026-06-24 22:53 UTC (permalink / raw)
To: Dmitry Guzman
Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel
In-Reply-To: <20260623-i2c-fault-reporting-v1-4-6db1a8aabf18@mobileye.com>
On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman
<Dmitry.Guzman@mobileye.com> wrote:
> I2C documentation Documentation/i2c/fault-codes.rst defines fault codes
> for different negative results in I2C transmittion. Previously,
> i2c-nomadik driver didn't implement them properly - it returned
> ETIMEDOUT on most errors and EIO on master arbitration lost.
>
> To comply with the documentation, return the proper fault codes for
> different conditions, namely:
>
> - EAGAIN if arbitration lost
> - EOVERFLOW if message is too long (>2047 bytes)
> - ENXIO if target address is not acknowledged
> - EIO on other errors detected by controller (for example, NACK on data)
> - ETIMEDOUT if driver gets timeout waiting for message completion
> without any fault condition detected by the controller (for example,
> too long message, or SDA/SCL line stuck on 0).
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 3/7] i2c: nomadik: do not try to retransmit I2C message series on errors
From: Linus Walleij @ 2026-06-24 22:51 UTC (permalink / raw)
To: Dmitry Guzman
Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel
In-Reply-To: <20260623-i2c-fault-reporting-v1-3-6db1a8aabf18@mobileye.com>
On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman
<Dmitry.Guzman@mobileye.com> wrote:
> i2c-nomadik driver of I2C bus controller in `xfer` callback retransmits
> the whole message series in cause of any fault, and returns fault only
> after third failed attempt. This behavior contradicts with API because
> not only it hides hardware faults, but also re-sends messages, while
> they are not guaranteed to be idempotent.
>
> Remove the triple attempt to send messages in `xfer` callback.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
This originally came from:
commit ebd10e0783d9fb92a147e60902e22c2d3f3ad69d
Author: Virupax Sadashivpetimath <virupax.sadashivpetimath@stericsson.com>
Date: Fri May 13 12:30:23 2011 +0200
i2c-nomadik: add code to retry on timeout failure
It is seen that i2c-nomadik controller randomly stops generating the
interrupts leading to a i2c timeout. As a workaround to this problem,
add retries to the on going transfer on failure.
Signed-off-by: Virupax Sadashivpetimath
<virupax.sadashivpetimath@stericsson.com>
Reviewed-by: Jonas ABERG <jonas.aberg@stericsson.com>
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Ben Dooks <ben-linux@fluff.org>
At that time the code looked very different:
for (j = 0; j < 3; j++) {
if (status || (dev->result)) {
(...)
break;
}
udelay(I2C_DELAY);
}
if (status == 0)
break;
We would only spin here if both status and dev->result
(the number of sent bytes) was 0. This doesn't seem to be
at all the case anymore!
I suppose it's a bit dubious code, so:
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 2/7] i2c: nomadik: optimize layout of struct nmk_i2c_dev
From: Linus Walleij @ 2026-06-24 22:36 UTC (permalink / raw)
To: Dmitry Guzman
Cc: Andi Shyti, Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
linux-i2c, linux-kernel, linux-trace-kernel, linux-arm-kernel
In-Reply-To: <20260623-i2c-fault-reporting-v1-2-6db1a8aabf18@mobileye.com>
Hi Dmitry,
thanks for your patch!
Also nice to see some kernel contributions directly from
MobilEye!
On Tue, Jun 23, 2026 at 6:32 PM Dmitry Guzman
<Dmitry.Guzman@mobileye.com> wrote:
> Put two bool variables `xfer_done` and `has_32b_bus` and two char
> variables `tft` and `rft` together in order to reduce struct size
> wasted for padding.
>
> Signed-off-by: Dmitry Guzman <Dmitry.Guzman@mobileye.com>
(...)
> struct nmk_i2c_dev {
> struct i2c_vendor_data *vendor;
> @@ -206,13 +206,13 @@ struct nmk_i2c_dev {
> u32 clk_freq;
> unsigned char tft;
> unsigned char rft;
^
Maybe you want to take the opportunity to change these
two into u8 if you're anyway changing the layout of this
struct?
Either way:
Reviewed-by: Linus Walleij <linusw@kernel.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v8 22/46] KVM: SEV: Make 'uaddr' parameter optional for KVM_SEV_SNP_LAUNCH_UPDATE
From: Ackerley Tng @ 2026-06-24 22:31 UTC (permalink / raw)
To: Fuad Tabba
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <CA+EHjTz3SW50EzxgXm8VysoaM21RReUVG2px_WUYU7zUwjXnpQ@mail.gmail.com>
Fuad Tabba <tabba@google.com> writes:
>
> [...snip...]
>
>> diff --git a/Documentation/virt/kvm/x86/amd-memory-encryption.rst b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
>> index bd04a908a8dbd..29409297f1ef0 100644
>> --- a/Documentation/virt/kvm/x86/amd-memory-encryption.rst
>> +++ b/Documentation/virt/kvm/x86/amd-memory-encryption.rst
>> @@ -503,7 +503,8 @@ secrets.
>>
>> It is required that the GPA ranges initialized by this command have had the
>> KVM_MEMORY_ATTRIBUTE_PRIVATE attribute set in advance. See the documentation
>> -for KVM_SET_MEMORY_ATTRIBUTES for more details on this aspect.
>> +for KVM_SET_MEMORY_ATTRIBUTES/KVM_SET_MEMORY_ATTRIBUTES2 for more details on
>> +this aspect.
>>
>> Upon success, this command is not guaranteed to have processed the entire
>> range requested. Instead, the ``gfn_start``, ``uaddr``, and ``len`` fields of
>> @@ -511,9 +512,13 @@ range requested. Instead, the ``gfn_start``, ``uaddr``, and ``len`` fields of
>> remaining range that has yet to be processed. The caller should continue
>> calling this command until those fields indicate the entire range has been
>> processed, e.g. ``len`` is 0, ``gfn_start`` is equal to the last GFN in the
>> -range plus 1, and ``uaddr`` is the last byte of the userspace-provided source
>> -buffer address plus 1. In the case where ``type`` is KVM_SEV_SNP_PAGE_TYPE_ZERO,
>> -``uaddr`` will be ignored completely.
>> +range plus 1, and ``uaddr`` (if specified) is the last byte of the
>> +userspace-provided source buffer address plus 1.
>> +
>> +In the case where ``type`` is KVM_SEV_SNP_PAGE_TYPE_ZERO, ``uaddr`` will be
>> +ignored completely. For all other page types, ``uaddr`` is optional if in-place
>> +conversion is enable, i.e. when the destination can also be the source, and is
>
> Typo: "is enable" -> "is enabled".
>
> "when the destination can also be the source" is hard to parse without
> context. Maybe: "i.e. when the data has been written directly to
> guest_memfd while the range was in the shared state".
>
> Also, how does userspace discover whether in-place conversion is
> enabled? A cross-reference to KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES
> would help here.
>
Will fix in the next revision. Thanks!
> Cheers,
> /fuad
>
>>
>> [...snip...]
>>
^ permalink raw reply
* Re: [PATCH v8 23/46] KVM: TDX: Make source page optional for KVM_TDX_INIT_MEM_REGION
From: Sean Christopherson @ 2026-06-24 22:31 UTC (permalink / raw)
To: Yan Zhao
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, willy, wyihan, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajpGxu2uQys+S2F8@yzhao56-desk.sh.intel.com>
On Tue, Jun 23, 2026, Yan Zhao wrote:
> On Tue, Jun 23, 2026 at 01:16:14PM +0800, Yan Zhao wrote:
> > On Mon, Jun 22, 2026 at 06:22:45PM -0700, Sean Christopherson wrote:
> > > On Mon, Jun 22, 2026, Yan Zhao wrote:
> > > > On Thu, Jun 18, 2026 at 05:32:00PM -0700, Ackerley Tng via B4 Relay wrote:
> > > > > diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> > > > > index ffe9d0db58c59..56d10333c61a7 100644
> > > > > --- a/arch/x86/kvm/vmx/tdx.c
> > > > > +++ b/arch/x86/kvm/vmx/tdx.c
> > > > > @@ -3198,8 +3198,12 @@ static int tdx_gmem_post_populate(struct kvm *kvm, gfn_t gfn, kvm_pfn_t pfn,
> > > > > if (KVM_BUG_ON(kvm_tdx->page_add_src, kvm))
> > > > > return -EIO;
> > > > >
> > > > > - if (!src_page)
> > > > > - return -EOPNOTSUPP;
> > > > > + if (!src_page) {
> > > > > + if (!gmem_in_place_conversion)
> > > > When userspace turns on gmem_in_place_conversion while creating guest_memfd
> > > > without the MMAP flag, the absence of src_page should still be treated as an
> > > > error.
> > >
> > > Why MMAP?
> > Hmm, I was showing a scenario that in-place conversion couldn't occur.
> > I didn't mean that with the MMAP flag, mmap() and user write must occur.
> >
> > > Shouldn't this be a general "if (!src_page && !up-to-date)"? Just
> > > because userspace _can_ mmap() the memory doesn't mean userspace _has_ mmap()'d
> > > and written memory. And when write() lands, MMAP wouldn't be necessary to
> > > initialize the memory.
> > Do you mean using up-to-date flag as below?
Yes? I didn't actually look at the implementation details.
> > if (!src_page) {
> > src_page = pfn_to_page(pfn);
> > if (!folio_test_uptodate(page_folio(src_page)))
> > return -EOPNOTSUPP;
> > }
>
> Another concern with this fix is that:
> commit "KVM: guest_memfd: Zero page while getting pfn" [1] always marks the
> folio uptodate before reaching post_populate().
>
> [1] https://lore.kernel.org/all/20260618-gmem-inplace-conversion-v8-21-9d2959357853@google.com/
>
> > One concern is that TDX now does not much care about the up-to-date flag since
> > TDX doesn't rely on the flag to clear pages on conversions.
> > I'm not sure if the flag can be reliably checked in this case. e.g.,
> > now the whole folio is marked up-to-date even if only part of it is faulted by
> > user access.
> > Ensuring that the up-to-date flag works correctly with huge page support seems
> > to have more effort than introducing a dedicated flag for TDX.
> >
> > > > Additionally, to properly enable in-place copying for the TDX initial memory
> > > > region, userspace must not only specify source_addr to NULL, but also follow
> > > > a specific sequence (where steps 1/2/3/7 are required only for in-place copy):
> > > > 1. create guest_memfd with MMAP flag
> > > > 2. mmap the guest_memfd.
> > > > 3. convert the initial memory range to shared.
> > > > 4. copy initial content to the source page.
> > > > 5. convert the initial memory range to private
> > > > 6. invoke ioctl KVM_TDX_INIT_MEM_REGION.
> > > > 7. do not unmap the source backend.
> > > >
> > > > So, would it be reasonable to introduce a dedicated flag that allows userspace
> > > > to explicitly opt into the in-place copy functionality? e.g.,
> > >
> > > Why? It's userspace's responsibility to get the above right. If userspace fails
> > > to provide a src_page when it doesn't want in-place copy, that's a userspace bug.
> > I mean if userspace specifies a NULL source_addr by mistake, it's better for
> > kernel to detect this mistake, similar to how it validates whether source_addr
> > is PAGE_ALIGNED.
The alignment case is different. If userspace provides an unaligned value, KVM
*can't* do what userspace is asking because hardware and thus KVM only supports
converting on page boundaries.
For a NULL source, KVM can still do what userspace is asking. Rejecting userspace's
request would then be making assumptions about what userspace wants.
> > Since userspace already needs to perform additional steps to enable in-place
> > copy, specifying a dedicated flag to indicate that the NULL source_addr is
> > intentional seems like a reasonable burden.
I don't see how it adds any value. I wouldn't be at all surprised if most VMMs
just wen up with code that does:
if (in-place) {
src = NULL;
flags |= KVM_TDX_IN_PLACE_COPY_INITIAL_MEMORY_REGION;
}
^ permalink raw reply
* Re: [PATCH v8 21/46] KVM: guest_memfd: Zero page while getting pfn
From: Ackerley Tng @ 2026-06-24 22:30 UTC (permalink / raw)
To: Yan Zhao
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, forkloop, pratyush, suzuki.poulose, aneesh.kumar, liam,
Paolo Bonzini, Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajpKK/SyRh8LExrY@yzhao56-desk.sh.intel.com>
Yan Zhao <yan.y.zhao@intel.com> writes:
> On Thu, Jun 18, 2026 at 05:31:58PM -0700, Ackerley Tng via B4 Relay wrote:
>> From: Ackerley Tng <ackerleytng@google.com>
>>
>> Move the folio initialization logic from kvm_gmem_get_pfn() into
>> __kvm_gmem_get_pfn() to also zero pages if the page is to be used in
>> kvm_gmem_populate().
>>
>> With in-place conversion, the existing data in a guest_memfd page can be
>> populated into guest memory through platform-specific ioctls.
>>
>> Without first zeroing the page obtained using __kvm_gmem_get_pfn(), it
>> might contain uninitialized host memory, which would leak to the guest if
>> the populate completes.
>>
>> guest_memfd pages are zeroed at most once in the page's entire lifetime
>> with guest_memfd, and that is tracked using the uptodate flag.
>>
>> Zeroing the page in __kvm_gmem_get_pfn() is chosen over zeroing in
>> kvm_gmem_get_folio() since other flows, such as a future write() syscall,
>> can get a page, write to the page and then set page uptodate without
>> zeroing.
>>
>> This aligns with the concept of zeroing before first use - the other place
>> where zeroing happens is in kvm_gmem_fault_user_mapping().
>>
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> ---
>> virt/kvm/guest_memfd.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index 90bc1a26512b6..86c9f5b0863cb 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -1137,6 +1137,11 @@ static struct folio *__kvm_gmem_get_pfn(struct file *file,
>> return ERR_PTR(-EHWPOISON);
>> }
>>
>> + if (!folio_test_uptodate(folio)) {
>> + clear_highpage(folio_page(folio, 0));
>> + folio_mark_uptodate(folio);
>> + }
> Note:
> In the __kvm_gmem_populate() path, this folio_mark_uptodate() call makes the
> later one after post_populate() pointless.
>
> __kvm_gmem_populate
> |1.__kvm_gmem_get_pfn
> | |->folio = kvm_gmem_get_folio()
> | | if (!folio_test_uptodate(folio))
> | | folio_mark_uptodate(folio);
> |2. ret = post_populate()
> |3. if (!ret)
> | folio_mark_uptodate(folio);
>
Good point! I'll remove the folio_mark_uptodate() in the populate path
then. Thanks!
>>
>> [...snip...]
>>
^ permalink raw reply
* Re: [PATCH v8 18/46] KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
From: Ackerley Tng @ 2026-06-24 22:25 UTC (permalink / raw)
To: Sean Christopherson
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajwMYCSrPlxg-Fok@google.com>
Sean Christopherson <seanjc@google.com> writes:
> On Thu, Jun 18, 2026, Ackerley Tng wrote:
>> When checking if a guest_memfd folio is safe for conversion, its refcount
>> is examined. A folio may be present in a per-CPU lru_add fbatch, which
>> temporarily increases its refcount.
>
> Under what circumstances does this happen,
It happened 100% of the time in selftests. Perhaps it's because in the
selftests the pages are almost always freshly allocated and so the
lru_add fbatch isn't full yet? (and that the host isn't super busy so
lru_add fbatch doesn't get drained yet).
I've not tested without this beyond selftests.
I don't think we can depend on workloads to drain the lru_add fbatch?
> and what alternatives are there for
> userspace to work around the issue?
The thing is, the refcounts don't come with a label of who added the
refcount so we can't really return a different error for lru_add fbatch
presence. All folios get added to the lru_add fbatch even if they're
unevictable and eventually not participate in LRU.
We could make userspace try fadvise(POSIX_FADV_DONTNEED)? I think that
has other problems, and this kind of makes userspace have one more user
to guess. Userspace already needs to check if the page is pinned for
DMA, and if it's not pinned for DMA, userspace already needs to retry
because of other possible kernel users...
^ permalink raw reply
* Re: [PATCH v8 15/46] KVM: guest_memfd: Call arch invalidate hooks on conversion
From: Suzuki K Poulose @ 2026-06-24 22:15 UTC (permalink / raw)
To: Ackerley Tng, Sean Christopherson, Fuad Tabba
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <CAEvNRgGX3GkazCWM=6y9YLgn=YemXuG==Oo+L58cac1Fd86_TQ@mail.gmail.com>
On 24/06/2026 18:46, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
>
>> On Fri, Jun 19, 2026, Fuad Tabba wrote:
>>> On Fri, 19 Jun 2026 at 01:31, Ackerley Tng via B4 Relay
>>> <devnull+ackerleytng.google.com@kernel.org> wrote:
>>>>
>>>> From: Ackerley Tng <ackerleytng@google.com>
>>>>
>>>> When memory in guest_memfd is converted from private to shared, the
>>>> platform-specific state associated with the guest-private pages must be
>>>> invalidated or cleaned up.
>>>>
>>>> Iterate over the folios in the affected range and call the
>>>> kvm_arch_gmem_invalidate() hook for each PFN range. This allows
>>>> architectures to perform necessary teardown, such as updating hardware
>>>> metadata or encryption states, before the pages are transitioned to the
>>>> shared state.
>>>>
>>>> Invoke this helper after indicating to KVM's mmu code that an invalidation
>>>> is in progress to stop in-flight page faults from succeeding.
>>>>
>>>> Reviewed-by: Fuad Tabba <tabba@google.com>
>>>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>>>
>>> Coming back to this after working through the arm64/pKVM side. My
>>> Reviewed-by here is from the previous round and the patch hasn't
>>> changed, but I missed an implication for arm64.
>>>
>>> kvm_arch_gmem_invalidate() is now called from two paths with the same
>>> (start, end) signature: folio teardown (kvm_gmem_free_folio) and
>>> private->shared conversion (here). For SNP/TDX that's fine, conversion is
>>> destructive anyway. For pKVM the two need opposite content semantics:
>>> conversion must preserve the page in place (same physical page, the point
>>> of in-place conversion without encryption), while teardown must scrub it
>>> before returning it to the host.
>>>
>>> The hook gets only a pfn range with no indication of which caller it's
>>> serving, so arm64 can't give the two paths the behaviour they need. It
>>> would help to signal intent on the conversion path: a reason/flag, a
>>> separate hook, or not routing non-destructive conversion through the
>>> teardown hook.
>>>
>>> arm64 isn't here yet, so this isn't urgent, but the hook is gaining a
>>> second caller now, and it's cheaper to leave room for the distinction
>>> than to change a generic contract other arches depend on later.
>>
>> Crud. It may not be urgent for arm64, but it's urgent for other reasons that
>> I "can't" describe in detail at the moment, and even if that weren't the case, I
>> think we should clean things up now. More below.
>>
>>>> virt/kvm/guest_memfd.c | 41 +++++++++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 41 insertions(+)
>>>>
>>>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>>>> index 433f79047b9d1..3c94442bc8131 100644
>>>> --- a/virt/kvm/guest_memfd.c
>>>> +++ b/virt/kvm/guest_memfd.c
>>>> @@ -607,6 +607,42 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
>>>> return safe;
>>>> }
>>>>
>>>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
>>>> +static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
>>
>> Not your fault, but kvm_arch_gmem_invalidate() is badly misnamed. It's not
>> "invalidating" anything, it's much more of a "free" callback, as SNP uses it to
>> put physical pages back into a shared state when a maybe-private folio is freed.
>>
>> As Fuad points out, (ab)using that hook for the private=>shared conversion case
>> "works", but not broadly. And it makes the bad name worse, because it's called
>> from code that _is_ doing true invalidations. For pKVM, it may not even need to
>> do anything invalidation-like.
>>
>
> Thanks, I also didn't like the naming of kvm_gmem_invalidate(),
> especially when conversions also calls
> kvm_gmem_invalidate_{start,end}() and those do different things.
>
>> To avoid a conflict with patches that are going to have priority over this series,
>> to set the stage for arm64 support, and to avoid avoid bleeding vendor details
>> into guest_memfd, as if they are core guest_memfd behavior (only SNP needs the
>> "invalidation" on this specific transition), I think we should add an arch hook
>> to do conversions straightaway.
>>
>> Unless there's a clever option I'm missing, it'll mean adding yet another
>> HAVE_KVM_ARCH_GMEM_XXX flag? Hmm, especially because IIUC, arm64/pKVM doesn't
>> need a callback for this case, only the free_folio case.
>>
>>>> +{
>>>> + struct folio_batch fbatch;
>>>> + pgoff_t next = start;
>>>> + int i;
>>>> +
>>>> + folio_batch_init(&fbatch);
>>>> + while (filemap_get_folios(inode->i_mapping, &next, end - 1, &fbatch)) {
>>>> + for (i = 0; i < folio_batch_count(&fbatch); ++i) {
>>>> + struct folio *folio = fbatch.folios[i];
>>>> + pgoff_t start_index, end_index;
>>>> + kvm_pfn_t start_pfn, end_pfn;
>>>> +
>>>> + start_index = max(start, folio->index);
>>>> + end_index = min(end, folio_next_index(folio));
>>>> + /*
>>>> + * end_index is either in folio or points to
>>>> + * the first page of the next folio. Hence,
>>>> + * all pages in range [start_index, end_index)
>>>> + * are contiguous.
>>>> + */
>>>> + start_pfn = folio_file_pfn(folio, start_index);
>>>> + end_pfn = start_pfn + end_index - start_index;
>>>> +
>>>> + kvm_arch_gmem_invalidate(start_pfn, end_pfn);
>>>> + }
>>>> +
>>>> + folio_batch_release(&fbatch);
>>>> + cond_resched();
>>>> + }
>>>> +}
>>>> +#else
>>>> +static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end) {}
>>>> +#endif
>>>> +
>>>> static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>>>> size_t nr_pages, uint64_t attrs,
>>>> pgoff_t *err_index)
>>>> @@ -647,7 +683,12 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>>>> */
>>>>
>>>> kvm_gmem_invalidate_start(inode, start, end);
>>>> +
>>>> + if (!to_private)
>>>> + kvm_gmem_invalidate(inode, start, end);
>>
>> E.g. instead make this something like this?
>>
>> kvm_gmem_set_pfn_attributes(...)
>>
>> Hrm, though that wastes folio lookups in the to_private case. So maybe just this,
>> assuming pKVM doesn't need to take additional action on conversions?
>>
>> if (!to_private)
>> kvm_gmem_make_shared(...)
>>
>> Actually, if we do that, then we don't need a separate arch hook, just a separate
>> config. It'll still bleed SNP details into guest_memfd, but it'll at least be
>> done in a way that's more explicitly arch specific (and it's no different than
>> what we already do for PREPARE...).
>>
>
> pKVM needs some arch guest_memfd lifecycle functions that
>
> + for conversion, doesn't do anything,
> + for teardown, resets page state (IIUC it'll be reset to
> PKVM_PAGE_OWNED (by the host))
>
> So I think we need different functions for those two stages in the
> lifecycle of a page with guest_memfd? What if we have
>
> CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES, which gates
>
> + kvm_gmem_should_set_pfn_attributes(attributes) and
> .gmem_should_set_pfn_attributes
> + kvm_gmem_set_pfn_attributes(start_pfn, end_pfn, attributes) and
> .gmem_set_pfn_attributes
>
> CONFIG_HAVE_KVM_ARCH_GMEM_TEARDOWN, which gates
>
> + kvm_gmem_teardown() and .gmem_teardown
>
> SNP:
>
> + .gmem_should_set_pfn_attributes = sev_gmem_should_set_pfn_attributes,
> and sev_gmem_should_set_pfn_attributes returns !is_private
> + Rename .gmem_invalidate and sev_gmem_invalidate to *set_pfn_attributes
> + .gmem_teardown = sev_gmem_set_pfn_attributes
>
> TDX:
>
> + Disable CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES
> + Disable CONFIG_HAVE_KVM_ARCH_GMEM_TEARDOWN
>
> pKVM:
>
> + Disable CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES
> + .gmem_teardown = pkvm_gmem_set_pfn_attributes
>
> Suzuki, does this work for ARM CCA?
Yep, that works for us. For CCA we would :
+ Disable CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES
+ Disable CONFIG_HAVE_KVM_ARCH_GMEM_TEARDOWN
In the future we might utilise the gmem_set_pfn_attributes call back.
Thanks
Suzuki
>
> This way,
>
> + The if (is_private) check doesn't leak SNP details into guest_memfd
> + .gmem_make_shared doesn't stick out without a .gmem_make_private
> + .gmem_set_pfn_attributes, .gmem_prepare and .gmem_teardown are aligned
> conceptually as lifecycle hooks
>
> + I think the private/shared check for prepare can also be folded into
> preparation.
> + Preparation perhaps doesn't need a should_prepare equivalent since
> there's no iteration and getting the gfn is just doing some math?
> + In another patch series?
>
>> E.g. this? There will still be a looming rename conflict, but that's easy enough
>> to handle.
>>
>> diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
>> index 9ce5be7843f2..8aead0abd788 100644
>> --- virt/kvm/guest_memfd.c
>> +++ virt/kvm/guest_memfd.c
>> @@ -648,8 +648,8 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
>> return safe;
>> }
>>
>> -#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
>> -static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
>> +#ifdef CONFIG_KVM_ARCH_GMEM_FREE_ON_SHARED_CONVERSION
>> +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end)
>> {
>> struct folio_batch fbatch;
>> pgoff_t next = start;
>> @@ -681,7 +681,7 @@ static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
>> }
>> }
>> #else
>> -static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end) {}
>> +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end) { }
>> #endif
>>
>> static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> @@ -729,7 +729,7 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> kvm_gmem_invalidate_start(inode, start, end);
>>
>> if (!to_private)
>> - kvm_gmem_invalidate(inode, start, end);
>> + kvm_gmem_make_shared(inode, start, end);
>>
>> mas_store_prealloc(&mas, xa_mk_value(attrs));
^ permalink raw reply
* Re: [PATCH v8 13/46] KVM: guest_memfd: Add base support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-06-24 21:10 UTC (permalink / raw)
To: Binbin Wu
Cc: aik, andrew.jones, brauner, chao.p.peng, david, jmattson,
jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <ede86ac4-d560-49a6-82d6-b33ac5fc9355@linux.intel.com>
Binbin Wu <binbin.wu@linux.intel.com> writes:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
>> From: Ackerley Tng <ackerleytng@google.com>
>>
>> Introduce base support for KVM_SET_MEMORY_ATTRIBUTES2 in guest_memfd, which
>> just updates attributes tracked by guest_memfd.
>>
>> Validate input fields in general. Guard usage of KVM_SET_MEMORY_ATTRIBUTES2
>> by making sure requested attributes are supported for this instance of kvm.
>>
>> A new KVM_SET_MEMORY_ATTRIBUTES2 is defined to support writes (unlike
>> KVM_SET_MEMORY_ATTRIBUTES) in addition to reads so it can provide error
>> details to userspace. This will be used in a later patch.
>>
>> The two ioctls use their corresponding structs with no overlap, but
>> backward compatibility is baked in for future support of
>> KVM_SET_MEMORY_ATTRIBUTES2 and struct kvm_memory_attributes2 in the VM
>> ioctl.
>>
>> The process of setting memory attributes is set up such that the later half
>> will not fail due to allocation. Any necessary checks are performed before
>> the point of no return.
>>
>> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
>> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
>> Co-developed-by: Sean Christoperson <seanjc@google.com>
>> Signed-off-by: Sean Christoperson <seanjc@google.com>
>
> s/Christoperson /Christopherson
>
Thanks!
>> Reviewed-by: Fuad Tabba <tabba@google.com>
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>> ---
>> include/uapi/linux/kvm.h | 13 ++++++
>> virt/kvm/Kconfig | 1 +
>> virt/kvm/guest_memfd.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++
>> virt/kvm/kvm_main.c | 12 +++++
>> 4 files changed, 142 insertions(+)
>>
>>
>
> [...]
>
>> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
>> index 297e4399fbd49..cfa2c78ba5fb9 100644
>> --- a/virt/kvm/Kconfig
>> +++ b/virt/kvm/Kconfig
>> @@ -102,6 +102,7 @@ config KVM_MMU_LOCKLESS_AGING
>>
>> config KVM_GUEST_MEMFD
>> select XARRAY_MULTI
>> + select KVM_MEMORY_ATTRIBUTES
>
> What's this?
> This config is gone.
>
I'm surprised this compiles... I'll fix it, thanks!
>> bool
>>
^ permalink raw reply
* Re: [PATCH v8 13/46] KVM: guest_memfd: Add base support for KVM_SET_MEMORY_ATTRIBUTES2
From: Ackerley Tng @ 2026-06-24 21:03 UTC (permalink / raw)
To: Fuad Tabba, Sean Christopherson
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CA+EHjTwLPCvZJgPv=8u3pgp+kwEwQbsXn_13FL3xUbJ7HRfXzw@mail.gmail.com>
Fuad Tabba <fuad.tabba@linux.dev> writes:
>
> [...snip...]
>
>> >
>> > Note sure if it's user error on my part, if I'm applying this to the
>> > wrong base, but I found a build break here on patch 13:
>> > kvm_gmem_invalidate_start() doesn't exist in the base tree. The
>> > function is kvm_gmem_invalidate_begin() here. The rename
>> > (190cc5370a8b6) landed via a different merge path and isn't an
>> > ancestor of the stated base.
>> >
>> > Patches 19 and 20 have the same mismatch. Fix for all three is
>> > s/kvm_gmem_invalidate_start/kvm_gmem_invalidate_begin/.
I took Sean's patches (off-list) and tried to combine it onto my
existing state. (I'm using b4 [1] to manage these series and I didn't
know I had to manually update the base-commit. Will try again next
revision.
[1] https://b4.docs.kernel.org/en/latest/
>>
>> Ya, Ackerley used a slightly older kvm/next to send the patches. I at least was
>> testing against kvm-x86/next, which does have the rename.
>>
>> Other than noting that this should be applied against the current kvm/next, I
>> don't think there's anything else to be done?
Should I base v9 on kvm/next, or kvm-x86/next?
>
> Agree. Sorry, didn't mean to be nit-picky, but this really threw me off :)
>
> Cheers,
> /fuad
^ permalink raw reply
* Re: [PATCH v8 10/46] KVM: guest_memfd: Wire up core private/shared attribute interfaces
From: Ackerley Tng @ 2026-06-24 20:44 UTC (permalink / raw)
To: Binbin Wu
Cc: aik, andrew.jones, brauner, chao.p.peng, david, jmattson,
jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <2ef455c3-a3f5-4ba1-86ea-b96416d163ce@linux.intel.com>
Binbin Wu <binbin.wu@linux.intel.com> writes:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
>
> [...]
>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index bca912db5be6e..e0e544ef47d69 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -926,6 +926,24 @@ int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
>> EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_get_pfn);
>>
>> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_POPULATE
>> +static bool kvm_gmem_range_is_private(struct file *file, pgoff_t index,
>> + size_t nr_pages, struct kvm *kvm, gfn_t gfn)
>> +{
>> + struct maple_tree *mt = &GMEM_I(file_inode(file))->attributes;
>> + pgoff_t end = index + nr_pages - 1;
>> + void *entry;
>> +
>> + if (!gmem_in_place_conversion)
>> + return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages,
>> + KVM_MEMORY_ATTRIBUTE_PRIVATE,
>> + KVM_MEMORY_ATTRIBUTE_PRIVATE);
>> +
>> + mt_for_each(mt, entry, index, end) {
>> + if (xa_to_value(entry) != KVM_MEMORY_ATTRIBUTE_PRIVATE)
>> + return false;
>> + }
>
> Patch 1 noted that "Ensuring every index is represented in the maple tree at all times".
> So I think the queried range should not be a hole in the maple tree.
> However, there is a inconsistency: in patch 1 kvm_gmem_get_attributes() explicitly
> checks for holes, but this patch does not.
>
>> + return true;
>> +}
>>
With Sean's suggestion for patch 1, I'll update this one to default to
the "init" state if xa_to_value(entry) is NULL.
Thanks!
^ permalink raw reply
* Re: [PATCH] tracing/user_events: fix use-after-free of enabler in user_event_mm_dup()
From: Beau Belgrave @ 2026-06-24 20:05 UTC (permalink / raw)
To: XIAO WU
Cc: Michael Bommarito, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, linux-trace-kernel, linux-kernel, stable
In-Reply-To: <tencent_89647CE40DC452B891C65C94D1B271DE8E07@qq.com>
On Tue, Jun 23, 2026 at 01:03:59AM +0800, XIAO WU wrote:
> Hi,
>
Hey XIAO WU,
> I came across the Sashiko AI review [1] in this thread and wanted to
> share some test results that may be useful.
>
Thanks!
> First — thank you for this patch! The enabler UAF in
> user_event_mm_dup() is a real bug and the fix (kfree → kfree_rcu) is
> the right approach for protecting the RCU list walkers. The selftest
> results you included in the commit are also really helpful.
>
> However, I was able to reproduce a second UAF on the *user_event*
> object that the Sashiko review flagged — it's still reachable after the
> patch is applied. I've included a PoC and crash log below.
>
> On Thu, Jun 18, 2026 at 06:27:43PM -0400, Michael Bommarito wrote:
> > @@ -404,7 +407,12 @@ static void user_event_enabler_destroy(struct
> user_event_enabler *enabler,
> > /* No longer tracking the event via the enabler */
> > user_event_put(enabler->event, locked);
> >
> > - kfree(enabler);
> > + /*
> > + * The enabler is removed from an RCU-traversed list
> > + * (user_event_mm_dup walks mm->enablers under rcu_read_lock only),
> > + * so the backing memory must outlive a grace period.
> > + */
> > + kfree_rcu(enabler, rcu);
> > }
>
> The issue: user_event_put(enabler->event, locked) is called
> synchronously, before kfree_rcu(enabler, rcu). If this drops the last
> reference to the user_event, delayed_destroy_user_event() is scheduled
> on a workqueue, which calls destroy_user_event() → kfree(user). The
> user_event memory is freed without RCU protection.
>
> But the enabler itself is now protected by kfree_rcu — it remains
> visible to RCU readers in user_event_mm_dup() during fork(). Those
> readers access enabler->event (via user_event_enabler_dup →
> user_event_get(orig->event)), which now points to freed memory:
>
> fork() unregister
> ──────── ──────────
> user_event_mm_dup()
> rcu_read_lock();
> list_for_each_entry_rcu(enabler, ...)
> user_event_enabler_destroy()
> list_del_rcu(enabler)
> user_event_put(enabler->event)
> → last ref!
> → schedule_work(put_work)
> kfree_rcu(enabler, rcu)
> user_event_enabler_dup(enabler, ...) [workqueue]
> enabler->event = delayed_destroy_user_event()
> user_event_get(orig->event); destroy_user_event()
> ↑ UAF: orig->event was freed! kfree(user_event)
>
While I cannot repro this locally on my 16 core machine, I do agree this
case needs to be handled correctly. The enabler should keep the ref to
the user_event until after an RCU grace period. I have this fix that
addresses it more completely than the original proposal.
I'm hoping you can try out this fix with your machine that does repro
the timing window. The below change needs self test fixes, since now the
free happens after an RCU grace period + work queue schedule. This is
because the self tests (abi_test and perf_test) assume after unreg the
last ref is immediate (which was never guaranteed).
Thanks,
-Beau
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index c4ba484f7b38..b860d8b70c7b 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -109,6 +109,9 @@ struct user_event_enabler {
/* Track enable bit, flags, etc. Aligned for bitops. */
unsigned long values;
+
+ /* Defer put so RCU list readers (user_event_mm_dup) are safe. */
+ struct rcu_work put_rwork;
};
/* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
@@ -396,17 +399,38 @@ static struct user_event_group *user_event_group_create(void)
return NULL;
};
-static void user_event_enabler_destroy(struct user_event_enabler *enabler,
- bool locked)
+static void delayed_user_event_enabler_put(struct work_struct *work)
{
- list_del_rcu(&enabler->mm_enablers_link);
+ struct user_event_enabler *enabler;
+
+ enabler = container_of(to_rcu_work(work), struct user_event_enabler, put_rwork);
/* No longer tracking the event via the enabler */
- user_event_put(enabler->event, locked);
+ user_event_put(enabler->event, false);
+ /* Run from queue_rcu_work(), no need for RCU */
kfree(enabler);
}
+static void user_event_enabler_destroy(struct user_event_enabler *enabler)
+{
+ list_del_rcu(&enabler->mm_enablers_link);
+
+ /*
+ * We need to hold onto the reference of the user_event for this enabler
+ * until an RCU grace period has elapsed. This ensures that we only ever
+ * put (which may free) the user_event after all CPUs have an updated
+ * enabler list. If during the RCU grace period more enablers are added,
+ * the user_event will be kept alive by new ref counts.
+ *
+ * If user_event_put() is called on the last reference, the event_mutex
+ * is taken. These cannot be taken in an RCU context, so we have to run
+ * this in a work queue only after an RCU grace period.
+ */
+ INIT_RCU_WORK(&enabler->put_rwork, delayed_user_event_enabler_put);
+ queue_rcu_work(system_percpu_wq, &enabler->put_rwork);
+}
+
static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr,
int attempt)
{
@@ -464,7 +488,7 @@ static void user_event_enabler_fault_fixup(struct work_struct *work)
/* User asked for enabler to be removed during fault */
if (test_bit(ENABLE_VAL_FREEING_BIT, ENABLE_BITOPS(enabler))) {
- user_event_enabler_destroy(enabler, true);
+ user_event_enabler_destroy(enabler);
goto out;
}
@@ -764,7 +788,7 @@ static void user_event_mm_destroy(struct user_event_mm *mm)
struct user_event_enabler *enabler, *next;
list_for_each_entry_safe(enabler, next, &mm->enablers, mm_enablers_link)
- user_event_enabler_destroy(enabler, false);
+ user_event_enabler_destroy(enabler);
mmdrop(mm->mm);
kfree(mm);
@@ -2645,7 +2669,7 @@ static long user_events_ioctl_unreg(unsigned long uarg)
flags |= enabler->values & ENABLE_VAL_COMPAT_MASK;
if (!test_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler)))
- user_event_enabler_destroy(enabler, true);
+ user_event_enabler_destroy(enabler);
/* Removed at least one */
ret = 0;
^ permalink raw reply related
* Re: [PATCH v8 32/46] KVM: selftests: Test conversion flow when INIT_SHARED
From: Fuad Tabba @ 2026-06-24 19:55 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-32-9d2959357853@google.com>
On Fri, 19 Jun 2026 at 01:31, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> Add a test case to verify that conversions between private and shared
> memory work correctly when the memory is initially created as shared.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> .../testing/selftests/kvm/x86/guest_memfd_conversions_test.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c b/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c
> index 8e09e241723e5..5b070d3374eae 100644
> --- a/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c
> +++ b/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c
> @@ -95,6 +95,12 @@ static void __gmem_conversions_##test(test_data_t *t, int nr_pages) \
> #define GMEM_CONVERSION_TEST_INIT_PRIVATE(test) \
> __GMEM_CONVERSION_TEST_INIT_PRIVATE(test, 1)
>
> +#define __GMEM_CONVERSION_TEST_INIT_SHARED(test, __nr_pages) \
> + GMEM_CONVERSION_TEST(test, __nr_pages, GUEST_MEMFD_FLAG_INIT_SHARED)
> +
> +#define GMEM_CONVERSION_TEST_INIT_SHARED(test) \
> + __GMEM_CONVERSION_TEST_INIT_SHARED(test, 1)
> +
> struct guest_check_data {
> void *mem;
> char expected_val;
> @@ -186,6 +192,12 @@ GMEM_CONVERSION_TEST_INIT_PRIVATE(init_private)
> test_convert_to_private(t, 0, 'C', 'E');
> }
>
> +GMEM_CONVERSION_TEST_INIT_SHARED(init_shared)
> +{
> + test_shared(t, 0, 0, 'A', 'B');
> + test_convert_to_private(t, 0, 'B', 'C');
> + test_convert_to_shared(t, 0, 'C', 'D', 'E');
> +}
>
> int main(int argc, char *argv[])
> {
>
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>
>
^ permalink raw reply
* Re: [PATCH v8 31/46] KVM: selftests: Test basic single-page conversion flow
From: Fuad Tabba @ 2026-06-24 19:45 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-31-9d2959357853@google.com>
On Fri, 19 Jun 2026 at 01:32, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> Add a selftest for the guest_memfd memory attribute conversion ioctls.
> The test starts the guest_memfd as all-private (the default state), and
> verifies the basic flow of converting a single page to shared and then back
> to private.
>
> Add infrastructure that supports extensions to other conversion flow
> tests. This infrastructure will be used in upcoming patches for other
> conversion tests.
>
> Add test as an x86-specific test since guest_memfd's testing
> vehicle (KVM_X86_SW_PROTECTED_VM) is x86-specific.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> tools/testing/selftests/kvm/Makefile.kvm | 1 +
> .../kvm/x86/guest_memfd_conversions_test.c | 199 +++++++++++++++++++++
> 2 files changed, 200 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
> index 4ace12606e937..b0e64a6dde21a 100644
> --- a/tools/testing/selftests/kvm/Makefile.kvm
> +++ b/tools/testing/selftests/kvm/Makefile.kvm
> @@ -152,6 +152,7 @@ TEST_GEN_PROGS_x86 += x86/max_vcpuid_cap_test
> TEST_GEN_PROGS_x86 += x86/triple_fault_event_test
> TEST_GEN_PROGS_x86 += x86/recalc_apic_map_test
> TEST_GEN_PROGS_x86 += x86/aperfmperf_test
> +TEST_GEN_PROGS_x86 += x86/guest_memfd_conversions_test
> TEST_GEN_PROGS_x86 += access_tracking_perf_test
> TEST_GEN_PROGS_x86 += coalesced_io_test
> TEST_GEN_PROGS_x86 += dirty_log_perf_test
> diff --git a/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c b/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c
> new file mode 100644
> index 0000000000000..8e09e241723e5
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/guest_memfd_conversions_test.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (c) 2024, Google LLC.
> + */
> +#include <sys/mman.h>
> +#include <unistd.h>
> +
> +#include <linux/align.h>
> +#include <linux/kvm.h>
> +#include <linux/sizes.h>
> +
> +#include "kvm_util.h"
> +#include "kselftest_harness.h"
> +#include "test_util.h"
> +#include "ucall_common.h"
> +
> +FIXTURE(gmem_conversions) {
> + struct kvm_vcpu *vcpu;
> + int gmem_fd;
> + /* HVA of the first byte of the memory mmap()-ed from gmem_fd. */
> + char *mem;
> +};
> +
> +typedef FIXTURE_DATA(gmem_conversions) test_data_t;
> +
> +FIXTURE_SETUP(gmem_conversions) { }
> +
> +static size_t page_size;
> +
> +static void guest_do_rmw(void);
> +#define GUEST_MEMFD_SHARING_TEST_GVA 0x90000000ULL
> +
> +/*
> + * Defer setup until the individual test is invoked so that tests can specify
> + * the number of pages and flags for the guest_memfd instance.
> + */
> +static void gmem_conversions_do_setup(test_data_t *t, int nr_pages,
> + int gmem_flags)
> +{
> + const struct vm_shape shape = {
> + .mode = VM_MODE_DEFAULT,
> + .type = KVM_X86_SW_PROTECTED_VM,
> + };
> + /*
> + * Use high GPA above APIC_DEFAULT_PHYS_BASE to avoid clashing with
> + * APIC_DEFAULT_PHYS_BASE.
> + */
> + const gpa_t gpa = SZ_4G;
> + const u32 slot = 1;
> + struct kvm_vm *vm;
> +
> + vm = __vm_create_shape_with_one_vcpu(shape, &t->vcpu, nr_pages, guest_do_rmw);
> +
> + vm_mem_add(vm, VM_MEM_SRC_SHMEM, gpa, slot, nr_pages,
> + KVM_MEM_GUEST_MEMFD, -1, 0, gmem_flags);
> +
> + t->gmem_fd = kvm_slot_to_fd(vm, slot);
> + t->mem = addr_gpa2hva(vm, gpa);
> + virt_map(vm, GUEST_MEMFD_SHARING_TEST_GVA, gpa, nr_pages);
> +}
> +
> +static void gmem_conversions_do_teardown(test_data_t *t)
> +{
> + /* No need to close gmem_fd, it's owned by the VM structure. */
> + kvm_vm_free(t->vcpu->vm);
> +}
> +
> +FIXTURE_TEARDOWN(gmem_conversions)
> +{
> + gmem_conversions_do_teardown(self);
> +}
> +
> +/*
> + * In these test definition macros, __nr_pages and nr_pages is used to set up
> + * the total number of pages in the guest_memfd under test. This will be
> + * available in the test definitions as nr_pages.
> + */
> +
> +#define __GMEM_CONVERSION_TEST(test, __nr_pages, flags) \
> +static void __gmem_conversions_##test(test_data_t *t, int nr_pages); \
> + \
> +TEST_F(gmem_conversions, test) \
> +{ \
> + gmem_conversions_do_setup(self, __nr_pages, flags); \
> + __gmem_conversions_##test(self, __nr_pages); \
> +} \
> +static void __gmem_conversions_##test(test_data_t *t, int nr_pages) \
> +
> +#define GMEM_CONVERSION_TEST(test, __nr_pages, flags) \
> + __GMEM_CONVERSION_TEST(test, __nr_pages, (flags) | GUEST_MEMFD_FLAG_MMAP)
> +
> +#define __GMEM_CONVERSION_TEST_INIT_PRIVATE(test, __nr_pages) \
> + GMEM_CONVERSION_TEST(test, __nr_pages, 0)
> +
> +#define GMEM_CONVERSION_TEST_INIT_PRIVATE(test) \
> + __GMEM_CONVERSION_TEST_INIT_PRIVATE(test, 1)
> +
> +struct guest_check_data {
> + void *mem;
> + char expected_val;
> + char write_val;
> +};
> +static struct guest_check_data guest_data;
> +
> +static void guest_do_rmw(void)
> +{
> + for (;;) {
> + char *mem = READ_ONCE(guest_data.mem);
> +
> + GUEST_ASSERT_EQ(READ_ONCE(*mem), READ_ONCE(guest_data.expected_val));
> + WRITE_ONCE(*mem, READ_ONCE(guest_data.write_val));
> +
> + GUEST_SYNC(0);
> + }
> +}
> +
> +static void run_guest_do_rmw(struct kvm_vcpu *vcpu, u64 pgoff,
> + char expected_val, char write_val)
> +{
> + struct ucall uc;
> + int r;
> +
> + guest_data.mem = (void *)GUEST_MEMFD_SHARING_TEST_GVA + pgoff * page_size;
> + guest_data.expected_val = expected_val;
> + guest_data.write_val = write_val;
> + sync_global_to_guest(vcpu->vm, guest_data);
> +
> + do {
> + r = __vcpu_run(vcpu);
> + } while (r == -1 && errno == EINTR);
> +
> + TEST_ASSERT_EQ(r, 0);
> +
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
> + case UCALL_SYNC:
> + break;
> + default:
> + TEST_FAIL("Unexpected ucall %lu", uc.cmd);
> + }
> +}
> +
> +static void host_do_rmw(char *mem, u64 pgoff, char expected_val,
> + char write_val)
> +{
> + TEST_ASSERT_EQ(READ_ONCE(mem[pgoff * page_size]), expected_val);
> + WRITE_ONCE(mem[pgoff * page_size], write_val);
> +}
> +
> +static void test_private(test_data_t *t, u64 pgoff, char starting_val,
> + char write_val)
> +{
> + TEST_EXPECT_SIGBUS(WRITE_ONCE(t->mem[pgoff * page_size], write_val));
> + run_guest_do_rmw(t->vcpu, pgoff, starting_val, write_val);
> + TEST_EXPECT_SIGBUS(READ_ONCE(t->mem[pgoff * page_size]));
> +}
> +
> +static void test_convert_to_private(test_data_t *t, u64 pgoff,
> + char starting_val, char write_val)
> +{
> + gmem_set_private(t->gmem_fd, pgoff * page_size, page_size);
> + test_private(t, pgoff, starting_val, write_val);
> +}
> +
> +static void test_shared(test_data_t *t, u64 pgoff, char starting_val,
> + char host_write_val, char write_val)
> +{
> + host_do_rmw(t->mem, pgoff, starting_val, host_write_val);
> + run_guest_do_rmw(t->vcpu, pgoff, host_write_val, write_val);
> + TEST_ASSERT_EQ(READ_ONCE(t->mem[pgoff * page_size]), write_val);
> +}
> +
> +static void test_convert_to_shared(test_data_t *t, u64 pgoff,
> + char starting_val, char host_write_val,
> + char write_val)
> +{
> + gmem_set_shared(t->gmem_fd, pgoff * page_size, page_size);
> + test_shared(t, pgoff, starting_val, host_write_val, write_val);
> +}
> +
> +GMEM_CONVERSION_TEST_INIT_PRIVATE(init_private)
> +{
> + test_private(t, 0, 0, 'A');
> + test_convert_to_shared(t, 0, 'A', 'B', 'C');
> + test_convert_to_private(t, 0, 'C', 'E');
> +}
> +
> +
> +int main(int argc, char *argv[])
> +{
> + TEST_REQUIRE(kvm_check_cap(KVM_CAP_VM_TYPES) & BIT(KVM_X86_SW_PROTECTED_VM));
> + TEST_REQUIRE(kvm_check_cap(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES) &
> + KVM_MEMORY_ATTRIBUTE_PRIVATE);
> +
> + page_size = getpagesize();
> +
> + return test_harness_run(argc, argv);
> +}
>
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>
>
^ permalink raw reply
* Re: [PATCH v8 30/46] KVM: selftests: Add helpers for calling ioctls on guest_memfd
From: Fuad Tabba @ 2026-06-24 19:26 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-30-9d2959357853@google.com>
On Fri, 19 Jun 2026 at 01:32, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Add helper functions to kvm_util.h to support calling ioctls, specifically
> KVM_SET_MEMORY_ATTRIBUTES2, on a guest_memfd file descriptor.
>
> Introduce gmem_ioctl() and __gmem_ioctl() macros, modeled after the
> existing vm_ioctl() helpers, to provide a standard way to call ioctls
> on a guest_memfd.
>
> Add gmem_set_memory_attributes() and its derivatives (gmem_set_private(),
> gmem_set_shared()) to set memory attributes on a guest_memfd region.
> Also provide "__" variants that return the ioctl error code instead of
> aborting the test. These helpers will be used by upcoming guest_memfd
> tests.
>
> To avoid code duplication, factor out the check for supported memory
> attributes into a new macro, TEST_ASSERT_SUPPORTED_ATTRIBUTES, and use
> it in both the existing vm_set_memory_attributes() and the new
> gmem_set_memory_attributes() helpers.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> tools/testing/selftests/kvm/include/kvm_util.h | 94 +++++++++++++++++++++++---
> 1 file changed, 86 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 0cacf3698b259..323d06b5699ec 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -392,6 +392,16 @@ static __always_inline void static_assert_is_vcpu(struct kvm_vcpu *vcpu) { }
> __TEST_ASSERT_VM_VCPU_IOCTL(!ret, #cmd, ret, (vcpu)->vm); \
> })
>
> +#define __gmem_ioctl(gmem_fd, cmd, arg) \
> + kvm_do_ioctl(gmem_fd, cmd, arg)
> +
> +#define gmem_ioctl(gmem_fd, cmd, arg) \
> +({ \
> + int ret = __gmem_ioctl(gmem_fd, cmd, arg); \
> + \
> + TEST_ASSERT(!ret, __KVM_IOCTL_ERROR(#cmd, ret)); \
> +})
> +
> /*
> * Looks up and returns the value corresponding to the capability
> * (KVM_CAP_*) given by cap.
> @@ -418,8 +428,16 @@ static inline void vm_enable_cap(struct kvm_vm *vm, u32 cap, u64 arg0)
> vm_ioctl(vm, KVM_ENABLE_CAP, &enable_cap);
> }
>
> +/*
> + * KVM_SET_MEMORY_ATTRIBUTES{,2} overwrites _all_ attributes. These
> + * flows need significant enhancements to support multiple attributes.
> + */
> +#define TEST_ASSERT_SUPPORTED_ATTRIBUTES(attributes) \
> + TEST_ASSERT(!(attributes) || (attributes) == KVM_MEMORY_ATTRIBUTE_PRIVATE, \
> + "Update me to support multiple attributes!")
> +
> static inline void vm_set_memory_attributes(struct kvm_vm *vm, gpa_t gpa,
> - u64 size, u64 attributes)
> + size_t size, u64 attributes)
> {
> struct kvm_memory_attributes attr = {
> .attributes = attributes,
> @@ -428,17 +446,11 @@ static inline void vm_set_memory_attributes(struct kvm_vm *vm, gpa_t gpa,
> .flags = 0,
> };
>
> - /*
> - * KVM_SET_MEMORY_ATTRIBUTES overwrites _all_ attributes. These flows
> - * need significant enhancements to support multiple attributes.
> - */
> - TEST_ASSERT(!attributes || attributes == KVM_MEMORY_ATTRIBUTE_PRIVATE,
> - "Update me to support multiple attributes!");
> + TEST_ASSERT_SUPPORTED_ATTRIBUTES(attributes);
>
> vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES, &attr);
> }
>
> -
> static inline void vm_mem_set_private(struct kvm_vm *vm, gpa_t gpa,
> u64 size)
> {
> @@ -451,6 +463,72 @@ static inline void vm_mem_set_shared(struct kvm_vm *vm, gpa_t gpa,
> vm_set_memory_attributes(vm, gpa, size, 0);
> }
>
> +static inline int __gmem_set_memory_attributes(int fd, u64 offset,
> + size_t size, u64 attributes,
> + u64 *error_offset)
> +{
> + struct kvm_memory_attributes2 attr = {
> + .attributes = attributes,
> + .offset = offset,
> + .size = size,
> + .flags = 0,
> + .error_offset = 0,
> + };
> + int r;
> +
> + r = __gmem_ioctl(fd, KVM_SET_MEMORY_ATTRIBUTES2, &attr);
> +
> + /* Copy error_offset regardless of r so caller can check. */
> + if (error_offset)
> + *error_offset = attr.error_offset;
> +
> + return r;
> +}
> +
> +static inline int __gmem_set_private(int fd, u64 offset, size_t size,
> + u64 *error_offset)
> +{
> + return __gmem_set_memory_attributes(fd, offset, size,
> + KVM_MEMORY_ATTRIBUTE_PRIVATE,
> + error_offset);
> +}
> +
> +static inline int __gmem_set_shared(int fd, u64 offset, size_t size,
> + u64 *error_offset)
> +{
> + return __gmem_set_memory_attributes(fd, offset, size, 0,
> + error_offset);
> +}
> +
> +static inline void gmem_set_memory_attributes(int fd, u64 offset,
> + size_t size, u64 attributes)
> +{
> + struct kvm_memory_attributes2 attr = {
> + .attributes = attributes,
> + .offset = offset,
> + .size = size,
> + .flags = 0,
> + };
> +
> + TEST_ASSERT_SUPPORTED_ATTRIBUTES(attributes);
> +
> + __TEST_REQUIRE(kvm_check_cap(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES) > 0,
> + "No valid attributes for guest_memfd ioctl!");
> +
> + gmem_ioctl(fd, KVM_SET_MEMORY_ATTRIBUTES2, &attr);
> +}
> +
> +static inline void gmem_set_private(int fd, u64 offset, size_t size)
> +{
> + gmem_set_memory_attributes(fd, offset, size,
> + KVM_MEMORY_ATTRIBUTE_PRIVATE);
> +}
> +
> +static inline void gmem_set_shared(int fd, u64 offset, size_t size)
> +{
> + gmem_set_memory_attributes(fd, offset, size, 0);
> +}
> +
> void vm_guest_mem_fallocate(struct kvm_vm *vm, gpa_t gpa, u64 size,
> bool punch_hole);
>
>
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>
>
^ permalink raw reply
* Re: [PATCH v8 29/46] KVM: selftests: Add selftests global for guest memory attributes capability
From: Fuad Tabba @ 2026-06-24 19:26 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-29-9d2959357853@google.com>
On Fri, 19 Jun 2026 at 01:32, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Add a global variable, kvm_has_gmem_attributes, to make the result of
> checking for KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES available to all tests.
>
> kvm_has_gmem_attributes is true if guest_memfd tracks memory attributes, as
> opposed to VM-level tracking.
>
> This global variable is synced to the guest for testing convenience, to
> avoid introducing subtle bugs when host/guest state is desynced.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> tools/testing/selftests/kvm/include/test_util.h | 2 ++
> tools/testing/selftests/kvm/lib/kvm_util.c | 5 +++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index a56271c237ae9..51287fac8138a 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -115,6 +115,8 @@ struct guest_random_state {
> extern u32 guest_random_seed;
> extern struct guest_random_state guest_rng;
>
> +extern bool kvm_has_gmem_attributes;
> +
> struct guest_random_state new_guest_random_state(u32 seed);
> u32 guest_random_u32(struct guest_random_state *state);
>
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index d5bbc80b2bf1c..b73817f7bc803 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -24,6 +24,8 @@ u32 guest_random_seed;
> struct guest_random_state guest_rng;
> static u32 last_guest_seed;
>
> +bool kvm_has_gmem_attributes;
> +
> static size_t vcpu_mmap_sz(void);
>
> int __open_path_or_exit(const char *path, int flags, const char *enoent_help)
> @@ -521,6 +523,7 @@ struct kvm_vm *__vm_create(struct vm_shape shape, u32 nr_runnable_vcpus,
> }
> guest_rng = new_guest_random_state(guest_random_seed);
> sync_global_to_guest(vm, guest_rng);
> + sync_global_to_guest(vm, kvm_has_gmem_attributes);
>
> kvm_arch_vm_post_create(vm, nr_runnable_vcpus);
>
> @@ -2286,6 +2289,8 @@ void __attribute((constructor)) kvm_selftest_init(void)
> guest_random_seed = last_guest_seed = random();
> pr_info("Random seed: 0x%x\n", guest_random_seed);
>
> + kvm_has_gmem_attributes = kvm_has_cap(KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES);
> +
> kvm_selftest_arch_init();
> }
>
>
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>
>
^ permalink raw reply
* Re: [PATCH v8 28/46] KVM: selftests: Add support for mmap() on guest_memfd in core library
From: Fuad Tabba @ 2026-06-24 19:07 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-28-9d2959357853@google.com>
On Fri, 19 Jun 2026 at 01:32, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Accept gmem_flags in vm_mem_add() to be able to create a guest_memfd within
> vm_mem_add().
>
> When vm_mem_add() is used to set up a guest_memfd for a memslot, set up the
> provided (or created) gmem_fd as the fd for the user memory region. This
> makes it available to be mmap()-ed from just like fds from other memory
> sources. mmap() from guest_memfd using the provided gmem_flags and
> gmem_offset.
>
> Add a kvm_slot_to_fd() helper to provide convenient access to the file
> descriptor of a memslot.
>
> Update existing callers of vm_mem_add() to pass 0 for gmem_flags to
> preserve existing behavior.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> [For guest_memfds, mmap() using gmem_offset instead of 0 all the time.]
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> tools/testing/selftests/kvm/include/kvm_util.h | 7 +++++-
> tools/testing/selftests/kvm/lib/kvm_util.c | 27 ++++++++++++----------
> .../kvm/x86/private_mem_conversions_test.c | 2 +-
> 3 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index d4c104cb0418f..0cacf3698b259 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -700,7 +700,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> gpa_t gpa, u32 slot, u64 npages, u32 flags);
> void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> gpa_t gpa, u32 slot, u64 npages, u32 flags,
> - int gmem_fd, u64 gmem_offset);
> + int gmem_fd, u64 gmem_offset, u64 gmem_flags);
>
> #ifndef vm_arch_has_protected_memory
> static inline bool vm_arch_has_protected_memory(struct kvm_vm *vm)
> @@ -732,6 +732,11 @@ void *addr_gva2hva(struct kvm_vm *vm, gva_t gva);
> gpa_t addr_hva2gpa(struct kvm_vm *vm, void *hva);
> void *addr_gpa2alias(struct kvm_vm *vm, gpa_t gpa);
>
> +static inline int kvm_slot_to_fd(struct kvm_vm *vm, u32 slot)
> +{
> + return memslot2region(vm, slot)->fd;
> +}
> +
> #ifndef vcpu_arch_put_guest
> #define vcpu_arch_put_guest(mem, val) do { (mem) = (val); } while (0)
> #endif
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 9b482778f7379..d5bbc80b2bf1c 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -978,12 +978,13 @@ void vm_set_user_memory_region2(struct kvm_vm *vm, u32 slot, u32 flags,
> /* FIXME: This thing needs to be ripped apart and rewritten. */
> void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> gpa_t gpa, u32 slot, u64 npages, u32 flags,
> - int gmem_fd, u64 gmem_offset)
> + int gmem_fd, u64 gmem_offset, u64 gmem_flags)
> {
> int ret;
> struct userspace_mem_region *region;
> size_t backing_src_pagesz = get_backing_src_pagesz(src_type);
> size_t mem_size = npages * vm->page_size;
> + off_t mmap_offset = 0;
> size_t alignment = 1;
>
> TEST_REQUIRE_SET_USER_MEMORY_REGION2();
> @@ -1055,8 +1056,6 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
>
> if (flags & KVM_MEM_GUEST_MEMFD) {
> if (gmem_fd < 0) {
> - u32 gmem_flags = 0;
> -
> TEST_ASSERT(!gmem_offset,
> "Offset must be zero when creating new guest_memfd");
> gmem_fd = vm_create_guest_memfd(vm, mem_size, gmem_flags);
> @@ -1077,13 +1076,17 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
> }
>
> region->fd = -1;
> - if (backing_src_is_shared(src_type))
> + if (flags & KVM_MEM_GUEST_MEMFD && gmem_flags & GUEST_MEMFD_FLAG_MMAP) {
> + region->fd = kvm_dup(gmem_fd);
> + mmap_offset = gmem_offset;
> + } else if (backing_src_is_shared(src_type)) {
> region->fd = kvm_memfd_alloc(region->mmap_size,
> src_type == VM_MEM_SRC_SHARED_HUGETLB);
> + }
>
> - region->mmap_start = kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE,
> - vm_mem_backing_src_alias(src_type)->flag,
> - region->fd);
> + region->mmap_start = __kvm_mmap(region->mmap_size, PROT_READ | PROT_WRITE,
> + vm_mem_backing_src_alias(src_type)->flag,
> + region->fd, mmap_offset);
>
> TEST_ASSERT(!is_backing_src_hugetlb(src_type) ||
> region->mmap_start == align_ptr_up(region->mmap_start, backing_src_pagesz),
> @@ -1129,10 +1132,10 @@ void vm_mem_add(struct kvm_vm *vm, enum vm_mem_backing_src_type src_type,
>
> /* If shared memory, create an alias. */
> if (region->fd >= 0) {
> - region->mmap_alias = kvm_mmap(region->mmap_size,
> - PROT_READ | PROT_WRITE,
> - vm_mem_backing_src_alias(src_type)->flag,
> - region->fd);
> + region->mmap_alias = __kvm_mmap(region->mmap_size,
> + PROT_READ | PROT_WRITE,
> + vm_mem_backing_src_alias(src_type)->flag,
> + region->fd, mmap_offset);
>
> /* Align host alias address */
> region->host_alias = align_ptr_up(region->mmap_alias, alignment);
> @@ -1143,7 +1146,7 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> enum vm_mem_backing_src_type src_type,
> gpa_t gpa, u32 slot, u64 npages, u32 flags)
> {
> - vm_mem_add(vm, src_type, gpa, slot, npages, flags, -1, 0);
> + vm_mem_add(vm, src_type, gpa, slot, npages, flags, -1, 0, 0);
> }
>
> /*
> diff --git a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c
> index 1d2f5d4fd45d7..861baff201e78 100644
> --- a/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c
> +++ b/tools/testing/selftests/kvm/x86/private_mem_conversions_test.c
> @@ -399,7 +399,7 @@ static void test_mem_conversions(enum vm_mem_backing_src_type src_type, u32 nr_v
> for (i = 0; i < nr_memslots; i++)
> vm_mem_add(vm, src_type, BASE_DATA_GPA + slot_size * i,
> BASE_DATA_SLOT + i, slot_size / vm->page_size,
> - KVM_MEM_GUEST_MEMFD, memfd, slot_size * i);
> + KVM_MEM_GUEST_MEMFD, memfd, slot_size * i, 0);
>
> for (i = 0; i < nr_vcpus; i++) {
> gpa_t gpa = BASE_DATA_GPA + i * per_cpu_size;
>
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>
>
^ permalink raw reply
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default
From: Fuad Tabba @ 2026-06-24 18:57 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Sean Christopherson, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers,
Jonathan Corbet, Shuah Khan, Shuah Khan, Vishal Annapurve,
Andrew Morton, Chris Li, Kairui Song, Kemeng Shi, Nhat Pham,
Barry Song, Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park,
Qi Zheng, Shakeel Butt, Kiryl Shutsemau, Baoquan He,
Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-24-9d2959357853@google.com>
On Fri, 19 Jun 2026 at 01:31, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> Make in-place conversion the default if the arch has private mem.
>
> The default can be overridden at compile type by enabling
compile _time_
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES, or at KVM load time through a module
> parameter.
>
> In-place conversion also implies tracking a guest's private/shared state in
> guest_memfd. To avoid inconsistencies in the way memory attributes are
> tracked between the per-VM or by guest_memfd, make the module_param
> read-only (0444).
>
> Document that using per-VM attributes for tracking private/shared state of
> guest memory is deprecated in favor of tracking in guest_memfd.
>
> Warn if the admin sets gmem_in_place_conversion as false when
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES is not enabled. Add warning in the code
> path where guest memory is populated for a CoCo VM, since that's the
> earliest point in a CoCo VM's lifecycle where memory attributes are
> queried. Unlike other query sites, this site is exclusively used by CoCo
> VMs.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kvm/Kconfig | 7 ++++++-
> virt/kvm/guest_memfd.c | 5 +++++
> virt/kvm/kvm_main.c | 3 ++-
> 3 files changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index c28393dc664eb..a3c189d765150 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -85,7 +85,12 @@ config KVM_VM_MEMORY_ATTRIBUTES
> bool "Enable per-VM PRIVATE vs. SHARED attributes (for CoCo VMs)"
> help
> Enable support for tracking PRIVATE vs. SHARED memory using per-VM
> - memory attributes.
> + memory attributes. Using per-VM attributes are deprecated in favor
nit:
are->is
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> + of tracking PRIVATE state in guest_memfd. Select this if you need
> + to run CoCo VMs using a VMM that doesn't support guest_memfd memory
> + attributes.
> +
> + If unsure, say N.
>
> config KVM_SW_PROTECTED_VM
> bool "Enable support for KVM software-protected VMs"
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 86c9f5b0863cb..5cb73543c03c8 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -1193,10 +1193,15 @@ static bool kvm_gmem_range_is_private(struct file *file, pgoff_t index,
> {
> struct maple_tree *mt = &GMEM_I(file_inode(file))->attributes;
>
> +#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
> if (!gmem_in_place_conversion)
> return kvm_range_has_vm_memory_attributes(kvm, gfn, gfn + nr_pages,
> KVM_MEMORY_ATTRIBUTE_PRIVATE,
> KVM_MEMORY_ATTRIBUTE_PRIVATE);
> +#else
> + if (WARN_ON_ONCE(!gmem_in_place_conversion))
> + return false;
> +#endif
>
> return kvm_gmem_range_has_attributes(mt, index, nr_pages,
> KVM_MEMORY_ATTRIBUTE_PRIVATE);
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index dd1d18a1d2f68..46e92b5dc3804 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -102,7 +102,8 @@ static bool __ro_after_init allow_unsafe_mappings;
> module_param(allow_unsafe_mappings, bool, 0444);
>
> #ifdef kvm_arch_has_private_mem
> -bool __ro_after_init gmem_in_place_conversion = false;
> +bool __ro_after_init gmem_in_place_conversion = !IS_ENABLED(CONFIG_KVM_VM_MEMORY_ATTRIBUTES);
> +module_param(gmem_in_place_conversion, bool, 0444);
> EXPORT_SYMBOL_FOR_KVM_INTERNAL(gmem_in_place_conversion);
> #endif
>
>
> --
> 2.55.0.rc0.738.g0c8ab3ebcc-goog
>
>
^ permalink raw reply
* Re: [PATCH v8 15/46] KVM: guest_memfd: Call arch invalidate hooks on conversion
From: Ackerley Tng @ 2026-06-24 17:46 UTC (permalink / raw)
To: Sean Christopherson, Fuad Tabba
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, willy, wyihan,
yan.y.zhao, forkloop, pratyush, suzuki.poulose, aneesh.kumar,
liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ajneQVLriUshjFIO@google.com>
Sean Christopherson <seanjc@google.com> writes:
> On Fri, Jun 19, 2026, Fuad Tabba wrote:
>> On Fri, 19 Jun 2026 at 01:31, Ackerley Tng via B4 Relay
>> <devnull+ackerleytng.google.com@kernel.org> wrote:
>> >
>> > From: Ackerley Tng <ackerleytng@google.com>
>> >
>> > When memory in guest_memfd is converted from private to shared, the
>> > platform-specific state associated with the guest-private pages must be
>> > invalidated or cleaned up.
>> >
>> > Iterate over the folios in the affected range and call the
>> > kvm_arch_gmem_invalidate() hook for each PFN range. This allows
>> > architectures to perform necessary teardown, such as updating hardware
>> > metadata or encryption states, before the pages are transitioned to the
>> > shared state.
>> >
>> > Invoke this helper after indicating to KVM's mmu code that an invalidation
>> > is in progress to stop in-flight page faults from succeeding.
>> >
>> > Reviewed-by: Fuad Tabba <tabba@google.com>
>> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>>
>> Coming back to this after working through the arm64/pKVM side. My
>> Reviewed-by here is from the previous round and the patch hasn't
>> changed, but I missed an implication for arm64.
>>
>> kvm_arch_gmem_invalidate() is now called from two paths with the same
>> (start, end) signature: folio teardown (kvm_gmem_free_folio) and
>> private->shared conversion (here). For SNP/TDX that's fine, conversion is
>> destructive anyway. For pKVM the two need opposite content semantics:
>> conversion must preserve the page in place (same physical page, the point
>> of in-place conversion without encryption), while teardown must scrub it
>> before returning it to the host.
>>
>> The hook gets only a pfn range with no indication of which caller it's
>> serving, so arm64 can't give the two paths the behaviour they need. It
>> would help to signal intent on the conversion path: a reason/flag, a
>> separate hook, or not routing non-destructive conversion through the
>> teardown hook.
>>
>> arm64 isn't here yet, so this isn't urgent, but the hook is gaining a
>> second caller now, and it's cheaper to leave room for the distinction
>> than to change a generic contract other arches depend on later.
>
> Crud. It may not be urgent for arm64, but it's urgent for other reasons that
> I "can't" describe in detail at the moment, and even if that weren't the case, I
> think we should clean things up now. More below.
>
>> > virt/kvm/guest_memfd.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> > 1 file changed, 41 insertions(+)
>> >
>> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> > index 433f79047b9d1..3c94442bc8131 100644
>> > --- a/virt/kvm/guest_memfd.c
>> > +++ b/virt/kvm/guest_memfd.c
>> > @@ -607,6 +607,42 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
>> > return safe;
>> > }
>> >
>> > +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
>> > +static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
>
> Not your fault, but kvm_arch_gmem_invalidate() is badly misnamed. It's not
> "invalidating" anything, it's much more of a "free" callback, as SNP uses it to
> put physical pages back into a shared state when a maybe-private folio is freed.
>
> As Fuad points out, (ab)using that hook for the private=>shared conversion case
> "works", but not broadly. And it makes the bad name worse, because it's called
> from code that _is_ doing true invalidations. For pKVM, it may not even need to
> do anything invalidation-like.
>
Thanks, I also didn't like the naming of kvm_gmem_invalidate(),
especially when conversions also calls
kvm_gmem_invalidate_{start,end}() and those do different things.
> To avoid a conflict with patches that are going to have priority over this series,
> to set the stage for arm64 support, and to avoid avoid bleeding vendor details
> into guest_memfd, as if they are core guest_memfd behavior (only SNP needs the
> "invalidation" on this specific transition), I think we should add an arch hook
> to do conversions straightaway.
>
> Unless there's a clever option I'm missing, it'll mean adding yet another
> HAVE_KVM_ARCH_GMEM_XXX flag? Hmm, especially because IIUC, arm64/pKVM doesn't
> need a callback for this case, only the free_folio case.
>
>> > +{
>> > + struct folio_batch fbatch;
>> > + pgoff_t next = start;
>> > + int i;
>> > +
>> > + folio_batch_init(&fbatch);
>> > + while (filemap_get_folios(inode->i_mapping, &next, end - 1, &fbatch)) {
>> > + for (i = 0; i < folio_batch_count(&fbatch); ++i) {
>> > + struct folio *folio = fbatch.folios[i];
>> > + pgoff_t start_index, end_index;
>> > + kvm_pfn_t start_pfn, end_pfn;
>> > +
>> > + start_index = max(start, folio->index);
>> > + end_index = min(end, folio_next_index(folio));
>> > + /*
>> > + * end_index is either in folio or points to
>> > + * the first page of the next folio. Hence,
>> > + * all pages in range [start_index, end_index)
>> > + * are contiguous.
>> > + */
>> > + start_pfn = folio_file_pfn(folio, start_index);
>> > + end_pfn = start_pfn + end_index - start_index;
>> > +
>> > + kvm_arch_gmem_invalidate(start_pfn, end_pfn);
>> > + }
>> > +
>> > + folio_batch_release(&fbatch);
>> > + cond_resched();
>> > + }
>> > +}
>> > +#else
>> > +static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end) {}
>> > +#endif
>> > +
>> > static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> > size_t nr_pages, uint64_t attrs,
>> > pgoff_t *err_index)
>> > @@ -647,7 +683,12 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> > */
>> >
>> > kvm_gmem_invalidate_start(inode, start, end);
>> > +
>> > + if (!to_private)
>> > + kvm_gmem_invalidate(inode, start, end);
>
> E.g. instead make this something like this?
>
> kvm_gmem_set_pfn_attributes(...)
>
> Hrm, though that wastes folio lookups in the to_private case. So maybe just this,
> assuming pKVM doesn't need to take additional action on conversions?
>
> if (!to_private)
> kvm_gmem_make_shared(...)
>
> Actually, if we do that, then we don't need a separate arch hook, just a separate
> config. It'll still bleed SNP details into guest_memfd, but it'll at least be
> done in a way that's more explicitly arch specific (and it's no different than
> what we already do for PREPARE...).
>
pKVM needs some arch guest_memfd lifecycle functions that
+ for conversion, doesn't do anything,
+ for teardown, resets page state (IIUC it'll be reset to
PKVM_PAGE_OWNED (by the host))
So I think we need different functions for those two stages in the
lifecycle of a page with guest_memfd? What if we have
CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES, which gates
+ kvm_gmem_should_set_pfn_attributes(attributes) and
.gmem_should_set_pfn_attributes
+ kvm_gmem_set_pfn_attributes(start_pfn, end_pfn, attributes) and
.gmem_set_pfn_attributes
CONFIG_HAVE_KVM_ARCH_GMEM_TEARDOWN, which gates
+ kvm_gmem_teardown() and .gmem_teardown
SNP:
+ .gmem_should_set_pfn_attributes = sev_gmem_should_set_pfn_attributes,
and sev_gmem_should_set_pfn_attributes returns !is_private
+ Rename .gmem_invalidate and sev_gmem_invalidate to *set_pfn_attributes
+ .gmem_teardown = sev_gmem_set_pfn_attributes
TDX:
+ Disable CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES
+ Disable CONFIG_HAVE_KVM_ARCH_GMEM_TEARDOWN
pKVM:
+ Disable CONFIG_HAVE_KVM_ARCH_GMEM_SET_PFN_ATTRIBUTES
+ .gmem_teardown = pkvm_gmem_set_pfn_attributes
Suzuki, does this work for ARM CCA?
This way,
+ The if (is_private) check doesn't leak SNP details into guest_memfd
+ .gmem_make_shared doesn't stick out without a .gmem_make_private
+ .gmem_set_pfn_attributes, .gmem_prepare and .gmem_teardown are aligned
conceptually as lifecycle hooks
+ I think the private/shared check for prepare can also be folded into
preparation.
+ Preparation perhaps doesn't need a should_prepare equivalent since
there's no iteration and getting the gfn is just doing some math?
+ In another patch series?
> E.g. this? There will still be a looming rename conflict, but that's easy enough
> to handle.
>
> diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
> index 9ce5be7843f2..8aead0abd788 100644
> --- virt/kvm/guest_memfd.c
> +++ virt/kvm/guest_memfd.c
> @@ -648,8 +648,8 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> return safe;
> }
>
> -#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
> -static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
> +#ifdef CONFIG_KVM_ARCH_GMEM_FREE_ON_SHARED_CONVERSION
> +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end)
> {
> struct folio_batch fbatch;
> pgoff_t next = start;
> @@ -681,7 +681,7 @@ static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
> }
> }
> #else
> -static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end) {}
> +static void kvm_gmem_make_shared(struct inode *inode, pgoff_t start, pgoff_t end) { }
> #endif
>
> static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> @@ -729,7 +729,7 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> kvm_gmem_invalidate_start(inode, start, end);
>
> if (!to_private)
> - kvm_gmem_invalidate(inode, start, end);
> + kvm_gmem_make_shared(inode, start, end);
>
> mas_store_prealloc(&mas, xa_mk_value(attrs));
^ permalink raw reply
* Re: [PATCH v8 18/46] KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
From: Sean Christopherson @ 2026-06-24 17:01 UTC (permalink / raw)
To: Binbin Wu
Cc: ackerleytng, aik, andrew.jones, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <6fc7f450-6d0a-494d-b295-297e4703148d@linux.intel.com>
On Tue, Jun 23, 2026, Binbin Wu wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > @@ -606,12 +608,20 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> > next = start;
> > while (safe && filemap_get_folios(mapping, &next, last, &fbatch)) {
> >
> > - for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> > + for (i = 0; i < folio_batch_count(&fbatch);) {
> > struct folio *folio = fbatch.folios[i];
> >
> > - if (folio_ref_count(folio) !=
> > - folio_nr_pages(folio) + filemap_get_folios_refcount) {
> > - safe = false;
> > + safe = (folio_ref_count(folio) ==
> > + folio_nr_pages(folio) +
> > + filemap_get_folios_refcount);
> > +
> > + if (safe) {
> > + ++i;
> > + } else if (folio_may_be_lru_cached(folio) &&
> > + !lru_drained) {
> > + lru_add_drain_all();
>
> It seems unprivileged userspace is able to trigger lru_add_drain_all() repeatedly
> by invoking KVM_SET_MEMORY_ATTRIBUTES2 in a loop, which could lead to DoS risk?
FIW, if there's a risk, then AFAICT fadvise() and memfd's F_ADD_SEALS already
have the same risk.
^ permalink raw reply
* Re: [PATCH v8 18/46] KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
From: Sean Christopherson @ 2026-06-24 16:57 UTC (permalink / raw)
To: Ackerley Tng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, willy,
wyihan, yan.y.zhao, forkloop, pratyush, suzuki.poulose,
aneesh.kumar, liam, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-18-9d2959357853@google.com>
On Thu, Jun 18, 2026, Ackerley Tng wrote:
> When checking if a guest_memfd folio is safe for conversion, its refcount
> is examined. A folio may be present in a per-CPU lru_add fbatch, which
> temporarily increases its refcount.
Under what circumstances does this happen, and what alternatives are there for
userspace to work around the issue?
^ permalink raw reply
* Re: [PATCH v2 1/2] signal: avoid shared siginfo namespace rewrites
From: Oleg Nesterov @ 2026-06-24 16:32 UTC (permalink / raw)
To: Bradley Morgan, Eric W. Biederman
Cc: Christian Brauner, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Andrew Morton, Peter Zijlstra, Marco Elver,
Aleksandr Nogikh, Thomas Gleixner, Adrian Huang, Kexin Sun,
linux-kernel, linux-trace-kernel, stable
In-Reply-To: <A35F5FF8-4FCB-4CE9-8DC5-E0A22071010E@grrlz.net>
On 06/24, Bradley Morgan wrote:
>
> Hey you two, sorry to impede in your conversation, but could we write
> your "conflicting" patch over my Patch 2?
>
> It's fine if you don't want to, it kind of kills two birds with one stone.
No, sorry, I don't ;) at least right now. Because I don't really like the
changes it adds into send_signal_locked(). But perhaps I didn't read it
carefully.
Can we return to it later? There is another reason... Currently I am very
busy but I am thinking about another change on top of your 1/2. Something
like below. Not sure it makes a lot of sense though.
Eric, do you think this optimization on top of 1/2 makes sense?
Oleg.
int send_signal_locked(int sig, struct kernel_siginfo *info,
struct task_struct *t, enum pid_type type)
{
/* Should SIGKILL or SIGSTOP be received by a pid namespace init? */
struct kernel_siginfo __info;
bool force = false;
if (info == SEND_SIG_NOINFO) {
/* Force if sent from an ancestor pid namespace */
force = !task_pid_nr_ns(current, task_active_pid_ns(t));
} else if (info == SEND_SIG_PRIV) {
/* Don't ignore kernel generated signals */
force = true;
} else if (has_si_pid_and_uid(info)) {
/* SIGKILL and SIGSTOP is special or has ids */
struct user_namespace *t_user_ns;
#ifdef CONFIG_USER_NS
rcu_read_lock();
t_user_ns = task_cred_xxx(t, user_ns);
if (current_user_ns() != t_user_ns) {
__info = *info;
info = &__info;
kuid_t uid = make_kuid(current_user_ns(), info->si_uid);
info->si_uid = from_kuid_munged(t_user_ns, uid);
}
rcu_read_unlock();
#endif
/* A kernel generated signal? */
force = (info->si_code == SI_KERNEL);
#ifdef CONFIG_PID_NS
/* From an ancestor pid namespace? */
if (!task_pid_nr_ns(current, task_active_pid_ns(t))) {
if (info != &__info) {
__info = *info;
info = &__info;
}
info->si_pid = 0;
force = true;
}
#endif
}
return __send_signal_locked(sig, info, t, type, force);
}
^ 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