| Message ID | 1362499403-16514-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, 2013-03-05 at 17:03 +0100, Paolo Bonzini wrote: > Resuming from suspend-to-RAM should not reset all devices. Only the > CPU should get a reset signal. Hm... on reflection, I don't actually know if this is true. Perhaps we *should* reset all devices. After all, in a real machine they'll all have been turned off and the RAM will have been in self-refresh. Surely they have to be reset? So maybe we should *let* the i440FX PAM registers get reset to point to ROM. And fix the firmware to *cope* with that, check to see if the shadow RAM already holds an image of a started-up firmware with the correct checksum, and jump back to it. That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't work if the PAM configuration is reset?
Il 05/03/2013 17:59, David Woodhouse ha scritto: > On Tue, 2013-03-05 at 17:03 +0100, Paolo Bonzini wrote: >> > Resuming from suspend-to-RAM should not reset all devices. Only the >> > CPU should get a reset signal. > Hm... on reflection, I don't actually know if this is true. > > Perhaps we *should* reset all devices. After all, in a real machine > they'll all have been turned off and the RAM will have been in > self-refresh. Surely they have to be reset? > > So maybe we should *let* the i440FX PAM registers get reset to point to > ROM. And fix the firmware to *cope* with that, check to see if the > shadow RAM already holds an image of a started-up firmware with the > correct checksum, and jump back to it. > > That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't work > if the PAM configuration is reset? Yeah, it sounded a bit weird when I wrote that commit message. This could be the case. How does it work on Coreboot? Paolo
Paolo Bonzini wrote: > > That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't > > work if the PAM configuration is reset? > > Yeah, it sounded a bit weird when I wrote that commit message. > This could be the case. How does it work on Coreboot? Yes all hardware except RAM in self-refresh and registers explicitly hooked to battery sleep power rail has reset state on resume from S3. Those registers seem to grow over time, the i440 probably doesn't have very many. The memory controller configuration would be stored there. Power on and resume are identical except for memory controller init and what should happen once init is finished. On power on jump to payload. On resume jump to S3 resume vector set by OSPM. This is another case where coreboot has infrastructure since a long time, which can be used by hypervisors. Please don't replicate that into SeaBIOS. In the end you will have what is essentially a fork of coreboot, which seems like a tremendous waste of effort. See if there is a 440 board in coreboot where suspend works. If yes, then making that work for the QEMU board should take only moderate effort. //Peter
Il 05/03/2013 18:26, Peter Stuge ha scritto: >>> > > That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't >>> > > work if the PAM configuration is reset? >> > >> > Yeah, it sounded a bit weird when I wrote that commit message. >> > This could be the case. How does it work on Coreboot? > Yes all hardware except RAM in self-refresh and registers explicitly > hooked to battery sleep power rail has reset state on resume from S3. > Those registers seem to grow over time, the i440 probably doesn't > have very many. The memory controller configuration would be stored > there. Ok, so it's not correct either with this patch or without. :) But without this patch it is more correct; things such as USB queue heads must be reset, or you'll corrupt random memory when the VM is restarted. Paolo
On 03/05/13 17:59, David Woodhouse wrote:> On Tue, 2013-03-05 at 17:03 +0100, Paolo Bonzini wrote: >> Resuming from suspend-to-RAM should not reset all devices. Only the >> CPU should get a reset signal. > > Hm... on reflection, I don't actually know if this is true. > > Perhaps we *should* reset all devices. After all, in a real machine > they'll all have been turned off and the RAM will have been in > self-refresh. Surely they have to be reset? > > So maybe we should *let* the i440FX PAM registers get reset to point to > ROM. And fix the firmware to *cope* with that, check to see if the > shadow RAM already holds an image of a started-up firmware with the > correct checksum, and jump back to it. > > That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't work > if the PAM configuration is reset? I think it is indeed a problem with SeaBIOS. (git.seabios.org (80.81.252.135) seems to be down ATM, so I can only check at f465e1ec.) Open romlayout.S: 622 reset_vector: 623 ljmpw $SEG_BIOS, $entry_post 537 entry_post: 538 cmpl $0, %cs:HaveRunPost // Check for resume/reboot 539 jnz entry_resume 540 ENTRY_INTO32 _cfunc32flat_handle_post // Normal entry point 239 entry_resume: ... 248 // Call handler. 249 jmp handle_resume handle_resume() [src/resume.c] inb_cmos(CMOS_RESET_CODE) It checks the CMOS only after looking at HaveRunPost. The value of HaveRunPost depends on the PAM settings. It's always 0 in ROM, in which case we continue at handle_post() [src/post.c]. Laszlo
Il 05/03/2013 20:12, Laszlo Ersek ha scritto: > On 03/05/13 17:59, David Woodhouse wrote:> On Tue, 2013-03-05 at 17:03 +0100, Paolo Bonzini wrote: >>> Resuming from suspend-to-RAM should not reset all devices. Only the >>> CPU should get a reset signal. >> >> Hm... on reflection, I don't actually know if this is true. >> >> Perhaps we *should* reset all devices. After all, in a real machine >> they'll all have been turned off and the RAM will have been in >> self-refresh. Surely they have to be reset? >> >> So maybe we should *let* the i440FX PAM registers get reset to point to >> ROM. And fix the firmware to *cope* with that, check to see if the >> shadow RAM already holds an image of a started-up firmware with the >> correct checksum, and jump back to it. >> >> That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't work >> if the PAM configuration is reset? > > I think it is indeed a problem with SeaBIOS. Open romlayout.S: [...] > > It checks the CMOS only after looking at HaveRunPost. The value of > HaveRunPost depends on the PAM settings. It's always 0 in ROM, in which > case we continue at handle_post() [src/post.c]. Actually, Peter explained that it is okay. S3 doesn't clear the PAM configuration, but S4 does. The PAM registers are attached to the same power line as the RAM. Paolo
On 03/05/13 20:25, Paolo Bonzini wrote: > Il 05/03/2013 20:12, Laszlo Ersek ha scritto: >> On 03/05/13 17:59, David Woodhouse wrote:> On Tue, 2013-03-05 at 17:03 +0100, Paolo Bonzini wrote: >>>> Resuming from suspend-to-RAM should not reset all devices. Only the >>>> CPU should get a reset signal. >>> >>> Hm... on reflection, I don't actually know if this is true. >>> >>> Perhaps we *should* reset all devices. After all, in a real machine >>> they'll all have been turned off and the RAM will have been in >>> self-refresh. Surely they have to be reset? >>> >>> So maybe we should *let* the i440FX PAM registers get reset to point to >>> ROM. And fix the firmware to *cope* with that, check to see if the >>> shadow RAM already holds an image of a started-up firmware with the >>> correct checksum, and jump back to it. >>> >>> That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't work >>> if the PAM configuration is reset? >> >> I think it is indeed a problem with SeaBIOS. Open romlayout.S: [...] >> >> It checks the CMOS only after looking at HaveRunPost. The value of >> HaveRunPost depends on the PAM settings. It's always 0 in ROM, in which >> case we continue at handle_post() [src/post.c]. > > Actually, Peter explained that it is okay. S3 doesn't clear the PAM > configuration, but S4 does. The PAM registers are attached to the same > power line as the RAM. Ah, you expect me to understand that "memory controller configuration" is a superset of the PAM registers :) Laszlo
On Tue, Mar 05, 2013 at 04:59:51PM +0000, David Woodhouse wrote: > On Tue, 2013-03-05 at 17:03 +0100, Paolo Bonzini wrote: > > Resuming from suspend-to-RAM should not reset all devices. Only the > > CPU should get a reset signal. > > Hm... on reflection, I don't actually know if this is true. > > Perhaps we *should* reset all devices. After all, in a real machine > they'll all have been turned off and the RAM will have been in > self-refresh. Surely they have to be reset? > > So maybe we should *let* the i440FX PAM registers get reset to point to > ROM. And fix the firmware to *cope* with that, check to see if the > shadow RAM already holds an image of a started-up firmware with the > correct checksum, and jump back to it. > > That is: perhaps it's a *SeaBIOS* bug that suspend/resume doesn't work > if the PAM configuration is reset? On a real machine the firmware would query the available memory and its settings via the smbus and then program the memory controller. Programming the memory controller without accessing any memory is a non-trivial task. QEMU doesn't emulate this low-level hardware - nor does it even fully emulate the PAM registers. So, I think the question isn't what does real-hardware do (there's no gain to be had in emulating all of this). Instead, I think the question is - what makes the most sense. Given that the objective is to restore the memory image and reset everything else, I think it would be best for QEMU's S3resume to completely restore the memory image - including the PAM settings. -Kevin
On Tue, 2013-03-05 at 20:45 -0500, Kevin O'Connor wrote: > > So, I think the question isn't what does real-hardware do (there's no > gain to be had in emulating all of this). Instead, I think the > question is - what makes the most sense. Well,... what we implement for the 'native SeaBIOS on qemu' case doesn't need to work on real hardware, it's true. But a bunch of other combinations *do* need to work on real hardware, so it's worth bearing it in mind to a certain extent. We wouldn't want Coreboot and OVMF to have to have workarounds for qemu behaving differently, just because we've done something simpler for SeaBIOS. But it looks like the PAM configuration is considered part of the memory setup and isn't lost in S3, so for the case I was wondering about it's fine. Qemu *is* behaving like real hardware, which *is* the simple option that makes most sense.
diff --git a/vl.c b/vl.c index c03edf1..2c93478 100644 --- a/vl.c +++ b/vl.c @@ -1973,8 +1973,8 @@ static bool main_loop_should_exit(void) } if (qemu_wakeup_requested()) { pause_all_vcpus(); + cpu_soft_reset(); cpu_synchronize_all_states(); - qemu_system_reset(VMRESET_SILENT); resume_all_vcpus(); monitor_protocol_event(QEVENT_WAKEUP, NULL); }
Resuming from suspend-to-RAM should not reset all devices. Only the CPU should get a reset signal. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)