Fix kernel pio emulation mistake

Fix kernel pio emulation mistake

[Dong, Eddie]

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

Kernel pio emulation return value is mistakenly checked, fortuantely it is not hit yet for normal OS bootup :(


Signed-off-by: Eddie Dong <Eddie.dong@linux.intel.com>



commit 98d3dc8b67ba0bc7f494de3ade8f2b5cfcadaeb4
Author: root <root@eddie-wb.localdomain>
Date:   Thu Mar 19 15:44:39 2009 +0800

    fix a bug when kernel PIO is emulated.

diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index ca91749..0edd2e7 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -1838,7 +1838,7 @@ special_insn:
 		io_dir_in = 0;
 	do_io:	if (kvm_emulate_pio(ctxt->vcpu, NULL, io_dir_in,
 				   (c->d & ByteOp) ? 1 : c->op_bytes,
-				   port) != 0) {
+				   port) == 0) {
 			c->eip = saved_eip;
 			goto cannot_emulate;
 		}

[-- Attachment #2: pio.patch --]
[-- Type: application/octet-stream, Size: 591 bytes --]

commit 98d3dc8b67ba0bc7f494de3ade8f2b5cfcadaeb4
Author: root <root@eddie-wb.localdomain>
Date:   Thu Mar 19 15:44:39 2009 +0800

    fix a bug when kernel PIO is emulated.

diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index ca91749..0edd2e7 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -1838,7 +1838,7 @@ special_insn:
 		io_dir_in = 0;
 	do_io:	if (kvm_emulate_pio(ctxt->vcpu, NULL, io_dir_in,
 				   (c->d & ByteOp) ? 1 : c->op_bytes,
-				   port) != 0) {
+				   port) == 0) {
 			c->eip = saved_eip;
 			goto cannot_emulate;
 		}

Re: Fix kernel pio emulation mistake

[Avi Kivity]

Dong, Eddie wrote:
> Kernel pio emulation return value is mistakenly checked, fortuantely it is not hit yet for normal OS bootup :(
>
>
> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
> index ca91749..0edd2e7 100644
> --- a/arch/x86/kvm/x86_emulate.c
> +++ b/arch/x86/kvm/x86_emulate.c
> @@ -1838,7 +1838,7 @@ special_insn:
>  		io_dir_in = 0;
>  	do_io:	if (kvm_emulate_pio(ctxt->vcpu, NULL, io_dir_in,
>  				   (c->d & ByteOp) ? 1 : c->op_bytes,
> -				   port) != 0) {
> +				   port) == 0) {
>  			c->eip = saved_eip;
>  			goto cannot_emulate;
>  		}

kvm_emulate_pio() returns 1 when emulation is complete, and 0 when 
emulation needs further processing in userspace.  So I think in both 
cases cannot_emulate is the wrong answer.  I think 'in' emulation gets 
it right.

-- 
error compiling committee.c: too many arguments to function

RE: Fix kernel pio emulation mistake

[Dong, Eddie]

> kvm_emulate_pio() returns 1 when emulation is complete,
> and 0 when emulation needs further processing in
> userspace.  So I think in both cases cannot_emulate is
> the wrong answer.  I think 'in' emulation gets it right.

OK, yes. Do u mean this?  I may misunderstand.

Thx, eddie

diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
index ca91749..838fdc9 100644
--- a/arch/x86/kvm/x86_emulate.c
+++ b/arch/x86/kvm/x86_emulate.c
@@ -1838,11 +1838,11 @@ special_insn:
 		io_dir_in = 0;
 	do_io:	if (kvm_emulate_pio(ctxt->vcpu, NULL, io_dir_in,
 				   (c->d & ByteOp) ? 1 : c->op_bytes,
-				   port) != 0) {
+				   port) == 0) {
 			c->eip = saved_eip;
-			goto cannot_emulate;
+			return -1;
 		}
-		break;
+		return 0;
 	case 0xf4:              /* hlt */
 		ctxt->vcpu->arch.halt_request = 1;
 		break; 

Re: Fix kernel pio emulation mistake

[Avi Kivity]

Dong, Eddie wrote:
>> kvm_emulate_pio() returns 1 when emulation is complete,
>> and 0 when emulation needs further processing in
>> userspace.  So I think in both cases cannot_emulate is
>> the wrong answer.  I think 'in' emulation gets it right.
>>     
>
> OK, yes. Do u mean this?  I may misunderstand.
>
> Thx, eddie
>
> diff --git a/arch/x86/kvm/x86_emulate.c b/arch/x86/kvm/x86_emulate.c
> index ca91749..838fdc9 100644
> --- a/arch/x86/kvm/x86_emulate.c
> +++ b/arch/x86/kvm/x86_emulate.c
> @@ -1838,11 +1838,11 @@ special_insn:
>  		io_dir_in = 0;
>  	do_io:	if (kvm_emulate_pio(ctxt->vcpu, NULL, io_dir_in,
>  				   (c->d & ByteOp) ? 1 : c->op_bytes,
> -				   port) != 0) {
> +				   port) == 0) {
>  			c->eip = saved_eip;
> -			goto cannot_emulate;
> +			return -1;
>  		}
> -		break;
> +		return 0;
>  	case 0xf4:              /* hlt */
>  		ctxt->vcpu->arch.halt_request = 1;
>  		break; 

I think this is right, but can you add a testcase to 
user/test/x86/realmode.c?

(you need to set the emulate_invalid_guest_state module parameter to 1 
to run this test)

-- 
error compiling committee.c: too many arguments to function