Re: [RFC PATCH v6 75/92] kvm: x86: disable gpa_available optimization in emulator_read_write_onepage()

2019-08-13 Thread Paolo Bonzini
On 13/08/19 16:33, Adalbert Lazăr wrote:
> On Tue, 13 Aug 2019 10:47:34 +0200, Paolo Bonzini  wrote:
>> On 09/08/19 18:00, Adalbert Lazăr wrote:
>>> If the EPT violation was caused by an execute restriction imposed by the
>>> introspection tool, gpa_available will point to the instruction pointer,
>>> not the to the read/write location that has to be used to emulate the
>>> current instruction.
>>>
>>> This optimization should be disabled only when the VM is introspected,
>>> not just because the introspection subsystem is present.
>>>
>>> Signed-off-by: Adalbert Lazăr 
>>
>> The right thing to do is to not set gpa_available for fetch failures in 
>> kvm_mmu_page_fault instead:
>>
>> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
>> index 24843cf49579..1bdca40fa831 100644
>> --- a/arch/x86/kvm/mmu.c
>> +++ b/arch/x86/kvm/mmu.c
>> @@ -5364,8 +5364,12 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t 
>> cr2, u64 error_code,
>>  enum emulation_result er;
>>  bool direct = vcpu->arch.mmu->direct_map;
>>  
>> -/* With shadow page tables, fault_address contains a GVA or nGPA.  */
>> -if (vcpu->arch.mmu->direct_map) {
>> +/*
>> + * With shadow page tables, fault_address contains a GVA or nGPA.
>> + * On a fetch fault, fault_address contains the instruction pointer.
>> + */
>> +if (vcpu->arch.mmu->direct_map &&
>> +likely(!(error_code & PFERR_FETCH_MASK)) {
>>  vcpu->arch.gpa_available = true;
>>  vcpu->arch.gpa_val = cr2;
>>  }
>
> Sure, but I think we'll have to extend the check.
> 
> Searching the logs I've found:
> 
> kvm/x86: re-translate broken translation that caused EPT violation
> 
> Signed-off-by: Mircea Cirjaliu 
> 
>  arch/x86/kvm/x86.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> /home/b/kvmi@9cad844~1/arch/x86/kvm/x86.c:4757,4762 - 
> /home/b/kvmi@9cad844/arch/x86/kvm/x86.c:4757,4763
>*/
>   if (vcpu->arch.gpa_available &&
>   emulator_can_use_gpa(ctxt) &&
> + (vcpu->arch.error_code & PFERR_GUEST_FINAL_MASK) &&
>   (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
>   gpa = vcpu->arch.gpa_val;
>   ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
> 

Yes, adding that check makes sense as well (still in kvm_mmu_page_fault).

Paolo
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH v6 75/92] kvm: x86: disable gpa_available optimization in emulator_read_write_onepage()

2019-08-13 Thread Adalbert Lazăr
On Tue, 13 Aug 2019 10:47:34 +0200, Paolo Bonzini  wrote:
> On 09/08/19 18:00, Adalbert Lazăr wrote:
> > If the EPT violation was caused by an execute restriction imposed by the
> > introspection tool, gpa_available will point to the instruction pointer,
> > not the to the read/write location that has to be used to emulate the
> > current instruction.
> > 
> > This optimization should be disabled only when the VM is introspected,
> > not just because the introspection subsystem is present.
> > 
> > Signed-off-by: Adalbert Lazăr 
> 
> The right thing to do is to not set gpa_available for fetch failures in 
> kvm_mmu_page_fault instead:
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 24843cf49579..1bdca40fa831 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -5364,8 +5364,12 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t 
> cr2, u64 error_code,
>   enum emulation_result er;
>   bool direct = vcpu->arch.mmu->direct_map;
>  
> - /* With shadow page tables, fault_address contains a GVA or nGPA.  */
> - if (vcpu->arch.mmu->direct_map) {
> + /*
> +  * With shadow page tables, fault_address contains a GVA or nGPA.
> +  * On a fetch fault, fault_address contains the instruction pointer.
> +  */
> + if (vcpu->arch.mmu->direct_map &&
> + likely(!(error_code & PFERR_FETCH_MASK)) {
>   vcpu->arch.gpa_available = true;
>   vcpu->arch.gpa_val = cr2;
>   }
> 
> 
> Paolo
> 
> > ---
> >  arch/x86/kvm/x86.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> > index 965c4f0108eb..3975331230b9 100644
> > --- a/arch/x86/kvm/x86.c
> > +++ b/arch/x86/kvm/x86.c
> > @@ -5532,7 +5532,7 @@ static int emulator_read_write_onepage(unsigned long 
> > addr, void *val,
> >  * operation using rep will only have the initial GPA from the NPF
> >  * occurred.
> >  */
> > -   if (vcpu->arch.gpa_available &&
> > +   if (vcpu->arch.gpa_available && !kvmi_is_present() &&
> > emulator_can_use_gpa(ctxt) &&
> > (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
> > gpa = vcpu->arch.gpa_val;
> > 
> 

Sure, but I think we'll have to extend the check.

Searching the logs I've found:

kvm/x86: re-translate broken translation that caused EPT violation

Signed-off-by: Mircea Cirjaliu 

 arch/x86/kvm/x86.c | 1 +
 1 file changed, 1 insertion(+)

/home/b/kvmi@9cad844~1/arch/x86/kvm/x86.c:4757,4762 - 
/home/b/kvmi@9cad844/arch/x86/kvm/x86.c:4757,4763
 */
if (vcpu->arch.gpa_available &&
emulator_can_use_gpa(ctxt) &&
+   (vcpu->arch.error_code & PFERR_GUEST_FINAL_MASK) &&
(addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
gpa = vcpu->arch.gpa_val;
ret = vcpu_is_mmio_gpa(vcpu, addr, gpa, write);
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC PATCH v6 75/92] kvm: x86: disable gpa_available optimization in emulator_read_write_onepage()

2019-08-13 Thread Paolo Bonzini
On 09/08/19 18:00, Adalbert Lazăr wrote:
> If the EPT violation was caused by an execute restriction imposed by the
> introspection tool, gpa_available will point to the instruction pointer,
> not the to the read/write location that has to be used to emulate the
> current instruction.
> 
> This optimization should be disabled only when the VM is introspected,
> not just because the introspection subsystem is present.
> 
> Signed-off-by: Adalbert Lazăr 

The right thing to do is to not set gpa_available for fetch failures in 
kvm_mmu_page_fault instead:

diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
index 24843cf49579..1bdca40fa831 100644
--- a/arch/x86/kvm/mmu.c
+++ b/arch/x86/kvm/mmu.c
@@ -5364,8 +5364,12 @@ int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t cr2, 
u64 error_code,
enum emulation_result er;
bool direct = vcpu->arch.mmu->direct_map;
 
-   /* With shadow page tables, fault_address contains a GVA or nGPA.  */
-   if (vcpu->arch.mmu->direct_map) {
+   /*
+* With shadow page tables, fault_address contains a GVA or nGPA.
+* On a fetch fault, fault_address contains the instruction pointer.
+*/
+   if (vcpu->arch.mmu->direct_map &&
+   likely(!(error_code & PFERR_FETCH_MASK)) {
vcpu->arch.gpa_available = true;
vcpu->arch.gpa_val = cr2;
}


Paolo

> ---
>  arch/x86/kvm/x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 965c4f0108eb..3975331230b9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -5532,7 +5532,7 @@ static int emulator_read_write_onepage(unsigned long 
> addr, void *val,
>* operation using rep will only have the initial GPA from the NPF
>* occurred.
>*/
> - if (vcpu->arch.gpa_available &&
> + if (vcpu->arch.gpa_available && !kvmi_is_present() &&
>   emulator_can_use_gpa(ctxt) &&
>   (addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
>   gpa = vcpu->arch.gpa_val;
> 

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[RFC PATCH v6 75/92] kvm: x86: disable gpa_available optimization in emulator_read_write_onepage()

2019-08-09 Thread Adalbert Lazăr
If the EPT violation was caused by an execute restriction imposed by the
introspection tool, gpa_available will point to the instruction pointer,
not the to the read/write location that has to be used to emulate the
current instruction.

This optimization should be disabled only when the VM is introspected,
not just because the introspection subsystem is present.

Signed-off-by: Adalbert Lazăr 
---
 arch/x86/kvm/x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 965c4f0108eb..3975331230b9 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5532,7 +5532,7 @@ static int emulator_read_write_onepage(unsigned long 
addr, void *val,
 * operation using rep will only have the initial GPA from the NPF
 * occurred.
 */
-   if (vcpu->arch.gpa_available &&
+   if (vcpu->arch.gpa_available && !kvmi_is_present() &&
emulator_can_use_gpa(ctxt) &&
(addr & ~PAGE_MASK) == (vcpu->arch.gpa_val & ~PAGE_MASK)) {
gpa = vcpu->arch.gpa_val;
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization