On 6/2/21 11:33 PM, Nolan wrote:
> On 5/31/21 4:23 AM, Philippe Mathieu-Daudé wrote:
>> Hi Nolan,
>>
>> Thanks for your patch!
>>
>> There is something odd with your email address, which apparently
>> became source...@sigbus.net instead of no...@sigbus.net.
>
> Ugh, oops. I was trying out sourcehut for this, and reflexively gave
> them a marker email. I'm pretty sure sourcehut won't sell my email
> address, so I'll just change it.
>
>> On 5/18/21 10:24 PM, ~nolanl wrote:
>>> From: Nolan Leake <no...@sigbus.net>
>>>
>>> This is just enough to make reboot and poweroff work.
>>
>> Please precise "for Linux kernels", since this doesn't work
>> with firmwares (ideally we should implement the FIRMWARE_NOTIFY_REBOOT
>> property - which Linux sends - to handle the machine reboot/halt
>> via the videocore firmware model).
>
> Thanks, good point re: this being tuned to what Linux (and u-boot) do.
> Poking around a bit, it looks like
> "trusted-firmware-a"/"arm-trusted-firmware" uses the same method, as do
> a couple of bare-metal/hobby OSes. Couldn't immediately figure out what
> FreeBSD does.
>
> I'm not sure I understand your point about FIRMWARE_NOTIFY_REBOOT, my
> understanding is that message is there to tell the GPU firmware that
> we're about to reboot, so it can do things like reload the PCIe USB
> chip's firmware. In my testing without the watchdog module loaded, my
> physical pi4 does not reboot, so it appears that sending
> FIRMWARE_NOTIFY_REBOOT is not enough on its own.
>From the ARM core point of view, once it sent the FIRMWARE_NOTIFY_REBOOT
message, it can't really power-off itself, it waits in a busy loop for
the VC to disable its power domain.
hw/misc/bcm2835_property.c is our model of the VC behavior. IMO this
should be where QEMU shuts down. How I'd model it is:
- ARM: sends FIRMWARE_NOTIFY_REBOOT and loops
- VC emulated via property: delays (200ms?) then calls
SHUTDOWN_CAUSE_GUEST_RESET.
(it helps to see hw/misc/bcm2835_property.c as an external component).
>>> qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
>>> + } else {
>>> + qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>>> + }
>>> + }
>>
>> Shouldn't we log the unsupported bits?
>
> I can add that, I didn't originally because the unsupported writes are
> expected.
I'd prefer we log them, even if unsupported, so in case something
behaves oddly we could look at those bits.
>>> +static void bcm2835_powermgt_reset(DeviceState *dev)
>>> +{
>>> + BCM2835PowerMgtState *s = BCM2835_POWERMGT(dev);
>>> +
>>> + s->rstc = 0x00000102;
>>> + s->rsts = 0x00001000;
>>> + s->wdog = 0x00000000;
>>
>> Where these bits come from?
>
> From my pi4. https://elinux.org/BCM2835_registers agrees (processed from
> Broadcom source code).
OK, so please add a comment referring to
https://elinux.org/BCM2835_registers#PM.
Looking forward for v2 :)
Phil.
