[RESEND,v4,8/9] KVM: MMU: Enable Lazy mode SPPT setup
diff mbox series

Message ID 20190814070403.6588-9-weijiang.yang@intel.com
State New
Headers show
Series
  • Enable Sub-page Write Protection Support
Related show

Commit Message

Yang Weijiang Aug. 14, 2019, 7:04 a.m. UTC
If SPP subpages are set while the physical page are not
available in EPT leaf entry, the mapping is first stored
in SPP access bitmap buffer. SPPT setup is deferred to
access to the protected page, in EPT page fault handler,
the SPPT enries are set up.

Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
---
 arch/x86/kvm/mmu.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Paolo Bonzini Aug. 19, 2019, 2:46 p.m. UTC | #1
On 14/08/19 09:04, Yang Weijiang wrote:
> +
> +	if (vcpu->kvm->arch.spp_active && level == PT_PAGE_TABLE_LEVEL)
> +		kvm_enable_spp_protection(vcpu->kvm, gfn);
> +

This would not enable SPP if the guest is backed by huge pages.
Instead, either the PT_PAGE_TABLE_LEVEL level must be forced for all
pages covered by SPP ranges, or (better) kvm_enable_spp_protection must
be able to cover multiple pages at once.

Paolo
Yang Weijiang Aug. 20, 2019, 1:12 p.m. UTC | #2
On Mon, Aug 19, 2019 at 04:46:54PM +0200, Paolo Bonzini wrote:
> On 14/08/19 09:04, Yang Weijiang wrote:
> > +
> > +	if (vcpu->kvm->arch.spp_active && level == PT_PAGE_TABLE_LEVEL)
> > +		kvm_enable_spp_protection(vcpu->kvm, gfn);
> > +
> 
> This would not enable SPP if the guest is backed by huge pages.
> Instead, either the PT_PAGE_TABLE_LEVEL level must be forced for all
> pages covered by SPP ranges, or (better) kvm_enable_spp_protection must
> be able to cover multiple pages at once.
> 
> Paolo
OK, I'll figure out how to make it, thanks!
Yang Weijiang Sept. 4, 2019, 1:49 p.m. UTC | #3
On Tue, Aug 20, 2019 at 09:12:14PM +0800, Yang Weijiang wrote:
> On Mon, Aug 19, 2019 at 04:46:54PM +0200, Paolo Bonzini wrote:
> > On 14/08/19 09:04, Yang Weijiang wrote:
> > > +
> > > +	if (vcpu->kvm->arch.spp_active && level == PT_PAGE_TABLE_LEVEL)
> > > +		kvm_enable_spp_protection(vcpu->kvm, gfn);
> > > +
> > 
> > This would not enable SPP if the guest is backed by huge pages.
> > Instead, either the PT_PAGE_TABLE_LEVEL level must be forced for all
> > pages covered by SPP ranges, or (better) kvm_enable_spp_protection must
> > be able to cover multiple pages at once.
> > 
> > Paolo
> OK, I'll figure out how to make it, thanks!
Hi, Paolo,
Regarding this change, I have some concerns, splitting EPT huge page
entries(e.g., 1GB page)will take long time compared with normal EPT page
fault processing, especially for multiple vcpus/pages,so the in-flight time increases,
but HW walks EPT for translations in the meantime, would it bring any side effect? 
or there's a way to mitigate it?

Thanks!
Paolo Bonzini Sept. 9, 2019, 5:10 p.m. UTC | #4
On 04/09/19 15:49, Yang Weijiang wrote:
>>> This would not enable SPP if the guest is backed by huge pages.
>>> Instead, either the PT_PAGE_TABLE_LEVEL level must be forced for all
>>> pages covered by SPP ranges, or (better) kvm_enable_spp_protection must
>>> be able to cover multiple pages at once.
>>>
>>> Paolo
>> OK, I'll figure out how to make it, thanks!
> Hi, Paolo,
> Regarding this change, I have some concerns, splitting EPT huge page
> entries(e.g., 1GB page)will take long time compared with normal EPT page
> fault processing, especially for multiple vcpus/pages,so the in-flight time increases,
> but HW walks EPT for translations in the meantime, would it bring any side effect? 
> or there's a way to mitigate it?

Sub-page permissions are only defined on EPT PTEs, not on large pages.
Therefore, in order to allow subpage permissions the EPT page tables
must already be split.

Paolo
Yang Weijiang Sept. 11, 2019, 12:23 a.m. UTC | #5
On Mon, Sep 09, 2019 at 07:10:22PM +0200, Paolo Bonzini wrote:
> On 04/09/19 15:49, Yang Weijiang wrote:
> >>> This would not enable SPP if the guest is backed by huge pages.
> >>> Instead, either the PT_PAGE_TABLE_LEVEL level must be forced for all
> >>> pages covered by SPP ranges, or (better) kvm_enable_spp_protection must
> >>> be able to cover multiple pages at once.
> >>>
> >>> Paolo
> >> OK, I'll figure out how to make it, thanks!
> > Hi, Paolo,
> > Regarding this change, I have some concerns, splitting EPT huge page
> > entries(e.g., 1GB page)will take long time compared with normal EPT page
> > fault processing, especially for multiple vcpus/pages,so the in-flight time increases,
> > but HW walks EPT for translations in the meantime, would it bring any side effect? 
> > or there's a way to mitigate it?
> 
> Sub-page permissions are only defined on EPT PTEs, not on large pages.
> Therefore, in order to allow subpage permissions the EPT page tables
> must already be split.
> 
> Paolo
Thanks, I've added code to handle hugepage, will be included in
next version patch.

Patch
diff mbox series

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 419878301375..f017fe6cd67b 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -4304,6 +4304,26 @@  check_hugepage_cache_consistency(struct kvm_vcpu *vcpu, gfn_t gfn, int level)
 	return kvm_mtrr_check_gfn_range_consistency(vcpu, gfn, page_num);
 }
 
+static int kvm_enable_spp_protection(struct kvm *kvm, u64 gfn)
+{
+	struct kvm_subpage spp_info = {0};
+	struct kvm_memory_slot *slot;
+
+	slot = gfn_to_memslot(kvm, gfn);
+	if (!slot)
+		return -EFAULT;
+
+	spp_info.base_gfn = gfn;
+	spp_info.npages = 1;
+
+	if (kvm_mmu_get_subpages(kvm, &spp_info, true) < 0)
+		return -EFAULT;
+
+	if (spp_info.access_map[0] != FULL_SPP_ACCESS)
+		kvm_mmu_set_subpages(kvm, &spp_info, true);
+
+	return 0;
+}
 static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa, u32 error_code,
 			  bool prefault)
 {
@@ -4355,6 +4375,10 @@  static int tdp_page_fault(struct kvm_vcpu *vcpu, gva_t gpa, u32 error_code,
 	if (likely(!force_pt_level))
 		transparent_hugepage_adjust(vcpu, &gfn, &pfn, &level);
 	r = __direct_map(vcpu, write, map_writable, level, gfn, pfn, prefault);
+
+	if (vcpu->kvm->arch.spp_active && level == PT_PAGE_TABLE_LEVEL)
+		kvm_enable_spp_protection(vcpu->kvm, gfn);
+
 	spin_unlock(&vcpu->kvm->mmu_lock);
 
 	return r;