[Qemu-devel] [PATCH] update bochs vbe interface

[Qemu-devel] [PATCH] update bochs vbe interface

[Gerd Hoffmann]

The bochs vbe interface got a new register a while back, which specifies
the linear framebuffer size in 64k units.  This patch adds support for
the new register to qemu.  With this patch applied vgabios 0.6c works
with qemu.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/vga.c     |    3 ++-
 hw/vga_int.h |    4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/vga.c b/hw/vga.c
index 6a1a059..a5c6997 100644
--- a/hw/vga.c
+++ b/hw/vga.c
@@ -1955,7 +1955,8 @@ void vga_common_reset(VGACommonState *s)
 #ifdef CONFIG_BOCHS_VBE
     s->vbe_index = 0;
     memset(s->vbe_regs, '\0', sizeof(s->vbe_regs));
-    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID0;
+    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
+    s->vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] = s->vram_size / (64 * 1024);
     s->vbe_start_addr = 0;
     s->vbe_line_offset = 0;
     s->vbe_bank_mask = (s->vram_size >> 16) - 1;
diff --git a/hw/vga_int.h b/hw/vga_int.h
index 23a42ef..4b8fc74 100644
--- a/hw/vga_int.h
+++ b/hw/vga_int.h
@@ -47,13 +47,15 @@
 #define VBE_DISPI_INDEX_VIRT_HEIGHT     0x7
 #define VBE_DISPI_INDEX_X_OFFSET        0x8
 #define VBE_DISPI_INDEX_Y_OFFSET        0x9
-#define VBE_DISPI_INDEX_NB              0xa
+#define VBE_DISPI_INDEX_VIDEO_MEMORY_64K 0xa
+#define VBE_DISPI_INDEX_NB              0xb
 
 #define VBE_DISPI_ID0                   0xB0C0
 #define VBE_DISPI_ID1                   0xB0C1
 #define VBE_DISPI_ID2                   0xB0C2
 #define VBE_DISPI_ID3                   0xB0C3
 #define VBE_DISPI_ID4                   0xB0C4
+#define VBE_DISPI_ID5                   0xB0C5
 
 #define VBE_DISPI_DISABLED              0x00
 #define VBE_DISPI_ENABLED               0x01
-- 
1.6.6.1

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> The bochs vbe interface got a new register a while back, which specifies
> the linear framebuffer size in 64k units.  This patch adds support for
> the new register to qemu.  With this patch applied vgabios 0.6c works
> with qemu.
>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>

It breaks migration.

vga.c:525:    if (s->vbe_index <= VBE_DISPI_INDEX_NB) {
vga.c:564:    if (s->vbe_index <= VBE_DISPI_INDEX_NB) {
vga.c:2218:        VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB),
vga_int.h:50:#define VBE_DISPI_INDEX_NB              0xa
vga_int.h:71:    uint16_t vbe_regs[VBE_DISPI_INDEX_NB];      \


2218 is the interesting line.  Can we freeze this patch until the
subsections stuff is done?

I hope to sent a proposal Friday?  And can use this as guinea pig?

Later, Juan.

> ---
>  hw/vga.c     |    3 ++-
>  hw/vga_int.h |    4 +++-
>  2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vga.c b/hw/vga.c
> index 6a1a059..a5c6997 100644
> --- a/hw/vga.c
> +++ b/hw/vga.c
> @@ -1955,7 +1955,8 @@ void vga_common_reset(VGACommonState *s)
>  #ifdef CONFIG_BOCHS_VBE
>      s->vbe_index = 0;
>      memset(s->vbe_regs, '\0', sizeof(s->vbe_regs));
> -    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID0;
> +    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
> +    s->vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] = s->vram_size / (64 * 1024);
>      s->vbe_start_addr = 0;
>      s->vbe_line_offset = 0;
>      s->vbe_bank_mask = (s->vram_size >> 16) - 1;
> diff --git a/hw/vga_int.h b/hw/vga_int.h
> index 23a42ef..4b8fc74 100644
> --- a/hw/vga_int.h
> +++ b/hw/vga_int.h
> @@ -47,13 +47,15 @@
>  #define VBE_DISPI_INDEX_VIRT_HEIGHT     0x7
>  #define VBE_DISPI_INDEX_X_OFFSET        0x8
>  #define VBE_DISPI_INDEX_Y_OFFSET        0x9
> -#define VBE_DISPI_INDEX_NB              0xa
> +#define VBE_DISPI_INDEX_VIDEO_MEMORY_64K 0xa
> +#define VBE_DISPI_INDEX_NB              0xb
>  
>  #define VBE_DISPI_ID0                   0xB0C0
>  #define VBE_DISPI_ID1                   0xB0C1
>  #define VBE_DISPI_ID2                   0xB0C2
>  #define VBE_DISPI_ID3                   0xB0C3
>  #define VBE_DISPI_ID4                   0xB0C4
> +#define VBE_DISPI_ID5                   0xB0C5
>  
>  #define VBE_DISPI_DISABLED              0x00
>  #define VBE_DISPI_ENABLED               0x01

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Gerd Hoffmann]

