diff mbox series

[PULL,v2,1/3] target/hppa: Fix OS reboot issues

Message ID 20230624115007.30332-2-deller@gmx.de (mailing list archive)
State New, archived
Headers show
Series [PULL,v2,1/3] target/hppa: Fix OS reboot issues | expand

Commit Message

Helge Deller June 24, 2023, 11:50 a.m. UTC
When the OS triggers a reboot, the reset helper function sends a
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET) together with an
EXCP_HLT exception to halt the CPUs.

So, at reboot when initializing the CPUs again, make sure to set all
instruction pointers to the firmware entry point, disable any interrupts,
disable data and instruction translations, enable PSW_Q bit  and tell qemu
to unhalt (halted=0) the CPUs again.

This fixes the various reboot issues which were seen when rebooting a
Linux VM, including the case where even the monarch CPU has been virtually
halted from the OS (e.g. via "chcpu -d 0" inside the Linux VM).

Signed-off-by: Helge Deller <deller@gmx.de>
---
 hw/hppa/machine.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

--
2.38.1

Comments

Michael Tokarev June 25, 2023, 8:57 a.m. UTC | #1
24.06.2023 14:50, Helge Deller пишет:
> When the OS triggers a reboot, the reset helper function sends a
> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET) together with an
> EXCP_HLT exception to halt the CPUs.
> 
> So, at reboot when initializing the CPUs again, make sure to set all
> instruction pointers to the firmware entry point, disable any interrupts,
> disable data and instruction translations, enable PSW_Q bit  and tell qemu
> to unhalt (halted=0) the CPUs again.
> 
> This fixes the various reboot issues which were seen when rebooting a
> Linux VM, including the case where even the monarch CPU has been virtually
> halted from the OS (e.g. via "chcpu -d 0" inside the Linux VM).

Is this a -stable material?  It applies cleanly to 8.0 and 7.2.

Thanks,

/mjt
Helge Deller June 25, 2023, 10:07 a.m. UTC | #2
On 6/25/23 10:57, Michael Tokarev wrote:
> 24.06.2023 14:50, Helge Deller пишет:
>> When the OS triggers a reboot, the reset helper function sends a
>> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET) together with an
>> EXCP_HLT exception to halt the CPUs.
>>
>> So, at reboot when initializing the CPUs again, make sure to set all
>> instruction pointers to the firmware entry point, disable any interrupts,
>> disable data and instruction translations, enable PSW_Q bit  and tell qemu
>> to unhalt (halted=0) the CPUs again.
>>
>> This fixes the various reboot issues which were seen when rebooting a
>> Linux VM, including the case where even the monarch CPU has been virtually
>> halted from the OS (e.g. via "chcpu -d 0" inside the Linux VM).
>
> Is this a -stable material?  It applies cleanly to 8.0 and 7.2.

Yes, please.
At least for 8.0 I think it should be added.
I didn't tested 7.2, but can do and would prefer it if could be added there too.

Thanks!
Helge
Helge Deller June 25, 2023, 11:20 a.m. UTC | #3
On 6/25/23 12:07, Helge Deller wrote:
> On 6/25/23 10:57, Michael Tokarev wrote:
>> 24.06.2023 14:50, Helge Deller пишет:
>>> When the OS triggers a reboot, the reset helper function sends a
>>> qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET) together with an
>>> EXCP_HLT exception to halt the CPUs.
>>>
>>> So, at reboot when initializing the CPUs again, make sure to set all
>>> instruction pointers to the firmware entry point, disable any interrupts,
>>> disable data and instruction translations, enable PSW_Q bit  and tell qemu
>>> to unhalt (halted=0) the CPUs again.
>>>
>>> This fixes the various reboot issues which were seen when rebooting a
>>> Linux VM, including the case where even the monarch CPU has been virtually
>>> halted from the OS (e.g. via "chcpu -d 0" inside the Linux VM).
>>
>> Is this a -stable material?  It applies cleanly to 8.0 and 7.2.
>
> Yes, please.
> At least for 8.0 I think it should be added.
> I didn't tested 7.2, but can do and would prefer it if could be added there too.

