[RFC PATCH 00/24] KVM: x86: Introduce memory protection attributes

[RFC PATCH 00/24] KVM: x86: Introduce memory protection attributes

[Paolo Bonzini]

This series introduces a mechanism to let userspace block read,
write or execute access to individual GFNs via KVM's memory
attribute mechanism, and have them reported via KVM_EXIT_MEMORY_FAULT.
It is mostly the work of Nicolas Saenz Julienne, with a bit of patch
reorganization and code cleanup on my side (and especially using the
revamped ACC_* mask in Linux 7.2).

The reason why it's so large is because  KVM needs to should check the
attributes anytime KVM takes GPAs as input for any action initiated by
the guest; if the memory attributes are incompatible with such action,
it should be stopped.  For more information see
https://lore.kernel.org/kvm/D3MJJCTNY7OM.WOB5W8AVBH9G@amazon.com/.

There are several bits missing:

- hypercall handling for non-HyperV hypercalls is untested.  Xen
  hypercalls are not a big deal (at least for me...) because userspace
  can always avoid using this feature together with Xen emulation,
  but KVM_HC_CLOCK_PAIRING can write to memory.  It should at least
  be tested!

- nested virtualization is untested and there is a known hole
  in patch 21

- MBEC/GMET support should be added too, I have taken it into account
  when rebasing Nicolas's work but haven't written the code yet

- the split between arch-independent and arch-dependent code in
  the tests can be improved.

- the pvclock test fails

The original idea for posting this was to let Sashiko loose on it.
Unfortunately, it probably won't be able to apply the patches :(
because they are based on kvm/next, so here it is in all its glory
for humans only.

Paolo

Anish Moorthy (1):
  KVM: Define and communicate KVM_EXIT_MEMORY_FAULT RWX flags to
    userspace

Nicolas Saenz Julienne (19):
  KVM: selftests: Take into account mixed memory fault flags
  KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas
  KVM: x86: Avoid warning when installing non-private memory attributes
  KVM: x86/mmu: Init memslot hugepage information for non-private_mem VMs too
  KVM: Introduce NR/NW/NX memory attributes
  KVM: Include memory protections in result of gfn->hva conversion
  KVM: Take memory protections into account in kvm_read/write_guest()
  KVM: Encapsulate memattrs array into anonymous struct
  KVM: Introduce a generation number for memory attributes
  KVM: Take memory protections into account for accesses with cached gfn->hva
  KVM: pfncache: Fail to refresh if it contains memory protections
  KVM: x86/mmu: Take memory protection attributes into account during faults
  KVM: x86/mmu: Issue memory fault exit if walk failed due to memory attribute
  KVM: x86/mmu: Do not update accessed/dirty if guest PTE is read-only
  KVM: x86/mmu: Do not prefetch sptes on gfns backed by memory attributes
  KVM: x86/mmu: Obsolete all roots if memattr contains gPTEs
  KVM: x86: selftests: Introduce memory attributes test
  KVM: x86: selftests: Introduce memory attributes PTE test
  KVM: x86: selftests: Introduce memory attributes side-channel tests

Paolo Bonzini (4):
  KVM: x86/mmu: intersect writability from __kvm_faultin_pfn with
    fault->map_writable
  KVM: x86/mmu: Extend map_writable to a full ACC_* mask
  KVM: pass kvm == NULL case to kvm_arch_has_private_mem
  KVM: Introduce kvm_check_gen()/kvm_memslots_check_gen()

 Documentation/virt/kvm/api.rst                |  19 +-
 arch/x86/include/asm/kvm_host.h               |   4 +-
 arch/x86/kvm/Kconfig                          |   4 +-
 arch/x86/kvm/hyperv.c                         |  32 ++
 arch/x86/kvm/mmu/mmu.c                        | 193 ++++++--
 arch/x86/kvm/mmu/mmu_internal.h               |  21 +-
 arch/x86/kvm/mmu/mmutrace.h                   |  29 ++
 arch/x86/kvm/mmu/paging_tmpl.h                |  25 +-
 arch/x86/kvm/mmu/spte.c                       |  12 +-
 arch/x86/kvm/mmu/spte.h                       |  13 +-
 arch/x86/kvm/mmu/tdp_mmu.c                    |   2 +-
 arch/x86/kvm/x86.c                            |  19 +-
 include/linux/kvm_host.h                      | 124 ++++-
 include/linux/kvm_types.h                     |   6 +-
 include/trace/events/kvm.h                    |  14 +-
 include/uapi/linux/kvm.h                      |   7 +
 tools/include/uapi/linux/kvm.h                |   3 +
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/include/kvm_util.h  |  33 +-
 .../selftests/kvm/include/x86/processor.h     |   1 +
 .../testing/selftests/kvm/lib/x86/processor.c |   5 +
 .../testing/selftests/kvm/memory_attributes.c | 457 ++++++++++++++++++
 .../selftests/kvm/x86/memory_attributes.c     | 385 +++++++++++++++
 .../kvm/x86/private_mem_kvm_exits_test.c      |   6 +-
 virt/kvm/kvm_main.c                           | 224 +++++++--
 virt/kvm/pfncache.c                           |  30 +-
 26 files changed, 1529 insertions(+), 140 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/memory_attributes.c
 create mode 100644 tools/testing/selftests/kvm/x86/memory_attributes.c

-- 
2.52.0

[PATCH 01/24] KVM: selftests: Take into account mixed memory fault flags

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Update x86_64/private_mem_kvm_exits_test to take into account memory
fault flags might contain multiple bits set while remaining valid---for
example KVM might return KVM_MEMORY_EXIT_FLAG_READ in addition to
KVM_MEMORY_EXIT_FLAG_PRIVATE.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .../testing/selftests/kvm/x86/private_mem_kvm_exits_test.c  | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/private_mem_kvm_exits_test.c b/tools/testing/selftests/kvm/x86/private_mem_kvm_exits_test.c
index 10db9fe6d906..70ed3d591ab6 100644
--- a/tools/testing/selftests/kvm/x86/private_mem_kvm_exits_test.c
+++ b/tools/testing/selftests/kvm/x86/private_mem_kvm_exits_test.c
@@ -75,7 +75,8 @@ static void test_private_access_memslot_deleted(void)
 	exit_reason = (u32)(u64)thread_return;
 
 	TEST_ASSERT_EQ(exit_reason, KVM_EXIT_MEMORY_FAULT);
-	TEST_ASSERT_EQ(vcpu->run->memory_fault.flags, KVM_MEMORY_EXIT_FLAG_PRIVATE);
+	TEST_ASSERT(vcpu->run->memory_fault.flags & KVM_MEMORY_EXIT_FLAG_PRIVATE,
+		    "Memory fault didn't occur on a private memory access");
 	TEST_ASSERT_EQ(vcpu->run->memory_fault.gpa, EXITS_TEST_GPA);
 	TEST_ASSERT_EQ(vcpu->run->memory_fault.size, EXITS_TEST_SIZE);
 
@@ -104,7 +105,8 @@ static void test_private_access_memslot_not_private(void)
 	exit_reason = run_vcpu_get_exit_reason(vcpu);
 
 	TEST_ASSERT_EQ(exit_reason, KVM_EXIT_MEMORY_FAULT);
-	TEST_ASSERT_EQ(vcpu->run->memory_fault.flags, KVM_MEMORY_EXIT_FLAG_PRIVATE);
+	TEST_ASSERT(vcpu->run->memory_fault.flags & KVM_MEMORY_EXIT_FLAG_PRIVATE,
+		    "Memory fault didn't occur on a private memory access");
 	TEST_ASSERT_EQ(vcpu->run->memory_fault.gpa, EXITS_TEST_GPA);
 	TEST_ASSERT_EQ(vcpu->run->memory_fault.size, EXITS_TEST_SIZE);
 
-- 
2.52.0

[PATCH 02/24] KVM: Define and communicate KVM_EXIT_MEMORY_FAULT RWX flags to userspace

[Paolo Bonzini]

From: Anish Moorthy <amoorthy@google.com>

kvm_prepare_memory_fault_exit() already takes parameters describing the
RWX-ness of the relevant access but doesn't actually do anything with
them. Define and use the flags necessary to pass this information on to
userspace.

Suggested-by: Sean Christopherson <seanjc@google.com>
Link: https://lore.kernel.org/kvm/ZR4N8cwzTMDanPUY@google.com/
Signed-off-by: Anish Moorthy <amoorthy@google.com>
Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Documentation/virt/kvm/api.rst | 5 +++++
 include/linux/kvm_host.h       | 9 ++++++++-
 include/uapi/linux/kvm.h       | 3 +++
 3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a5f9ee92f43e..1be1bc0de5d2 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -7259,6 +7259,9 @@ spec refer, https://github.com/riscv/riscv-sbi-doc.
 
 		/* KVM_EXIT_MEMORY_FAULT */
 		struct {
+  #define KVM_MEMORY_EXIT_FLAG_READ     (1ULL << 0)
+  #define KVM_MEMORY_EXIT_FLAG_WRITE    (1ULL << 1)
+  #define KVM_MEMORY_EXIT_FLAG_EXEC     (1ULL << 2)
   #define KVM_MEMORY_EXIT_FLAG_PRIVATE	(1ULL << 3)
 			__u64 flags;
 			__u64 gpa;
@@ -7270,6 +7273,8 @@ could not be resolved by KVM.  The 'gpa' and 'size' (in bytes) describe the
 guest physical address range [gpa, gpa + size) of the fault.  The 'flags' field
 describes properties of the faulting access that are likely pertinent:
 
+ - KVM_MEMORY_EXIT_FLAG_READ/WRITE/EXEC - When set, indicates that the memory
+   fault occurred on a read/write/exec access respectively.
  - KVM_MEMORY_EXIT_FLAG_PRIVATE - When set, indicates the memory fault occurred
    on a private memory access.  When clear, indicates the fault occurred on a
    shared access.
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index ab8cfaec82d3..2278b17f2f28 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2519,8 +2519,15 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu,
 	vcpu->run->memory_fault.gpa = gpa;
 	vcpu->run->memory_fault.size = size;
 
-	/* RWX flags are not (yet) defined or communicated to userspace. */
 	vcpu->run->memory_fault.flags = 0;
+
+	if (is_write)
+		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_WRITE;
+	else if (is_exec)
+		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_EXEC;
+	else
+		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_READ;
+
 	if (is_private)
 		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_PRIVATE;
 }
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 419011097fa8..720b1ffe880b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -457,6 +457,9 @@ struct kvm_run {
 		} notify;
 		/* KVM_EXIT_MEMORY_FAULT */
 		struct {
+#define KVM_MEMORY_EXIT_FLAG_READ       (1ULL << 0)
+#define KVM_MEMORY_EXIT_FLAG_WRITE      (1ULL << 1)
+#define KVM_MEMORY_EXIT_FLAG_EXEC       (1ULL << 2)
 #define KVM_MEMORY_EXIT_FLAG_PRIVATE	(1ULL << 3)
 			__u64 flags;
 			__u64 gpa;
-- 
2.52.0

[PATCH 03/24] KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/hyperv.c           | 32 ++++++++++++++++++++++++++++++++
 arch/x86/kvm/x86.c              |  7 +++++++
 include/uapi/linux/kvm.h        |  1 +
 4 files changed, 41 insertions(+)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index b517257a6315..d0c1d0ab03c2 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1274,6 +1274,7 @@ struct kvm_hv {
 	struct kvm_hv_syndbg hv_syndbg;
 
 	bool xsaves_xsavec_checked;
+	bool hcall_fault_exit;
 };
 #endif
 
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index 1ee0d23f8949..d688759df081 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -2531,11 +2531,30 @@ static bool hv_check_hypercall_access(struct kvm_vcpu_hv *hv_vcpu, u16 code)
 	return true;
 }
 
+static unsigned int kvm_hv_hypercall_mem_access(u16 code)
+{
+	switch (code) {
+	case HVCALL_SIGNAL_EVENT:
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
+	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
+	case HVCALL_SEND_IPI:
+	case HVCALL_SEND_IPI_EX:
+		return KVM_MEMORY_EXIT_FLAG_READ;
+	}
+
+	return KVM_MEMORY_EXIT_FLAG_WRITE;
+}
+
 int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 {
 	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
+	struct kvm *kvm = vcpu->kvm;
 	struct kvm_hv_hcall hc;
 	u64 ret = HV_STATUS_SUCCESS;
+	unsigned int access;
+	unsigned long addr;
 
 	/*
 	 * hypercall generates UD from non zero cpl and real mode
@@ -2590,6 +2609,19 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
 		kvm_hv_hypercall_read_xmm(&hc);
 	}
 
+	if (!hc.fast && kvm->arch.hyperv.hcall_fault_exit) {
+		bool writable = true;
+		access = kvm_hv_hypercall_mem_access(hc.code);
+		addr = kvm_vcpu_gfn_to_hva_prot(vcpu, gpa_to_gfn(hc.ingpa), &writable);
+		if (addr == KVM_HVA_ERR_BAD ||
+		    (access == KVM_MEMORY_EXIT_FLAG_WRITE && !writable)) {
+			kvm_prepare_memory_fault_exit(vcpu, hc.ingpa, PAGE_SIZE,
+						      access == KVM_MEMORY_EXIT_FLAG_WRITE,
+						      false, false);
+			return -EFAULT;
+		}
+	}
+
 	switch (hc.code) {
 	case HVCALL_NOTIFY_LONG_SPIN_WAIT:
 		if (unlikely(hc.rep || hc.var_cnt)) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0626e835e9eb..352d289076f6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2224,6 +2224,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_HYPERV_CPUID:
 	case KVM_CAP_HYPERV_ENFORCE_CPUID:
 	case KVM_CAP_SYS_HYPERV_CPUID:
+	case KVM_CAP_HYPERV_HCALL_FAULT_EXIT:
 #endif
 	case KVM_CAP_PCI_SEGMENT:
 	case KVM_CAP_DEBUGREGS:
@@ -4190,6 +4191,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		mutex_unlock(&kvm->lock);
 		break;
 	}
+#ifdef CONFIG_KVM_HYPERV
+	case KVM_CAP_HYPERV_HCALL_FAULT_EXIT:
+		kvm->arch.hyperv.hcall_fault_exit = cap->args[0];
+		r = 0;
+		break;
+#endif
 	default:
 		r = -EINVAL;
 		break;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 720b1ffe880b..3b235e953923 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1000,6 +1000,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_S390_KEYOP 247
 #define KVM_CAP_S390_VSIE_ESAMODE 248
 #define KVM_CAP_S390_HPAGE_2G 249
+#define KVM_CAP_HYPERV_HCALL_FAULT_EXIT 250
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
-- 
2.52.0

[PATCH 04/24] KVM: x86/mmu: intersect writability from __kvm_faultin_pfn with fault->map_writable

[Paolo Bonzini]

fault->map_writable is currently a pure output of __kvm_faultin_pfn(),
which is the only thing that restricts it.  This will no longer hold once
memory protections derived from memory attributes are applied: those
compute their own access permissions that combine with those from
__kvm_faultin_pfn().

Applying them *before* faulting in the pfn lets a fault that violates
the attributes exit to userspace without the cost of gup and/or an
async #PF; but it means that permissions will then be restricted in two
independent steps, first by memory attributes and then by __kvm_faultin_pfn().
Switch fault->map_writable to that model by letting kvm_mmu_faultin_pfn()
only clear bits rather than assign them.

No functional change intended: nothing writes fault->map_writable between
the initializer and __kvm_mmu_faultin_pfn() yet, so the AND is equivalent
to the assignment it replaces.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 6c13da942bfc..cf6a409b76ae 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4603,7 +4603,7 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
 		return r;
 	}
 
-	fault->map_writable = !(fault->slot->flags & KVM_MEM_READONLY);
+	fault->map_writable &= !(fault->slot->flags & KVM_MEM_READONLY);
 	fault->max_level = kvm_max_level_for_order(max_order);
 
 	return RET_PF_CONTINUE;
@@ -4613,13 +4613,14 @@ static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
 				 struct kvm_page_fault *fault)
 {
 	unsigned int foll = fault->write ? FOLL_WRITE : 0;
+	bool writable;
 
 	if (fault->is_private || kvm_memslot_is_gmem_only(fault->slot))
 		return kvm_mmu_faultin_pfn_gmem(vcpu, fault);
 
 	foll |= FOLL_NOWAIT;
 	fault->pfn = __kvm_faultin_pfn(fault->slot, fault->gfn, foll,
-				       &fault->map_writable, &fault->refcounted_page);
+				       &writable, &fault->refcounted_page);
 
 	/*
 	 * If resolving the page failed because I/O is needed to fault-in the
@@ -4628,7 +4629,7 @@ static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
 	 * other failures are terminal, i.e. retrying won't help.
 	 */
 	if (fault->pfn != KVM_PFN_ERR_NEEDS_IO)
-		return RET_PF_CONTINUE;
+		goto out_pf_continue;
 
 	if (!fault->prefetch && kvm_can_do_async_pf(vcpu)) {
 		trace_kvm_try_async_get_page(fault->addr, fault->gfn);
@@ -4649,8 +4650,10 @@ static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
 	foll |= FOLL_INTERRUPTIBLE;
 	foll &= ~FOLL_NOWAIT;
 	fault->pfn = __kvm_faultin_pfn(fault->slot, fault->gfn, foll,
-				       &fault->map_writable, &fault->refcounted_page);
+				       &writable, &fault->refcounted_page);
 
+out_pf_continue:
+	fault->map_writable &= writable;
 	return RET_PF_CONTINUE;
 }
 
@@ -4968,6 +4971,7 @@ static int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 		.is_private = err & PFERR_PRIVATE_ACCESS,
 
 		.pfn = KVM_PFN_ERR_FAULT,
+		.map_writable = true,
 	};
 	int r;
 
-- 
2.52.0

[PATCH 05/24] KVM: x86/mmu: Extend map_writable to a full ACC_* mask

[Paolo Bonzini]

Support for memory protection attributes opens the door to installing
non-executable mappings. Instead of introducing yet another member in
struct kvm_page_fault and another argument to make_spte(), make the
existing member map_writable a mask of ACC_* bits.  This also avoids the
need for make_spte() to map a single bool to either the NX bit or
the XS/XU bits together.

Unlike for mappings that are not writable because the fault did
not request write premission, it is not not necessary to track
executability for these SPTEs; the gfn is always available and it will
be possible to access the attributes directly in FNAME(sync_spte).

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c          | 21 ++++++++++++---------
 arch/x86/kvm/mmu/mmu_internal.h |  2 +-
 arch/x86/kvm/mmu/paging_tmpl.h  |  8 +++++---
 arch/x86/kvm/mmu/spte.c         | 12 ++++++------
 arch/x86/kvm/mmu/spte.h         |  2 +-
 arch/x86/kvm/mmu/tdp_mmu.c      |  2 +-
 6 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index cf6a409b76ae..424f4e113682 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3074,7 +3074,7 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,
 	u64 spte;
 
 	/* Prefetching always gets a writable pfn.  */
-	bool host_writable = !fault || fault->map_writable;
+	unsigned host_access = fault ? fault->host_access : ACC_ALL;
 	bool prefetch = !fault || fault->prefetch;
 	bool write_fault = fault && fault->write;
 
@@ -3111,7 +3111,7 @@ static int mmu_set_spte(struct kvm_vcpu *vcpu, struct kvm_memory_slot *slot,
 	}
 
 	wrprot = make_spte(vcpu, sp, slot, pte_access, gfn, pfn, *sptep, prefetch,
-			   false, host_writable, &spte);
+			   false, host_access, &spte);
 
 	if (*sptep == spte) {
 		ret = RET_PF_SPURIOUS;
@@ -3558,7 +3558,7 @@ static int kvm_handle_noslot_fault(struct kvm_vcpu *vcpu,
 
 	fault->slot = NULL;
 	fault->pfn = KVM_PFN_NOSLOT;
-	fault->map_writable = false;
+	fault->host_access = 0;
 
 	/*
 	 * If MMIO caching is disabled, emulate immediately without
@@ -4583,7 +4583,8 @@ static void kvm_mmu_finish_page_fault(struct kvm_vcpu *vcpu,
 				      struct kvm_page_fault *fault, int r)
 {
 	kvm_release_faultin_page(vcpu->kvm, fault->refcounted_page,
-				 r == RET_PF_RETRY, fault->map_writable);
+				 r == RET_PF_RETRY,
+				 !!(fault->host_access & ACC_WRITE_MASK));
 }
 
 static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
@@ -4603,9 +4604,10 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
 		return r;
 	}
 
-	fault->map_writable &= !(fault->slot->flags & KVM_MEM_READONLY);
-	fault->max_level = kvm_max_level_for_order(max_order);
+	if (fault->slot->flags & KVM_MEM_READONLY)
+		fault->host_access &= ~ACC_WRITE_MASK;
 
+	fault->max_level = kvm_max_level_for_order(max_order);
 	return RET_PF_CONTINUE;
 }
 
@@ -4653,7 +4655,8 @@ static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
 				       &writable, &fault->refcounted_page);
 
 out_pf_continue:
-	fault->map_writable &= writable;
+	if (!writable)
+		fault->host_access &= ~ACC_WRITE_MASK;
 	return RET_PF_CONTINUE;
 }
 
@@ -4971,7 +4974,7 @@ static int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
 		.is_private = err & PFERR_PRIVATE_ACCESS,
 
 		.pfn = KVM_PFN_ERR_FAULT,
-		.map_writable = true,
+		.host_access = ACC_ALL,
 	};
 	int r;
 
@@ -5165,7 +5168,7 @@ int kvm_tdp_mmu_map_private_pfn(struct kvm_vcpu *vcpu, gfn_t gfn, kvm_pfn_t pfn)
 		.gfn = gfn,
 		.slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn),
 		.pfn = pfn,
-		.map_writable = true,
+		.host_access = ACC_ALL,
 	};
 	struct kvm *kvm = vcpu->kvm;
 	int r;
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index c29002c60126..00215b9f309f 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -280,7 +280,7 @@ struct kvm_page_fault {
 	unsigned long mmu_seq;
 	kvm_pfn_t pfn;
 	struct page *refcounted_page;
-	bool map_writable;
+	u8 host_access;
 
 	/*
 	 * Indicates the guest is trying to write a gfn that contains one or
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index e73fc09ec4db..1871d334fed7 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -933,7 +933,7 @@ static gpa_t FNAME(gva_to_gpa)(struct kvm_vcpu *vcpu, struct kvm_pagewalk *w,
  */
 static int FNAME(sync_spte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, int i)
 {
-	bool host_writable;
+	u8 host_access;
 	gpa_t first_pte_gpa;
 	u64 *sptep, spte;
 	struct kvm_memory_slot *slot;
@@ -990,11 +990,13 @@ static int FNAME(sync_spte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, int
 
 	sptep = &sp->spt[i];
 	spte = *sptep;
-	host_writable = spte & shadow_host_writable_mask;
+	host_access = ACC_ALL;
+	if (!(spte & shadow_host_writable_mask))
+		host_access &= ~ACC_WRITE_MASK;
 	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
 	make_spte(vcpu, sp, slot, pte_access, gfn,
 		  spte_to_pfn(spte), spte, true, true,
-		  host_writable, &spte);
+		  host_access, &spte);
 
 	/*
 	 * There is no need to mark the pfn dirty, as the new protections must
diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
index bdf72a98c19c..bc6d32c4c854 100644
--- a/arch/x86/kvm/mmu/spte.c
+++ b/arch/x86/kvm/mmu/spte.c
@@ -188,7 +188,7 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	       const struct kvm_memory_slot *slot,
 	       unsigned int pte_access, gfn_t gfn, kvm_pfn_t pfn,
 	       u64 old_spte, bool prefetch, bool synchronizing,
-	       bool host_writable, u64 *new_spte)
+	       unsigned int host_access, u64 *new_spte)
 {
 	int level = sp->role.level;
 	u64 spte = SPTE_MMU_PRESENT_MASK;
@@ -206,6 +206,11 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	if (!prefetch || synchronizing)
 		spte |= shadow_accessed_mask;
 
+	if (host_access & ACC_WRITE_MASK)
+		spte |= shadow_host_writable_mask;
+
+	pte_access &= host_access;
+
 	/*
 	 * For simplicity, enforce the NX huge page mitigation even if not
 	 * strictly necessary.  KVM could ignore the mitigation if paging is
@@ -245,11 +250,6 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	if (kvm_x86_ops.get_mt_mask)
 		spte |= kvm_x86_call(get_mt_mask)(vcpu, gfn,
 						  kvm_is_mmio_pfn(pfn, &is_host_mmio));
-	if (host_writable)
-		spte |= shadow_host_writable_mask;
-	else
-		pte_access &= ~ACC_WRITE_MASK;
-
 	if (shadow_me_value && !kvm_is_mmio_pfn(pfn, &is_host_mmio))
 		spte |= shadow_me_value;
 
diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
index e730717824b3..589f3954633e 100644
--- a/arch/x86/kvm/mmu/spte.h
+++ b/arch/x86/kvm/mmu/spte.h
@@ -563,7 +563,7 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
 	       const struct kvm_memory_slot *slot,
 	       unsigned int pte_access, gfn_t gfn, kvm_pfn_t pfn,
 	       u64 old_spte, bool prefetch, bool synchronizing,
-	       bool host_writable, u64 *new_spte);
+	       unsigned int host_access, u64 *new_spte);
 u64 make_small_spte(struct kvm *kvm, u64 huge_spte,
 		    union kvm_mmu_page_role role, int index);
 u64 make_huge_spte(struct kvm *kvm, u64 small_spte, int level);
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index ce3f2efadb05..4d9e7b6868e2 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -1143,7 +1143,7 @@ static int tdp_mmu_map_handle_target_level(struct kvm_vcpu *vcpu,
 	else
 		wrprot = make_spte(vcpu, sp, fault->slot, sp->role.access, iter->gfn,
 				   fault->pfn, iter->old_spte, fault->prefetch,
-				   false, fault->map_writable, &new_spte);
+				   false, fault->host_access, &new_spte);
 
 	if (new_spte == iter->old_spte)
 		ret = RET_PF_SPURIOUS;
-- 
2.52.0

[PATCH 06/24] KVM: x86: Avoid warning when installing non-private memory attributes

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

In preparation to introducing RWX memory attributes. Make sure
user-space is attempting to install a memory attribute with
KVM_MEMORY_ATTRIBUTE_PRIVATE before throwing a warning on systems with
no private memory support.

The WARN is really a duplicate of the kvm_supported_mem_attributes()
test in kvm_vm_ioctl_set_mem_attributes(), but then all of them should
be redundant...  Keep it as a special check for the private-memory
attribute, to avoid that kvm_mmu_page_fault() sets PFERR_PRIVATE_ACCESS;
that bit would send KVM down kvm_mmu_faultin_pfn_gmem(), which is such
a wrong path that it's worth catching it early.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 424f4e113682..055e0b45a8ee 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -8078,9 +8078,15 @@ static void hugepage_set_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
 bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
 					struct kvm_gfn_range *range)
 {
+	unsigned long attrs = range->arg.attributes;
 	struct kvm_memory_slot *slot = range->slot;
 	int level;
 
+	if (!kvm_arch_has_private_mem(kvm)) {
+		WARN_ON(attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE);
+		attrs &= ~KVM_MEMORY_ATTRIBUTE_PRIVATE;
+	}
+
 	/*
 	 * Zap SPTEs even if the slot can't be mapped PRIVATE.  KVM x86 only
 	 * supports KVM_MEMORY_ATTRIBUTE_PRIVATE, and so it *seems* like KVM
@@ -8092,9 +8098,6 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
 	 * Zapping SPTEs in this case ensures KVM will reassess whether or not
 	 * a hugepage can be used for affected ranges.
 	 */
-	if (WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm)))
-		return false;
-
 	if (WARN_ON_ONCE(range->end <= range->start))
 		return false;
 
@@ -8165,16 +8168,17 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
 	lockdep_assert_held_write(&kvm->mmu_lock);
 	lockdep_assert_held(&kvm->slots_lock);
 
+	if (!kvm_arch_has_private_mem(kvm)) {
+		WARN_ON(attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE);
+		attrs &= ~KVM_MEMORY_ATTRIBUTE_PRIVATE;
+	}
+
 	/*
 	 * Calculate which ranges can be mapped with hugepages even if the slot
 	 * can't map memory PRIVATE.  KVM mustn't create a SHARED hugepage over
 	 * a range that has PRIVATE GFNs, and conversely converting a range to
 	 * SHARED may now allow hugepages.
-	 */
-	if (WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm)))
-		return false;
-
-	/*
+	 *
 	 * The sequence matters here: upper levels consume the result of lower
 	 * level's scanning.
 	 */