On 03/24/10 18:04, Juan Quintela wrote:
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>> The bochs vbe interface got a new register a while back, which specifies
>> the linear framebuffer size in 64k units.  This patch adds support for
>> the new register to qemu.  With this patch applied vgabios 0.6c works
>> with qemu.
>>
>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>
> It breaks migration.
>
> vga.c:2218:        VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB),

> 2218 is the interesting line.  Can we freeze this patch until the
> subsections stuff is done?

Yea, I see.  Well, the new register doesn't carry any state, it is 
read-only information for the guest.  So the easy way out is to simply 
not save it as we don't have to.

cheers,
   Gerd

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 03/24/10 18:04, Juan Quintela wrote:
>> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>> The bochs vbe interface got a new register a while back, which specifies
>>> the linear framebuffer size in 64k units.  This patch adds support for
>>> the new register to qemu.  With this patch applied vgabios 0.6c works
>>> with qemu.
>>>
>>> Signed-off-by: Gerd Hoffmann<kraxel@redhat.com>
>>
>> It breaks migration.
>>
>> vga.c:2218:        VMSTATE_UINT16_ARRAY(vbe_regs, VGACommonState, VBE_DISPI_INDEX_NB),
>
>> 2218 is the interesting line.  Can we freeze this patch until the
>> subsections stuff is done?
>
> Yea, I see.  Well, the new register doesn't carry any state, it is
> read-only information for the guest.  So the easy way out is to simply
> not save it as we don't have to.

Thinking a bit more about it.

Humm, I think it means.  Can you migrate from a "new" vga to an old one,
and maintain it working?

-    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID0;
+    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
+    s->vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] = s->vram_size / (64 * 1024);

After migration, vbe_regs[VBE_DISPI_INDEX_ID] would have the value
VBE_DISPI_ID5, but vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] will have
any random value, no?

>
> cheers,
>   Gerd

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Gerd Hoffmann]

On 03/24/10 23:28, Juan Quintela wrote:
> Humm, I think it means.  Can you migrate from a "new" vga to an old one,
> and maintain it working?

Depends on the vgabios version.

vgabios 0.6c will not support vesa gfx modes on older qemu, no matter 
whenever you started fresh or migrated to it.

> -    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID0;
> +    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
> +    s->vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] = s->vram_size / (64 * 1024);

> After migration, vbe_regs[VBE_DISPI_INDEX_ID] would have the value
> VBE_DISPI_ID5, but vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] will have
> any random value, no?

vgabios uses both once at init time.  resetting vga will reset the vbe 
regs too.  So when migrating from new to old qemu:  Before reset vgabios 
will have the video memory size saved somewhere.  After reset ID will 
reset to ID0, and in case you are running vgabios 0.6c vesa gfx modes 
will stop working.

cheers,
   Gerd

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
> On 03/24/10 23:28, Juan Quintela wrote:
>> Humm, I think it means.  Can you migrate from a "new" vga to an old one,
>> and maintain it working?
>
> Depends on the vgabios version.
>
> vgabios 0.6c will not support vesa gfx modes on older qemu, no matter
> whenever you started fresh or migrated to it.
>
>> -    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID0;
>> +    s->vbe_regs[VBE_DISPI_INDEX_ID] = VBE_DISPI_ID5;
>> +    s->vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] = s->vram_size / (64 * 1024);
>
>> After migration, vbe_regs[VBE_DISPI_INDEX_ID] would have the value
>> VBE_DISPI_ID5, but vbe_regs[VBE_DISPI_INDEX_VIDEO_MEMORY_64K] will have
>> any random value, no?
>
> vgabios uses both once at init time. 

