* [PATCH] Check use I/O bitmap first before unconditional I/O exit
@ 2013-12-30 20:56 Zhihui Zhang
2013-12-31 8:55 ` Jan Kiszka
2014-01-08 21:01 ` Marcelo Tosatti
0 siblings, 2 replies; 5+ messages in thread
From: Zhihui Zhang @ 2013-12-30 20:56 UTC (permalink / raw)
To: gleb, pbonzini, hpa; +Cc: kvm
According to Table C-1 of Intel SDM 3C, a VM exit happens on an I/O instruction when
"use I/O bitmaps" VM-execution control was 0 _and_ the "unconditional I/O exiting"
VM-execution control was 1. So we can't just check "unconditional I/O exiting" alone.
This patch was improved by suggestion from Jan Kiszka.
Signed-off-by: Zhihui Zhang <zzhsuny@gmail.com>
---
arch/x86/kvm/vmx.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index b2fe1c2..45617db 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -6460,11 +6460,8 @@ 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))
- return 0;
+ return nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING);
exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Check use I/O bitmap first before unconditional I/O exit
2013-12-30 20:56 [PATCH] Check use I/O bitmap first before unconditional I/O exit Zhihui Zhang
@ 2013-12-31 8:55 ` Jan Kiszka
2014-01-08 21:01 ` Marcelo Tosatti
1 sibling, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2013-12-31 8:55 UTC (permalink / raw)
To: Zhihui Zhang, gleb, pbonzini, hpa; +Cc: kvm
[-- Attachment #1: Type: text/plain, Size: 1130 bytes --]
On 2013-12-30 21:56, Zhihui Zhang wrote:
> According to Table C-1 of Intel SDM 3C, a VM exit happens on an I/O instruction when
> "use I/O bitmaps" VM-execution control was 0 _and_ the "unconditional I/O exiting"
> VM-execution control was 1. So we can't just check "unconditional I/O exiting" alone.
> This patch was improved by suggestion from Jan Kiszka.
>
> Signed-off-by: Zhihui Zhang <zzhsuny@gmail.com>
> ---
> arch/x86/kvm/vmx.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index b2fe1c2..45617db 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -6460,11 +6460,8 @@ 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))
> - return 0;
> + return nested_cpu_has(vmcs12, CPU_BASED_UNCOND_IO_EXITING);
>
> exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
>
>
Reviewed-by: Jan Kiszka <jan.kiszka@siemens.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 263 bytes --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check use I/O bitmap first before unconditional I/O exit
2013-12-30 20:56 [PATCH] Check use I/O bitmap first before unconditional I/O exit Zhihui Zhang
2013-12-31 8:55 ` Jan Kiszka
@ 2014-01-08 21:01 ` Marcelo Tosatti
1 sibling, 0 replies; 5+ messages in thread
From: Marcelo Tosatti @ 2014-01-08 21:01 UTC (permalink / raw)
To: Zhihui Zhang; +Cc: gleb, pbonzini, hpa, kvm
On Mon, Dec 30, 2013 at 03:56:29PM -0500, Zhihui Zhang wrote:
> According to Table C-1 of Intel SDM 3C, a VM exit happens on an I/O instruction when
> "use I/O bitmaps" VM-execution control was 0 _and_ the "unconditional I/O exiting"
> VM-execution control was 1. So we can't just check "unconditional I/O exiting" alone.
> This patch was improved by suggestion from Jan Kiszka.
>
> Signed-off-by: Zhihui Zhang <zzhsuny@gmail.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Check use I/O bitmap first before unconditional I/O exit
2013-12-30 15:09 Zhihui Zhang
@ 2013-12-30 19:35 ` Jan Kiszka
0 siblings, 0 replies; 5+ messages in thread
From: Jan Kiszka @ 2013-12-30 19:35 UTC (permalink / raw)
To: Zhihui Zhang, gleb, pbonzini, hpa; +Cc: kvm
[-- 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 --]
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] Check use I/O bitmap first before unconditional I/O exit
@ 2013-12-30 15:09 Zhihui Zhang
2013-12-30 19:35 ` Jan Kiszka
0 siblings, 1 reply; 5+ messages in thread
From: Zhihui Zhang @ 2013-12-30 15:09 UTC (permalink / raw)
To: gleb, pbonzini, hpa; +Cc: kvm
Signed-off-by: Zhihui Zhang <zzhsuny@gmail.com>
---
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;
return 0;
+ }
exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
--
1.8.1.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-01-08 21:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-30 20:56 [PATCH] Check use I/O bitmap first before unconditional I/O exit Zhihui Zhang
2013-12-31 8:55 ` Jan Kiszka
2014-01-08 21:01 ` Marcelo Tosatti
-- strict thread matches above, loose matches on Subject: below --
2013-12-30 15:09 Zhihui Zhang
2013-12-30 19:35 ` Jan Kiszka
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.