Re: [PATCH] Check use I/O bitmap first before unconditional I/O exit

[Jan Kiszka <jan.kiszka@web.de>]

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

On 2013-12-30 16:09, Zhihui Zhang wrote:
> Signed-off-by: Zhihui Zhang <zzhsuny@gmail.com>

You should explain the "why" here, e.g. point to the SDM section that
requires this ordering.

> ---
>  arch/x86/kvm/vmx.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index b2fe1c2..44d4f829 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6460,11 +6460,11 @@ static bool nested_vmx_exit_handled_io(struct kvm_vcpu *vcpu,
>  	int size;
>  	u8 b;
>  
> -	if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
> -		return 1;
> -
> -	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
> +	if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS)) {
> +		if (nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING))
> +			return 1;

Minor nit, but what about

if (!nested_cpu_has(vmcs12, CPU_BASED_USE_IO_BITMAPS))
	return nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING);

instead?

Logic is correct, thanks for addressing this.

Could you also write an according sub-test in kvm-unit-tests [1]? We are
already checking IO bitmap accesses, but not yet unconditional PIO
exiting. Would be great!

Jan

[1] https://git.kernel.org/cgit/virt/kvm/kvm-unit-tests.git

>  		return 0;
> +	}
>  
>  	exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
>  
> 



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