Re: [PATCH 5/5] KVM: Fix the usage of X86EMUL_* values in x86.c

[Marcelo Tosatti <mtosatti@redhat.com>]

On Thu, Jan 28, 2010 at 11:03:34PM +0900, Takuya Yoshikawa wrote:
> pio_copy_data() and load|save_guest_segment_descriptor()
> return X86EMUL_* values. Mixing up these values with 0, 1, ...
> may produce unpridictable bugs.
> 
> Signed-off-by: Takuya Yoshikawa <yoshikawa.takuya@oss.ntt.co.jp>
> ---
>  arch/x86/kvm/x86.c |   27 +++++++++++++++------------
>  1 files changed, 15 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 78b8ddb..67f8231 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3454,7 +3454,6 @@ int complete_pio(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_pio_request *io = &vcpu->arch.pio;
>  	long delta;
> -	int r;
>  	unsigned long val;
>  
>  	if (!io->string) {
> @@ -3465,9 +3464,9 @@ int complete_pio(struct kvm_vcpu *vcpu)
>  		}
>  	} else {
>  		if (io->in) {
> -			r = pio_copy_data(vcpu);
> -			if (r)
> -				return r;
> +			int ret = pio_copy_data(vcpu);
> +			if (ret != X86EMUL_CONTINUE)
> +				return 1;
>  		}
>  
>  		delta = 1;
> @@ -3567,7 +3566,6 @@ int kvm_emulate_pio_string(struct kvm_vcpu *vcpu, int in,
>  		  gva_t address, int rep, unsigned port)
>  {
>  	unsigned now, in_page;
> -	int ret = 0;
>  
>  	vcpu->run->exit_reason = KVM_EXIT_IO;
>  	vcpu->run->io.direction = in ? KVM_EXIT_IO_IN : KVM_EXIT_IO_OUT;
> @@ -3613,20 +3611,22 @@ int kvm_emulate_pio_string(struct kvm_vcpu *vcpu, int in,
>  
>  	if (!vcpu->arch.pio.in) {
>  		/* string PIO write */
> -		ret = pio_copy_data(vcpu);
> +		int ret = pio_copy_data(vcpu);
>  		if (ret == X86EMUL_PROPAGATE_FAULT) {
>  			kvm_inject_gp(vcpu, 0);
>  			return 1;
>  		}
> -		if (ret == 0 && !pio_string_write(vcpu)) {
> +		if (ret == X86EMUL_UNHANDLEABLE)
> +			return 1;
> +		if (ret == X86EMUL_CONTINUE && !pio_string_write(vcpu)) {
>  			complete_pio(vcpu);
>  			if (vcpu->arch.pio.count == 0)
> -				ret = 1;
> +				return 1;
>  		}
>  	}
>  	/* no string PIO read support yet */
>  
> -	return ret;
> +	return 0;
>  }
>  EXPORT_SYMBOL_GPL(kvm_emulate_pio_string);

This function is used by the emulator, and as such should return
X86_EMUL values?