-- 
2.52.0

[PATCH 07/24] KVM: x86/mmu: Init memslot hugepage information for non-private_mem VMs too

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

The list of supported memory attributes is about to grow, and they will
be available on any VM type (as opposed to only ones targeted at
confidential computing). As such, update the check in
kvm_mmu_init_memslot_memory_attributes() to initialize huge page
information if any kind of memory attributes is available for the VM.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c   | 2 +-
 include/linux/kvm_host.h | 5 +++++
 virt/kvm/kvm_main.c      | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 055e0b45a8ee..80f581afb6a0 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -8231,7 +8231,7 @@ void kvm_mmu_init_memslot_memory_attributes(struct kvm *kvm,
 {
 	int level;
 
-	if (!kvm_arch_has_private_mem(kvm))
+	if (!kvm_supported_mem_attributes(kvm))
 		return;
 
 	for (level = PG_LEVEL_2M; level <= KVM_MAX_HUGEPAGE_LEVEL; level++) {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2278b17f2f28..4731e8a9e6db 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2552,12 +2552,17 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
 					struct kvm_gfn_range *range);
 bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
 					 struct kvm_gfn_range *range);
+u64 kvm_supported_mem_attributes(struct kvm *kvm);
 
 static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
 {
 	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
 }
 #else
+static inline u64 kvm_supported_mem_attributes(struct kvm *kvm)
+{
+	return 0;
+}
 static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
 {
 	return false;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index e44c20c04961..4ecc3579d163 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2419,7 +2419,7 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
 #endif /* CONFIG_KVM_GENERIC_DIRTYLOG_READ_PROTECT */
 
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
-static u64 kvm_supported_mem_attributes(struct kvm *kvm)
+u64 kvm_supported_mem_attributes(struct kvm *kvm)
 {
 	if (!kvm || kvm_arch_has_private_mem(kvm))
 		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
-- 
2.52.0

[PATCH 08/24] KVM: pass kvm == NULL case to kvm_arch_has_private_mem

[Paolo Bonzini]

Allow the architecture-specific code to enable CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
without exposing KVM_MEMORY_ATTRIBUTE_PRIVATE.  This is mostly for
consistency after introducing memory protection attributes; an
architecture like Arm might want to add support for
KVM_MEMORY_ATTRIBUTE_PRIVATE without having the protection
attributes show up in KVM_CHECK_EXTENSION.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/include/asm/kvm_host.h | 2 +-
 virt/kvm/kvm_main.c             | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index d0c1d0ab03c2..5d329fbb8acb 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2005,7 +2005,7 @@ enum kvm_intr_type {
 	 (!!in_nmi() == ((vcpu)->arch.handling_intr_from_guest == KVM_HANDLING_NMI)))
 
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
-#define kvm_arch_has_private_mem(kvm) ((kvm)->arch.has_private_mem)
+#define kvm_arch_has_private_mem(kvm) (!(kvm) || (kvm)->arch.has_private_mem)
 #endif
 
 #define kvm_arch_has_readonly_mem(kvm) (!(kvm)->arch.has_protected_state)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 4ecc3579d163..fa4473d7c920 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2421,7 +2421,7 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 u64 kvm_supported_mem_attributes(struct kvm *kvm)
 {
-	if (!kvm || kvm_arch_has_private_mem(kvm))
+	if (kvm_arch_has_private_mem(kvm))
 		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
 
 	return 0;
-- 
2.52.0

[PATCH 09/24] KVM: Introduce NR/NW/NX memory attributes

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Introduce memory attributes to map guest physical memory regions as
non-readable, non-writable, and/or non-executable. Only a subset of flag
combinations is supported. Notably write and exec permissions require
read permission, and memory protection attributes are incompatible with
private memory.

As mentioned in 5a475554db1e ("KVM: Introduce per-page memory attributes",
2023-11-13), bits 0-2 of the memory attributes were reserved for RWX
protection; they are negated to support current memory attribute users
which use 0 to indicate no special treatment.  Since 0 is not available,
a non-negated version of the flags would need an extra bit to express
no-access (R=0/W=0/X=0) mappings.

Unfortunately this precaution did not age too well; KVM now supports
MBEC/GMET and adding mode-based memory protections will require a
non-contiguous bit.  But that's something left for later.

Different architectures may have different limitations on the set of
valid protections, for example execution-only and XU=0 mappings are
supported by Intel but not AMD processors[1].  So, add an architecture-specific
callback and add a basic implementation for x86.

[1] When adding support for MBEC/GMET, since NX would remain to mean no
    execution at all, it is possible to use either an NXS bit or two
    separate NXS/NXU bits in addition to NX.  The former would only
    support permissions that are available with either MBEC or GMET.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Documentation/virt/kvm/api.rst  | 14 +++++++---
 arch/x86/include/asm/kvm_host.h |  1 +
 arch/x86/kvm/Kconfig            |  4 +--
 arch/x86/kvm/mmu/mmu.c          | 47 +++++++++++++++++++++++++--------
 arch/x86/kvm/x86.c              |  2 --
 include/linux/kvm_host.h        | 28 +++++++++++++++++++-
 include/uapi/linux/kvm.h        |  3 +++
 virt/kvm/kvm_main.c             | 32 +++++++++++++++++++---
 8 files changed, 107 insertions(+), 24 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 1be1bc0de5d2..d11142c5eca1 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6394,15 +6394,23 @@ of guest physical memory.
 	__u64 flags;
   };
 
+  #define KVM_MEMORY_ATTRIBUTE_NR                (1ULL << 0)
+  #define KVM_MEMORY_ATTRIBUTE_NW                (1ULL << 1)
+  #define KVM_MEMORY_ATTRIBUTE_NX                (1ULL << 2)
   #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
 
 The address and size must be page aligned.  The supported attributes can be
 retrieved via ioctl(KVM_CHECK_EXTENSION) on KVM_CAP_MEMORY_ATTRIBUTES.  If
 executed on a VM, KVM_CAP_MEMORY_ATTRIBUTES precisely returns the attributes
 supported by that VM.  If executed at system scope, KVM_CAP_MEMORY_ATTRIBUTES
-returns all attributes supported by KVM.  The only attribute defined at this
-time is KVM_MEMORY_ATTRIBUTE_PRIVATE, which marks the associated gfn as being
-guest private memory.
+returns all attributes supported by KVM.  The attribute defined at this
+time are:
+
+ - KVM_MEMORY_ATTRIBUTE_NR/NW/NX - Respectively marks the memory region as
+   non-read, non-write and/or non-exec.  Note that write-only, exec-only and
+   write-exec mappings are not supported.
+ - KVM_MEMORY_ATTRIBUTE_PRIVATE - Which marks the associated gfn as being guest
+   private memory.
 
 Note, there is no "get" API.  Userspace is responsible for explicitly tracking
 the state of a gfn/page as needed.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 5d329fbb8acb..268e98a36643 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -2005,6 +2005,7 @@ enum kvm_intr_type {
 	 (!!in_nmi() == ((vcpu)->arch.handling_intr_from_guest == KVM_HANDLING_NMI)))
 
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#define kvm_arch_has_memory_protection_attributes(kvm) (!(kvm) || !(kvm)->arch.has_private_mem)
 #define kvm_arch_has_private_mem(kvm) (!(kvm) || (kvm)->arch.has_private_mem)
 #endif
 
diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 801bf9e520db..d9c5bd71e1c5 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -48,6 +48,7 @@ config KVM_X86
 	select KVM_GENERIC_PRE_FAULT_MEMORY
 	select KVM_WERROR if WERROR
 	select KVM_GUEST_MEMFD if X86_64
+	select KVM_GENERIC_MEMORY_ATTRIBUTES
 
 config KVM
 	tristate "Kernel-based Virtual Machine (KVM) support"
@@ -84,7 +85,6 @@ config KVM_SW_PROTECTED_VM
 	bool "Enable support for KVM software-protected VMs"
 	depends on EXPERT
 	depends on KVM_X86 && X86_64
-	select KVM_GENERIC_MEMORY_ATTRIBUTES
 	help
 	  Enable support for KVM software-protected VMs.  Currently, software-
 	  protected VMs are purely a development and testing vehicle for
@@ -135,7 +135,6 @@ config KVM_INTEL_TDX
 	bool "Intel Trust Domain Extensions (TDX) support"
 	default y
 	depends on INTEL_TDX_HOST
-	select KVM_GENERIC_MEMORY_ATTRIBUTES
 	select HAVE_KVM_ARCH_GMEM_POPULATE
 	help
 	  Provides support for launching Intel Trust Domain Extensions (TDX)
@@ -159,7 +158,6 @@ config KVM_AMD_SEV
 	depends on KVM_AMD && X86_64
 	depends on CRYPTO_DEV_SP_PSP && !(KVM_AMD=y && CRYPTO_DEV_CCP_DD=m)
 	select ARCH_HAS_CC_PLATFORM
-	select KVM_GENERIC_MEMORY_ATTRIBUTES
 	select HAVE_KVM_ARCH_GMEM_PREPARE
 	select HAVE_KVM_ARCH_GMEM_INVALIDATE
 	select HAVE_KVM_ARCH_GMEM_POPULATE
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 80f581afb6a0..b6463b0b0b6d 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -8056,7 +8056,6 @@ void kvm_mmu_pre_destroy_vm(struct kvm *kvm)
 		vhost_task_stop(kvm->arch.nx_huge_page_recovery_thread);
 }
 
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 static bool hugepage_test_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
 				int level)
 {
@@ -8088,16 +8087,23 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
 	}
 
 	/*
-	 * Zap SPTEs even if the slot can't be mapped PRIVATE.  KVM x86 only
-	 * supports KVM_MEMORY_ATTRIBUTE_PRIVATE, and so it *seems* like KVM
-	 * can simply ignore such slots.  But if userspace is making memory
-	 * PRIVATE, then KVM must prevent the guest from accessing the memory
-	 * as shared.  And if userspace is making memory SHARED and this point
-	 * is reached, then at least one page within the range was previously
-	 * PRIVATE, i.e. the slot's possible hugepage ranges are changing.
-	 * Zapping SPTEs in this case ensures KVM will reassess whether or not
-	 * a hugepage can be used for affected ranges.
+	 * For KVM_MEMORY_ATTRIBUTE_PRIVATE:
+	 *  Zap SPTEs even if the slot can't be mapped PRIVATE.  KVM x86 only
+	 *  supports KVM_MEMORY_ATTRIBUTE_PRIVATE, and so it *seems* like KVM
+	 *  can simply ignore such slots.  But if userspace is making memory
+	 *  PRIVATE, then KVM must prevent the guest from accessing the memory
+	 *  as shared.  And if userspace is making memory SHARED and this point
+	 *  is reached, then at least one page within the range was previously
+	 *  PRIVATE, i.e. the slot's possible hugepage ranges are changing.
+	 *  Zapping SPTEs in this case ensures KVM will reassess whether or not
+	 *  a hugepage can be used for affected ranges.
+	 *
+	 * For KVM_MEMORY_ATTRIBUTE_NR/NW/NX:
+	 *  Zap even when loosening restrictions R=>RW, which is nost strictly
+	 *  necessary, but will allow KVM to reasses whether a hugepage can be
+	 *  used for the affected pages.
 	 */
+
 	if (WARN_ON_ONCE(range->end <= range->start))
 		return false;
 
@@ -8262,4 +8268,23 @@ void kvm_mmu_init_memslot_memory_attributes(struct kvm *kvm,
 		}
 	}
 }
-#endif
+
+/* The bits are flipped but remain in the same position.  */
+#define KVM_PROT_READ  KVM_MEMORY_ATTRIBUTE_NR
+#define KVM_PROT_WRITE KVM_MEMORY_ATTRIBUTE_NW
+#define KVM_PROT_EXEC  KVM_MEMORY_ATTRIBUTE_NX
+
+bool kvm_arch_mem_attributes_supported_prot(struct kvm *kvm, unsigned long attrs)
+{
+	unsigned long prot = (attrs & KVM_MEMORY_ATTRIBUTE_PROT) ^ KVM_MEMORY_ATTRIBUTE_PROT;
+
+        /* Private memory and access permissions are incompatible */
+	if (attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE)
+		return false;
+
+	/* For now do now support exec-only, even though EPT can handle it.  */
+	if (prot && !(prot & KVM_PROT_READ))
+		return false;
+
+	return true;
+}
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 352d289076f6..e2240f817c22 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10084,9 +10084,7 @@ static int kvm_alloc_memslot_metadata(struct kvm *kvm,
 		}
 	}
 
-#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 	kvm_mmu_init_memslot_memory_attributes(kvm, slot);
-#endif
 
 	if (kvm_page_track_create_memslot(kvm, slot, npages))
 		goto out_free;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 4731e8a9e6db..2dd1b65799e4 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -722,7 +722,9 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
 }
 #endif
 
-#ifndef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+bool kvm_arch_mem_attributes_supported_prot(struct kvm *kvm, unsigned long attrs);
+#else
 static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
 {
 	return false;
@@ -2540,7 +2542,25 @@ static inline bool kvm_memslot_is_gmem_only(const struct kvm_memory_slot *slot)
 	return slot->flags & KVM_MEMSLOT_GMEM_ONLY;
 }
 
+static inline bool kvm_mem_attributes_may_read(u64 attrs)
+{
+	return !(attrs & KVM_MEMORY_ATTRIBUTE_NR);
+}
+
+static inline bool kvm_mem_attributes_may_write(u64 attrs)
+{
+	return !(attrs & KVM_MEMORY_ATTRIBUTE_NW);
+}
+
+static inline bool kvm_mem_attributes_may_exec(u64 attrs)
+{
+	return !(attrs & KVM_MEMORY_ATTRIBUTE_NX);
+}
+
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
+#define KVM_MEMORY_ATTRIBUTE_PROT \
+	(KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW | KVM_MEMORY_ATTRIBUTE_NX)
+
 static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
 {
 	return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));
@@ -2552,6 +2572,7 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
 					struct kvm_gfn_range *range);
 bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
 					 struct kvm_gfn_range *range);
+bool kvm_mem_attributes_valid(struct kvm *kvm, unsigned long attrs);
 u64 kvm_supported_mem_attributes(struct kvm *kvm);
 
 static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
@@ -2559,6 +2580,11 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
 	return kvm_get_memory_attributes(kvm, gfn) & KVM_MEMORY_ATTRIBUTE_PRIVATE;
 }
 #else
