* [PATCH] nested VMX: fix I/O port exit emulation
@ 2013-12-03 13:44 Jan Beulich
2013-12-03 13:55 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2013-12-03 13:44 UTC (permalink / raw)
To: xen-devel; +Cc: Matthew Daley, Eddie Dong, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 1808 bytes --]
For multi-byte operations all affected ports' bits in the bitmap need
to be checked, not just the first port's one.
Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
u32 ctrl;
- u8 *bitmap;
nvcpu->nv_vmexit_pending = 0;
nvmx->intr.intr_info = 0;
@@ -2220,15 +2219,22 @@ int nvmx_n2_vmexit_handler(struct cpu_us
if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
{
unsigned long qual;
- u16 port;
+ u16 port, size;
__vmread(EXIT_QUALIFICATION, &qual);
- port = qual >> 16;
- bitmap = nvmx->iobitmap[port >> 15];
- if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
- nvcpu->nv_vmexit_pending = 1;
+ for ( port = qual >> 16, size = (qual & 7) + 1; ; )
+ {
+ const u8 *bitmap = nvmx->iobitmap[port >> 15];
+
+ if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
+ nvcpu->nv_vmexit_pending = 1;
+ if ( !--size )
+ break;
+ if ( !++port )
+ nvcpu->nv_vmexit_pending = 1;
+ } while ( !nvcpu->nv_vmexit_pending );
if ( !nvcpu->nv_vmexit_pending )
- gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
+ printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
}
else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
nvcpu->nv_vmexit_pending = 1;
[-- Attachment #2: nVMX-IO-port-multiple.patch --]
[-- Type: text/plain, Size: 1845 bytes --]
nested VMX: fix I/O port exit emulation
For multi-byte operations all affected ports' bits in the bitmap need
to be checked, not just the first port's one.
Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
u32 ctrl;
- u8 *bitmap;
nvcpu->nv_vmexit_pending = 0;
nvmx->intr.intr_info = 0;
@@ -2220,15 +2219,22 @@ int nvmx_n2_vmexit_handler(struct cpu_us
if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
{
unsigned long qual;
- u16 port;
+ u16 port, size;
__vmread(EXIT_QUALIFICATION, &qual);
- port = qual >> 16;
- bitmap = nvmx->iobitmap[port >> 15];
- if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
- nvcpu->nv_vmexit_pending = 1;
+ for ( port = qual >> 16, size = (qual & 7) + 1; ; )
+ {
+ const u8 *bitmap = nvmx->iobitmap[port >> 15];
+
+ if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
+ nvcpu->nv_vmexit_pending = 1;
+ if ( !--size )
+ break;
+ if ( !++port )
+ nvcpu->nv_vmexit_pending = 1;
+ } while ( !nvcpu->nv_vmexit_pending );
if ( !nvcpu->nv_vmexit_pending )
- gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
+ printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
}
else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
nvcpu->nv_vmexit_pending = 1;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] nested VMX: fix I/O port exit emulation
2013-12-03 13:44 [PATCH] nested VMX: fix I/O port exit emulation Jan Beulich
@ 2013-12-03 13:55 ` Andrew Cooper
2013-12-03 14:02 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2013-12-03 13:55 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Matthew Daley, Eddie Dong, Jun Nakajima
[-- Attachment #1.1: Type: text/plain, Size: 2193 bytes --]
On 03/12/13 13:44, Jan Beulich wrote:
> For multi-byte operations all affected ports' bits in the bitmap need
> to be checked, not just the first port's one.
>
> Reported-by: Matthew Daley <mattd@bugfuzz.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> u32 ctrl;
> - u8 *bitmap;
>
> nvcpu->nv_vmexit_pending = 0;
> nvmx->intr.intr_info = 0;
> @@ -2220,15 +2219,22 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
> {
> unsigned long qual;
> - u16 port;
> + u16 port, size;
>
> __vmread(EXIT_QUALIFICATION, &qual);
> - port = qual >> 16;
> - bitmap = nvmx->iobitmap[port >> 15];
> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
> - nvcpu->nv_vmexit_pending = 1;
> + for ( port = qual >> 16, size = (qual & 7) + 1; ; )
> + {
> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
> +
> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
> + nvcpu->nv_vmexit_pending = 1;
> + if ( !--size )
> + break;
> + if ( !++port )
> + nvcpu->nv_vmexit_pending = 1;
> + } while ( !nvcpu->nv_vmexit_pending );
You have a rather odd looking "for () { } while ()" loop, which appears
to be a while loop with no body and a constant loop condition. Is this
intended?
~Andrew
> if ( !nvcpu->nv_vmexit_pending )
> - gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
> + printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
> }
> else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
> nvcpu->nv_vmexit_pending = 1;
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
[-- Attachment #1.2: Type: text/html, Size: 3174 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] nested VMX: fix I/O port exit emulation
2013-12-03 13:55 ` Andrew Cooper
@ 2013-12-03 14:02 ` Jan Beulich
2013-12-03 14:06 ` [PATCH v2] " Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2013-12-03 14:02 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Matthew Daley, Eddie Dong, Jun Nakajima
>>> On 03.12.13 at 14:55, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 03/12/13 13:44, Jan Beulich wrote:
>> For multi-byte operations all affected ports' bits in the bitmap need
>> to be checked, not just the first port's one.
>>
>> Reported-by: Matthew Daley <mattd@bugfuzz.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
>> u32 ctrl;
>> - u8 *bitmap;
>>
>> nvcpu->nv_vmexit_pending = 0;
>> nvmx->intr.intr_info = 0;
>> @@ -2220,15 +2219,22 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>> {
>> unsigned long qual;
>> - u16 port;
>> + u16 port, size;
>>
>> __vmread(EXIT_QUALIFICATION, &qual);
>> - port = qual >> 16;
>> - bitmap = nvmx->iobitmap[port >> 15];
>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>> - nvcpu->nv_vmexit_pending = 1;
>> + for ( port = qual >> 16, size = (qual & 7) + 1; ; )
>> + {
>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>> +
>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>> + nvcpu->nv_vmexit_pending = 1;
>> + if ( !--size )
>> + break;
>> + if ( !++port )
>> + nvcpu->nv_vmexit_pending = 1;
>> + } while ( !nvcpu->nv_vmexit_pending );
>
> You have a rather odd looking "for () { } while ()" loop, which appears
> to be a while loop with no body and a constant loop condition. Is this
> intended?
Oops, no, of course not.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread* [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 14:02 ` Jan Beulich
@ 2013-12-03 14:06 ` Jan Beulich
2013-12-03 14:30 ` Andrew Cooper
` (3 more replies)
0 siblings, 4 replies; 17+ messages in thread
From: Jan Beulich @ 2013-12-03 14:06 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Matthew Daley, Eddie Dong, Jun Nakajima
[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]
For multi-byte operations all affected ports' bits in the bitmap need
to be checked, not just the first port's one.
Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Fix loop construct.
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
u32 ctrl;
- u8 *bitmap;
nvcpu->nv_vmexit_pending = 0;
nvmx->intr.intr_info = 0;
@@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
{
unsigned long qual;
- u16 port;
+ u16 port, size;
__vmread(EXIT_QUALIFICATION, &qual);
port = qual >> 16;
- bitmap = nvmx->iobitmap[port >> 15];
- if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
- nvcpu->nv_vmexit_pending = 1;
+ size = (qual & 7) + 1;
+ do {
+ const u8 *bitmap = nvmx->iobitmap[port >> 15];
+
+ if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
+ nvcpu->nv_vmexit_pending = 1;
+ if ( !--size )
+ break;
+ if ( !++port )
+ nvcpu->nv_vmexit_pending = 1;
+ } while ( !nvcpu->nv_vmexit_pending );
if ( !nvcpu->nv_vmexit_pending )
- gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
+ printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
}
else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
nvcpu->nv_vmexit_pending = 1;
[-- Attachment #2: nVMX-IO-port-multiple.patch --]
[-- Type: text/plain, Size: 1849 bytes --]
nested VMX: fix I/O port exit emulation
For multi-byte operations all affected ports' bits in the bitmap need
to be checked, not just the first port's one.
Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Fix loop construct.
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
u32 ctrl;
- u8 *bitmap;
nvcpu->nv_vmexit_pending = 0;
nvmx->intr.intr_info = 0;
@@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
{
unsigned long qual;
- u16 port;
+ u16 port, size;
__vmread(EXIT_QUALIFICATION, &qual);
port = qual >> 16;
- bitmap = nvmx->iobitmap[port >> 15];
- if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
- nvcpu->nv_vmexit_pending = 1;
+ size = (qual & 7) + 1;
+ do {
+ const u8 *bitmap = nvmx->iobitmap[port >> 15];
+
+ if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
+ nvcpu->nv_vmexit_pending = 1;
+ if ( !--size )
+ break;
+ if ( !++port )
+ nvcpu->nv_vmexit_pending = 1;
+ } while ( !nvcpu->nv_vmexit_pending );
if ( !nvcpu->nv_vmexit_pending )
- gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
+ printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
}
else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
nvcpu->nv_vmexit_pending = 1;
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 14:06 ` [PATCH v2] " Jan Beulich
@ 2013-12-03 14:30 ` Andrew Cooper
2013-12-03 15:55 ` Jan Beulich
2013-12-04 1:51 ` Zhang, Yang Z
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2013-12-03 14:30 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Matthew Daley, Eddie Dong, Jun Nakajima
On 03/12/13 14:06, Jan Beulich wrote:
> For multi-byte operations all affected ports' bits in the bitmap need
> to be checked, not just the first port's one.
>
> Reported-by: Matthew Daley <mattd@bugfuzz.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Fix loop construct.
>
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> u32 ctrl;
> - u8 *bitmap;
>
> nvcpu->nv_vmexit_pending = 0;
> nvmx->intr.intr_info = 0;
> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
> {
> unsigned long qual;
> - u16 port;
> + u16 port, size;
>
> __vmread(EXIT_QUALIFICATION, &qual);
> port = qual >> 16;
> - bitmap = nvmx->iobitmap[port >> 15];
> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
> - nvcpu->nv_vmexit_pending = 1;
> + size = (qual & 7) + 1;
This should be (qual & 3) for the correct size (Bit 3 is the
direction). Is it worth also verifying that ((qual & 3) != 2)?
~Andrew
> + do {
> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
> +
> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
> + nvcpu->nv_vmexit_pending = 1;
> + if ( !--size )
> + break;
> + if ( !++port )
> + nvcpu->nv_vmexit_pending = 1;
> + } while ( !nvcpu->nv_vmexit_pending );
> if ( !nvcpu->nv_vmexit_pending )
> - gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
> + printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
> }
> else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
> nvcpu->nv_vmexit_pending = 1;
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 14:30 ` Andrew Cooper
@ 2013-12-03 15:55 ` Jan Beulich
2013-12-03 15:58 ` Andrew Cooper
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2013-12-03 15:55 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Matthew Daley, Eddie Dong, Jun Nakajima
>>> On 03.12.13 at 15:30, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>> {
>> unsigned long qual;
>> - u16 port;
>> + u16 port, size;
>>
>> __vmread(EXIT_QUALIFICATION, &qual);
>> port = qual >> 16;
>> - bitmap = nvmx->iobitmap[port >> 15];
>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>> - nvcpu->nv_vmexit_pending = 1;
>> + size = (qual & 7) + 1;
>
> This should be (qual & 3) for the correct size (Bit 3 is the
> direction).
Right - bit 3 is the direction. Bits 0..2 are the size. Hence the
mask ought to be 7.
> Is it worth also verifying that ((qual & 3) != 2)?
I don't think so - there's no harm to our code here if an undefined
value was there.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 15:55 ` Jan Beulich
@ 2013-12-03 15:58 ` Andrew Cooper
0 siblings, 0 replies; 17+ messages in thread
From: Andrew Cooper @ 2013-12-03 15:58 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Matthew Daley, Eddie Dong, Jun Nakajima
On 03/12/13 15:55, Jan Beulich wrote:
>>>> On 03.12.13 at 15:30, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>>> {
>>> unsigned long qual;
>>> - u16 port;
>>> + u16 port, size;
>>>
>>> __vmread(EXIT_QUALIFICATION, &qual);
>>> port = qual >> 16;
>>> - bitmap = nvmx->iobitmap[port >> 15];
>>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>>> - nvcpu->nv_vmexit_pending = 1;
>>> + size = (qual & 7) + 1;
>> This should be (qual & 3) for the correct size (Bit 3 is the
>> direction).
> Right - bit 3 is the direction. Bits 0..2 are the size. Hence the
> mask ought to be 7.
D'oh - I cant count, even with the manual for reference.
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
>
>> Is it worth also verifying that ((qual & 3) != 2)?
> I don't think so - there's no harm to our code here if an undefined
> value was there.
>
> Jan
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 14:06 ` [PATCH v2] " Jan Beulich
2013-12-03 14:30 ` Andrew Cooper
@ 2013-12-04 1:51 ` Zhang, Yang Z
2013-12-04 2:08 ` Andrew Cooper
2013-12-04 7:52 ` Jan Beulich
2013-12-04 8:34 ` Dong, Eddie
2013-12-04 9:51 ` Egger, Christoph
3 siblings, 2 replies; 17+ messages in thread
From: Zhang, Yang Z @ 2013-12-04 1:51 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Andrew Cooper, Matthew Daley, Dong, Eddie, Nakajima, Jun
Jan Beulich wrote on 2013-12-03:
> For multi-byte operations all affected ports' bits in the bitmap need to be
> checked, not just the first port's one.
>
> Reported-by: Matthew Daley <mattd@bugfuzz.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Fix loop construct.
>
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> u32 ctrl;
> - u8 *bitmap;
>
> nvcpu->nv_vmexit_pending = 0;
> nvmx->intr.intr_info = 0;
> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
> {
> unsigned long qual;
> - u16 port;
> + u16 port, size;
>
> __vmread(EXIT_QUALIFICATION, &qual);
> port = qual >> 16;
> - bitmap = nvmx->iobitmap[port >> 15];
> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
> - nvcpu->nv_vmexit_pending = 1;
> + size = (qual & 7) + 1;
> + do {
> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
> +
> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
> + nvcpu->nv_vmexit_pending = 1;
> + if ( !--size )
> + break;
> + if ( !++port )
> + nvcpu->nv_vmexit_pending = 1;
If port overflow, will it cause vmexit or maybe other fault like GP or just be ignored? Also, you need to check the DF bit to know the string direction before updating the port.
Best regards,
Yang
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 1:51 ` Zhang, Yang Z
@ 2013-12-04 2:08 ` Andrew Cooper
2013-12-04 2:16 ` Zhang, Yang Z
2013-12-04 7:52 ` Jan Beulich
1 sibling, 1 reply; 17+ messages in thread
From: Andrew Cooper @ 2013-12-04 2:08 UTC (permalink / raw)
To: Zhang, Yang Z, Jan Beulich, xen-devel
Cc: Matthew Daley, Dong, Eddie, Nakajima, Jun
On 04/12/2013 01:51, Zhang, Yang Z wrote:
> Jan Beulich wrote on 2013-12-03:
>> For multi-byte operations all affected ports' bits in the bitmap need to be
>> checked, not just the first port's one.
>>
>> Reported-by: Matthew Daley <mattd@bugfuzz.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Fix loop construct.
>>
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
>> u32 ctrl;
>> - u8 *bitmap;
>>
>> nvcpu->nv_vmexit_pending = 0;
>> nvmx->intr.intr_info = 0;
>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>> {
>> unsigned long qual;
>> - u16 port;
>> + u16 port, size;
>>
>> __vmread(EXIT_QUALIFICATION, &qual);
>> port = qual >> 16;
>> - bitmap = nvmx->iobitmap[port >> 15];
>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>> - nvcpu->nv_vmexit_pending = 1;
>> + size = (qual & 7) + 1;
>> + do {
>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>> +
>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>> + nvcpu->nv_vmexit_pending = 1;
>> + if ( !--size )
>> + break;
>> + if ( !++port )
>> + nvcpu->nv_vmexit_pending = 1;
> If port overflow, will it cause vmexit or maybe other fault like GP or just be ignored? Also, you need to check the DF bit to know the string direction before updating the port.
>
> Best regards,
> Yang
>
>
What does the direction flag have to do with the port(s) used as the
target of an ins/outs instruction? I was under the impression that it
solely acted as an increment/decrement on si/di.
~Andrew
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 2:08 ` Andrew Cooper
@ 2013-12-04 2:16 ` Zhang, Yang Z
0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Yang Z @ 2013-12-04 2:16 UTC (permalink / raw)
To: Andrew Cooper, Jan Beulich, xen-devel
Cc: Matthew Daley, Dong, Eddie, Nakajima, Jun
Andrew Cooper wrote on 2013-12-04:
> On 04/12/2013 01:51, Zhang, Yang Z wrote:
>> Jan Beulich wrote on 2013-12-03:
>>> For multi-byte operations all affected ports' bits in the bitmap
>>> need to be checked, not just the first port's one.
>>>
>>> Reported-by: Matthew Daley <mattd@bugfuzz.com>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>> ---
>>> v2: Fix loop construct.
>>>
>>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>>> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>>> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>>> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
>>> u32 ctrl;
>>> - u8 *bitmap;
>>>
>>> nvcpu->nv_vmexit_pending = 0;
>>> nvmx->intr.intr_info = 0;
>>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>>> {
>>> unsigned long qual;
>>> - u16 port;
>>> + u16 port, size;
>>>
>>> __vmread(EXIT_QUALIFICATION, &qual);
>>> port = qual >> 16;
>>> - bitmap = nvmx->iobitmap[port >> 15];
>>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>>> - nvcpu->nv_vmexit_pending = 1;
>>> + size = (qual & 7) + 1;
>>> + do {
>>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>>> +
>>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>>> + nvcpu->nv_vmexit_pending = 1;
>>> + if ( !--size )
>>> + break;
>>> + if ( !++port )
>>> + nvcpu->nv_vmexit_pending = 1;
>> If port overflow, will it cause vmexit or maybe other fault like GP
>> or just be
> ignored? Also, you need to check the DF bit to know the string
> direction before updating the port.
>>
>> Best regards,
>> Yang
>>
>>
>
> What does the direction flag have to do with the port(s) used as the
> target of an ins/outs instruction? I was under the impression that it
> solely acted as an increment/decrement on si/di.
>
Uh.. It seems DF only impact the memory location, ioport is not affected, See what SDM said.
After the byte, word, or doubleword is transfer from the I/O port to the memory location, the DI/EDI/RDI register
is incremented or decremented automatically according to the setting of the DF flag in the EFLAGS register. (If the
DF flag is 0, the (E)DI register is incremented; if the DF flag is 1, the (E)DI register is decremented.) The (E)DI
register is incremented or decremented by 1 for byte operations, by 2 for word operations, or by 4 for doubleword
operations.
> ~Andrew
Best regards,
Yang
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 1:51 ` Zhang, Yang Z
2013-12-04 2:08 ` Andrew Cooper
@ 2013-12-04 7:52 ` Jan Beulich
2013-12-04 8:07 ` Zhang, Yang Z
1 sibling, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2013-12-04 7:52 UTC (permalink / raw)
To: Yang Z Zhang
Cc: Andrew Cooper, Matthew Daley, Eddie Dong, Jun Nakajima, xen-devel
>>> On 04.12.13 at 02:51, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Jan Beulich wrote on 2013-12-03:
>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>> {
>> unsigned long qual;
>> - u16 port;
>> + u16 port, size;
>>
>> __vmread(EXIT_QUALIFICATION, &qual);
>> port = qual >> 16;
>> - bitmap = nvmx->iobitmap[port >> 15];
>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>> - nvcpu->nv_vmexit_pending = 1;
>> + size = (qual & 7) + 1;
>> + do {
>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>> +
>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>> + nvcpu->nv_vmexit_pending = 1;
>> + if ( !--size )
>> + break;
>> + if ( !++port )
>> + nvcpu->nv_vmexit_pending = 1;
>
> If port overflow, will it cause vmexit or maybe other fault like GP or just
> be ignored?
The documentation is explicit here: It causes a VM exit. And hence
the emulation is doing so too.
> Also, you need to check the DF bit to know the string direction
> before updating the port.
I think Andrew already sufficiently clarified that part.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 7:52 ` Jan Beulich
@ 2013-12-04 8:07 ` Zhang, Yang Z
2013-12-04 8:51 ` Jan Beulich
0 siblings, 1 reply; 17+ messages in thread
From: Zhang, Yang Z @ 2013-12-04 8:07 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Matthew Daley, Dong, Eddie, Nakajima, Jun,
xen-devel
Jan Beulich wrote on 2013-12-04:
>>>> On 04.12.13 at 02:51, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> Jan Beulich wrote on 2013-12-03:
>>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>>> {
>>> unsigned long qual;
>>> - u16 port;
>>> + u16 port, size;
>>>
>>> __vmread(EXIT_QUALIFICATION, &qual);
>>> port = qual >> 16;
>>> - bitmap = nvmx->iobitmap[port >> 15];
>>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>>> - nvcpu->nv_vmexit_pending = 1;
>>> + size = (qual & 7) + 1;
>>> + do {
>>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>>> +
>>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>>> + nvcpu->nv_vmexit_pending = 1;
>>> + if ( !--size )
>>> + break;
>>> + if ( !++port )
>>> + nvcpu->nv_vmexit_pending = 1;
>>
>> If port overflow, will it cause vmexit or maybe other fault like GP
>> or just be ignored?
>
> The documentation is explicit here: It causes a VM exit. And hence the
> emulation is doing so too.
Ok. BTW, which chapter tells this? I didn't find the corresponding chapter in Intel SDM. :(
>
>> Also, you need to check the DF bit to know the string direction
>> before updating the port.
>
> I think Andrew already sufficiently clarified that part.
>
> Jan
Best regards,
Yang
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 8:07 ` Zhang, Yang Z
@ 2013-12-04 8:51 ` Jan Beulich
2013-12-05 1:38 ` Zhang, Yang Z
0 siblings, 1 reply; 17+ messages in thread
From: Jan Beulich @ 2013-12-04 8:51 UTC (permalink / raw)
To: Yang Z Zhang
Cc: Andrew Cooper, Matthew Daley, Eddie Dong, Jun Nakajima, xen-devel
>>> On 04.12.13 at 09:07, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
> Jan Beulich wrote on 2013-12-04:
>>>>> On 04.12.13 at 02:51, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>>> Jan Beulich wrote on 2013-12-03:
>>>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>>>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>>>> {
>>>> unsigned long qual;
>>>> - u16 port;
>>>> + u16 port, size;
>>>>
>>>> __vmread(EXIT_QUALIFICATION, &qual);
>>>> port = qual >> 16;
>>>> - bitmap = nvmx->iobitmap[port >> 15];
>>>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>>>> - nvcpu->nv_vmexit_pending = 1;
>>>> + size = (qual & 7) + 1;
>>>> + do {
>>>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>>>> +
>>>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>>>> + nvcpu->nv_vmexit_pending = 1;
>>>> + if ( !--size )
>>>> + break;
>>>> + if ( !++port )
>>>> + nvcpu->nv_vmexit_pending = 1;
>>>
>>> If port overflow, will it cause vmexit or maybe other fault like GP
>>> or just be ignored?
>>
>> The documentation is explicit here: It causes a VM exit. And hence the
>> emulation is doing so too.
>
> Ok. BTW, which chapter tells this? I didn't find the corresponding chapter
> in Intel SDM. :(
"25.1.3 Instructions That Cause VM Exits Conditionally" says
"● IN, INS/INSB/INSW/INSD, OUT, OUTS/OUTSB/OUTSW/OUTSD. The behavior of each of these instructions
is determined by the settings of the “unconditional I/O exiting” and “use I/O bitmaps” VM-execution
controls:
— If both controls are 0, the instruction executes normally.
— If the “unconditional I/O exiting” VM-execution control is 1 and the “use I/O bitmaps” VM-execution
control
is 0, the instruction causes a VM exit.
— If the “use I/O bitmaps” VM-execution control is 1, the instruction causes a VM exit if it attempts to access
an I/O port corresponding to a bit set to 1 in the appropriate I/O bitmap (see Section 24.6.4). If an I/O
operation “wraps around” the 16-bit I/O-port space (accesses ports FFFFH and 0000H), the I/O instruction
causes a VM exit (the “unconditional I/O exiting” VM-execution control is ignored if the “use I/O bitmaps”
VM-execution control is 1)."
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 8:51 ` Jan Beulich
@ 2013-12-05 1:38 ` Zhang, Yang Z
0 siblings, 0 replies; 17+ messages in thread
From: Zhang, Yang Z @ 2013-12-05 1:38 UTC (permalink / raw)
To: Jan Beulich
Cc: Andrew Cooper, Matthew Daley, Dong, Eddie, Nakajima, Jun,
xen-devel
Jan Beulich wrote on 2013-12-04:
>>>> On 04.12.13 at 09:07, "Zhang, Yang Z" <yang.z.zhang@intel.com> wrote:
>> Jan Beulich wrote on 2013-12-04:
>>>>>> On 04.12.13 at 02:51, "Zhang, Yang Z" <yang.z.zhang@intel.com>
> wrote:
>>>> Jan Beulich wrote on 2013-12-03:
>>>>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>>>>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>>>>> {
>>>>> unsigned long qual;
>>>>> - u16 port;
>>>>> + u16 port, size;
>>>>>
>>>>> __vmread(EXIT_QUALIFICATION, &qual);
>>>>> port = qual >> 16;
>>>>> - bitmap = nvmx->iobitmap[port >> 15];
>>>>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>>>>> - nvcpu->nv_vmexit_pending = 1;
>>>>> + size = (qual & 7) + 1;
>>>>> + do {
>>>>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>>>>> +
>>>>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>>>>> + nvcpu->nv_vmexit_pending = 1;
>>>>> + if ( !--size )
>>>>> + break;
>>>>> + if ( !++port )
>>>>> + nvcpu->nv_vmexit_pending = 1;
>>>>
>>>> If port overflow, will it cause vmexit or maybe other fault like
>>>> GP or just be ignored?
>>>
>>> The documentation is explicit here: It causes a VM exit. And hence
>>> the emulation is doing so too.
>>
>> Ok. BTW, which chapter tells this? I didn't find the corresponding
>> chapter in Intel SDM. :(
>
> "25.1.3 Instructions That Cause VM Exits Conditionally" says
>
> "● IN, INS/INSB/INSW/INSD, OUT, OUTS/OUTSB/OUTSW/OUTSD. The behavior of
> each of these instructions is determined by the settings of the
> “unconditional I/O exiting” and “use I/O bitmaps” VM-execution controls:
> — If both controls are 0, the instruction executes normally. — If the
> “unconditional I/O exiting” VM-execution control is 1 and the “use I/O
> bitmaps” VM-execution control is 0, the instruction causes a VM exit. —
> If the “use I/O bitmaps” VM-execution control is 1, the instruction
> causes a VM exit if it attempts to access an I/O port corresponding to a
> bit set to 1 in the appropriate I/O bitmap (see Section 24.6.4). If an
> I/O operation “wraps around” the 16-bit I/O-port space (accesses ports
> FFFFH and 0000H), the I/O instruction causes a VM exit (the
> “unconditional I/O exiting” VM-execution control is ignored if the “use
> I/O bitmaps” VM-execution control is 1)."
>
Got it. Thanks.
> Jan
Best regards,
Yang
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 14:06 ` [PATCH v2] " Jan Beulich
2013-12-03 14:30 ` Andrew Cooper
2013-12-04 1:51 ` Zhang, Yang Z
@ 2013-12-04 8:34 ` Dong, Eddie
2013-12-04 9:51 ` Egger, Christoph
3 siblings, 0 replies; 17+ messages in thread
From: Dong, Eddie @ 2013-12-04 8:34 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Andrew Cooper, Matthew Daley, Dong, Eddie, Nakajima, Jun
Acked-by Eddie Dong <eddie.dong@intel.com>
-----Original Message-----
From: Jan Beulich [mailto:JBeulich@suse.com]
Sent: Tuesday, December 03, 2013 10:07 PM
To: xen-devel
Cc: Matthew Daley; Andrew Cooper; Dong, Eddie; Nakajima, Jun
Subject: [PATCH v2] nested VMX: fix I/O port exit emulation
For multi-byte operations all affected ports' bits in the bitmap need to be checked, not just the first port's one.
Reported-by: Matthew Daley <mattd@bugfuzz.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Fix loop construct.
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
u32 ctrl;
- u8 *bitmap;
nvcpu->nv_vmexit_pending = 0;
nvmx->intr.intr_info = 0;
@@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
{
unsigned long qual;
- u16 port;
+ u16 port, size;
__vmread(EXIT_QUALIFICATION, &qual);
port = qual >> 16;
- bitmap = nvmx->iobitmap[port >> 15];
- if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
- nvcpu->nv_vmexit_pending = 1;
+ size = (qual & 7) + 1;
+ do {
+ const u8 *bitmap = nvmx->iobitmap[port >> 15];
+
+ if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
+ nvcpu->nv_vmexit_pending = 1;
+ if ( !--size )
+ break;
+ if ( !++port )
+ nvcpu->nv_vmexit_pending = 1;
+ } while ( !nvcpu->nv_vmexit_pending );
if ( !nvcpu->nv_vmexit_pending )
- gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
+ printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
}
else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
nvcpu->nv_vmexit_pending = 1;
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-03 14:06 ` [PATCH v2] " Jan Beulich
` (2 preceding siblings ...)
2013-12-04 8:34 ` Dong, Eddie
@ 2013-12-04 9:51 ` Egger, Christoph
2013-12-04 10:05 ` Jan Beulich
3 siblings, 1 reply; 17+ messages in thread
From: Egger, Christoph @ 2013-12-04 9:51 UTC (permalink / raw)
To: Jan Beulich, xen-devel
Cc: Andrew Cooper, Matthew Daley, Eddie Dong, Jun Nakajima
On 03.12.13 15:06, Jan Beulich wrote:
> For multi-byte operations all affected ports' bits in the bitmap need
> to be checked, not just the first port's one.
>
> Reported-by: Matthew Daley <mattd@bugfuzz.com>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Fix loop construct.
>
> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
> u32 ctrl;
> - u8 *bitmap;
>
> nvcpu->nv_vmexit_pending = 0;
> nvmx->intr.intr_info = 0;
> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
> {
> unsigned long qual;
> - u16 port;
> + u16 port, size;
>
> __vmread(EXIT_QUALIFICATION, &qual);
> port = qual >> 16;
> - bitmap = nvmx->iobitmap[port >> 15];
> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
> - nvcpu->nv_vmexit_pending = 1;
> + size = (qual & 7) + 1;
> + do {
> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
> +
> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
> + nvcpu->nv_vmexit_pending = 1;
> + if ( !--size )
> + break;
> + if ( !++port )
> + nvcpu->nv_vmexit_pending = 1;
> + } while ( !nvcpu->nv_vmexit_pending );
> if ( !nvcpu->nv_vmexit_pending )
> - gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
> + printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
> }
> else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
> nvcpu->nv_vmexit_pending = 1;
>
Can you use #define's for the bit operations, please?
That makes the code more readable and avoids copy & paste errors.
Christoph
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2] nested VMX: fix I/O port exit emulation
2013-12-04 9:51 ` Egger, Christoph
@ 2013-12-04 10:05 ` Jan Beulich
0 siblings, 0 replies; 17+ messages in thread
From: Jan Beulich @ 2013-12-04 10:05 UTC (permalink / raw)
To: Christoph Egger
Cc: Andrew Cooper, Matthew Daley, Eddie Dong, Jun Nakajima, xen-devel
>>> On 04.12.13 at 10:51, "Egger, Christoph" <chegger@amazon.de> wrote:
> On 03.12.13 15:06, Jan Beulich wrote:
>> For multi-byte operations all affected ports' bits in the bitmap need
>> to be checked, not just the first port's one.
>>
>> Reported-by: Matthew Daley <mattd@bugfuzz.com>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Fix loop construct.
>>
>> --- a/xen/arch/x86/hvm/vmx/vvmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
>> @@ -2134,7 +2134,6 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> struct nestedvcpu *nvcpu = &vcpu_nestedhvm(v);
>> struct nestedvmx *nvmx = &vcpu_2_nvmx(v);
>> u32 ctrl;
>> - u8 *bitmap;
>>
>> nvcpu->nv_vmexit_pending = 0;
>> nvmx->intr.intr_info = 0;
>> @@ -2220,15 +2219,23 @@ int nvmx_n2_vmexit_handler(struct cpu_us
>> if ( ctrl & CPU_BASED_ACTIVATE_IO_BITMAP )
>> {
>> unsigned long qual;
>> - u16 port;
>> + u16 port, size;
>>
>> __vmread(EXIT_QUALIFICATION, &qual);
>> port = qual >> 16;
>> - bitmap = nvmx->iobitmap[port >> 15];
>> - if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 0x7)) )
>> - nvcpu->nv_vmexit_pending = 1;
>> + size = (qual & 7) + 1;
>> + do {
>> + const u8 *bitmap = nvmx->iobitmap[port >> 15];
>> +
>> + if ( bitmap[(port & 0x7fff) >> 3] & (1 << (port & 7)) )
>> + nvcpu->nv_vmexit_pending = 1;
>> + if ( !--size )
>> + break;
>> + if ( !++port )
>> + nvcpu->nv_vmexit_pending = 1;
>> + } while ( !nvcpu->nv_vmexit_pending );
>> if ( !nvcpu->nv_vmexit_pending )
>> - gdprintk(XENLOG_WARNING, "L0 PIO %x.\n", port);
>> + printk(XENLOG_G_WARNING "L0 PIO %04x\n", port);
>> }
>> else if ( ctrl & CPU_BASED_UNCOND_IO_EXITING )
>> nvcpu->nv_vmexit_pending = 1;
>>
>
> Can you use #define's for the bit operations, please?
> That makes the code more readable and avoids copy & paste errors.
That would be a separate cleanup patch, as there's code in vmx.c
also wanting to then use such #define-s.
Jan
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2013-12-05 1:38 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-12-03 13:44 [PATCH] nested VMX: fix I/O port exit emulation Jan Beulich
2013-12-03 13:55 ` Andrew Cooper
2013-12-03 14:02 ` Jan Beulich
2013-12-03 14:06 ` [PATCH v2] " Jan Beulich
2013-12-03 14:30 ` Andrew Cooper
2013-12-03 15:55 ` Jan Beulich
2013-12-03 15:58 ` Andrew Cooper
2013-12-04 1:51 ` Zhang, Yang Z
2013-12-04 2:08 ` Andrew Cooper
2013-12-04 2:16 ` Zhang, Yang Z
2013-12-04 7:52 ` Jan Beulich
2013-12-04 8:07 ` Zhang, Yang Z
2013-12-04 8:51 ` Jan Beulich
2013-12-05 1:38 ` Zhang, Yang Z
2013-12-04 8:34 ` Dong, Eddie
2013-12-04 9:51 ` Egger, Christoph
2013-12-04 10:05 ` Jan Beulich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).