Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Wed, Feb 08, 2012 at 09:18:24AM +0200, Gleb Natapov wrote: On Tue, Feb 07, 2012 at 07:35:34PM -0500, Kevin O'Connor wrote: I'm concerned about the VGA passthrough case. (I know that's not common and has other issues, but I also know several people have been working with it.) As near as I can tell, running the VGA rom on S3 resume has as much chance of breaking things as helping things. It's fine for the cirrus/bochsvga vgaroms that are totally under our control, but it'd be an open guess for any third-party code. (Again, if someone has documentation to the contrary please let me know.) VGA passthrough does not work with QEMU without code changes. Whoever works on it will have to provide etc/s3-resume-vga-init file with appropriate value. My patch above does not remove run time selection, it only changes the default. True. I view running the vgabios on s3 a hack and think an explicit please apply hack flag is nicer than the inverse. However, it's clear this hack helps the majority of qemu/kvm users. So, I'm okay with changing the default. It is a change of default though (upstream kvm/qemu has never run the vgabios on s3 resume before). So, we need to make sure there's proper notice of the change and assuming no objection I'll go forward with it. -Kevin
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Mon, Feb 06, 2012 at 07:09:42PM -0500, Kevin O'Connor wrote:
On Mon, Feb 06, 2012 at 01:43:42PM -0200, Luiz Capitulino wrote:
Kevin O'Connor ke...@koconnor.net wrote:
On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote:
On Fri, 03 Feb 2012 11:23:05 -0600
Michael Roth mdr...@linux.vnet.ibm.com wrote:
I'd been tracking Gerd's QMP wakeup series as the s3 resolution we
need
for guest-suspend, is that still the case?
Yes. But now I remembered about a seabios bug with S3... Need to check
if
it were already addressed.
I'm not aware of any recent S3 defects in SeaBIOS. If there is a
defect, please let me know.
(I am aware of recent discussions on SeaBIOS and it running the
vgabios on s3-resume, but I would not classify that issue as a
defect.)
The problem is that, the screen goes black after resuming from S3. Gleb
debugged it a bit and he said that it was caused by a change in seabios.
Please, take a look at the last three comments in this bz:
https://bugzilla.redhat.com/show_bug.cgi?id=772614
Perhaps a semantic distinction, but I don't consider that to be a
seabios defect.
Non optimal default. The default didn't change BTW, but it was config
parameter before and we changed it for RHEL. Now config parameter is
gone.
In any case, I don't think this was addressed. Gerd published a patch
that can address this in qemu:
http://www.seabios.org/pipermail/seabios/2012-January/002944.html
Strictly speaking the patch is incorrect since it introduces the file
for all architectures, but I do not think qemu is the right place to
tune SeaBIOS defaults. I propose this patch instead:
---
Run vgabios during S3 resume by default on QEMU. QEMU still able to modify
SeaBIOS behavior if it wishes so by providing etc/s3-resume-vga-init file.
Gleb Natapov g...@redhat.com
diff --git a/src/optionroms.c b/src/optionroms.c
index 27cfffd..06db1c1 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -423,7 +423,7 @@ vga_setup(void)
// Load some config settings that impact VGA.
EnforceChecksum = romfile_loadint(etc/optionroms-checksum, 1);
-S3ResumeVgaInit = romfile_loadint(etc/s3-resume-vga-init, 0);
+S3ResumeVgaInit = romfile_loadint(etc/s3-resume-vga-init,
!CONFIG_COREBOOT);
ScreenAndDebug = romfile_loadint(etc/screen-and-debug, 1);
if (CONFIG_OPTIONROMS_DEPLOYED) {
--
Gleb.
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Tue, Feb 07, 2012 at 10:44:39AM +0200, Gleb Natapov wrote: On Mon, Feb 06, 2012 at 07:09:42PM -0500, Kevin O'Connor wrote: Perhaps a semantic distinction, but I don't consider that to be a seabios defect. Non optimal default. The default didn't change BTW, but it was config parameter before and we changed it for RHEL. Now config parameter is gone. Config parameter moved from compile time to runtime. But I agree it is unfortunate that knowledge of that didn't get to the right people. In any case, I don't think this was addressed. Gerd published a patch that can address this in qemu: http://www.seabios.org/pipermail/seabios/2012-January/002944.html Strictly speaking the patch is incorrect since it introduces the file for all architectures, but I do not think qemu is the right place to tune SeaBIOS defaults. I propose this patch instead: [...] // Load some config settings that impact VGA. EnforceChecksum = romfile_loadint(etc/optionroms-checksum, 1); -S3ResumeVgaInit = romfile_loadint(etc/s3-resume-vga-init, 0); +S3ResumeVgaInit = romfile_loadint(etc/s3-resume-vga-init, !CONFIG_COREBOOT); ScreenAndDebug = romfile_loadint(etc/screen-and-debug, 1); I'm concerned about the VGA passthrough case. (I know that's not common and has other issues, but I also know several people have been working with it.) As near as I can tell, running the VGA rom on S3 resume has as much chance of breaking things as helping things. It's fine for the cirrus/bochsvga vgaroms that are totally under our control, but it'd be an open guess for any third-party code. (Again, if someone has documentation to the contrary please let me know.) So, compiling this into SeaBIOS doesn't seems like the right choice to me. -Kevin
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Tue, Feb 07, 2012 at 07:35:34PM -0500, Kevin O'Connor wrote: In any case, I don't think this was addressed. Gerd published a patch that can address this in qemu: http://www.seabios.org/pipermail/seabios/2012-January/002944.html Strictly speaking the patch is incorrect since it introduces the file for all architectures, but I do not think qemu is the right place to tune SeaBIOS defaults. I propose this patch instead: [...] // Load some config settings that impact VGA. EnforceChecksum = romfile_loadint(etc/optionroms-checksum, 1); -S3ResumeVgaInit = romfile_loadint(etc/s3-resume-vga-init, 0); +S3ResumeVgaInit = romfile_loadint(etc/s3-resume-vga-init, !CONFIG_COREBOOT); ScreenAndDebug = romfile_loadint(etc/screen-and-debug, 1); I'm concerned about the VGA passthrough case. (I know that's not common and has other issues, but I also know several people have been working with it.) As near as I can tell, running the VGA rom on S3 resume has as much chance of breaking things as helping things. It's fine for the cirrus/bochsvga vgaroms that are totally under our control, but it'd be an open guess for any third-party code. (Again, if someone has documentation to the contrary please let me know.) VGA passthrough does not work with QEMU without code changes. Whoever works on it will have to provide etc/s3-resume-vga-init file with appropriate value. My patch above does not remove run time selection, it only changes the default. So, compiling this into SeaBIOS doesn't seems like the right choice to me. It is still run time selectable. I think it is best of both worlds. -- Gleb.
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Sat, 4 Feb 2012 10:34:32 -0500 Kevin O'Connor ke...@koconnor.net wrote: On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote: On Fri, 03 Feb 2012 11:23:05 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need for guest-suspend, is that still the case? Yes. But now I remembered about a seabios bug with S3... Need to check if it were already addressed. Hi Luiz, I'm not aware of any recent S3 defects in SeaBIOS. If there is a defect, please let me know. (I am aware of recent discussions on SeaBIOS and it running the vgabios on s3-resume, but I would not classify that issue as a defect.) The problem is that, the screen goes black after resuming from S3. Gleb debugged it a bit and he said that it was caused by a change in seabios. Please, take a look at the last three comments in this bz: https://bugzilla.redhat.com/show_bug.cgi?id=772614
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Mon, Feb 06, 2012 at 01:43:42PM -0200, Luiz Capitulino wrote: Kevin O'Connor ke...@koconnor.net wrote: On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote: On Fri, 03 Feb 2012 11:23:05 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need for guest-suspend, is that still the case? Yes. But now I remembered about a seabios bug with S3... Need to check if it were already addressed. I'm not aware of any recent S3 defects in SeaBIOS. If there is a defect, please let me know. (I am aware of recent discussions on SeaBIOS and it running the vgabios on s3-resume, but I would not classify that issue as a defect.) The problem is that, the screen goes black after resuming from S3. Gleb debugged it a bit and he said that it was caused by a change in seabios. Please, take a look at the last three comments in this bz: https://bugzilla.redhat.com/show_bug.cgi?id=772614 Perhaps a semantic distinction, but I don't consider that to be a seabios defect. In any case, I don't think this was addressed. Gerd published a patch that can address this in qemu: http://www.seabios.org/pipermail/seabios/2012-January/002944.html -Kevin
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Fri, Feb 03, 2012 at 05:16:27PM -0200, Luiz Capitulino wrote: On Fri, 03 Feb 2012 11:23:05 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need for guest-suspend, is that still the case? Yes. But now I remembered about a seabios bug with S3... Need to check if it were already addressed. Hi Luiz, I'm not aware of any recent S3 defects in SeaBIOS. If there is a defect, please let me know. (I am aware of recent discussions on SeaBIOS and it running the vgabios on s3-resume, but I would not classify that issue as a defect.) -Kevin
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Thu, 2 Feb 2012 13:58:52 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this?
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Fri, 03 Feb 2012 10:37:25 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: On 02/03/2012 08:18 AM, Luiz Capitulino wrote: On Thu, 2 Feb 2012 13:58:52 -0600 Michael Rothmdr...@linux.vnet.ibm.com wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this? Hopefully soon, was planning on waiting for the suspend/hibernate bits but we seem to be blocked on the s3 issues and I have other patches accumulating on top of win32 (hesitant to base those on master since this patchset does a lot of refactoring that might affect them), so I figured I'd push this for merge since it doesn't have any dependencies outside master. The S3 issues seem sorted to me, but I don't oppose having this series in first.
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On 02/03/2012 08:18 AM, Luiz Capitulino wrote: On Thu, 2 Feb 2012 13:58:52 -0600 Michael Rothmdr...@linux.vnet.ibm.com wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this? Hopefully soon, was planning on waiting for the suspend/hibernate bits but we seem to be blocked on the s3 issues and I have other patches accumulating on top of win32 (hesitant to base those on master since this patchset does a lot of refactoring that might affect them), so I figured I'd push this for merge since it doesn't have any dependencies outside master.
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On 02/03/2012 10:45 AM, Luiz Capitulino wrote: On Fri, 03 Feb 2012 10:37:25 -0600 Michael Rothmdr...@linux.vnet.ibm.com wrote: On 02/03/2012 08:18 AM, Luiz Capitulino wrote: On Thu, 2 Feb 2012 13:58:52 -0600 Michael Rothmdr...@linux.vnet.ibm.com wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this? Hopefully soon, was planning on waiting for the suspend/hibernate bits but we seem to be blocked on the s3 issues and I have other patches accumulating on top of win32 (hesitant to base those on master since this patchset does a lot of refactoring that might affect them), so I figured I'd push this for merge since it doesn't have any dependencies outside master. The S3 issues seem sorted to me, but I don't oppose having this series in first. Thanks, in retrospect I probably should've just gotten these out of the way weeks ago since they'd immediately clobber git blame. I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need for guest-suspend, is that still the case? I guess those are coming through your QMP queue?
Re: [Qemu-devel] [PATCH v2 0/8] qemu-ga: add support for Windows
On Fri, 03 Feb 2012 11:23:05 -0600 Michael Roth mdr...@linux.vnet.ibm.com wrote: On 02/03/2012 10:45 AM, Luiz Capitulino wrote: On Fri, 03 Feb 2012 10:37:25 -0600 Michael Rothmdr...@linux.vnet.ibm.com wrote: On 02/03/2012 08:18 AM, Luiz Capitulino wrote: On Thu, 2 Feb 2012 13:58:52 -0600 Michael Rothmdr...@linux.vnet.ibm.com wrote: These patches apply on top of qemu.git master, and can also be obtained from: git://github.com/mdroth/qemu.git qga-win32-v2 Luiz/Gal, I decided not to roll the suspend/hibernate stuff into this series since the s3 situation isn't fully sorted out yet. The file structure is a little different now, posix/linux-specific stuff goes in qga/commands-posix.c, win32-specific stuff in qga/commands-win32.c, but other than that it should be a straightforward rebase if this gets merged first. I think I'll have to rebase my series on top of this one, when do you plan to merge this? Hopefully soon, was planning on waiting for the suspend/hibernate bits but we seem to be blocked on the s3 issues and I have other patches accumulating on top of win32 (hesitant to base those on master since this patchset does a lot of refactoring that might affect them), so I figured I'd push this for merge since it doesn't have any dependencies outside master. The S3 issues seem sorted to me, but I don't oppose having this series in first. Thanks, in retrospect I probably should've just gotten these out of the way weeks ago since they'd immediately clobber git blame. I'd been tracking Gerd's QMP wakeup series as the s3 resolution we need for guest-suspend, is that still the case? Yes. But now I remembered about a seabios bug with S3... Need to check if it were already addressed. I guess those are coming through your QMP queue? Oh, as the QMP part is trivial I thought someone else would pick them up, but I can do that.