+static inline bool kvm_mem_attributes_valid(struct kvm *kvm,
+					       unsigned long attrs)
+{
+	return false;
+}
 static inline u64 kvm_supported_mem_attributes(struct kvm *kvm)
 {
 	return 0;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 3b235e953923..6a96eb3218dc 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -1653,6 +1653,9 @@ struct kvm_memory_attributes {
 	__u64 flags;
 };
 
+#define KVM_MEMORY_ATTRIBUTE_NR		       (1ULL << 0)
+#define KVM_MEMORY_ATTRIBUTE_NW		       (1ULL << 1)
+#define KVM_MEMORY_ATTRIBUTE_NX		       (1ULL << 2)
 #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
 
 #define KVM_CREATE_GUEST_MEMFD	_IOWR(KVMIO,  0xd4, struct kvm_create_guest_memfd)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index fa4473d7c920..a47b62c2c9ce 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2421,10 +2421,15 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 u64 kvm_supported_mem_attributes(struct kvm *kvm)
 {
-	if (kvm_arch_has_private_mem(kvm))
-		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
+	u64 supported_attrs = 0;
 
-	return 0;
+	if (kvm_arch_has_memory_protection_attributes(kvm))
+		supported_attrs |= KVM_MEMORY_ATTRIBUTE_PROT;
+
+	if (kvm_arch_has_private_mem(kvm))
+		supported_attrs |= KVM_MEMORY_ATTRIBUTE_PRIVATE;
+
+	return supported_attrs;
 }
 
 /*
@@ -2596,6 +2601,25 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 
 	return r;
 }
+
+bool __weak kvm_arch_mem_attributes_supported_prot(struct kvm *kvm, unsigned long attrs)
+{
+	WARN_ON_ONCE("KVM_MEMORY_ATTRIBUTE_PROT requires kvm_arch_mem_attributes_supported_prot()");
+	return false;
+}
+
+bool kvm_mem_attributes_valid(struct kvm *kvm, unsigned long attrs)
+{
+	if (attrs & ~kvm_supported_mem_attributes(kvm))
+		return false;
+
+	if ((attrs & KVM_MEMORY_ATTRIBUTE_PROT) &&
+	    !kvm_arch_mem_attributes_supported_prot(kvm, attrs))
+		return false;
+
+	return true;
+}
+
 static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
 					   struct kvm_memory_attributes *attrs)
 {
@@ -2604,7 +2628,7 @@ static int kvm_vm_ioctl_set_mem_attributes(struct kvm *kvm,
 	/* flags is currently not used. */
 	if (attrs->flags)
 		return -EINVAL;
-	if (attrs->attributes & ~kvm_supported_mem_attributes(kvm))
+	if (!kvm_mem_attributes_valid(kvm, attrs->attributes))
 		return -EINVAL;
 	if (attrs->size == 0 || attrs->address + attrs->size < attrs->address)
 		return -EINVAL;
-- 
2.52.0

[PATCH 10/24] KVM: Include memory protections in result of gfn->hva conversion

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

All paths that lead to guest memory accesses now need to check whether
memory attributes allow that access.  Users of gfn_to_hva,
kvm_vcpu_gfn_to_hva and their *_prot variant can get it more or
less for free via erroneous return values, so do this first.

Note however that this is not true of the variants that take a cached
kvm_memslots pointer.  These include caches (gfn-to-hva and gfn-to-pfn)
and page faults, both of which will need specific changes; but the more
optimized functions in virt/kvm/kvm_main.c such as kvm_read_guest()
and kvm_write_guest() also retrieve the memslot high in the call chain,
and therefore they will need changes in __kvm_read/write_guest_page().

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/linux/kvm_host.h | 18 +++++++++++++
 virt/kvm/kvm_main.c      | 55 +++++++++++++++++++++++++++++++++++++---
 2 files changed, 69 insertions(+), 4 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 2dd1b65799e4..f10ab70b99dc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2593,8 +2593,26 @@ static inline bool kvm_mem_is_private(struct kvm *kvm, gfn_t gfn)
 {
 	return false;
 }
+static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
+{
+	return 0;
+}
 #endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
 
+static inline int kvm_mem_attributes_may_read_gfn(struct kvm *kvm, gfn_t gfn)
+{
+	unsigned long attrs = kvm_get_memory_attributes(kvm, gfn);
+
+	return kvm_mem_attributes_may_read(attrs);
+}
+
+static inline int kvm_mem_attributes_may_write_gfn(struct kvm *kvm, gfn_t gfn)
+{
+	unsigned long attrs = kvm_get_memory_attributes(kvm, gfn);
+
+	return kvm_mem_attributes_may_write(attrs);
+}
+
 #ifdef CONFIG_KVM_GUEST_MEMFD
 int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
 		     gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index a47b62c2c9ce..56016aab0aad 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2764,13 +2764,38 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(gfn_to_hva_memslot);
 
 unsigned long gfn_to_hva(struct kvm *kvm, gfn_t gfn)
 {
-	return gfn_to_hva_many(gfn_to_memslot(kvm, gfn), gfn, NULL);
+	unsigned long addr;
+
+	addr = gfn_to_hva_many(gfn_to_memslot(kvm, gfn), gfn, NULL);
+	if (kvm_is_error_hva(addr))
+		return addr;
+
+	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
+		return KVM_HVA_ERR_BAD;
+
+	if (!kvm_mem_attributes_may_write_gfn(kvm, gfn))
+		return KVM_HVA_ERR_RO_BAD;
+
+	return addr;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(gfn_to_hva);
 
 unsigned long kvm_vcpu_gfn_to_hva(struct kvm_vcpu *vcpu, gfn_t gfn)
 {
-	return gfn_to_hva_many(kvm_vcpu_gfn_to_memslot(vcpu, gfn), gfn, NULL);
+	struct kvm *kvm = vcpu->kvm;
+	unsigned long addr;
+
+	addr = gfn_to_hva_many(kvm_vcpu_gfn_to_memslot(vcpu, gfn), gfn, NULL);
+	if (kvm_is_error_hva(addr))
+		return addr;
+
+	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
+		return KVM_HVA_ERR_BAD;
+
+	if (!kvm_mem_attributes_may_write_gfn(kvm, gfn))
+		return KVM_HVA_ERR_RO_BAD;
+
+	return addr;
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_gfn_to_hva);
 
@@ -2796,15 +2821,37 @@ unsigned long gfn_to_hva_memslot_prot(struct kvm_memory_slot *slot,
 unsigned long gfn_to_hva_prot(struct kvm *kvm, gfn_t gfn, bool *writable)
 {
 	struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
+	unsigned long addr;
 
-	return gfn_to_hva_memslot_prot(slot, gfn, writable);
+	addr = gfn_to_hva_memslot_prot(slot, gfn, writable);
+	if (kvm_is_error_hva(addr))
+		return addr;
+
+	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
+		return KVM_HVA_ERR_BAD;
+
+	if (!kvm_mem_attributes_may_write_gfn(kvm, gfn))
+		*writable = false;
+
+	return addr;
 }
 
 unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu *vcpu, gfn_t gfn, bool *writable)
 {
 	struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
+	unsigned long addr;
 
-	return gfn_to_hva_memslot_prot(slot, gfn, writable);
+	addr = gfn_to_hva_memslot_prot(slot, gfn, writable);
+	if (kvm_is_error_hva(addr))
+		return addr;
+
+	if (!kvm_mem_attributes_may_read_gfn(vcpu->kvm, gfn))
+		return KVM_HVA_ERR_BAD;
+
+	if (!kvm_mem_attributes_may_write_gfn(vcpu->kvm, gfn))
+		*writable = false;
+
+	return addr;
 }
 
 static bool kvm_is_ad_tracked_page(struct page *page)
-- 
2.52.0

[PATCH 11/24] KVM: Take memory protections into account in kvm_read/write_guest()

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Take into account memory attributes when accessing guest memory through
the kvm_read/write_guest() family of functions.

All of these pass a struct kvm_memory_slot pointer to the actual
workhorse functions, in order to share code between the VM-wide and
vCPU-specific version of the functions (the latter of which handles the
multi-address-space case).  For this reason they need specific changes
and do not work even though the gfn_to_hva() path has been taught already
about memory protection attributes.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/kvm_main.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 56016aab0aad..d920ce5b6739 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3249,8 +3249,8 @@ static int next_segment(unsigned long len, int offset)
 }
 
 /* Copy @len bytes from guest memory at '(@gfn * PAGE_SIZE) + @offset' to @data */
-static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn,
-				 void *data, int offset, int len)
+static int __kvm_read_guest_page(struct kvm *kvm, struct kvm_memory_slot *slot,
+				 gfn_t gfn, void *data, int offset, int len)
 {
 	int r;
 	unsigned long addr;
@@ -3261,6 +3261,10 @@ static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn,
 	addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
 	if (kvm_is_error_hva(addr))
 		return -EFAULT;
+
+	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
+		return -EFAULT;
+
 	r = __copy_from_user(data, (void __user *)addr + offset, len);
 	if (r)
 		return -EFAULT;
@@ -3272,7 +3276,7 @@ int kvm_read_guest_page(struct kvm *kvm, gfn_t gfn, void *data, int offset,
 {
 	struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
 
-	return __kvm_read_guest_page(slot, gfn, data, offset, len);
+	return __kvm_read_guest_page(kvm, slot, gfn, data, offset, len);
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_read_guest_page);
 
@@ -3281,7 +3285,7 @@ int kvm_vcpu_read_guest_page(struct kvm_vcpu *vcpu, gfn_t gfn, void *data,
 {
 	struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
 
-	return __kvm_read_guest_page(slot, gfn, data, offset, len);
+	return __kvm_read_guest_page(vcpu->kvm, slot, gfn, data, offset, len);
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_read_guest_page);
 
@@ -3325,8 +3329,9 @@ int kvm_vcpu_read_guest(struct kvm_vcpu *vcpu, gpa_t gpa, void *data, unsigned l
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_read_guest);
 
-static int __kvm_read_guest_atomic(struct kvm_memory_slot *slot, gfn_t gfn,
-			           void *data, int offset, unsigned long len)
+static int __kvm_read_guest_atomic(struct kvm *kvm,
+				   struct kvm_memory_slot *slot, gfn_t gfn,
+				   void *data, int offset, unsigned long len)
 {
 	int r;
 	unsigned long addr;
@@ -3334,6 +3339,9 @@ static int __kvm_read_guest_atomic(struct kvm_memory_slot *slot, gfn_t gfn,
 	if (WARN_ON_ONCE(offset + len > PAGE_SIZE))
 		return -EFAULT;
 
+	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
+		return -EFAULT;
+
 	addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
 	if (kvm_is_error_hva(addr))
 		return -EFAULT;
@@ -3352,7 +3360,7 @@ int kvm_vcpu_read_guest_atomic(struct kvm_vcpu *vcpu, gpa_t gpa,
 	struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
 	int offset = offset_in_page(gpa);
 
-	return __kvm_read_guest_atomic(slot, gfn, data, offset, len);
+	return __kvm_read_guest_atomic(vcpu->kvm, slot, gfn, data, offset, len);
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_read_guest_atomic);
 
@@ -3370,6 +3378,10 @@ static int __kvm_write_guest_page(struct kvm *kvm,
 	addr = gfn_to_hva_memslot(memslot, gfn);
 	if (kvm_is_error_hva(addr))
 		return -EFAULT;
+
+	if (!kvm_mem_attributes_may_write_gfn(kvm, gfn))
+		return -EFAULT;
+
 	r = __copy_to_user((void __user *)addr + offset, data, len);
 	if (r)
 		return -EFAULT;
-- 
2.52.0

[PATCH 12/24] KVM: Encapsulate memattrs array into anonymous struct

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

The metadata surrounding memory attributes is about to grow, so
encapsulate the memory attributes array within an anonymous struct to
provide namespacing.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/linux/kvm_host.h |  8 +++++---
 virt/kvm/kvm_main.c      | 10 +++++-----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index f10ab70b99dc..341f2e97f3cb 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -874,8 +874,10 @@ struct kvm {
 	struct notifier_block pm_notifier;
 #endif
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
-	/* Protected by slots_lock (for writes) and RCU (for reads) */
-	struct xarray mem_attr_array;
+	struct {
+		/* Protected by slots_lock (for writes) and RCU (for reads) */
+		struct xarray array;
+	} mem_attrs;
 #endif
 	char stats_id[KVM_STATS_NAME_SIZE];
 };
@@ -2563,7 +2565,7 @@ static inline bool kvm_mem_attributes_may_exec(u64 attrs)
 
 static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
 {
-	return xa_to_value(xa_load(&kvm->mem_attr_array, gfn));
+	return xa_to_value(xa_load(&kvm->mem_attrs.array, gfn));
 }
 
 bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d920ce5b6739..20113069562e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1116,7 +1116,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 	rcuwait_init(&kvm->mn_memslots_update_rcuwait);
 	xa_init(&kvm->vcpu_array);
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
-	xa_init(&kvm->mem_attr_array);
+	xa_init(&kvm->mem_attrs.array);
 #endif
 
 	INIT_LIST_HEAD(&kvm->gpc_list);
@@ -1301,7 +1301,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
 	srcu_barrier(&kvm->srcu);
 	cleanup_srcu_struct(&kvm->srcu);
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
-	xa_destroy(&kvm->mem_attr_array);
+	xa_destroy(&kvm->mem_attrs.array);
 #endif
 	kvm_arch_free_vm(kvm);
 	preempt_notifier_dec();
@@ -2439,7 +2439,7 @@ u64 kvm_supported_mem_attributes(struct kvm *kvm)
 bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 				     unsigned long mask, unsigned long attrs)
 {
-	XA_STATE(xas, &kvm->mem_attr_array, start);
+	XA_STATE(xas, &kvm->mem_attrs.array, start);
 	unsigned long index;
 	void *entry;
 
@@ -2578,7 +2578,7 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 	 * partway through setting the new attributes.
 	 */
 	for (i = start; i < end; i++) {
-		r = xa_reserve(&kvm->mem_attr_array, i, GFP_KERNEL_ACCOUNT);
+		r = xa_reserve(&kvm->mem_attrs.array, i, GFP_KERNEL_ACCOUNT);
 		if (r)
 			goto out_unlock;
 
@@ -2588,7 +2588,7 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 	kvm_handle_gfn_range(kvm, &pre_set_range);
 
 	for (i = start; i < end; i++) {
-		r = xa_err(xa_store(&kvm->mem_attr_array, i, entry,
+		r = xa_err(xa_store(&kvm->mem_attrs.array, i, entry,
 				    GFP_KERNEL_ACCOUNT));
 		KVM_BUG_ON(r, kvm);
 		cond_resched();
-- 
2.52.0

[PATCH 13/24] KVM: Introduce kvm_check_gen()/kvm_memslots_check_gen()

[Paolo Bonzini]

In many cases, retrieving kvm_memslots is followed by a check on
the generation of the slots.  Introduce a helper function that
either does the check alone, or compounds it with returning the
struct kvm_memslots* to the caller.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c       | 10 ++--------
 include/linux/kvm_host.h | 12 ++++++++++++
 virt/kvm/kvm_main.c      |  8 ++++----
 virt/kvm/pfncache.c      | 10 ++++------
 4 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index e2240f817c22..58f544df47e1 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2032,7 +2032,6 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
 {
 	struct gfn_to_hva_cache *ghc = &vcpu->arch.st.cache;
 	struct kvm_steal_time __user *st;
-	struct kvm_memslots *slots;
 	gpa_t gpa = vcpu->arch.st.msr_val & KVM_STEAL_VALID_BITS;
 	u64 steal;
 	u32 version;
@@ -2048,9 +2047,7 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
 	if (WARN_ON_ONCE(current->mm != vcpu->kvm->mm))
 		return;
 
-	slots = kvm_memslots(vcpu->kvm);
-
-	if (unlikely(slots->generation != ghc->generation ||
+	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) ||
 		     gpa != ghc->gpa ||
 		     kvm_is_error_hva(ghc->hva) || !ghc->memslot)) {
 		/* We rely on the fact that it fits in a single page. */
@@ -2601,7 +2598,6 @@ static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
 {
 	struct gfn_to_hva_cache *ghc = &vcpu->arch.st.cache;
 	struct kvm_steal_time __user *st;
-	struct kvm_memslots *slots;
 	static const u8 preempted = KVM_VCPU_PREEMPTED;
 	gpa_t gpa = vcpu->arch.st.msr_val & KVM_STEAL_VALID_BITS;
 
@@ -2628,9 +2624,7 @@ static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
 	if (unlikely(current->mm != vcpu->kvm->mm))
 		return;
 
-	slots = kvm_memslots(vcpu->kvm);
-
-	if (unlikely(slots->generation != ghc->generation ||
+	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) ||
 		     gpa != ghc->gpa ||
 		     kvm_is_error_hva(ghc->hva) || !ghc->memslot))
 		return;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 341f2e97f3cb..122af87d6f9a 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -2559,6 +2559,18 @@ static inline bool kvm_mem_attributes_may_exec(u64 attrs)
 	return !(attrs & KVM_MEMORY_ATTRIBUTE_NX);
 }
 
+static inline bool kvm_memslots_check_gen(struct kvm *kvm, u64 slots_generation, struct kvm_memslots **p_slots)
+{
+	struct kvm_memslots *slots = *p_slots = kvm_memslots(kvm);
+	return slots->generation == slots_generation;
+}
+
+static inline bool kvm_check_gen(struct kvm *kvm, u64 slots_generation)
+{
+	struct kvm_memslots *slots;
+	return kvm_memslots_check_gen(kvm, slots_generation, &slots);
+}
+
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 #define KVM_MEMORY_ATTRIBUTE_PROT \
 	(KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW | KVM_MEMORY_ATTRIBUTE_NX)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 20113069562e..1afaec56ecb3 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3502,14 +3502,14 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 				  void *data, unsigned int offset,
 				  unsigned long len)
 {
-	struct kvm_memslots *slots = kvm_memslots(kvm);
+	struct kvm_memslots *slots;
 	int r;
 	gpa_t gpa = ghc->gpa + offset;
 
 	if (WARN_ON_ONCE(len + offset > ghc->len))
 		return -EINVAL;
 
-	if (slots->generation != ghc->generation) {
+	if (unlikely(!kvm_memslots_check_gen(kvm, ghc->generation, &slots))) {
 		if (__kvm_gfn_to_hva_cache_init(slots, ghc, ghc->gpa, ghc->len))
 			return -EFAULT;
 	}
@@ -3540,14 +3540,14 @@ int kvm_read_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 				 void *data, unsigned int offset,
 				 unsigned long len)
 {
-	struct kvm_memslots *slots = kvm_memslots(kvm);
+	struct kvm_memslots *slots;
 	int r;
 	gpa_t gpa = ghc->gpa + offset;
 
 	if (WARN_ON_ONCE(len + offset > ghc->len))
 		return -EINVAL;
 
-	if (slots->generation != ghc->generation) {
+	if (unlikely(!kvm_memslots_check_gen(kvm, ghc->generation, &slots))) {
 		if (__kvm_gfn_to_hva_cache_init(slots, ghc, ghc->gpa, ghc->len))
 			return -EFAULT;
 	}
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 728d2c1b488a..e09703d249bb 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -72,8 +72,6 @@ static bool kvm_gpc_is_valid_len(gpa_t gpa, unsigned long uhva,
 
 bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
 {
-	struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
-
 	if (!gpc->active)
 		return false;
 
@@ -81,7 +79,7 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
 	 * If the page was cached from a memslot, make sure the memslots have
 	 * not been re-configured.
 	 */
-	if (!kvm_is_error_gpa(gpc->gpa) && gpc->generation != slots->generation)
+	if (!kvm_is_error_gpa(gpc->gpa) && !kvm_check_gen(gpc->kvm, gpc->generation))
 		return false;
 
 	if (kvm_is_error_hva(gpc->uhva))
@@ -290,12 +288,12 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 		if (gpc->uhva != old_uhva)
 			hva_change = true;
 	} else {
-		struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
+		struct kvm_memslots *slots;
 
 		page_offset = offset_in_page(gpa);
 
-		if (gpc->gpa != gpa || gpc->generation != slots->generation ||
-		    kvm_is_error_hva(gpc->uhva)) {
+		if (!kvm_memslots_check_gen(gpc->kvm, gpc->generation, &slots) ||
+		    gpc->gpa != gpa || kvm_is_error_hva(gpc->uhva)) {
 			gfn_t gfn = gpa_to_gfn(gpa);
 
 			gpc->gpa = gpa;
-- 
2.52.0

[PATCH 14/24] KVM: Introduce a generation number for memory attributes

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Introduce a generation number to track memory attribute modifications.
This will allow KVM components to invalidate any assumptions they might
have about guest's physical addresses and their permissions when memory
attributes change.

Like with memory slot updates, it's mandatory for components that access
guest memory based on cached information to do so within a KVM SRCU
read-side critical section, and that they validate the generation number
before accessing memory. This, in combination with the
synchronize_srcu() call within the memory attributes ioctl handler,
ensures the following:
- A memory attribute modification operation only returns after all
  users of outdated GPA data are done running.
- Any component accessing cached data after the memory attribute
  modification returned will see the updated generation number.

Additionally, loads/stores of the generation number have
acquire/release semantics; which ensures all attribute writes are
visible before updating the generation, and loads from attributes happen
after having read the current generation number.

Ultimately, since synchronize_srcu_expedited() is an expensive
operation, only perform it when absolutely necessary. Do so if the
introduced memory attribute is known to require synchronization or if
the attribute being cleared contained a memory attribute that required
synchronization.

There shouldn't be any performance loss for memory attributes that
don't require synchronization (and in general for any VMM that does not
apply memory protections), because the attributes generation will always
remain unchanged.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c         |  4 +-
 include/linux/kvm_host.h   | 33 ++++++++++++++--
 include/linux/kvm_types.h  |  6 ++-
 include/trace/events/kvm.h | 14 +++++--
 virt/kvm/kvm_main.c        | 80 +++++++++++++++++++++++++++++++++-----
 virt/kvm/pfncache.c        | 13 ++++---
 6 files changed, 123 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 58f544df47e1..5229f40d074c 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2047,7 +2047,7 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
 	if (WARN_ON_ONCE(current->mm != vcpu->kvm->mm))
 		return;
 
-	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) ||
+	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->slots_generation, ghc->attrs_generation) ||
 		     gpa != ghc->gpa ||
 		     kvm_is_error_hva(ghc->hva) || !ghc->memslot)) {
 		/* We rely on the fact that it fits in a single page. */
@@ -2624,7 +2624,7 @@ static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
 	if (unlikely(current->mm != vcpu->kvm->mm))
 		return;
 
-	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) ||
+	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->slots_generation, ghc->attrs_generation) ||
 		     gpa != ghc->gpa ||
 		     kvm_is_error_hva(ghc->hva) || !ghc->memslot))
 		return;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 122af87d6f9a..1fad3eb303c1 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -877,6 +877,7 @@ struct kvm {
 	struct {
 		/* Protected by slots_lock (for writes) and RCU (for reads) */
 		struct xarray array;
+		u64 generation;
 	} mem_attrs;
 #endif
 	char stats_id[KVM_STATS_NAME_SIZE];
@@ -2559,29 +2560,49 @@ static inline bool kvm_mem_attributes_may_exec(u64 attrs)
 	return !(attrs & KVM_MEMORY_ATTRIBUTE_NX);
 }
 
-static inline bool kvm_memslots_check_gen(struct kvm *kvm, u64 slots_generation, struct kvm_memslots **p_slots)
+static inline bool kvm_memslots_check_gen(struct kvm *kvm, u64 slots_generation,
+					  u64 attrs_generation, struct kvm_memslots **p_slots)
 {
 	struct kvm_memslots *slots = *p_slots = kvm_memslots(kvm);
-	return slots->generation == slots_generation;
+	return slots->generation == slots_generation && kvm->mem_attrs.generation == attrs_generation;
 }
 
-static inline bool kvm_check_gen(struct kvm *kvm, u64 slots_generation)
+static inline bool kvm_check_gen(struct kvm *kvm, u64 slots_generation,
+				 u64 attrs_generation)
 {
 	struct kvm_memslots *slots;
-	return kvm_memslots_check_gen(kvm, slots_generation, &slots);
+	return kvm_memslots_check_gen(kvm, slots_generation, attrs_generation, &slots);
 }
 
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 #define KVM_MEMORY_ATTRIBUTE_PROT \
 	(KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW | KVM_MEMORY_ATTRIBUTE_NX)
 
+#define KVM_MEMORY_ATTRIBUTE_NEEDS_SYNC_MASK KVM_MEMORY_ATTRIBUTE_PROT
+
 static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
 {
 	return xa_to_value(xa_load(&kvm->mem_attrs.array, gfn));
 }
 
+static inline u64 kvm_mem_attributes_generation(struct kvm *kvm)
+{
+	RCU_LOCKDEP_WARN(!lockdep_is_held(&kvm->slots_lock) &&
+				 !srcu_read_lock_held(&kvm->srcu),
+			 "Suspicious memory attribute generation usage\n");
+
+	/*
+	 * The acquire pairs with the release in kvm_vm_set_mem_attributes().
+	 * Memory attributes should only be queried _after_ storing the
+	 * generation number.
+	 */
+	return smp_load_acquire(&kvm->mem_attrs.generation);
+}
+
 bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 				     unsigned long mask, unsigned long attrs);
+bool kvm_range_has_any_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
+					unsigned long mask);
 bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
 					struct kvm_gfn_range *range);
 bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
@@ -2611,6 +2632,10 @@ static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn
 {
 	return 0;
 }
+static inline u64 kvm_mem_attributes_generation(struct kvm *kvm)
+{
+	return 0;
+}
 #endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
 
 static inline int kvm_mem_attributes_may_read_gfn(struct kvm *kvm, gfn_t gfn)
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index a568d8e6f4e8..7d911220e00d 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -74,7 +74,8 @@ typedef u64            hfn_t;
 typedef hfn_t kvm_pfn_t;
 
 struct gfn_to_hva_cache {
-	u64 generation;
+	u64 slots_generation;
+	u64 attrs_generation;
 	gpa_t gpa;
 	unsigned long hva;
 	unsigned long len;
@@ -82,7 +83,8 @@ struct gfn_to_hva_cache {
 };
 
 struct gfn_to_pfn_cache {
-	u64 generation;
+	u64 slots_generation;
+	u64 attrs_generation;
 	gpa_t gpa;
 	unsigned long uhva;
 	struct kvm_memory_slot *memslot;
diff --git a/include/trace/events/kvm.h b/include/trace/events/kvm.h
index b282e3a86769..a620131e9010 100644
--- a/include/trace/events/kvm.h
+++ b/include/trace/events/kvm.h
@@ -365,23 +365,29 @@ TRACE_EVENT(kvm_dirty_ring_exit,
  * @attr:	The value of the attribute being set.
  */
 TRACE_EVENT(kvm_vm_set_mem_attributes,
-	TP_PROTO(gfn_t start, gfn_t end, unsigned long attr),
-	TP_ARGS(start, end, attr),
+	TP_PROTO(gfn_t start, gfn_t end, unsigned long attr, bool sync, u64 generation),
+	TP_ARGS(start, end, attr, sync, generation),
 
 	TP_STRUCT__entry(
 		__field(gfn_t,		start)
 		__field(gfn_t,		end)
 		__field(unsigned long,	attr)
+		__field(bool,		sync)
+		__field(u64,		generation)
 	),
 
 	TP_fast_assign(
 		__entry->start		= start;
 		__entry->end		= end;
 		__entry->attr		= attr;
+		__entry->sync		= sync;
+		__entry->generation	= generation;
 	),
 
-	TP_printk("%#016llx -- %#016llx [0x%lx]",
-		  __entry->start, __entry->end, __entry->attr)
+	TP_printk("%#016llx -- %#016llx [0x%lx], sync %d gen %llu",
+		  __entry->start, __entry->end, __entry->attr,
+		  __entry->sync, __entry->generation)
+
 );
 #endif /* CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES */
 
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1afaec56ecb3..2fe4087319ad 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1117,6 +1117,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
 	xa_init(&kvm->vcpu_array);
 #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
 	xa_init(&kvm->mem_attrs.array);
+	kvm->mem_attrs.generation = 0;
 #endif
 
 	INIT_LIST_HEAD(&kvm->gpc_list);
@@ -2467,6 +2468,51 @@ bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 	return true;
 }
 
+/*
+ * Returns true if _any_ gfns in the range [@start, @end) have attributes that
+ * match _any_ bit in @mask.
+ */
+bool kvm_range_has_any_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
+					unsigned long mask)
+{
+	XA_STATE(xas, &kvm->mem_attrs.array, start);
+	void *entry;
+
+	mask &= kvm_supported_mem_attributes(kvm);
+	if (!mask)
+		return false;
+
+	if (end == start + 1)
+		return !!(kvm_get_memory_attributes(kvm, start) & mask);
+
+	guard(rcu)();
+	for (;;) {
+		do {
+			entry = xas_next(&xas);
+		} while (xas_retry(&xas, entry));
+
+		if (xas.xa_index >= end)
+			break;
+
+		if (xa_to_value(entry) & mask)
+			return true;
+	}
+
+	return false;
+}
+
+static bool kvm_range_memory_attributes_need_sync(struct kvm *kvm,
+						  gfn_t start, gfn_t end,
+						  unsigned long attributes)
+{
+	u64 mask = KVM_MEMORY_ATTRIBUTE_NEEDS_SYNC_MASK;
+
+	if (attributes & mask)
+		return true;
+
+	return kvm_range_has_any_memory_attributes(kvm, start, end, mask);
+}
+
 static __always_inline void kvm_handle_gfn_range(struct kvm *kvm,
 						 struct kvm_mmu_notifier_range *range)
 {
@@ -2559,20 +2605,25 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 		.on_lock = kvm_mmu_invalidate_end,
 		.may_block = true,
 	};
+	bool sync = false;
 	unsigned long i;
 	void *entry;
 	int r = 0;
 
 	entry = attributes ? xa_mk_value(attributes) : NULL;
 
-	trace_kvm_vm_set_mem_attributes(start, end, attributes);
-
 	mutex_lock(&kvm->slots_lock);
 
 	/* Nothing to do if the entire range has the desired attributes. */
 	if (kvm_range_has_memory_attributes(kvm, start, end, ~0, attributes))
 		goto out_unlock;
 
+	sync = kvm_range_memory_attributes_need_sync(kvm, start, end,
+						     attributes);
+
+	trace_kvm_vm_set_mem_attributes(start, end, attributes, sync,
+					kvm->mem_attrs.generation + 1);
+
 	/*
 	 * Reserve memory ahead of time to avoid having to deal with failures
 	 * partway through setting the new attributes.
@@ -2594,10 +2645,16 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
 		cond_resched();
 	}
 
+	/* Pairs with acquire in kvm_mem_attributes_generation() */
+	smp_store_release(&kvm->mem_attrs.generation,
+			  kvm->mem_attrs.generation + 1);
+
 	kvm_handle_gfn_range(kvm, &post_set_range);
 
 out_unlock:
 	mutex_unlock(&kvm->slots_lock);
+	if (sync)
+		synchronize_srcu_expedited(&kvm->srcu);
 
 	return r;
 }
@@ -3449,7 +3506,8 @@ int kvm_vcpu_write_guest(struct kvm_vcpu *vcpu, gpa_t gpa, const void *data,
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_vcpu_write_guest);
 
-static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
+static int __kvm_gfn_to_hva_cache_init(struct kvm *kvm,
+				       struct kvm_memslots *slots,
 				       struct gfn_to_hva_cache *ghc,
 				       gpa_t gpa, unsigned long len)
 {
@@ -3459,8 +3517,8 @@ static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
 	gfn_t nr_pages_needed = end_gfn - start_gfn + 1;
 	gfn_t nr_pages_avail;
 
-	/* Update ghc->generation before performing any error checks. */
-	ghc->generation = slots->generation;
+	/* Update ghc->slots_generation before performing any error checks. */
+	ghc->slots_generation = slots->generation;
 
 	if (start_gfn > end_gfn) {
 		ghc->hva = KVM_HVA_ERR_BAD;
@@ -3479,6 +3537,8 @@ static int __kvm_gfn_to_hva_cache_init(struct kvm_memslots *slots,
 			return -EFAULT;
 	}
 
+	ghc->attrs_generation = kvm_mem_attributes_generation(kvm);
+
 	/* Use the slow path for cross page reads and writes. */
 	if (nr_pages_needed == 1)
 		ghc->hva += offset;
@@ -3494,7 +3554,7 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 			      gpa_t gpa, unsigned long len)
 {
 	struct kvm_memslots *slots = kvm_memslots(kvm);
-	return __kvm_gfn_to_hva_cache_init(slots, ghc, gpa, len);
+	return __kvm_gfn_to_hva_cache_init(kvm, slots, ghc, gpa, len);
 }
 EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gfn_to_hva_cache_init);
 
@@ -3509,8 +3569,8 @@ int kvm_write_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 	if (WARN_ON_ONCE(len + offset > ghc->len))
 		return -EINVAL;
 
-	if (unlikely(!kvm_memslots_check_gen(kvm, ghc->generation, &slots))) {
-		if (__kvm_gfn_to_hva_cache_init(slots, ghc, ghc->gpa, ghc->len))
+	if (unlikely(!kvm_memslots_check_gen(kvm, ghc->slots_generation, ghc->attrs_generation, &slots))) {
+		if (__kvm_gfn_to_hva_cache_init(kvm, slots, ghc, ghc->gpa, ghc->len))
 			return -EFAULT;
 	}
 
@@ -3547,8 +3607,8 @@ int kvm_read_guest_offset_cached(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 	if (WARN_ON_ONCE(len + offset > ghc->len))
 		return -EINVAL;
 
-	if (unlikely(!kvm_memslots_check_gen(kvm, ghc->generation, &slots))) {
-		if (__kvm_gfn_to_hva_cache_init(slots, ghc, ghc->gpa, ghc->len))
+	if (unlikely(!kvm_memslots_check_gen(kvm, ghc->slots_generation, ghc->attrs_generation, &slots))) {
+		if (__kvm_gfn_to_hva_cache_init(kvm, slots, ghc, ghc->gpa, ghc->len))
 			return -EFAULT;
 	}
 
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index e09703d249bb..46ffae69fe77 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -76,10 +76,11 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
 		return false;
 
 	/*
-	 * If the page was cached from a memslot, make sure the memslots have
-	 * not been re-configured.
+	 * If the page was cached from a memslot, make sure the memslots nor
+	 * memory attributes have not been re-configured.
 	 */
-	if (!kvm_is_error_gpa(gpc->gpa) && !kvm_check_gen(gpc->kvm, gpc->generation))
+	if (!kvm_is_error_gpa(gpc->gpa) &&
+	    !kvm_check_gen(gpc->kvm, gpc->slots_generation, gpc->attrs_generation))
 		return false;
 
 	if (kvm_is_error_hva(gpc->uhva))
@@ -253,6 +254,7 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
 
 static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva)
 {
+	struct kvm *kvm = gpc->kvm;
 	unsigned long page_offset;
 	bool unmap_old = false;
 	unsigned long old_uhva;
@@ -292,12 +294,13 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 
 		page_offset = offset_in_page(gpa);
 
-		if (!kvm_memslots_check_gen(gpc->kvm, gpc->generation, &slots) ||
+		if (!kvm_memslots_check_gen(gpc->kvm, gpc->slots_generation, gpc->attrs_generation, &slots) ||
 		    gpc->gpa != gpa || kvm_is_error_hva(gpc->uhva)) {
 			gfn_t gfn = gpa_to_gfn(gpa);
 
+			gpc->attrs_generation = kvm_mem_attributes_generation(kvm);
 			gpc->gpa = gpa;
-			gpc->generation = slots->generation;
+			gpc->slots_generation = slots->generation;
 			gpc->memslot = __gfn_to_memslot(slots, gfn);
 			gpc->uhva = gfn_to_hva_memslot(gpc->memslot, gfn);
 
-- 
2.52.0

[PATCH 15/24] KVM: Take memory protections into account for accesses with cached gfn->hva

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Account for memory attributes when accessing guest memory through
kvm_get/put_guest().

This requires tracking the memory attributes generation as part of
gfn_to_hva_cache's data, invalidate the cached information if the
generation changes, and failing to refresh the cache if restrictive
memory attributes are found within the GPA range.

Similar to how gfn_to_hva_cache disallows caching gfns mapped within
read-only memory slots, gfns marked as read-only by memory attributes
will also fail to initialize. Unsurprisingly, the same behaviour applies
to gfns mapped as non-accessible (NR/NW), while non-executable mappings
are okay.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/linux/kvm_host.h | 19 +++++++++++++++++--
 virt/kvm/kvm_main.c      | 15 +++++++++++----
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 1fad3eb303c1..71ab2cbecbd1 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1351,7 +1351,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 	typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset);	\
 	int __ret = -EFAULT;						\
 									\
-	if (!kvm_is_error_hva(__addr))					\
+	if (!kvm_is_error_hva(__addr) &&                                \
+	    kvm_mem_attributes_may_read_gfn(kvm, gfn))		\
 		__ret = get_user(v, __uaddr);				\
 	__ret;								\
 })
@@ -1371,7 +1372,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
 	typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset);	\
 	int __ret = -EFAULT;						\
 									\
-	if (!kvm_is_error_hva(__addr))					\
+	if (!kvm_is_error_hva(__addr) &&                                \
+	    kvm_mem_attributes_may_write_gfn(kvm, gfn))		\
 		__ret = put_user(v, __uaddr);				\
 	if (!__ret)							\
 		mark_page_dirty(kvm, gfn);				\
@@ -2632,6 +2634,12 @@ static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn
 {
 	return 0;
 }
+static inline bool kvm_range_has_any_memory_attributes(struct kvm *kvm,
+						       gfn_t start, gfn_t end,
+						       unsigned long mask)
+{
+	return false;
+}
 static inline u64 kvm_mem_attributes_generation(struct kvm *kvm)
 {
 	return 0;
@@ -2652,6 +2660,13 @@ static inline int kvm_mem_attributes_may_write_gfn(struct kvm *kvm, gfn_t gfn)
 	return kvm_mem_attributes_may_write(attrs);
 }
 
+static inline bool kvm_range_has_rw_memory_protections(struct kvm *kvm,
+						       gfn_t start, gfn_t end)
+{
+	return kvm_range_has_any_memory_attributes(kvm, start, end,
+			KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW);
+}
+
 #ifdef CONFIG_KVM_GUEST_MEMFD
 int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
 		     gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 2fe4087319ad..a6fd17851c2f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -3529,15 +3529,22 @@ static int __kvm_gfn_to_hva_cache_init(struct kvm *kvm,
 	 * If the requested region crosses two memslots, we still
 	 * verify that the entire region is valid here.
 	 */
-	for ( ; start_gfn <= end_gfn; start_gfn += nr_pages_avail) {
-		ghc->memslot = __gfn_to_memslot(slots, start_gfn);
-		ghc->hva = gfn_to_hva_many(ghc->memslot, start_gfn,
-					   &nr_pages_avail);
+	for (gfn_t gfn = start_gfn ; gfn <= end_gfn; gfn += nr_pages_avail) {
+		ghc->memslot = __gfn_to_memslot(slots, gfn);
+		ghc->hva = gfn_to_hva_many(ghc->memslot, gfn, &nr_pages_avail);
 		if (kvm_is_error_hva(ghc->hva))
 			return -EFAULT;
 	}
 
+	/*
+	 * RW memory attributes are incompatible with GHC. The RW protection
+	 * check has to happen after storing the generation number.
+	 */
 	ghc->attrs_generation = kvm_mem_attributes_generation(kvm);
+	if (kvm_range_has_rw_memory_protections(kvm, start_gfn, end_gfn + 1)) {
+		ghc->hva = KVM_HVA_ERR_BAD;
+		return -EFAULT;
+	}
 
 	/* Use the slow path for cross page reads and writes. */
 	if (nr_pages_needed == 1)
-- 
2.52.0

[PATCH 16/24] KVM: pfncache: Fail to refresh if it contains memory protections

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Track the memory attributes generation as part of pfncache's data,
invalidate the cached information if the generation changes, and fail to
refresh the cache if restrictive memory attributes are found within the
GPA range.

Similar to how pfncache disallows caching gfns mapped within read-only
memory slots, gfns marked as read-only by memory attributes will also
fail to initialize. Unsurprisingly, the same behaviour applies to gfns
mapped as non-accessible (NR/NW), while non-executable mappings are fine.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 virt/kvm/pfncache.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 46ffae69fe77..39935740136d 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -298,7 +298,18 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
 		    gpc->gpa != gpa || kvm_is_error_hva(gpc->uhva)) {
 			gfn_t gfn = gpa_to_gfn(gpa);
 
+			/*
+			 * RW memory attributes are incompatible with GPC. The
+			 * RW protection check has to happen after storing the
+			 * generation number.
+			 */
 			gpc->attrs_generation = kvm_mem_attributes_generation(kvm);
+			if (kvm_range_has_rw_memory_protections(kvm, gfn, gfn + 1)) {
+				gpc->uhva = KVM_HVA_ERR_BAD;
+				ret = -EFAULT;
+				goto out;
+			}
+
 			gpc->gpa = gpa;
 			gpc->slots_generation = slots->generation;
 			gpc->memslot = __gfn_to_memslot(slots, gfn);
-- 
2.52.0

[PATCH 17/24] KVM: x86/mmu: Take memory protection attributes into account during faults

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Take memory protection attributes when faulting guest memory.  Prohibited
memory accesses will cause a user-space -EFAULT exit just like private
memory accesses.  Userspace will either enable the access or bump it to
the guest as some kind of exception (e.g. a VTL return).

Since the struct kvm_page_fault already has the access type in PFERR_*
format, the check is done via the kvm_page_format permissions table.
This means that it supports naturally all page table format variants,
and it can even handle mode-based memory protection when the host
uses MBEC/GMET.  The only thing that needs some care is to build the
restricted ACC_* mask with the root page's own access mask as a base
(and not ACC_ALL).  Otherwise, supervisor mode execution would be
handled incorrectly on AMD processors with GMET.

To avoid spamming the trace buffer too much, the new trace event only
kicks in if memory protection attributes are present for the faulted gfn.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c          | 49 +++++++++++++++++++++++++++++++++
 arch/x86/kvm/mmu/mmu_internal.h | 19 +++++++++++++
 arch/x86/kvm/mmu/mmutrace.h     | 29 +++++++++++++++++++
 arch/x86/kvm/mmu/paging_tmpl.h  |  2 +-
 arch/x86/kvm/mmu/spte.h         | 11 ++------
 5 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index b6463b0b0b6d..59c2a04f5648 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -4611,6 +4611,50 @@ static int kvm_mmu_faultin_pfn_gmem(struct kvm_vcpu *vcpu,
 	return RET_PF_CONTINUE;
 }
 
+static inline unsigned kvm_get_gfn_protections(struct kvm_vcpu *vcpu, gfn_t gfn)
+{
+	struct kvm *kvm = vcpu->kvm;
+	unsigned int access = vcpu->arch.mmu->root_role.access;
+	unsigned long attrs = kvm_get_memory_attributes(kvm, gfn);
+	if (!attrs)
+		return access;
+
+	WARN_ON_ONCE(!kvm_mem_attributes_valid(kvm, attrs));
+
+	if (!kvm_mem_attributes_may_read(attrs))
+		access &= ~ACC_READ_MASK;
+	if (!kvm_mem_attributes_may_write(attrs))
+		access &= ~ACC_WRITE_MASK;
+	if (!kvm_mem_attributes_may_exec(attrs)) {
+		access &= ~ACC_EXEC_MASK;
+		if (shadow_xu_mask)
+			access &= ~ACC_USER_EXEC_MASK;
+	}
+
+	return access;
+}
+
+static int kvm_faultin_memory_protections(struct kvm_vcpu *vcpu,
+					  struct kvm_page_fault *fault)
+{
+	unsigned access;
+
+	/* Memory attributes don't apply to MMIO regions */
+	if (unlikely(!fault->slot))
+		return RET_PF_CONTINUE;
+
+	access = kvm_get_gfn_protections(vcpu, fault->gfn);
+	if (access == ACC_ALL)
+		return RET_PF_CONTINUE;
+
+	trace_kvm_faultin_memory_protections(vcpu, fault, access);
+	if (__permission_fault(vcpu->arch.mmu, access, fault))
+		return -EFAULT;
+
+	fault->host_access &= access;
+	return RET_PF_CONTINUE;
+}
+
 static int __kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
 				 struct kvm_page_fault *fault)
 {
@@ -4691,6 +4735,11 @@ static int kvm_mmu_faultin_pfn(struct kvm_vcpu *vcpu,
 	if (unlikely(!slot))
 		return kvm_handle_noslot_fault(vcpu, fault, access);
 
+	if (kvm_faultin_memory_protections(vcpu, fault)) {
+		kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
+		return -EFAULT;
+	}
+
 	/*
 	 * Retry the page fault if the gfn hit a memslot that is being deleted
 	 * or moved.  This ensures any existing SPTEs for the old memslot will
diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
index 00215b9f309f..0bad60c5aab6 100644
--- a/arch/x86/kvm/mmu/mmu_internal.h
+++ b/arch/x86/kvm/mmu/mmu_internal.h
@@ -290,6 +290,25 @@ struct kvm_page_fault {
 	bool write_fault_to_shadow_pgtable;
 };
 
+/*
+ * Returns true if the access indicated by @fault is forbidden by the existing
+ * SPTE protections.
+ */
+static inline bool __permission_fault(struct kvm_mmu *mmu, unsigned access,
+				      struct kvm_page_fault *fault)
+{
+	unsigned pfec;
+
+	/*
+	 * RSVD is handled elsewhere, and is used for SMAP in the context
+	 * of accessing fmt.permissions[].  SPTEs never use PK or SS, as
+	 * they are not supported for shadow paging and irrelevant for TDP.
+	 */
+	pfec = fault->error_code & (
+		PFERR_WRITE_MASK | PFERR_USER_MASK | PFERR_FETCH_MASK);
+	return (mmu->fmt.permissions[pfec >> 1] >> access) & 1;
+}
+
 /*
  * Return values of handle_mmio_page_fault(), mmu.page_fault(), fast_page_fault(),
  * and of course kvm_mmu_do_page_fault().
diff --git a/arch/x86/kvm/mmu/mmutrace.h b/arch/x86/kvm/mmu/mmutrace.h
index 8354d9f39777..6bbd66a827b6 100644
--- a/arch/x86/kvm/mmu/mmutrace.h
+++ b/arch/x86/kvm/mmu/mmutrace.h
@@ -447,6 +447,35 @@ TRACE_EVENT(
 		  __entry->gfn, __entry->spte, __entry->level, __entry->errno)
 );
 
+TRACE_EVENT(kvm_faultin_memory_protections,
+	TP_PROTO(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
+		 unsigned access),
+	TP_ARGS(vcpu, fault, access),
+
+	TP_STRUCT__entry(
+		__field(unsigned int, vcpu_id)
+		__field(unsigned long, guest_rip)
+		__field(u64, fault_address)
+		__field(bool, write)
+		__field(bool, exec)
+		__field(unsigned, access)
+	),
+
+	TP_fast_assign(
+		__entry->vcpu_id = vcpu->vcpu_id;
+		__entry->guest_rip = kvm_rip_read(vcpu);
+		__entry->fault_address = fault->gfn;
+		__entry->write = fault->write;
+		__entry->exec = fault->exec;
+		__entry->access = access;
+	),
+
+	TP_printk("vcpu %d rip 0x%lx gfn 0x%016llx access %s protections 0x%x",
+		  __entry->vcpu_id, __entry->guest_rip, __entry->fault_address,
+		  __entry->exec ? "X" : (__entry->write ? "W" : "R"),
+		  __entry->access)
+);
+
 #endif /* _TRACE_KVMMMU_H */
 
 #undef TRACE_INCLUDE_PATH
diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 1871d334fed7..cdf05cd76d63 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -990,7 +990,7 @@ static int FNAME(sync_spte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp, int
 
 	sptep = &sp->spt[i];
 	spte = *sptep;
-	host_access = ACC_ALL;
+	host_access = kvm_get_gfn_protections(vcpu, gfn);
 	if (!(spte & shadow_host_writable_mask))
 		host_access &= ~ACC_WRITE_MASK;
 	slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
diff --git a/arch/x86/kvm/mmu/spte.h b/arch/x86/kvm/mmu/spte.h
index 589f3954633e..14f322944671 100644
--- a/arch/x86/kvm/mmu/spte.h
+++ b/arch/x86/kvm/mmu/spte.h
@@ -491,7 +491,7 @@ static inline bool is_mmu_writable_spte(u64 spte)
 static inline bool spte_permission_fault(struct kvm_mmu *mmu, u64 spte,
 					 struct kvm_page_fault *fault)
 {
-	unsigned pfec, pte_access;
+	unsigned pte_access;
 
 	if (!is_shadow_present_pte(spte))
 		return true;
@@ -511,14 +511,7 @@ static inline bool spte_permission_fault(struct kvm_mmu *mmu, u64 spte,
 		pte_access |= spte & shadow_xu_mask ? ACC_USER_EXEC_MASK : 0;
 	}
 
-	/*
-	 * RSVD is handled elsewhere, and is used for SMAP in the context
-	 * of accessing fmt.permissions[].  SPTEs never use PK or SS, as
-	 * they are not supported for shadow paging and irrelevant for TDP.
-	 */
-	pfec = fault->error_code & (
-		PFERR_WRITE_MASK | PFERR_USER_MASK | PFERR_FETCH_MASK);
-	return (mmu->fmt.permissions[pfec >> 1] >> pte_access) & 1;
+	return __permission_fault(mmu, pte_access, fault);
 }
 
 /*
-- 
2.52.0

[PATCH 18/24] KVM: x86/mmu: Issue memory fault exit if walk failed due to memory attribute

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

During execution, page table entries are subject to memory protection
simply because their GPA is obtained with an EPT walk.  During emulation,
the checks need to be done by hand and result in an -EFAULT userspace
exit.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index cdf05cd76d63..23d7a7d9769f 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -89,6 +89,7 @@ struct guest_walker {
 	unsigned int pt_access[PT_MAX_FULL_LEVELS];
 	unsigned int pte_access;
 	gfn_t gfn;
+	bool memory_attributes_fault;
 	struct x86_exception fault;
 };
 
@@ -342,6 +343,7 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 
 	trace_kvm_mmu_pagetable_walk(addr, access);
 retry_walk:
+	walker->memory_attributes_fault = false;
 	walker->level = w->cpu_role.base.level;
 	pte           = kvm_mmu_get_guest_pgd(vcpu, w);
 	have_ad       = PT_HAVE_ACCESSED_DIRTY(w);
@@ -411,6 +413,12 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 		if (unlikely(kvm_is_error_hva(host_addr)))
 			goto error;
 
+		if (!kvm_mem_attributes_may_read_gfn(vcpu->kvm, gpa_to_gfn(real_gpa))) {
+			walker->memory_attributes_fault = true;
+			walker->gfn = gpa_to_gfn(real_gpa);
+			goto error;
+		}
+
 		ptep_user = (pt_element_t __user *)((void *)host_addr + offset);
 		if (unlikely(get_user(pte, ptep_user)))
 			goto error;
@@ -820,6 +828,12 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
 	 * The page is not mapped by the guest.  Let the guest handle it.
 	 */
 	if (!r) {
+		if (walker.memory_attributes_fault) {
+			fault->gfn = walker.gfn;
+			kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
+			return -EFAULT;
+		}
+
 		if (!fault->prefetch)
 			__kvm_inject_emulated_page_fault(vcpu, &walker.fault, true);
 
-- 
2.52.0

[PATCH 19/24] KVM: x86/mmu: Do not update accessed/dirty if guest PTE is read-only

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

In this case do not go all the way out to userspace [RFC]

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/paging_tmpl.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
index 23d7a7d9769f..ee5c23d1b77a 100644
--- a/arch/x86/kvm/mmu/paging_tmpl.h
+++ b/arch/x86/kvm/mmu/paging_tmpl.h
@@ -419,6 +419,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
 			goto error;
 		}
 
+		if (!kvm_mem_attributes_may_write_gfn(vcpu->kvm, gpa_to_gfn(real_gpa)))
+			walker->pte_writable[walker->level - 1] = false;
+
 		ptep_user = (pt_element_t __user *)((void *)host_addr + offset);
 		if (unlikely(get_user(pte, ptep_user)))
 			goto error;
-- 
2.52.0

[PATCH 20/24] KVM: x86/mmu: Do not prefetch sptes on gfns backed by memory attributes

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Prefetched SPTEs are always given full access.  Do not prefetch GFNs
that have memory protections applied.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 59c2a04f5648..8e85f672a08d 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -3156,6 +3156,9 @@ static bool kvm_mmu_prefetch_sptes(struct kvm_vcpu *vcpu, gfn_t gfn, u64 *sptep,
 		return false;
 
 	for (i = 0; i < nr_pages; i++, gfn++, sptep++) {
+		if (kvm_get_memory_attributes(vcpu->kvm, gfn))
+			continue;
+
 		mmu_set_spte(vcpu, slot, sptep, access, gfn,
 			     page_to_pfn(pages[i]), NULL);
 
-- 
2.52.0

[PATCH 21/24] KVM: x86/mmu: Obsolete all roots if memattr contains gPTEs

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

see comments inside - this patch is incomplete.

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/mmu/mmu.c | 47 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 45 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 8e85f672a08d..676bc6b61f03 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -6989,11 +6989,11 @@ static void kvm_zap_obsolete_pages(struct kvm *kvm)
  * not use any resource of the being-deleted slot or all slots
  * after calling the function.
  */
-static void kvm_mmu_zap_all_fast(struct kvm *kvm)
+static void __kvm_mmu_zap_all_fast(struct kvm *kvm)
 {
 	lockdep_assert_held(&kvm->slots_lock);
+	lockdep_assert_held_write(&kvm->mmu_lock);
 
-	write_lock(&kvm->mmu_lock);
 	trace_kvm_mmu_zap_all_fast(kvm);
 
 	/*
@@ -7030,7 +7030,21 @@ static void kvm_mmu_zap_all_fast(struct kvm *kvm)
 	kvm_make_all_cpus_request(kvm, KVM_REQ_MMU_FREE_OBSOLETE_ROOTS);
 
 	kvm_zap_obsolete_pages(kvm);
+}
 
+/*
+ * Fast invalidate all shadow pages and use lock-break technique
+ * to zap obsolete pages.
+ *
+ * It's required when memslot is being deleted or VM is being
+ * destroyed, in these cases, we should ensure that KVM MMU does
+ * not use any resource of the being-deleted slot or all slots
+ * after calling the function.
+ */
+static void kvm_mmu_zap_all_fast(struct kvm *kvm)
+{
+	write_lock(&kvm->mmu_lock);
+	__kvm_mmu_zap_all_fast(kvm);
 	write_unlock(&kvm->mmu_lock);
 
 	/*
@@ -8221,6 +8235,8 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
 {
 	unsigned long attrs = range->arg.attributes;
 	struct kvm_memory_slot *slot = range->slot;
+	bool gen_update = false;
+	struct kvm_mmu_page *sp;
 	int level;
 
 	lockdep_assert_held_write(&kvm->mmu_lock);
@@ -8281,6 +8297,33 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
 				hugepage_set_mixed(slot, gfn, level);
 		}
 	}
+
+	/*
+	 * There are special considerations when applying an memory protection
+	 * attibute against a GPTE page. If set read-only, access/dirty bits
+	 * within that page shouldn't be updated. If set non-accesible,
+	 * accessing a virtual address that requires traversing that GPTE page
+	 * should fault.
+	 *
+	 * On TDP enabled guests, the CPU faults on the GPTE address upon
+	 * detecting such a situation.
+	 *
+	 * On non-TDP, upon detecting this situation, and based on the fact it
+	 * should be a rare occasion, invalidate all the mmu roots.
+	 */
+	for (gfn_t gfn = range->start; gfn < range->end; gfn++) {
+		for_each_gfn_valid_sp_with_gptes(kvm, sp, gfn) {
+			gen_update = true;
+			trace_printk("needs gen update! %llx\n", gfn);
+			goto exit_loop;
+		}
+	}
+exit_loop:
+	if (gen_update)
+		__kvm_mmu_zap_all_fast(kvm);
+
+	// todo: what to do with kvm_tdp_mmu_zap_invalidated_roots()?
+	// add kvm_arch_post_set_memory_attributes_unlocked?
 	return false;
 }
 
-- 
2.52.0

[PATCH 22/24] KVM: x86: selftests: Introduce memory attributes test

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 tools/include/uapi/linux/kvm.h                |   3 +
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../testing/selftests/kvm/include/kvm_util.h  |  23 +-
 .../testing/selftests/kvm/memory_attributes.c | 362 ++++++++++++++++++
 .../selftests/kvm/x86/memory_attributes.c     |  43 +++
 5 files changed, 422 insertions(+), 10 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/memory_attributes.c
 create mode 100644 tools/testing/selftests/kvm/x86/memory_attributes.c

diff --git a/tools/include/uapi/linux/kvm.h b/tools/include/uapi/linux/kvm.h
index d0c0c8605976..fe2f500c932d 100644
--- a/tools/include/uapi/linux/kvm.h
+++ b/tools/include/uapi/linux/kvm.h
@@ -1639,6 +1639,9 @@ struct kvm_memory_attributes {
 	__u64 flags;
 };
 
+#define KVM_MEMORY_ATTRIBUTE_NR		       (1ULL << 0)
+#define KVM_MEMORY_ATTRIBUTE_NW		       (1ULL << 1)
+#define KVM_MEMORY_ATTRIBUTE_NX		       (1ULL << 2)
 #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
 
 #define KVM_CREATE_GUEST_MEMFD	_IOWR(KVMIO,  0xd4, struct kvm_create_guest_memfd)
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 4ace12606e93..3e2a6e4398eb 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -162,6 +162,7 @@ TEST_GEN_PROGS_x86 += rseq_test
 TEST_GEN_PROGS_x86 += steal_time
 TEST_GEN_PROGS_x86 += system_counter_offset_test
 TEST_GEN_PROGS_x86 += pre_fault_memory_test
+TEST_GEN_PROGS_x86 += memory_attributes
 
 # Compiled outputs used by test targets
 TEST_GEN_PROGS_EXTENDED_x86 += x86/nx_huge_pages_test
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 04a910164a29..54d73ea46e11 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -418,24 +418,27 @@ static inline void vm_enable_cap(struct kvm_vm *vm, u32 cap, u64 arg0)
 	vm_ioctl(vm, KVM_ENABLE_CAP, &enable_cap);
 }
 
-static inline void vm_set_memory_attributes(struct kvm_vm *vm, gpa_t gpa,
-					    u64 size, u64 attributes)
+static inline int __vm_set_memory_attributes(struct kvm_vm *vm, u64 gpa,
+					     u64 size, u64 attributes,
+					     u64 flags)
 {
 	struct kvm_memory_attributes attr = {
 		.attributes = attributes,
 		.address = gpa,
 		.size = size,
-		.flags = 0,
+		.flags = flags,
 	};
 
-	/*
-	 * KVM_SET_MEMORY_ATTRIBUTES overwrites _all_ attributes.  These flows
-	 * need significant enhancements to support multiple attributes.
-	 */
-	TEST_ASSERT(!attributes || attributes == KVM_MEMORY_ATTRIBUTE_PRIVATE,
-		    "Update me to support multiple attributes!");
+	return __vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES, &attr);
+}
 
-	vm_ioctl(vm, KVM_SET_MEMORY_ATTRIBUTES, &attr);
+static inline void vm_set_memory_attributes(struct kvm_vm *vm, gpa_t gpa,
+					    u64 size, u64 attributes)
+{
+	int rc;
+
+	rc = __vm_set_memory_attributes(vm, gpa, size, attributes, 0);
+	TEST_ASSERT_VM_VCPU_IOCTL(!rc, KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
 }
 
 
diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
new file mode 100644
index 000000000000..7066ae791b99
--- /dev/null
+++ b/tools/testing/selftests/kvm/memory_attributes.c
@@ -0,0 +1,362 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024, Amazon.com, Inc. or its affiliates. All Rights Reserved
+ *
+ * Test for KVM_MEMORY_ATTRIBUTES
+ */
+#include <pthread.h>
+#include <signal.h>
+
+#include "test_util.h"
+#include "ucall_common.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "hyperv.h"
+#include "apic.h"
+#include "asm/pvclock-abi.h"
+
+#define KVM_MEMORY_ATTRIBUTE_NO_ACCESS                       \
+	(KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW | \
+	 KVM_MEMORY_ATTRIBUTE_NX)
+
+#define MMIO_GPA	0x700000000
+#define MMIO_GVA	MMIO_GPA
+
+enum {
+	TEST_OP_NOP,
+	TEST_OP_READ,
+	TEST_OP_WRITE,
+	TEST_OP_EXEC,
+	TEST_OP_EXIT,
+};
+
+const char *test_op_names[] =
+{
+	[TEST_OP_READ] = "Read",
+	[TEST_OP_WRITE] = "Write",
+	[TEST_OP_EXEC] = "Exec",
+	[TEST_OP_EXIT] = "Exit",
+};
+
+struct test_data {
+	uint8_t op;
+	int stage;
+	gva_t vaddr;
+
+	struct kvm_vcpu *vcpu;
+};
+
+static struct test_data *test_data;
+
+static uint64_t arch_controlled_read(gva_t addr);
+static void arch_controlled_write(gva_t addr, uint64_t val);
+static void arch_controlled_exec(gva_t addr);
+static void arch_write_return_insn(struct kvm_vm *vm, gpa_t vaddr);
+
+static void guest_code(void *data)
+{
+	struct test_data *test_data = data;
+	int stage = 1;
+
+	while (true) {
+		gva_t vaddr = READ_ONCE(test_data->vaddr);
+
+		switch(READ_ONCE(test_data->op)) {
+		case TEST_OP_READ:
+			(void) arch_controlled_read(vaddr);
+			GUEST_SYNC(stage++);
+			break;
+		case TEST_OP_WRITE:
+			arch_controlled_write(vaddr, 1);
+			GUEST_SYNC(stage++);
+			break;
+		case TEST_OP_EXEC:
+			arch_controlled_exec(vaddr);
+			GUEST_SYNC(stage++);
+			break;
+		default:
+			goto exit;
+		};
+	}
+
+exit:
+	GUEST_DONE();
+}
+
+static void vcpu_run_and_inc_stage(struct kvm_vcpu *vcpu)
+{
+	struct ucall uc;
+
+	vcpu_run(vcpu);
+
+	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO);
+	switch (get_ucall(vcpu, &uc)) {
+	case UCALL_SYNC:
+		TEST_ASSERT(uc.args[1] == test_data->stage,
+			    "Unexpected stage: %ld (%d expected)",
+			    uc.args[1], test_data->stage);
+		break;
+	case UCALL_ABORT:
+		REPORT_GUEST_ASSERT(uc);
+		/* NOT REACHED */
+	default:
+		TEST_FAIL("Unknown ucall %lu", uc.cmd);
+	}
+
+	test_data->stage++;
+}
+
+static void test_page_restricted(struct kvm_vcpu *vcpu, int op,
+				 gva_t vaddr, gpa_t fault_paddr,
+				 uint64_t fault_reason)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	int rc;
+
+	test_data->op = op;
+	test_data->vaddr = vaddr;
+
+	rc = _vcpu_run(vcpu);
+	TEST_ASSERT(rc == -1 && errno == EFAULT,
+		    "KVM_RUN IOCTL didn't return EFAULT on %s, rc %d, errno %d",
+		    test_op_names[op], rc, errno);
+	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_MEMORY_FAULT);
+	TEST_ASSERT_EQ(vcpu->run->memory_fault.gpa, fault_paddr);
+	TEST_ASSERT_EQ(vcpu->run->memory_fault.flags, fault_reason);
+	TEST_ASSERT_EQ(vcpu->run->memory_fault.size, vm->page_size);
+}
+
+static void test_page_accessible(struct kvm_vcpu *vcpu, int op, gva_t vaddr)
+{
+	test_data->op = op;
+	test_data->vaddr = vaddr;
+	vcpu_run_and_inc_stage(vcpu);
+}
+
+#include "x86/memory_attributes.c"
+
+/*
+ * We want to test the following cases:
+ * - Sucessful access to GPAs backed by memory attributes (for ex. read access
+ *   on an read-only page).
+ * - First fault after setting memory attributes, with unpopulated SPTEs/EPTS.
+ * - Fault caused by an SPTE/EPT reflecting the memory attributes.
+ *
+ * The list of ops below tests the 3 situations for each memory attribute
+ * combination.
+ */
+const struct memory_access {
+	const char *name;
+	uint64_t attrs;
+	int ops[5];
+} access_array[] = {
+	{ "all allowed", 0, { TEST_OP_READ, TEST_OP_WRITE, TEST_OP_EXEC } },
+	{ "no write (unmapped)", KVM_MEMORY_ATTRIBUTE_NW,
+	  { TEST_OP_WRITE, TEST_OP_READ, TEST_OP_EXEC, TEST_OP_WRITE } },
+	{ "no write (mapped)", KVM_MEMORY_ATTRIBUTE_NW,
+	  { TEST_OP_READ, TEST_OP_WRITE, TEST_OP_READ, TEST_OP_EXEC, TEST_OP_WRITE } },
+	{ "no exec (unmapped)", KVM_MEMORY_ATTRIBUTE_NX,
+	  { TEST_OP_EXEC, TEST_OP_READ, TEST_OP_WRITE, TEST_OP_EXEC } },
+	{ "no exec (mapped)", KVM_MEMORY_ATTRIBUTE_NX,
+	  { TEST_OP_READ, TEST_OP_EXEC, TEST_OP_READ, TEST_OP_WRITE, TEST_OP_EXEC } },
+	{ "read only", KVM_MEMORY_ATTRIBUTE_NW | KVM_MEMORY_ATTRIBUTE_NX,
+	  { TEST_OP_EXEC, TEST_OP_WRITE, TEST_OP_READ, TEST_OP_WRITE,
+	    TEST_OP_EXEC } },
+	{ "no access (map on read)", KVM_MEMORY_ATTRIBUTE_NO_ACCESS,
+	  { TEST_OP_READ, TEST_OP_WRITE, TEST_OP_EXEC } },
+	{ "no access (map on write)", KVM_MEMORY_ATTRIBUTE_NO_ACCESS,
+	  { TEST_OP_WRITE, TEST_OP_READ, TEST_OP_EXEC } },
+	{ "no access (map on exec)", KVM_MEMORY_ATTRIBUTE_NO_ACCESS,
+	  { TEST_OP_EXEC, TEST_OP_READ, TEST_OP_WRITE } },
+	/* Verify everything is back to normal */
+	{ "all allowed (2)", 0, { TEST_OP_READ, TEST_OP_WRITE, TEST_OP_EXEC } },
+};
+
+static void test_page_access(struct kvm_vcpu *vcpu, gva_t vaddr,
+			     uint64_t attrs, const int ops[])
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gpa_t paddr = addr_gva2gpa(vm, vaddr);
+
+	for (int i = 0; i < ARRAY_SIZE(access_array[0].ops); i++) {
+		int op = ops[i];
+
+		if (op == TEST_OP_NOP)
+			continue;
+
+		/*
+		 * We're about to have the guest jump into 'vaddr', make it a
+		 * 'ret' instruction so it returns right away.
+		 */
+		if (op == TEST_OP_EXEC)
+			arch_write_return_insn(vm, paddr);
+
+		vm_set_memory_attributes(vm, paddr, vm->page_size, attrs);
+
+		/*
+		 * Attributes are negated, a match means the operation should
+		 * fail.
+		 */
+		if (attrs & BIT_ULL(op - 1)) {
+			test_page_restricted(vcpu, op, vaddr, paddr, BIT(op - 1));
+			vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+		}
+
+		test_page_accessible(vcpu, op, vaddr);
+	}
+
+}
+
+static void test_memory_access(struct kvm_vcpu *vcpu, gva_t test_vm_vaddr,
+			       size_t size)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gpa_t test_vm_paddr = addr_gva2gpa(vm, test_vm_vaddr);
+
+	printf("gva %lx gpa %lx\n", test_vm_vaddr, test_vm_paddr);
+	for (size_t i = 0; i < ARRAY_SIZE(access_array); i++) {
+		uint64_t attrs = access_array[i].attrs;
+
+		printf("starting %s test...\n", access_array[i].name);
+		vm_set_memory_attributes(vm, test_vm_paddr, size, attrs);
+
+		for (gva_t vaddr = test_vm_vaddr;
+		     vaddr < test_vm_vaddr + size; vaddr += PAGE_SIZE) {
+			test_page_access(vcpu, vaddr, attrs, access_array[i].ops);
+		}
+	}
+}
+
+static void test_memattrs_ignore_mmio(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vm *vm = vcpu->vm;
+
+	vm_set_memory_attributes(vm, MMIO_GPA, vm->page_size,
+				 KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
+
+	test_data->op = TEST_OP_READ;
+	test_data->vaddr = MMIO_GVA;
+	vcpu_run(vcpu);
+	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_MMIO);
+	TEST_ASSERT_EQ(vcpu->run->mmio.phys_addr, MMIO_GPA);
+	TEST_ASSERT_EQ(vcpu->run->mmio.is_write, 0);
+	TEST_ASSERT_EQ(vcpu->run->mmio.len, 8);
+	vcpu_run_and_inc_stage(vcpu);
+
+	test_data->op = TEST_OP_WRITE;
+	test_data->vaddr = MMIO_GVA;
+	vcpu_run(vcpu);
+	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_MMIO);
+	TEST_ASSERT_EQ(vcpu->run->mmio.phys_addr, MMIO_GPA);
+	TEST_ASSERT_EQ(vcpu->run->mmio.is_write, 1);
+	TEST_ASSERT_EQ(vcpu->run->mmio.len, 8);
+	vcpu_run_and_inc_stage(vcpu);
+
+	vm_set_memory_attributes(vm, MMIO_GPA, vm->page_size, 0);
+}
+
+static void test_input_validation(struct kvm_vm *vm)
+{
+	uint64_t flags, gpa = 0, size = 0, attrs = 0;
+	int rc;
+
+	/* 'flags' is unsupported */
+	flags = BIT(0);
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* 'size' can't be 0 */
+	flags = 0;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* 'gpa' shouldn't overflow */
+	gpa = 0ULL - vm->page_size;
+	size = vm->page_size;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* 'gpa' should be page aligned */
+	gpa = 1;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* 'size' should be page aligned */
+	gpa = 0;
+	size = 1;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* exec mappings require read access */
+	size = vm->page_size;
+	attrs = KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* write mappings require read access */
+	size = vm->page_size;
+	attrs = KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NX;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+
+	/* private mappings are incompatible with access restrictions */
+	attrs = KVM_MEMORY_ATTRIBUTE_NW | KVM_MEMORY_ATTRIBUTE_PRIVATE;
+	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);
+	TEST_ASSERT_VM_VCPU_IOCTL(rc == -1 && errno == EINVAL,
+				  KVM_SET_MEMORY_ATTRIBUTES, rc, vm);
+}
+
+static void test_finalize(struct kvm_vcpu *vcpu)
+{
+	test_data->op = TEST_OP_EXIT;
+	vcpu_run(vcpu);
+	TEST_ASSERT_EQ(get_ucall(vcpu, NULL), UCALL_DONE);
+}
+
+static struct test_data *init_test_data(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gva_t test_data_vm_vaddr;
+
+	test_data_vm_vaddr = vm_alloc_page(vm);
+	vcpu_args_set(vcpu, 1, test_data_vm_vaddr);
+
+	test_data = addr_gva2hva(vm, test_data_vm_vaddr);
+	test_data->stage = 1;
+	test_data->vcpu = vcpu;
+
+	return test_data;
+}
+
+int main(int argc, char *argv[])
+{
+	uint32_t guest_page_size = vm_guest_mode_params[VM_MODE_DEFAULT].page_size;
+	unsigned int ptes_per_page = guest_page_size / 8;
+	size_t size = guest_page_size * ptes_per_page * 2; /* 2 huge-pages */
+	struct kvm_vcpu *vcpu;
+	gva_t test_mem;
+	struct kvm_vm *vm;
+
+	TEST_REQUIRE(kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES) &
+		     KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
+
+	vm = __vm_create_with_one_vcpu(&vcpu, size, guest_code);
+	test_mem = vm_alloc(vm, size, KVM_UTIL_MIN_VADDR);
+	virt_map(vcpu->vm, MMIO_GVA, MMIO_GPA, 1);
+	test_data = init_test_data(vcpu);
+
+	test_input_validation(vm);
+	test_memory_access(vcpu, test_mem, size);
+	test_memattrs_ignore_mmio(vcpu);
+	test_finalize(vcpu);
+
+	kvm_vm_free(vm);
+	return 0;
+}
diff --git a/tools/testing/selftests/kvm/x86/memory_attributes.c b/tools/testing/selftests/kvm/x86/memory_attributes.c
new file mode 100644
index 000000000000..2e1148f5146d
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/memory_attributes.c
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2024, Amazon.com, Inc. or its affiliates. All Rights Reserved
+ *
+ * Test for KVM_MEMORY_ATTRIBUTES
+ */
+#include "kvm_util.h"
+#include "apic.h"
+
+uint64_t arch_controlled_read(gva_t addr)
+{
+	uint64_t val;
+
+	asm volatile("mov %[addr], %%rax \n\r"
+		     "mov (%%rax), %[val] \n\r"
+		     : [val] "=r" (val)
+		     : [addr] "m"(addr)
+		     : "memory", "rax");
+
+	return val;
+}
+
+void arch_controlled_write(gva_t addr, uint64_t val)
+{
+	asm volatile("mov %[addr], %%rax \n\r"
+		     "mov %[val], %%rbx \n\r"
+		     "mov %%rbx, (%%rax) \n\r"
+		     :: [addr] "m" (addr), [val] "m" (val)
+		     : "memory", "rax", "rbx");
+}
+
+void arch_controlled_exec(gva_t addr)
+{
+	asm volatile("mov %[addr], %%rax \n\r"
+		     "call *%%rax \n\t"
+		     :: [addr] "m"(addr)
+		     : "memory", "rax");
+}
+
+void arch_write_return_insn(struct kvm_vm *vm, gpa_t vaddr)
+{
+	memset(addr_gpa2hva(vm, vaddr), 0xc3, 1);
+}
-- 
2.52.0

[PATCH 23/24] KVM: x86: selftests: Introduce memory attributes PTE test

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Introduce the memory attributes PTE tests. This test confirms the
behaviour of memory attributes access restrictions when installed on a
page that holds guest page table entries. Notably two cases are taken
into account:
- The page is made non-accesible. In such case the next access to a
  virtual memory address translated by that paging structure should
  fault.
- The page is made read-only. In such case the next access to a virtual
  memory address translated by that paging structure should either fault
  or succeed yet not perform any writes into the PTE (accessed and
  dirty bits).

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .../testing/selftests/kvm/memory_attributes.c |  38 +++++-
 .../selftests/kvm/x86/memory_attributes.c     | 126 ++++++++++++++++++
 2 files changed, 162 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
index 7066ae791b99..c66d5d085a9c 100644
--- a/tools/testing/selftests/kvm/memory_attributes.c
+++ b/tools/testing/selftests/kvm/memory_attributes.c
@@ -22,11 +22,16 @@
 #define MMIO_GPA	0x700000000
 #define MMIO_GVA	MMIO_GPA
 
+#define PT_WRITABLE_MASK	BIT_ULL(1)
+#define PT_ACCESSED_MASK	BIT_ULL(5)
+#define PTE_VADDR		0x1000000000
+
 enum {
 	TEST_OP_NOP,
 	TEST_OP_READ,
 	TEST_OP_WRITE,
 	TEST_OP_EXEC,
+	TEST_OP_INVPLG,
 	TEST_OP_EXIT,
 };
 
@@ -35,6 +40,7 @@ const char *test_op_names[] =
 	[TEST_OP_READ] = "Read",
 	[TEST_OP_WRITE] = "Write",
 	[TEST_OP_EXEC] = "Exec",
+	[TEST_OP_INVPLG] = "Invplg",
 	[TEST_OP_EXIT] = "Exit",
 };
 
@@ -42,6 +48,7 @@ struct test_data {
 	uint8_t op;
 	int stage;
 	gva_t vaddr;
+	uint64_t expected_val;
 
 	struct kvm_vcpu *vcpu;
 };
@@ -59,6 +66,7 @@ static void guest_code(void *data)
 	int stage = 1;
 
 	while (true) {
+		uint64_t expected_val = READ_ONCE(test_data->expected_val);
 		gva_t vaddr = READ_ONCE(test_data->vaddr);
 
 		switch(READ_ONCE(test_data->op)) {
@@ -67,13 +75,20 @@ static void guest_code(void *data)
 			GUEST_SYNC(stage++);
 			break;
 		case TEST_OP_WRITE:
-			arch_controlled_write(vaddr, 1);
+			arch_controlled_write(vaddr, expected_val);
 			GUEST_SYNC(stage++);
 			break;
 		case TEST_OP_EXEC:
 			arch_controlled_exec(vaddr);
 			GUEST_SYNC(stage++);
 			break;
+#ifdef __x86_64__
+		case TEST_OP_INVPLG:
+			asm volatile("invlpg (%0)"
+				     :: "b" (vaddr): "memory");
+			GUEST_SYNC(stage++);
+			break;
+#endif
 		default:
 			goto exit;
 		};
@@ -106,6 +121,21 @@ static void vcpu_run_and_inc_stage(struct kvm_vcpu *vcpu)
 	test_data->stage++;
 }
 
+static int test_page(struct kvm_vcpu *vcpu, int op, gva_t vaddr)
+{
+	int rc;
+
+	test_data->op = op;
+	test_data->vaddr = vaddr;
+
+	rc = _vcpu_run(vcpu);
+
+	if (rc >= 0)
+		test_data->stage++;
+
+	return rc < 0 ? -errno : rc;
+}
+
 static void test_page_restricted(struct kvm_vcpu *vcpu, int op,
 				 gva_t vaddr, gpa_t fault_paddr,
 				 uint64_t fault_reason)
@@ -122,7 +152,8 @@ static void test_page_restricted(struct kvm_vcpu *vcpu, int op,
 		    test_op_names[op], rc, errno);
 	TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_MEMORY_FAULT);
 	TEST_ASSERT_EQ(vcpu->run->memory_fault.gpa, fault_paddr);
-	TEST_ASSERT_EQ(vcpu->run->memory_fault.flags, fault_reason);
+	if (fault_reason)
+		TEST_ASSERT_EQ(vcpu->run->memory_fault.flags, fault_reason);
 	TEST_ASSERT_EQ(vcpu->run->memory_fault.size, vm->page_size);
 }
 
@@ -355,6 +386,9 @@ int main(int argc, char *argv[])
 	test_input_validation(vm);
 	test_memory_access(vcpu, test_mem, size);
 	test_memattrs_ignore_mmio(vcpu);
+#ifdef __x86_64__
+	arch_test_memory_access_pte(vcpu, test_mem);
+#endif
 	test_finalize(vcpu);
 
 	kvm_vm_free(vm);
diff --git a/tools/testing/selftests/kvm/x86/memory_attributes.c b/tools/testing/selftests/kvm/x86/memory_attributes.c
index 2e1148f5146d..3d2bcfe71884 100644
--- a/tools/testing/selftests/kvm/x86/memory_attributes.c
+++ b/tools/testing/selftests/kvm/x86/memory_attributes.c
@@ -41,3 +41,129 @@ void arch_write_return_insn(struct kvm_vm *vm, gpa_t vaddr)
 {
 	memset(addr_gpa2hva(vm, vaddr), 0xc3, 1);
 }
+
+/*
+ * This test validates that, during a page walk, if the page a PTE is placed in
+ * is read-only the accesss and dirty bits will not be written. Note There's a
+ * slight variation in behaviour between TDP and non-TDP VMs:
+ *  - With TDP enabled, KVM issues a fault exit upon observing the non-writable
+ *  page.
+ *  - With non-TDP, the access bit is not set, but the walk succeeds.
+ *
+ *  This is aligned with read-only memslots' behaviour.
+ */
+static void test_memory_access_pte_ro(struct kvm_vcpu *vcpu, gva_t vaddr)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gpa_t paddr;
+	u64 *pte;
+	const u64 accessed_mask = PTE_ACCESSED_MASK(&vm->mmu);
+
+	pte = vm_get_pte(vm, vaddr);
+	paddr = addr_hva2gpa(vm, pte) & GENMASK(61, vm->page_shift);
+
+	*pte &= ~accessed_mask;
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NW);
+	if (test_page(vcpu, TEST_OP_READ, vaddr) < 0) {
+		test_page_restricted(vcpu, TEST_OP_READ, vaddr, paddr,
+				     /* write PTE's accessed bit */
+				     KVM_MEMORY_EXIT_FLAG_WRITE);
+
+		vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+		test_page_accessible(vcpu, TEST_OP_READ, vaddr);
+		TEST_ASSERT_EQ(*pte & accessed_mask, accessed_mask);
+
+		/* Re-run the test, now vaddr is backed by an EPT. */
+		*pte &= ~accessed_mask;
+		vm_set_memory_attributes(vm, paddr, vm->page_size,
+					 KVM_MEMORY_ATTRIBUTE_NW);
+		test_page_restricted(vcpu, TEST_OP_READ, vaddr, paddr,
+				     /* write PTE's accessed bit */
+				     KVM_MEMORY_EXIT_FLAG_WRITE);
+		vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+		test_page_accessible(vcpu, TEST_OP_READ, vaddr);
+	} else {
+		TEST_ASSERT_EQ(*pte & accessed_mask, 0);
+		vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	}
+}
+
+/*
+ * This test validates that, during a page walk, if the page a PTE is placed in
+ * is maked as non-accesible, KVM issues a fault exit.
+ */
+static void test_memory_access_pte_nr(struct kvm_vcpu *vcpu, gva_t vaddr)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gpa_t paddr;
+	uint64_t *pte;
+
+	pte = vm_get_pte(vm, vaddr);
+	paddr = addr_hva2gpa(vm, pte) & GENMASK(61, vm->page_shift);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size,
+				 KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
+
+	test_page_restricted(vcpu, TEST_OP_READ, vaddr, paddr, 0);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	test_page_accessible(vcpu, TEST_OP_READ, vaddr);
+
+	/* Re-run the test, now vaddr is backed by an SPTE. */
+	vm_set_memory_attributes(vm, paddr, vm->page_size,
+				 KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
+	test_page_restricted(vcpu, TEST_OP_READ, vaddr, paddr, 0);
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	test_page_accessible(vcpu, TEST_OP_READ, vaddr);
+}
+
+static void test_memory_access_sync_spte(struct kvm_vcpu *vcpu, gva_t vaddr)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gpa_t paddr = addr_gva2gpa(vm, vaddr);
+	uint64_t *pte, old_pte, new_pte;
+
+	pte = vm_get_pte(vm, vaddr);
+	gpa_t pte_paddr = addr_hva2gpa(vm, pte);
+	gpa_t pte_page_paddr = pte_paddr & GENMASK(61, vm->page_shift);
+	int pte_offset = pte_paddr - pte_page_paddr;
+	virt_pg_map(vm, PTE_VADDR, pte_page_paddr);
+	old_pte = *pte;
+
+	/* Set vmaddr as non-executable */
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NX);
+
+	/*
+	 * Make sure SPTEs are populated as previous op might have destroyed
+	 * them. We new have a non-executable SPTE.
+	 */
+	test_page_accessible(vcpu, TEST_OP_READ, vaddr);
+
+	/*
+	 * Update PTE, make it non-writable and flush TLBs to make sure we go
+	 * through the sync_spte path. This should update the SPTE and make it
+	 * read-only.
+	 */
+	new_pte = (old_pte & ~PT_WRITABLE_MASK) | PT_ACCESSED_MASK;
+	test_data->expected_val = new_pte;
+	test_page_accessible(vcpu, TEST_OP_WRITE, PTE_VADDR + pte_offset);
+	TEST_ASSERT_EQ(*pte, new_pte);
+	test_page_accessible(vcpu, TEST_OP_INVPLG, vaddr);
+
+	/* The not executable attrs remain valid */
+	arch_write_return_insn(vm, vaddr);
+	test_page_restricted(vcpu, TEST_OP_EXEC, vaddr, paddr,
+			     KVM_MEMORY_EXIT_FLAG_EXEC);
+
+	/* Cleanup */
+	*pte = old_pte;
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	test_page_accessible(vcpu, TEST_OP_EXEC, vaddr);
+}
+
+static void arch_test_memory_access_pte(struct kvm_vcpu *vcpu, gva_t vaddr)
+{
+	test_memory_access_pte_nr(vcpu, vaddr);
+	test_memory_access_pte_ro(vcpu, vaddr);
+	test_memory_access_sync_spte(vcpu, vaddr);
+}
-- 
2.52.0

