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

2007-12-08 Thread Bernhard Kauer
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
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 -	1.95
+++ target-i386/helper.c	8 Dec 2007 20:44:28 -
@@ -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;


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

2007-12-07 Thread 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 [EMAIL PROTECTED]

Cheers,

Alex




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

2007-12-07 Thread 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

2007-12-07 Thread Bernhard Kauer
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
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 -	1.95
+++ target-i386/helper.c	7 Dec 2007 14:10:57 -
@@ -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;