This series fixes a bug reported by Maxim where the virt board's
functionality intended to allow a guest in the Secure world to
shutdown or reset the system was broken. Patch 1 from Maxim (seen
already on-list) fixes a silly cut-n-paste error in the gpio-pwr
device. The rest of the series fixes the trickier part of the bug.
The bug is caused because the gpio-pwr device and the virt board
wiring assume that the GPIO outputs used to trigger shutdown and reset
are active-high. However, for the PL061 GPIO lines default to inputs,
and our PL061 implementation currently hardcodes "assume lines have
pullup resistors so that when configured for input they act like a
logical 1 output". The only reason this doesn't mean that we
immediately shutdown/reset on startup is a second PL061 bug where we
don't assert the GPIO lines to the active-1-pullup behaviour on reset,
but instead do this the first time that pl061_active() is called,
which is typically when the guest touches the PL061. It happens that
OVMF doesn't do that until it decides it actually wants to do a
shutdown, so the bug was manifesting as "we got a reset instead of
shutdown".
I did an audit of our PL061 users in-tree:
* highbank, versatilepb create PL061 devices but don't connect any of
the GPIO inputs or outputs
* realview, sbsa_ref connect only inputs, never outputs
* virt has an NS PL061 which is used only for inputs, and an S PL061
which is used for outputs (and doesn't work correctly, as described
above)
* stellaris uses its PL061s for both inputs and outputs; the output
handling is for the SSI device chip selects (and is not really
correctly modelling what the hardware does). These PL061s are the
"Luminary" variant.
The Realview and Versatile boards really do want pull-up behaviour (as
documented in their TRMs). For 'virt' and 'sbsa_ref' we can define
whatever behaviour we like; 'virt' needs pull-down unless we want to
redesign the gpio-pwr device, as noted above. The Luminary PL061s in
the stellaris board have extra registers not provided in the stock
PL061 which let the guest control whether to enable pullup or pulldown
or to leave the line truly floating. I don't know what highbank
hardware needs, but since it doesn't connect any inputs or outputs it
doesn't really matter.
These patches make the implementation honour the Luminary PL061 PUR
and PDR registers, and add QOM properties so that boards using the
stock PL061 can configure whether the lines are pullup, pulldown, or
floating. We default to pullup to retain existing behaviour, and make
the virt board explicitly configure the pulldown it wants. Once that
is all in place, we can make the PL061 assert the GPIO lines to the
pullup/pulldown state on reset (by converting it to 3-phase reset).
A few patches early in the series do some cleanup of the PL061:
converting it to tracepoints, some minor refactoring, and a
documentation patch. The last two patches are comments only, and
document a couple of issues with our stellaris board model which I
noticed while I was making sure I hadn't broken it with my PL061
changes. Given that the stellaris board is old and not very useful
these days, I don't want to put in a lot of effort trying to "fix"
things which aren't causing any issues with guest images we know
about, but I did want to record what I'd figured out from various data
sheets so it doesn't get forgotten...
thanks
-- PMM
Maxim Uvarov (1):
hw/gpio/gpio_pwr: use shutdown function for reboot
Peter Maydell (10):
hw/gpio/pl061: Convert DPRINTF to tracepoints
hw/gpio/pl061: Clean up read/write offset handling logic
hw/gpio/pl061: Add tracepoints for register read and write
hw/gpio/pl061: Document the interface of this device
hw/gpio/pl061: Honour Luminary PL061 PUR and PDR registers
hw/gpio/pl061: Make pullup/pulldown of outputs configurable
hw/arm/virt: Make PL061 GPIO lines pulled low, not high
hw/gpio/pl061: Convert to 3-phase reset and assert GPIO lines
correctly on reset
hw/gpio/pl061: Document a shortcoming in our implementation
hw/arm/stellaris: Expand comment about handling of OLED chipselect
hw/arm/stellaris.c | 56 ++++++-
hw/arm/virt.c | 3 +
hw/gpio/gpio_pwr.c | 2 +-
hw/gpio/pl061.c | 343 ++++++++++++++++++++++++++++++++++---------
hw/gpio/trace-events | 9 ++
5 files changed, 341 insertions(+), 72 deletions(-)
--
2.20.1
