Enable Sub-Page Write Protection Support

Review

Quoted 2 lines · Yang Weijiang

(quoted diff) +2 -0

+
+#define SUBPAGE_MAX_BITMAP   64

Please rename this to KVM_SUBPAGE_MAX_PAGES

Documentation/virt/kvm/api.txt Valid values for 'action': · line 4134

Quoted 3 lines

Documentation/virt/kvm/api.txt +3 -0

+struct kvm_subpage_info {
+	__u64 gfn;    /* the first page gfn of the contiguous pages */
+	__u64 npages; /* number of 4K pages */

This can be

	u32 npages;
	u32 flags;

Check that the flags are 0, and fail the ioctl if they aren't. This will make it easy to extend the API in the future.

Review

Quoted 2 lines

(quoted diff) +2 -0

+	__u32 access_map[SUBPAGE_MAX_BITMAP]; /* sub-page write-access bitmap array */
+};

Please make this access_map[0], since the number of entries actually depends on npages.

Likewise, kvm_arch_vm_ioctl should read the header first, then allocate memory for the access_map and read into it. It's probably simpler if you make kvm_vm_ioctl_get_subpages/kvm_vm_ioctl_set_subpages take parameters like

int kvm_vm_ioctl_get_subpages(struct kvm *kvm, u64 gfn, u32 npages,

			      u32 *access_map);
int kvm_vm_ioctl_set_subpages(struct kvm *kvm, u64 gfn, u32 npages,
			      u32 *access_map);

Thanks,

Paolo

arch/x86/include/asm/cpufeatures.h line 231

Quoted 5 lines · Yang Weijiang

arch/x86/include/asm/cpufeatures.h +1 -0

@@ -228,6 +228,7 @@
 #define X86_FEATURE_FLEXPRIORITY	( 8*32+ 2) /* Intel FlexPriority */
 #define X86_FEATURE_EPT			( 8*32+ 3) /* Intel Extended Page Table */
 #define X86_FEATURE_VPID		( 8*32+ 4) /* Intel Virtual Processor ID */
+#define X86_FEATURE_SPP			( 8*32+ 5) /* Intel EPT-based Sub-Page Write Protection */

Please do not include X86_FEATURE_SPP. In general I don't like the VMX features word, but apart from that SPP is not a feature that affects all VMs in the same way as EPT or FlexPriority.

Paolo

Quoted 11 lines · Paolo Bonzini

arch/x86/include/asm/cpufeatures.h line 231

Quoted 5 lines · Yang Weijiang

arch/x86/include/asm/cpufeatures.h +1 -0

@@ -228,6 +228,7 @@
 #define X86_FEATURE_FLEXPRIORITY	( 8*32+ 2) /* Intel FlexPriority */
 #define X86_FEATURE_EPT			( 8*32+ 3) /* Intel Extended Page Table */
 #define X86_FEATURE_VPID		( 8*32+ 4) /* Intel Virtual Processor ID */
+#define X86_FEATURE_SPP			( 8*32+ 5) /* Intel EPT-based Sub-Page Write Protection */

Please do not include X86_FEATURE_SPP. In general I don't like the VMX features word, but apart from that SPP is not a feature that affects all VMs in the same way as EPT or FlexPriority.

So what's a friendly way to let a user check if SPP feature is there?

Quoted 12 lines · Yang Weijiang

Quoted 11 lines · Paolo Bonzini

arch/x86/include/asm/cpufeatures.h line 231

Quoted 5 lines · Yang Weijiang

arch/x86/include/asm/cpufeatures.h +1 -0

@@ -228,6 +228,7 @@
 #define X86_FEATURE_FLEXPRIORITY	( 8*32+ 2) /* Intel FlexPriority */
 #define X86_FEATURE_EPT			( 8*32+ 3) /* Intel Extended Page Table */
 #define X86_FEATURE_VPID		( 8*32+ 4) /* Intel Virtual Processor ID */
+#define X86_FEATURE_SPP			( 8*32+ 5) /* Intel EPT-based Sub-Page Write Protection */

Please do not include X86_FEATURE_SPP. In general I don't like the VMX features word, but apart from that SPP is not a feature that affects all VMs in the same way as EPT or FlexPriority.

Quoted 1 line

So what's a friendly way to let a user check if SPP feature is there?

QEMU for example ships with a program called vmxcap (though it requires root). We also could write a program to analyze the KVM capabilities and print them, and put it in tools/kvm.

