[PATCH 1/2] kvm tools, vesa: Cleanup code in bios/int10.c

[PATCH 1/2] kvm tools, vesa: Cleanup code in bios/int10.c

[Pekka Enberg]

This patch cleans up the code in bios/int10.c without changing functionality.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: John Floren <john@jfloren.net>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 tools/kvm/bios/int10.c |   88 +++++++++++++++++++++++++----------------------
 1 files changed, 47 insertions(+), 41 deletions(-)

diff --git a/tools/kvm/bios/int10.c b/tools/kvm/bios/int10.c
index 57647a1..f7fecac 100644
--- a/tools/kvm/bios/int10.c
+++ b/tools/kvm/bios/int10.c
@@ -29,10 +29,9 @@ struct vesa_general_info {
 	u16	modes[2];		/* 20 */
 	char	oem_string[11];		/* 24 */
 
-	u8 reserved[223];		/* 35 */
+	u8	reserved[223];		/* 35 */
 } __attribute__ ((packed));
 
-
 struct vminfo {
 	u16	mode_attr;		/* 0 */
 	u8	win_attr[2];		/* 2 */
@@ -87,56 +86,63 @@ static inline void int10_putchar(struct int10_args *args)
 	outb(0x3f8, al);
 }
 
+static void vbe_get_mode(struct int10_args *args)
+{
+	struct vminfo *info = (struct vminfo *) args->edi;
+
+	*info = (struct vminfo) {
+		.mode_attr		= 0xd9, /* 11011011 */
+		.logical_scan		= VESA_WIDTH*4,
+		.h_res			= VESA_WIDTH,
+		.v_res			= VESA_HEIGHT,
+		.bpp			= VESA_BPP,
+		.memory_layout		= 6,
+		.memory_planes		= 1,
+		.lfb_ptr		= VESA_MEM_ADDR,
+		.rmask			= 8,
+		.gmask			= 8,
+		.bmask			= 8,
+		.resv_mask		= 8,
+		.resv_pos		= 24,
+		.bpos			= 16,
+		.gpos			= 8,
+	};
+}
+
+static void vbe_get_info(struct int10_args *args)
+{
+	struct vesa_general_info *info = (struct vesa_general_info *) args->edi;
+
+	*info = (struct vesa_general_info) {
+		.signature		= VESA_MAGIC,
+		.version		= 0x102,
+		.vendor_string		= &info->oem_string,
+		.capabilities		= 0x10,
+		.video_mode_ptr		= &info->modes,
+		.total_memory		= (4 * VESA_WIDTH * VESA_HEIGHT) / 0x10000,
+		.oem_string		= "KVM VESA",
+		.modes			= { 0x0112, 0xffff },
+	};
+}
+
+#define VBE_STATUS_OK		0x004F
+
 static void int10_vesa(struct int10_args *args)
 {
 	u8 al;
-	struct vesa_general_info *destination;
-	struct vminfo *vi;
 
 	al = args->eax;
 
 	switch (al) {
-	case 0:
-		/* Set controller info */
-
-		destination = (struct vesa_general_info *)args->edi;
-		*destination = (struct vesa_general_info) {
-			.signature	= VESA_MAGIC,
-			.version	= 0x102,
-			.vendor_string	= &destination->oem_string,
-			.capabilities	= 0x10,
-			.video_mode_ptr	= &destination->modes,
-			.total_memory	= (4*VESA_WIDTH * VESA_HEIGHT) / 0x10000,
-			.oem_string	= "KVM VESA",
-			.modes		= { 0x0112, 0xffff },
-		};
-
+	case 0x00:
+		vbe_get_info(args);
 		break;
-	case 1:
-		vi = (struct vminfo *)args->edi;
-		*vi = (struct vminfo) {
-			.mode_attr	= 0xd9, /* 11011011 */
-			.logical_scan	= VESA_WIDTH*4,
-			.h_res		= VESA_WIDTH,
-			.v_res		= VESA_HEIGHT,
-			.bpp		= VESA_BPP,
-			.memory_layout	= 6,
-			.memory_planes	= 1,
-			.lfb_ptr	= VESA_MEM_ADDR,
-			.rmask		= 8,
-			.gmask		= 8,
-			.bmask		= 8,
-			.resv_mask	= 8,
-			.resv_pos	= 24,
-			.bpos		= 16,
-			.gpos		= 8,
-		};
-
+	case 0x01:
+		vbe_get_mode(args);
 		break;
 	}
 
-	args->eax			= 0x004f; /* return success every time */
-
+	args->eax = VBE_STATUS_OK;
 }
 
 bioscall void int10_handler(struct int10_args *args)
-- 
1.7.0.4

[PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()

[Pekka Enberg]

This patch fixes access to 'ah' in int10_vesa() by masking the high bits.

Cc: Ingo Molnar <mingo@elte.hu>
Cc: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: John Floren <john@jfloren.net>
Cc: Sasha Levin <levinsasha928@gmail.com>
Signed-off-by: Pekka Enberg <penberg@kernel.org>
---
 tools/kvm/bios/int10.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/tools/kvm/bios/int10.c b/tools/kvm/bios/int10.c
index f7fecac..498a93a 100644
--- a/tools/kvm/bios/int10.c
+++ b/tools/kvm/bios/int10.c
@@ -131,7 +131,7 @@ static void int10_vesa(struct int10_args *args)
 {
 	u8 al;
 
-	al = args->eax;
+	al = args->eax & 0xff;
 
 	switch (al) {
 	case 0x00:
-- 
1.7.0.4

Re: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()

[Cyrill Gorcunov]

On Fri, Jun 03, 2011 at 10:37:04PM +0300, Pekka Enberg wrote:
> This patch fixes access to 'ah' in int10_vesa() by masking the high bits.
> 
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: John Floren <john@jfloren.net>
> Cc: Sasha Levin <levinsasha928@gmail.com>
> Signed-off-by: Pekka Enberg <penberg@kernel.org>
> ---
... 

Ack, thanks Pekka!

   Cyrill

Re: [PATCH 1/2] kvm tools, vesa: Cleanup code in bios/int10.c

[Cyrill Gorcunov]

On Fri, Jun 03, 2011 at 10:37:03PM +0300, Pekka Enberg wrote:
> This patch cleans up the code in bios/int10.c without changing functionality.
> 
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Cyrill Gorcunov <gorcunov@gmail.com>
> Cc: John Floren <john@jfloren.net>
> Cc: Sasha Levin <levinsasha928@gmail.com>
> Signed-off-by: Pekka Enberg <penberg@kernel.org>
> ---
>  tools/kvm/bios/int10.c |   88 +++++++++++++++++++++++++----------------------
>  1 files changed, 47 insertions(+), 41 deletions(-)
> 

Looks good to me, thanks Pekka. Verifying vesa data is still in my
todo list, sorry for taking it too long :/

  Cyrill

Re: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()

[Avi Kivity]

On 06/03/2011 10:37 PM, Pekka Enberg wrote:
> This patch fixes access to 'ah' in int10_vesa() by masking the high bits.
>
> @@ -131,7 +131,7 @@ static void int10_vesa(struct int10_args *args)
>   {
>   	u8 al;
>
> -	al = args->eax;
> +	al = args->eax&  0xff;

Isn't this reading %al?  And the high bits are masked by the type of the 
variable?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()

[Cyrill Gorcunov]

On Mon, Jun 06, 2011 at 11:28:56AM +0300, Avi Kivity wrote:
> On 06/03/2011 10:37 PM, Pekka Enberg wrote:
> >This patch fixes access to 'ah' in int10_vesa() by masking the high bits.
> >
> >@@ -131,7 +131,7 @@ static void int10_vesa(struct int10_args *args)
> >  {
> >  	u8 al;
> >
> >-	al = args->eax;
> >+	al = args->eax&  0xff;
> 
> Isn't this reading %al?  And the high bits are masked by the type of
> the variable?
> 

Type conversion will do the work but having explicit masking is
a way better I believe, at least it makes this code snippet notable.

	Cyrill

Re: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()

[Pekka Enberg]

On Mon, Jun 6, 2011 at 11:37 AM, Cyrill Gorcunov <gorcunov@gmail.com> wrote:
> On Mon, Jun 06, 2011 at 11:28:56AM +0300, Avi Kivity wrote:
>> On 06/03/2011 10:37 PM, Pekka Enberg wrote:
>> >This patch fixes access to 'ah' in int10_vesa() by masking the high bits.
>> >
>> >@@ -131,7 +131,7 @@ static void int10_vesa(struct int10_args *args)
>> >  {
>> >     u8 al;
>> >
>> >-    al = args->eax;
>> >+    al = args->eax&  0xff;
>>
>> Isn't this reading %al?  And the high bits are masked by the type of
>> the variable?
>>
>
> Type conversion will do the work but having explicit masking is
> a way better I believe, at least it makes this code snippet notable.

True but the patch description is bogus as it really doesn't _fix_ anything.

                        Pekka

Re: [PATCH 2/2] kvm tools, vesa: Fix 'ah' access in int10_vesa()

[Cyrill Gorcunov]

On Mon, Jun 06, 2011 at 12:09:25PM +0300, Pekka Enberg wrote:
...
> > Type conversion will do the work but having explicit masking is
> > a way better I believe, at least it makes this code snippet notable.
> 
> True but the patch description is bogus as it really doesn't _fix_ anything.
> 
>                         Pekka

Yup, I somehow missed 'fix' word in log, looked only in patch body, sorry.

	Cyrill