* [PATCH v8 8/8] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
Add a 'gpat' field to kvm_svm_nested_state_hdr to carry L2's guest PAT
value across save and restore.
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and the vCPU is in
guest mode with nested NPT enabled, save vmcb02's g_pat into the header on
KVM_GET_NESTED_STATE, and restore it on KVM_SET_NESTED_STATE.
Host-initiated accesses to IA32_PAT (via KVM_GET/SET_MSRS) always target
L1's hPAT, so they cannot be used to save or restore gPAT. The separate
header field ensures that KVM_GET/SET_MSRS and KVM_GET/SET_NESTED_STATE are
independent and can be ordered arbitrarily during save and restore.
Note that struct kvm_svm_nested_state_hdr is included in a union padded to
120 bytes, so there is room to add the gpat field without changing any
offsets.
Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
Documentation/virt/kvm/api.rst | 1 +
arch/x86/include/uapi/asm/kvm.h | 1 +
arch/x86/kvm/svm/nested.c | 19 +++++++++++++++++++
3 files changed, 21 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 0a2d873ca5a3..d6bbb7bad173 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4965,6 +4965,7 @@ Errors:
struct kvm_svm_nested_state_hdr {
__u64 vmcb_pa;
+ __u64 gpat;
};
struct kvm_vmx_nested_state_data {
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 3ada2fa9ca86..1585ec804066 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -533,6 +533,7 @@ struct kvm_svm_nested_state_data {
struct kvm_svm_nested_state_hdr {
__u64 vmcb_pa;
+ __u64 gpat;
};
/* for KVM_CAP_NESTED_STATE */
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index cf6356c775e6..9682099193d4 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1868,6 +1868,9 @@ static int svm_get_nested_state(struct kvm_vcpu *vcpu,
/* First fill in the header and copy it out. */
if (is_guest_mode(vcpu)) {
kvm_state.hdr.svm.vmcb_pa = svm->nested.vmcb12_gpa;
+ kvm_state.hdr.svm.gpat = 0;
+ if (l2_has_separate_pat(vcpu))
+ kvm_state.hdr.svm.gpat = svm->vmcb->save.g_pat;
kvm_state.size += KVM_STATE_NESTED_SVM_VMCB_SIZE;
kvm_state.flags |= KVM_STATE_NESTED_GUEST_MODE;
@@ -1920,6 +1923,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
struct vmcb_save_area *save;
struct vmcb_save_area_cached save_cached;
struct vmcb_ctrl_area_cached ctl_cached;
+ bool use_separate_l2_pat;
unsigned long cr0;
int ret;
@@ -1996,6 +2000,17 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
!nested_vmcb_check_save(vcpu, &save_cached, false))
goto out_free;
+ /*
+ * Validate gPAT when the shared PAT quirk is disabled (i.e. L2
+ * has its own gPAT). This is done separately from the
+ * vmcb_save_area_cached validation above, because gPAT is L2
+ * state, but the vmcb_save_area_cached is populated with L1 state.
+ */
+ use_separate_l2_pat = (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP) &&
+ !kvm_check_has_quirk(vcpu->kvm,
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+ if (use_separate_l2_pat && !kvm_pat_valid(kvm_state->hdr.svm.gpat))
+ goto out_free;
/*
* All checks done, we can enter guest mode. Userspace provides
@@ -2022,6 +2037,10 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
nested_copy_vmcb_control_to_cache(svm, ctl);
svm_switch_vmcb(svm, &svm->nested.vmcb02);
+
+ if (use_separate_l2_pat)
+ vmcb_set_gpat(svm->vmcb, kvm_state->hdr.svm.gpat);
+
nested_vmcb02_prepare_control(svm);
/*
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 7/8] KVM: Documentation: document KVM_{GET,SET}_NESTED_STATE for SVM
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
Document the nested state constants and structures for SVM that were added
by commit cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and
KVM_SET_NESTED_STATE").
Fixes: cc440cdad5b7 ("KVM: nSVM: implement KVM_GET_NESTED_STATE and KVM_SET_NESTED_STATE")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
Documentation/virt/kvm/api.rst | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 2d56f17e3760..0a2d873ca5a3 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -4942,10 +4942,13 @@ Errors:
#define KVM_STATE_NESTED_FORMAT_SVM 1
#define KVM_STATE_NESTED_VMX_VMCS_SIZE 0x1000
+ #define KVM_STATE_NESTED_SVM_VMCB_SIZE 0x1000
#define KVM_STATE_NESTED_VMX_SMM_GUEST_MODE 0x00000001
#define KVM_STATE_NESTED_VMX_SMM_VMXON 0x00000002
+ #define KVM_STATE_NESTED_GIF_SET 0x00000100
+
#define KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE 0x00000001
struct kvm_vmx_nested_state_hdr {
@@ -4960,11 +4963,19 @@ Errors:
__u64 preemption_timer_deadline;
};
+ struct kvm_svm_nested_state_hdr {
+ __u64 vmcb_pa;
+ };
+
struct kvm_vmx_nested_state_data {
__u8 vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
__u8 shadow_vmcs12[KVM_STATE_NESTED_VMX_VMCS_SIZE];
};
+ struct kvm_svm_nested_state_data {
+ __u8 vmcb12[KVM_STATE_NESTED_SVM_VMCB_SIZE];
+ };
+
This ioctl copies the vcpu's nested virtualization state from the kernel to
userspace.
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 6/8] KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
According to the APM volume 3 pseudo-code for "VMRUN," when nested paging
is enabled in the vmcb, the guest PAT register (gPAT) is saved to the vmcb
on emulated VMEXIT.
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and the vCPU is in
guest mode with nested NPT enabled, save the vmcb02 g_pat field to the
vmcb12 g_pat field on emulated VMEXIT.
Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 8c9dd685b616..cf6356c775e6 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1250,6 +1250,9 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
vmcb12->save.dr6 = svm->vcpu.arch.dr6;
vmcb12->save.cpl = vmcb02->save.cpl;
+ if (l2_has_separate_pat(vcpu))
+ vmcb12->save.g_pat = vmcb02->save.g_pat;
+
if (guest_cpu_cap_has(vcpu, X86_FEATURE_SHSTK)) {
vmcb12->save.s_cet = vmcb02->save.s_cet;
vmcb12->save.isst_addr = vmcb02->save.isst_addr;
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH] KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and the vCPU is in
guest mode with nested NPT enabled, guest accesses to IA32_PAT are
redirected to the gPAT register, which is stored in VMCB02's g_pat field.
Non-guest accesses (e.g. from userspace) to IA32_PAT are always redirected
to hPAT, which is stored in vcpu->arch.pat.
Directing host-initiated accesses to hPAT ensures that KVM_GET/SET_MSRS and
KVM_GET/SET_NESTED_STATE are independent of each other and can be ordered
arbitrarily during save and restore. gPAT is saved and restored separately
via KVM_GET/SET_NESTED_STATE.
Use WARN_ON_ONCE to flag any host-initiated accesses originating from KVM
itself rather than userspace.
Use pr_warn_once to flag any use of the common MSR-handling code (now
shared by VMX and TDX) for IA32_PAT by a vCPU that is SVM-capable.
Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
Co-developed-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/nested.c | 9 ---------
arch/x86/kvm/svm/svm.c | 36 +++++++++++++++++++++++++++++++++---
arch/x86/kvm/svm/svm.h | 1 -
3 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 58574e803812..8c9dd685b616 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -703,15 +703,6 @@ static int nested_svm_load_cr3(struct kvm_vcpu *vcpu, unsigned long cr3,
return 0;
}
-void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
-{
- if (!svm->nested.vmcb02.ptr)
- return;
-
- /* FIXME: merge g_pat from vmcb01 and vmcb12. */
- vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
-}
-
static bool nested_vmcb12_has_lbrv(struct kvm_vcpu *vcpu)
{
return guest_cpu_cap_has(vcpu, X86_FEATURE_LBRV) &&
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 56b6bd5dfdca..8d968ead6f45 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2767,6 +2767,20 @@ static bool sev_es_prevent_msr_access(struct kvm_vcpu *vcpu,
!msr_write_intercepted(vcpu, msr_info->index);
}
+static bool svm_pat_accesses_gpat(struct kvm_vcpu *vcpu, bool from_host)
+{
+ /*
+ * When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and nested
+ * NPT is enabled, L2 has a separate PAT from L1. Guest accesses
+ * to IA32_PAT while running L2 target L2's gPAT; host-initiated
+ * accesses always target L1's hPAT so that KVM_GET/SET_MSRS and
+ * KVM_GET/SET_NESTED_STATE are independent of each other and can
+ * be ordered arbitrarily during save and restore.
+ */
+ WARN_ON_ONCE(from_host && vcpu->wants_to_run);
+ return !from_host && is_guest_mode(vcpu) && l2_has_separate_pat(vcpu);
+}
+
static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
{
struct vcpu_svm *svm = to_svm(vcpu);
@@ -2883,6 +2897,12 @@ static int svm_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
case MSR_AMD64_DE_CFG:
msr_info->data = svm->msr_decfg;
break;
+ case MSR_IA32_CR_PAT:
+ if (svm_pat_accesses_gpat(vcpu, msr_info->host_initiated)) {
+ msr_info->data = svm->vmcb->save.g_pat;
+ break;
+ }
+ return kvm_get_msr_common(vcpu, msr_info);
default:
return kvm_get_msr_common(vcpu, msr_info);
}
@@ -2966,13 +2986,23 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
break;
case MSR_IA32_CR_PAT:
+ if (svm_pat_accesses_gpat(vcpu, msr->host_initiated)) {
+ if (!kvm_pat_valid(data))
+ return 1;
+
+ vmcb_set_gpat(svm->vmcb, data);
+ break;
+ }
+
ret = kvm_set_msr_common(vcpu, msr);
if (ret)
break;
- vmcb_set_gpat(svm->vmcb01.ptr, data);
- if (is_guest_mode(vcpu))
- nested_vmcb02_compute_g_pat(svm);
+ if (npt_enabled) {
+ vmcb_set_gpat(svm->vmcb01.ptr, data);
+ if (is_guest_mode(vcpu) && !l2_has_separate_pat(vcpu))
+ vmcb_set_gpat(svm->vmcb, data);
+ }
break;
case MSR_IA32_SPEC_CTRL:
if (!msr->host_initiated &&
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index fdd6286d965e..677e899bda33 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -881,7 +881,6 @@ void nested_copy_vmcb_control_to_cache(struct vcpu_svm *svm,
void nested_copy_vmcb_save_to_cache(struct vcpu_svm *svm,
struct vmcb_save_area *save);
void nested_sync_control_from_vmcb02(struct vcpu_svm *svm);
-void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm);
void svm_switch_vmcb(struct vcpu_svm *svm, struct kvm_vmcb_info *target_vmcb);
extern struct kvm_x86_nested_ops svm_nested_ops;
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 4/8] KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and nested NPT is
enabled in vmcb12, copy the (cached and validated) vmcb12 g_pat field to
vmcb02's g_pat, giving L2 its own independent guest PAT register.
When the quirk is enabled (default), or when NPT is enabled but nested NPT
is disabled, copy L1's IA32_PAT MSR to the vmcb02 g_pat field, since L2
shares the IA32_PAT MSR with L1.
When NPT is disabled, the g_pat field is ignored by hardware.
Fixes: 15038e147247 ("KVM: SVM: obey guest PAT")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 515a8545e8e0..58574e803812 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -727,9 +727,6 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
struct kvm_vcpu *vcpu = &svm->vcpu;
- nested_vmcb02_compute_g_pat(svm);
- vmcb_mark_dirty(vmcb02, VMCB_NPT);
-
/* Load the nested guest state */
if (svm->nested.vmcb12_gpa != svm->nested.last_vmcb12_gpa) {
new_vmcb12 = true;
@@ -760,6 +757,13 @@ static void nested_vmcb02_prepare_save(struct vcpu_svm *svm)
vmcb_mark_dirty(vmcb02, VMCB_CET);
}
+ if (l2_has_separate_pat(vcpu)) {
+ if (unlikely(new_vmcb12 || vmcb12_is_dirty(control, VMCB_NPT)))
+ vmcb_set_gpat(vmcb02, svm->nested.save.g_pat);
+ } else if (npt_enabled) {
+ vmcb_set_gpat(vmcb02, vcpu->arch.pat);
+ }
+
kvm_set_rflags(vcpu, save->rflags | X86_EFLAGS_FIXED);
svm_set_efer(vcpu, svm->nested.save.efer);
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 3/8] KVM: x86: nSVM: Cache and validate vmcb12 g_pat
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
When KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled and nested paging is
enabled in vmcb12, validate g_pat at emulated VMRUN and cause an immediate
VMEXIT with exit code VMEXIT_INVALID if it is invalid, as specified in the
APM, volume 2: "Nested Paging and VMRUN/VMEXIT."
Fixes: 3d6368ef580a ("KVM: SVM: Add VMRUN handler")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 23 +++++++++++++++++++----
arch/x86/kvm/svm/svm.h | 1 +
2 files changed, 20 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 3575c9386e94..515a8545e8e0 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -410,7 +410,8 @@ static bool nested_vmcb_check_controls(struct kvm_vcpu *vcpu,
/* Common checks that apply to both L1 and L2 state. */
static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
- struct vmcb_save_area_cached *save)
+ struct vmcb_save_area_cached *save,
+ bool check_gpat)
{
if (CC(!(save->efer & EFER_SVME)))
return false;
@@ -445,6 +446,15 @@ static bool nested_vmcb_check_save(struct kvm_vcpu *vcpu,
if (CC(!kvm_valid_efer(vcpu, save->efer)))
return false;
+ /*
+ * If userspace contrives to get an invalid g_pat into vmcb02 by
+ * disabling KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT in a race with
+ * this check, it should be prepared for the KVM_EXIT_FAIL_ENTRY
+ * that will follow.
+ */
+ if (check_gpat && CC(!kvm_pat_valid(save->g_pat)))
+ return false;
+
return true;
}
@@ -452,7 +462,8 @@ int nested_svm_check_cached_vmcb12(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
- if (!nested_vmcb_check_save(vcpu, &svm->nested.save) ||
+ if (!nested_vmcb_check_save(vcpu, &svm->nested.save,
+ l2_has_separate_pat(vcpu)) ||
!nested_vmcb_check_controls(vcpu, &svm->nested.ctl))
return -EINVAL;
@@ -562,6 +573,7 @@ static void __nested_copy_vmcb_save_to_cache(struct vmcb_save_area_cached *to,
to->rax = from->rax;
to->cr2 = from->cr2;
+ to->g_pat = from->g_pat;
svm_copy_lbrs(to, from);
}
@@ -1974,13 +1986,16 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
/*
* Validate host state saved from before VMRUN (see
- * nested_svm_check_permissions).
+ * nested_svm_check_permissions). Note that the g_pat field is not
+ * validated, because (a) it may have been clobbered by SMM before
+ * KVM_GET_NESTED_STATE, and (b) it is not loaded at emulated
+ * #VMEXIT.
*/
__nested_copy_vmcb_save_to_cache(&save_cached, save);
if (!(save->cr0 & X86_CR0_PG) ||
!(save->cr0 & X86_CR0_PE) ||
(save->rflags & X86_EFLAGS_VM) ||
- !nested_vmcb_check_save(vcpu, &save_cached))
+ !nested_vmcb_check_save(vcpu, &save_cached, false))
goto out_free;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index dfb0a73be606..fdd6286d965e 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -165,6 +165,7 @@ struct vmcb_save_area_cached {
u64 isst_addr;
u64 rax;
u64 cr2;
+ u64 g_pat;
u64 dbgctl;
u64 br_from;
u64 br_to;
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 2/8] KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest mode
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
When running an L2 guest and writing to MSR_IA32_CR_PAT, the host PAT value
is stored in both vmcb01's g_pat field and vmcb02's g_pat field, but the
clean bit was only being cleared for vmcb02.
Introduce the helper vmcb_set_gpat() which sets vmcb->save.g_pat and marks
the VMCB dirty for VMCB_NPT. Use this helper in both svm_set_msr() for
updating vmcb01 and in nested_vmcb02_compute_g_pat() for updating vmcb02,
ensuring both VMCBs' NPT fields are properly marked dirty.
Fixes: 4995a3685f1b ("KVM: SVM: Use a separate vmcb for the nested L2 guest")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/nested.c | 2 +-
arch/x86/kvm/svm/svm.c | 3 +--
arch/x86/kvm/svm/svm.h | 6 ++++++
3 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 961804df5f45..3575c9386e94 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -697,7 +697,7 @@ void nested_vmcb02_compute_g_pat(struct vcpu_svm *svm)
return;
/* FIXME: merge g_pat from vmcb01 and vmcb12. */
- svm->nested.vmcb02.ptr->save.g_pat = svm->vmcb01.ptr->save.g_pat;
+ vmcb_set_gpat(svm->nested.vmcb02.ptr, svm->vmcb01.ptr->save.g_pat);
}
static bool nested_vmcb12_has_lbrv(struct kvm_vcpu *vcpu)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index e7fdd7a9c280..56b6bd5dfdca 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2970,10 +2970,9 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
if (ret)
break;
- svm->vmcb01.ptr->save.g_pat = data;
+ vmcb_set_gpat(svm->vmcb01.ptr, data);
if (is_guest_mode(vcpu))
nested_vmcb02_compute_g_pat(svm);
- vmcb_mark_dirty(svm->vmcb, VMCB_NPT);
break;
case MSR_IA32_SPEC_CTRL:
if (!msr->host_initiated &&
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index a91942269f6a..dfb0a73be606 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -453,6 +453,12 @@ static inline bool vmcb12_is_dirty(struct vmcb_ctrl_area_cached *control, int bi
return !test_bit(bit, (unsigned long *)&control->clean);
}
+static inline void vmcb_set_gpat(struct vmcb *vmcb, u64 data)
+{
+ vmcb->save.g_pat = data;
+ vmcb_mark_dirty(vmcb, VMCB_NPT);
+}
+
static __always_inline struct vcpu_svm *to_svm(struct kvm_vcpu *vcpu)
{
return container_of(vcpu, struct vcpu_svm, vcpu);
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 1/8] KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
In-Reply-To: <20260407190343.325299-1-jmattson@google.com>
Define a quirk to control whether nested SVM shares L1's PAT with L2
(legacy behavior) or gives L2 its own independent gPAT (correct behavior
per the APM).
When the quirk is enabled (default), L2 shares L1's PAT, preserving the
legacy KVM behavior. When userspace disables the quirk, KVM correctly
virtualizes the PAT for nested SVM guests, giving L2 a separate gPAT as
specified in the AMD architecture.
Signed-off-by: Jim Mattson <jmattson@google.com>
---
Documentation/virt/kvm/api.rst | 14 ++++++++++++++
arch/x86/include/asm/kvm_host.h | 3 ++-
arch/x86/include/uapi/asm/kvm.h | 1 +
arch/x86/kvm/svm/svm.h | 10 ++++++++++
4 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 032516783e96..2d56f17e3760 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8551,6 +8551,20 @@ KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM By default, KVM relaxes the consisten
bit to be cleared. Note that the vmcs02
bit is still completely controlled by the
host, regardless of the quirk setting.
+
+KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT By default, KVM for nested SVM guests
+ shares the IA32_PAT MSR between L1 and
+ L2. This is legacy behavior and does
+ not match the AMD architecture
+ specification. When this quirk is
+ disabled and nested paging (NPT) is
+ enabled for L2, KVM correctly
+ virtualizes a separate guest PAT
+ register for L2, using the g_pat
+ field in the VMCB. When NPT is
+ disabled for L2, L1 and L2 continue
+ to share the IA32_PAT MSR regardless
+ of the quirk setting.
======================================== ================================================
7.32 KVM_CAP_MAX_VCPU_ID
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index c470e40a00aa..f77d64bbd409 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2526,7 +2526,8 @@ int memslot_rmap_alloc(struct kvm_memory_slot *slot, unsigned long npages);
KVM_X86_QUIRK_SLOT_ZAP_ALL | \
KVM_X86_QUIRK_STUFF_FEATURE_MSRS | \
KVM_X86_QUIRK_IGNORE_GUEST_PAT | \
- KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM)
+ KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM | \
+ KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT)
#define KVM_X86_CONDITIONAL_QUIRKS \
(KVM_X86_QUIRK_CD_NW_CLEARED | \
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 5f2b30d0405c..3ada2fa9ca86 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -477,6 +477,7 @@ struct kvm_sync_regs {
#define KVM_X86_QUIRK_STUFF_FEATURE_MSRS (1 << 8)
#define KVM_X86_QUIRK_IGNORE_GUEST_PAT (1 << 9)
#define KVM_X86_QUIRK_VMCS12_ALLOW_FREEZE_IN_SMM (1 << 10)
+#define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT (1 << 11)
#define KVM_STATE_NESTED_FORMAT_VMX 0
#define KVM_STATE_NESTED_FORMAT_SVM 1
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index fd0652b32c81..a91942269f6a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -630,6 +630,16 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP;
}
+static inline bool l2_has_separate_pat(struct kvm_vcpu *vcpu)
+{
+ /*
+ * If KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled while a vCPU
+ * is running, the L2 IA32_PAT semantics for that vCPU are undefined.
+ */
+ return nested_npt_enabled(to_svm(vcpu)) &&
+ !kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
+}
+
static inline bool nested_vnmi_enabled(struct vcpu_svm *svm)
{
return guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_VNMI) &&
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply related
* [PATCH v8 0/8] KVM: x86: nSVM: Improve PAT virtualization
From: Jim Mattson @ 2026-04-07 19:03 UTC (permalink / raw)
To: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Sean Christopherson,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, kvm, linux-doc, linux-kernel, Yosry Ahmed
Cc: Jim Mattson
Currently, KVM's implementation of nested SVM treats the PAT MSR the same
way whether or not nested NPT is enabled: L1 and L2 share a single
PAT. However, the AMD APM specifies that when nested NPT is enabled, the host
(L1) and the guest (L2) should have independent PATs: hPAT for L1 and gPAT
for L2.
This patch series implements independent PATs for L1 and L2 when nested NPT
is enabled, but only when a new quirk, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT,
is disabled. By default, the quirk is enabled, preserving KVM's legacy
behavior. When the quirk is disabled, KVM correctly virtualizes a separate
PAT register for L2, using the g_pat field in the VMCB.
Guest accesses to the IA32_PAT MSR are redirected to either hPAT or gPAT
depending on the current mode and whether nested NPT is enabled. All other
accesses, including userspace accesses via KVM_{GET,SET}_MSRS, continue to
reference hPAT. L2's gPAT is saved and restored via a new 'gpat' field in
kvm_svm_nested_state_hdr, which is within the existing padding of the header
to maintain ABI compatibility.
v1: https://lore.kernel.org/kvm/20260113003016.3511895-1-jmattson@google.com/
v2: https://lore.kernel.org/kvm/20260115232154.3021475-1-jmattson@google.com/
v3: https://lore.kernel.org/kvm/20260205214326.1029278-1-jmattson@google.com/
v4: https://lore.kernel.org/kvm/20260212155905.3448571-1-jmattson@google.com/
v5: https://lore.kernel.org/kvm/20260224005500.1471972-1-jmattson@google.com/
v6: https://lore.kernel.org/kvm/20260326174944.3820245-1-jmattson@google.com/
v7: https://lore.kernel.org/kvm/20260327234023.2659476-1-jmattson@google.com/
v7 -> v8:
* Indentation changes to conform to Sean's aesthetic [Sean]
* Updated comment in svm_pat_accesses_gpat() [Sean]
* Restored the common behavior for get/set IA32_PAT [Sean]
* Reordered declarations in svm_set_nested_state() for ASCII art [Sean]
* Dropped the selftest [Sean]
Jim Mattson (8):
KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT
KVM: x86: nSVM: Clear VMCB_NPT clean bit when updating hPAT from guest
mode
KVM: x86: nSVM: Cache and validate vmcb12 g_pat
KVM: x86: nSVM: Set vmcb02.g_pat correctly for nested NPT
KVM: x86: nSVM: Redirect IA32_PAT accesses to either hPAT or gPAT
KVM: x86: nSVM: Save gPAT to vmcb12.g_pat on VMEXIT
KVM: Documentation: document KVM_{GET,SET}_NESTED_STATE for SVM
KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
Documentation/virt/kvm/api.rst | 26 ++++++++++++++
arch/x86/include/asm/kvm_host.h | 3 +-
arch/x86/include/uapi/asm/kvm.h | 2 ++
arch/x86/kvm/svm/nested.c | 64 ++++++++++++++++++++++++---------
arch/x86/kvm/svm/svm.c | 41 +++++++++++++++++----
arch/x86/kvm/svm/svm.h | 18 +++++++++-
6 files changed, 130 insertions(+), 24 deletions(-)
--
2.53.0.1213.gd9a14994de-goog
^ permalink raw reply
* Re: [PATCH] docs: proc: document ProtectionKey in smaps
From: Randy Dunlap @ 2026-04-07 18:58 UTC (permalink / raw)
To: Kevin Brodsky, Dave Hansen, linux-doc
Cc: linux-kernel, Yury Khrustalev, Jonathan Corbet, Shuah Khan,
Dave Hansen, Andrew Morton, Lorenzo Stoakes, Vlastimil Babka,
David Hildenbrand, Mark Rutland, linux-fsdevel, linux-mm
In-Reply-To: <2d2aac86-2780-4a29-9eef-116c26485812@arm.com>
On 4/7/26 8:12 AM, Kevin Brodsky wrote:
> On 07/04/2026 16:42, Dave Hansen wrote:
>> On 4/7/26 05:51, Kevin Brodsky wrote:
>>> +If both the kernel and the system support protection keys (pkeys),
>>> +"ProtectionKey" indicates the memory protection key associated with the
>>> +virtual memory area.
>> I think you're trying to get across the point here that the kernel needs
>> to know about protection keys, have it enabled, and be running on a CPU
>> with pkey support.
>
> Indeed.
>
>> To me "system" is a bit ambiguous here but _can_ refer to the whole
>> hardware/software system as a whole. To avoid redundancy, I'd say either:
>>
>> If both the kernel and the processor support protection keys...
>>
>> or
>>
>> If the system supports protection keys...
>
> I see your point. By "system" I essentially mean the hardware (the SoC).
> In general I would tend to avoid "processor" because not all CPUs in a
> system necessarily have the same features, and some features require
> hardware support beyond the CPU itself. Terminology is hard...
>
> Happy to replace "system" with "hardware" if that's clearer :)
I think that "system" is too nebulous there, so I would prefer to see
"hardware" instead.
thanks.
--
~Randy
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Jim Mattson @ 2026-04-07 18:40 UTC (permalink / raw)
To: Pawan Gupta
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <20260407171151.2gf2idjbmph35ypb@desk>
On Tue, Apr 7, 2026 at 10:12 AM Pawan Gupta
<pawan.kumar.gupta@linux.intel.com> wrote:
>
> On Tue, Apr 07, 2026 at 09:46:07AM -0700, Jim Mattson wrote:
> > On Tue, Apr 7, 2026 at 9:40 AM Pawan Gupta
> > <pawan.kumar.gupta@linux.intel.com> wrote:
> > >
> > > On Mon, Apr 06, 2026 at 07:23:25AM -0700, Jim Mattson wrote:
> > > > Yes, but the guest needs a way to determine whether the hypervisor
> > > > will do what's necessary to make the short sequence effective. And, in
> > > > particular, no KVM hypervisor today is prepared to do that.
> > > >
> > > > When running under a hypervisor, without BHI_CTRL and without any
> > > > evidence to the contrary, the guest must assume that the longer
> > > > sequence is necessary. At the very least, we need a CPUID or MSR bit
> > > > that says, "the short BHB clearing sequence is adequate for this
> > > > vCPU."
> > >
> > > After discussing this internally, the consensus is that the best path
> > > forward is to add virtual SPEC_CTRL support to KVM, which also aligns with
> > > Intel's guidance. In the long term, virtual SPEC_CTRL can benefit future
> > > mitigations as well. As with many other mitigations (e.g. microcode), the
> > > guest would rely on the host to enforce the appropriate protections.
> >
> > I don't think it's reasonable for the guest to rely on a future
> > implementation to enforce the appropriate protections.
> >
> > This is already a problem today. If a guest sees that BHI_CTRL is
> > unavailable, it will deploy the short BHB clearing sequence and
> > declare that the vulnerability is mitigated. That isn't true if the
> > guest is running on Alder Lake or newer.
>
> In any case, there is a change required in the kernel either for the guest
> or the host, they both are future implementations. Why not implement the
> one that is more future proof.
There will always be old hypervisors. True future-proofing requires
that the guest be able to distinguish an old hypervisor from a new
one.
My proposal is as follows:
1. The (advanced) hypervisor can advertise to the guest (via CPUID bit
or MSR bit) that the short BHB clearing sequence is adequate. This may
mean either that the VM will only be hosted on pre-Alder Lake hardware
or that the hypervisor will set BHI_DIS_S behind the back of the
guest. Presumably, this bit would not be reported if BHI_CTRL is
advertised to the guest.
2. If the guest sees this bit, then it can use the short sequence. If
it doesn't see this bit, it must use the long sequence.
^ permalink raw reply
* Re: [PATCH] sched_ext: Documentation: Fix scx_bpf_move_to_local kfunc name
From: Tejun Heo @ 2026-04-07 18:18 UTC (permalink / raw)
To: fangqiurong; +Cc: linux-kernel, linux-doc, corbet
In-Reply-To: <20260407093405.2573184-1-fangqiurong@kylinos.cn>
Hello,
On Tue, Apr 7, 2026 at 05:34:05PM +0800, fangqiurong@kylinos.cn wrote:
> The correct kfunc name is scx_bpf_dsq_move_to_local(), not
> scx_bpf_move_to_local(). Fix the two references in the
> Scheduling Cycle section.
Applied to sched_ext/for-7.1. The patch had the author From: line
set to fangqiurong@kylinos.com while the envelope and Signed-off-by
were @kylinos.cn -- I aligned the recorded author with the
Signed-off-by (.cn). Please let me know if that's wrong.
Thanks.
--
tejun
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Pawan Gupta @ 2026-04-07 17:52 UTC (permalink / raw)
To: Jon Kohler
Cc: Jim Mattson, x86@kernel.org, Nikolay Borisov, H. Peter Anvin,
Josh Poimboeuf, David Kaplan, Sean Christopherson,
Borislav Petkov, Dave Hansen, Peter Zijlstra, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, KP Singh, Jiri Olsa,
David S. Miller, David Laight, Andy Lutomirski, Thomas Gleixner,
Ingo Molnar, David Ahern, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, John Fastabend, Stanislav Fomichev,
Hao Luo, Paolo Bonzini, Jonathan Corbet,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Asit Mallick,
Tao Zhang, bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, chao.gao@intel.com
In-Reply-To: <FAA31092-E1CA-4D79-8CEC-3DB0F6F1C792@nutanix.com>
On Tue, Apr 07, 2026 at 05:12:06PM +0000, Jon Kohler wrote:
>
>
> > On Apr 7, 2026, at 11:46 AM, Jim Mattson <jmattson@google.com> wrote:
> >
> > On Tue, Apr 7, 2026 at 9:40 AM Pawan Gupta
> > <pawan.kumar.gupta@linux.intel.com> wrote:
> >>
> >> On Mon, Apr 06, 2026 at 07:23:25AM -0700, Jim Mattson wrote:
> >>> Yes, but the guest needs a way to determine whether the hypervisor
> >>> will do what's necessary to make the short sequence effective. And, in
> >>> particular, no KVM hypervisor today is prepared to do that.
> >>>
> >>> When running under a hypervisor, without BHI_CTRL and without any
> >>> evidence to the contrary, the guest must assume that the longer
> >>> sequence is necessary. At the very least, we need a CPUID or MSR bit
> >>> that says, "the short BHB clearing sequence is adequate for this
> >>> vCPU."
> >>
> >> After discussing this internally, the consensus is that the best path
> >> forward is to add virtual SPEC_CTRL support to KVM, which also aligns with
> >> Intel's guidance. In the long term, virtual SPEC_CTRL can benefit future
> >> mitigations as well. As with many other mitigations (e.g. microcode), the
> >> guest would rely on the host to enforce the appropriate protections.
>
> Would we have to wait for virtual SPEC_CTRL to get this optimization?
The optimization works with or without virtual-SPEC_CTRL.
> Or would that be a future enhancement to make this more prescriptive?
Virtual-SPEC_CTRL enables safer guest migrations between pre and post Alder
Lake CPUs w.r.t. Native BHI mitigation. It is not related to VMSCAPE.
^ permalink raw reply
* Re: [PATCH v2 00/16] fs,x86/resctrl: Add kernel-mode (e.g., PLZA) support to the resctrl subsystem
From: Reinette Chatre @ 2026-04-07 17:48 UTC (permalink / raw)
To: Babu Moger, corbet, tony.luck, Dave.Martin, james.morse, tglx,
mingo, bp, dave.hansen
Cc: skhan, x86, hpa, peterz, juri.lelli, vincent.guittot,
dietmar.eggemann, rostedt, bsegall, mgorman, vschneid, kas,
rick.p.edgecombe, akpm, pmladek, rdunlap, dapeng1.mi, kees, elver,
paulmck, lirongqing, safinaskar, fvdl, seanjc, pawan.kumar.gupta,
xin, tiala, Neeraj.Upadhyay, chang.seok.bae, thomas.lendacky,
elena.reshetova, linux-doc, linux-kernel, linux-coco, kvm,
eranian, peternewman
In-Reply-To: <5a740f47-d3f3-45af-9d8c-ebcf3dd89c0d@amd.com>
Hi Babu,
On 4/6/26 3:45 PM, Babu Moger wrote:
> Hi Reinette,
>
> Sorry for the late response. I was trying to get confirmation about the use case.
No problem. I appreciate that you did this so that we can make sure resctrl supports
needed use cases.
>
> On 3/31/26 17:24, Reinette Chatre wrote:
>> On 3/30/26 11:46 AM, Babu Moger wrote:
>>> On 3/27/26 17:11, Reinette Chatre wrote:
>>>> On 3/26/26 10:12 AM, Babu Moger wrote:
>>>>> On 3/24/26 17:51, Reinette Chatre wrote:
>>>>>> On 3/12/26 1:36 PM, Babu Moger wrote:
>> can have domains that span different CPUs. There thus seem to be a built in assumption of what a "domain"
>> means for PQR_PLZA_ASSOC so it sounds to me as though, instead of saying that "PQR_PLZA_ASSOC needs
>> to be the same in QoS domain" it may be more accurate to, for example, say that "PQR_PLZA_ASSOC has L3 scope"?
>
> Yes.
Above is about L3 scope ...
>>
>> This seems to be what this implementation does since it hardcodes PQR_PLZA_ASSOC scope to the L3
>> resource but that creates dependency to the L3 resource that would make PLZA unusable if, for example,
>> the user boots with "rdt=!l3cat" while wanting to use PLZA to manage MBA allocations when in kernel?
>
> Yes. that is correct. It should not be attached to one resource. We need to change it to global scope.
Can I interpret "global scope" as "all online CPUs"? Doing so will simplify
supporting this feature. It does not sound practical for a user wanting to assign
different resource groups to kernel work done in different domains ... the guidance should
instead be to just set the allocations of one resource group to what is needed in the different
domains? There may be more flexibility when supporting per-domain RMIDs though but so far
it sounds as though the focus is global. We can consider what needs to be done to support
some type of "per-domain" assignment as exercise whether current interface could support it
in the future.
...
>>> There are multiple ways this feature can be applied. For simplicity, the discussion below focuses only on CLOSID.
>>>
>>>
>>> 1. Global PLZA enablement
>>>
>>> PLZA can be configured as a global feature by setting |PQR_PLZA_ASSOC.closid = CLOSID| and |PQR_PLZA_ASSOC.plza_en = 1| on all threads in the system. A dedicated CLOSID is reserved for this purpose,
>>
>> Also discussed during v1 is that there is no need to dedicate a CLOSID for this purpose.
>> There could be an "unthrottled" CLOSID to which all high priority user space tasks as
>> well as all kernel work of all tasks are assigned.
>> If user space chooses to dedicate a CLOSID for kernel work then that should supported and
>> interface can allow that, but there is no need for resctrl to enforce this.
(above is comment about dedicated group - please see below)
> Yes. I agree. The changes in context switch code is a concern.
>
> You covered some of the cases I was thinking(xx_set_individual).
>
> How about this idea?
>
> I suggest splitting the PLZA into two distinct aspects:
>
> 1. How PLZA is applied within a resource group
>
> 2. How PLZA is monitored
I think I see where you are going here. While the "How PLZA is monitored" naming
refers to "monitoring" I *think* what you are separating here is (a) how PLZA is configured
(CLOSID and RMID settings) and (b) how that PLZA configuration is assigned to tasks/CPUs,
not just within a resource group but across the system. Please see below.
> Introduce a new file, "info/kmode_type", to describe how kmode applies in the system.
ack. "in the system" as you have above, not "within a resource group" as mentioned
before that.
>
> # cat info/kmode_type
> [global] <- Kernel mode applies to the entire system (all CPUs/tasks)
> cpus <- Kernel mode applies only to the CPUs in the group
> tasks <- Kernel mode applies only to the tasks in the group
>
> The "global" option is the default right now and it is current common use-case.
>
> The "info/kmode_type -> cpus" option introduces new files
> "kmode_cpus" and "kmode_cpus_list" for users to apply kmode to
> specific set of CPUs. This lets users change the CPU set for PLZA.
Where were you thinking about placing these files in the hierarchy?
> The PLZA MSR is updated when user changes the association to the
> file. No context switch code changes are needed. This will be
> dedicated group. The current resctrl group files, "cpus, cpus_list
Why does this have to be a dedicated group? One of the conclusions from v1
discussion was that the "PLZA group" need *not* be a dedicated group. I repeated that
in my earlier response that I left quoted above. You did not respond to these
conclusions and statements in this regard while you keep coming back to this
needing to be a dedicated group without providing a motivation to do so.
Could you please elaborate why a dedicated group is required?
> and tasks" will not be accessible in this mode. This option give
These files can continue to be accessible.
> some flexibility for the user without the context switch overhead.
Dedicating a resource group to PLZA removes flexibility though, no?
>
> The "info/kmode_type -> tasks" option introduces a new file,
> "kmode_tasks", for users to apply kmode to specific set of tasks.
> This requires context switch changes. This will be dedicated group.
> The current resctrl group files, "cpus, cpus_list and tasks" will
> not be accessible in this mode. We currently have no use case for
> this, so it will not be supported now.
Thank you for confirming. This is a relief.
>
>
> Add a file, "info/kmode_monitor", to describe how kmode is monitored.
>
> # cat info/kmode_monitor
> [inherit_ctrl_and_mon] <- Kernel uses the same CLOSID/RMID as user. Default option for the "global"
> assign_ctrl_inherit_mon <- One CLOSID for all kernel work; RMID inherited from user.
> assign_ctrl_assign_mon <- One resource group (CLOSID+RMID) for all kernel work. Default option for "cpu" type.
My first thought is that the naming is confusing. resctrl has a very strong relationship between
"RMID" and "monitoring" so naming a file "monitor" that deals with allocation/ctrl/CLOSID is
potentially confusion.
Apart from that, while I think I understand where you are going by separating the mode into
two files I am concerned about future complications needing to accommodate all different
combinations of the (now) essentially two modes. My preference is thus to keep this simple by
keeping the mode within one file.
Even so, when stepping back, it does not really look like we need to separate the "global"
and "per CPU" modes. We could just have a single "per CPU" mode and the "global" is just
its default of "all CPUs", no?
Consider, for example, the implementation just consisting of:
# cat info/kernel_mode
[inherit_ctrl_and_mon]
global_assign_ctrl_inherit_mon_per_cpu
global_assign_ctrl_assign_mon_per_cpu
>
> Rename “kernel_mode_assignment” to “kmode_group” to assign the specific group to kmode. This file usage is same as before.
>
> #cat info/kmode_groups (Renamed "kernel_mode_assignment")
> //
Please consider the intent of this file when thinking about names. The idea is that "info/kernel_mode"
specifies the "mode" of how kernel work is handled and it determines the configuration files used in that
mode as well as the syntax when interacting with those files. By renaming "kernel_mode_assignment" to
"kmode_groups" it implicitly requires all future kernel mode enhancements to need some data related to "groups".
In summary, I think this can be simplified by introducing just two new files in info/ that enables the
user to (a) select and (b) configure the "kernel mode". To start there can be just two modes,
global_assign_ctrl_inherit_mon_per_cpu and global_assign_ctrl_assign_mon_per_cpu.
global_assign_ctrl_inherit_mon_per_cpu mode requires a control group in kernel_mode_assignment while
global_assign_ctrl_assign_mon_per_cpu requires a control and monitoring group.
The resource group in info/kernel_mode_assignment gets two additional files "kernel_mode_cpus" and
"kernel_mode_cpus_list" that contains the CPUs enabled with the kernel mode configuration, by default
it will be all online CPUs. The resource group can continue to be used to manage allocations of and
monitor user space tasks. Specifically, the "cpus", "cpus_list", and "tasks" files remain.
A user wanting just "global" settings will get just that when writing the group to
info/kernel_mode_assignment. A user wanting "per CPU" settings can follow the
info/kernel_mode_assignment setting with changes to that resource group's kernel_mode_cpus/kernel_mode_cpus_list
files. Any task running on a CPU that is *not* in kernel_mode_cpus/kernel_mode_cpus_list can be
expected to inherit both CLOSID and RMID from user space for all kernel work.
Reinette
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Jon Kohler @ 2026-04-07 17:12 UTC (permalink / raw)
To: Jim Mattson
Cc: Pawan Gupta, x86@kernel.org, Nikolay Borisov, H. Peter Anvin,
Josh Poimboeuf, David Kaplan, Sean Christopherson,
Borislav Petkov, Dave Hansen, Peter Zijlstra, Alexei Starovoitov,
Daniel Borkmann, Andrii Nakryiko, KP Singh, Jiri Olsa,
David S. Miller, David Laight, Andy Lutomirski, Thomas Gleixner,
Ingo Molnar, David Ahern, Martin KaFai Lau, Eduard Zingerman,
Song Liu, Yonghong Song, John Fastabend, Stanislav Fomichev,
Hao Luo, Paolo Bonzini, Jonathan Corbet,
linux-kernel@vger.kernel.org, kvm@vger.kernel.org, Asit Mallick,
Tao Zhang, bpf@vger.kernel.org, netdev@vger.kernel.org,
linux-doc@vger.kernel.org, chao.gao@intel.com
In-Reply-To: <CALMp9eTA3cXxuOT4dq=6y1hx52gPH1ywwTEmPQ5-fA-vz6r3VQ@mail.gmail.com>
> On Apr 7, 2026, at 11:46 AM, Jim Mattson <jmattson@google.com> wrote:
>
> On Tue, Apr 7, 2026 at 9:40 AM Pawan Gupta
> <pawan.kumar.gupta@linux.intel.com> wrote:
>>
>> On Mon, Apr 06, 2026 at 07:23:25AM -0700, Jim Mattson wrote:
>>> Yes, but the guest needs a way to determine whether the hypervisor
>>> will do what's necessary to make the short sequence effective. And, in
>>> particular, no KVM hypervisor today is prepared to do that.
>>>
>>> When running under a hypervisor, without BHI_CTRL and without any
>>> evidence to the contrary, the guest must assume that the longer
>>> sequence is necessary. At the very least, we need a CPUID or MSR bit
>>> that says, "the short BHB clearing sequence is adequate for this
>>> vCPU."
>>
>> After discussing this internally, the consensus is that the best path
>> forward is to add virtual SPEC_CTRL support to KVM, which also aligns with
>> Intel's guidance. In the long term, virtual SPEC_CTRL can benefit future
>> mitigations as well. As with many other mitigations (e.g. microcode), the
>> guest would rely on the host to enforce the appropriate protections.
Would we have to wait for virtual SPEC_CTRL to get this optimization?
Or would that be a future enhancement to make this more prescriptive?
>
> I don't think it's reasonable for the guest to rely on a future
> implementation to enforce the appropriate protections.
>
> This is already a problem today. If a guest sees that BHI_CTRL is
> unavailable, it will deploy the short BHB clearing sequence and
> declare that the vulnerability is mitigated. That isn't true if the
> guest is running on Alder Lake or newer.
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Pawan Gupta @ 2026-04-07 17:11 UTC (permalink / raw)
To: Jim Mattson
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <CALMp9eTA3cXxuOT4dq=6y1hx52gPH1ywwTEmPQ5-fA-vz6r3VQ@mail.gmail.com>
On Tue, Apr 07, 2026 at 09:46:07AM -0700, Jim Mattson wrote:
> On Tue, Apr 7, 2026 at 9:40 AM Pawan Gupta
> <pawan.kumar.gupta@linux.intel.com> wrote:
> >
> > On Mon, Apr 06, 2026 at 07:23:25AM -0700, Jim Mattson wrote:
> > > Yes, but the guest needs a way to determine whether the hypervisor
> > > will do what's necessary to make the short sequence effective. And, in
> > > particular, no KVM hypervisor today is prepared to do that.
> > >
> > > When running under a hypervisor, without BHI_CTRL and without any
> > > evidence to the contrary, the guest must assume that the longer
> > > sequence is necessary. At the very least, we need a CPUID or MSR bit
> > > that says, "the short BHB clearing sequence is adequate for this
> > > vCPU."
> >
> > After discussing this internally, the consensus is that the best path
> > forward is to add virtual SPEC_CTRL support to KVM, which also aligns with
> > Intel's guidance. In the long term, virtual SPEC_CTRL can benefit future
> > mitigations as well. As with many other mitigations (e.g. microcode), the
> > guest would rely on the host to enforce the appropriate protections.
>
> I don't think it's reasonable for the guest to rely on a future
> implementation to enforce the appropriate protections.
>
> This is already a problem today. If a guest sees that BHI_CTRL is
> unavailable, it will deploy the short BHB clearing sequence and
> declare that the vulnerability is mitigated. That isn't true if the
> guest is running on Alder Lake or newer.
In any case, there is a change required in the kernel either for the guest
or the host, they both are future implementations. Why not implement the
one that is more future proof.
^ permalink raw reply
* Re: [PATCH v7 1/9] KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT
From: Sean Christopherson @ 2026-04-07 17:00 UTC (permalink / raw)
To: Jim Mattson
Cc: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-doc, linux-kernel, linux-kselftest, Yosry Ahmed
In-Reply-To: <CALMp9eQR_ZivpcARLyvDK3w+frpwU8bj2Z+ZvA_fLdCtTq3Vhg@mail.gmail.com>
On Tue, Apr 07, 2026, Jim Mattson wrote:
> On Mon, Apr 6, 2026 at 4:27 PM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Fri, Mar 27, 2026, Jim Mattson wrote:
> > > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > > index ff1e4b4dc998..74014110b550 100644
> > > --- a/arch/x86/kvm/svm/svm.h
> > > +++ b/arch/x86/kvm/svm/svm.h
> > > @@ -616,6 +616,17 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
> > > return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP;
> > > }
> > >
> > > +static inline bool l2_has_separate_pat(struct vcpu_svm *svm)
> >
> > Take @vcpu instead of @svm. All of the callers have a "vcpu", but not all have
> > a local "svm". That will shorten the quirk check far enough to let it poke out.
>
> What is the actual line length limit?
There's a "medium-firm" limit at 80 and a "mostly-hard" limit at 100. 100 isn't
a true hard limit to allow for things like pre-formatted strings, and cases where
the only way to stay under 100 chars would (arguably) yield less readable code
overall, e.g. msr-index.h deliberately has this
#define MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI (1ULL << MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT)
and not
#define MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI \
(1ULL << MSR_CORE_PERF_GLOBAL_OVF_CTRL_TRACE_TOPA_PMI_BIT)
> > > +{
> > > + /*
> > > + * If KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled while a vCPU
> > > + * is running, the L2 IA32_PAT semantics for that vCPU are undefined.
> > > + */
> > > + return nested_npt_enabled(svm) &&
> > > + !kvm_check_has_quirk(svm->vcpu.kvm,
> > > + KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
> >
> > Align indentation. With the @svm => @vcpu change, this becomes:
> >
> > return nested_npt_enabled(to_svm(vcpu)) &&
> > !kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
>
> You wouldn't happen to know the Emacs configuration for the alignment
> you like, would you? I asked Gemini, but it lied to me.
Heh, no. Any time I unintentionally end up in Emacs, I have to do a search just
to figure out how to save and exit :-)
^ permalink raw reply
* Re: [PATCH v7 8/9] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
From: Sean Christopherson @ 2026-04-07 16:54 UTC (permalink / raw)
To: Jim Mattson
Cc: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-doc, linux-kernel, linux-kselftest, Yosry Ahmed
In-Reply-To: <CALMp9eSysKOVGF_xakbT59tVsgER6oEYpJuK9=hQutjY=ZpM-A@mail.gmail.com>
On Tue, Apr 07, 2026, Jim Mattson wrote:
> On Tue, Apr 7, 2026 at 7:14 AM Sean Christopherson <seanjc@google.com> wrote:
> > > > use_separate_l2_pat = (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP);
> > > > if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT))
> > > > use_separate_l2_pat = false;
> > >
> > > Wow. I really have no idea how to predict what you're going to want
> > > the code to look like. How is this better than the original?!?
> >
> > It doesn't immediately wrap after the "=". Similar to my view on wrapping before
> > function names[*], I find wrapping immediately after an assignment operator to be
> > unnecessarily difficult to read as it doesn't provide any context for single-line
> > searches.
>
> That's actually a good argument to *never* wrap a line. If a line is
> broken at all, the interesting context might follow the line break.
Don't let perfect be the enemy of good. :-)
> > I'm pretty darn consistent in my dislike for that style: I count 26 instances in
> > arch/x86/kvm that match "\s=\n", and only two of those carry my SoB or R-b. I
> > simply missed the wrap in kvm_vcpu_apicv_activated() that was added by commit
> > 896046474f8d ("KVM: x86: Introduce kvm_x86_call() to simplify static calls of
> > kvm_x86_ops"), and I'll give myself a pass for commit 8764ed55c970 ("KVM: x86:
> > Whitelist port 0x7e for pre-incrementing %rip") as that predates treating
> > checkpatch's 80 char limit as a soft limit.
>
> Might I suggest that you should provide a tool—something like
> checkpatch.pl—that flags style violations?
Or maybe extend checkpatch with an optional "feature"? Or subsystem-specific
rules?
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Jim Mattson @ 2026-04-07 16:46 UTC (permalink / raw)
To: Pawan Gupta
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <20260407163943.y6tkh26z2rfktn3y@desk>
On Tue, Apr 7, 2026 at 9:40 AM Pawan Gupta
<pawan.kumar.gupta@linux.intel.com> wrote:
>
> On Mon, Apr 06, 2026 at 07:23:25AM -0700, Jim Mattson wrote:
> > Yes, but the guest needs a way to determine whether the hypervisor
> > will do what's necessary to make the short sequence effective. And, in
> > particular, no KVM hypervisor today is prepared to do that.
> >
> > When running under a hypervisor, without BHI_CTRL and without any
> > evidence to the contrary, the guest must assume that the longer
> > sequence is necessary. At the very least, we need a CPUID or MSR bit
> > that says, "the short BHB clearing sequence is adequate for this
> > vCPU."
>
> After discussing this internally, the consensus is that the best path
> forward is to add virtual SPEC_CTRL support to KVM, which also aligns with
> Intel's guidance. In the long term, virtual SPEC_CTRL can benefit future
> mitigations as well. As with many other mitigations (e.g. microcode), the
> guest would rely on the host to enforce the appropriate protections.
I don't think it's reasonable for the guest to rely on a future
implementation to enforce the appropriate protections.
This is already a problem today. If a guest sees that BHI_CTRL is
unavailable, it will deploy the short BHB clearing sequence and
declare that the vulnerability is mitigated. That isn't true if the
guest is running on Alder Lake or newer.
^ permalink raw reply
* Re: [PATCH v9 02/10] x86/bhi: Make clear_bhb_loop() effective on newer CPUs
From: Pawan Gupta @ 2026-04-07 16:39 UTC (permalink / raw)
To: Jim Mattson
Cc: x86, Jon Kohler, Nikolay Borisov, H. Peter Anvin, Josh Poimboeuf,
David Kaplan, Sean Christopherson, Borislav Petkov, Dave Hansen,
Peter Zijlstra, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, KP Singh, Jiri Olsa, David S. Miller,
David Laight, Andy Lutomirski, Thomas Gleixner, Ingo Molnar,
David Ahern, Martin KaFai Lau, Eduard Zingerman, Song Liu,
Yonghong Song, John Fastabend, Stanislav Fomichev, Hao Luo,
Paolo Bonzini, Jonathan Corbet, linux-kernel, kvm, Asit Mallick,
Tao Zhang, bpf, netdev, linux-doc, chao.gao
In-Reply-To: <CALMp9eR70eE2U63gzNzTiic0PqJVGv3CBBuVUOVbi3nqbWKZkQ@mail.gmail.com>
On Mon, Apr 06, 2026 at 07:23:25AM -0700, Jim Mattson wrote:
> Yes, but the guest needs a way to determine whether the hypervisor
> will do what's necessary to make the short sequence effective. And, in
> particular, no KVM hypervisor today is prepared to do that.
>
> When running under a hypervisor, without BHI_CTRL and without any
> evidence to the contrary, the guest must assume that the longer
> sequence is necessary. At the very least, we need a CPUID or MSR bit
> that says, "the short BHB clearing sequence is adequate for this
> vCPU."
After discussing this internally, the consensus is that the best path
forward is to add virtual SPEC_CTRL support to KVM, which also aligns with
Intel's guidance. In the long term, virtual SPEC_CTRL can benefit future
mitigations as well. As with many other mitigations (e.g. microcode), the
guest would rely on the host to enforce the appropriate protections.
^ permalink raw reply
* Re: [PATCH v5 2/3] ima: trim N IMA event log records
From: Roberto Sassu @ 2026-04-07 16:19 UTC (permalink / raw)
To: steven chen, linux-integrity
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, corbet,
serge, paul, jmorris, linux-security-module, anirudhve,
gregorylumen, nramas, sushring, linux-doc
In-Reply-To: <20260401172956.4581-3-chenste@linux.microsoft.com>
On Wed, 2026-04-01 at 10:29 -0700, steven chen wrote:
> Trim N entries of the IMA event logs. Do not clean the hash table.
The very first change of this patch is the kernel option
ima_flush_htable option that I introduced for my use case.
At the bottom of this patch you actually check the ima_flush_htable
boolean, and delete the measurements entries without disconnecting them
from the hash table, so the digest lookup is done on freed memory.
Next, you duplicated my changes regarding the measurements list
counter. But instead of removing the old counter from the hash table,
you keep incrementing both, but use the new one.
In ima_log_trim_open(), you use again my duplicated code to manage
exclusive write/concurrent read scheme for the measurement interfaces.
However, for read, if the process does not have CAP_SYS_ADMIN it falls
back calling _ima_measurements_open(). Not sure it was intended.
And, in ima_log_trim_release(), you check again CAP_SYS_ADMIN which is
redundant, you would not reach this code if the same requirements were
not met at open time. You also return an error on close().
In ima_log_trim_write(), you do manual string to number conversion for
your first number and use kstrtoul() for the second.
The measurements lists and the associated counter are atomically
updated in ima_add_digest_entry(), but not atomically accessed in
ima_delete_event_log(). Also, the measurements list is traversed
without _rcu variant or lock.
While this trimming scheme aims at minimizing the kernel space and user
space delay, it also introduces the following problem. If two agents
perform a TPM quote that include a different number of entries, there
is no guarantee that the one willing to trim less entries wins. Which
means that, one agent could end up not seeing the most recent entries,
as they were already trimmed by the other agent.
My solution is not affected by this problem, since there will be only
one process collecting all the measurements in user space and exposing
them to the agents.
Also, I didn't understand why T and ima_measure_users have to be
preserved on soft reboots. Especially ima_measure_users reflects the
state of open files for a particular kernel, but on soft reboot a new
kernel is booted.
I personally will not endorse a solution based on the ima_trim_log
interface. I could accept trimming N even more efficiently than we
currently do with a lockless walk to determine the cutting position in
ima_queue_stage(), so that we don't need to splice back entries to the
measurement list. This would be a replacement of patch 11 in my patch
set, but this would be as far as I would like to go.
Roberto
> The values saved in hash table were already used.
>
> Provide a userspace interface ima_trim_log:
> When read this interface, it returns total number T of entries trimmed
> since system boot up.
> When write to this interface need to provide two numbers T:N to let
> kernel to trim N entries of IMA event logs.
>
> Kernel measurement list lock time performance improvement by not
> clean the hash table.
>
> when kernel get log trim request T:N
> - Get the T, compare with the total trimmed number
> - if equal, then do trim N and change T to T+N
> - else return error
>
> Signed-off-by: steven chen <chenste@linux.microsoft.com>
> ---
> .../admin-guide/kernel-parameters.txt | 4 +
> security/integrity/ima/ima.h | 4 +-
> security/integrity/ima/ima_fs.c | 198 +++++++++++++++++-
> security/integrity/ima/ima_kexec.c | 2 +-
> security/integrity/ima/ima_queue.c | 96 +++++++++
> 5 files changed, 296 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index e92c0056e4e0..cd1a1d0bf0e2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2197,6 +2197,10 @@
> Use the canonical format for the binary runtime
> measurements, instead of host native format.
>
> + ima_flush_htable [IMA]
> + Flush the measurement list hash table when trim all
> + or a part of it for deletion.
> +
> ima_hash= [IMA]
> Format: { md5 | sha1 | rmd160 | sha256 | sha384
> | sha512 | ... }
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e3d71d8d56e3..5cbee3a295a0 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -243,11 +243,13 @@ void ima_post_key_create_or_update(struct key *keyring, struct key *key,
> const void *payload, size_t plen,
> unsigned long flags, bool create);
> #endif
> -
> +extern atomic_long_t ima_number_entries;
> #ifdef CONFIG_IMA_KEXEC
> void ima_measure_kexec_event(const char *event_name);
> +long ima_delete_event_log(long req_val);
> #else
> static inline void ima_measure_kexec_event(const char *event_name) {}
> +static inline long ima_delete_event_log(long req_val) { return 0; }
> #endif
>
> /*
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 87045b09f120..8e26e0f34311 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -21,6 +21,9 @@
> #include <linux/rcupdate.h>
> #include <linux/parser.h>
> #include <linux/vmalloc.h>
> +#include <linux/ktime.h>
> +#include <linux/timekeeping.h>
> +#include <linux/ima.h>
>
> #include "ima.h"
>
> @@ -38,6 +41,17 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
>
> static int valid_policy = 1;
>
> +#define IMA_LOG_TRIM_REQ_NUM_LENGTH 15
> +#define IMA_LOG_TRIM_REQ_TOTAL_LENGTH 32
> +atomic_long_t ima_number_entries = ATOMIC_LONG_INIT(0);
> +static long trimcount;
> +/* mutex protects atomicity of trimming measurement list
> + * and also protects atomicity the measurement list read
> + * write operation.
> + */
> +static DEFINE_MUTEX(ima_measure_lock);
> +static long ima_measure_users;
> +
> static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> loff_t *ppos, atomic_long_t *val)
> {
> @@ -64,8 +78,7 @@ static ssize_t ima_show_measurements_count(struct file *filp,
> char __user *buf,
> size_t count, loff_t *ppos)
> {
> - return ima_show_htable_value(buf, count, ppos, &ima_htable.len);
> -
> + return ima_show_htable_value(buf, count, ppos, &ima_number_entries);
> }
>
> static const struct file_operations ima_measurements_count_ops = {
> @@ -202,16 +215,77 @@ static const struct seq_operations ima_measurments_seqops = {
> .show = ima_measurements_show
> };
>
> +/*
> + * _ima_measurements_open - open the IMA measurements file
> + * @inode: inode of the file being opened
> + * @file: file being opened
> + * @seq_ops: sequence operations for the file
> + *
> + * Returns 0 on success, or negative error code.
> + * Implements mutual exclusion between readers and writer
> + * of the measurements file. Multiple readers are allowed,
> + * but writer get exclusive access only no other readers/writers.
> + * Readers is not allowed when there is a writer.
> + */
> +static int _ima_measurements_open(struct inode *inode, struct file *file,
> + const struct seq_operations *seq_ops)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + if (write && !capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + mutex_lock(&ima_measure_lock);
> + if ((write && ima_measure_users != 0) ||
> + (!write && ima_measure_users < 0)) {
> + mutex_unlock(&ima_measure_lock);
> + return -EBUSY;
> + }
> +
> + ret = seq_open(file, seq_ops);
> + if (ret < 0) {
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> + }
> +
> + if (write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> +}
> +
> static int ima_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_measurments_seqops);
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static int ima_measurements_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> + int ret;
> +
> + mutex_lock(&ima_measure_lock);
> + ret = seq_release(inode, file);
> + if (!ret) {
> + if (!write)
> + ima_measure_users--;
> + else
> + ima_measure_users++;
> + }
> +
> + mutex_unlock(&ima_measure_lock);
> + return ret;
> }
>
> static const struct file_operations ima_measurements_ops = {
> .open = ima_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> };
>
> void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)
> @@ -279,14 +353,114 @@ static const struct seq_operations ima_ascii_measurements_seqops = {
>
> static int ima_ascii_measurements_open(struct inode *inode, struct file *file)
> {
> - return seq_open(file, &ima_ascii_measurements_seqops);
> + return _ima_measurements_open(inode, file, &ima_ascii_measurements_seqops);
> }
>
> static const struct file_operations ima_ascii_measurements_ops = {
> .open = ima_ascii_measurements_open,
> .read = seq_read,
> .llseek = seq_lseek,
> - .release = seq_release,
> + .release = ima_measurements_release,
> +};
> +
> +static int ima_log_trim_open(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return _ima_measurements_open(inode, file, &ima_measurments_seqops);
> +}
> +
> +static ssize_t ima_log_trim_read(struct file *file, char __user *buf, size_t size, loff_t *ppos)
> +{
> + char tmpbuf[IMA_LOG_TRIM_REQ_NUM_LENGTH];
> + ssize_t len;
> +
> + len = scnprintf(tmpbuf, sizeof(tmpbuf), "%li\n", trimcount);
> + return simple_read_from_buffer(buf, size, ppos, tmpbuf, len);
> +}
> +
> +static ssize_t ima_log_trim_write(struct file *file,
> + const char __user *buf, size_t datalen, loff_t *ppos)
> +{
> + char tmpbuf[IMA_LOG_TRIM_REQ_TOTAL_LENGTH];
> + char *p = tmpbuf;
> + long count, ret, val = 0, max = LONG_MAX;
> +
> + if (*ppos > 0 || datalen > IMA_LOG_TRIM_REQ_TOTAL_LENGTH || datalen < 2) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + if (copy_from_user(tmpbuf, buf, datalen) != 0) {
> + ret = -EFAULT;
> + goto out;
> + }
> +
> + p = tmpbuf;
> +
> + while (*p && *p != ':') {
> + if (!isdigit((unsigned char)*p))
> + return -EINVAL;
> +
> + /* digit value */
> + int d = *p - '0';
> +
> + /* overflow check: val * 10 + d > max -> (val > (max - d) / 10) */
> + if (val > (max - d) / 10)
> + return -ERANGE;
> +
> + val = val * 10 + d;
> + p++;
> + }
> +
> + if (*p != ':')
> + return -EINVAL;
> +
> + /* verify trim count matches */
> + if (val != trimcount)
> + return -EINVAL;
> +
> + p++; /* skip ':' */
> + ret = kstrtoul(p, 0, &count);
> +
> + if (ret < 0)
> + goto out;
> +
> + ret = ima_delete_event_log(count);
> +
> + if (ret < 0)
> + goto out;
> +
> + trimcount += ret;
> +
> + ret = datalen;
> +out:
> + return ret;
> +}
> +
> +static int ima_log_trim_release(struct inode *inode, struct file *file)
> +{
> + bool write = !!(file->f_mode & FMODE_WRITE);
> +
> + if (!write && capable(CAP_SYS_ADMIN))
> + return 0;
> + else if (!capable(CAP_SYS_ADMIN))
> + return -EPERM;
> +
> + return ima_measurements_release(inode, file);
> +}
> +
> +static const struct file_operations ima_log_trim_ops = {
> + .open = ima_log_trim_open,
> + .read = ima_log_trim_read,
> + .write = ima_log_trim_write,
> + .llseek = generic_file_llseek,
> + .release = ima_log_trim_release
> };
>
> static ssize_t ima_read_policy(char *path)
> @@ -528,6 +702,18 @@ int __init ima_fs_init(void)
> goto out;
> }
>
> + if (IS_ENABLED(CONFIG_IMA_LOG_TRIMMING)) {
> + dentry = securityfs_create_file("ima_trim_log",
> + S_IRUSR | S_IRGRP | S_IWUSR | S_IWGRP,
> + ima_dir, NULL, &ima_log_trim_ops);
> + if (IS_ERR(dentry)) {
> + ret = PTR_ERR(dentry);
> + goto out;
> + }
> + }
> +
> + trimcount = 0;
> +
> dentry = securityfs_create_file("runtime_measurements_count",
> S_IRUSR | S_IRGRP, ima_dir, NULL,
> &ima_measurements_count_ops);
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> index 7362f68f2d8b..bee997683e03 100644
> --- a/security/integrity/ima/ima_kexec.c
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -41,7 +41,7 @@ void ima_measure_kexec_event(const char *event_name)
> int n;
>
> buf_size = ima_get_binary_runtime_size();
> - len = atomic_long_read(&ima_htable.len);
> + len = atomic_long_read(&ima_number_entries);
>
> n = scnprintf(ima_kexec_event, IMA_KEXEC_EVENT_LEN,
> "kexec_segment_size=%lu;ima_binary_runtime_size=%lu;"
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 590637e81ad1..07225e19b9b5 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -22,6 +22,14 @@
>
> #define AUDIT_CAUSE_LEN_MAX 32
>
> +bool ima_flush_htable;
> +static int __init ima_flush_htable_setup(char *str)
> +{
> + ima_flush_htable = true;
> + return 1;
> +}
> +__setup("ima_flush_htable", ima_flush_htable_setup);
> +
> /* pre-allocated array of tpm_digest structures to extend a PCR */
> static struct tpm_digest *digests;
>
> @@ -114,6 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
> list_add_tail_rcu(&qe->later, &ima_measurements);
>
> atomic_long_inc(&ima_htable.len);
> + atomic_long_inc(&ima_number_entries);
> if (update_htable) {
> key = ima_hash_key(entry->digests[ima_hash_algo_idx].digest);
> hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> @@ -220,6 +229,93 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> return result;
> }
>
> +/**
> + * ima_delete_event_log - delete IMA event entry
> + * @num_records: number of records to delete
> + *
> + * delete num_records entries off the measurement list.
> + * Returns num_records, or negative error code.
> + */
> +long ima_delete_event_log(long num_records)
> +{
> + long len, cur = num_records, tmp_len = 0;
> + struct ima_queue_entry *qe, *qe_tmp;
> + LIST_HEAD(ima_measurements_to_delete);
> + struct list_head *list_ptr;
> +
> + if (!IS_ENABLED(CONFIG_IMA_LOG_TRIMMING))
> + return -EOPNOTSUPP;
> +
> + if (num_records <= 0)
> + return num_records;
> +
> + list_ptr = &ima_measurements;
> +
> + len = atomic_long_read(&ima_number_entries);
> +
> + if (num_records <= len) {
> + list_for_each_entry(qe, list_ptr, later) {
> + if (cur > 0) {
> + tmp_len += get_binary_runtime_size(qe->entry);
> + --cur;
> + }
> + if (cur == 0) {
> + qe_tmp = qe;
> + break;
> + }
> + }
> + }
> + else {
> + return -ENOENT;
> + }
> +
> +
> + mutex_lock(&ima_extend_list_mutex);
> + len = atomic_long_read(&ima_number_entries);
> +
> + if (num_records == len) {
> + list_replace(&ima_measurements, &ima_measurements_to_delete);
> + INIT_LIST_HEAD(&ima_measurements);
> + atomic_long_set(&ima_number_entries, 0);
> + list_ptr = &ima_measurements_to_delete;
> + }
> + else {
> + __list_cut_position(&ima_measurements_to_delete, &ima_measurements,
> + &qe_tmp->later);
> + atomic_long_sub(num_records, &ima_number_entries);
> + if (IS_ENABLED(CONFIG_IMA_KEXEC))
> + binary_runtime_size -= tmp_len;
> + }
> +
> + mutex_unlock(&ima_extend_list_mutex);
> +
> + if (ima_flush_htable)
> + synchronize_rcu();
> +
> + list_for_each_entry_safe(qe, qe_tmp, &ima_measurements_to_delete, later) {
> + /*
> + * Ok because after list delete qe is only accessed by
> + * ima_lookup_digest_entry().
> + */
> + for (int i = 0; i < qe->entry->template_desc->num_fields; i++) {
> + kfree(qe->entry->template_data[i].data);
> + qe->entry->template_data[i].data = NULL;
> + qe->entry->template_data[i].len = 0;
> + }
> +
> + list_del(&qe->later);
> +
> + /* No leak if !ima_flush_htable, referenced by ima_htable. */
> + if (ima_flush_htable) {
> + kfree(qe->entry->digests);
> + kfree(qe->entry);
> + kfree(qe);
> + }
> + }
> +
> + return num_records;
> +}
> +
> int ima_restore_measurement_entry(struct ima_template_entry *entry)
> {
> int result = 0;
^ permalink raw reply
* Re: [PATCH v7 1/9] KVM: x86: Define KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT
From: Jim Mattson @ 2026-04-07 16:27 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-doc, linux-kernel, linux-kselftest, Yosry Ahmed
In-Reply-To: <adRBZuqNlBozaDrK@google.com>
On Mon, Apr 6, 2026 at 4:27 PM Sean Christopherson <seanjc@google.com> wrote:
>
> On Fri, Mar 27, 2026, Jim Mattson wrote:
> > diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> > index ff1e4b4dc998..74014110b550 100644
> > --- a/arch/x86/kvm/svm/svm.h
> > +++ b/arch/x86/kvm/svm/svm.h
> > @@ -616,6 +616,17 @@ static inline bool nested_npt_enabled(struct vcpu_svm *svm)
> > return svm->nested.ctl.misc_ctl & SVM_MISC_ENABLE_NP;
> > }
> >
> > +static inline bool l2_has_separate_pat(struct vcpu_svm *svm)
>
> Take @vcpu instead of @svm. All of the callers have a "vcpu", but not all have
> a local "svm". That will shorten the quirk check far enough to let it poke out.
What is the actual line length limit?
> > +{
> > + /*
> > + * If KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT is disabled while a vCPU
> > + * is running, the L2 IA32_PAT semantics for that vCPU are undefined.
> > + */
> > + return nested_npt_enabled(svm) &&
> > + !kvm_check_has_quirk(svm->vcpu.kvm,
> > + KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
>
> Align indentation. With the @svm => @vcpu change, this becomes:
>
> return nested_npt_enabled(to_svm(vcpu)) &&
> !kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
You wouldn't happen to know the Emacs configuration for the alignment
you like, would you? I asked Gemini, but it lied to me.
> > +}
> > +
> > static inline bool nested_vnmi_enabled(struct vcpu_svm *svm)
> > {
> > return guest_cpu_cap_has(&svm->vcpu, X86_FEATURE_VNMI) &&
> > --
> > 2.53.0.1018.g2bb0e51243-goog
> >
^ permalink raw reply
* Re: [PATCH] hwmon: (yogafan) various markup improvements
From: Sergio Melas @ 2026-04-07 16:12 UTC (permalink / raw)
To: Guenter Roeck
Cc: Randy Dunlap, linux-kernel, linux-hwmon, Jonathan Corbet,
Shuah Khan, linux-doc
In-Reply-To: <7752cce3-3362-42c0-becd-96dbc7b17cab@roeck-us.net>
Hi Guenter,
My apologies for the confusion—I am still learning the standard
workflow. I understand now why applying Randy’s patch immediately is
the correct move.
When I mentioned the "next version," I was thinking about a major
expansion I am currently preparing (v1, second round). It expands
support to nearly all Lenovo and Xiaoxin models. Because the database
has grown so much, I’ve had to significantly change the table format
in the .rst file to keep it readable. So i was referring to this new
table (see below). Fully open to modify the format if you thin is not
ok.
As an automation engineer , this process is quite new to me, so I
appreciate your patience as I learn the proper terms and procedures. I
will ensure my next submission is rebased on your current tree with
Randy's improvements.
Best regards, Sergio
::
================================================
LENOVO FAN CONTROLLER Hardware Abstraction Layer
================================================
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| MODEL | FAMILY / SERIES | OFFSET | FULL ACPI OBJECT PATH
| WIDTH | NMAX | RMAX | MULT |
+=============+===================+=========+================================+========+=======+=======+======+
| 82N7 | Yoga 14cACN | 0x06 | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 0 | 5500 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83E2 | Yoga Pro 9i | 0xFE/FF | _SB.PCI0.LPC0.EC0.FANS
(Fan1) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83E2 | Yoga Pro 9i | 0xFE/FF | _SB.PCI0.LPC0.EC0.FA2S
(Fan2) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83CV | Yoga Pro 9 (Aura) | 0xFE | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 0 | 6000 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83DN | Yoga Pro 7 | 0xFE | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 0 | 6000 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82A2 / 82A3 | Yoga Slim 7 | 0x06 | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 0 | 5500 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83JC / 83DX | Xiaoxin Pro 14/16 | 0xFE | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 80 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83FD / 83DE | Xiaoxin Pro | 0xFE/FF |
_SB.PCI0.LPC0.EC0.FAN0/.FANS | 8-bit | 0 | 5000 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 81YM / 82FG | IdeaPad 5 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 4500 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 83AK | ThinkBook G7 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 5400 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 81X1 | Flex 5 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 4500 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legion 9 | Legion 9i / Extr | 0xFE/FF | _SB.PCI0.LPC0.EC0.FANS
(Fan1) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legion 9 | Legion 9i / Extr | 0xFE/FF | _SB.PCI0.LPC0.EC0.FA2S
(Fan2) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legion 9 | Legion 9i / Extr | 0xFE/FF | _SB.PCI0.LPC0.EC0.FA3S
(Fan3) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82WQ | Legion 7i (Int) | 0xFE/FF | _SB.PCI0.LPC0.EC0.FANS
(Fan1) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82WQ | Legion 7i (Int) | 0xFE/FF | _SB.PCI0.LPC0.EC0.FA2S
(Fan2) | 16-bit | 0 | 8000 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82JW / 82JU | Legion 5 (AMD) | 0xFE/FF | _SB.PCI0.LPC0.EC0.FANS
(Fan1) | 16-bit | 0 | 6500 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82JW / 82JU | Legion 5 (AMD) | 0xFE/FF | _SB.PCI0.LPC0.EC0.FA2S
(Fan2) | 16-bit | 0 | 6500 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| GeekPro | GeekPro G5000/6k | 0xFE/FF | _SB.PCI0.LPC0.EC0.FANS
(Fan1) | 16-bit | 0 | 6500 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82XV / 83DV | LOQ 15/16 | 0xFE/FF | _SB.PCI0.LPC0.EC0.FANS
(Fan1) | 16-bit | 0 | 6500 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 82XV / 83DV | LOQ 15/16 | 0xFE/FF | _SB.PCI0.LPC0.EC0.FA2S
(Fan2) | 16-bit | 0 | 6500 | 1 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80V2 / 81C3 | Yoga 710/720 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 59 | 4500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80S7 | Yoga 510 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 41 | 4500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80JH | Yoga 3 14 | 0x06 |
_SB.PCI0.LPC0.EC0.FAN0/.FANS | 8-bit | 80 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 20344 | Yoga 2 13 | 0xAB | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 8 | 4200 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 2191 / 20191| Yoga 13 | 0xF2/F3 | _SB.PCI0.LPC0.EC0.FAN1/2
| 8-bit | 0 | 5000 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legacy | Yoga 11s | 0x56 |
_SB.PCI0.LPC0.EC0.FAN0/.FANS | 8-bit | 80 | 4500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 20GJ / 20GK | ThinkPad 13 | 0x85 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 7 | 5500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 1143 | ThinkPad E520 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 4200 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 3698 | ThinkPad Helix | 0x2F | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 4500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 20M7 / 20M8 | ThinkPad L380 | 0x95 | _SB.PCI0.LPC0.EC0.FAN1
| 8-bit | 0 | 4600 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 20NR / 20NS | ThinkPad L390 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 5500 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 2464 / 2468 | ThinkPad L530 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 4400 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 2356 | ThinkPad T430s | 0x2F | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 20AQ / 20AR | ThinkPad T440s | 0x4E | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 5200 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 20BE / 20BF | ThinkPad T540p | 0x2F | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 5500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 3051 | ThinkPad x121e | 0x2F | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 4500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 4290 | ThinkPad x220i | 0x2F | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 2324 / 2325 | ThinkPad x230 | 0x2F | _SB.PCI0.LPC0.EC0.FANS
| 8-bit | 7 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 81AX | V330-15IKB | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 5100 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legacy | IdeaPad Y580 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 35 | 4800 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legacy | IdeaPad V580 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 0 | 5000 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80SR / 80SX | IdeaPad 500S-13 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 44 | 5500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80S1 | IdeaPad 500S-14 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 116 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80TK | IdeaPad 510S | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 41 | 5100 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80S9 | IdeaPad 710S | 0x95/98 | _SB.PCI0.LPC0.EC0.FAN1/2
| 8-bit | 0 | 5200 | 100 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80KU | U31-70 | 0x06 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 44 | 5500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| 80S1 | U41-70 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 116 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legacy | U160 | 0x95 | _SB.PCI0.LPC0.EC0.FAN0
| 8-bit | 64 | 4500 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
| Legacy | U330p/U430p | 0x92 | _SB.PCI0.LPC0.EC0.FAN0
| 16-bit | 768 | 5000 | 0 |
+-------------+-------------------+---------+--------------------------------+--------+-------+-------+------+
Note for the raw_RPM we have 2 cases:
* Discrete Level Estimation
**Nmax > 0 then raw_RPM = (Rmax * IN) / Nmax**
* Continuous Unit Mapping
**Nmax = 0 then raw_RPM = IN * Multiplier**
^ permalink raw reply
* Re: (sashiko review) [PATCH v6 1/1] mm/damon: add node_eligible_mem_bp and node_ineligible_mem_bp goal metrics
From: SeongJae Park @ 2026-04-07 16:05 UTC (permalink / raw)
To: SeongJae Park
Cc: Ravi Jonnalagadda, damon, linux-mm, linux-kernel, linux-doc, akpm,
corbet, bijan311, ajayjoshi, honggyu.kim, yunjeong.mun
In-Reply-To: <20260407001310.78557-1-sj@kernel.org>
Adding another thought at the end of the mail without cutting the previous
unrelated questions, so that Ravi can answer all my questions at once.
On Mon, 6 Apr 2026 17:13:08 -0700 SeongJae Park <sj@kernel.org> wrote:
> On Mon, 6 Apr 2026 12:47:56 -0700 Ravi Jonnalagadda <ravis.opensrc@gmail.com> wrote:
>
> > On Sun, Apr 5, 2026 at 3:45 PM SeongJae Park <sj@kernel.org> wrote:
> > >
> > >
> > > Ravi, thank you for reposting this patch after the rebase. This time sashiko
> > > was able to review this, and found good points including things that deserve
> > > another revision of this patch.
> > >
> > > Forwarding full sashiko review in a reply format with my inline comments below,
> > > for sharing details of my view and doing followup discussions via mails. Ravi,
> > > could you please reply?
> > >
> >
> > Thanks SJ, providing your comments on top of sashiko's review is very helpful.
>
> I'm glad to hear that it is working for you :)
>
> [...]
> > > > +static unsigned long damos_calc_eligible_bytes(struct damon_ctx *c,
> > > > > + struct damos *s, int nid, unsigned long *total)
> > > > > +{
> [...]
> > > > > + struct folio *folio;
> > > > > + unsigned long folio_sz, counted;
> > > > > +
> > > > > + folio = damon_get_folio(PHYS_PFN(addr));
> > > >
> > > > What happens if this metric is assigned to a DAMON context configured for
> > > > virtual address space monitoring? If the context uses DAMON_OPS_VADDR,
> > > > passing a user-space virtual address to PHYS_PFN() might cause invalid
> > > > memory accesses or out-of-bounds page struct reads. Should this code
> > > > explicitly verify the operations type first?
> > >
> > > Good finding. We intend to support only paddr ops. But there is no guard for
> > > using this on vaddr ops configuration. Ravi, could we add underlying ops
> > > check? I think damon_commit_ctx() is a good place to add that. The check
> > > could be something like below?
> > >
> >
> > I plan to add the ops type check directly in the metric functions
> > (damos_get_node_eligible_mem_bp and its counterpart) rather than in
> > damon_commit_ctx(). The functions will return 0 early
> > if c->ops.id != DAMON_OPS_PADDR.
> >
> > That said, if you prefer the damon_commit_ctx() validation approach to
> > reject the configuration outright, I can implement it that way instead.
> > Please let me know your preference.
>
> I'd prefer damon_commit_ctx() validation approach since it would give users
> more clear message of the failure.
>
> >
> > > '''
> > > --- a/mm/damon/core.c
> > > +++ b/mm/damon/core.c
> > > @@ -1515,10 +1515,23 @@ static int damon_commit_sample_control(
> > > int damon_commit_ctx(struct damon_ctx *dst, struct damon_ctx *src)
> > > {
> > > int err;
> > > + struct damos *scheme;
> > > + struct damos_quota_goal *goal;
> > >
> > > dst->maybe_corrupted = true;
> > > if (!is_power_of_2(src->min_region_sz))
> > > return -EINVAL;
> > > + if (src->ops.id != DAMON_OPS_PADDR) {
> > > + damon_for_each_scheme(scheme, src) {
> > > + damos_for_each_quota_goal(goal, &scheme->quota) {
> > > + switch (goal->metric) {
> > > + case DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP:
> > > + case DAMOS_QUOTA_NODE_INELIGIBLE_MEMPBP:
> > > + return -EINVAL;
> > > + }
> > > + }
> > > + }
> > > + }
> > >
> > > err = damon_commit_schemes(dst, src);
> > > if (err)
> > > '''
> [...]
> > > > > + /* Compute ineligible ratio directly: 10000 - eligible_bp */
> > > > > + return 10000 - mult_frac(node_eligible, 10000, total_eligible);
> > > > > +}
> > > >
> > > > Does this return value match the documented metric? The formula computes the
> > > > percentage of the system's eligible memory located on other NUMA nodes,
> > > > rather than the amount of actual ineligible (filtered out) memory residing
> > > > on the target node. Could this semantic mismatch cause confusion when
> > > > configuring quota policies?
> > >
> > > Nice catch. The name and the documentation are confusing. We actually
> > > confused a few times in previous revisions, and I'm again confused now. IIUC,
> > > the current implementation is the intended and right one for the given use
> > > case, though. If my understanding is correct, how about renaming
> > > DAMOS_QUOTA_NODE_INELIGIBLE_MEM_BP to
> > > DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP_COMPLEMENT, and updating the documentation
> > > together? Ravi, what do you think?
> > >
> >
> > Agreed, the current name is confusing. How about
> > DAMOS_QUOTA_NODE_ELIGIBLE_MEM_BP_OFFNODE?
> >
> > The rationale is that this metric measures "eligible memory that is off
> > this node" (i.e., on other nodes).
> >
> > I think "offnode" conveys the physical meaning more directly than "complement".
> > That said, I'm happy to go with "complement" if you prefer.
> > both are clearer than "ineligible".
>
> Thank you for the nice suggestion. I like "offnode" term. But I think having
> "node" twice on the name is not really efficient for people who print code on
> papers. What about DAMOS_QUOTA_OFFNODE_ELIGIBLE_MEM_BP?
>
> But... Maybe more importantly... Now I realize this means that
> offnode_eligible_mem_bp with target nid 0 is just same to node_eligible_mem_bp
> with target nid 1, on your test setup. Maybe we don't really need
> offnode_eligible_mem_bp? That is, your test setup could be like below.
>
> '''
> For maintaining hot memory on DRAM (node 0) and CXL (node 1) in a 7:3
> ratio:
>
> PUSH scheme: migrate_hot from node 0 -> node 1
> goal: node_eligible_mem_bp, nid=1, target=3000
> "Move hot pages from DRAM to CXL if less thatn 30% of hot data is
> in CXL"
>
> PULL scheme: migrate_hot from node 1 -> node 0
> goal: node_eligible_mem_bp, nid=0, target=7000
> "Move hot pages from CXL to DRAM if less than 70% of hot data is
> in DRAM"
> '''
>
> And the schemes are more easy to read and understand for me. This seems even
> straightforward to scale for >2 nodes. For example, if we want hot memory
> distribution of 5:3:2 to nodes 0:1:2,
>
> Two schemes for migrating hot pages out of node 0
> - migrate_hot from node 0 -> node 1
> - goal: node_eligible_mem_bp, nid=1, target=3000
> - migrate_hot from node 0 -> node 2
> - goal: node_eligible_mem_bp, nid=2, target=2000
>
> Two schemes for migrating hot pages out of node 1
> - migrate_hot from node 1 -> node 0
> - goal: node_eligible_mem_bp, nid=0, target=5000
> - migrate_hot from node 1 -> node 2
> - goal: node_eligible_mem_bp, nid=2, target=2000
>
> Two schemes for migrating hot pages out of node 2
> - migrate_hot from node 2 -> node 0
> - goal: node_eligible_mem_bp, nid=0, target=5000
> - migrate_hot from node 2 -> node 1
> - goal: node_eligible_mem_bp, nid=1, target=3000
>
> Do you think this makes sense? If it makes sense and works for your use case,
> what about dropping the offnode goal type?
Now I recall I suggested the offnode metric because I suggested to run a
kdamond per node. That is, having one kdamond that monitors only node 0 and
migrate hot memory to node 1, and another kdamond that monitors only node 1 and
migrate hot memory to node 0. And I suggested to do so because I knew it is
suboptimal to run DAMOS schemes with node filter.
We made a change [1] for making that more optimum, though. The change is now
in mm-stable, so hopefully it will be available from 7.1-rc1. So I believe the
single quota goal metric should work now. Ravi, could you share what you
think?
[1] commit e1ace69c33ec ("mm/damon/core: set quota-score histogram with core filters")
Thanks,
SJ
[...]
^ permalink raw reply
* Re: [PATCH v7 8/9] KVM: x86: nSVM: Save/restore gPAT with KVM_{GET,SET}_NESTED_STATE
From: Jim Mattson @ 2026-04-07 15:47 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, Jonathan Corbet, Shuah Khan, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
kvm, linux-doc, linux-kernel, linux-kselftest, Yosry Ahmed
In-Reply-To: <adURPZJEDs50NPkB@google.com>
On Tue, Apr 7, 2026 at 7:14 AM Sean Christopherson <seanjc@google.com> wrote:
>
> On Mon, Apr 06, 2026, Jim Mattson wrote:
> > On Mon, Apr 6, 2026 at 4:47 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Fri, Mar 27, 2026, Jim Mattson wrote:
> > > > @@ -1918,6 +1921,7 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> > > > struct vmcb_save_area_cached save_cached;
> > > > struct vmcb_ctrl_area_cached ctl_cached;
> > > > unsigned long cr0;
> > > > + bool use_separate_l2_pat;
> > >
> > > Land this above "cr0" to preserve the inverted fir tree.
> > >
> > > > int ret;
> > > >
> > > > BUILD_BUG_ON(sizeof(struct vmcb_control_area) + sizeof(struct vmcb_save_area) >
> > > > @@ -1993,6 +1997,18 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
> > > > !nested_vmcb_check_save(vcpu, &save_cached, false))
> > > > goto out_free;
> > > >
> > > > + /*
> > > > + * Validate gPAT when the shared PAT quirk is disabled (i.e. L2
> > > > + * has its own gPAT). This is done separately from the
> > > > + * vmcb_save_area_cached validation above, because gPAT is L2
> > > > + * state, but the vmcb_save_area_cached is populated with L1 state.
> > > > + */
> > > > + use_separate_l2_pat =
> > > > + (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP) &&
> > > > + !kvm_check_has_quirk(vcpu->kvm,
> > > > + KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
> > >
> > > I vote for either:
> > >
> > > use_separate_l2_pat = (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP) &&
> > > !kvm_check_has_quirk(vcpu->kvm,
> > > KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT);
> > >
> > LOL! Aren't you the one who keeps complaining that my indentation
> > doesn't line up? Are you schizophrenic?
>
> Huh? That is aligned. Perhaps it's whitespace damaged by your MUA?
Indeed. It was.
> > > or
> > >
> > > use_separate_l2_pat = (ctl_cached.misc_ctl & SVM_MISC_ENABLE_NP);
> > > if (kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_NESTED_SVM_SHARED_PAT))
> > > use_separate_l2_pat = false;
> >
> > Wow. I really have no idea how to predict what you're going to want
> > the code to look like. How is this better than the original?!?
>
> It doesn't immediately wrap after the "=". Similar to my view on wrapping before
> function names[*], I find wrapping immediately after an assignment operator to be
> unnecessarily difficult to read as it doesn't provide any context for single-line
> searches.
That's actually a good argument to *never* wrap a line. If a line is
broken at all, the interesting context might follow the line break.
> I'm pretty darn consistent in my dislike for that style: I count 26 instances in
> arch/x86/kvm that match "\s=\n", and only two of those carry my SoB or R-b. I
> simply missed the wrap in kvm_vcpu_apicv_activated() that was added by commit
> 896046474f8d ("KVM: x86: Introduce kvm_x86_call() to simplify static calls of
> kvm_x86_ops"), and I'll give myself a pass for commit 8764ed55c970 ("KVM: x86:
> Whitelist port 0x7e for pre-incrementing %rip") as that predates treating
> checkpatch's 80 char limit as a soft limit.
Might I suggest that you should provide a tool—something like
checkpatch.pl—that flags style violations?
^ 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