[0/3] Split TYPE_GPIOPWR and reuse in E500 machines

Message

Bernhard Beschow Sept. 23, 2024, 10:15 a.m. UTC

This series is part of a bigger series exploring data-driven machine creation
using device tree blobs on top of the e500 machines [1]. The idea is to
instantiate a QEMU device model for each device tree node containing a
compatible property. [1] achieves feature-parity with the hardcoded machines
when supplied the same device tree blob that the hardcoded machine would
generate.

Just like the ARM virt machine, the ppce500 machine implements a
"gpio-poweroff"-compatible device tree node. Unfortunately, the implementation
isn't shared which this series fixes. In order to reflect device tree which has
separate bindings for gpio-poweroff and gpio-reset, and to prepare for the
above, the gpio-pwr device model is split.

Note: If the split seems too fine-grained, the existing gpio-pwr device model
could probably be reused in ppce500, too.

Testing done:
* Build qemu_ppc64_e5500_defconfig in Buildroot, run it in the ppce500 machine
  and issue the `poweroff` command. Observe that QEMU is shut down cleanly.
* ARM virt: How to test the secure path?

[1] https://github.com/shentok/qemu/tree/e500-fdt

Bernhard Beschow (3):
  MAINTAINERS: Add hw/gpio/gpio_pwr.c
  hw/gpio/gpio_pwr: Split into separate gpio_poweroff and gpio_restart
    devices
  hw/ppc/e500: Reuse TYPE_GPIO_POWEROFF

 MAINTAINERS             |  2 ++
 hw/arm/virt.c           | 32 +++++++++++++------
 hw/gpio/gpio_poweroff.c | 51 ++++++++++++++++++++++++++++++
 hw/gpio/gpio_pwr.c      | 70 -----------------------------------------
 hw/gpio/gpio_restart.c  | 51 ++++++++++++++++++++++++++++++
 hw/ppc/e500.c           | 15 ++-------
 hw/arm/Kconfig          |  3 +-
 hw/gpio/Kconfig         |  5 ++-
 hw/gpio/meson.build     |  3 +-
 hw/ppc/Kconfig          |  1 +
 10 files changed, 138 insertions(+), 95 deletions(-)
 create mode 100644 hw/gpio/gpio_poweroff.c
 delete mode 100644 hw/gpio/gpio_pwr.c
 create mode 100644 hw/gpio/gpio_restart.c

Comments

Peter Maydell Sept. 30, 2024, 12:57 p.m. UTC | #1

On Mon, 23 Sept 2024 at 11:16, Bernhard Beschow <[e-mail]> wrote:
>
> This series is part of a bigger series exploring data-driven machine creation
> using device tree blobs on top of the e500 machines [1]. The idea is to
> instantiate a QEMU device model for each device tree node containing a
> compatible property. [1] achieves feature-parity with the hardcoded machines
> when supplied the same device tree blob that the hardcoded machine would
> generate.

FWIW, on Arm I have generally pushed back against the idea
of "create a QEMU machine from a device tree", because the
device tree does not in general contain sufficient information
to create a QEMU machine. It only has the information that
Linux needs to use the devices, which is a subset of the
total "how do all these devices exist and get wired together".
(There are some special cases like some Xilinx FPGA systems,
where both the device tree and the hardware were autogenerated
from a common source definition, and so there's enough of
a constraint on what the hardware can be to make it workable.)

> Just like the ARM virt machine, the ppce500 machine implements a
> "gpio-poweroff"-compatible device tree node. Unfortunately, the implementation
> isn't shared which this series fixes. In order to reflect device tree which has
> separate bindings for gpio-poweroff and gpio-reset, and to prepare for the
> above, the gpio-pwr device model is split.
>
> Note: If the split seems too fine-grained, the existing gpio-pwr device model
> could probably be reused in ppce500, too.

I definitely like getting rid of the hand-coded
qemu_allocate_irq() in e500.c. But I don't really see
the benefit of splitting gpio_pwr into two devices.
If you only need the power-off behaviour, you can
leave the restart gpio input not connected to anything.

thanks
-- PMM

Bernhard Beschow Oct. 5, 2024, 10:11 a.m. UTC | #2

Am 30. September 2024 12:57:17 UTC schrieb Peter Maydell <[e-mail]>:
>On Mon, 23 Sept 2024 at 11:16, Bernhard Beschow <[e-mail]> wrote:
>>
>> This series is part of a bigger series exploring data-driven machine creation
>> using device tree blobs on top of the e500 machines [1]. The idea is to
>> instantiate a QEMU device model for each device tree node containing a
>> compatible property. [1] achieves feature-parity with the hardcoded machines
>> when supplied the same device tree blob that the hardcoded machine would
>> generate.
>
>FWIW, on Arm I have generally pushed back against the idea
>of "create a QEMU machine from a device tree", because the
>device tree does not in general contain sufficient information
>to create a QEMU machine. It only has the information that
>Linux needs to use the devices, which is a subset of the
>total "how do all these devices exist and get wired together".
>(There are some special cases like some Xilinx FPGA systems,
>where both the device tree and the hardware were autogenerated
>from a common source definition, and so there's enough of
>a constraint on what the hardware can be to make it workable.)
>
>> Just like the ARM virt machine, the ppce500 machine implements a
>> "gpio-poweroff"-compatible device tree node. Unfortunately, the implementation
>> isn't shared which this series fixes. In order to reflect device tree which has
>> separate bindings for gpio-poweroff and gpio-reset, and to prepare for the
>> above, the gpio-pwr device model is split.
>>
>> Note: If the split seems too fine-grained, the existing gpio-pwr device model
>> could probably be reused in ppce500, too.
>
>I definitely like getting rid of the hand-coded
>qemu_allocate_irq() in e500.c. But I don't really see
>the benefit of splitting gpio_pwr into two devices.
>If you only need the power-off behaviour, you can
>leave the restart gpio input not connected to anything.

New series sent which removes qemu_allocate_irq() usage in e500.c: https://lore.kernel.org/qemu-devel/[e-mail]/

Best regards,
Bernhard

>
>thanks
>-- PMM