Message ID | 20240806160756.182524-3-jmarcin@redhat.com |
---|---|
State | New |
Headers | show |
Series | virtio-mem: Implement support for suspend+wake-up with plugged memory | expand |
On 06.08.24 18:07, Juraj Marcin wrote: > Some devices need to distinguish cold start reset from waking up from a > suspended state. This patch adds new value to the enum, and updates the > i386 wakeup method to use this new reset type. > > Signed-off-by: Juraj Marcin <jmarcin@redhat.com> > --- > docs/devel/reset.rst | 7 +++++++ > hw/i386/pc.c | 2 +- > include/hw/resettable.h | 2 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst > index 9746a4e8a0..30c9a0cc2b 100644 > --- a/docs/devel/reset.rst > +++ b/docs/devel/reset.rst > @@ -44,6 +44,13 @@ The Resettable interface handles reset types with an enum ``ResetType``: > value on each cold reset, such as RNG seed information, and which they > must not reinitialize on a snapshot-load reset. > > +``RESET_TYPE_WAKEUP`` > + This type is used when the machine is woken up from a suspended state (deep > + sleep, suspend-to-ram). Devices that must not be reset to their initial state > + after wake-up (for example virtio-mem) can use this state to differentiate > + cold start from wake-up can use this state to differentiate cold start from > + wake-up. > + > Devices which implement reset methods must treat any unknown ``ResetType`` > as equivalent to ``RESET_TYPE_COLD``; this will reduce the amount of > existing code we need to change if we add more types in future. ^ That sounds like using this for x86 below should just work. Reviewed-by: David Hildenbrand <david@redhat.com>
On Tue, 6 Aug 2024 at 17:08, Juraj Marcin <jmarcin@redhat.com> wrote: > > Some devices need to distinguish cold start reset from waking up from a > suspended state. This patch adds new value to the enum, and updates the > i386 wakeup method to use this new reset type. > > Signed-off-by: Juraj Marcin <jmarcin@redhat.com> > --- > docs/devel/reset.rst | 7 +++++++ > hw/i386/pc.c | 2 +- > include/hw/resettable.h | 2 ++ > 3 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst > index 9746a4e8a0..30c9a0cc2b 100644 > --- a/docs/devel/reset.rst > +++ b/docs/devel/reset.rst > @@ -44,6 +44,13 @@ The Resettable interface handles reset types with an enum ``ResetType``: > value on each cold reset, such as RNG seed information, and which they > must not reinitialize on a snapshot-load reset. > > +``RESET_TYPE_WAKEUP`` > + This type is used when the machine is woken up from a suspended state (deep > + sleep, suspend-to-ram). Devices that must not be reset to their initial state > + after wake-up (for example virtio-mem) can use this state to differentiate > + cold start from wake-up can use this state to differentiate cold start from > + wake-up. I feel like this needs more clarity about what this is, since as a reset type it's a general behaviour, not a machine specific one. What exactly is "wakeup" and when does it happen? How does it differ from what you might call a "warm" reset, where the user pressed the front-panel reset button? Why is virtio-mem in particular interesting here? thanks -- PMM
On Thu, Aug 8, 2024 at 2:18 PM Peter Maydell <peter.maydell@linaro.org> wrote: > > On Tue, 6 Aug 2024 at 17:08, Juraj Marcin <jmarcin@redhat.com> wrote: > > > > Some devices need to distinguish cold start reset from waking up from a > > suspended state. This patch adds new value to the enum, and updates the > > i386 wakeup method to use this new reset type. > > > > Signed-off-by: Juraj Marcin <jmarcin@redhat.com> > > --- > > docs/devel/reset.rst | 7 +++++++ > > hw/i386/pc.c | 2 +- > > include/hw/resettable.h | 2 ++ > > 3 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst > > index 9746a4e8a0..30c9a0cc2b 100644 > > --- a/docs/devel/reset.rst > > +++ b/docs/devel/reset.rst > > @@ -44,6 +44,13 @@ The Resettable interface handles reset types with an enum ``ResetType``: > > value on each cold reset, such as RNG seed information, and which they > > must not reinitialize on a snapshot-load reset. > > > > +``RESET_TYPE_WAKEUP`` > > + This type is used when the machine is woken up from a suspended state (deep > > + sleep, suspend-to-ram). Devices that must not be reset to their initial state > > + after wake-up (for example virtio-mem) can use this state to differentiate > > + cold start from wake-up can use this state to differentiate cold start from > > + wake-up. > > I feel like this needs more clarity about what this is, since > as a reset type it's a general behaviour, not a machine > specific one. What exactly is "wakeup" and when does it happen? > How does it differ from what you might call a "warm" reset, > where the user pressed the front-panel reset button? > Why is virtio-mem in particular interesting here? Thank you for the feedback! I have rewritten the paragraph: This type is called for a reset when the system is being woken up from a suspended state using the ``qemu_system_wakeup()`` function. If the machine type needs to reset in its ``MachineClass::wakeup()`` method, this reset type should be used so that devices can differentiate system wake-up from other reset types. For example, a virtio-mem device must not unplug its memory during wake-up, as that would clear the guest RAM. Is it clearer? Thank you! > > thanks > -- PMM > -- Juraj Marcin
On 08.08.24 17:28, Juraj Marcin wrote: > On Thu, Aug 8, 2024 at 2:18 PM Peter Maydell <peter.maydell@linaro.org> wrote: >> >> On Tue, 6 Aug 2024 at 17:08, Juraj Marcin <jmarcin@redhat.com> wrote: >>> >>> Some devices need to distinguish cold start reset from waking up from a >>> suspended state. This patch adds new value to the enum, and updates the >>> i386 wakeup method to use this new reset type. >>> >>> Signed-off-by: Juraj Marcin <jmarcin@redhat.com> >>> --- >>> docs/devel/reset.rst | 7 +++++++ >>> hw/i386/pc.c | 2 +- >>> include/hw/resettable.h | 2 ++ >>> 3 files changed, 10 insertions(+), 1 deletion(-) >>> >>> diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst >>> index 9746a4e8a0..30c9a0cc2b 100644 >>> --- a/docs/devel/reset.rst >>> +++ b/docs/devel/reset.rst >>> @@ -44,6 +44,13 @@ The Resettable interface handles reset types with an enum ``ResetType``: >>> value on each cold reset, such as RNG seed information, and which they >>> must not reinitialize on a snapshot-load reset. >>> >>> +``RESET_TYPE_WAKEUP`` >>> + This type is used when the machine is woken up from a suspended state (deep >>> + sleep, suspend-to-ram). Devices that must not be reset to their initial state >>> + after wake-up (for example virtio-mem) can use this state to differentiate >>> + cold start from wake-up can use this state to differentiate cold start from >>> + wake-up. >> >> I feel like this needs more clarity about what this is, since >> as a reset type it's a general behaviour, not a machine >> specific one. What exactly is "wakeup" and when does it happen? >> How does it differ from what you might call a "warm" reset, >> where the user pressed the front-panel reset button? >> Why is virtio-mem in particular interesting here? > > Thank you for the feedback! > > I have rewritten the paragraph: > > This type is called for a reset when the system is being woken up from > a suspended state using the ``qemu_system_wakeup()`` function. If the > machine type needs to reset in its ``MachineClass::wakeup()`` method, > this reset type should be used so that devices can differentiate > system wake-up from other reset types. For example, a virtio-mem > device must not unplug its memory during wake-up, as that would clear > the guest RAM. > > Is it clearer? Thank you! Conceptually, if we want to avoid the "WAKEUP" terminology here, maybe we should consider talking about a WARM reset -- in contrast to a COLD one? During a WARM reset, memory content is supposed to stay untouched, which is what we effectively want to achieve with virtio-mem. Peter, what would be your preference?
On Thu, 8 Aug 2024 at 16:31, David Hildenbrand <david@redhat.com> wrote: > > On 08.08.24 17:28, Juraj Marcin wrote: > > On Thu, Aug 8, 2024 at 2:18 PM Peter Maydell <peter.maydell@linaro.org> wrote: > >> > >> On Tue, 6 Aug 2024 at 17:08, Juraj Marcin <jmarcin@redhat.com> wrote: > >>> +``RESET_TYPE_WAKEUP`` > >>> + This type is used when the machine is woken up from a suspended state (deep > >>> + sleep, suspend-to-ram). Devices that must not be reset to their initial state > >>> + after wake-up (for example virtio-mem) can use this state to differentiate > >>> + cold start from wake-up can use this state to differentiate cold start from > >>> + wake-up. > >> > >> I feel like this needs more clarity about what this is, since > >> as a reset type it's a general behaviour, not a machine > >> specific one. What exactly is "wakeup" and when does it happen? > >> How does it differ from what you might call a "warm" reset, > >> where the user pressed the front-panel reset button? > >> Why is virtio-mem in particular interesting here? > > > > Thank you for the feedback! > > > > I have rewritten the paragraph: > > > > This type is called for a reset when the system is being woken up from > > a suspended state using the ``qemu_system_wakeup()`` function. If the > > machine type needs to reset in its ``MachineClass::wakeup()`` method, > > this reset type should be used so that devices can differentiate > > system wake-up from other reset types. For example, a virtio-mem > > device must not unplug its memory during wake-up, as that would clear > > the guest RAM. > > > > Is it clearer? Thank you! > > Conceptually, if we want to avoid the "WAKEUP" terminology here, maybe > we should consider talking about a WARM reset -- in contrast to a COLD one? > > During a WARM reset, memory content is supposed to stay untouched, which > is what we effectively want to achieve with virtio-mem. Right, I guess that's my question -- is "WAKEUP" ever any different from "WARM" reset? I think the latter is a more common general concept. On the other hand it looks like we already have the concept in the runstate state machine of RUN_STATE_SUSPENDED versus RUN_STATE_RUNNING, and so if we define "WAKEUP" as "goes from SUSPENDED to RUNNING" that's quite a clearly defined condition. Whereas WARM reset would be a much wider range of things and ought to include for instance "guest triggers a reset by writing to port 92" and all the other SHUTDOWN_CAUSE_GUEST_RESET cases. (Side note: do we document all these runstates and transitions anywhere?) For virtio-mem, on a guest-triggered reset, should it (a) definitely not unplug all the hotplugged memory (b) definitely unplug all the hotplugged memory (c) we don't care? If (a) then that seems to push towards calling all these cases of a "warm" reset; if (b) then that would be a reason to make "warm" and "wakeup" different. thanks -- PMM
On 08.08.24 17:56, Peter Maydell wrote: > On Thu, 8 Aug 2024 at 16:31, David Hildenbrand <david@redhat.com> wrote: >> >> On 08.08.24 17:28, Juraj Marcin wrote: >>> On Thu, Aug 8, 2024 at 2:18 PM Peter Maydell <peter.maydell@linaro.org> wrote: >>>> >>>> On Tue, 6 Aug 2024 at 17:08, Juraj Marcin <jmarcin@redhat.com> wrote: >>>>> +``RESET_TYPE_WAKEUP`` >>>>> + This type is used when the machine is woken up from a suspended state (deep >>>>> + sleep, suspend-to-ram). Devices that must not be reset to their initial state >>>>> + after wake-up (for example virtio-mem) can use this state to differentiate >>>>> + cold start from wake-up can use this state to differentiate cold start from >>>>> + wake-up. >>>> >>>> I feel like this needs more clarity about what this is, since >>>> as a reset type it's a general behaviour, not a machine >>>> specific one. What exactly is "wakeup" and when does it happen? >>>> How does it differ from what you might call a "warm" reset, >>>> where the user pressed the front-panel reset button? >>>> Why is virtio-mem in particular interesting here? >>> >>> Thank you for the feedback! >>> >>> I have rewritten the paragraph: >>> >>> This type is called for a reset when the system is being woken up from >>> a suspended state using the ``qemu_system_wakeup()`` function. If the >>> machine type needs to reset in its ``MachineClass::wakeup()`` method, >>> this reset type should be used so that devices can differentiate >>> system wake-up from other reset types. For example, a virtio-mem >>> device must not unplug its memory during wake-up, as that would clear >>> the guest RAM. >>> >>> Is it clearer? Thank you! >> >> Conceptually, if we want to avoid the "WAKEUP" terminology here, maybe >> we should consider talking about a WARM reset -- in contrast to a COLD one? >> >> During a WARM reset, memory content is supposed to stay untouched, which >> is what we effectively want to achieve with virtio-mem. > > Right, I guess that's my question -- is "WAKEUP" ever any > different from "WARM" reset? I think the latter is a more > common general concept. > > On the other hand it looks like we already have the > concept in the runstate state machine of > RUN_STATE_SUSPENDED versus RUN_STATE_RUNNING, and so if we > define "WAKEUP" as "goes from SUSPENDED to RUNNING" that's > quite a clearly defined condition. Right. > Whereas WARM reset would > be a much wider range of things and ought to include for > instance "guest triggers a reset by writing to port 92" > and all the other SHUTDOWN_CAUSE_GUEST_RESET cases. > (Side note: do we document all these runstates and transitions > anywhere?) > > For virtio-mem, on a guest-triggered reset, should it > (a) definitely not unplug all the hotplugged memory > (b) definitely unplug all the hotplugged memory > (c) we don't care? During ordinary system resets (COLD) where RAM content is not guaranteed to survive: Effectively (b) During special kexec-style resets (e.g., on s390x there is a difference) where RAM content must survive: Effectively (a) On implementing architectures (x86, arm64), kexec-style resets are really like warm resets. For example, when we trigger kexec->kdump we must not suddenly lose the RAM content. In that sense, at least virtio-mem wants to treat WARM and WAKEUP resets alike. But I agree that simply because virtio-mem want sot treat them alike doesn't mean that we should represent in QEMU using a single reset type. WARM reboots like s390x supports are rather stuff for the future (once s390x actually supports virtio-mem and could end up triggering it). > > If (a) then that seems to push towards calling all these > cases of a "warm" reset; if (b) then that would be a > reason to make "warm" and "wakeup" different. Likely different then.
On Thu, 8 Aug 2024 at 17:04, David Hildenbrand <david@redhat.com> wrote: > > On 08.08.24 17:56, Peter Maydell wrote: > > Right, I guess that's my question -- is "WAKEUP" ever any > > different from "WARM" reset? I think the latter is a more > > common general concept. > > > > On the other hand it looks like we already have the > > concept in the runstate state machine of > > RUN_STATE_SUSPENDED versus RUN_STATE_RUNNING, and so if we > > define "WAKEUP" as "goes from SUSPENDED to RUNNING" that's > > quite a clearly defined condition. > > Right. > > > Whereas WARM reset would > > be a much wider range of things and ought to include for > > instance "guest triggers a reset by writing to port 92" > > and all the other SHUTDOWN_CAUSE_GUEST_RESET cases. > > (Side note: do we document all these runstates and transitions > > anywhere?) > > > > For virtio-mem, on a guest-triggered reset, should it > > (a) definitely not unplug all the hotplugged memory > > (b) definitely unplug all the hotplugged memory > > (c) we don't care? > > During ordinary system resets (COLD) where RAM content is not guaranteed > to survive: "COLD" isn't an "ordinary system reset", though -- COLD reset is more like "I powered the computer off and then turned it on again". A "WARM" reset is like "I pressed the front panel reset button, or the watchdog device fired and reset the system". We don't currently really model front-panel or watchdog reset properly, we assume that it's close enough to have it be the same as power-off-and-on reset (and there are some kludges in various places where it's not quite right). > Effectively (b) > > During special kexec-style resets (e.g., on s390x there is a difference) > where RAM content must survive: > > Effectively (a) > > > On implementing architectures (x86, arm64), kexec-style resets are > really like warm resets. For example, when we trigger kexec->kdump we > must not suddenly lose the RAM content. (There's an awkward conflict here with our rom blob system, which currently does a "copy any guest images back into RAM" on reset. Should we do that on warm reset? For some usecases you want those original images back again, but for "guest did a kexec" you almost certainly don't...) > In that sense, at least virtio-mem wants to treat WARM and WAKEUP resets > alike. But I agree that simply because virtio-mem want sot treat them > alike doesn't mean that we should represent in QEMU using a single reset > type. On the other hand, if virtio-mem does want to treat them the same we could start with only implementing WARM and not add WAKEUP until we have a use-case for it. By the way, I assume this patchseries is 9.2 material, not trying to fix a bug for this release ? thanks -- PMM
On 08.08.24 18:17, Peter Maydell wrote: > On Thu, 8 Aug 2024 at 17:04, David Hildenbrand <david@redhat.com> wrote: >> >> On 08.08.24 17:56, Peter Maydell wrote: >>> Right, I guess that's my question -- is "WAKEUP" ever any >>> different from "WARM" reset? I think the latter is a more >>> common general concept. >>> >>> On the other hand it looks like we already have the >>> concept in the runstate state machine of >>> RUN_STATE_SUSPENDED versus RUN_STATE_RUNNING, and so if we >>> define "WAKEUP" as "goes from SUSPENDED to RUNNING" that's >>> quite a clearly defined condition. >> >> Right. >> >>> Whereas WARM reset would >>> be a much wider range of things and ought to include for >>> instance "guest triggers a reset by writing to port 92" >>> and all the other SHUTDOWN_CAUSE_GUEST_RESET cases. >>> (Side note: do we document all these runstates and transitions >>> anywhere?) >>> >>> For virtio-mem, on a guest-triggered reset, should it >>> (a) definitely not unplug all the hotplugged memory >>> (b) definitely unplug all the hotplugged memory >>> (c) we don't care? >> >> During ordinary system resets (COLD) where RAM content is not guaranteed >> to survive: > > "COLD" isn't an "ordinary system reset", though -- COLD > reset is more like "I powered the computer off and then > turned it on again". A "WARM" reset is like "I pressed > the front panel reset button, or the watchdog device > fired and reset the system". We don't currently really > model front-panel or watchdog reset properly, we assume > that it's close enough to have it be the same as > power-off-and-on reset (and there are some kludges in > various places where it's not quite right). Agreed, there are many flavors of resets, even different flavors of warm ones I'm afraid. To summarize, if a VM does an ordinary reset ("shutdown -r") we want to unplug all hotplugged memory. Same with most external resets we currently implement. In all other caes, we likely want to keep the memory hotplugged and RAM content alive. I think we did something similar with DIMMs/ACPI devices with unplug requests in that past, but I'm not able to locate that code easily. > >> Effectively (b) >> >> During special kexec-style resets (e.g., on s390x there is a difference) >> where RAM content must survive: >> >> Effectively (a) >> >> >> On implementing architectures (x86, arm64), kexec-style resets are >> really like warm resets. For example, when we trigger kexec->kdump we >> must not suddenly lose the RAM content. > > (There's an awkward conflict here with our rom blob > system, which currently does a "copy any guest images > back into RAM" on reset. Should we do that on warm > reset? For some usecases you want those original > images back again, but for "guest did a kexec" you > almost certainly don't...) Right, maybe on some warm resets where the ROM blobs are required and might have gotten corrupted you would actually want to restore them, maybe? It's difficult. At least for "guest did a kexec", I *think* x86 just doesn't trigger any special reset in QEMU. I know that s390x triggers a special subsystem reset (s390x has various types of resets ...). > >> In that sense, at least virtio-mem wants to treat WARM and WAKEUP resets >> alike. But I agree that simply because virtio-mem want sot treat them >> alike doesn't mean that we should represent in QEMU using a single reset >> type. > > On the other hand, if virtio-mem does want to treat them > the same we could start with only implementing WARM and > not add WAKEUP until we have a use-case for it. Makes sense. > > By the way, I assume this patchseries is 9.2 material, > not trying to fix a bug for this release ? Yes, it can wait. Thanks for the guidance!
On Thu, 8 Aug 2024 at 17:29, David Hildenbrand <david@redhat.com> wrote: > > On 08.08.24 18:17, Peter Maydell wrote: > > On Thu, 8 Aug 2024 at 17:04, David Hildenbrand <david@redhat.com> wrote: > >> > >> On 08.08.24 17:56, Peter Maydell wrote: > >>> Right, I guess that's my question -- is "WAKEUP" ever any > >>> different from "WARM" reset? I think the latter is a more > >>> common general concept. > >>> > >>> On the other hand it looks like we already have the > >>> concept in the runstate state machine of > >>> RUN_STATE_SUSPENDED versus RUN_STATE_RUNNING, and so if we > >>> define "WAKEUP" as "goes from SUSPENDED to RUNNING" that's > >>> quite a clearly defined condition. > >> > >> Right. > >> > >>> Whereas WARM reset would > >>> be a much wider range of things and ought to include for > >>> instance "guest triggers a reset by writing to port 92" > >>> and all the other SHUTDOWN_CAUSE_GUEST_RESET cases. > >>> (Side note: do we document all these runstates and transitions > >>> anywhere?) > >>> > >>> For virtio-mem, on a guest-triggered reset, should it > >>> (a) definitely not unplug all the hotplugged memory > >>> (b) definitely unplug all the hotplugged memory > >>> (c) we don't care? > >> > >> During ordinary system resets (COLD) where RAM content is not guaranteed > >> to survive: > > > > "COLD" isn't an "ordinary system reset", though -- COLD > > reset is more like "I powered the computer off and then > > turned it on again". A "WARM" reset is like "I pressed > > the front panel reset button, or the watchdog device > > fired and reset the system". We don't currently really > > model front-panel or watchdog reset properly, we assume > > that it's close enough to have it be the same as > > power-off-and-on reset (and there are some kludges in > > various places where it's not quite right). > > Agreed, there are many flavors of resets, even different flavors of warm > ones I'm afraid. > > To summarize, if a VM does an ordinary reset ("shutdown -r") we want to > unplug all hotplugged memory. Same with most external resets we > currently implement. In all other caes, we likely want to keep the > memory hotplugged and RAM content alive. OK, if we want 'shutdown -r' to unplug hotplugged memory then that sounds like we do want a WAKEUP reset type. (Though I'm surprised that we want that -- my expectation would have been that the hotplugged memory should stay, because if you do real hardware hotplug of RAM then it doesn't all pop back out of the sockets when you press the reset button on the front panel :-)) thanks -- PMM
On 08.08.24 19:30, Peter Maydell wrote: > On Thu, 8 Aug 2024 at 17:29, David Hildenbrand <david@redhat.com> wrote: >> >> On 08.08.24 18:17, Peter Maydell wrote: >>> On Thu, 8 Aug 2024 at 17:04, David Hildenbrand <david@redhat.com> wrote: >>>> >>>> On 08.08.24 17:56, Peter Maydell wrote: >>>>> Right, I guess that's my question -- is "WAKEUP" ever any >>>>> different from "WARM" reset? I think the latter is a more >>>>> common general concept. >>>>> >>>>> On the other hand it looks like we already have the >>>>> concept in the runstate state machine of >>>>> RUN_STATE_SUSPENDED versus RUN_STATE_RUNNING, and so if we >>>>> define "WAKEUP" as "goes from SUSPENDED to RUNNING" that's >>>>> quite a clearly defined condition. >>>> >>>> Right. >>>> >>>>> Whereas WARM reset would >>>>> be a much wider range of things and ought to include for >>>>> instance "guest triggers a reset by writing to port 92" >>>>> and all the other SHUTDOWN_CAUSE_GUEST_RESET cases. >>>>> (Side note: do we document all these runstates and transitions >>>>> anywhere?) >>>>> >>>>> For virtio-mem, on a guest-triggered reset, should it >>>>> (a) definitely not unplug all the hotplugged memory >>>>> (b) definitely unplug all the hotplugged memory >>>>> (c) we don't care? >>>> >>>> During ordinary system resets (COLD) where RAM content is not guaranteed >>>> to survive: >>> >>> "COLD" isn't an "ordinary system reset", though -- COLD >>> reset is more like "I powered the computer off and then >>> turned it on again". A "WARM" reset is like "I pressed >>> the front panel reset button, or the watchdog device >>> fired and reset the system". We don't currently really >>> model front-panel or watchdog reset properly, we assume >>> that it's close enough to have it be the same as >>> power-off-and-on reset (and there are some kludges in >>> various places where it's not quite right). >> >> Agreed, there are many flavors of resets, even different flavors of warm >> ones I'm afraid. >> >> To summarize, if a VM does an ordinary reset ("shutdown -r") we want to >> unplug all hotplugged memory. Same with most external resets we >> currently implement. In all other caes, we likely want to keep the >> memory hotplugged and RAM content alive. > > OK, if we want 'shutdown -r' to unplug hotplugged memory > then that sounds like we do want a WAKEUP reset type. > (Though I'm surprised that we want that -- my expectation > would have been that the hotplugged memory should stay, > because if you do real hardware hotplug of RAM then it > doesn't all pop back out of the sockets when you press > the reset button on the front panel :-)) :) The main difference to ordinary RAM is that (virtio-mem) device memory is always detected by a guest OS driver, it's never part of the bios/firmware-provided memory map. And not all OSes support virtio-mem. So one reason we added that handling in the beginning was if you would reboot from OS X to Y, whereby X suppors virtio-mem (and we hotplugged memory) and Y does not, then you would not leave that unusable-but-memory-consuming memory around for Y (possibly leaving customers paying for dynamic memory capacities angry). Another reason was to force the fresh OS to effectively defragment hotplugged device memory (so we have less mixture of plugged and unplugged memory blocks that any previous OS might have left behind after unplugging memory). Yet another reason was that one could have pending unplug requests (e.g., unplug 1 GiB, but only part of that request was fulfilled by the old OS), and this way we can easily enforce that once we get the chance to. So to summarize, the idea was to "start from scratch" in all cases where it is safe. For wakeup, it apparently wasn't safe :)
diff --git a/docs/devel/reset.rst b/docs/devel/reset.rst index 9746a4e8a0..30c9a0cc2b 100644 --- a/docs/devel/reset.rst +++ b/docs/devel/reset.rst @@ -44,6 +44,13 @@ The Resettable interface handles reset types with an enum ``ResetType``: value on each cold reset, such as RNG seed information, and which they must not reinitialize on a snapshot-load reset. +``RESET_TYPE_WAKEUP`` + This type is used when the machine is woken up from a suspended state (deep + sleep, suspend-to-ram). Devices that must not be reset to their initial state + after wake-up (for example virtio-mem) can use this state to differentiate + cold start from wake-up can use this state to differentiate cold start from + wake-up. + Devices which implement reset methods must treat any unknown ``ResetType`` as equivalent to ``RESET_TYPE_COLD``; this will reduce the amount of existing code we need to change if we add more types in future. diff --git a/hw/i386/pc.c b/hw/i386/pc.c index ccb9731c91..49efd0a997 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -1716,7 +1716,7 @@ static void pc_machine_reset(MachineState *machine, ResetType type) static void pc_machine_wakeup(MachineState *machine) { cpu_synchronize_all_states(); - pc_machine_reset(machine, RESET_TYPE_COLD); + pc_machine_reset(machine, RESET_TYPE_WAKEUP); cpu_synchronize_all_post_reset(); } diff --git a/include/hw/resettable.h b/include/hw/resettable.h index 7e249deb8b..edb1f1361b 100644 --- a/include/hw/resettable.h +++ b/include/hw/resettable.h @@ -29,6 +29,7 @@ typedef struct ResettableState ResettableState; * Types of reset. * * + Cold: reset resulting from a power cycle of the object. + * + Wakeup: reset resulting from a wake-up from a suspended state. * * TODO: Support has to be added to handle more types. In particular, * ResettableState structure needs to be expanded. @@ -36,6 +37,7 @@ typedef struct ResettableState ResettableState; typedef enum ResetType { RESET_TYPE_COLD, RESET_TYPE_SNAPSHOT_LOAD, + RESET_TYPE_WAKEUP, } ResetType; /*
Some devices need to distinguish cold start reset from waking up from a suspended state. This patch adds new value to the enum, and updates the i386 wakeup method to use this new reset type. Signed-off-by: Juraj Marcin <jmarcin@redhat.com> --- docs/devel/reset.rst | 7 +++++++ hw/i386/pc.c | 2 +- include/hw/resettable.h | 2 ++ 3 files changed, 10 insertions(+), 1 deletion(-)