| Message ID | 1269440070-26788-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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
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
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
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
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.
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
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
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(-)