Just tested: The patches work nicely when applied to v7.2 as well.
Could you add them to -stable (or what is the process)?

Thanks,
Helge
Michael Tokarev June 26, 2023, 12:22 p.m. UTC | #4
25.06.2023 14:20, Helge Deller wrote:

>>> Is this a -stable material?  It applies cleanly to 8.0 and 7.2.
>>
>> Yes, please.
>> At least for 8.0 I think it should be added.
>> I didn't tested 7.2, but can do and would prefer it if could be added there too.
> 
> Just tested: The patches work nicely when applied to v7.2 as well.
> Could you add them to -stable (or what is the process)?

Hmm. Now I'm a bit confused.

Initially I thought it's just this patch, 1/3, "target/hppa: Fix OS reboot issues",
to which I replied, is the one which should perhaps go to -stable.  But now I see
it is the whole series, all 3 changes, which are needed.

And for 7.2, things are a bit more interesting in this context: seabios-hppa version
in 7.2 is 6, it changed to version 7 in qemu 8.0, and changed further by this patchset.
There isn't much differences between seabios-hppa 6 and 7, so it's probably okay to
have it of version 7 in qemu-7.2-stable too (esp. since the change in there is another
bugfix, with debian 12 as a reproducer).

So, just to confirm: do we update seabios-hppa to this commit 673d2595d4f773cc266cbf
(version 8) in both stable-7.2 and stable-8.0? :)

The changes in there seems to be okay anyway, should be good to have in -stable.

Besides, I'm having sort of difficult time cherry-picking the commit which updates
seabios-hppa submodule and hppa-firmware.img file, - git tells me there's a conflict
(even when applying to stable-8.0 branch) but I see no changes to this areas in-
between (yes, a conflict when applying to 7.2 is expected).  So I ended up in
doing:

staging-8.0$ git cherry-pick -xs 34ec3aea54368a92b62a55c656335885ba8c65ef