Thanks,

Paolo

Quoted 4 lines

QEMU for example ships with a program called vmxcap (though it requires root). We also could write a program to analyze the KVM capabilities and print them, and put it in tools/kvm.

OK, will update vmxcap to add SPP feature bit, thanks!

Quoted 5 lines · Yang Weijiang

Quoted 4 lines

QEMU for example ships with a program called vmxcap (though it requires root). We also could write a program to analyze the KVM capabilities and print them, and put it in tools/kvm.

OK, will update vmxcap to add SPP feature bit, thanks!

It's already there. :)

Paolo

arch/x86/kvm/vmx/spp.c line 205

Quoted 5 lines · Yang Weijiang

arch/x86/kvm/vmx/spp.c new file +5 -0

+			if (old_spte != spp_spte) {
+				spp_spte_set(iter.sptep, spp_spte);
+				//kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu);
+				kvm_flush_remote_tlbs(vcpu->kvm);
+			}

Please do not leave commented code in the middle of the series.

Paolo

arch/x86/kvm/vmx/spp.c line 24

Quoted 5 lines · Yang Weijiang

arch/x86/kvm/vmx/spp.c new file +5 -0

+static int is_spp_shadow_present(u64 pte)
+{
+	return pte & PT_PRESENT_MASK;
+}
+

This should not be needed, is_shadow_present_pte works well for SPP PTEs as well (and in fact you're already using it here and there, so it's confusing to have both).

Paolo

arch/x86/kvm/vmx/spp.c bool is_spp_spte(struct kvm_mmu_page *sp) · line 198

Quoted 9 lines · Yang Weijiang

arch/x86/kvm/vmx/spp.c +8 -0

+/*
+ * all vcpus share the same SPPT, vcpu->arch.mmu->sppt_root points to same
+ * SPPT root page, so any vcpu will do.
+ */
+static struct kvm_vcpu *kvm_spp_get_vcpu(struct kvm *kvm)
+{
+	struct kvm_vcpu *vcpu = NULL;
+	int idx;

Is this true? Perhaps you need one with VALID_PAGE(vcpu->arch.mmu->sppt_root) for kvm_spp_set_permission?

Also, since vcpu->arch.mmu->sppt_root is the same for all vCPUs, perhaps it should be kvm->arch.sppt_root instead?

If you can get rid of this function, it would be much better (but if you cannot, kvm_get_vcpu(kvm, 0) should give the same result).

Review

Quoted 3 lines

(quoted diff) +2 -0

+	if (npages > SUBPAGE_MAX_BITMAP)
+		return -EFAULT;

This is not needed here, the restriction only applies to the ioctl.

Paolo

arch/x86/kvm/x86.c +4 -0

+	case KVM_INIT_SPP: {
+		r = kvm_vm_ioctl_init_spp(kvm);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}

include/uapi/linux/kvm.h +0 -0

index 9460830de536..700f0825336d 100644

Quoted 15 lines · Yang Weijiang

arch/x86/kvm/x86.c +4 -0

+	case KVM_INIT_SPP: {
+		r = kvm_vm_ioctl_init_spp(kvm);
+		break;
+	}
 	default:
 		r = -ENOTTY;
 	}

include/uapi/linux/kvm.h +0 -0

index 9460830de536..700f0825336d 100644
@@ -1257,6 +1257,9 @@ struct kvm_vfio_spapr_tce {
 					struct kvm_userspace_memory_region)
 #define KVM_SET_TSS_ADDR          _IO(KVMIO,   0x47)
 #define KVM_SET_IDENTITY_MAP_ADDR _IOW(KVMIO,  0x48, __u64)