[PATCH 24/24] KVM: x86: selftests: Introduce memory attributes side-channel tests

[Paolo Bonzini]

From: Nicolas Saenz Julienne <nsaenz@amazon.com>

Introduce memory attributes selftests to catch any vulnerable
side-channels.

Memory attributes access restrictions are vulnerable to side-channel
attacks. This means that any KVM operation initiated by the guest that
requires guest memory access (which is the case for most
para-virtualised interfaces) needs to consider memory attributes.

The tests confirm this requirement is upheld for a variety of use-cases,
and were selected to exercise specific approaches to guest memory
access, including:
 - kvm_read/write_guest()
 - gfn_to_hva_cache
 - gfn_to_pfn_cache
 - Guest page walker (which uses __try_cmpxchg_user())

Signed-off-by: Nicolas Saenz Julienne <nsaenz@amazon.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .../testing/selftests/kvm/include/kvm_util.h  |  10 +
 .../selftests/kvm/include/x86/processor.h     |   1 +
 .../testing/selftests/kvm/lib/x86/processor.c |   5 +
 .../testing/selftests/kvm/memory_attributes.c |  63 ++++-
 .../selftests/kvm/x86/memory_attributes.c     | 216 ++++++++++++++++++
 5 files changed, 294 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 54d73ea46e11..3160a25966a1 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -883,6 +883,16 @@ static inline int vcpu_get_stats_fd(struct kvm_vcpu *vcpu)
 	return fd;
 }
 
