[Qemu-devel] [PATCH] SVM IOIO intercept does not check all bits

[Qemu-devel] [PATCH] SVM IOIO intercept does not check all bits

[Bernhard Kauer]

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

The SVM IOIO intercept does not check all bits in the IO permission map
for in/outs with word or long operand size. The attached patch fix this.


        Bernhard Kauer

[-- Attachment #2: qemu_ioio.diff --]
[-- Type: text/x-diff, Size: 563 bytes --]

Index: target-i386/helper.c
--- target-i386/helper.c	18 Nov 2007 01:44:38 -0000	1.95
+++ target-i386/helper.c	6 Dec 2007 19:22:55 -0000
@@ -4250,8 +4331,8 @@
             uint64_t addr = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.iopm_base_pa));
             uint16_t port = (uint16_t) (param >> 16);
 
-            if(ldub_phys(addr + port / 8) & (1 << (port % 8)))
-                vmexit(type, param);
+	    if(ldub_phys(addr + port / 8) & (((1 << ((param >> 4) & 0x7)) - 1) << (port % 8)))
+	      vmexit(type, param);
         }
         break;
 

Re: [Qemu-devel] [PATCH] SVM IOIO intercept does not check all bits

[Alexander Graf]

Hi,

On Dec 6, 2007, at 8:31 PM, Bernhard Kauer wrote:

> The SVM IOIO intercept does not check all bits in the IO permission  
> map
> for in/outs with word or long operand size. The attached patch fix  
> this.
>
>
>        Bernhard Kauer
> <qemu_ioio.diff>

Could you please make this more readable? Apart from that the patch is  
fine if the highest bit in the IOIO vector is to be set. I could not  
find that in the documentation.

Cheers,

Alex

Re: [Qemu-devel] [PATCH] SVM IOIO intercept does not check all bits

[Bernhard Kauer]

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

On Fri, Dec 07, 2007 at 02:10:35PM +0100, Alexander Graf wrote:
> Could you please make this more readable?

Not easy by a one liner. I splitted the mask calculation in a separate line.
Is it better now?

> Apart from that the patch is fine if the highest bit in the IOIO vector is
> to be set. I could not find that in the documentation.


I did not understand your comment. Perhaps you mean the following
sentence from the AMD vol. 2:

>> For IN/OUT instructions that access more than a single byte, the
>> permission bits for all bytes are checked; if any bit is set to 1,
>> the I/O operation is intercepted.


Greetings,

	Bernhard

[-- Attachment #2: qemu_ioio.diff --]
[-- Type: text/x-diff, Size: 726 bytes --]

Index: target-i386/helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-i386/helper.c,v
retrieving revision 1.95
diff -u -r1.95 helper.c
--- target-i386/helper.c	18 Nov 2007 01:44:38 -0000	1.95
+++ target-i386/helper.c	7 Dec 2007 14:10:57 -0000
@@ -4250,7 +4331,8 @@
             uint64_t addr = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.iopm_base_pa));
             uint16_t port = (uint16_t) (param >> 16);
 
-            if(ldub_phys(addr + port / 8) & (1 << (port % 8)))
+	    uint8_t mask = (1 << ((param >> 4) & 0x7)) - 1;
+	    if(ldub_phys(addr + port / 8) & (mask << (port % 8)))
                 vmexit(type, param);
         }
         break;

Re: [Qemu-devel] [PATCH] SVM IOIO intercept does not check all bits

[Alexander Graf]

On Dec 7, 2007, at 3:20 PM, Bernhard Kauer wrote:

> On Fri, Dec 07, 2007 at 02:10:35PM +0100, Alexander Graf wrote:
>> Could you please make this more readable?
>
> Not easy by a one liner. I splitted the mask calculation in a  
> separate line.
> Is it better now?

Much better, yes.

>
>
>> Apart from that the patch is fine if the highest bit in the IOIO  
>> vector is
>> to be set. I could not find that in the documentation.
>
>
> I did not understand your comment. Perhaps you mean the following
> sentence from the AMD vol. 2:

I actually misread your patch, nevermind.

>
>
>>> For IN/OUT instructions that access more than a single byte, the
>>> permission bits for all bytes are checked; if any bit is set to 1,
>>> the I/O operation is intercepted.
>>>

That was the one. Thank you.

>
>
> Greetings,
>
> 	Bernhard
> <qemu_ioio.diff>


Looks good now (apart from whitespaces).

Signed-off-by: Alexander Graf <alex@csgraf.de>

Cheers,

Alex

Re: [Qemu-devel] [PATCH] SVM IOIO intercept does not check all bits

[Bernhard Kauer]

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

On Fri, Dec 07, 2007 at 04:16:00PM +0100, Alexander Graf wrote:
>>>> For IN/OUT instructions that access more than a single byte, the
>>>> permission bits for all bytes are checked; if any bit is set to 1,
>>>> the I/O operation is intercepted.
>>>>
>
> That was the one. Thank you.


Unfortunately there is another bug in this line. As there
is only a single byte read from the permission bitmap, an
unaligned 4-byte access to port 0x7 would be possible even
when the access to port 0x8-0xa is not allowed. The updated
patch fixes also this case.


	Bernhard Kauer

[-- Attachment #2: qemu_ioio.diff --]
[-- Type: text/x-diff, Size: 739 bytes --]

Index: target-i386/helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-i386/helper.c,v
retrieving revision 1.95
diff -u -r1.95 helper.c
--- target-i386/helper.c	18 Nov 2007 01:44:38 -0000	1.95
+++ target-i386/helper.c	8 Dec 2007 20:44:28 -0000
@@ -4250,7 +4332,8 @@
             uint64_t addr = ldq_phys(env->vm_vmcb + offsetof(struct vmcb, control.iopm_base_pa));
             uint16_t port = (uint16_t) (param >> 16);
 
-            if(ldub_phys(addr + port / 8) & (1 << (port % 8)))
+            uint16_t mask = (1 << ((param >> 4) & 7)) - 1;
+            if(lduw_phys(addr + port / 8) & (mask << (port & 7)))
                 vmexit(type, param);
         }
         break;