Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-20 Thread Yang Weijiang
On Mon, Aug 19, 2019 at 05:13:47PM +0200, Paolo Bonzini wrote:
> On 14/08/19 09:03, Yang Weijiang wrote:
> >  static struct kvm_x86_ops vmx_x86_ops __ro_after_init = {
> > .cpu_has_kvm_support = cpu_has_kvm_support,
> > .disabled_by_bios = vmx_disabled_by_bios,
> > @@ -7740,6 +7783,11 @@ static struct kvm_x86_ops vmx_x86_ops 
> > __ro_after_init = {
> > .get_vmcs12_pages = NULL,
> > .nested_enable_evmcs = NULL,
> > .need_emulation_on_page_fault = vmx_need_emulation_on_page_fault,
> > +
> > +   .get_spp_status = vmx_get_spp_status,
> > +   .get_subpages = vmx_get_subpages,
> > +   .set_subpages = vmx_set_subpages,
> > +   .init_spp = vmx_init_spp,
> >  };
> 
> There's no need for the get_subpages kvm_x86_ops.  You do need init_spp
> of course, but you do not need get_spp_status either; instead you can
> check if init_spp is NULL (if NULL, SPP is not supported).
So first set .init_spp = NULL, then if all SPP preconditions meet, set
.init_spp = vmx_init_spp?

> In addition, kvm_mmu_set_subpages should not set up the SPP pages.  This
> should be handled entirely by handle_spp when the processor reports an
> SPPT miss, so you do not need that callback either.  You may need a
> flush_subpages callback, called by kvm_mmu_set_subpages to clear the SPP
> page tables for a given GPA range.  The next access then will cause an
> SPPT miss.
Good suggestion, thanks! Will do change.

> Finally, please move all SPP-related code to arch/x86/kvm/vmx/{spp.c,spp.h}.
>
Sure.

> Thanks,
> 
> Paolo
> 
 


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-20 Thread Yang Weijiang
On Mon, Aug 19, 2019 at 05:05:22PM +0200, Paolo Bonzini wrote:
> On 14/08/19 09:03, Yang Weijiang wrote:
> > +
> > +int kvm_mmu_get_subpages(struct kvm *kvm, struct kvm_subpage *spp_info,
> > +bool mmu_locked)
> > +{
> > +   u32 *access = spp_info->access_map;
> > +   gfn_t gfn = spp_info->base_gfn;
> > +   int npages = spp_info->npages;
> > +   struct kvm_memory_slot *slot;
> > +   int i;
> > +   int ret;
> > +
> > +   if (!kvm->arch.spp_active)
> > + return -ENODEV;
> > +
> > +   if (!mmu_locked)
> > + spin_lock(>mmu_lock);
> > +
> 
> Do not add this argument.  Just lock mmu_lock in the callers.
> 
> Paolo
OK, will remove it, thanks!


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-20 Thread Yang Weijiang
On Mon, Aug 19, 2019 at 05:15:01PM +0200, Paolo Bonzini wrote:
> On 19/08/19 04:08, Yang Weijiang wrote:
> >> KVM_GET_NESTED_STATE has the requested information. If
> >> data.vmx.vmxon_pa is anything other than -1, then the vCPU is in VMX
> >> operation. If (flags & KVM_STATE_NESTED_GUEST_MODE), then L2 is
> >> active.
> > Thanks Jim, I'll reference the code and make necessary change in next
> > SPP patch release.
> 
> Since SPP will not be used very much in the beginning, it would be
> simplest to enable it only if nested==0.
> 
> Paolo
Thanks Paolo! Sure, will make the change and document it.


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-19 Thread Paolo Bonzini
On 19/08/19 04:08, Yang Weijiang wrote:
>> KVM_GET_NESTED_STATE has the requested information. If
>> data.vmx.vmxon_pa is anything other than -1, then the vCPU is in VMX
>> operation. If (flags & KVM_STATE_NESTED_GUEST_MODE), then L2 is
>> active.
> Thanks Jim, I'll reference the code and make necessary change in next
> SPP patch release.

Since SPP will not be used very much in the beginning, it would be
simplest to enable it only if nested==0.

Paolo


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-19 Thread Paolo Bonzini
On 14/08/19 09:03, Yang Weijiang wrote:
> init_spp() must be called before {get, set}_subpage
> functions, it creates subpage access bitmaps for memory pages
> and issues a KVM request to setup SPPT root pages.
> 
> kvm_mmu_set_subpages() is to enable SPP bit in EPT leaf page
> and setup corresponding SPPT entries. The mmu_lock
> is held before above operation. If it's called in EPT fault and
> SPPT mis-config induced handler, mmu_lock is acquired outside
> the function, otherwise, it's acquired inside it.
> 
> kvm_mmu_get_subpages() is used to query access bitmap for
> protected page, it's also used in EPT fault handler to check
> whether the fault EPT page is SPP protected as well.
> 
> Co-developed-by: He Chen 
> Signed-off-by: He Chen 
> Co-developed-by: Zhang Yi 
> Signed-off-by: Zhang Yi 
> Signed-off-by: Yang Weijiang 
> ---
>  arch/x86/include/asm/kvm_host.h |  18 
>  arch/x86/include/asm/vmx.h  |   2 +
>  arch/x86/kvm/mmu.c  | 160 
>  arch/x86/kvm/vmx/vmx.c  |  48 ++
>  arch/x86/kvm/x86.c  |  40 
>  include/linux/kvm_host.h|   4 +-
>  include/uapi/linux/kvm.h|   9 ++
>  7 files changed, 280 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 44f6e1757861..5c4882015acc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -398,8 +398,13 @@ struct kvm_mmu {
>   void (*invlpg)(struct kvm_vcpu *vcpu, gva_t gva, hpa_t root_hpa);
>   void (*update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>  u64 *spte, const void *pte);
> + int (*get_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> + int (*set_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> + int (*init_spp)(struct kvm *kvm);
> +
>   hpa_t root_hpa;
>   gpa_t root_cr3;
> + hpa_t sppt_root;
>   union kvm_mmu_role mmu_role;
>   u8 root_level;
>   u8 shadow_root_level;
> @@ -927,6 +932,8 @@ struct kvm_arch {
>  
>   bool guest_can_read_msr_platform_info;
>   bool exception_payload_enabled;
> +
> + bool spp_active;
>  };
>  
>  struct kvm_vm_stat {
> @@ -1199,6 +1206,11 @@ struct kvm_x86_ops {
>   uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu);
>  
>   bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu);
> +
> + bool (*get_spp_status)(void);
> + int (*get_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> + int (*set_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> + int (*init_spp)(struct kvm *kvm);
>  };
>  
>  struct kvm_arch_async_pf {
> @@ -1417,6 +1429,12 @@ void kvm_mmu_invlpg(struct kvm_vcpu *vcpu, gva_t gva);
>  void kvm_mmu_invpcid_gva(struct kvm_vcpu *vcpu, gva_t gva, unsigned long 
> pcid);
>  void kvm_mmu_new_cr3(struct kvm_vcpu *vcpu, gpa_t new_cr3, bool 
> skip_tlb_flush);
>  
> +int kvm_mmu_get_subpages(struct kvm *kvm, struct kvm_subpage *spp_info,
> +  bool mmu_locked);
> +int kvm_mmu_set_subpages(struct kvm *kvm, struct kvm_subpage *spp_info,
> +  bool mmu_locked);
> +int kvm_mmu_init_spp(struct kvm *kvm);
> +
>  void kvm_enable_tdp(void);
>  void kvm_disable_tdp(void);
>  
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index a2c9e18e0ad7..6cb05ac07453 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -224,6 +224,8 @@ enum vmcs_field {
>   XSS_EXIT_BITMAP_HIGH= 0x202D,
>   ENCLS_EXITING_BITMAP= 0x202E,
>   ENCLS_EXITING_BITMAP_HIGH   = 0x202F,
> + SPPT_POINTER= 0x2030,
> + SPPT_POINTER_HIGH   = 0x2031,
>   TSC_MULTIPLIER  = 0x2032,
>   TSC_MULTIPLIER_HIGH = 0x2033,
>   GUEST_PHYSICAL_ADDRESS  = 0x2400,
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 16e390fd6e58..16d3ca544b67 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -3756,6 +3756,9 @@ void kvm_mmu_free_roots(struct kvm_vcpu *vcpu, struct 
> kvm_mmu *mmu,
>   (mmu->root_level >= PT64_ROOT_4LEVEL || mmu->direct_map)) {
>   mmu_free_root_page(vcpu->kvm, >root_hpa,
>  _list);
> + if (vcpu->kvm->arch.spp_active)
> + mmu_free_root_page(vcpu->kvm, >sppt_root,
> +_list);
>   } else {
>   for (i = 0; i < 4; ++i)
>   if (mmu->pae_root[i] != 0)
> @@ -4414,6 +4417,158 @@ int kvm_mmu_setup_spp_structure(struct kvm_vcpu *vcpu,
>   return ret;
>  }
>  EXPORT_SYMBOL_GPL(kvm_mmu_setup_spp_structure);
> +
> +int kvm_mmu_init_spp(struct kvm *kvm)
> +{
> + int i, ret;
> + struct kvm_vcpu 

Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-19 Thread Paolo Bonzini
On 14/08/19 09:03, Yang Weijiang wrote:
> +
> +int kvm_mmu_get_subpages(struct kvm *kvm, struct kvm_subpage *spp_info,
> +  bool mmu_locked)
> +{
> + u32 *access = spp_info->access_map;
> + gfn_t gfn = spp_info->base_gfn;
> + int npages = spp_info->npages;
> + struct kvm_memory_slot *slot;
> + int i;
> + int ret;
> +
> + if (!kvm->arch.spp_active)
> +   return -ENODEV;
> +
> + if (!mmu_locked)
> +   spin_lock(>mmu_lock);
> +

Do not add this argument.  Just lock mmu_lock in the callers.

Paolo


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-19 Thread Yang Weijiang
On Thu, Aug 15, 2019 at 04:03:31PM +0200, Vitaly Kuznetsov wrote:
> Yang Weijiang  writes:
> 
> > After looked into the issue and others, I feel to make SPP co-existing
> > with nested VM is not good, the major reason is, L1 pages protected by
> > SPP are transparent to L1 VM, if it launches L2 VM, probably the
> > pages would be allocated to L2 VM, and that will bother to L1 and L2.
> > Given the feature is new and I don't see nested VM can benefit
> > from it right now, I would like to make SPP and nested feature mutually
> > exclusive, i.e., detecting if the other part is active before activate one
> > feature,what do you think of it? 
> 
> I was mostly worried about creating a loophole (if I understand
> correctly) for guests to defeat SPP protection: just launching a nested
> guest and giving it a protected page. I don't see a problem if we limit
> SPP to non-nested guests as step 1: we, however, need to document this
> side-effect of the ioctl. Also, if you decide to do this enforecement,
> I'd suggest you forbid VMLAUCH/VMRESUME and not VMXON as kvm module
> loads in linux guests automatically when the hardware is suitable.
> 
> Thanks,
> 
> -- 
> Vitaly
OK, I'll follow your suggestion to add the exclusion, thanks!


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-18 Thread Yang Weijiang
On Fri, Aug 16, 2019 at 11:19:46AM -0700, Jim Mattson wrote:
> On Fri, Aug 16, 2019 at 6:29 AM Yang Weijiang  wrote:
> 
> > Thanks Jim and Sean! Could we add a new flag in kvm to identify if nested 
> > VM is on
> > or off? That would make things easier. When VMLAUNCH is trapped,
> > set the flag, if VMXOFF is trapped, clear the flag.
> 
> KVM_GET_NESTED_STATE has the requested information. If
> data.vmx.vmxon_pa is anything other than -1, then the vCPU is in VMX
> operation. If (flags & KVM_STATE_NESTED_GUEST_MODE), then L2 is
> active.
Thanks Jim, I'll reference the code and make necessary change in next
SPP patch release.


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-16 Thread Jim Mattson
On Fri, Aug 16, 2019 at 6:29 AM Yang Weijiang  wrote:

> Thanks Jim and Sean! Could we add a new flag in kvm to identify if nested VM 
> is on
> or off? That would make things easier. When VMLAUNCH is trapped,
> set the flag, if VMXOFF is trapped, clear the flag.

KVM_GET_NESTED_STATE has the requested information. If
data.vmx.vmxon_pa is anything other than -1, then the vCPU is in VMX
operation. If (flags & KVM_STATE_NESTED_GUEST_MODE), then L2 is
active.


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-16 Thread Yang Weijiang
On Thu, Aug 15, 2019 at 09:38:44AM -0700, Sean Christopherson wrote:
> On Thu, Aug 15, 2019 at 09:25:41AM -0700, Jim Mattson wrote:
> > On Thu, Aug 15, 2019 at 6:41 AM Yang Weijiang  
> > wrote:
> > 
> > > Hi, Vitaly,
> > > After looked into the issue and others, I feel to make SPP co-existing
> > > with nested VM is not good, the major reason is, L1 pages protected by
> > > SPP are transparent to L1 VM, if it launches L2 VM, probably the
> > > pages would be allocated to L2 VM, and that will bother to L1 and L2.
> > > Given the feature is new and I don't see nested VM can benefit
> > > from it right now, I would like to make SPP and nested feature mutually
> > > exclusive, i.e., detecting if the other part is active before activate one
> > > feature,what do you think of it?
> > > thanks!
> > 
> > How do you propose making the features mutually exclusive?
> 
> I haven't looked at the details or the end to end flow, but would it make
> sense to exit to userspace on nested VMLAUNCH/VMRESUME if there are SPP
> mappings?  And have the SPP ioctl() kick vCPUs out of guest.
> 
> KVM already exits on SPP violations, so presumably this is something that
> can be punted to userspace.
Thanks Jim and Sean! Could we add a new flag in kvm to identify if nested VM is 
on
or off? That would make things easier. When VMLAUNCH is trapped,
set the flag, if VMXOFF is trapped, clear the flag.


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-15 Thread Sean Christopherson
On Thu, Aug 15, 2019 at 09:25:41AM -0700, Jim Mattson wrote:
> On Thu, Aug 15, 2019 at 6:41 AM Yang Weijiang  wrote:
> 
> > Hi, Vitaly,
> > After looked into the issue and others, I feel to make SPP co-existing
> > with nested VM is not good, the major reason is, L1 pages protected by
> > SPP are transparent to L1 VM, if it launches L2 VM, probably the
> > pages would be allocated to L2 VM, and that will bother to L1 and L2.
> > Given the feature is new and I don't see nested VM can benefit
> > from it right now, I would like to make SPP and nested feature mutually
> > exclusive, i.e., detecting if the other part is active before activate one
> > feature,what do you think of it?
> > thanks!
> 
> How do you propose making the features mutually exclusive?

I haven't looked at the details or the end to end flow, but would it make
sense to exit to userspace on nested VMLAUNCH/VMRESUME if there are SPP
mappings?  And have the SPP ioctl() kick vCPUs out of guest.

KVM already exits on SPP violations, so presumably this is something that
can be punted to userspace.


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-15 Thread Jim Mattson
On Thu, Aug 15, 2019 at 6:41 AM Yang Weijiang  wrote:

> Hi, Vitaly,
> After looked into the issue and others, I feel to make SPP co-existing
> with nested VM is not good, the major reason is, L1 pages protected by
> SPP are transparent to L1 VM, if it launches L2 VM, probably the
> pages would be allocated to L2 VM, and that will bother to L1 and L2.
> Given the feature is new and I don't see nested VM can benefit
> from it right now, I would like to make SPP and nested feature mutually
> exclusive, i.e., detecting if the other part is active before activate one
> feature,what do you think of it?
> thanks!

How do you propose making the features mutually exclusive?


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-15 Thread Vitaly Kuznetsov
Yang Weijiang  writes:

> After looked into the issue and others, I feel to make SPP co-existing
> with nested VM is not good, the major reason is, L1 pages protected by
> SPP are transparent to L1 VM, if it launches L2 VM, probably the
> pages would be allocated to L2 VM, and that will bother to L1 and L2.
> Given the feature is new and I don't see nested VM can benefit
> from it right now, I would like to make SPP and nested feature mutually
> exclusive, i.e., detecting if the other part is active before activate one
> feature,what do you think of it? 

I was mostly worried about creating a loophole (if I understand
correctly) for guests to defeat SPP protection: just launching a nested
guest and giving it a protected page. I don't see a problem if we limit
SPP to non-nested guests as step 1: we, however, need to document this
side-effect of the ioctl. Also, if you decide to do this enforecement,
I'd suggest you forbid VMLAUCH/VMRESUME and not VMXON as kvm module
loads in linux guests automatically when the hardware is suitable.

Thanks,

-- 
Vitaly


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-15 Thread Yang Weijiang
On Wed, Aug 14, 2019 at 02:43:39PM +0200, Vitaly Kuznetsov wrote:
> Yang Weijiang  writes:
> 
> > init_spp() must be called before {get, set}_subpage
> > functions, it creates subpage access bitmaps for memory pages
> > and issues a KVM request to setup SPPT root pages.
> >
> > kvm_mmu_set_subpages() is to enable SPP bit in EPT leaf page
> > and setup corresponding SPPT entries. The mmu_lock
> > is held before above operation. If it's called in EPT fault and
> > SPPT mis-config induced handler, mmu_lock is acquired outside
> > the function, otherwise, it's acquired inside it.
> >
> > kvm_mmu_get_subpages() is used to query access bitmap for
> > protected page, it's also used in EPT fault handler to check
> > whether the fault EPT page is SPP protected as well.
> >
> > Co-developed-by: He Chen 
> > Signed-off-by: He Chen 
> > Co-developed-by: Zhang Yi 
> > Signed-off-by: Zhang Yi 
> > Signed-off-by: Yang Weijiang 
> > ---
> >  arch/x86/include/asm/kvm_host.h |  18 
> >  arch/x86/include/asm/vmx.h  |   2 +
> >  arch/x86/kvm/mmu.c  | 160 
> >  arch/x86/kvm/vmx/vmx.c  |  48 ++
> >  arch/x86/kvm/x86.c  |  40 
> >  include/linux/kvm_host.h|   4 +-
> >  include/uapi/linux/kvm.h|   9 ++
> >  7 files changed, 280 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h 
> > b/arch/x86/include/asm/kvm_host.h
> > index 44f6e1757861..5c4882015acc 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -398,8 +398,13 @@ struct kvm_mmu {
> > void (*invlpg)(struct kvm_vcpu *vcpu, gva_t gva, hpa_t root_hpa);
> > void (*update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> >u64 *spte, const void *pte);
> > +   int (*get_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> > +   int (*set_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> > +   int (*init_spp)(struct kvm *kvm);
> > +
> > hpa_t root_hpa;
> > gpa_t root_cr3;
> > +   hpa_t sppt_root;
> 
> (I'm sorry if this was previously discussed, I didn't look into previous
> submissions).
> 
> What happens when we launch a nested guest and switch vcpu->arch.mmu to
> point at arch.guest_mmu? sppt_root will point to INVALID_PAGE and SPP
> won't be enabled in VMCS?
> 
> (I'm sorry again, I'm likely missing something obvious)
> 
> -- 
> Vitaly
Hi, Vitaly,
After looked into the issue and others, I feel to make SPP co-existing
with nested VM is not good, the major reason is, L1 pages protected by
SPP are transparent to L1 VM, if it launches L2 VM, probably the
pages would be allocated to L2 VM, and that will bother to L1 and L2.
Given the feature is new and I don't see nested VM can benefit
from it right now, I would like to make SPP and nested feature mutually
exclusive, i.e., detecting if the other part is active before activate one
feature,what do you think of it? 
thanks! 


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-14 Thread Yang Weijiang
On Wed, Aug 14, 2019 at 02:43:39PM +0200, Vitaly Kuznetsov wrote:
> Yang Weijiang  writes:
> 
> > init_spp() must be called before {get, set}_subpage
> > functions, it creates subpage access bitmaps for memory pages
> > and issues a KVM request to setup SPPT root pages.
> >
> > kvm_mmu_set_subpages() is to enable SPP bit in EPT leaf page
> > and setup corresponding SPPT entries. The mmu_lock
> > is held before above operation. If it's called in EPT fault and
> > SPPT mis-config induced handler, mmu_lock is acquired outside
> > the function, otherwise, it's acquired inside it.
> >
> > kvm_mmu_get_subpages() is used to query access bitmap for
> > protected page, it's also used in EPT fault handler to check
> > whether the fault EPT page is SPP protected as well.
> >
> > Co-developed-by: He Chen 
> > Signed-off-by: He Chen 
> > Co-developed-by: Zhang Yi 
> > Signed-off-by: Zhang Yi 
> > Signed-off-by: Yang Weijiang 
> > ---
> >  arch/x86/include/asm/kvm_host.h |  18 
> >  arch/x86/include/asm/vmx.h  |   2 +
> >  arch/x86/kvm/mmu.c  | 160 
> >  arch/x86/kvm/vmx/vmx.c  |  48 ++
> >  arch/x86/kvm/x86.c  |  40 
> >  include/linux/kvm_host.h|   4 +-
> >  include/uapi/linux/kvm.h|   9 ++
> >  7 files changed, 280 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/include/asm/kvm_host.h 
> > b/arch/x86/include/asm/kvm_host.h
> > index 44f6e1757861..5c4882015acc 100644
> > --- a/arch/x86/include/asm/kvm_host.h
> > +++ b/arch/x86/include/asm/kvm_host.h
> > @@ -398,8 +398,13 @@ struct kvm_mmu {
> > void (*invlpg)(struct kvm_vcpu *vcpu, gva_t gva, hpa_t root_hpa);
> > void (*update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> >u64 *spte, const void *pte);
> > +   int (*get_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> > +   int (*set_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> > +   int (*init_spp)(struct kvm *kvm);
> > +
> > hpa_t root_hpa;
> > gpa_t root_cr3;
> > +   hpa_t sppt_root;
> 
> (I'm sorry if this was previously discussed, I didn't look into previous
> submissions).
> 
> What happens when we launch a nested guest and switch vcpu->arch.mmu to
> point at arch.guest_mmu? sppt_root will point to INVALID_PAGE and SPP
> won't be enabled in VMCS?
> 
> (I'm sorry again, I'm likely missing something obvious)
> 
> -- 
> Vitaly

Hi, Vitaly,
Thanks for raising a good qeustion, I must have missed the nested case,
I'll double check how to support the nested case.


Re: [PATCH RESEND v4 5/9] KVM: VMX: Add init/set/get functions for SPP

2019-08-14 Thread Vitaly Kuznetsov
Yang Weijiang  writes:

> init_spp() must be called before {get, set}_subpage
> functions, it creates subpage access bitmaps for memory pages
> and issues a KVM request to setup SPPT root pages.
>
> kvm_mmu_set_subpages() is to enable SPP bit in EPT leaf page
> and setup corresponding SPPT entries. The mmu_lock
> is held before above operation. If it's called in EPT fault and
> SPPT mis-config induced handler, mmu_lock is acquired outside
> the function, otherwise, it's acquired inside it.
>
> kvm_mmu_get_subpages() is used to query access bitmap for
> protected page, it's also used in EPT fault handler to check
> whether the fault EPT page is SPP protected as well.
>
> Co-developed-by: He Chen 
> Signed-off-by: He Chen 
> Co-developed-by: Zhang Yi 
> Signed-off-by: Zhang Yi 
> Signed-off-by: Yang Weijiang 
> ---
>  arch/x86/include/asm/kvm_host.h |  18 
>  arch/x86/include/asm/vmx.h  |   2 +
>  arch/x86/kvm/mmu.c  | 160 
>  arch/x86/kvm/vmx/vmx.c  |  48 ++
>  arch/x86/kvm/x86.c  |  40 
>  include/linux/kvm_host.h|   4 +-
>  include/uapi/linux/kvm.h|   9 ++
>  7 files changed, 280 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 44f6e1757861..5c4882015acc 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -398,8 +398,13 @@ struct kvm_mmu {
>   void (*invlpg)(struct kvm_vcpu *vcpu, gva_t gva, hpa_t root_hpa);
>   void (*update_pte)(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
>  u64 *spte, const void *pte);
> + int (*get_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> + int (*set_subpages)(struct kvm *kvm, struct kvm_subpage *spp_info);
> + int (*init_spp)(struct kvm *kvm);
> +
>   hpa_t root_hpa;
>   gpa_t root_cr3;
> + hpa_t sppt_root;

(I'm sorry if this was previously discussed, I didn't look into previous
submissions).

What happens when we launch a nested guest and switch vcpu->arch.mmu to
point at arch.guest_mmu? sppt_root will point to INVALID_PAGE and SPP
won't be enabled in VMCS?

(I'm sorry again, I'm likely missing something obvious)

-- 
Vitaly