ioemu: drop unused 4MB video memory

PATCH In review Samuel Thibault 4 participants 12 messages Jul 10, 2008 – Jul 14, 2008

lore

Share & tools

lore

Samuel Thibault

author

2008-07-10 16:18 UTC #0 lore raw

PATCH 1/1 ioemu: drop unused 4MB video memory

ioemu: drop unused 4MB video memory

Since we only emulate the cirrus VGA video card which is only able to expose 4MB video memory, we don't need more than that.

Signed-off-by Samuel Thibault <[email protected]>

tools/ioemu/vl.h +1 -1

diff -r 7a918e567837 tools/ioemu/vl.h
@@ -927,7 +927,7 @@
 
 /* vga.c */
 
-#define VGA_RAM_SIZE (8192 * 1024)
+#define VGA_RAM_SIZE (4096 * 1024)
 
 /* in ms */
 #define GUI_REFRESH_INTERVAL 30

tools/python/xen/xend/image.py +4 -4

diff -r 7a918e567837 tools/python/xen/xend/image.py
@@ -833,8 +833,8 @@
         # buffer io page, buffer pio page and memmap info page
         extra_pages = 1024 + 5
         mem_kb += extra_pages * page_kb
-        # Add 8 MiB overhead for QEMU's video RAM.
-        return mem_kb + 8192
+        # Add 4 MiB overhead for QEMU's video RAM.
+        return mem_kb + 4096
 
     def getRequiredInitialReservation(self):
         return self.vm.getMemoryTarget()

@@ -876,8 +876,8 @@
         return rc
 
     def getRequiredAvailableMemory(self, mem_kb):
-        # Add 8 MiB overhead for QEMU's video RAM.
-        return mem_kb + 8192
+        # Add 4 MiB overhead for QEMU's video RAM.
+        return mem_kb + 4096
 
     def getRequiredInitialReservation(self):
         return self.vm.getMemoryTarget()

Ian Jackson

2008-07-14 09:35 UTC #1 lore raw

Samuel Thibault writes ("[Xen-devel] [PATCH] ioemu: drop unused 4MB video memory"):

Quoted 4 lines

ioemu: drop unused 4MB video memory
Since we only emulate the cirrus VGA video card which is only able to expose 4MB video memory, we don't need more than that.

I see this has gone into xen-unstable and also the new qemu-xen tree[1].

But it would be nice to understand the compatibility properties:
* Evidently it breaks when tools have this change but qemu-dm doesn't. Is there a way to avoid this being a fatal error ?
* I think I'm correct to assume that this change is harmless for a new qemu-dm and old tools ?
* What about save/restore ?

Ian.

Ian Jackson

2008-07-14 09:35 UTC #2 lore raw

iwj writes ("Re: [Xen-devel] [PATCH] ioemu: drop unused 4MB video memory"):

Quoted 1 line

I see this has gone into xen-unstable and also the new qemu-xen tree[1].

[1] which apparently hasn't been getting pushed properly which I'm investigating.

Ian.

Samuel Thibault

author

2008-07-14 10:40 UTC #3 lore raw

In reply to Ian Jackson

Hello,

Quoted 11 lines · Ian Jackson, le Mon 14 Jul 2008 10:35:15 +0100, a écrit :

Samuel Thibault writes ("[Xen-devel] [PATCH] ioemu: drop unused 4MB video memory"):
Quoted 4 lines
ioemu: drop unused 4MB video memory
Since we only emulate the cirrus VGA video card which is only able to expose 4MB video memory, we don't need more than that.
I see this has gone into xen-unstable and also the new qemu-xen tree[1].
But it would be nice to understand the compatibility properties:
* Evidently it breaks when tools have this change but qemu-dm doesn't.

Yes.

Quoted 1 line

Is there a way to avoid this being a fatal error ?

Unfortunately VGA_RAM_SIZE is used in a lot of places of qemu so that'd be a bit hard.

Quoted 2 lines

* I think I'm correct to assume that this change is harmless for a new qemu-dm and old tools ?

Yes.

Quoted 1 line

* What about save/restore ?

Ah, right, we need a compatibility version for that, I'll work on it.

Samuel

Keir Fraser

2008-07-14 10:42 UTC #4 lore raw

Quoted 3 lines · Samuel Thibault

Quoted 1 line
* What about save/restore ?
Ah, right, we need a compatibility version for that, I'll work on it.

What's the compatibility problem? Is VGA_RAM_SIZE baked into the qemu save image for some reason?

-- Keir

Samuel Thibault

author

2008-07-14 10:49 UTC #5 lore raw

Quoted 8 lines · Keir Fraser, le Mon 14 Jul 2008 11:42:35 +0100, a écrit :

Quoted 3 lines · Samuel Thibault
Quoted 1 line
* What about save/restore ?
Ah, right, we need a compatibility version for that, I'll work on it.
What's the compatibility problem? Is VGA_RAM_SIZE baked into the qemu save image for some reason?

The video memory is currently written by qemu yes. Actually, if that could be moved to libxc it would be some benefit, since that's a penalty hit for live migration.

Samuel

Samuel Thibault

author

2008-07-14 11:09 UTC #6 lore raw

Quoted 11 lines · Samuel Thibault, le Mon 14 Jul 2008 11:49:33 +0100, a écrit :