Then our big problem is migration between read of the 1st register and
of the 2nd register, no?

Furthermore, older vga bios, seing VBE_DISPI_ID5, what are they going to
do?

> resetting vga will reset the vbe
> regs too.

Yes, but I guess you agree that forcing a reset in the middle of a live
migration makes the whole point of migration moot :-)

>  So when migrating from new to old qemu:  Before reset
> vgabios will have the video memory size saved somewhere.  After reset
> ID will reset to ID0, and in case you are running vgabios 0.6c vesa
> gfx modes will stop working.

I see this part, but I still think that we have a window where we can be
in very bad shape, no?  I guess that we don't support anything different
that vgabios, so ....

> cheers,
>   Gerd

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Gerd Hoffmann]

   Hi,

> Then our big problem is migration between read of the 1st register and
> of the 2nd register, no?

"big"?  The window is quite small, and I think we have a bunch of 
simliar issues elsewhere in qemu.  They are hardly avoidable for new -> 
old migration when adding new features to emulated devices.

Well, maybe sections can fix it, but probably only in case the old qemu 
is new enougth that it can handle sections too.

> Furthermore, older vga bios, seing VBE_DISPI_ID5, what are they going to
> do?

Work as they did before ;)

>>   So when migrating from new to old qemu:  Before reset
>> vgabios will have the video memory size saved somewhere.  After reset
>> ID will reset to ID0, and in case you are running vgabios 0.6c vesa
>> gfx modes will stop working.
>
> I see this part, but I still think that we have a window where we can be
> in very bad shape, no?  I guess that we don't support anything different
> that vgabios, so ....

See above.  Worst case is that vesa graphics modes stop working, even if 
you hit the race window (vgabios will think you have 0 MB video ram then 
and refuse all gfx modes).

cheers,
   Gerd

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Juan Quintela]

Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
>> Then our big problem is migration between read of the 1st register and
>> of the 2nd register, no?
>
> "big"? 

That is why I asked.  I have no clue about how many times that register
is read.

> The window is quite small, and I think we have a bunch of
> simliar issues elsewhere in qemu.  They are hardly avoidable for new
> -> 
> old migration when adding new features to emulated devices.

yeap, but normally we don't allow migration from new to old for this
very reason.  Not allowing from new to old fixes this issue.  Sections
can't help here :(

> Well, maybe sections can fix it, but probably only in case the old
> qemu is new enougth that it can handle sections too.

sections don't help here :(
We are changing how hardware works under the BIOS code, we told 1st that
we have a feature and when BIOS go to use it, feature has disappeared.

>> Furthermore, older vga bios, seing VBE_DISPI_ID5, what are they going to
>> do?
>
> Work as they did before ;)

/me just like BIOS :) 

>>>   So when migrating from new to old qemu:  Before reset
>>> vgabios will have the video memory size saved somewhere.  After reset
>>> ID will reset to ID0, and in case you are running vgabios 0.6c vesa
>>> gfx modes will stop working.
>>
>> I see this part, but I still think that we have a window where we can be
>> in very bad shape, no?  I guess that we don't support anything different
>> that vgabios, so ....
>
> See above.  Worst case is that vesa graphics modes stop working, even
> if you hit the race window (vgabios will think you have 0 MB video ram
> then and refuse all gfx modes).

If it is only read during startup, once, I think that geting 0 size is
ok.  Extra bonus for modifying vgabios to handle this case nicely?

Should be a case of

if (val == 0) {
   do like old bios
}

no?

Later, Juan.

[Qemu-devel] Re: [PATCH] update bochs vbe interface

[Gerd Hoffmann]

On 03/25/10 13:14, Juan Quintela wrote:
> Gerd Hoffmann<kraxel@redhat.com>  wrote:
>>    Hi,
>>
>>> Then our big problem is migration between read of the 1st register and
>>> of the 2nd register, no?
>>
>> "big"?
>
> That is why I asked.  I have no clue about how many times that register
> is read.

Once at boot.

> If it is only read during startup, once, I think that geting 0 size is
> ok.  Extra bonus for modifying vgabios to handle this case nicely?

I doubt it is worth the trouble.  Also note that the current qemu 
vgabios doesn't care at all.  It matters once we actually update vgabios 
to 0.6c.

cheers,
   Gerd