error: Could not read 20cf9c785c3aa493a39be532642883a7a1dc360c
warning: Cannot merge binary files: pc-bios/hppa-firmware.img (HEAD vs. 34ec3aea54 (target/hppa: Update to SeaBIOS-hppa version 8))
Auto-merging pc-bios/hppa-firmware.img
CONFLICT (content): Merge conflict in pc-bios/hppa-firmware.img
Failed to merge submodule roms/seabios-hppa (commits don't follow merge-base)
CONFLICT (submodule): Merge conflict in roms/seabios-hppa
Recursive merging with submodules currently only supports trivial cases.
Please manually handle the merging of each conflicted submodule.
This can be accomplished with the following steps:
  - go to submodule (roms/seabios-hppa), and either merge commit 673d2595
    or update to an existing commit which has merged those changes
  - come back to superproject and run:

       git add roms/seabios-hppa

    to record the above merge or update
  - resolve any other conflicts in the superproject
  - commit the resulting index in the superproject
error: could not apply 34ec3aea54... target/hppa: Update to SeaBIOS-hppa version 8
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

staging-8.0$ git checkout 34ec3aea54368a92b62a55c656335885ba8c65ef roms/seabios-hppa pc-bios/hppa-firmware.img

Updated 1 path from dc78364a05

staging-8.0$ git status

On branch staging-8.0
You are currently cherry-picking commit 34ec3aea54.
   (all conflicts fixed: run "git cherry-pick --continue")
   (use "git cherry-pick --skip" to skip this patch)
   (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   pc-bios/hppa-firmware.img
	modified:   roms/seabios-hppa

staging-8.0$ git cherry-pick --continue
(commit changes)

Yes, this moves us to the right contents of both the submodule and the binary
file (the same as on master), but I don't see why it goes the way it goes.


Besides, it looks like this is the first change to submodules within qemu-stable
series..


Thanks,

/mjt
Helge Deller June 26, 2023, 4:20 p.m. UTC | #5
On 6/26/23 14:22, Michael Tokarev wrote:
> 25.06.2023 14:20, Helge Deller wrote:
>
>>>> Is this a -stable material?  It applies cleanly to 8.0 and 7.2.
>>>
>>> Yes, please.
>>> At least for 8.0 I think it should be added.
>>> I didn't tested 7.2, but can do and would prefer it if could be added there too.
>>
>> Just tested: The patches work nicely when applied to v7.2 as well.
>> Could you add them to -stable (or what is the process)?
>
> Hmm. Now I'm a bit confused.
>
> Initially I thought it's just this patch, 1/3, "target/hppa: Fix OS reboot issues",
> to which I replied, is the one which should perhaps go to -stable.  But now I see
> it is the whole series, all 3 changes, which are needed.
>
> And for 7.2, things are a bit more interesting in this context: seabios-hppa version
> in 7.2 is 6, it changed to version 7 in qemu 8.0, and changed further by this patchset.
> There isn't much differences between seabios-hppa 6 and 7, so it's probably okay to
> have it of version 7 in qemu-7.2-stable too (esp. since the change in there is another
> bugfix, with debian 12 as a reproducer).
>
> So, just to confirm: do we update seabios-hppa to this commit 673d2595d4f773cc266cbf
> (version 8) in both stable-7.2 and stable-8.0? :)

Yes.

> The changes in there seems to be okay anyway, should be good to have in -stable.

Yes.

> Besides, I'm having sort of difficult time cherry-picking the commit which updates
> seabios-hppa submodule and hppa-firmware.img file, - git tells me there's a conflict
> (even when applying to stable-8.0 branch) but I see no changes to this areas in-
> between (yes, a conflict when applying to 7.2 is expected).  So I ended up in
> doing:

starting in both branches (staging-7.2 and staging-8.0) this works for me:

git cherry-pick bb9c998ca9343d445c76b69fa15dea9db692f526
git cherry-pick 50ba97e928b44ff5bc731c9ffe68d86acbe44639
git cherry-pick 069d296669448b9eef72c6332ae84af962d9582c
git cherry-pick 34ec3aea54368a92b62a55c656335885ba8c65ef

Helge
Michael Tokarev June 26, 2023, 4:33 p.m. UTC | #6
26.06.2023 19:20, Helge Deller wrote:
..
> starting in both branches (staging-7.2 and staging-8.0) this works for me:
> 
> git cherry-pick bb9c998ca9343d445c76b69fa15dea9db692f526
> git cherry-pick 50ba97e928b44ff5bc731c9ffe68d86acbe44639
> git cherry-pick 069d296669448b9eef72c6332ae84af962d9582c
> git cherry-pick 34ec3aea54368a92b62a55c656335885ba8c65ef

Got it. Somehow I missed bb9c998ca9343d445c76b69fa15dea9db692f526.
It all works fine now.

Thank you!

/mjt
diff mbox series

Patch

diff --git a/hw/hppa/machine.c b/hw/hppa/machine.c
index b00a91ecfe..9facef7f14 100644
--- a/hw/hppa/machine.c
+++ b/hw/hppa/machine.c
@@ -418,10 +418,16 @@  static void hppa_machine_reset(MachineState *ms, ShutdownCause reason)

     /* Start all CPUs at the firmware entry point.
      *  Monarch CPU will initialize firmware, secondary CPUs
-     *  will enter a small idle look and wait for rendevouz. */
+     *  will enter a small idle loop and wait for rendevouz. */
     for (i = 0; i < smp_cpus; i++) {
-        cpu_set_pc(CPU(cpu[i]), firmware_entry);
+        CPUState *cs = CPU(cpu[i]);
+
+        cpu_set_pc(cs, firmware_entry);
+        cpu[i]->env.psw = PSW_Q;
         cpu[i]->env.gr[5] = CPU_HPA + i * 0x1000;
+
+        cs->exception_index = -1;
+        cs->halted = 0;
     }

     /* already initialized by machine_hppa_init()? */