Quoted 8 lines · Keir Fraser, le Mon 14 Jul 2008 11:42:35 +0100, a écrit :
Quoted 3 lines · Samuel Thibault
Quoted 1 line
* What about save/restore ?
Ah, right, we need a compatibility version for that, I'll work on it.
What's the compatibility problem? Is VGA_RAM_SIZE baked into the qemu save image for some reason?
The video memory is currently written by qemu yes.

Actually, the problem is also that everything (including qemu's dirty bitmap) gets initialized to 4MB before the load, and it'd be clumsy to plumb a reallocation. Bummer. It'd be simpler to just go expense these 4MB.

Samuel

Trolle Selander

2008-07-14 11:40 UTC #7 lore raw

I believe we have this disabled currently, but there is code in upstream qemu to support higher resolutions that actually make use of more memory through stdvga + VBE bios exensions. It seems a bit counterproductive to put in a lot of work to trim this down to 4 megs now if we're eventually going to support the stdvga + VBE combo for higher resolutions. Or is this 4/8 meg allocation stricly for cirrus, and stdvga mem allocation entirely separate?

/ Trolle

Quoted 20 lines · Samuel Thibault

Quoted 2 lines · Samuel Thibault, le Mon 14 Jul 2008 11:49:33 +0100, a écrit :
Quoted 1 line · Keir Fraser, le Mon 14 Jul 2008 11:42:35 +0100, a écrit :
On 14/7/08 11:40, "Samuel Thibault" <[email protected]>
Quoted 6 lines · wrote:
Quoted 6 lines
Quoted 4 lines
Quoted 2 lines
* What about save/restore ?
Ah, right, we need a compatibility version for that, I'll work on it.
What's the compatibility problem? Is VGA_RAM_SIZE baked into the qemu
save
Quoted 3 lines
Quoted 1 line
image for some reason?
The video memory is currently written by qemu yes.
Actually, the problem is also that everything (including qemu's dirty bitmap) gets initialized to 4MB before the load, and it'd be clumsy to plumb a reallocation. Bummer. It'd be simpler to just go expense these 4MB.

Quoted 7 lines

Samuel
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Samuel Thibault

author

2008-07-14 11:46 UTC #8 lore raw

Quoted 5 lines · Trolle Selander, le Mon 14 Jul 2008 12:40:06 +0100, a écrit :

I believe we have this disabled currently, but there is code in upstream qemu to support higher resolutions that actually make use of more memory through stdvga + VBE bios exensions. It seems a bit counterproductive to put in a lot of work to trim this down to 4 megs now if we're eventually going to support the stdvga + VBE combo for higher resolutions.

Agreed.

Quoted 2 lines

Or is this 4/8 meg allocation stricly for cirrus, and stdvga mem allocation entirely separate?

These allocations are independent, but are working the same way.

Samuel

Ian Jackson

2008-07-14 12:53 UTC #9 lore raw

In reply to Samuel Thibault

Samuel Thibault writes ("Re: [Xen-devel] [PATCH] ioemu: drop unused 4MB video memory"):

Quoted 5 lines

Quoted 1 line · Ian Jackson, le Mon 14 Jul 2008 10:35:15 +0100, a écrit :
Is there a way to avoid this being a fatal error ?
Unfortunately VGA_RAM_SIZE is used in a lot of places of qemu so that'd be a bit hard.

Err, you're missing my point, which is that we want it to work with old qemu and new hypervisor. Obviously changing qemu now will not allow us to do that ...

Quoted 3 lines

Quoted 1 line
* What about save/restore ?
Ah, right, we need a compatibility version for that, I'll work on it.

For the moment Keir and I have reverted this pair of changes.

Ian.

Samuel Thibault

author

2008-07-14 12:58 UTC #10 lore raw

Quoted 10 lines · Ian Jackson, le Mon 14 Jul 2008 13:53:39 +0100, a écrit :

Samuel Thibault writes ("Re: [Xen-devel] [PATCH] ioemu: drop unused 4MB video memory"):
Quoted 5 lines
Quoted 1 line · Ian Jackson, le Mon 14 Jul 2008 10:35:15 +0100, a écrit :
Is there a way to avoid this being a fatal error ?
Unfortunately VGA_RAM_SIZE is used in a lot of places of qemu so that'd be a bit hard.
Err, you're missing my point, which is that we want it to work with old qemu and new hypervisor. Obviously changing qemu now will not allow us to do that ...

Oops, right. No, there is no way. Note however that the issue are with the tools, not the hypervisor.

Samuel

Ian Jackson

2008-07-14 13:47 UTC #11 lore raw

Samuel Thibault writes ("Re: [Xen-devel] [PATCH] ioemu: drop unused 4MB video memory"):

Quoted 7 lines

Quoted 3 lines · Ian Jackson, le Mon 14 Jul 2008 13:53:39 +0100, a écrit :
Err, you're missing my point, which is that we want it to work with old qemu and new hypervisor. Obviously changing qemu now will not allow us to do that ...
Oops, right. No, there is no way. Note however that the issue are with the tools, not the hypervisor.

Yes, sorry, that's what I meant.

Perhaps there should be some way for the tools to find out what fixes or features qemu-dm has.

Ian.