+static inline struct kvm_translation vcpu_translate(struct kvm_vcpu *vcpu,
+						    u64 gva)
+{
+	struct kvm_translation tr;
+
+	tr.linear_address = gva;
+	vcpu_ioctl(vcpu, KVM_TRANSLATE, &tr);
+	return tr;
+}
+
 int __kvm_has_device_attr(int dev_fd, u32 group, u64 attr);
 
 static inline void kvm_has_device_attr(int dev_fd, u32 group, u64 attr)
diff --git a/tools/testing/selftests/kvm/include/x86/processor.h b/tools/testing/selftests/kvm/include/x86/processor.h
index 7d3a27bc0d84..d9876377968f 100644
--- a/tools/testing/selftests/kvm/include/x86/processor.h
+++ b/tools/testing/selftests/kvm/include/x86/processor.h
@@ -1422,6 +1422,7 @@ static inline bool kvm_is_lbrv_enabled(void)
 	return !!get_kvm_amd_param_integer("lbrv");
 }
 
+u64 *vm_get_pte_level(struct kvm_vm *vm, gva_t gva, int *level);
 u64 *vm_get_pte(struct kvm_vm *vm, gva_t gva);
 
 u64 kvm_hypercall(u64 nr, u64 a0, u64 a1, u64 a2, u64 a3);
diff --git a/tools/testing/selftests/kvm/lib/x86/processor.c b/tools/testing/selftests/kvm/lib/x86/processor.c
index ef56dcefe011..8b990016e764 100644
--- a/tools/testing/selftests/kvm/lib/x86/processor.c
+++ b/tools/testing/selftests/kvm/lib/x86/processor.c
@@ -397,6 +397,11 @@ u64 *tdp_get_pte(struct kvm_vm *vm, u64 l2_gpa)
 	return __vm_get_page_table_entry(vm, &vm->stage2_mmu, l2_gpa, &level);
 }
 
+u64 *vm_get_pte_level(struct kvm_vm *vm, gva_t gva, int *level)
+{
+	return __vm_get_page_table_entry(vm, &vm->mmu, gva, level);
+}
+
 u64 *vm_get_pte(struct kvm_vm *vm, gva_t gva)
 {
 	int level = PG_LEVEL_4K;
diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
index c66d5d085a9c..843fafcf4d96 100644
--- a/tools/testing/selftests/kvm/memory_attributes.c
+++ b/tools/testing/selftests/kvm/memory_attributes.c
@@ -19,6 +19,8 @@
 	(KVM_MEMORY_ATTRIBUTE_NR | KVM_MEMORY_ATTRIBUTE_NW | \
 	 KVM_MEMORY_ATTRIBUTE_NX)
 
+#define HV_STATUS_INVALID_HYPERCALL_INPUT	3
+
 #define MMIO_GPA	0x700000000
 #define MMIO_GVA	MMIO_GPA
 
@@ -26,12 +28,35 @@
 #define PT_ACCESSED_MASK	BIT_ULL(5)
 #define PTE_VADDR		0x1000000000
 
+static volatile uint64_t ipis_rcvd;
+
+static pthread_t vcpu_thread;
+
+struct hv_vpset {
+	u64 format;
+	u64 valid_bank_mask;
+	u64 bank_contents[2];
+};
+
+enum HV_GENERIC_SET_FORMAT {
+	HV_GENERIC_SET_SPARSE_4K,
+	HV_GENERIC_SET_ALL,
+};
+
+struct hv_send_ipi_ex {
+	u32 vector;
+	u32 reserved;
+	struct hv_vpset vp_set;
+};
+
 enum {
 	TEST_OP_NOP,
 	TEST_OP_READ,
 	TEST_OP_WRITE,
 	TEST_OP_EXEC,
 	TEST_OP_INVPLG,
+	TEST_OP_HYPERV_HYPERCALL_INPUT,
+	TEST_OP_MONITOR_ADDRESS,
 	TEST_OP_EXIT,
 };
 
@@ -41,6 +66,8 @@ const char *test_op_names[] =
 	[TEST_OP_WRITE] = "Write",
 	[TEST_OP_EXEC] = "Exec",
 	[TEST_OP_INVPLG] = "Invplg",
+	[TEST_OP_HYPERV_HYPERCALL_INPUT] = "HvHcall input",
+	[TEST_OP_MONITOR_ADDRESS] = "Monitor address",
 	[TEST_OP_EXIT] = "Exit",
 };
 
@@ -48,6 +75,8 @@ struct test_data {
 	uint8_t op;
 	int stage;
 	gva_t vaddr;
+	gpa_t paddr;
+	uint8_t confirm_read;
 	uint64_t expected_val;
 
 	struct kvm_vcpu *vcpu;
@@ -59,19 +88,27 @@ static uint64_t arch_controlled_read(gva_t addr);
 static void arch_controlled_write(gva_t addr, uint64_t val);
 static void arch_controlled_exec(gva_t addr);
 static void arch_write_return_insn(struct kvm_vm *vm, gpa_t vaddr);
+static void arch_guest_init(void);
 
 static void guest_code(void *data)
 {
 	struct test_data *test_data = data;
 	int stage = 1;
 
+	arch_guest_init();
+
 	while (true) {
 		uint64_t expected_val = READ_ONCE(test_data->expected_val);
+		bool confirm_read = READ_ONCE(test_data->confirm_read);
 		gva_t vaddr = READ_ONCE(test_data->vaddr);
+		gpa_t paddr = READ_ONCE(test_data->paddr);
+		u64 val;
 
 		switch(READ_ONCE(test_data->op)) {
 		case TEST_OP_READ:
-			(void) arch_controlled_read(vaddr);
+			val = arch_controlled_read(vaddr);
+			if (confirm_read)
+				GUEST_ASSERT_EQ(expected_val, val);
 			GUEST_SYNC(stage++);
 			break;
 		case TEST_OP_WRITE:
@@ -88,6 +125,25 @@ static void guest_code(void *data)
 				     :: "b" (vaddr): "memory");
 			GUEST_SYNC(stage++);
 			break;
+		case TEST_OP_HYPERV_HYPERCALL_INPUT: {
+			hyperv_hypercall(HVCALL_SEND_IPI_EX, paddr, 0);
+			asm volatile ("sti; hlt; cli;");
+			GUEST_ASSERT_EQ(ipis_rcvd, 1);
+			GUEST_SYNC(stage++);
+			break;
+		}
+		case TEST_OP_MONITOR_ADDRESS: {
+			uint64_t *pval = (uint64_t *)vaddr;
+			uint64_t val = READ_ONCE(*pval);
+
+			while (READ_ONCE(*pval) == val &&
+			       /* So host can force the op out of the loop */
+			       READ_ONCE(test_data->op) == TEST_OP_MONITOR_ADDRESS)
+				asm volatile("nop");
+
+			GUEST_SYNC(stage++);
+			break;
+		}
 #endif
 		default:
 			goto exit;
@@ -145,6 +201,7 @@ static void test_page_restricted(struct kvm_vcpu *vcpu, int op,
 
 	test_data->op = op;
 	test_data->vaddr = vaddr;
+	test_data->paddr = fault_paddr;
 
 	rc = _vcpu_run(vcpu);
 	TEST_ASSERT(rc == -1 && errno == EFAULT,
@@ -382,12 +439,16 @@ int main(int argc, char *argv[])
 	test_mem = vm_alloc(vm, size, KVM_UTIL_MIN_VADDR);
 	virt_map(vcpu->vm, MMIO_GVA, MMIO_GPA, 1);
 	test_data = init_test_data(vcpu);
+#ifdef __x86_64__
+	vcpu_set_hv_cpuid(vcpu);
+#endif
 
 	test_input_validation(vm);
 	test_memory_access(vcpu, test_mem, size);
 	test_memattrs_ignore_mmio(vcpu);
 #ifdef __x86_64__
 	arch_test_memory_access_pte(vcpu, test_mem);
+	arch_test_side_channels(vcpu, test_mem, size);
 #endif
 	test_finalize(vcpu);
 
diff --git a/tools/testing/selftests/kvm/x86/memory_attributes.c b/tools/testing/selftests/kvm/x86/memory_attributes.c
index 3d2bcfe71884..5203d284d2d3 100644
--- a/tools/testing/selftests/kvm/x86/memory_attributes.c
+++ b/tools/testing/selftests/kvm/x86/memory_attributes.c
@@ -42,6 +42,11 @@ void arch_write_return_insn(struct kvm_vm *vm, gpa_t vaddr)
 	memset(addr_gpa2hva(vm, vaddr), 0xc3, 1);
 }
 
+void arch_guest_init(void)
+{
+	x2apic_enable();
+}
+
 /*
  * This test validates that, during a page walk, if the page a PTE is placed in
  * is read-only the accesss and dirty bits will not be written. Note There's a
@@ -167,3 +172,214 @@ static void arch_test_memory_access_pte(struct kvm_vcpu *vcpu, gva_t vaddr)
 	test_memory_access_pte_ro(vcpu, vaddr);
 	test_memory_access_sync_spte(vcpu, vaddr);
 }
+
+#define IPI_VECTOR	 0xfe
+
+static void guest_ipi_handler_hv(struct ex_regs *regs)
+{
+	ipis_rcvd++;
+	wrmsr(HV_X64_MSR_EOI, 1);
+}
+
+/*
+ * This test verifies that the Hyper-V hypercall exit handler takes memory
+ * attributes into account before accessing input data. It coordinates with the
+ * guest through the 'TEST_OP_HYPERV_HYPERCALL_INPUT' operation and instructs
+ * the guest to issue two PV IPIs. The first PV IPI fails because the input
+ * data is held in read-protected memory. Subsequently, the memory protection
+ * is lifted, and the second PV IPI succeeds.
+ */
+static void test_side_channel_hyperv_hypercall_inputs(struct kvm_vcpu *vcpu,
+						      gva_t vaddr,
+						      size_t size)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	struct hv_send_ipi_ex *ipi_ex = addr_gva2hva(vm, vaddr);
+	gpa_t paddr = addr_gva2gpa(vm, test_data->vaddr);
+
+	if (!kvm_has_cap(KVM_CAP_HYPERV_SEND_IPI) ||
+	    !kvm_has_cap(KVM_CAP_HYPERV_HCALL_FAULT_EXIT))
+		return;
+
+	vm_enable_cap(vcpu->vm, KVM_CAP_HYPERV_HCALL_FAULT_EXIT, 1);
+
+	ipis_rcvd = 0;
+	vm_install_exception_handler(vm, IPI_VECTOR, guest_ipi_handler_hv);
+	vcpu_set_msr(vcpu, HV_X64_MSR_GUEST_OS_ID, HYPERV_LINUX_OS_ID);
+
+	ipi_ex->vector = IPI_VECTOR;
+	ipi_ex->vp_set.format = HV_GENERIC_SET_ALL;
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
+	test_page_restricted(vcpu, TEST_OP_HYPERV_HYPERCALL_INPUT, vaddr, paddr,
+			     KVM_MEMORY_EXIT_FLAG_READ);
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	vcpu_run_and_inc_stage(vcpu);
+}
+
+/*
+ * Verifies that the guest page table walker fails the walk if it encounters a
+ * page table entry address read-protected by a memory attribute.
+ */
+static void test_side_channel_emul_page_walks(struct kvm_vcpu *vcpu,
+					      gva_t test_vm_addr,
+					      size_t size)
+{
+	const uint64_t pte_addr_mask = GENMASK(51, 12);
+	struct kvm_vm *vm = vcpu->vm;
+	struct kvm_translation tr;
+	int level = PG_LEVEL_1G;
+	gpa_t paddr;
+	uint64_t *pte;
+
+	pte = vm_get_pte_level(vm, test_vm_addr, &level);
+	TEST_ASSERT_EQ(level, PG_LEVEL_1G);
+	paddr = *pte & pte_addr_mask;
+
+	tr = vcpu_translate(vcpu, test_vm_addr);
+	TEST_ASSERT_EQ(tr.valid, true);
+	TEST_ASSERT_EQ(tr.physical_address, addr_gva2gpa(vm, test_vm_addr));
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
+	tr = vcpu_translate(vcpu, test_vm_addr);
+	TEST_ASSERT_EQ(tr.valid, false);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+}
+
+static void vm_set_vapic_addr(struct kvm_vcpu *vcpu, uint64_t addr)
+{
+	struct kvm_vapic_addr va;
+
+	va.vapic_addr = addr;
+	vcpu_ioctl(vcpu, KVM_SET_VAPIC_ADDR, &va);
+}
+
+/*
+ * Perform a dummy regs update to issue a KVM_REQ_EVENT. This forces
+ * vapic to be synced before entering the guest.
+ */
+static void vcpu_force_vapic_update(struct kvm_vcpu *vcpu)
+{
+	struct kvm_regs regs;
+
+	vcpu_regs_get(vcpu, &regs);
+	vcpu_regs_set(vcpu, &regs);
+}
+
+/*
+ * Setup the vapic address on a GPA that is write-protected. Force an vapic
+ * update and validate its contents were not changes. Then, lift the write
+ * restriction and validate the page's contents are updated.
+ */
+static void test_side_channel_vapic_addr(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gva_t vaddr = vm_alloc_page(vm);
+	gpa_t paddr = addr_gva2gpa(vm, vaddr);
+
+	vm_set_vapic_addr(vcpu, paddr);
+	test_data->op = TEST_OP_READ;
+	test_data->vaddr = vaddr;
+	test_data->confirm_read = 1;
+	test_data->expected_val = ~0ULL >> 32;
+	memset(addr_gva2hva(vm, vaddr), 0xff, sizeof(uint32_t));
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NW);
+	vcpu_run_and_inc_stage(vcpu);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	vcpu_force_vapic_update(vcpu);
+	test_data->expected_val = 0ULL;
+	vcpu_run_and_inc_stage(vcpu);
+
+	vm_set_vapic_addr(vcpu, 0);
+	test_data->confirm_read = 0;
+}
+
+static void *vcpu_worker(void *data)
+{
+	struct test_data *test_data = data;
+	struct kvm_vcpu *vcpu = test_data->vcpu;
+
+	vcpu_run_and_inc_stage(vcpu);
+
+	return NULL;
+}
+
+/*
+ * Set up the pvclock page and validate that KVM periodically updates the
+ * 'tsc_timestamp' field. Subsequently, make the pvclock page non-writable and
+ * verify that the 'tsc_timestamp' field is no longer updated.
+ */
+static void test_side_channel_pvclock(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gva_t vaddr = vm_alloc_page(vm);
+	gpa_t paddr = addr_gva2gpa(vm, vaddr);
+	vcpu_set_msr(vcpu, MSR_KVM_SYSTEM_TIME_NEW, paddr | 0x1);
+
+	test_data->op = TEST_OP_MONITOR_ADDRESS;
+	test_data->vaddr = vaddr + offsetof(struct pvclock_vcpu_time_info, tsc_timestamp);
+
+	pthread_create(&vcpu_thread, NULL, vcpu_worker, test_data);
+	usleep(msecs_to_usecs(1000));
+	TEST_ASSERT_EQ(pthread_tryjoin_np(vcpu_thread, NULL), 0);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NW);
+	pthread_create(&vcpu_thread, NULL, vcpu_worker, test_data);
+	usleep(msecs_to_usecs(1000));
+	TEST_ASSERT_EQ(pthread_tryjoin_np(vcpu_thread, NULL), EBUSY);
+
+	/* Force the 'monitor_address' guest operation to finish */
+	test_data->op = TEST_OP_NOP;
+	TEST_ASSERT_EQ(pthread_join(vcpu_thread, NULL), 0);
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+	vcpu_set_msr(vcpu, MSR_KVM_SYSTEM_TIME_NEW, 0);
+}
+
+/*
+ * Write to MSR_KVM_WALL_CLOCK_NEW and verify that the struct's 'version' field
+ * is updated. Subsequently, make the target guest physical address
+ * non-writable, and verify the 'version' field isn't updated anymore.
+ */
+static void test_side_channel_wallclock(struct kvm_vcpu *vcpu)
+{
+	struct kvm_vm *vm = vcpu->vm;
+	gva_t vaddr = vm_alloc_page(vm);
+	gpa_t paddr = addr_gva2gpa(vm, vaddr);
+	struct pvclock_wall_clock *wc = addr_gva2hva(vm, vaddr);
+
+	wc->version = 0x0;
+	vcpu_set_msr(vcpu, MSR_KVM_WALL_CLOCK_NEW, paddr);
+	TEST_ASSERT_EQ(wc->version, 2);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NW);
+	vcpu_set_msr(vcpu, MSR_KVM_WALL_CLOCK_NEW, paddr);
+	TEST_ASSERT_EQ(wc->version, 2);
+
+	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
+}
+
+/*
+ * Memory attributes are vulnerable to side-channel attacks. This means that
+ * any KVM operation initiated by the guest that requires guest memory access
+ * (which is the case for most pv-interfaces) needs to consider memory
+ * attributes.
+ *
+ * The following tests validate that this requirement is upheld for a variety
+ * of use-cases. These test cases were selected to exercise specific approaches
+ * to accessing guest memory, including:
+ *
+ *  - kvm_read/write_guest()
+ *  - gfn_to_hva_cache
+ *  - gfn_to_pfn_cache
+ *  - Guest page walker
+ */
+static void arch_test_side_channels(struct kvm_vcpu *vcpu, gva_t test_vm_addr,
+			            size_t size)
+{
+	test_side_channel_hyperv_hypercall_inputs(vcpu, test_vm_addr, size);
+	test_side_channel_emul_page_walks(vcpu, test_vm_addr, size);
+	test_side_channel_vapic_addr(vcpu);
+	test_side_channel_wallclock(vcpu);
+	test_side_channel_pvclock(vcpu);
+}
-- 
2.52.0

Re: [PATCH 13/24] KVM: Introduce kvm_check_gen()/kvm_memslots_check_gen()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The refactoring to use the new `kvm_check_gen()` helper was not applied to `arch/loongarch/kvm/vcpu.c`.
--

--- Patch [13]: [PATCH 13/24] KVM: Introduce kvm_check_gen()/kvm_memslots_check_gen() ---
commit 80a164bf326fe8a6112aa42a54b4beddb26e73c6
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: Introduce kvm_check_gen()/kvm_memslots_check_gen()

This commit introduces kvm_check_gen() and kvm_memslots_check_gen() helper
functions to combine retrieving kvm_memslots and checking the generation of
the slots. It refactors several sites in x86 and core KVM code to use the
new helpers.

> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index e2240f817c22e..58f544df47e14 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2032,7 +2032,6 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
[ ... ]
> -	slots = kvm_memslots(vcpu->kvm);
> -
> -	if (unlikely(slots->generation != ghc->generation ||
> +	if (unlikely(!kvm_check_gen(vcpu->kvm, ghc->generation) ||
>  		     gpa != ghc->gpa ||
>  		     kvm_is_error_hva(ghc->hva) || !ghc->memslot)) {

[Severity: Low]
Should arch/loongarch/kvm/vcpu.c also be updated to use this new helper?

It looks like kvm_update_stolen_time() and kvm_vcpu_set_pv_preempted() in
that file still use the old pattern:

arch/loongarch/kvm/vcpu.c:kvm_update_stolen_time() {
    ...
    slots = kvm_memslots(vcpu->kvm);
    if (slots->generation != ghc->generation || gpa != ghc->gpa) {
    ...
}

Does LoongArch need similar updates to maintain consistency across the
codebase?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=13

Re: [PATCH 10/24] KVM: Include memory protections in result of gfn->hva conversion

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional NULL pointer dereference in gfn_to_hva_prot() and kvm_vcpu_gfn_to_hva_prot() when applying memory attributes.
--

--- Patch [10]: [PATCH 10/24] KVM: Include memory protections in result of gfn->hva conversion ---
commit bca8d72704eb8c45ae205232d29cab272cbba365
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: Include memory protections in result of gfn->hva conversion

This commit adds memory attribute checks to paths leading to guest memory
accesses. Functions like gfn_to_hva and kvm_vcpu_gfn_to_hva now return error
values based on memory attributes, though cached variants will need separate
changes.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
[ ... ]
> @@ -2796,15 +2821,37 @@ unsigned long gfn_to_hva_memslot_prot(struct kvm_memory_slot *slot,
>  unsigned long gfn_to_hva_prot(struct kvm *kvm, gfn_t gfn, bool *writable)
>  {
>  	struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> +	unsigned long addr;
> +
> +	addr = gfn_to_hva_memslot_prot(slot, gfn, writable);
> +	if (kvm_is_error_hva(addr))
> +		return addr;
> +
> +	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
> +		return KVM_HVA_ERR_BAD;
>  
> -	return gfn_to_hva_memslot_prot(slot, gfn, writable);
> +	if (!kvm_mem_attributes_may_write_gfn(kvm, gfn))
> +		*writable = false;

[Severity: High]
Does this code unconditionally dereference the writable pointer?

If a caller invokes gfn_to_hva_prot() with a NULL writable pointer, and the
GFN has memory attributes that restrict write access, will this crash the
kernel? The KVM API allows the writable pointer to be NULL.

> +
> +	return addr;
>  }
>  
>  unsigned long kvm_vcpu_gfn_to_hva_prot(struct kvm_vcpu *vcpu, gfn_t gfn, bool *writable)
>  {
>  	struct kvm_memory_slot *slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn);
> +	unsigned long addr;
> +
> +	addr = gfn_to_hva_memslot_prot(slot, gfn, writable);
> +	if (kvm_is_error_hva(addr))
> +		return addr;
> +
> +	if (!kvm_mem_attributes_may_read_gfn(vcpu->kvm, gfn))
> +		return KVM_HVA_ERR_BAD;
> +
> +	if (!kvm_mem_attributes_may_write_gfn(vcpu->kvm, gfn))
> +		*writable = false;

[Severity: High]
Similar to the above, can this cause a NULL pointer dereference if
kvm_vcpu_gfn_to_hva_prot() is called with a NULL writable pointer?

>  
> -	return gfn_to_hva_memslot_prot(slot, gfn, writable);
> +	return addr;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=10

Re: [PATCH 02/24] KVM: Define and communicate KVM_EXIT_MEMORY_FAULT RWX flags to userspace

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Low] The newly introduced KVM_MEMORY_EXIT_FLAG_* UAPI flags are not synced to the tools/ directory.

Pre-existing issues:
- [High] ARM64 page fault handlers incorrectly report guest_memfd memory faults as shared by passing `is_private=false` to `kvm_prepare_memory_fault_exit`.
--

--- Patch [2]: [PATCH 02/24] KVM: Define and communicate KVM_EXIT_MEMORY_FAULT RWX flags to userspace ---
commit 2373c31f86321f944cc2fad5b744fcd833b349a9
Author: Anish Moorthy <amoorthy@google.com>

KVM: Define and communicate KVM_EXIT_MEMORY_FAULT RWX flags to userspace

kvm_prepare_memory_fault_exit() already takes parameters describing the
RWX-ness of the relevant access but doesn't actually do anything with
them. Define and use the flags necessary to pass this information on to
userspace.

Link: https://lore.kernel.org/kvm/ZR4N8cwzTMDanPUY@google.com/

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index ab8cfaec82d31..2278b17f2f289 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
[ ... ]
> @@ -2519,8 +2519,15 @@ static inline void kvm_prepare_memory_fault_exit(struct kvm_vcpu *vcpu,
>  	vcpu->run->memory_fault.gpa = gpa;
>  	vcpu->run->memory_fault.size = size;
>  
> -	/* RWX flags are not (yet) defined or communicated to userspace. */
>  	vcpu->run->memory_fault.flags = 0;
> +
> +	if (is_write)
> +		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_WRITE;
> +	else if (is_exec)
> +		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_EXEC;
> +	else
> +		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_READ;
> +
>  	if (is_private)
>  		vcpu->run->memory_fault.flags |= KVM_MEMORY_EXIT_FLAG_PRIVATE;

[Severity: High]
This isn't a regression introduced by this patch, but does the ARM64 code
pass the incorrect is_private value for guest_memfd faults?

Looking at arch/arm64/kvm/mmu.c:gmem_abort(), KVM calls this with false for
a failure originating from kvm_gmem_get_pfn() on a private memory slot:

		kvm_prepare_memory_fault_exit(s2fd->vcpu, s2fd->fault_ipa, PAGE_SIZE,
					      write_fault, exec_fault, false);

Similarly in arch/arm64/kvm/nested.c:kvm_translate_vncr():

			kvm_prepare_memory_fault_exit(vcpu, vt->wr.pa, PAGE_SIZE,
					      write_fault, false, false);

Will this cause userspace to receive a KVM_EXIT_MEMORY_FAULT for a private
memory access with the KVM_MEMORY_EXIT_FLAG_PRIVATE bit incorrectly cleared?

>  }
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 419011097fa8e..720b1ffe880b2 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -457,6 +457,9 @@ struct kvm_run {
>  		} notify;
>  		/* KVM_EXIT_MEMORY_FAULT */
>  		struct {
> +#define KVM_MEMORY_EXIT_FLAG_READ       (1ULL << 0)
> +#define KVM_MEMORY_EXIT_FLAG_WRITE      (1ULL << 1)
> +#define KVM_MEMORY_EXIT_FLAG_EXEC       (1ULL << 2)

[Severity: Low]
Are these new flags missing from tools/include/uapi/linux/kvm.h?

Could this regression cause compilation errors or missing macros for tools
that rely on the in-tree tools/ copy of the UAPI headers?

>  #define KVM_MEMORY_EXIT_FLAG_PRIVATE	(1ULL << 3)
>  			__u64 flags;
>  			__u64 gpa;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=2

Re: [PATCH 03/24] KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Unconditionally validates the input parameter GPA (`hc.ingpa`) for all slow hypercalls, causing Guest DoS for hypercalls without input parameters.
- [High] Improperly enforces write access on the input parameter block (`hc.ingpa`) and fails to validate the output parameter block (`hc.outgpa`).
- [High] Treats untranslated L2 GPAs as L1 GPAs, allowing an L2 guest to trivially DoS the L1 VMM.
- [Medium] Reports an inaccurate fault range to userspace by using an unaligned GPA and a hardcoded PAGE_SIZE.
- [Medium] Unsynchronized mutability of the `KVM_CAP_HYPERV_HCALL_FAULT_EXIT` capability violates KVM guidelines.
--

--- Patch [3]: [PATCH 03/24] KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas ---
commit 2ee0ff5e023dcf16d5e8bf3cfc26d7ec91c1d297
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86: hyperv: Introduce memory fault on hcalls with bad ingpas

This commit introduces a new capability, KVM_CAP_HYPERV_HCALL_FAULT_EXIT,
which allows KVM to exit to userspace with a memory fault when a guest
provides an invalid input GPA during a hypercall.

> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
[ ... ]
> @@ -2531,11 +2531,30 @@ static bool hv_check_hypercall_access(struct kvm_vcpu_hv *hv_vcpu, u16 code)
>  	return true;
>  }
>  
> +static unsigned int kvm_hv_hypercall_mem_access(u16 code)
> +{
> +	switch (code) {
> +	case HVCALL_SIGNAL_EVENT:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_LIST_EX:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE:
> +	case HVCALL_FLUSH_VIRTUAL_ADDRESS_SPACE_EX:
> +	case HVCALL_SEND_IPI:
> +	case HVCALL_SEND_IPI_EX:
> +		return KVM_MEMORY_EXIT_FLAG_READ;
> +	}
> +
> +	return KVM_MEMORY_EXIT_FLAG_WRITE;

[Severity: High]
Does this default to requiring write access for all unlisted hypercalls?

If a guest issues an unlisted hypercall (e.g., HVCALL_POST_MESSAGE) with its
input parameter block in read-only memory, this would return
KVM_MEMORY_EXIT_FLAG_WRITE, causing KVM to incorrectly demand write access
and reject a valid read-only buffer with a memory fault.

> +}
> +
>  int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_vcpu_hv *hv_vcpu = to_hv_vcpu(vcpu);
> +	struct kvm *kvm = vcpu->kvm;
>  	struct kvm_hv_hcall hc;
>  	u64 ret = HV_STATUS_SUCCESS;
> +	unsigned int access;
> +	unsigned long addr;
>  
>  	/*
>  	 * hypercall generates UD from non zero cpl and real mode
> @@ -2590,6 +2609,19 @@ int kvm_hv_hypercall(struct kvm_vcpu *vcpu)
>  		kvm_hv_hypercall_read_xmm(&hc);
>  	}
>  
> +	if (!hc.fast && kvm->arch.hyperv.hcall_fault_exit) {
> +		bool writable = true;
> +		access = kvm_hv_hypercall_mem_access(hc.code);
> +		addr = kvm_vcpu_gfn_to_hva_prot(vcpu, gpa_to_gfn(hc.ingpa), &writable);

[Severity: High]
Does this code unconditionally validate hc.ingpa for all slow hypercalls?

If a guest invokes a hypercall that does not take an input parameter
(like HVCALL_NOTIFY_LONG_SPIN_WAIT), its parameter registers may be
uninitialized. If the uninitialized register contains an unmapped GPA,
wouldn't KVM inject a KVM_EXIT_MEMORY_FAULT and return -EFAULT, causing
userspace to terminate the VM?

[Severity: High]
Are untranslated L2 GPAs treated as L1 GPAs here?

When an L2 guest executes a hypercall, KVM reads hc.ingpa and blindly passes
it to kvm_vcpu_gfn_to_hva_prot() without translating it or checking
is_guest_mode().

If an L2 guest passes an invalid L1 GPA, this could return -EFAULT and
trigger a KVM_EXIT_MEMORY_FAULT to L0 userspace, terminating the entire
L1 VM. Could this allow an L2 guest to trivially crash L1?

> +		if (addr == KVM_HVA_ERR_BAD ||
> +		    (access == KVM_MEMORY_EXIT_FLAG_WRITE && !writable)) {

[Severity: High]
Is the output parameter block (hc.outgpa) validated anywhere in this new
mechanism?

It looks like we enforce the write access requirement on the input
parameter GPA, but completely omit validation for hc.outgpa. Hypercalls
writing to unmapped memory might bypass the memory fault mechanism entirely.

> +			kvm_prepare_memory_fault_exit(vcpu, hc.ingpa, PAGE_SIZE,
> +						      access == KVM_MEMORY_EXIT_FLAG_WRITE,
> +						      false, false);

[Severity: Medium]
Could this report an inaccurate fault range to userspace?

We are preparing a memory fault exit using the exact unaligned address
provided by the guest (hc.ingpa) but with a hardcoded size of PAGE_SIZE. If
hc.ingpa is near a page boundary, the reported fault range would span two
pages, even though we only validated a single GFN for hc.ingpa.

> +			return -EFAULT;
> +		}
> +	}
> +
>  	switch (hc.code) {
>  	case HVCALL_NOTIFY_LONG_SPIN_WAIT:
[ ... ]
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
[ ... ]
> @@ -4190,6 +4191,12 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>  		mutex_unlock(&kvm->lock);
>  		break;
>  	}
> +#ifdef CONFIG_KVM_HYPERV
> +	case KVM_CAP_HYPERV_HCALL_FAULT_EXIT:
> +		kvm->arch.hyperv.hcall_fault_exit = cap->args[0];

[Severity: Medium]
Should this capability state be protected by kvm->lock and restricted to
before vCPUs are created?

Userspace can toggle the capability dynamically, and modifying this flag
without checks creates state-machine transitions that could race with actively
executing hypercalls. New flags that change VM or vCPU behavior typically
default to immutable once vCPUs are running.

> +		r = 0;
> +		break;
> +#endif
>  	default:
>  		r = -EINVAL;
>  		break;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=3

Re: [PATCH 20/24] KVM: x86/mmu: Do not prefetch sptes on gfns backed by memory attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Skipping SPTE prefetch for pages with memory attributes leaks the `struct page` reference.
--

--- Patch [20]: [PATCH 20/24] KVM: x86/mmu: Do not prefetch sptes on gfns backed by memory attributes ---
commit f42ac0f993cf93f62265ecaa532948b8790213ce
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86/mmu: Do not prefetch sptes on gfns backed by memory attributes
    
This commit modifies KVM's MMU prefetching to check for memory attributes on
each GFN. If memory protections are applied to a GFN, the code skips
prefetching the SPTE for that page.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 59c2a04f5648c..8e85f672a08d5 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3156,6 +3156,9 @@ static bool kvm_mmu_prefetch_sptes(struct kvm_vcpu *vcpu, gfn_t gfn, u64 *sptep,
>  		return false;
>  
>  	for (i = 0; i < nr_pages; i++, gfn++, sptep++) {
> +		if (kvm_get_memory_attributes(vcpu->kvm, gfn))
> +			continue;
> +

[Severity: High]
Does this leak the struct page reference?

Earlier in kvm_mmu_prefetch_sptes(), kvm_prefetch_pages() populates the
pages[] array, which elevates the reference count for each page. By using
continue here, the loop skips over the required cleanup call at the bottom
of the loop:

    kvm_release_page_dirty(pages[i]);

Should this code release the page before continuing to the next iteration?

>  		mmu_set_spte(vcpu, slot, sptep, access, gfn,
>  			     page_to_pfn(pages[i]), NULL);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=20

Re: [PATCH 11/24] KVM: Take memory protections into account in kvm_read/write_guest()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Cached guest memory accesses bypass memory attribute protections.
--

--- Patch [11]: [PATCH 11/24] KVM: Take memory protections into account in kvm_read/write_guest() ---
commit 2db3d9f175080ff067377dd7b0148b9955953e26
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: Take memory protections into account in kvm_read/write_guest()
    
This patch adds memory attribute checks to the kvm_read_guest() and
kvm_write_guest() family of functions. It plumbs the kvm struct to the
internal page accessors so they can query memory attributes. If the
attributes do not permit the access, the functions return an error.

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 56016aab0aad3..d920ce5b67392 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c

[ ... ]

> @@ -3261,6 +3261,10 @@ static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn,
>  	addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
>  	if (kvm_is_error_hva(addr))
>  		return -EFAULT;
> +
> +	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
> +		return -EFAULT;
> +

[Severity: Critical]
Since memory attributes are now enforced for standard guest reads and writes,
do the cached memory accessors also need to enforce these protections?

Functions like kvm_read_guest_offset_cached() and
kvm_write_guest_offset_cached() appear to read and write directly using the
cached HVA without verifying attributes:

virt/kvm/kvm_main.c:kvm_read_guest_offset_cached() {
    ...
    if (kvm_is_error_hva(ghc->hva))
        return -EFAULT;
    ...
    r = __copy_from_user(data, (void __user *)ghc->hva + offset, len);
    ...
}

Similarly, host kernel mapping paths like __kvm_vcpu_map() use
kvm_follow_pfn() and map guest memory without checking attributes:

virt/kvm/kvm_main.c:__kvm_vcpu_map() {
    struct kvm_follow_pfn kfp = {
        .slot = kvm_vcpu_gfn_to_memslot(vcpu, gfn),
        .gfn = gfn,
        ...
    };
    ...
    map->pfn = kvm_follow_pfn(&kfp);
    ...
}

And gpc resolving paths like hva_to_pfn_retry() in virt/kvm/pfncache.c also
rely on kvm_follow_pfn() directly.

Could a malicious guest bypass memory attribute protections (e.g., read/write
restrictions or private memory boundaries) by pointing a cached memory
structure, such as a virtio ring or APIC, to restricted memory?

Also, if memory attributes are dynamically updated, how do we ensure
gfn_to_hva_cache and gfn_to_pfn_cache are properly invalidated, given that
attribute updates don't seem to bump the memslot generation or trigger
gfn_to_pfn_cache_invalidate_start()?

>  	r = __copy_from_user(data, (void __user *)addr + offset, len);
>  	if (r)
>  		return -EFAULT;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=11

Re: [PATCH 14/24] KVM: Introduce a generation number for memory attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] xas_next() causes O(N) RCU stalls in kvm_range_has_any_memory_attributes()
- [High] Unconditional increment of kvm->mem_attrs.generation causes global cache invalidation
- [Medium] Data race and potential torn read of kvm->mem_attrs.generation
- [Low] Access to mem_attrs outside of #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES breaks compilation
--

--- Patch [14]: [PATCH 14/24] KVM: Introduce a generation number for memory attributes ---
commit 1b3b4442db618c511fecddd4a021c24933baeaa1
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>
Subject: KVM: Introduce a generation number for memory attributes

Introduce a generation number to track memory attribute modifications.
This will allow KVM components to invalidate any assumptions they might
have about guest's physical addresses and their permissions when memory
attributes change.

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 122af87d6f9a5..1fad3eb303c17 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h

[ ... ]

> @@ -2559,29 +2560,49 @@ static inline bool kvm_mem_attributes_may_exec(u64 attrs)
>  	return !(attrs & KVM_MEMORY_ATTRIBUTE_NX);
>  }
>  
> -static inline bool kvm_memslots_check_gen(struct kvm *kvm, u64 slots_generation, struct kvm_memslots **p_slots)
> +static inline bool kvm_memslots_check_gen(struct kvm *kvm, u64 slots_generation,
> +					  u64 attrs_generation, struct kvm_memslots **p_slots)
>  {
>  	struct kvm_memslots *slots = *p_slots = kvm_memslots(kvm);
> -	return slots->generation == slots_generation;
> +	return slots->generation == slots_generation && kvm->mem_attrs.generation == attrs_generation;

[Severity: Low]
Does this break compilation when CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES is
disabled? The mem_attrs field in struct kvm is guarded by this config,
but it is accessed unconditionally here. Should this use the
kvm_mem_attributes_generation() helper instead?

[Severity: Medium]
Is there a data race on kvm->mem_attrs.generation here?

kvm_vm_set_mem_attributes() updates the generation counter using
smp_store_release(). Since kvm_memslots_check_gen() reads it concurrently
in lockless fast-paths, a plain C read could lead to torn reads on
some architectures. Would it be safer to use READ_ONCE() or the
kvm_mem_attributes_generation() helper?

>  }

[ ... ]

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1afaec56ecb37..2fe4087319adf 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c

[ ... ]

> +bool kvm_range_has_any_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> +					unsigned long mask)
> +{
> +	XA_STATE(xas, &kvm->mem_attrs.array, start);
> +	void *entry;
> +
> +	mask &= kvm_supported_mem_attributes(kvm);
> +	if (!mask)
> +		return false;
> +
> +	if (end == start + 1)
> +		return !!(kvm_get_memory_attributes(kvm, start) & mask);
> +
> +	guard(rcu)();
> +	for (;;) {
> +		do {
> +			entry = xas_next(&xas);
> +		} while (xas_retry(&xas, entry));
> +
> +		if (xas.xa_index >= end)
> +			break;
> +
> +		if (xa_to_value(entry) & mask)
> +			return true;
> +	}
> +
> +	return false;
> +}

[Severity: High]
Can this loop cause CPU soft lockups and RCU stalls when querying a large,
sparse memory range?

If the requested range is empty, xas_next() will return NULL and increment
the internal array index by 1 step-by-step. For millions of pages, this
will iterate millions of times under guard(rcu)().

Should this use xas_find() or xas_for_each() to skip empty entries in O(1)
time instead?

[ ... ]

> @@ -2594,10 +2645,16 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
>  		cond_resched();
>  	}
>  
> +	/* Pairs with acquire in kvm_mem_attributes_generation() */
> +	smp_store_release(&kvm->mem_attrs.generation,
> +			  kvm->mem_attrs.generation + 1);

[Severity: High]
The commit message states: "There shouldn't be any performance loss for
memory attributes that don't require synchronization ... because the
attributes generation will always remain unchanged."

However, kvm->mem_attrs.generation is unconditionally incremented here,
even if the sync flag is false. Will this global increment invalidate all
gfn_to_hva_cache and gfn_to_pfn_cache entries across the entire VM
unnecessarily? Should this be conditional on the sync flag being true?

> +
>  	kvm_handle_gfn_range(kvm, &post_set_range);
>  
>  out_unlock:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=14

Re: [PATCH 09/24] KVM: Introduce NR/NW/NX memory attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] KVM advertises and accepts memory protection attributes (NR/NW/NX) but fails to enforce them in the MMU, leading to guest sandbox bypass.
- [High] A guest-controlled ioctl can trigger an unbounded, non-yielding loop holding `mmu_lock` in write mode, causing host soft lockups.
- [Medium] `kvm_range_has_memory_attributes()` incorrectly assumes any present XArray entry means non-zero attributes, ignoring the `mask`.
- [Low] Incorrect usage of `WARN_ON_ONCE` with a string literal condition.
- [Low] Leftover debugging code (`trace_printk`) and unresolved `// todo:` comments in production code.
--

--- Patch [9]: [PATCH 09/24] KVM: Introduce NR/NW/NX memory attributes ---
commit dce195fe8dd20af5f86a55891fc7d52cd1a8ed11
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: Introduce NR/NW/NX memory attributes

This commit introduces memory attributes to map guest physical memory
regions as non-readable, non-writable, and/or non-executable. It defines
the KVM_MEMORY_ATTRIBUTE_NR/NW/NX flags and adds basic architecture-specific
callbacks for x86 to support these protection attributes.

> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 3b235e9539239..6a96eb3218dc4 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1653,6 +1653,9 @@ struct kvm_memory_attributes {
>  	__u64 flags;
>  };
>  
> +#define KVM_MEMORY_ATTRIBUTE_NR		       (1ULL << 0)
> +#define KVM_MEMORY_ATTRIBUTE_NW		       (1ULL << 1)
> +#define KVM_MEMORY_ATTRIBUTE_NX		       (1ULL << 2)

[Severity: Medium]
Does the introduction of multiple memory attributes break the fast-path
optimization in kvm_range_has_memory_attributes()?

If attrs == 0, the fast-path optimization ignores the mask:

virt/kvm/kvm_main.c:kvm_range_has_memory_attributes() {
    ...
	if (!attrs)
		return !xas_find(&xas, end - 1);
    ...
}

Now that there are multiple attributes, a page might have
KVM_MEMORY_ATTRIBUTE_NR set but KVM_MEMORY_ATTRIBUTE_PRIVATE cleared.
If a caller checks if PRIVATE is 0 (i.e. mask = KVM_MEMORY_ATTRIBUTE_PRIVATE,
attrs = 0), the function incorrectly returns false simply because the NR entry
exists in the XArray.

>  #define KVM_MEMORY_ATTRIBUTE_PRIVATE           (1ULL << 3)
>  
>  #define KVM_CREATE_GUEST_MEMFD	_IOWR(KVMIO,  0xd4, struct kvm_create_guest_memfd)

[ ... ]

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 80f581afb6a01..b6463b0b0b6d4 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -8088,16 +8087,23 @@ bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
>  	}
>  
>  	/*
> -	 * Zap SPTEs even if the slot can't be mapped PRIVATE.  KVM x86 only
> -	 * supports KVM_MEMORY_ATTRIBUTE_PRIVATE, and so it *seems* like KVM
> -	 * can simply ignore such slots.  But if userspace is making memory
> -	 * PRIVATE, then KVM must prevent the guest from accessing the memory
> -	 * as shared.  And if userspace is making memory SHARED and this point
> -	 * is reached, then at least one page within the range was previously
> -	 * PRIVATE, i.e. the slot's possible hugepage ranges are changing.
> -	 * Zapping SPTEs in this case ensures KVM will reassess whether or not
> -	 * a hugepage can be used for affected ranges.
> +	 * For KVM_MEMORY_ATTRIBUTE_PRIVATE:
> +	 *  Zap SPTEs even if the slot can't be mapped PRIVATE.  KVM x86 only
> +	 *  supports KVM_MEMORY_ATTRIBUTE_PRIVATE, and so it *seems* like KVM
> +	 *  can simply ignore such slots.  But if userspace is making memory
> +	 *  PRIVATE, then KVM must prevent the guest from accessing the memory
> +	 *  as shared.  And if userspace is making memory SHARED and this point
> +	 *  is reached, then at least one page within the range was previously
> +	 *  PRIVATE, i.e. the slot's possible hugepage ranges are changing.
> +	 *  Zapping SPTEs in this case ensures KVM will reassess whether or not
> +	 *  a hugepage can be used for affected ranges.
> +	 *
> +	 * For KVM_MEMORY_ATTRIBUTE_NR/NW/NX:
> +	 *  Zap even when loosening restrictions R=>RW, which is nost strictly
> +	 *  necessary, but will allow KVM to reasses whether a hugepage can be
> +	 *  used for the affected pages.
>  	 */

[Severity: High]
Does setting these new attributes trigger an unbounded, non-yielding loop
holding the MMU lock?

When the KVM_SET_MEMORY_ATTRIBUTES ioctl is called, it eventually calls
kvm_arch_post_set_memory_attributes():

arch/x86/kvm/mmu/mmu.c:kvm_arch_post_set_memory_attributes() {
    ...
	for (gfn_t gfn = range->start; gfn < range->end; gfn++) {
		for_each_gfn_valid_sp_with_gptes(kvm, sp, gfn) {
			gen_update = true;
			trace_printk("needs gen update! %llx\n", gfn);
			goto exit_loop;
		}
	}
exit_loop:
    ...
	// todo: what to do with kvm_tdp_mmu_zap_invalidated_roots()?
	// add kvm_arch_post_set_memory_attributes_unlocked?
	return false;
}

Since the GFN range is controlled by the ioctl and can be massive, and the
loop executes under mmu_lock for write without cond_resched(), could this
freeze KVM and trigger host RCU stalls and soft lockups if no GPTEs are
found to break the loop early?

[Severity: Low]
Are the trace_printk() and literal "// todo:" comments in the above snippet
intended to remain in production code?

Using trace_printk() permanently allocates a large trace ring buffer at boot
and emits a loud warning banner in dmesg.

> +
>  	if (WARN_ON_ONCE(range->end <= range->start))
>  		return false;

[ ... ]

> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index fa4473d7c920a..a47b62c2c9cee 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2421,10 +2421,15 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
>  #ifdef CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES
>  u64 kvm_supported_mem_attributes(struct kvm *kvm)
>  {
> +	u64 supported_attrs = 0;
> +
> +	if (kvm_arch_has_memory_protection_attributes(kvm))
> +		supported_attrs |= KVM_MEMORY_ATTRIBUTE_PROT;
> +

[Severity: Critical]
Does KVM advertise and accept memory protection attributes without enforcing
them in the MMU?

KVM advertises KVM_MEMORY_ATTRIBUTE_PROT support to userspace here and allows
setting NR/NW/NX. However, the page fault handlers (e.g.,
__kvm_mmu_faultin_pfn, direct_page_fault, kvm_tdp_mmu_map) do not appear to
query kvm_get_memory_attributes() to intersect the requested access with the
memory attributes.

When the guest faults again after the SPTEs are zapped, the SPTE is rebuilt
with full host permissions, bypassing the memory protections entirely. Can this
allow a guest to escape the intended sandbox?

>  	if (kvm_arch_has_private_mem(kvm))
> -		return KVM_MEMORY_ATTRIBUTE_PRIVATE;
> +		supported_attrs |= KVM_MEMORY_ATTRIBUTE_PRIVATE;
>  
> -	return 0;
> +	return supported_attrs;
>  }

[ ... ]

> @@ -2596,6 +2601,25 @@ static int kvm_vm_set_mem_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
>  
>  	return r;
>  }
> +
> +bool __weak kvm_arch_mem_attributes_supported_prot(struct kvm *kvm, unsigned long attrs)
> +{
> +	WARN_ON_ONCE("KVM_MEMORY_ATTRIBUTE_PROT requires kvm_arch_mem_attributes_supported_prot()");

[Severity: Low]
Should this use WARN_ONCE(1, "...") instead?

The WARN_ON_ONCE macro expects a boolean expression. Since a string literal
evaluates to true, this will print the string's memory address or pointer as
the condition in the stack trace instead of the intended message.

> +	return false;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=9

Re: [PATCH 05/24] KVM: x86/mmu: Extend map_writable to a full ACC_* mask

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Generic bitwise masking of `pte_access` with `host_access` fails to clear the user-mode execution bit on EPT, allowing the guest user-mode to execute host-restricted non-executable mappings.
--

--- Patch [5]: [PATCH 05/24] KVM: x86/mmu: Extend map_writable to a full ACC_* mask ---
commit fbce49ecd47269b3bd1cd08c8c69aaddfb1e0d62
Author: Paolo Bonzini <pbonzini@redhat.com>

KVM: x86/mmu: Extend map_writable to a full ACC_* mask

This commit updates KVM page fault structures to use a full ACC_* mask
instead of a boolean map_writable field. This prepares the MMU to support
non-executable mappings from memory protection attributes, simplifying
make_spte() by directly passing the host_access mask.

> diff --git a/arch/x86/kvm/mmu/spte.c b/arch/x86/kvm/mmu/spte.c
> --- a/arch/x86/kvm/mmu/spte.c
> +++ b/arch/x86/kvm/mmu/spte.c

[ ... ]

> @@ -206,6 +206,11 @@ bool make_spte(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>  	if (!prefetch || synchronizing)
>  		spte |= shadow_accessed_mask;
>  
> +	if (host_access & ACC_WRITE_MASK)
> +		spte |= shadow_host_writable_mask;
> +
> +	pte_access &= host_access;
> +

[Severity: Medium]
Will this bitwise AND logic inadvertently allow guest user-mode execution of
non-executable mappings on EPT?

Because ACC_USER_EXEC_MASK aliases ACC_USER_MASK in KVM, a host_access mask
that clears ACC_EXEC_MASK but retains ACC_USER_MASK (to permit guest
user-mode data accesses) will fail to clear ACC_USER_EXEC_MASK here.

When execution reaches the non-executable handling later in make_spte():

    } else {
        if (pte_access & ACC_EXEC_MASK)
            spte |= shadow_xs_mask;
        if (pte_access & ACC_USER_EXEC_MASK)
            spte |= shadow_xu_mask;
    }

Could this result in shadow_xu_mask being set, inadvertently granting guest
user-mode execution despite the host_access restriction when Mode-Based
Execute Control (MBEC) is enabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=5

Re: [PATCH 21/24] KVM: x86/mmu: Obsolete all roots if memattr contains gPTEs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Unbounded iteration over a potentially massive guest-controlled GFN range while holding `mmu_lock` for write causes host soft lockups.
- [High] Invalidated TDP MMU roots are never zapped, leading to a massive permanent memory leak of page tables and root references.
- [Medium] A `trace_printk()` call was left in the production code path, creating an information leak and triggering a prominent boot warning.
--

--- Patch [21]: [PATCH 21/24] KVM: x86/mmu: Obsolete all roots if memattr contains gPTEs ---
commit 6d375c0ae74e01be130495175a78c166ea5a1aac
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86/mmu: Obsolete all roots if memattr contains gPTEs

This commit introduces a mechanism to zap obsolete pages and all mmu roots
when applying memory protection attributes against a GPTE page.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 8e85f672a08d5..676bc6b61f032 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c

[ ... ]

> @@ -8281,6 +8297,33 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
>  				hugepage_set_mixed(slot, gfn, level);
>  		}
>  	}
> +
> +	/*
> +	 * There are special considerations when applying an memory protection
> +	 * attibute against a GPTE page. If set read-only, access/dirty bits
> +	 * within that page shouldn't be updated. If set non-accesible,
> +	 * accessing a virtual address that requires traversing that GPTE page
> +	 * should fault.
> +	 *
> +	 * On TDP enabled guests, the CPU faults on the GPTE address upon
> +	 * detecting such a situation.
> +	 *
> +	 * On non-TDP, upon detecting this situation, and based on the fact it
> +	 * should be a rare occasion, invalidate all the mmu roots.
> +	 */
> +	for (gfn_t gfn = range->start; gfn < range->end; gfn++) {

[Severity: High]
Does this unbounded loop over a guest-controlled range cause host soft
lockups? It iterates over every 4K page from range->start to range->end
while kvm->mmu_lock is held for write, disabling preemption. If a massive
range is passed (e.g., hundreds of gigabytes), iterating millions of pages
without yielding could stall the host CPU.

> +		for_each_gfn_valid_sp_with_gptes(kvm, sp, gfn) {
> +			gen_update = true;
> +			trace_printk("needs gen update! %llx\n", gfn);

[Severity: Medium]
Is this trace_printk() intended for production? Using it unconditionally
allocates tracing ring buffers globally and emits a prominent warning to
dmesg at boot, which is generally avoided in upstream code.

> +			goto exit_loop;
> +		}
> +	}
> +exit_loop:
> +	if (gen_update)
> +		__kvm_mmu_zap_all_fast(kvm);
> +
> +	// todo: what to do with kvm_tdp_mmu_zap_invalidated_roots()?

[Severity: High]
Does this sequence leak KVM MMU page tables and root structures?

When tdp_mmu_enabled is true, __kvm_mmu_zap_all_fast() marks TDP roots as
invalid and takes a reference to them via kvm_tdp_mmu_invalidate_roots().

The caller must subsequently invoke kvm_tdp_mmu_zap_invalidated_roots()
to consume these scheduled roots and put the gifted references. Since this
cleanup step is skipped here, the roots appear to be leaked permanently.

> +	// add kvm_arch_post_set_memory_attributes_unlocked?
> +	return false;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=21

Re: [PATCH 19/24] KVM: x86/mmu: Do not update accessed/dirty if guest PTE is read-only

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Silently skipping Accessed/Dirty (A/D) bit updates for guest page tables protected by read-only memory attributes allows KVM to map writable SPTEs without updating the guest's dirty bit, causing silent data corruption in the guest OS.
- [High] When a guest page table walk fails due to a memory attribute read fault, KVM incorrectly reports the original instruction's access type (write/exec) to userspace instead of reporting a read fault on the page table.
- [High] KVM injects a fatal guest Page Fault instead of exiting to userspace when an emulated instruction's page table walk hits a memory attribute read fault.
--

--- Patch [19]: [PATCH 19/24] KVM: x86/mmu: Do not update accessed/dirty if guest PTE is read-only ---
commit 0ad73786bede76212593913d2d0fa39e55055d34
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86/mmu: Do not update accessed/dirty if guest PTE is read-only
    
This commit modifies the guest page table walk in KVM's x86 MMU to check if
the guest page table's physical address is writable according to KVM memory
attributes. If not, it marks the PTE as not writable in the guest walker.
This appears to be an optimization to avoid exiting to userspace when guest
page tables are mapped read-only.

> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index 23d7a7d9769fe..ee5c23d1b77a8 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -419,6 +419,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>  			goto error;
>  		}
>  
> +		if (!kvm_mem_attributes_may_write_gfn(vcpu->kvm, gpa_to_gfn(real_gpa)))
> +			walker->pte_writable[walker->level - 1] = false;
> +

[Severity: High]
Does this silently skip Accessed/Dirty (A/D) bit updates for guest page
tables?

When the VMM sets a page table's GPA as read-only via KVM memory attributes,
this sets pte_writable to false. Later, FNAME(update_accessed_dirty_bits)()
uses this flag to handle read-only ROM slots:

		if (unlikely(!walker->pte_writable[level - 1]))
			continue;

Since FNAME(update_accessed_dirty_bits)() reports success by returning 0, KVM
will proceed to map the leaf page as writable in the shadow page table (SPTE).

Could this lead to silent data corruption in the guest OS, since it can then
write to the page without faulting while its own page table is never updated?


[Severity: High]
Does KVM report the incorrect access flags to the VMM when a page table walk
fails due to a memory attribute read fault?

In FNAME(page_fault)(), KVM prepares the exit using fault->gfn = walker.gfn
but passes the original fault object:

		if (walker.memory_attributes_fault) {
			fault->gfn = walker.gfn;
			kvm_mmu_prepare_memory_fault_exit(vcpu, fault);
			return -EFAULT;
		}

This passes the write/exec flags of the instruction attempting to access the
leaf page, rather than the KVM page walker's read access to the guest page
table. Could this confuse the VMM's fault handling logic?


[Severity: High]
Is the memory attribute read fault correctly handled by the instruction
emulator?

When FNAME(walk_addr_generic)() encounters a page table mapped as unreadable,
it sets walker.memory_attributes_fault = true and populates a synthetic #PF in
walker.fault.

In FNAME(gva_to_gpa)():

	r = FNAME(walk_addr_generic)(&walker, vcpu, w, addr, access);

	if (r) {
		gpa = gfn_to_gpa(walker.gfn);
		gpa |= addr & ~PAGE_MASK;
	} else if (exception)
		*exception = walker.fault;

The memory_attributes_fault flag is ignored, and the synthetic #PF is bubbled
up to the emulator, which will blindly inject it into the guest. 

Should this trigger a KVM_EXIT_MEMORY_FAULT instead of injecting a fatal guest
page fault?

>  		ptep_user = (pt_element_t __user *)((void *)host_addr + offset);
>  		if (unlikely(get_user(pte, ptep_user)))
>  			goto error;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=19

Re: [PATCH 06/24] KVM: x86: Avoid warning when installing non-private memory attributes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The defensive sanitization of memory attributes is ineffective, containing a dead store and failing to prevent inconsistent xarray state.
--

--- Patch [6]: [PATCH 06/24] KVM: x86: Avoid warning when installing non-private memory attributes ---
commit 08f754e61d84422c72539df0f4a05fb7d05a02f5
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86: Avoid warning when installing non-private memory attributes

This commit adds a check to warn and clear KVM_MEMORY_ATTRIBUTE_PRIVATE in
both kvm_arch_pre_set_memory_attributes() and
kvm_arch_post_set_memory_attributes() when private memory is not supported.
It replaces the unconditional WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm))
checks that were previously throwing warnings.

> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 424f4e1136822..055e0b45a8ee9 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -8078,9 +8078,15 @@ static void hugepage_set_mixed(struct kvm_memory_slot *slot, gfn_t gfn,
>  bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
>  					struct kvm_gfn_range *range)
>  {
> +	unsigned long attrs = range->arg.attributes;
>  	struct kvm_memory_slot *slot = range->slot;
>  	int level;
>  
> +	if (!kvm_arch_has_private_mem(kvm)) {
> +		WARN_ON(attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE);
> +		attrs &= ~KVM_MEMORY_ATTRIBUTE_PRIVATE;
> +	}
> +

[Severity: Medium]
Is this attrs update effective, or does it result in a dead store?

Looking further down in kvm_arch_pre_set_memory_attributes(), the unmap
filter logic evaluates the unmodified range->arg.attributes instead of using
this newly sanitized attrs variable:

	/* Unmap the old attribute page. */
	if (range->arg.attributes & KVM_MEMORY_ATTRIBUTE_PRIVATE)
		range->attr_filter = KVM_FILTER_SHARED;
	else
		range->attr_filter = KVM_FILTER_PRIVATE;

Does this mean the invalid attributes are still processed if they manage to
reach this path?

[ ... ]

> @@ -8165,16 +8168,17 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
>  	lockdep_assert_held_write(&kvm->mmu_lock);
>  	lockdep_assert_held(&kvm->slots_lock);
>  
> +	if (!kvm_arch_has_private_mem(kvm)) {
> +		WARN_ON(attrs & KVM_MEMORY_ATTRIBUTE_PRIVATE);
> +		attrs &= ~KVM_MEMORY_ATTRIBUTE_PRIVATE;
> +	}
> +

[Severity: Medium]
Could sanitizing attrs locally here lead to inconsistent state evaluations
later on?

In virt/kvm/kvm_main.c:kvm_vm_set_mem_attributes(), the unmodified attributes
argument is unconditionally stored into the global xarray before the
arch-specific callbacks run:

	r = xa_err(xa_store(&kvm->mem_attr_array, i, entry,
			    GFP_KERNEL_ACCOUNT));

If kvm_arch_post_set_memory_attributes() passes this locally sanitized attrs
variable into hugepage_has_attrs(), will it erroneously compare the sanitized
value against the unsanitized values stored in the xarray?

>  	/*
>  	 * Calculate which ranges can be mapped with hugepages even if the slot
>  	 * can't map memory PRIVATE.  KVM mustn't create a SHARED hugepage over
>  	 * a range that has PRIVATE GFNs, and conversely converting a range to
>  	 * SHARED may now allow hugepages.
> -	 */
> -	if (WARN_ON_ONCE(!kvm_arch_has_private_mem(kvm)))
> -		return false;
> -
> -	/*
>  	 * The sequence matters here: upper levels consume the result of lower
>  	 * level's scanning.
>  	 */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=6

Re: [PATCH 22/24] KVM: x86: selftests: Introduce memory attributes test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Massive unintended memory allocation in the memory attributes test.
- [Medium] Masked validation test for unsupported KVM_SET_MEMORY_ATTRIBUTES flags.
--

--- Patch [22]: [PATCH 22/24] KVM: x86: selftests: Introduce memory attributes test ---
commit 4443977bb70263dd7f96620eede82bb13ee6a48d
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86: selftests: Introduce memory attributes test

This commit introduces a new selftest for the KVM_SET_MEMORY_ATTRIBUTES
ioctl. It tests various memory attribute combinations, MMIO access handling,
and input parameter validations.

> diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
> new file mode 100644
> index 0000000000000..7066ae791b99a
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/memory_attributes.c

[ ... ]

> +static void test_input_validation(struct kvm_vm *vm)
> +{
> +	uint64_t flags, gpa = 0, size = 0, attrs = 0;
> +	int rc;
> +
> +	/* 'flags' is unsupported */
> +	flags = BIT(0);
> +	rc = __vm_set_memory_attributes(vm, gpa, size, attrs, flags);

[Severity: Medium]
Could this validation test pass for the wrong reason?

Because size is initialized to 0, the ioctl will return -EINVAL even if the
kernel's validation for the flags parameter is broken or missing. Setting
size to a valid, page-aligned non-zero value here would ensure this check
actually isolates and verifies the rejection of unsupported flags.

[ ... ]

> +int main(int argc, char *argv[])
> +{
> +	uint32_t guest_page_size = vm_guest_mode_params[VM_MODE_DEFAULT].page_size;
> +	unsigned int ptes_per_page = guest_page_size / 8;
> +	size_t size = guest_page_size * ptes_per_page * 2; /* 2 huge-pages */
> +	struct kvm_vcpu *vcpu;
> +	gva_t test_mem;
> +	struct kvm_vm *vm;
> +
> +	TEST_REQUIRE(kvm_check_cap(KVM_CAP_MEMORY_ATTRIBUTES) &
> +		     KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
> +
> +	vm = __vm_create_with_one_vcpu(&vcpu, size, guest_code);

[Severity: Medium]
Does this code inadvertently request a massive amount of memory?

The size variable is calculated in bytes (e.g. 4 MB), but it is passed to
__vm_create_with_one_vcpu() which expects the second parameter to be a page
count (extra_mem_pages). Passing a byte count directly causes the test to
request millions of extra pages for page tables (e.g. 16 GB), which could
lead to test failures due to ENOMEM on systems with stricter overcommit
limits or insufficient free memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=22

Re: [PATCH 15/24] KVM: Take memory protections into account for accesses with cached gfn->hva

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] gfn_to_pfn_cache (GPC) bypasses memory attribute protections (NR/NW).
- [Medium] Redundant memory attribute checks in __kvm_get_guest and __kvm_put_guest lead to duplicate xarray lookups (performance regression).
- [Low] kvm_memslots_check_gen() causes a build failure when CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES is disabled.
--

--- Patch [15]: [PATCH 15/24] KVM: Take memory protections into account for accesses with cached gfn->hva ---
commit 51177f153d521afab2fb1286cac8c924ca7aa90d
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: Take memory protections into account for accesses with cached gfn->hva

Account for memory attributes when accessing guest memory through
kvm_get/put_guest().

This requires tracking the memory attributes generation as part of
gfn_to_hva_cache's data, invalidate the cached information if the
generation changes, and failing to refresh the cache if restrictive
memory attributes are found within the GPA range.

> Similar to how gfn_to_hva_cache disallows caching gfns mapped within
> read-only memory slots, gfns marked as read-only by memory attributes
> will also fail to initialize. Unsurprisingly, the same behaviour applies
> to gfns mapped as non-accessible (NR/NW), while non-executable mappings
> are okay.

[Severity: High]
Does gfn_to_pfn_cache (GPC) bypass memory attribute protections (NR/NW) while
this patch updates gfn_to_hva_cache (GHC)?

In virt/kvm/pfncache.c:__kvm_gpc_refresh():

			gpc->attrs_generation = kvm_mem_attributes_generation(kvm);
			gpc->gpa = gpa;
			gpc->slots_generation = slots->generation;
			gpc->memslot = __gfn_to_memslot(slots, gfn);
			gpc->uhva = gfn_to_hva_memslot(gpc->memslot, gfn);

The HVA is retrieved without evaluating memory attributes to see if access is
actually permitted. KVM can bypass guest memory isolation boundaries, writing
to memory that the guest expects to be read-only (NW). Should gfn_to_pfn_cache
also validate kvm_mem_attributes_may_write_gfn()?

> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 1fad3eb303c17..71ab2cbecbd12 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
[ ... ]
> @@ -1351,7 +1351,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
>  	typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset);	\
>  	int __ret = -EFAULT;						\
>  									\
> -	if (!kvm_is_error_hva(__addr))					\
> +	if (!kvm_is_error_hva(__addr) &&                                \
> +	    kvm_mem_attributes_may_read_gfn(kvm, gfn))		\
>  		__ret = get_user(v, __uaddr);				\
>  	__ret;								\
>  })

[Severity: Medium]
Are these memory attribute checks redundant and introducing an unnecessary
xarray lookup on the fast path?

gfn_to_hva() inherently checks memory attributes and fails if not permitted,
rendering these macro checks redundant:

virt/kvm/kvm_main.c:gfn_to_hva() {
    ...
	if (!kvm_mem_attributes_may_write_gfn(kvm, gfn))
		return KVM_HVA_ERR_RO_BAD;
    ...
}

If !kvm_is_error_hva(__addr) is true, the memory is already guaranteed to be
readable and writable.

> @@ -1371,7 +1372,8 @@ int kvm_gfn_to_hva_cache_init(struct kvm *kvm, struct gfn_to_hva_cache *ghc,
>  	typeof(v) __user *__uaddr = (typeof(__uaddr))(__addr + offset);	\
>  	int __ret = -EFAULT;						\
>  									\
> -	if (!kvm_is_error_hva(__addr))					\
> +	if (!kvm_is_error_hva(__addr) &&                                \
> +	    kvm_mem_attributes_may_write_gfn(kvm, gfn))		\
>  		__ret = put_user(v, __uaddr);				\
>  	if (!__ret)							\
>  		mark_page_dirty(kvm, gfn);				\

[Severity: Medium]
Similar to __kvm_get_guest above, does this inject a redundant xarray lookup
since !kvm_is_error_hva() already confirmed the memory is writable?

[ ... ]
> @@ -2632,6 +2634,12 @@ static inline unsigned long kvm_get_memory_attributes(struct kvm *kvm, gfn_t gfn
>  {
>  	return 0;
>  }
> +static inline bool kvm_range_has_any_memory_attributes(struct kvm *kvm,
> +						       gfn_t start, gfn_t end,
> +						       unsigned long mask)
> +{
> +	return false;
> +}
>  static inline u64 kvm_mem_attributes_generation(struct kvm *kvm)
>  {
>  	return 0;
>  }

[Severity: Low]
Will kvm_memslots_check_gen() cause a build failure when
CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES is disabled?

kvm_memslots_check_gen() unconditionally accesses kvm->mem_attrs.generation:

include/linux/kvm_host.h:kvm_memslots_check_gen() {
    ...
	return slots->generation == slots_generation && 
               kvm->mem_attrs.generation == attrs_generation;
}

However, the mem_attrs struct member is conditionally compiled under
CONFIG_KVM_GENERIC_MEMORY_ATTRIBUTES. Should kvm_memslots_check_gen() use the
kvm_mem_attributes_generation(kvm) wrapper instead, which correctly returns 0
when the feature is disabled?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=15

Re: [PATCH 18/24] KVM: x86/mmu: Issue memory fault exit if walk failed due to memory attribute

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] KVM reports misleading access flags (read/write/exec) to userspace in KVM_EXIT_MEMORY_FAULT when a software page table walk fails due to a memory attribute violation.
- [High] During instruction emulation, memory attribute faults on guest page table walks are ignored by FNAME(gva_to_gpa), causing KVM to incorrectly inject a spurious #PF into the guest instead of exiting to userspace.
--

--- Patch [18]: [PATCH 18/24] KVM: x86/mmu: Issue memory fault exit if walk failed due to memory attribute ---
commit 724220bf46ef5e18dcf744c814e78fa2b6296962
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86/mmu: Issue memory fault exit if walk failed due to memory attribute

This commit adds memory attribute checks to the guest page table walker. If
the walker encounters a page table entry residing in a memory region with
restricted attributes, it exits to userspace with a memory fault rather than
injecting a spurious page fault into the guest.

> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index cdf05cd76d63d..23d7a7d9769fe 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h

[ ... ]

> @@ -820,6 +828,12 @@ static int FNAME(page_fault)(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
>  	 * The page is not mapped by the guest.  Let the guest handle it.
>  	 */
>  	if (!r) {
> +		if (walker.memory_attributes_fault) {
> +			fault->gfn = walker.gfn;
> +			kvm_mmu_prepare_memory_fault_exit(vcpu, fault);

[Severity: High]
When setting up the memory fault exit here, do we end up reporting the
original instruction's access flags instead of the walker's?

The kvm_mmu_prepare_memory_fault_exit() function uses fault->write and
fault->exec. If an instruction fetch triggers the page fault, and KVM's
subsequent read of the guest page table fails the attribute check, userspace
might incorrectly be told the guest attempted to execute the page table
itself rather than just read it.

Could this cause the VMM to mismanage memory protections or spuriously
terminate the VM?

[Severity: High]
Does FNAME(gva_to_gpa) also need to check walker.memory_attributes_fault?

When FNAME(gva_to_gpa) translates guest virtual addresses during instruction
emulation, it calls into the walk functions but doesn't check the new
walker.memory_attributes_fault flag:

FNAME(gva_to_gpa)() {
        ...
        if (ndata) {
                ...
        } else if (exception)
                *exception = walker.fault;

        return gpa;
}

Will this cause KVM to incorrectly inject a spurious page fault into the guest
instead of exiting to userspace if an attribute violation occurs during
emulation?

> +			return -EFAULT;
> +		}
> +
>  		if (!fault->prefetch)
>  			__kvm_inject_emulated_page_fault(vcpu, &walker.fault, true);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=18

Re: [PATCH 23/24] KVM: x86: selftests: Introduce memory attributes PTE test

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] `test_memory_access_sync_spte()` incorrectly passes a Guest Virtual Address (`vaddr`) to `arch_write_return_insn()`, which requires a Guest Physical Address (`paddr`), causing test crashes or memory corruption.
- [Medium] `test_page()` silently ignores guest-side assertions (`UCALL_ABORT`), potentially masking test failures and leaving the host/guest state out of sync.
--

--- Patch [23]: [PATCH 23/24] KVM: x86: selftests: Introduce memory attributes PTE test ---
commit 129595776ab1f8d4ce03183d898471949d9d7971
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

    KVM: x86: selftests: Introduce memory attributes PTE test

This patch introduces tests to verify the behavior of memory attributes
(non-accessible and read-only) when installed on a page that holds guest
page table entries.

> diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
> index 7066ae791b99a..c66d5d085a9c1 100644
> --- a/tools/testing/selftests/kvm/memory_attributes.c
> +++ b/tools/testing/selftests/kvm/memory_attributes.c

[ ... ]

> @@ -106,6 +121,21 @@ static void vcpu_run_and_inc_stage(struct kvm_vcpu *vcpu)
>  	test_data->stage++;
>  }
>  
> +static int test_page(struct kvm_vcpu *vcpu, int op, gva_t vaddr)
> +{
> +	int rc;
> +
> +	test_data->op = op;
> +	test_data->vaddr = vaddr;
> +
> +	rc = _vcpu_run(vcpu);
> +
> +	if (rc >= 0)
> +		test_data->stage++;

[Severity: Medium]
If the guest asserts (e.g., via GUEST_ASSERT), will this silently ignore the
failure?

When a guest assertion fires, KVM exits with KVM_EXIT_IO and the guest spins
in UCALL_ABORT. _vcpu_run() returns 0 in this case. Without decoding the
ucall struct, the test assumes the operation succeeded, increments the stage,
and continues, which could mask test failures.

> +
> +	return rc < 0 ? -errno : rc;
> +}

[ ... ]

> diff --git a/tools/testing/selftests/kvm/x86/memory_attributes.c b/tools/testing/selftests/kvm/x86/memory_attributes.c
> index 2e1148f5146dd..3d2bcfe718848 100644
> --- a/tools/testing/selftests/kvm/x86/memory_attributes.c
> +++ b/tools/testing/selftests/kvm/x86/memory_attributes.c

[ ... ]

> +static void test_memory_access_sync_spte(struct kvm_vcpu *vcpu, gva_t vaddr)
> +{
> +	struct kvm_vm *vm = vcpu->vm;
> +	gpa_t paddr = addr_gva2gpa(vm, vaddr);
> +	uint64_t *pte, old_pte, new_pte;

[ ... ]

> +	/* The not executable attrs remain valid */
> +	arch_write_return_insn(vm, vaddr);

[Severity: Medium]
Does this incorrectly pass a Guest Virtual Address (vaddr) to a function that
expects a Guest Physical Address?

arch_write_return_insn() takes a gpa_t and passes it to addr_gpa2hva():

void arch_write_return_insn(struct kvm_vm *vm, gpa_t vaddr)
{
	memset(addr_gpa2hva(vm, vaddr), 0xc3, 1);
}

If fed a GVA instead of the locally calculated paddr, will this cause the test
to crash due to a failed memory mapping translation, or silently corrupt the
wrong host virtual address?

> +	test_page_restricted(vcpu, TEST_OP_EXEC, vaddr, paddr,
> +			     KVM_MEMORY_EXIT_FLAG_EXEC);
> +
> +	/* Cleanup */
> +	*pte = old_pte;
> +	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);
> +	test_page_accessible(vcpu, TEST_OP_EXEC, vaddr);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=23

Re: [PATCH 24/24] KVM: x86: selftests: Introduce memory attributes side-channel tests

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] `test_side_channel_pvclock` assumes KVM periodically updates `tsc_timestamp` in the background, which is false and causes the guest to hang indefinitely.
- [Medium] `test_side_channel_hyperv_hypercall_inputs` fails to zero-initialize the `struct hv_send_ipi_ex` object in reused guest memory, causing the hypercall to fail.
- [Low] `test_side_channel_hyperv_hypercall_inputs` computes the physical address using stale `test_data->vaddr` instead of the passed parameter.
- [Low] `vcpu_translate()` passes a partially uninitialized `struct kvm_translation` to the KVM ioctl.
--

--- Patch [24]: [PATCH 24/24] KVM: x86: selftests: Introduce memory attributes side-channel tests ---
commit 4f5cb036a6a68d87d1e673199973b66af14cd6a4
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86: selftests: Introduce memory attributes side-channel tests
    
Introduce memory attributes selftests to catch any vulnerable
side-channels.

> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 54d73ea46e11f..3160a25966a10 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -883,6 +883,16 @@ static inline int vcpu_get_stats_fd(struct kvm_vcpu *vcpu)
>  	return fd;
>  }
>  
> +static inline struct kvm_translation vcpu_translate(struct kvm_vcpu *vcpu,
> +						    u64 gva)
> +{
> +	struct kvm_translation tr;
> +
> +	tr.linear_address = gva;
> +	vcpu_ioctl(vcpu, KVM_TRANSLATE, &tr);

[Severity: Low]
Does this pass uninitialized stack memory to the kernel?

Since only linear_address is initialized, the remainder of the struct contains
uninitialized stack memory. When the kernel reads the struct via
copy_from_user(), it will read this uninitialized memory, which can trigger
MemorySanitizer or Valgrind failures in CI environments.

Should this struct be zero-initialized first?

> +	return tr;
> +}
> +
>  int __kvm_has_device_attr(int dev_fd, u32 group, u64 attr);

[ ... ]

> diff --git a/tools/testing/selftests/kvm/memory_attributes.c b/tools/testing/selftests/kvm/memory_attributes.c
> index c66d5d085a9c1..843fafcf4d96d 100644
> --- a/tools/testing/selftests/kvm/memory_attributes.c
> +++ b/tools/testing/selftests/kvm/memory_attributes.c

[ ... ]

> @@ -88,6 +125,25 @@ static void guest_code(void *data)
>  				     :: "b" (vaddr): "memory");
>  			GUEST_SYNC(stage++);
>  			break;
> +		case TEST_OP_HYPERV_HYPERCALL_INPUT: {
> +			hyperv_hypercall(HVCALL_SEND_IPI_EX, paddr, 0);
> +			asm volatile ("sti; hlt; cli;");
> +			GUEST_ASSERT_EQ(ipis_rcvd, 1);
> +			GUEST_SYNC(stage++);
> +			break;
> +		}
> +		case TEST_OP_MONITOR_ADDRESS: {
> +			uint64_t *pval = (uint64_t *)vaddr;
> +			uint64_t val = READ_ONCE(*pval);
> +
> +			while (READ_ONCE(*pval) == val &&
> +			       /* So host can force the op out of the loop */
> +			       READ_ONCE(test_data->op) == TEST_OP_MONITOR_ADDRESS)
> +				asm volatile("nop");

[Severity: Medium]
Can this loop hang the guest indefinitely?

In test_side_channel_pvclock(), the guest is expected to exit this loop when
the pvclock tsc_timestamp changes. However, KVM only updates the pvclock
structure synchronously before entering the guest (e.g., when processing
KVM_REQ_CLOCK_UPDATE).

Since the guest is spinning here without causing a VM-exit, KVM never gets a
chance to process clock updates, tsc_timestamp will never change, and the host
thread will time out waiting in pthread_tryjoin_np().

> +
> +			GUEST_SYNC(stage++);
> +			break;
> +		}
>  #endif

[ ... ]

> diff --git a/tools/testing/selftests/kvm/x86/memory_attributes.c b/tools/testing/selftests/kvm/x86/memory_attributes.c
> index 3d2bcfe718848..5203d284d2d36 100644
> --- a/tools/testing/selftests/kvm/x86/memory_attributes.c
> +++ b/tools/testing/selftests/kvm/x86/memory_attributes.c

[ ... ]

> +static void test_side_channel_hyperv_hypercall_inputs(struct kvm_vcpu *vcpu,
> +						      gva_t vaddr,
> +						      size_t size)
> +{
> +	struct kvm_vm *vm = vcpu->vm;
> +	struct hv_send_ipi_ex *ipi_ex = addr_gva2hva(vm, vaddr);
> +	gpa_t paddr = addr_gva2gpa(vm, test_data->vaddr);

[Severity: Low]
Does this use stale global state instead of the vaddr parameter?

Using test_data->vaddr here before it gets updated for this test phase relies
on it holding a left-over value from the previous test run. Should this use the
vaddr parameter passed into the function instead?

> +
> +	if (!kvm_has_cap(KVM_CAP_HYPERV_SEND_IPI) ||
> +	    !kvm_has_cap(KVM_CAP_HYPERV_HCALL_FAULT_EXIT))
> +		return;
> +
> +	vm_enable_cap(vcpu->vm, KVM_CAP_HYPERV_HCALL_FAULT_EXIT, 1);
> +
> +	ipis_rcvd = 0;
> +	vm_install_exception_handler(vm, IPI_VECTOR, guest_ipi_handler_hv);
> +	vcpu_set_msr(vcpu, HV_X64_MSR_GUEST_OS_ID, HYPERV_LINUX_OS_ID);
> +
> +	ipi_ex->vector = IPI_VECTOR;
> +	ipi_ex->vp_set.format = HV_GENERIC_SET_ALL;

[Severity: Medium]
Will this hypercall fail due to an uninitialized reserved field?

The struct hv_send_ipi_ex object is mapped over previously used guest memory
(vaddr). Because the reserved field isn't initialized here, it will contain
garbage data left over from prior tests.

The KVM handler for HVCALL_SEND_IPI_EX explicitly checks that reserved == 0 and
will return HV_STATUS_INVALID_PARAMETER otherwise, causing the test to fail.

Should the struct be cleared using memset() first?

> +	vm_set_memory_attributes(vm, paddr, vm->page_size, KVM_MEMORY_ATTRIBUTE_NO_ACCESS);
> +	test_page_restricted(vcpu, TEST_OP_HYPERV_HYPERCALL_INPUT, vaddr, paddr,
> +			     KVM_MEMORY_EXIT_FLAG_READ);
> +	vm_set_memory_attributes(vm, paddr, vm->page_size, 0);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=24

Re: [PATCH 17/24] KVM: x86/mmu: Take memory protection attributes into account during faults

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The new `kvm_faultin_memory_protections` trace event directly calls `kvm_rip_read(vcpu)` inside its `TP_fast_assign()` block. This introduces a dangerous side effect that mutates vCPU state and triggers a host-side DoS (via `WARN_ON_ONCE` and potential kernel panic) for confidential VMs like Intel TDX.
--

--- Patch [17]: [PATCH 17/24] KVM: x86/mmu: Take memory protection attributes into account during faults ---
commit df2f2adf06ac42cf5912c212a4b41b02bf9ceb82
Author: Nicolas Saenz Julienne <nsaenz@amazon.com>

KVM: x86/mmu: Take memory protection attributes into account during faults

This commit updates the x86 MMU fault handling to check memory protection
attributes. If a guest accesses memory with prohibited attributes, it
exits to userspace with -EFAULT. It also adds a trace event for these
memory protection faults.

> diff --git a/arch/x86/kvm/mmu/mmutrace.h b/arch/x86/kvm/mmu/mmutrace.h
> --- a/arch/x86/kvm/mmu/mmutrace.h
> +++ b/arch/x86/kvm/mmu/mmutrace.h
> @@ -447,6 +447,35 @@ TRACE_EVENT(
[ ... ]
> +TRACE_EVENT(kvm_faultin_memory_protections,
> +	TP_PROTO(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault,
> +		 unsigned access),
> +	TP_ARGS(vcpu, fault, access),
> +
> +	TP_STRUCT__entry(
> +		__field(unsigned int, vcpu_id)
> +		__field(unsigned long, guest_rip)
> +		__field(u64, fault_address)
> +		__field(bool, write)
> +		__field(bool, exec)
> +		__field(unsigned, access)
> +	),
> +
> +	TP_fast_assign(
> +		__entry->vcpu_id = vcpu->vcpu_id;
> +		__entry->guest_rip = kvm_rip_read(vcpu);
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
Does this code trigger a side-effect that crashes the host for confidential
VMs like Intel TDX?

Reading the instruction pointer dynamically via kvm_rip_read() inside a
tracepoint evaluates kvm_register_is_available(). If the register is not
marked available, it calls the architecture-specific cache_reg hook to
fetch it. 

For TDX guests, VCPU_REG_RIP is explicitly omitted from the available
register set. Consequently, kvm_rip_read() will fall back to calling
vt_cache_reg(), which executes a WARN_ON_ONCE(is_td_vcpu(vcpu)).

Since an untrusted TDX guest can intentionally trigger this fault by
accessing restricted memory, this allows a malicious guest to trigger a
host-side denial of service if tracing is enabled and panic_on_warn is set.

Should this use tracing_kvm_rip_read() instead to safely fetch the RIP
without triggering caching side-effects?

> +		__entry->fault_address = fault->gfn;
> +		__entry->write = fault->write;
> +		__entry->exec = fault->exec;
> +		__entry->access = access;
> +	),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716181456.402786-1-pbonzini@redhat.com?part=17

Re: [PATCH 11/24] KVM: Take memory protections into account in kvm_read/write_guest()

[Edgecombe, Rick P]

On Thu, 2026-07-16 at 14:14 -0400, Paolo Bonzini wrote:
> @@ -3261,6 +3261,10 @@ static int __kvm_read_guest_page(struct kvm_memory_slot *slot, gfn_t gfn,
>  	addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
>  	if (kvm_is_error_hva(addr))
>  		return -EFAULT;
> +
> +	if (!kvm_mem_attributes_may_read_gfn(kvm, gfn))
> +		return -EFAULT;
> +
>  	r = __copy_from_user(data, (void __user *)addr + offset, len);
>  	if (r)
>  		return -EFAULT;

Since the emulate kvm_fetch_guest_virt() path ends up calling this, probably
somewhere the execute attribute needs to be checked too. I don't know how much
of a concern it is in practice.