[RFC,v6,76/92] kvm: x86: disable EPT A/D bits if introspection is present
diff mbox series

Message ID 20190809160047.8319-77-alazar@bitdefender.com
State New
Headers show
Series
  • VM introspection
Related show

Commit Message

Adalbert Lazăr Aug. 9, 2019, 4 p.m. UTC
Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>
---
 arch/x86/kvm/vmx/vmx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paolo Bonzini Aug. 13, 2019, 9:18 a.m. UTC | #1
On 09/08/19 18:00, Adalbert Lazăr wrote:
> Signed-off-by: Adalbert Lazăr <alazar@bitdefender.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index dc648ba47df3..152c58b63f69 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7718,7 +7718,7 @@ static __init int hardware_setup(void)
>  	    !cpu_has_vmx_invept_global())
>  		enable_ept = 0;
>  
> -	if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
> +	if (!cpu_has_vmx_ept_ad_bits() || !enable_ept || kvmi_is_present())
>  		enable_ept_ad_bits = 0;
>  
>  	if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)
> 

Why?

Paolo
Paolo Bonzini Aug. 13, 2019, 9:05 p.m. UTC | #2
On 13/08/19 20:36, Mihai Donțu wrote:
>> Why?
> When EPT A/D is enabled, all guest page table walks are treated as
> writes (like AMD's NPT). Thus, an introspection tool hooking the guest
> page tables would trigger a flood of VMEXITs (EPT write violations)
> that will get the introspected VM into an unusable state.
> 
> Our implementation of such an introspection tool builds a cache of
> {cr3, gva} -> gpa, which is why it needs to monitor all guest PTs by
> hooking them for write.

Please include the kvm list too.

One issue here is that it changes the nested VMX ABI.  Can you leave EPT
A/D in place for the shadow EPT MMU, but not for "regular" EPT pages?

Also, what is the state of introspection support on AMD?

Paolo
Mihai Donțu Aug. 14, 2019, 8:53 a.m. UTC | #3
On Tue, 2019-08-13 at 23:05 +0200, Paolo Bonzini wrote:
> On 13/08/19 20:36, Mihai Donțu wrote:
> > > Why?
> > When EPT A/D is enabled, all guest page table walks are treated as
> > writes (like AMD's NPT). Thus, an introspection tool hooking the guest
> > page tables would trigger a flood of VMEXITs (EPT write violations)
> > that will get the introspected VM into an unusable state.
> > 
> > Our implementation of such an introspection tool builds a cache of
> > {cr3, gva} -> gpa, which is why it needs to monitor all guest PTs by
> > hooking them for write.
> 
> Please include the kvm list too.

I apologize. I did not notice I trimmed the CC list.

> One issue here is that it changes the nested VMX ABI.  Can you leave EPT
> A/D in place for the shadow EPT MMU, but not for "regular" EPT pages?

I'm not sure I follow. Something like this?

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 48e3cdd7b009..569e8f4d5dd7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2853,7 +2853,7 @@ u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa)
        eptp |= (get_ept_level(vcpu) == 5) ? VMX_EPTP_PWL_5 : VMX_EPTP_PWL_4;
 
        if (enable_ept_ad_bits &&
-           (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
+           ((!is_guest_mode(vcpu) && !kvmi_is_present()) || nested_ept_ad_enabled(vcpu)))
                eptp |= VMX_EPTP_AD_ENABLE_BIT;
        eptp |= (root_hpa & PAGE_MASK);

> Also, what is the state of introspection support on AMD?

The way we'd like to do introspection is not currently possible on AMD
CPUs. The reasons being:
 * the NPT has the behavior I talked above (guest PT walks translate to
   writes)
 * it is not possible to mark a guest page as execute-only
 * there is no equivalent to Intel's MTF, though it can _probably_ be
   emulated using a creative trick involving the debug registers

If, however, none of the above are of importance for other users,
everything else should work. I have to admit, though, we have not done
any tests.

Regards,
Paolo Bonzini Aug. 14, 2019, 10:36 a.m. UTC | #4
On 14/08/19 10:53, Mihai Donțu wrote:
> On Tue, 2019-08-13 at 23:05 +0200, Paolo Bonzini wrote:
>> On 13/08/19 20:36, Mihai Donțu wrote:
>>>> Why?
>>> When EPT A/D is enabled, all guest page table walks are treated as
>>> writes (like AMD's NPT). Thus, an introspection tool hooking the guest
>>> page tables would trigger a flood of VMEXITs (EPT write violations)
>>> that will get the introspected VM into an unusable state.
>>>
>>> Our implementation of such an introspection tool builds a cache of
>>> {cr3, gva} -> gpa, which is why it needs to monitor all guest PTs by
>>> hooking them for write.
>>
>> Please include the kvm list too.
> 
> I apologize. I did not notice I trimmed the CC list.
> 
>> One issue here is that it changes the nested VMX ABI.  Can you leave EPT
>> A/D in place for the shadow EPT MMU, but not for "regular" EPT pages?
> 
> I'm not sure I follow. Something like this?
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 48e3cdd7b009..569e8f4d5dd7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2853,7 +2853,7 @@ u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa)
>         eptp |= (get_ept_level(vcpu) == 5) ? VMX_EPTP_PWL_5 : VMX_EPTP_PWL_4;
>  
>         if (enable_ept_ad_bits &&
> -           (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu)))
> +           ((!is_guest_mode(vcpu) && !kvmi_is_present()) || nested_ept_ad_enabled(vcpu)))
>                 eptp |= VMX_EPTP_AD_ENABLE_BIT;
>         eptp |= (root_hpa & PAGE_MASK);

Almost, because you also have to change the accessed/dirty bits for
mmu.c.  Probably by moving the call to kvm_mmu_set_mask_ptes from
vmx_enable_tdp to vmx_set_cr3.

Paolo

>> Also, what is the state of introspection support on AMD?
> 
> The way we'd like to do introspection is not currently possible on AMD
> CPUs. The reasons being:
>  * the NPT has the behavior I talked above (guest PT walks translate to
>    writes)
>  * it is not possible to mark a guest page as execute-only
>  * there is no equivalent to Intel's MTF, though it can _probably_ be
>    emulated using a creative trick involving the debug registers
> 
> If, however, none of the above are of importance for other users,
> everything else should work. I have to admit, though, we have not done
> any tests.
> 
> Regards,
>

Patch
diff mbox series

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index dc648ba47df3..152c58b63f69 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7718,7 +7718,7 @@  static __init int hardware_setup(void)
 	    !cpu_has_vmx_invept_global())
 		enable_ept = 0;
 
-	if (!cpu_has_vmx_ept_ad_bits() || !enable_ept)
+	if (!cpu_has_vmx_ept_ad_bits() || !enable_ept || kvmi_is_present())
 		enable_ept_ad_bits = 0;
 
 	if (!cpu_has_vmx_unrestricted_guest() || !enable_ept)