[PATCH] KVM: nSVM: Fix IOIO bitmap evaluation

[PATCH] KVM: nSVM: Fix IOIO bitmap evaluation

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1723 bytes --]

From: Jan Kiszka <jan.kiszka@siemens.com>

First, kvm_read_guest returns 0 on success. And then we need to take the
access size into account when testing the bitmap: intercept if any of
bits corresponding to the access is set.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

This does not yet solve all IOIO issues, there are emulation-related
bugs remaining, but this fix is self-contained.

 arch/x86/kvm/svm.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index c79766e1..3483ac9 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -2116,22 +2116,27 @@ static void nested_svm_unmap(struct page *page)
 
 static int nested_svm_intercept_ioio(struct vcpu_svm *svm)
 {
-	unsigned port;
-	u8 val, bit;
+	unsigned port, size, iopm_len;
+	u16 val, mask;
+	u8 start_bit;
 	u64 gpa;
 
 	if (!(svm->nested.intercept & (1ULL << INTERCEPT_IOIO_PROT)))
 		return NESTED_EXIT_HOST;
 
 	port = svm->vmcb->control.exit_info_1 >> 16;
+	size = (svm->vmcb->control.exit_info_1 & SVM_IOIO_SIZE_MASK) >>
+		SVM_IOIO_SIZE_SHIFT;
 	gpa  = svm->nested.vmcb_iopm + (port / 8);
-	bit  = port % 8;
-	val  = 0;
+	start_bit = port % 8;
+	iopm_len = (start_bit + size > 8) ? 2 : 1;
+	mask = (0xf >> (4 - size)) << start_bit;
+	val = 0;
 
-	if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, 1))
-		val &= (1 << bit);
+	if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, iopm_len))
+		return NESTED_EXIT_DONE;
 
-	return val ? NESTED_EXIT_DONE : NESTED_EXIT_HOST;
+	return (val & mask) ? NESTED_EXIT_DONE : NESTED_EXIT_HOST;
 }
 
 static int nested_svm_exit_handled_msr(struct vcpu_svm *svm)
-- 
1.8.4.5


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] KVM: nSVM: Fix IOIO bitmap evaluation

[Paolo Bonzini]

Il 30/06/2014 10:54, Jan Kiszka ha scritto:
> +		SVM_IOIO_SIZE_SHIFT;
>  	gpa  = svm->nested.vmcb_iopm + (port / 8);
> -	bit  = port % 8;
> -	val  = 0;
> +	start_bit = port % 8;
> +	iopm_len = (start_bit + size > 8) ? 2 : 1;
> +	mask = (0xf >> (4 - size)) << start_bit;

These two lines are tricky. :)

The patch looks good, I'll write a test case if you don't beat me to it.

Paolo

> +	val = 0;
>

Re: [PATCH] KVM: nSVM: Fix IOIO bitmap evaluation

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 708 bytes --]

On 2014-06-30 17:08, Paolo Bonzini wrote:
> Il 30/06/2014 10:54, Jan Kiszka ha scritto:
>> +        SVM_IOIO_SIZE_SHIFT;
>>      gpa  = svm->nested.vmcb_iopm + (port / 8);
>> -    bit  = port % 8;
>> -    val  = 0;
>> +    start_bit = port % 8;
>> +    iopm_len = (start_bit + size > 8) ? 2 : 1;
>> +    mask = (0xf >> (4 - size)) << start_bit;
> 
> These two lines are tricky. :)

That last line was, indeed... :)

> 
> The patch looks good, I'll write a test case if you don't beat me to it.

I'm not going to beat you if you write any test case. :)

Didn't check the svm part of the unit tests recently, if they are as
easily extensible for such scenarios like vmx is now.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]

Re: [PATCH] KVM: nSVM: Fix IOIO bitmap evaluation

[Joerg Roedel]

On Mon, Jun 30, 2014 at 10:54:17AM +0200, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> First, kvm_read_guest returns 0 on success. And then we need to take the
> access size into account when testing the bitmap: intercept if any of
> bits corresponding to the access is set.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Joerg Roedel <jroedel@suse.de>
Acked-by: Joerg Roedel <jroedel@suse.de>

I have the slight hope that this fixes the issues with L2 Linux guests
on L1 Windows hypervisors. Have to check that at some point :)

> -	if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, 1))
> -		val &= (1 << bit);
> +	if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, iopm_len))
> +		return NESTED_EXIT_DONE;

Not related to that fix, but as a further improvement we should probably
do a #vmexit(invalid-vmcb) or something if we can't read the iopm.


	Joerg

Re: [PATCH] KVM: nSVM: Fix IOIO bitmap evaluation

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1090 bytes --]

On 2014-07-01 17:23, Joerg Roedel wrote:
> On Mon, Jun 30, 2014 at 10:54:17AM +0200, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> First, kvm_read_guest returns 0 on success. And then we need to take the
>> access size into account when testing the bitmap: intercept if any of
>> bits corresponding to the access is set.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Reviewed-by: Joerg Roedel <jroedel@suse.de>
> Acked-by: Joerg Roedel <jroedel@suse.de>
> 
> I have the slight hope that this fixes the issues with L2 Linux guests
> on L1 Windows hypervisors. Have to check that at some point :)
> 
>> -	if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, 1))
>> -		val &= (1 << bit);
>> +	if (kvm_read_guest(svm->vcpu.kvm, gpa, &val, iopm_len))
>> +		return NESTED_EXIT_DONE;
> 
> Not related to that fix, but as a further improvement we should probably
> do a #vmexit(invalid-vmcb) or something if we can't read the iopm.

Yes, thought about this as well when thinking about kvm_read_guest
failing. Some for MSR bitmap.

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]