include/uapi/linux/kvm.h struct kvm_vfio_spapr_tce { · line 1262

Quoted 3 lines

include/uapi/linux/kvm.h +3 -0

+#define KVM_SUBPAGES_GET_ACCESS   _IOR(KVMIO,  0x49, __u64)
+#define KVM_SUBPAGES_SET_ACCESS   _IOW(KVMIO,  0x4a, __u64)
+#define KVM_INIT_SPP              _IOW(KVMIO,  0x4b, __u64)

You also need to define a capability and return a value for it in kvm_vm_ioctl_check_extension. We could return SUBPAGE_MAX_BITMAP (now KVM_SUBPAGE_MAX_PAGES). And instead of introducing KVM_INIT_SPP, you can then use KVM_ENABLE_CAP on the new capability.

Paolo

arch/x86/kvm/mmu.c static bool fast_page_fault(struct kvm_vcpu *vcpu, gva_t gva, int level, · line 3478

Quoted 5 lines · Yang Weijiang

arch/x86/kvm/mmu.c +5 -0

+			if (spte & PT_SPP_MASK) {
+				fault_handled = true;
+				vcpu->run->exit_reason = KVM_EXIT_SPP;
+				vcpu->run->spp.addr = gva;
+				kvm_skip_emulated_instruction(vcpu);

Do you really want to skip the current instruction? Who will do the write?

arch/x86/kvm/vmx/vmx.c static int handle_monitor_trap(struct kvm_vcpu *vcpu) · line 5376

Quoted 1 line

arch/x86/kvm/vmx/vmx.c +1 -0

+		pr_info("SPP - SPPT entry missing! gfn = 0x%llx\n", gfn);

Please replace pr_info with a tracepoint.

arch/x86/kvm/vmx/vmx.c static int handle_monitor_trap(struct kvm_vcpu *vcpu) · line 5379

Quoted 3 lines

arch/x86/kvm/vmx/vmx.c +3 -0

+		slot = gfn_to_memslot(vcpu->kvm, gfn);
+		if (!slot)
+			return -EFAULT;

You want either a goto to the misconfig case, so that there is a warn

Quoted 4 lines

arch/x86/kvm/vmx/vmx.c +4 -0

+		spp_info.base_gfn = gfn;
+		spp_info.npages = 1;
+
+		spin_lock(&vcpu->kvm->mmu_lock);

arch/x86/kvm/vmx/vmx.c static int handle_monitor_trap(struct kvm_vcpu *vcpu) · line 5391

Quoted 2 lines

arch/x86/kvm/vmx/vmx.c +2 -0

+		ret = kvm_spp_get_permission(vcpu->kvm, &spp_info);
+		if (ret == 1) {

Can you clarify when ret will not be 1? In this case you already have a slot, so it seems to me that you do not need to go through kvm_spp_get_permission and you can just test "if (kvm->arch.spp_active)". But then, spp_active should be 1 if you get here, I think?

arch/x86/kvm/vmx/vmx.c static int handle_monitor_trap(struct kvm_vcpu *vcpu) · line 5423

Quoted 2 lines

arch/x86/kvm/vmx/vmx.c +2 -0

+	pr_alert("SPP - SPPT Misconfiguration!\n");
+	return 0;

pr_alert not needed since you've just warned.

Paolo

Quoted 9 lines · Paolo Bonzini

arch/x86/kvm/mmu.c static bool fast_page_fault(struct kvm_vcpu *vcpu, gva_t gva, int level, · line 3478

Quoted 5 lines · Yang Weijiang

arch/x86/kvm/mmu.c +5 -0

+			if (spte & PT_SPP_MASK) {
+				fault_handled = true;
+				vcpu->run->exit_reason = KVM_EXIT_SPP;
+				vcpu->run->spp.addr = gva;
+				kvm_skip_emulated_instruction(vcpu);

Do you really want to skip the current instruction? Who will do the write?

If the destination memory is SPP protected, the target memory is expected unchanged on a "write op" in guest, so would like to skip current instruction.

Quoted 10 lines · Yang Weijiang

Quoted 9 lines · Paolo Bonzini

arch/x86/kvm/mmu.c static bool fast_page_fault(struct kvm_vcpu *vcpu, gva_t gva, int level, · line 3478

Quoted 5 lines · Yang Weijiang

arch/x86/kvm/mmu.c +5 -0

+			if (spte & PT_SPP_MASK) {
+				fault_handled = true;
+				vcpu->run->exit_reason = KVM_EXIT_SPP;
+				vcpu->run->spp.addr = gva;
+				kvm_skip_emulated_instruction(vcpu);

Do you really want to skip the current instruction? Who will do the write?

Quoted 3 lines

If the destination memory is SPP protected, the target memory is expected unchanged on a "write op" in guest, so would like to skip current instruction.

This is how you are expecting SPP to be used, but another possibility is to unprotect and reenter the guest. In this case kvm_skip_emulated_instruction would be wrong (and once this decision is made, it would be very, very hard to change it).

However, you clearly need a way to skip the instruction, and for that you could store the current instruction length in vcpu->run->spp. Then userspace can adjust RIP manually if desired.

Thanks,

Paolo

arch/x86/kvm/mmu.c int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code, · line 5440

Quoted 8 lines · Yang Weijiang

arch/x86/kvm/mmu.c +4 -0

@@ -5400,6 +5434,10 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, u64 error_code,
 		r = vcpu->arch.mmu->page_fault(vcpu, cr2,
 					       lower_32_bits(error_code),
 					       false);
+
+		if (vcpu->run->exit_reason == KVM_EXIT_SPP)
+			return 0;
+

Instead of this, please add a RET_PF_USERSPACE case to the RET_PF_* enum in mmu.c.

Paolo

arch/x86/kvm/mmu.c +7 -0

+			/*
+			 * if it's EPT leaf entry and the physical page is
+			 * SPP protected, then re-enable SPP protection for
+			 * the page.
+			 */
+			if (kvm->arch.spp_active &&
+			    level == PT_PAGE_TABLE_LEVEL) {

Quoted 12 lines · Yang Weijiang

arch/x86/kvm/mmu.c +12 -0

+			/*
+			 * if it's EPT leaf entry and the physical page is
+			 * SPP protected, then re-enable SPP protection for
+			 * the page.
+			 */
+			if (kvm->arch.spp_active &&
+			    level == PT_PAGE_TABLE_LEVEL) {
+				struct kvm_subpage spp_info = {0};
+				int i;
+
+				spp_info.base_gfn = gfn;
+				spp_info.npages = 1;

arch/x86/kvm/mmu.c static int kvm_set_pte_rmapp(struct kvm *kvm, struct kvm_rmap_head *rmap_head, · line 1847

Quoted 5 lines

arch/x86/kvm/mmu.c +5 -0

+				i = kvm_spp_get_permission(kvm, &spp_info);
+				if (i == 1 &&
+				    spp_info.access_map[0] != FULL_SPP_ACCESS)
+					new_spte |= PT_SPP_MASK;
+			}

This can use gfn_to_subpage_wp_info directly (or is_spp_protected if you prefer).

Paolo

EPT-Based Sub-Page write Protection(SPP) allows Virtual Machine Monitor(VMM) specify write-permission for guest physical memory at a sub-page(128 byte) granularity. When SPP works, HW enforces write-access check for sub-pages within a protected 4KB page.

The feature targets to provide fine-grained memory protection for usages such as memory guard and VM introspection etc.

SPP is active when the "sub-page write protection" (bit 23) is 1 in Secondary VM-Execution Controls. The feature is backed with a Sub-Page Permission Table(SPPT), and subpage permission vector is stored in the leaf entry of SPPT. The root page is referenced via a Sub-Page Permission Table Pointer (SPPTP) in VMCS.

To enable SPP for guest memory, the guest page should be first mapped to a 4KB EPT entry, then set SPP bit 61 of the corresponding entry. While HW walks EPT, it traverses SPPT with the gpa to look up the sub-page permission vector within SPPT leaf entry. If the corresponding bit is set, write to sub-page is permitted, otherwise, SPP induced EPT violation is generated.

This patch serial passed SPP function test and selftest on Ice-Lake platform.

Please refer to the SPP introduction document in this patch set and Intel SDM for details:

Intel SDM:
https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf

SPP selftest patch:
https://lkml.org/lkml/2019/6/18/1197

Patch 1: Documentation for SPP and related API. Patch 2: Add control flags for Sub-Page Protection(SPP). Patch 3: Add SPP Table setup functions.
Patch 4: Add functions to create/destroy SPP bitmap block. Patch 5: Introduce user-space SPP IOCTLs.
Patch 6: Set up SPP paging table at vmentry/vmexit. Patch 7: Enable Lazy mode SPP protection.
Patch 8: Handle SPP protected pages when VM memory changes. Patch 9: Add SPP protection check in emulation case.

Change logs:
V6 -> V7:
1. Configured all available protected pages once SPP induced vmexit happens since there's no PRESENT bit in SPPT leaf entry.
2. Changed SPP protection check flow in tdp_page_fault().
3. Code refactor and minior fixes.

V5 -> V6:
1. Added SPP protection patch for emulation cases per Jim's review.
2. Modified documentation and added API description per Jim's review.
3. Other minior changes suggested by Jim.

V4 -> V5:
1. Enable SPP support for Hugepage(1GB/2MB) to extend application.
2. Make SPP miss vm-exit handler as the unified place to set up SPPT.
3. If SPP protected pages are access-tracked or dirty-page-tracked, store SPP flag in reserved address bit, restore it in fast_page_fault() handler.
4. Move SPP specific functions to vmx/spp.c and vmx/spp.h
5. Rebased code to kernel v5.3
6. Other change suggested by KVM community.

V3 -> V4:
1. Modified documentation to make it consistent with patches.
2. Allocated SPPT root page in init_spp() instead of vmx_set_cr3() to avoid SPPT miss error.
3. Added back co-developers and sign-offs.

V2 -> V3:
1. Rebased patches to kernel 5.1 release
2. Deferred SPPT setup to EPT fault handler if the page is not available while set_subpage() is being called.
3. Added init IOCTL to reduce extra cost if SPP is not used.
4. Refactored patch structure, cleaned up cross referenced functions.
5. Added code to deal with memory swapping/migration/shrinker cases.

V2 -> V1:
1. Rebased to 4.20-rc1
2. Move VMCS change to a separated patch.
3. Code refine and Bug fix

Yang Weijiang (9):
Documentation: Introduce EPT based Subpage Protection and related
ioctls
vmx: spp: Add control flags for Sub-Page Protection(SPP)
mmu: spp: Add SPP Table setup functions
mmu: spp: Add functions to create/destroy SPP bitmap block
x86: spp: Introduce user-space SPP IOCTLs
vmx: spp: Set up SPP paging table at vmentry/vmexit
mmu: spp: Enable Lazy mode SPP protection
mmu: spp: Handle SPP protected pages when VM memory changes
x86: spp: Add SPP protection check in emulation.

 Documentation/virt/kvm/api.txt        |  46 ++
 Documentation/virtual/kvm/spp_kvm.txt | 180 +++++++
 arch/x86/include/asm/cpufeatures.h    |   1 +
 arch/x86/include/asm/kvm_host.h       |  10 +-
 arch/x86/include/asm/vmx.h            |  10 +
 arch/x86/include/uapi/asm/vmx.h       |   2 +
 arch/x86/kernel/cpu/intel.c           |   4 +
 arch/x86/kvm/mmu.c                    |  78 ++-
 arch/x86/kvm/mmu.h                    |   2 +
 arch/x86/kvm/vmx/capabilities.h       |   5 +
 arch/x86/kvm/vmx/spp.c                | 651 ++++++++++++++++++++++++++
 arch/x86/kvm/vmx/spp.h                |  28 ++
 arch/x86/kvm/vmx/vmx.c                | 113 +++++
 arch/x86/kvm/x86.c                    |  87 ++++
 include/uapi/linux/kvm.h              |  17 +
 15 files changed, 1232 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/virtual/kvm/spp_kvm.txt
 create mode 100644 arch/x86/kvm/vmx/spp.c
 create mode 100644 arch/x86/kvm/vmx/spp.h

Quoted 22 lines · Yang Weijiang

EPT-Based Sub-Page write Protection(SPP) allows Virtual Machine Monitor(VMM) specify write-permission for guest physical memory at a sub-page(128 byte) granularity. When SPP works, HW enforces write-access check for sub-pages within a protected 4KB page.

The feature targets to provide fine-grained memory protection for usages such as memory guard and VM introspection etc.

SPP is active when the "sub-page write protection" (bit 23) is 1 in Secondary VM-Execution Controls. The feature is backed with a Sub-Page Permission Table(SPPT), and subpage permission vector is stored in the leaf entry of SPPT. The root page is referenced via a Sub-Page Permission Table Pointer (SPPTP) in VMCS.

To enable SPP for guest memory, the guest page should be first mapped to a 4KB EPT entry, then set SPP bit 61 of the corresponding entry. While HW walks EPT, it traverses SPPT with the gpa to look up the sub-page permission vector within SPPT leaf entry. If the corresponding bit is set, write to sub-page is permitted, otherwise, SPP induced EPT violation is generated.

This patch serial passed SPP function test and selftest on Ice-Lake platform.

Quoted 8 lines

Please refer to the SPP introduction document in this patch set and Intel SDM for details:

Intel SDM:
https://software.intel.com/sites/default/files/managed/39/c5/325462-sdm-vol-1-2abcd-3abcd.pdf

SPP selftest patch:
https://lkml.org/lkml/2019/6/18/1197

On top of the changes I sent for the individual patches, please move vmx/spp.c to mmu/spp.c, and vmx/spp.h to spp.h (I've just sent a patch to create the mmu/ directory). Also, please include the selftest in this series.

Paolo