* Re: [PATCH v6 18/43] KVM: Move KVM_VM_MEMORY_ATTRIBUTES config definition to x86
From: Fuad Tabba @ 2026-05-21 8:07 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, 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, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260507-gmem-inplace-conversion-v6-18-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Sean Christopherson <seanjc@google.com>
>
> Bury KVM_VM_MEMORY_ATTRIBUTES in x86 to discourage other architectures
> from adding support for per-VM memory attributes, because tracking private
> vs. shared memory on a per-VM basis is now deprecated in favor of tracking
> on a per-guest_memfd basis, and no other memory attributes are on the
> horizon.
>
> This will also allow modifying KVM_VM_MEMORY_ATTRIBUTES to be
> user-selectable (in x86) without creating weirdness in KVM's Kconfigs.
> Now that guest_memfd support memory attributes, it's entirely possible to
> run x86 CoCo VMs without support for KVM_VM_MEMORY_ATTRIBUTES.
>
> Leave the code itself in common KVM so that it's trivial to undo this
> change if new per-VM attributes do come along.
>
> 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
> ---
> arch/x86/kvm/Kconfig | 4 ++++
> virt/kvm/Kconfig | 4 ----
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> index 26f6afd51bbdc..b6d65ee664d0f 100644
> --- a/arch/x86/kvm/Kconfig
> +++ b/arch/x86/kvm/Kconfig
> @@ -80,6 +80,10 @@ config KVM_WERROR
>
> If in doubt, say "N".
>
> +config KVM_VM_MEMORY_ATTRIBUTES
> + select KVM_MEMORY_ATTRIBUTES
> + bool
> +
> config KVM_SW_PROTECTED_VM
> bool "Enable support for KVM software-protected VMs"
> depends on EXPERT
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index e371e079e2c50..663de6421eda2 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -103,10 +103,6 @@ config KVM_MMU_LOCKLESS_AGING
> config KVM_MEMORY_ATTRIBUTES
> bool
>
> -config KVM_VM_MEMORY_ATTRIBUTES
> - select KVM_MEMORY_ATTRIBUTES
> - bool
> -
> config KVM_GUEST_MEMFD
> select XARRAY_MULTI
> select KVM_MEMORY_ATTRIBUTES
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 17/43] KVM: guest_memfd: Determine invalidation filter from memory attributes
From: Fuad Tabba @ 2026-05-21 7:56 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, 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, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260507-gmem-inplace-conversion-v6-17-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> Before conversion, the range filter doesn't really matter:
>
> + For non-CoCo VMs that use guest_memfd, they have no mirrored tdp, so
> KVM_DIRECT_ROOTS would have been invalidated anyway.
> + CoCo VMs could not use INIT_SHARED, and there's no conversion support, so
> always using KVM_FILTER_PRIVATE would have worked.
>
> Now with conversion support, update kvm_gmem_get_invalidate_filter to
> inspect the memory attributes maple tree for a given range.
>
> Instead of determining the invalidation filter based on static inode
> flags, iterate through the attributes maple tree for the specific range
> being invalidated. This allows KVM to identify if the range contains
> private pages, shared pages, or both, and set the filter bits
> accordingly.
>
> Update kvm_gmem_invalidate_begin and kvm_gmem_release to pass the range
> parameters to the filter helper to ensure invalidation accurately
> targets the memory types present in the affected range.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> virt/kvm/guest_memfd.c | 27 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 7 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 9f6eebfb68f6b..c9f155c2dc5c5 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -193,12 +193,24 @@ static struct folio *kvm_gmem_get_folio(struct inode *inode, pgoff_t index)
> return folio;
> }
>
> -static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(struct inode *inode)
> +static enum kvm_gfn_range_filter kvm_gmem_get_invalidate_filter(
> + struct inode *inode, pgoff_t start, pgoff_t end)
> {
> - if (GMEM_I(inode)->flags & GUEST_MEMFD_FLAG_INIT_SHARED)
> - return KVM_FILTER_SHARED;
> + struct gmem_inode *gi = GMEM_I(inode);
> + enum kvm_gfn_range_filter filter = 0;
> + void *entry;
> +
> + lockdep_assert(mt_lock_is_held(&gi->attributes));
> +
> + mt_for_each(&gi->attributes, entry, start, end - 1) {
> + filter |= (xa_to_value(entry) & KVM_MEMORY_ATTRIBUTE_PRIVATE) ?
> + KVM_FILTER_PRIVATE : KVM_FILTER_SHARED;
> +
> + if (filter == (KVM_FILTER_PRIVATE | KVM_FILTER_SHARED))
> + break;
> + }
>
> - return KVM_FILTER_PRIVATE;
> + return filter;
> }
>
> static void __kvm_gmem_invalidate_begin(struct gmem_file *f, pgoff_t start,
> @@ -244,7 +256,7 @@ static void kvm_gmem_invalidate_begin(struct inode *inode, pgoff_t start,
> enum kvm_gfn_range_filter attr_filter;
> struct gmem_file *f;
>
> - attr_filter = kvm_gmem_get_invalidate_filter(inode);
> + attr_filter = kvm_gmem_get_invalidate_filter(inode, start, end);
>
> kvm_gmem_for_each_file(f, inode)
> __kvm_gmem_invalidate_begin(f, start, end, attr_filter);
> @@ -367,6 +379,7 @@ static long kvm_gmem_fallocate(struct file *file, int mode, loff_t offset,
> static int kvm_gmem_release(struct inode *inode, struct file *file)
> {
> struct gmem_file *f = file->private_data;
> + enum kvm_gfn_range_filter filter;
> struct kvm_memory_slot *slot;
> struct kvm *kvm = f->kvm;
> unsigned long index;
> @@ -398,8 +411,8 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> * memory, as its lifetime is associated with the inode, not the file.
> */
> end = i_size_read(inode) >> PAGE_SHIFT;
> - __kvm_gmem_invalidate_begin(f, 0, end,
> - kvm_gmem_get_invalidate_filter(inode));
> + filter = kvm_gmem_get_invalidate_filter(inode, 0, end);
> + __kvm_gmem_invalidate_begin(f, 0, end, filter);
> __kvm_gmem_invalidate_end(f, 0, end);
>
> list_del(&f->entry);
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* [PATCH kernel] crypto/ccp/tsm: Enable the root port after the endpoint
From: Alexey Kardashevskiy @ 2026-05-21 7:43 UTC (permalink / raw)
To: linux-crypto
Cc: linux-kernel, Tom Lendacky, Herbert Xu, David S. Miller,
Dan Williams, Alexey Kardashevskiy, x86, linux-coco,
Pratik R . Sampat, Xu Yilun, Aneesh Kumar K . V
The PCIe r7.0, chapter "6.33.8 Other IDE Rules" mandates if selective IDE
is enabled for config requersts, a stream must be enabled on the endpoint
before enabling it on the rootport:
===
For Selective IDE, the Stream must not be used until it has been enabled in
both Partner Ports. For cases where one of the Partner Ports is a Root Port
and Selective IDE for Configuration Requests is enabled, the other
Partner Port must be enabled prior to the Root Port. For other scenarios,
the mechanisms to satisfy this requirement are implementation-specific.
===
Do what the spec says.
Fixes: 4be423572da1 ("crypto/ccp: Implement SEV-TIO PCIe IDE (phase1)")
Signed-off-by: Alexey Kardashevskiy <aik@amd.com>
---
drivers/crypto/ccp/sev-dev-tsm.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/ccp/sev-dev-tsm.c b/drivers/crypto/ccp/sev-dev-tsm.c
index b07ae529b591..10549a2cc2ae 100644
--- a/drivers/crypto/ccp/sev-dev-tsm.c
+++ b/drivers/crypto/ccp/sev-dev-tsm.c
@@ -58,13 +58,13 @@ static int stream_enable(struct pci_ide *ide)
struct pci_dev *rp = pcie_find_root_port(ide->pdev);
int ret;
- ret = pci_ide_stream_enable(rp, ide);
- if (ret)
+ ret = pci_ide_stream_enable(ide->pdev, ide);
+ if (ret && ret != -ENXIO)
return ret;
- ret = pci_ide_stream_enable(ide->pdev, ide);
- if (ret)
- pci_ide_stream_disable(rp, ide);
+ ret = pci_ide_stream_enable(rp, ide);
+ if (ret && ret != -ENXIO)
+ pci_ide_stream_disable(ide->pdev, ide);
return ret;
}
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v6 16/43] KVM: guest_memfd: Use actual size for invalidation in kvm_gmem_release()
From: Fuad Tabba @ 2026-05-21 7:30 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, 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, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260507-gmem-inplace-conversion-v6-16-91ab5a8b19a4@google.com>
Hi Ackerley,
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> __kvm_gmem_invalidate_begin() and __kvm_gmem_invalidate_end() actually do
> not specially handle -1ul. -1ul is used as a huge number, which legal
> indices do not exceed, and hence the invalidation works as expected.
>
> Since a later patch is going to make use of the exact range, calculate the
> size of the guest_memfd inode and use it as the end range for invalidating
> SPTEs.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Want to look at what Sashiko has to say? Seems to be a real issue:
https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=16
If I understand correctly, the fix should simple: use
check_add_overflow() to validate the offset and size parameters in
kvm_gmem_bind()
int kvm_gmem_bind(struct kvm *kvm, struct kvm_memory_slot *slot,
unsigned int fd, loff_t offset)
{
loff_t size = slot->npages << PAGE_SHIFT;
+ loff_t end;
unsigned long start, end_index;
struct gmem_file *f;
...
- if (offset < 0 || !PAGE_ALIGNED(offset) ||
- offset + size > i_size_read(inode))
+ if (offset < 0 || !PAGE_ALIGNED(offset) ||
+ check_add_overflow(offset, size, &end) ||
+ end > i_size_read(inode))
goto err;
What do you think?
/fuad
> ---
> virt/kvm/guest_memfd.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 050a8c092b1a3..9f6eebfb68f6b 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -370,6 +370,7 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> struct kvm_memory_slot *slot;
> struct kvm *kvm = f->kvm;
> unsigned long index;
> + pgoff_t end;
>
> /*
> * Prevent concurrent attempts to *unbind* a memslot. This is the last
> @@ -396,9 +397,10 @@ static int kvm_gmem_release(struct inode *inode, struct file *file)
> * Zap all SPTEs pointed at by this file. Do not free the backing
> * memory, as its lifetime is associated with the inode, not the file.
> */
> - __kvm_gmem_invalidate_begin(f, 0, -1ul,
> + end = i_size_read(inode) >> PAGE_SHIFT;
> + __kvm_gmem_invalidate_begin(f, 0, end,
> kvm_gmem_get_invalidate_filter(inode));
> - __kvm_gmem_invalidate_end(f, 0, -1ul);
> + __kvm_gmem_invalidate_end(f, 0, end);
>
> list_del(&f->entry);
>
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 05/43] KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
From: Fuad Tabba @ 2026-05-21 7:19 UTC (permalink / raw)
To: Ackerley Tng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, 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, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CAEvNRgGQvMdDmVfbk42EY_PGN0ybTp-x21Zj+pg_X1mk9iCRtA@mail.gmail.com>
On Wed, 20 May 2026 at 22:44, Ackerley Tng <ackerleytng@google.com> wrote:
>
> Fuad Tabba <tabba@google.com> writes:
>
> >
> > [...snip...]
> >
> >> +unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> >> +{
> >> + struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> >> + struct inode *inode;
> >> +
> >> + /*
> >> + * If this gfn has no associated memslot, there's no chance of the gfn
> >> + * being backed by private memory, since guest_memfd must be used for
> >> + * private memory, and guest_memfd must be associated with some memslot.
> >> + */
> >> + if (!slot)
> >> + return 0;
> >> +
> >> + CLASS(gmem_get_file, file)(slot);
> >> + if (!file)
> >> + return 0;
> >> +
> >> + inode = file_inode(file);
> >> +
> >> + /*
> >> + * Rely on the maple tree's internal RCU lock to ensure a
> >> + * stable result. This result can become stale as soon as the
> >> + * lock is dropped, so the caller _must_ still protect
> >> + * consumption of private vs. shared by checking
> >> + * mmu_invalidate_retry_gfn() under mmu_lock to serialize
> >> + * against ongoing attribute updates.
> >> + */
> >> + return kvm_gmem_get_attributes(inode, kvm_gmem_get_index(slot, gfn));
> >> +}
> >
> > Doesn't this imply that all consumers of kvm_mem_is_private() should
> > validate the result using mmu_lock and the invalidation sequence?
>
> Let me know how I can improve the comment.
Given Sean's context, the comment is good I think. I would quibble
with the the "_must_ still protect" phrasing being a bit too strict.
Maybe just soften it slightly to acknowledge the exception? Something like:
* lock is dropped, so callers that require a strict result _must_ protect
* consumption of private vs. shared by checking mmu_invalidate_retry_gfn()
* under mmu_lock to serialize against ongoing attribute updates. Callers
* doing lockless reads must be able to tolerate a stale result.
That aligns the comment with how KVM is actually using it today. That
said, this is nitpicking. Feel free to use or ignore.
>
> I think the "consumption" of private vs shared here actually means
> something like "don't commit a page being faulted into page tables based
> on the result of kvm_gmem_get_memory_attributes() without checking
> kvm->mmu_invalidate_in_progress.", since a racing conversion may
> complete before you commit.
>
> kvm_mem_is_private() is used from these places:
>
> 1. Fault handling in KVM, like page_fault_can_be_fast(),
> kvm_mmu_faultin_pfn(), kvm_mmu_page_fault(): this already handles the
> entire mmu_lock and invalidation dance. No fault will be committed if
> a racing conversion happened after kvm_mem_is_private() but before
> the commit.
>
> 2. kvm_mmu_max_mapping_level() from recovering huge pages after
> disabling dirty logging: Other than that it can't be used with
> guest_memfd now since dirty logging can't be used with guest_memfd
> and guest_memfd memslots are not updatable, this holds mmu_lock
> throughout until the huge page recovery is done. invalidate_begin
> also involves zapping the pages in the range, so if the order of
> events is
>
> | Thread A | Thread B |
> |------------------------------|-------------------|
> | invalidate_begin + zap | |
> | update attributes maple_tree | recover huge page |
> | invalidate_end | |
>
> Then recovering will never see the zapped pages, nothing to
> recover, no kvm_mem_is_private() lookup.
>
> 3. kvm_arch_vcpu_pre_fault_memory()
>
> This eventually calls kvm_tdp_mmu_page_fault(), which checks
> is_page_fault_stale(), so it does check before committing.
>
> Were there any other calls I missed?
The one I was looking at was `sev_handle_rmp_fault()`, which does a lockless
read without the retry loop. But as Sean just pointed out, that path can
tolerate false positives/negatives and relies on the guest faulting again,
so the lack of synchronization there is existing behavior and considered "fine".
>
> > sev_handle_rmp_fault() calls kvm_mem_is_private() without holding
> > mmu_lock and without any retry mechanism. Is that a problem?
> >
>
> Sean already replied on your actual question separately :)
>
> > Cheers,
> > /fuad
> >
> >
> >>
> >> [...snip...]
> >>
^ permalink raw reply
* Re: [PATCH v6 15/43] KVM: guest_memfd: Handle lru_add fbatch refcounts during conversion safety check
From: Fuad Tabba @ 2026-05-21 7:13 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, 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, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260507-gmem-inplace-conversion-v6-15-91ab5a8b19a4@google.com>
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> 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. This can lead to a false positive,
> incorrectly indicating that the folio is in use and preventing the
> conversion, even if it is otherwise safe. The conversion process might not
> be on the same CPU that holds the folio in its fbatch, making a simple
> per-CPU check insufficient.
>
> To address this, drain all CPUs' lru_add fbatches if an unexpectedly high
> refcount is encountered during the safety check. This is performed at most
> once per conversion request. Draining only if the folio in question may be
> lru cached.
>
> guest_memfd folios are unevictable, so they can only reside in the lru_add
> fbatch. If the folio's refcount is still unsafe after draining, then the
> conversion is truly deemed unsafe.
>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Not an area I've worked with that much, but it seems right to me:
Reviewed-by: Fuad Tabba <tabba@google.com>
Cheers,
/fuad
> ---
> mm/swap.c | 2 ++
> virt/kvm/guest_memfd.c | 18 ++++++++++++++----
> 2 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/mm/swap.c b/mm/swap.c
> index 5cc44f0de9877..3134d9d3d7c30 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -37,6 +37,7 @@
> #include <linux/page_idle.h>
> #include <linux/local_lock.h>
> #include <linux/buffer_head.h>
> +#include <linux/kvm_types.h>
>
> #include "internal.h"
>
> @@ -904,6 +905,7 @@ void lru_add_drain_all(void)
> lru_add_drain();
> }
> #endif /* CONFIG_SMP */
> +EXPORT_SYMBOL_FOR_KVM(lru_add_drain_all);
>
> atomic_t lru_disable_count = ATOMIC_INIT(0);
>
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 034b72b4947fb..050a8c092b1a3 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -8,6 +8,7 @@
> #include <linux/mempolicy.h>
> #include <linux/pseudo_fs.h>
> #include <linux/pagemap.h>
> +#include <linux/swap.h>
>
> #include "kvm_mm.h"
>
> @@ -596,18 +597,27 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> const int filemap_get_folios_refcount = 1;
> pgoff_t last = start + nr_pages - 1;
> struct folio_batch fbatch;
> + bool lru_drained = false;
> bool safe = true;
> int i;
>
> folio_batch_init(&fbatch);
> while (safe && filemap_get_folios(mapping, &start, 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();
> + lru_drained = true;
> + } else {
> *err_index = folio->index;
> break;
> }
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v6 11/43] KVM: guest_memfd: Ensure pages are not in use before conversion
From: Fuad Tabba @ 2026-05-21 7:09 UTC (permalink / raw)
To: ackerleytng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, 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, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260507-gmem-inplace-conversion-v6-11-91ab5a8b19a4@google.com>
Hi Ackerley,
On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
<devnull+ackerleytng.google.com@kernel.org> wrote:
>
> From: Ackerley Tng <ackerleytng@google.com>
>
> When converting memory to private in guest_memfd, it is necessary to ensure
> that the pages are not currently being accessed by any other part of the
> kernel or userspace to avoid any current user writing to guest private
> memory.
>
> guest_memfd checks for unexpected refcounts to determine whether a page is
> still in use. The only expected refcounts after unmapping the range
> requested for conversion are those that are held by guest_memfd itself.
>
> Update the kvm_memory_attributes2 structure to include an error_offset
> field. This allows KVM to report the exact offset where a conversion
> failed to userspace. If the safety check fails, return -EAGAIN and copy
> the error_offset back to userspace so that it can potentially retry the
> operation or handle the failure gracefully.
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> ---
> include/uapi/linux/kvm.h | 3 ++-
> virt/kvm/guest_memfd.c | 65 ++++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 62 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index e6bbf68a83813..0b55258573d3d 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1658,7 +1658,8 @@ struct kvm_memory_attributes2 {
> __u64 size;
> __u64 attributes;
> __u64 flags;
> - __u64 reserved[12];
> + __u64 error_offset;
> + __u64 reserved[11];
> };
>
> #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 91e89b188f583..9d82642a025e9 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -572,9 +572,42 @@ static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
> return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
> }
>
> +static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
> + size_t nr_pages, pgoff_t *err_index)
> +{
> + struct address_space *mapping = inode->i_mapping;
> + const int filemap_get_folios_refcount = 1;
> + pgoff_t last = start + nr_pages - 1;
> + struct folio_batch fbatch;
> + bool safe = true;
> + int i;
> +
> + folio_batch_init(&fbatch);
> + while (safe && filemap_get_folios(mapping, &start, last, &fbatch)) {
> +
> + for (i = 0; i < folio_batch_count(&fbatch); ++i) {
> + struct folio *folio = fbatch.folios[i];
> +
> + if (folio_ref_count(folio) !=
> + folio_nr_pages(folio) + filemap_get_folios_refcount) {
> + safe = false;
> + *err_index = folio->index;
> + break;
https://sashiko.dev/#/patchset/20260507-gmem-inplace-conversion-v6-0-91ab5a8b19a4%40google.com?part=11
Sashiko raised a few issues here, but I think this one might be
genuine. Can you look into it please?
If that's right, when huge page support lands, if start falls in the
middle of a large folio, returning folio->index as the err_index will
return an offset strictly less than the requested start. A naive
userspace retry loop resuming from error_offset would step backwards
and corrupt attributes on memory it didn't intend to convert.
err_index should be clamped to max(start, folio->index).
Cheers,
/fuad
> + }
> + }
> +
> + folio_batch_release(&fbatch);
> + cond_resched();
> + }
> +
> + return safe;
> +}
> +
> static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> - size_t nr_pages, uint64_t attrs)
> + size_t nr_pages, uint64_t attrs,
> + pgoff_t *err_index)
> {
> + bool to_private = attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE;
> struct address_space *mapping = inode->i_mapping;
> struct gmem_inode *gi = GMEM_I(inode);
> pgoff_t end = start + nr_pages;
> @@ -588,8 +621,21 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>
> mas_init(&mas, mt, start);
> r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
> - if (r)
> + if (r) {
> + *err_index = start;
> goto out;
> + }
> +
> + if (to_private) {
> + unmap_mapping_pages(mapping, start, nr_pages, false);
> +
> + if (!kvm_gmem_is_safe_for_conversion(inode, start, nr_pages,
> + err_index)) {
> + mas_destroy(&mas);
> + r = -EAGAIN;
> + goto out;
> + }
> + }
>
> /*
> * From this point on guest_memfd has performed necessary
> @@ -609,9 +655,10 @@ static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
> struct gmem_file *f = file->private_data;
> struct inode *inode = file_inode(file);
> struct kvm_memory_attributes2 attrs;
> + pgoff_t err_index;
> size_t nr_pages;
> pgoff_t index;
> - int i;
> + int i, r;
>
> if (copy_from_user(&attrs, argp, sizeof(attrs)))
> return -EFAULT;
> @@ -635,8 +682,16 @@ static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
>
> nr_pages = attrs.size >> PAGE_SHIFT;
> index = attrs.offset >> PAGE_SHIFT;
> - return __kvm_gmem_set_attributes(inode, index, nr_pages,
> - attrs.attributes);
> + r = __kvm_gmem_set_attributes(inode, index, nr_pages, attrs.attributes,
> + &err_index);
> + if (r) {
> + attrs.error_offset = ((uint64_t)err_index) << PAGE_SHIFT;
> +
> + if (copy_to_user(argp, &attrs, sizeof(attrs)))
> + return -EFAULT;
> + }
> +
> + return r;
> }
>
> static long kvm_gmem_ioctl(struct file *file, unsigned int ioctl,
>
> --
> 2.54.0.563.g4f69b47b94-goog
>
>
^ permalink raw reply
* Re: [PATCH v14 10/44] arm64: RMI: Add support for SRO
From: Gavin Shan @ 2026-05-21 4:38 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Suzuki K Poulose, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve,
WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-11-steven.price@arm.com>
Hi Steven,
On 5/13/26 11:17 PM, Steven Price wrote:
> RMM v2.0 introduces the concept of "Stateful RMI Operations" (SRO). This
> means that an SMC can return with an operation still in progress. The
> host is excepted to continue the operation until is reaches a conclusion
> (either success or failure). During this process the RMM can request
> additional memory ('donate') or hand memory back to the host
> ('reclaim'). The host can request an in progress operation is cancelled,
> but still continue the operation until it has completed (otherwise the
> incomplete operation may cause future RMM operations to fail).
>
> The SRO is tracked using a struct rmi_sro_state object which keeps track
> of any memory which has been allocated but not yet consumed by the RMM
> or reclaimed from the RMM. This allows the memory to be reused in a
> future request within the same operation. It will also permit an
> operation to be done in a context where memory allocation may be
> difficult (e.g. atomic context) with the option to abort the operation
> and retry the memory allocation outside of the atomic context. The
> memory stored in the struct rmi_sro_state object can then be reused on
> the subsequent attempt.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> v14:
> * SRO support has improved although is still not fully complete. The
> infrastructure has been moved out of KVM.
> ---
> arch/arm64/include/asm/rmi_cmds.h | 1 +
> arch/arm64/kernel/rmi.c | 359 ++++++++++++++++++++++++++++++
> 2 files changed, 360 insertions(+)
>
> diff --git a/arch/arm64/include/asm/rmi_cmds.h b/arch/arm64/include/asm/rmi_cmds.h
> index eb213c8e6f26..1a7b0c8f1e38 100644
> --- a/arch/arm64/include/asm/rmi_cmds.h
> +++ b/arch/arm64/include/asm/rmi_cmds.h
> @@ -35,6 +35,7 @@ struct rmi_sro_state {
>
> int rmi_delegate_range(phys_addr_t phys, unsigned long size);
> int rmi_undelegate_range(phys_addr_t phys, unsigned long size);
> +int free_delegated_page(phys_addr_t phys);
>
> static inline int rmi_delegate_page(phys_addr_t phys)
> {
> diff --git a/arch/arm64/kernel/rmi.c b/arch/arm64/kernel/rmi.c
> index 08cef54acadb..a8107ca9bb6d 100644
> --- a/arch/arm64/kernel/rmi.c
> +++ b/arch/arm64/kernel/rmi.c
> @@ -48,6 +48,365 @@ int rmi_undelegate_range(phys_addr_t phys, unsigned long size)
> return ret;
> }
>
> +static unsigned long donate_req_to_size(unsigned long donatereq)
> +{
> + unsigned long unit_size = RMI_DONATE_SIZE(donatereq);
> +
> + switch (unit_size) {
> + case 0:
> + return PAGE_SIZE;
> + case 1:
> + return PMD_SIZE;
> + case 2:
> + return PUD_SIZE;
> + case 3:
> + return P4D_SIZE;
> + }
> + unreachable();
> +}
> +
It's worthy to have 'inline'. {P4D, PUD, PMD}_SIZE can be equal if there are
no P4D and PUD, depending on CONFIG_PGTABLE_LEVELS. In this case, can the
'unit_size' be translated to wrong value?
> +static void rmi_smccc_invoke(struct arm_smccc_1_2_regs *regs_in,
> + struct arm_smccc_1_2_regs *regs_out)
> +{
> + struct arm_smccc_1_2_regs regs = *regs_in;
> + unsigned long status;
> +
> + do {
> + arm_smccc_1_2_invoke(®s, regs_out);
> + status = RMI_RETURN_STATUS(regs_out->a0);
> + } while (status == RMI_BUSY || status == RMI_BLOCKED);
> +}
> +
> +int free_delegated_page(phys_addr_t phys)
> +{
> + if (WARN_ON(rmi_undelegate_page(phys))) {
> + /* Undelegate failed: leak the page */
> + return -EBUSY;
> + }
> +
> + free_page((unsigned long)phys_to_virt(phys));
> +
> + return 0;
> +}
> +
> +static int rmi_sro_ensure_capacity(struct rmi_sro_state *sro,
> + unsigned long count)
> +{
> + if (WARN_ON_ONCE(sro->addr_count > RMI_MAX_ADDR_LIST))
> + return -EOVERFLOW;
> +
> + if (count > RMI_MAX_ADDR_LIST - sro->addr_count)
> + return -ENOSPC;
> +
> + return 0;
> +}
> +
> +static int rmi_sro_donate_contig(struct rmi_sro_state *sro,
> + unsigned long sro_handle,
> + unsigned long donatereq,
> + struct arm_smccc_1_2_regs *out_regs,
> + gfp_t gfp)
> +{
> + unsigned long unit_size = RMI_DONATE_SIZE(donatereq);
> + unsigned long unit_size_bytes = donate_req_to_size(donatereq);
> + unsigned long count = RMI_DONATE_COUNT(donatereq);
> + unsigned long state = RMI_DONATE_STATE(donatereq);
> + unsigned long size = unit_size_bytes * count;
> + unsigned long addr_range;
> + int ret;
> + void *virt;
> + phys_addr_t phys;
> + struct arm_smccc_1_2_regs regs = {
> + SMC_RMI_OP_MEM_DONATE,
> + sro_handle
> + };
> +
> + for (int i = 0; i < sro->addr_count; i++) {
> + unsigned long entry = sro->addr_list[i];
> +
> + if (RMI_ADDR_RANGE_SIZE(entry) == unit_size &&
> + RMI_ADDR_RANGE_COUNT(entry) == count &&
> + RMI_ADDR_RANGE_STATE(entry) == state) {
> + sro->addr_count--;
> + swap(sro->addr_list[sro->addr_count],
> + sro->addr_list[i]);
> +
> + goto out;
> + }
> + }
> +
> + ret = rmi_sro_ensure_capacity(sro, 1);
> + if (ret)
> + return ret;
> +
> + virt = alloc_pages_exact(size, gfp);
> + if (!virt)
> + return -ENOMEM;
> + phys = virt_to_phys(virt);
> +
alloc_pages_exact() will fail if the requested size exceeds the maximal allowed
size (1 << MAX_PAGE_ORDER). The maximal size is usually smaller than PUD_SIZE
but PUD_SIZE is allowed by the RMM.
> + if (state == RMI_OP_MEM_DELEGATED) {
> + if (rmi_delegate_range(phys, size)) {
> + free_pages_exact(virt, size);
> + return -ENXIO;
> + }
> + }
> +
> + addr_range = phys & RMI_ADDR_RANGE_ADDR_MASK;
> + FIELD_MODIFY(RMI_ADDR_RANGE_SIZE_MASK, &addr_range, unit_size);
> + FIELD_MODIFY(RMI_ADDR_RANGE_COUNT_MASK, &addr_range, count);
> + FIELD_MODIFY(RMI_ADDR_RANGE_STATE_MASK, &addr_range, state);
> +
> + sro->addr_list[sro->addr_count] = addr_range;
> +
> +out:
> + regs.a2 = virt_to_phys(&sro->addr_list[sro->addr_count]);
> + regs.a3 = 1;
> + rmi_smccc_invoke(®s, out_regs);
> +
> + unsigned long donated_granules = out_regs->a1;
> + unsigned long donated_size = donated_granules << PAGE_SHIFT;
> +
> + if (donated_granules == 0) {
> + /* No pages used by the RMM */
> + sro->addr_count++;
> + } else if (donated_size < size) {
> + phys = sro->addr_list[sro->addr_count] & RMI_ADDR_RANGE_ADDR_MASK;
> +
> + /* Not all granules used by the RMM, free the remaining pages */
> + for (long i = donated_size; i < size; i += PAGE_SIZE) {
> + if (state == RMI_OP_MEM_DELEGATED)
> + free_delegated_page(phys + i);
> + else
> + __free_page(phys_to_page(phys + i));
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int rmi_sro_donate_noncontig(struct rmi_sro_state *sro,
> + unsigned long sro_handle,
> + unsigned long donatereq,
> + struct arm_smccc_1_2_regs *out_regs,
> + gfp_t gfp)
> +{
> + unsigned long unit_size = RMI_DONATE_SIZE(donatereq);
> + unsigned long unit_size_bytes = donate_req_to_size(donatereq);
> + unsigned long count = RMI_DONATE_COUNT(donatereq);
> + unsigned long state = RMI_DONATE_STATE(donatereq);
> + unsigned long found = 0;
> + unsigned long addr_list_start = sro->addr_count;
> + int ret;
> + struct arm_smccc_1_2_regs regs = {
> + SMC_RMI_OP_MEM_DONATE,
> + sro_handle
> + };
> +
> + for (int i = 0; i < addr_list_start && found < count; i++) {
> + unsigned long entry = sro->addr_list[i];
> +
> + if (RMI_ADDR_RANGE_SIZE(entry) == unit_size &&
> + RMI_ADDR_RANGE_COUNT(entry) == 1 &&
> + RMI_ADDR_RANGE_STATE(entry) == state) {
> + addr_list_start--;
> + swap(sro->addr_list[addr_list_start],
> + sro->addr_list[i]);
> + found++;
> + i--;
> + }
> + }
> +
> + ret = rmi_sro_ensure_capacity(sro, count - found);
> + if (ret)
> + return ret;
> +
> + while (found < count) {
> + unsigned long addr_range;
> + void *virt = alloc_pages_exact(unit_size_bytes, gfp);
> + phys_addr_t phys;
> +
> + if (!virt)
> + return -ENOMEM;
> +
> + phys = virt_to_phys(virt);
> +
> + if (state == RMI_OP_MEM_DELEGATED) {
> + if (rmi_delegate_range(phys, unit_size_bytes)) {
> + free_pages_exact(virt, unit_size_bytes);
> + return -ENXIO;
> + }
> + }
> +
> + addr_range = phys & RMI_ADDR_RANGE_ADDR_MASK;
> + FIELD_MODIFY(RMI_ADDR_RANGE_SIZE_MASK, &addr_range, unit_size);
> + FIELD_MODIFY(RMI_ADDR_RANGE_COUNT_MASK, &addr_range, 1);
> + FIELD_MODIFY(RMI_ADDR_RANGE_STATE_MASK, &addr_range, state);
> +
> + sro->addr_list[sro->addr_count++] = addr_range;
> + found++;
> + }
> +
> + regs.a2 = virt_to_phys(&sro->addr_list[addr_list_start]);
> + regs.a3 = found;
> + rmi_smccc_invoke(®s, out_regs);
> +
> + unsigned long donated_granules = out_regs->a1;
> +
> + if (WARN_ON(donated_granules & ((unit_size_bytes >> PAGE_SHIFT) - 1))) {
> + /*
> + * FIXME: RMM has only consumed part of a huge page, this leaks
> + * the rest of the huge page
> + */
> + donated_granules = ALIGN(donated_granules,
> + (unit_size_bytes >> PAGE_SHIFT));
> + }
> + unsigned long donated_blocks = donated_granules / (unit_size_bytes >> PAGE_SHIFT);
> +
> + if (WARN_ON(donated_blocks > found))
> + donated_blocks = found;
> +
> + unsigned long undonated_blocks = found - donated_blocks;
> +
> + while (donated_blocks && undonated_blocks) {
> + sro->addr_count--;
> + swap(sro->addr_list[addr_list_start],
> + sro->addr_list[sro->addr_count]);
> + addr_list_start++;
> +
> + donated_blocks--;
> + undonated_blocks--;
> + }
> + sro->addr_count -= donated_blocks;
> +
> + return 0;
> +}
> +
> +static int rmi_sro_donate(struct rmi_sro_state *sro,
> + unsigned long sro_handle,
> + unsigned long donatereq,
> + struct arm_smccc_1_2_regs *regs,
> + gfp_t gfp)
> +{
> + unsigned long count = RMI_DONATE_COUNT(donatereq);
> +
> + if (WARN_ON(!count))
> + return 0;
> +
> + if (RMI_DONATE_CONTIG(donatereq)) {
> + return rmi_sro_donate_contig(sro, sro_handle, donatereq,
> + regs, gfp);
> + } else {
> + return rmi_sro_donate_noncontig(sro, sro_handle, donatereq,
> + regs, gfp);
> + }
> +}
> +
> +static int rmi_sro_reclaim(struct rmi_sro_state *sro,
> + unsigned long sro_handle,
> + struct arm_smccc_1_2_regs *out_regs)
> +{
> + unsigned long capacity;
> + struct arm_smccc_1_2_regs regs;
> + int ret;
> +
> + ret = rmi_sro_ensure_capacity(sro, 1);
> + if (ret)
> + rmi_sro_free(sro);
> +
> + capacity = RMI_MAX_ADDR_LIST - sro->addr_count;
> +
> + regs = (struct arm_smccc_1_2_regs){
> + SMC_RMI_OP_MEM_RECLAIM,
> + sro_handle,
> + virt_to_phys(&sro->addr_list[sro->addr_count]),
> + capacity
> + };
> + rmi_smccc_invoke(®s, out_regs);
> +
> + if (WARN_ON_ONCE(out_regs->a1 > capacity))
> + out_regs->a1 = capacity;
> +
> + sro->addr_count += out_regs->a1;
> +
> + return 0;
> +}
> +
> +void rmi_sro_free(struct rmi_sro_state *sro)
> +{
> + for (int i = 0; i < sro->addr_count; i++) {
> + unsigned long entry = sro->addr_list[i];
> + unsigned long addr = RMI_ADDR_RANGE_ADDR(entry);
> + unsigned long unit_size = RMI_ADDR_RANGE_SIZE(entry);
> + unsigned long count = RMI_ADDR_RANGE_COUNT(entry);
> + unsigned long state = RMI_ADDR_RANGE_STATE(entry);
> + unsigned long size = donate_req_to_size(unit_size) * count;
> +
> + if (state == RMI_OP_MEM_DELEGATED) {
> + if (WARN_ON(rmi_undelegate_range(addr, size))) {
> + /* Leak the pages */
> + continue;
> + }
> + }
> + free_pages_exact(phys_to_virt(addr), size);
> + }
> +
> + sro->addr_count = 0;
> +}
> +
> +unsigned long rmi_sro_execute(struct rmi_sro_state *sro, gfp_t gfp)
> +{
> + unsigned long sro_handle;
> + struct arm_smccc_1_2_regs regs;
> + struct arm_smccc_1_2_regs *regs_in = &sro->regs;
> +
> + rmi_smccc_invoke(regs_in, ®s);
> +
> + sro_handle = regs.a1;
> +
> + while (RMI_RETURN_STATUS(regs.a0) == RMI_INCOMPLETE) {
> + bool can_cancel = RMI_RETURN_CAN_CANCEL(regs.a0);
> + int ret;
> +
> + switch (RMI_RETURN_MEMREQ(regs.a0)) {
> + case RMI_OP_MEM_REQ_NONE:
> + regs = (struct arm_smccc_1_2_regs){
> + SMC_RMI_OP_CONTINUE, sro_handle, 0
> + };
> + rmi_smccc_invoke(®s, ®s);
> + break;
'ret' isn't initialized for case RMI_OP_MEM_REQ_NONE.
> + case RMI_OP_MEM_REQ_DONATE:
> + ret = rmi_sro_donate(sro, sro_handle, regs.a2, ®s,
> + gfp);
> + break;
> + case RMI_OP_MEM_REQ_RECLAIM:
> + ret = rmi_sro_reclaim(sro, sro_handle, ®s);
> + break;
> + default:
> + ret = WARN_ON(1);
> + break;
> + }
> +
> + if (ret) {
> + if (can_cancel) {
> + /*
> + * FIXME: Handle cancelling properly!
> + *
> + * If the operation has failed due to memory
> + * allocation failure then the information on
> + * the memory allocation should be saved, so
> + * that the allocation can be repeated outside
> + * of any context which prevented the
> + * allocation.
> + */
> + }
> + if (WARN_ON(ret))
> + return ret;
> + }
> + }
> +
> + return regs.a0;
> +}
> +
> static int rmi_check_version(void)
> {
> struct arm_smccc_res res;
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v14 08/44] arm64: RMI: Ensure that the RMM has GPT entries for memory
From: Gavin Shan @ 2026-05-21 0:58 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Suzuki K Poulose, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve,
WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-9-steven.price@arm.com>
Hi Steven,
On 5/13/26 11:17 PM, Steven Price wrote:
> The RMM maintains the state of all the granules in the system to make
> sure that the host is abiding by the rules. This state can be maintained
> at different granularity, per page (TRACKING_FINE) or per region
> (TRACKING_COARSE). The region size depends on the underlying
> "RMI_GRANULE_SIZE". For a "coarse" region all pages in the region must
> be of the same state, this implies we need to have "fine" tracking for
> DRAM, so that we can delegated individual pages.
>
> For now we only support a statically carved out memory for tracking
> granules for the "fine" regions. This can be extended in the future to
> allow modifying the tracking granularity and remove the need for a
> static allocation.
>
> Similarly, the firmware may create L0 GPT entries describing the total
> address space. But if we change the "PAS" (Physical Address Space) of a
> granule then the firmware may need to create L1 tables to track the PAS
> at a finer granularity.
>
> Note: support is currently missing for SROs which means that if the RMM
> needs memory donating this will fail (and render CCA unusable in Linux).
> This effectively means that the L1 GPT tables must be created before
> Linux starts.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * Moved out of KVM
> ---
> arch/arm64/include/asm/rmi_cmds.h | 2 +
> arch/arm64/kernel/rmi.c | 103 ++++++++++++++++++++++++++++++
> 2 files changed, 105 insertions(+)
>
> diff --git a/arch/arm64/include/asm/rmi_cmds.h b/arch/arm64/include/asm/rmi_cmds.h
> index 9179934925c5..9078a2920a7c 100644
> --- a/arch/arm64/include/asm/rmi_cmds.h
> +++ b/arch/arm64/include/asm/rmi_cmds.h
> @@ -33,6 +33,8 @@ struct rmi_sro_state {
> } while (RMI_RETURN_STATUS(res.a0) == RMI_BUSY || \
> RMI_RETURN_STATUS(res.a0) == RMI_BLOCKED)
>
> +bool rmi_is_available(void);
> +
> unsigned long rmi_sro_execute(struct rmi_sro_state *sro, gfp_t gfp);
> void rmi_sro_free(struct rmi_sro_state *sro);
>
> diff --git a/arch/arm64/kernel/rmi.c b/arch/arm64/kernel/rmi.c
> index a14ead5dedda..52a415e99500 100644
> --- a/arch/arm64/kernel/rmi.c
> +++ b/arch/arm64/kernel/rmi.c
> @@ -7,6 +7,8 @@
>
> #include <asm/rmi_cmds.h>
>
> +static bool arm64_rmi_is_available;
> +
> unsigned long rmm_feat_reg0;
> unsigned long rmm_feat_reg1;
>
> @@ -88,6 +90,102 @@ static int rmi_configure(void)
> return 0;
> }
>
> +/*
> + * For now we set the tracking_region_size to 0 for RMI_RMM_CONFIG_SET().
> + * TODO: Support other tracking sizes (via Kconfig option).
> + */
> +#ifdef CONFIG_PAGE_SIZE_4KB
> +#define RMM_GRANULE_TRACKING_SIZE SZ_1G
> +#elif defined(CONFIG_PAGE_SIZE_16KB)
> +#define RMM_GRANULE_TRACKING_SIZE SZ_32M
> +#elif defined(CONFIG_PAGE_SIZE_64KB)
> +#define RMM_GRANULE_TRACKING_SIZE SZ_512M
> +#endif
> +
RMM_GRANULE_TRACKING_SIZE is never used in this series.
> +/*
> + * Make sure the area is tracked by RMM at FINE granularity.
> + * We do not support changing the tracking yet.
> + */
> +static int rmi_verify_memory_tracking(phys_addr_t start, phys_addr_t end)
> +{
> + while (start < end) {
> + unsigned long ret, category, state, next;
> +
> + ret = rmi_granule_tracking_get(start, end, &category, &state, &next);
> + if (ret != RMI_SUCCESS ||
> + state != RMI_TRACKING_FINE ||
> + category != RMI_MEM_CATEGORY_CONVENTIONAL) {
> + /* TODO: Set granule tracking in this case */
> + pr_err("Granule tracking for region isn't fine/conventional: %llx",
> + start);
> + return -ENODEV;
> + }
> + start = next;
> + }
> +
> + return 0;
> +}
> +
> +static unsigned long rmi_l0gpt_size(void)
> +{
> + return 1UL << (30 + FIELD_GET(RMI_FEATURE_REGISTER_1_L0GPTSZ,
> + rmm_feat_reg1));
> +}
> +
rmi_l0gpt_size() is only used by rmi_create_gpts(), its logic can be
combined to that function.
> +static int rmi_create_gpts(phys_addr_t start, phys_addr_t end)
> +{
> + unsigned long l0gpt_sz = rmi_l0gpt_size();
> +
> + start = ALIGN_DOWN(start, l0gpt_sz);
> + end = ALIGN(end, l0gpt_sz);
> +
> + while (start < end) {
> + int ret = rmi_gpt_l1_create(start);
> +
> + /*
> + * Make sure the L1 GPT tables are created for the region.
> + * RMI_ERROR_GPT indicates the L1 table already exists.
> + */
> + if (ret && ret != RMI_ERROR_GPT) {
> + /*
> + * FIXME: Handle SRO so that memory can be donated for
> + * the tables.
> + */
> + pr_err("GPT Level1 table missing for %llx\n", start);
> + return -ENOMEM;
> + }
> + start += l0gpt_sz;
> + }
> +
> + return 0;
> +}
> +
> +static int rmi_init_metadata(void)
> +{
> + phys_addr_t start, end;
> + const struct memblock_region *r;
> +
> + for_each_mem_region(r) {
> + int ret;
> +
> + start = memblock_region_memory_base_pfn(r) << PAGE_SHIFT;
> + end = memblock_region_memory_end_pfn(r) << PAGE_SHIFT;
> + ret = rmi_verify_memory_tracking(start, end);
> + if (ret)
> + return ret;
> + ret = rmi_create_gpts(start, end);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +bool rmi_is_available(void)
> +{
> + return arm64_rmi_is_available;
> +}
> +
> static int __init arm64_init_rmi(void)
> {
> /* Continue without realm support if we can't agree on a version */
> @@ -101,6 +199,11 @@ static int __init arm64_init_rmi(void)
>
> if (rmi_configure())
> return 0;
> + if (rmi_init_metadata())
> + return 0;
> +
> + arm64_rmi_is_available = true;
> + pr_info("RMI configured");
>
> return 0;
> }
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v14 07/44] arm64: RMI: Configure the RMM with the host's page size
From: Gavin Shan @ 2026-05-21 0:51 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Suzuki K Poulose, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve,
WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-8-steven.price@arm.com>
Hi Steven,
On 5/13/26 11:17 PM, Steven Price wrote:
> RMM v2.0 brings the ability to set the RMM's granule size. Check the
> feature registers and configure the RMM so that it matches the host's
> page size. This means that operations can be done with a granulatity
> equal to PAGE_SIZE.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v13:
> * Moved out of KVM.
> ---
> arch/arm64/kernel/rmi.c | 42 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/arm64/kernel/rmi.c b/arch/arm64/kernel/rmi.c
> index 99c1ccc35c11..a14ead5dedda 100644
> --- a/arch/arm64/kernel/rmi.c
> +++ b/arch/arm64/kernel/rmi.c
> @@ -49,6 +49,45 @@ static int rmi_check_version(void)
> return 0;
> }
>
> +static int rmi_configure(void)
> +{
> + struct rmm_config *config __free(free_page) = NULL;
> + unsigned long ret;
> +
> + config = (struct rmm_config *)get_zeroed_page(GFP_KERNEL);
> + if (!config)
> + return -ENOMEM;
> +
> + switch (PAGE_SIZE) {
> + case SZ_4K:
> + config->rmi_granule_size = RMI_GRANULE_SIZE_4KB;
> + break;
> + case SZ_16K:
> + config->rmi_granule_size = RMI_GRANULE_SIZE_16KB;
> + break;
> + case SZ_64K:
> + config->rmi_granule_size = RMI_GRANULE_SIZE_64KB;
> + break;
> + default:
> + pr_err("Unsupported PAGE_SIZE for RMM\n");
> + return -EINVAL;
> + }
> +
> + ret = rmi_rmm_config_set(virt_to_phys(config));
> + if (ret) {
> + pr_err("RMM config set failed\n");
> + return -EINVAL;
> + }
> +
Looking at branch 'topics/rmm-v2.0-poc_2' of RMM implementation, the granule size
is fixed to be 4KB at present. I'm not sure if I have looked into correct RMM
implementation, but 'topics/rmm-v2.0-poc_2' is recommended one in the cover
letter.
Besides, there has checks in the handler of the RMI command to make sure that
struct rmm_config::tracking_region_size to be 1GB, indicated by zero. It maybe
worthy to set it before call to rmi_rmm_config_set().
config.tracking_region_size = 0; /* 1GB */
ret = rmi_rmm_config_set(virt_to_phys(config));
> + ret = rmi_rmm_activate();
> + if (ret) {
> + pr_err("RMM activate failed\n");
> + return -ENXIO;
> + }
> +
> + return 0;
> +}
> +
> static int __init arm64_init_rmi(void)
> {
> /* Continue without realm support if we can't agree on a version */
> @@ -60,6 +99,9 @@ static int __init arm64_init_rmi(void)
> if (WARN_ON(rmi_features(1, &rmm_feat_reg1)))
> return 0;
>
> + if (rmi_configure())
> + return 0;
> +
> return 0;
> }
> subsys_initcall(arm64_init_rmi);
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v14 06/44] arm64: RMI: Check for RMI support at init
From: Gavin Shan @ 2026-05-21 0:39 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Suzuki K Poulose, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve,
WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-7-steven.price@arm.com>
Hi Steven,
On 5/13/26 11:17 PM, Steven Price wrote:
> Query the RMI version number and check if it is a compatible version.
> The first two feature registers are read and exposed for future code to
> use.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> v14:
> * This moves the basic RMI setup into the 'kernel' directory. This is
> because RMI will be used for some features outside of KVM so should
> be available even if KVM isn't compiled in.
> ---
> arch/arm64/include/asm/rmi_cmds.h | 3 ++
> arch/arm64/kernel/Makefile | 2 +-
> arch/arm64/kernel/cpufeature.c | 1 +
> arch/arm64/kernel/rmi.c | 65 +++++++++++++++++++++++++++++++
> 4 files changed, 70 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm64/kernel/rmi.c
>
[...]
> diff --git a/arch/arm64/kernel/rmi.c b/arch/arm64/kernel/rmi.c
> new file mode 100644
> index 000000000000..99c1ccc35c11
> --- /dev/null
> +++ b/arch/arm64/kernel/rmi.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) 2023-2025 ARM Ltd.
> + */
> +
> +#include <linux/memblock.h>
> +
> +#include <asm/rmi_cmds.h>
> +
> +unsigned long rmm_feat_reg0;
> +unsigned long rmm_feat_reg1;
> +
> +static int rmi_check_version(void)
> +{
> + struct arm_smccc_res res;
> + unsigned short version_major, version_minor;
> + unsigned long host_version = RMI_ABI_VERSION(RMI_ABI_MAJOR_VERSION,
> + RMI_ABI_MINOR_VERSION);
> + unsigned long aa64pfr0 = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
> +
> + /* If RME isn't supported, then RMI can't be */
> + if (cpuid_feature_extract_unsigned_field(aa64pfr0, ID_AA64PFR0_EL1_RME_SHIFT) == 0)
> + return -ENXIO;
> +
> + arm_smccc_1_1_invoke(SMC_RMI_VERSION, host_version, &res);
> +
> + if (res.a0 == SMCCC_RET_NOT_SUPPORTED)
> + return -ENXIO;
> +
> + version_major = RMI_ABI_VERSION_GET_MAJOR(res.a1);
> + version_minor = RMI_ABI_VERSION_GET_MINOR(res.a1);
> +
> + if (res.a0 != RMI_SUCCESS) {
> + unsigned short high_version_major, high_version_minor;
> +
> + high_version_major = RMI_ABI_VERSION_GET_MAJOR(res.a2);
> + high_version_minor = RMI_ABI_VERSION_GET_MINOR(res.a2);
> +
> + pr_err("Unsupported RMI ABI (v%d.%d - v%d.%d) we want v%d.%d\n",
> + version_major, version_minor,
> + high_version_major, high_version_minor,
> + RMI_ABI_MAJOR_VERSION,
> + RMI_ABI_MINOR_VERSION);
> + return -ENXIO;
> + }
> +
> + pr_info("RMI ABI version %d.%d\n", version_major, version_minor);
> +
> + return 0;
> +}
> +
> +static int __init arm64_init_rmi(void)
> +{
> + /* Continue without realm support if we can't agree on a version */
> + if (rmi_check_version())
> + return 0;
Is this still a valid point that we have to return zero on errors returned
from rmi_check_version() or other other function calls like rmi_features()?
arm64_init_rmi() is triggered by subsys_initcall() where the return value
needs to indicate success or failure. It's fine to return error code from
arm64_init_rmi() in the path.
> +
> + if (WARN_ON(rmi_features(0, &rmm_feat_reg0)))
> + return 0;
> + if (WARN_ON(rmi_features(1, &rmm_feat_reg1)))
> + return 0;
> +
> + return 0;
> +}
> +subsys_initcall(arm64_init_rmi);
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v14 05/44] arm64: RMI: Add wrappers for RMI calls
From: Gavin Shan @ 2026-05-21 0:21 UTC (permalink / raw)
To: Steven Price, kvm, kvmarm
Cc: Catalin Marinas, Marc Zyngier, Will Deacon, James Morse,
Oliver Upton, Suzuki K Poulose, Zenghui Yu, linux-arm-kernel,
linux-kernel, Joey Gouly, Alexandru Elisei, Christoffer Dall,
Fuad Tabba, linux-coco, Ganapatrao Kulkarni, Shanker Donthineni,
Alper Gun, Aneesh Kumar K . V, Emi Kisanuki, Vishal Annapurve,
WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <20260513131757.116630-6-steven.price@arm.com>
Hi Steven,
On 5/13/26 11:17 PM, Steven Price wrote:
> The wrappers make the call sites easier to read and deal with the
> boiler plate of handling the error codes from the RMM.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes from v13:
> * Update to RMM v2.0-bet1 spec including some SRO support (there still
> some FIXMEs where SRO support is incomplete).
> Changes from v12:
> * Update to RMM v2.0 specification
> Changes from v8:
> * Switch from arm_smccc_1_2_smc() to arm_smccc_1_2_invoke() in
> rmi_rtt_read_entry() for consistency.
> Changes from v7:
> * Minor renaming of parameters and updated comments
> Changes from v5:
> * Further improve comments
> Changes from v4:
> * Improve comments
> Changes from v2:
> * Make output arguments optional.
> * Mask RIPAS value rmi_rtt_read_entry()
> * Drop unused rmi_rtt_get_phys()
> ---
> arch/arm64/include/asm/rmi_cmds.h | 661 ++++++++++++++++++++++++++++++
> 1 file changed, 661 insertions(+)
> create mode 100644 arch/arm64/include/asm/rmi_cmds.h
>
> diff --git a/arch/arm64/include/asm/rmi_cmds.h b/arch/arm64/include/asm/rmi_cmds.h
> new file mode 100644
> index 000000000000..04f7066894e9
> --- /dev/null
> +++ b/arch/arm64/include/asm/rmi_cmds.h
> @@ -0,0 +1,661 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (C) 2023 ARM Ltd.
> + */
> +
> +#ifndef __ASM_RMI_CMDS_H
> +#define __ASM_RMI_CMDS_H
> +
> +#include <linux/arm-smccc.h>
> +
[...]
> +
> +/**
> + * rmi_rtt_destroy() - Destroy an RTT
> + * @rd: PA of the RD
> + * @ipa: Base of the IPA range described by the RTT
> + * @level: Depth of the RTT within the tree
> + * @out_rtt: Pointer to write the PA of the RTT which was destroyed
> + * @out_top: Pointer to write the top IPA of non-live RTT entries
> + *
In most cases, the parameters are well explained in RMM-v2.0-bet1 spec, I think
it's nice to keep the code and the spec synchronized. For those specific parameters
of this function, they're well explained in RMM-v2.0-bet1 spec as below.
@rd: PA of the RD for the target realm
@ipa: Base of the IPA range described by the RTT
@level: RTT level
@out_rtt: PA of the RTT which was destroyed
@out_top: Top IPA of non-live RTT entries, from entry at which the RTT walk terminated
> + * Destroys an RTT. The RTT must be non-live, i.e. none of the entries in the
> + * table are in ASSIGNED or TABLE state.
> + *
> + * Return: RMI return code.
> + */
> +static inline int rmi_rtt_destroy(unsigned long rd,
> + unsigned long ipa,
> + long level,
> + unsigned long *out_rtt,
> + unsigned long *out_top)
> +{
> + struct arm_smccc_res res;
> +
> + arm_smccc_1_1_invoke(SMC_RMI_RTT_DESTROY, rd, ipa, level, &res);
> +
> + if (out_rtt)
> + *out_rtt = res.a1;
> + if (out_top)
> + *out_top = res.a2;
> +
> + return res.a0;
> +}
> +
[...]
Thanks,
Gavin
^ permalink raw reply
* Re: [PATCH v3 39/41] x86/paravirt: Move using_native_sched_clock() stub into timer.h
From: David Woodhouse @ 2026-05-21 0:00 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-40-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 382 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Now that timer.h ended up with CONFIG_PARAVIRT #ifdeffery anyways, move the
> PARAVIRT=n using_native_sched_clock() stub into timer.h as a "free"
> optimization.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 38/41] x86/paravirt: kvmclock: Setup kvmclock early iff it's sched_clock
From: David Woodhouse @ 2026-05-20 23:59 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-39-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 699 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Rework the seemingly generic x86_cpuinit_ops.early_percpu_clock_init hook
> into a dedicated PV sched_clock hook, as the only reason the hook exists
> is to allow kvmclock to enable its PV clock on secondary CPUs before the
> kernel tries to reference sched_clock, e.g. when grabbing a timestamp for
> printk.
>
> Rearranging the hook doesn't exactly reduce complexity; arguably it does
> the opposite. But as-is, it's practically impossible to understand *why*
> kvmclock needs to do early configuration.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 37/41] x86/kvmclock: Use TSC for sched_clock if it's constant and non-stop
From: David Woodhouse @ 2026-05-20 23:56 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-38-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1706 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Prefer the TSC over kvmclock for sched_clock if the TSC is constant,
> nonstop, and not marked unstable via command line. I.e. use the same
> criteria as tweaking the clocksource rating so that TSC is preferred over
> kvmclock. Per the below comment from native_sched_clock(), sched_clock
> is more tolerant of slop than clocksource; using TSC for clocksource but
> not sched_clock makes little to no sense, especially now that KVM CoCo
> guests with a trusted TSC use TSC, not kvmclock.
>
> /*
> * Fall back to jiffies if there's no TSC available:
> * ( But note that we still use it if the TSC is marked
> * unstable. We do this because unlike Time Of Day,
> * the scheduler clock tolerates small errors and it's
> * very important for it to be as fast as the platform
> * can achieve it. )
> */
>
> The only advantage of using kvmclock is that doing so allows for early
> and common detection of PVCLOCK_GUEST_STOPPED, but that code has been
> broken for over two years with nary a complaint, i.e. it can't be
> _that_ valuable. And as above, certain types of KVM guests are losing
> the functionality regardless, i.e. acknowledging PVCLOCK_GUEST_STOPPED
> needs to be decoupled from sched_clock() no matter what.
>
> Link: https://lore.kernel.org/all/Z4hDK27OV7wK572A@google.com
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Yay! (Albeit only for sched_clock, and we should do Xen too)
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 36/41] x86/kvmclock: Get local APIC bus frequency from PV CPUID Timing Info
From: Woodhouse, David @ 2026-05-20 23:55 UTC (permalink / raw)
To: tglx@kernel.org, longli@microsoft.com, luto@kernel.org,
alexey.makhalov@broadcom.com, jstultz@google.com,
dave.hansen@linux.intel.com, ajay.kaher@broadcom.com,
jan.kiszka@siemens.com, haiyangz@microsoft.com, kas@kernel.org,
seanjc@google.com, pbonzini@redhat.com, kys@microsoft.com,
decui@microsoft.com, daniel.lezcano@kernel.org,
wei.liu@kernel.org, peterz@infradead.org, jgross@suse.com
Cc: boris.ostrovsky@oracle.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, mhklinux@outlook.com,
thomas.lendacky@amd.com, linux-kernel@vger.kernel.org,
bcm-kernel-feedback-list@broadcom.com, tglx@linutronix.de,
nikunj@amd.com, xen-devel@lists.xenproject.org,
linux-hyperv@vger.kernel.org, vkuznets@redhat.com,
rick.p.edgecombe@intel.com, virtualization@lists.linux.dev,
sboyd@kernel.org, x86@kernel.org
In-Reply-To: <20260515191942.1892718-37-seanjc@google.com>
[-- Attachment #1.1: Type: text/plain, Size: 647 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> When running as a KVM guest with kvmclock support enabled, stuff the APIC
> timer period/frequency with the local APIC bus frequency reported in
> CPUID.0x40000010.EBX instead of trying to calibrate/guess the frequency.
>
> See Documentation/virt/kvm/x86/cpuid.rst for details.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
I still don't much like the way this is done inside kvm_get_tsc_khz().
We also probably ought to be looking for the timing leaf on other
hypervisors including VMware and probably Bhyve too. Should it be done
somewhere else?
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5964 bytes --]
[-- Attachment #2.1: Type: text/plain, Size: 215 bytes --]
Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
[-- Attachment #2.2: Type: text/html, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 33/41] x86/kvmclock: Mark TSC as reliable when it's constant and nonstop
From: David Woodhouse @ 2026-05-20 23:51 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-34-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1064 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Mark the TSC as reliable if the hypervisor (KVM) has enumerated the TSC
> as constant and nonstop, and the admin hasn't explicitly marked the TSC
> as unstable. Like most (all?) virtualization setups, any secondary
> clocksource that's used as a watchdog is guaranteed to be less reliable
> than a constant, nonstop TSC, as all clocksources the kernel uses as a
> watchdog are all but guaranteed to be emulated when running as a KVM
> guest. I.e. any observed discrepancies between the TSC and watchdog will
> be due to jitter in the watchdog.
>
> This is especially true for KVM, as the watchdog clocksource is usually
> emulated in host userspace, i.e. reading the clock incurs a roundtrip
> cost of thousands of cycles.
>
> Marking the TSC reliable addresses a flaw where the TSC will occasionally
> be marked unstable if the host is under moderate/heavy load.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 32/41] x86/tsc: Rejects attempts to override TSC calibration with lesser routine
From: David Woodhouse @ 2026-05-20 23:50 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-33-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 918 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> When registering a TSC frequency calibration routine, sanity check that
> the incoming routine is as robust as the outgoing routine, and reject the
> incoming routine if the sanity check fails.
>
> Because native calibration routines only mark the TSC frequency as known
> and reliable when they actually run, the effective progression of
> capabilities is: None (native) => Known and maybe Reliable (PV) =>
> Known and Reliable (CoCo). Violating that progression for a PV override
> is relatively benign, but messing up the progression when CoCo is
> involved is more problematic, as it likely means a trusted source of
> information (hardware/firmware) is being discarded in favor of a less
> trusted source (hypervisor).
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 31/41] x86/tsc: Pass KNOWN_FREQ and RELIABLE as params to registration
From: Woodhouse, David @ 2026-05-20 23:49 UTC (permalink / raw)
To: tglx@kernel.org, longli@microsoft.com, luto@kernel.org,
alexey.makhalov@broadcom.com, jstultz@google.com,
dave.hansen@linux.intel.com, ajay.kaher@broadcom.com,
jan.kiszka@siemens.com, haiyangz@microsoft.com, kas@kernel.org,
seanjc@google.com, pbonzini@redhat.com, kys@microsoft.com,
decui@microsoft.com, daniel.lezcano@kernel.org,
wei.liu@kernel.org, peterz@infradead.org, jgross@suse.com
Cc: boris.ostrovsky@oracle.com, linux-coco@lists.linux.dev,
kvm@vger.kernel.org, mhklinux@outlook.com,
thomas.lendacky@amd.com, linux-kernel@vger.kernel.org,
bcm-kernel-feedback-list@broadcom.com, tglx@linutronix.de,
nikunj@amd.com, xen-devel@lists.xenproject.org,
linux-hyperv@vger.kernel.org, vkuznets@redhat.com,
rick.p.edgecombe@intel.com, virtualization@lists.linux.dev,
sboyd@kernel.org, x86@kernel.org
In-Reply-To: <20260515191942.1892718-32-seanjc@google.com>
[-- Attachment #1.1: Type: text/plain, Size: 1363 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Add a "tsc_properties" set of flags and use it to annotate whether the
> TSC operates at a known and/or reliable frequency when registering a
> paravirtual TSC calibration routine. Currently, each PV flow manually
> sets the associated feature flags, but often in haphazard fashion that
> makes it difficult for unfamiliar readers to see the properties of the
> TSC when running under a particular hypervisor.
>
> The other, bigger issue with manually setting the feature flags is that
> it decouples the flags from the calibration routine. E.g. in theory, PV
> code could mark the TSC as having a known frequency, but then have its
> PV calibration discarded in favor of a method that doesn't use that known
> frequency. Passing the TSC properties along with the calibration routine
> will allow adding sanity checks to guard against replacing a "better"
> calibration routine with a "worse" routine.
>
> As a bonus, the flags also give developers working on new PV code a heads
> up that they should at least mark the TSC as having a known frequency.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #1.2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5964 bytes --]
[-- Attachment #2.1: Type: text/plain, Size: 215 bytes --]
Amazon Development Centre (London) Ltd. Registered in England and Wales with registration number 04543232 with its registered office at 1 Principal Place, Worship Street, London EC2A 2FA, United Kingdom.
[-- Attachment #2.2: Type: text/html, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 30/41] x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted TSC
From: David Woodhouse @ 2026-05-20 23:45 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-31-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Silently ignore attempts to switch to a paravirt sched_clock when running
> as a CoCo guest with trusted TSC. In hand-wavy theory, a misbehaving
> hypervisor could attack the guest by manipulating the PV clock to affect
> guest scheduling in some weird and/or predictable way. More importantly,
> reading TSC on such platforms is faster than any PV clock, and sched_clock
> is all about speed.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
And kvmclock. And Xen.
Are there *any* reasons we'd use a PV sched_clock when the TSC is
usable?
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
> arch/x86/kernel/tsc.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
> index 3c15fc10e501..ac4abfec1f05 100644
> --- a/arch/x86/kernel/tsc.c
> +++ b/arch/x86/kernel/tsc.c
> @@ -283,6 +283,15 @@ bool using_native_sched_clock(void)
> int __init __paravirt_set_sched_clock(u64 (*func)(void), bool stable,
> void (*save)(void), void (*restore)(void))
> {
> + /*
> + * Don't replace TSC with a PV clock when running as a CoCo guest and
> + * the TSC is secure/trusted; PV clocks are emulated by the hypervisor,
> + * which isn't in the guest's TCB.
> + */
> + if (cc_platform_has(CC_ATTR_GUEST_SNP_SECURE_TSC) ||
> + boot_cpu_has(X86_FEATURE_TDX_GUEST))
> + return -EPERM;
> +
> if (!stable)
> clear_sched_clock_stable();
>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 29/41] x86/paravirt: Plumb a return code into __paravirt_set_sched_clock()
From: David Woodhouse @ 2026-05-20 23:44 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-30-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 733 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Add a return code to __paravirt_set_sched_clock() so that the kernel can
> reject attempts to use a PV sched_clock without breaking the caller. E.g.
> when running as a CoCo VM with a secure TSC, using a PV clock is generally
> undesirable.
>
> Note, kvmclock is the only PV clock that does anything "extra" beyond
> simply registering itself as sched_clock, i.e. is the only caller that
> needs to check the new return value.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Oooh... can we use this to reject the kvmclock when we have a stable
and reliable TSC even for non-CoCo guests?
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 28/41] x86/paravirt: Mark __paravirt_set_sched_clock() as __init
From: David Woodhouse @ 2026-05-20 23:42 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-29-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 485 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Annotate __paravirt_set_sched_clock() as __init, and make its wrapper
> __always_inline to ensure sanitizers don't result in a non-inline version
> hanging around. All callers run during __init, and changing sched_clock
> after boot would be all kinds of crazy.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 27/41] x86/kvmclock: Enable kvmclock on APs during onlining if kvmclock isn't sched_clock
From: David Woodhouse @ 2026-05-20 23:27 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-28-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1455 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> In anticipation of making x86_cpuinit.early_percpu_clock_init(), i.e.
> kvm_setup_secondary_clock(), a dedicated sched_clock hook that will be
> invoked if and only if kvmclock is set as sched_clock, ensure APs enable
> their kvmclock during CPU online. While a redundant write to the MSR is
> technically ok, skip the registration when kvmclock is sched_clock so that
> it's somewhat obvious that kvmclock *needs* to be enabled during early
> bringup when it's being used as sched_clock.
>
> Plumb in the BSP's resume path purely for documentation purposes. Both
> KVM (as-a-guest) and timekeeping/clocksource hook syscore_ops, and it's
> not super obvious that using KVM's hooks would be flawed. E.g. it would
> work today, because KVM's hooks happen to run after/before timekeeping's
> hooks during suspend/resume, but that's sheer dumb luck as the order in
> which syscore_ops are invoked depends entirely on when a subsystem is
> initialized and thus registers its hooks.
>
> Opportunsitically make the registration messages more precise to help
> debug issues where kvmclock is enabled too late.
That's a hard word to type, isn't it?
> Opportunstically WARN in kvmclock_{suspend,resume}() to harden against
> future bugs.
So is that :)
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 26/41] x86/kvmclock: WARN if wall clock is read while kvmclock is suspended
From: David Woodhouse @ 2026-05-20 23:19 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-27-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 766 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> WARN if kvmclock is still suspended when its wallclock is read, i.e. when
> the kernel reads its persistent clock. The wallclock subtly depends on
> the BSP's kvmclock being enabled, and returns garbage if kvmclock is
> disabled.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
Although I still hate the whole KVM wallclock thing, as the kvmclock
itself is monotonic_raw, so adding that to the wallclock epoch is kind
of wrong.
Maybe the host should updated the wallclock occasionally to keep it up
to date...
Or maybe the guest should prefer the KVM_HC_CLOCK_PAIRING hypercall if
it exists, over kvm-wallclock.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 25/41] x86/kvmclock: Hook clocksource.suspend/resume when kvmclock isn't sched_clock
From: David Woodhouse @ 2026-05-20 23:06 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-26-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 409 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Save/restore kvmclock across suspend/resume via clocksource hooks when
> kvmclock isn't being used for sched_clock. This will allow using kvmclock
> as a clocksource (or for wallclock!) without also using it for sched_clock.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ 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