[PATCH RFC 0/3] hw/misc/pvpanic: add support for normal shutdowns

[PATCH RFC 0/3] hw/misc/pvpanic: add support for normal shutdowns

[Thomas Weißschuh]

Shutdown requests are normally hardware dependent.
By extending pvpanic to also handle shutdown requests, guests can
submit such requests with an easily implementable and cross-platform
mechanism.

The background is the usage of minimal Linux kernels with different
architectures for testing purposes.
Poweroff support varies highly per architecture and requires a bunch of
code to be compiled to work.
pvpanic on the other hand is very small and uniform.

I sent an RFC[0] for this before to qemu-devel and lkml which didn't
generate feedback, so let's discuss the concrete proposal.

Patch 1 and 2 are general cleanups, that seems useful even without this
proposal being implemented.

I'll send the corresponding Linux patch to LKML.

[0] https://lore.kernel.org/all/984794aa-4af0-4c68-a74e-7420ec3151a5@t-8ch.de/

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
---
Thomas Weißschuh (3):
      hw/misc/pvpanic: centralize definition of supported events
      tests/qtest/pvanic: use centralized definition of supported events
      hw/misc/pvpanic: add support for normal shutdowns

 docs/specs/pvpanic.rst                   | 2 ++
 hw/misc/pvpanic-isa.c                    | 3 +--
 hw/misc/pvpanic-pci.c                    | 3 +--
 hw/misc/pvpanic.c                        | 8 ++++++--
 include/hw/misc/pvpanic.h                | 2 ++
 include/standard-headers/linux/pvpanic.h | 1 +
 tests/qtest/pvpanic-pci-test.c           | 5 +++--
 tests/qtest/pvpanic-test.c               | 5 +++--
 8 files changed, 19 insertions(+), 10 deletions(-)
---
base-commit: d762bf97931b58839316b68a570eecc6143c9e3e
change-id: 20231104-pvpanic-shutdown-02e4b4cb4949

Best regards,
-- 
Thomas Weißschuh <thomas@t-8ch.de>

[PATCH RFC 1/3] hw/misc/pvpanic: centralize definition of supported events

[Thomas Weißschuh]

The different components of pvpanic duplicate the list of supported
events. Move it to the shared header file to minimize changes when new
events are added.

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
---
 hw/misc/pvpanic-isa.c     | 3 +--
 hw/misc/pvpanic-pci.c     | 3 +--
 hw/misc/pvpanic.c         | 3 +--
 include/hw/misc/pvpanic.h | 2 ++
 4 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/misc/pvpanic-isa.c b/hw/misc/pvpanic-isa.c
index ccec50f61bbd..9a923b786907 100644
--- a/hw/misc/pvpanic-isa.c
+++ b/hw/misc/pvpanic-isa.c
@@ -21,7 +21,6 @@
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
 #include "hw/isa/isa.h"
-#include "standard-headers/linux/pvpanic.h"
 #include "hw/acpi/acpi_aml_interface.h"
 
 OBJECT_DECLARE_SIMPLE_TYPE(PVPanicISAState, PVPANIC_ISA_DEVICE)
@@ -102,7 +101,7 @@ static void build_pvpanic_isa_aml(AcpiDevAmlIf *adev, Aml *scope)
 static Property pvpanic_isa_properties[] = {
     DEFINE_PROP_UINT16(PVPANIC_IOPORT_PROP, PVPanicISAState, ioport, 0x505),
     DEFINE_PROP_UINT8("events", PVPanicISAState, pvpanic.events,
-                      PVPANIC_PANICKED | PVPANIC_CRASH_LOADED),
+                      PVPANIC_EVENTS),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/misc/pvpanic-pci.c b/hw/misc/pvpanic-pci.c
index fbcaa50731b3..8898d280d2ef 100644
--- a/hw/misc/pvpanic-pci.c
+++ b/hw/misc/pvpanic-pci.c
@@ -21,7 +21,6 @@
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
 #include "hw/pci/pci_device.h"
-#include "standard-headers/linux/pvpanic.h"
 
 OBJECT_DECLARE_SIMPLE_TYPE(PVPanicPCIState, PVPANIC_PCI_DEVICE)
 
@@ -55,7 +54,7 @@ static void pvpanic_pci_realizefn(PCIDevice *dev, Error **errp)
 
 static Property pvpanic_pci_properties[] = {
     DEFINE_PROP_UINT8("events", PVPanicPCIState, pvpanic.events,
-                      PVPANIC_PANICKED | PVPANIC_CRASH_LOADED),
+                      PVPANIC_EVENTS),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index 1540e9091a45..a4982cc5928e 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -21,13 +21,12 @@
 #include "hw/qdev-properties.h"
 #include "hw/misc/pvpanic.h"
 #include "qom/object.h"
-#include "standard-headers/linux/pvpanic.h"
 
 static void handle_event(int event)
 {
     static bool logged;
 
-    if (event & ~(PVPANIC_PANICKED | PVPANIC_CRASH_LOADED) && !logged) {
+    if (event & ~PVPANIC_EVENTS && !logged) {
         qemu_log_mask(LOG_GUEST_ERROR, "pvpanic: unknown event %#x.\n", event);
         logged = true;
     }
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index fab94165d03d..198047dc86ff 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -17,11 +17,13 @@
 
 #include "exec/memory.h"
 #include "qom/object.h"
+#include "standard-headers/linux/pvpanic.h"
 
 #define TYPE_PVPANIC_ISA_DEVICE "pvpanic"
 #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci"
 
 #define PVPANIC_IOPORT_PROP "ioport"
+#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED)
 
 /*
  * PVPanicState for any device type

-- 
2.42.0

[PATCH RFC 2/3] tests/qtest/pvanic: use centralized definition of supported events

[Thomas Weißschuh]

Avoid the necessity to update all tests when new events are added
to the device.

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
---
 tests/qtest/pvpanic-pci-test.c | 5 +++--
 tests/qtest/pvpanic-test.c     | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/pvpanic-pci-test.c b/tests/qtest/pvpanic-pci-test.c
index 2c05b376ba72..b372caf41dc0 100644
--- a/tests/qtest/pvpanic-pci-test.c
+++ b/tests/qtest/pvpanic-pci-test.c
@@ -16,6 +16,7 @@
 #include "qapi/qmp/qdict.h"
 #include "libqos/pci.h"
 #include "libqos/pci-pc.h"
+#include "hw/misc/pvpanic.h"
 #include "hw/pci/pci_regs.h"
 
 static void test_panic_nopause(void)
@@ -34,7 +35,7 @@ static void test_panic_nopause(void)
     bar = qpci_iomap(dev, 0, NULL);
 
     qpci_memread(dev, bar, 0, &val, sizeof(val));
-    g_assert_cmpuint(val, ==, 3);
+    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
     val = 1;
     qpci_memwrite(dev, bar, 0, &val, sizeof(val));
@@ -67,7 +68,7 @@ static void test_panic(void)
     bar = qpci_iomap(dev, 0, NULL);
 
     qpci_memread(dev, bar, 0, &val, sizeof(val));
-    g_assert_cmpuint(val, ==, 3);
+    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
     val = 1;
     qpci_memwrite(dev, bar, 0, &val, sizeof(val));
diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
index 78f1cf8186b0..ccc603472f5d 100644
--- a/tests/qtest/pvpanic-test.c
+++ b/tests/qtest/pvpanic-test.c
@@ -10,6 +10,7 @@
 #include "qemu/osdep.h"
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
+#include "hw/misc/pvpanic.h"
 
 static void test_panic_nopause(void)
 {
@@ -20,7 +21,7 @@ static void test_panic_nopause(void)
     qts = qtest_init("-device pvpanic -action panic=none");
 
     val = qtest_inb(qts, 0x505);
-    g_assert_cmpuint(val, ==, 3);
+    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
     qtest_outb(qts, 0x505, 0x1);
 
@@ -43,7 +44,7 @@ static void test_panic(void)
     qts = qtest_init("-device pvpanic -action panic=pause");
 
     val = qtest_inb(qts, 0x505);
-    g_assert_cmpuint(val, ==, 3);
+    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
 
     qtest_outb(qts, 0x505, 0x1);
 

-- 
2.42.0

[PATCH RFC 3/3] hw/misc/pvpanic: add support for normal shutdowns

[Thomas Weißschuh]

Shutdown requests are normally hardware dependent.
By extending pvpanic to also handle shutdown requests, guests can
submit such requests with an easily implementable and cross-platform
mechanism.

Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
---
 docs/specs/pvpanic.rst                   | 2 ++
 hw/misc/pvpanic.c                        | 5 +++++
 include/hw/misc/pvpanic.h                | 2 +-
 include/standard-headers/linux/pvpanic.h | 1 +
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/docs/specs/pvpanic.rst b/docs/specs/pvpanic.rst
index f894bc19555f..796cc0348a38 100644
--- a/docs/specs/pvpanic.rst
+++ b/docs/specs/pvpanic.rst
@@ -29,6 +29,8 @@ bit 1
   a guest panic has happened and will be handled by the guest;
   the host should record it or report it, but should not affect
   the execution of the guest.
+bit 2
+  a guest shutdown has happened and should be processed by the host
 
 PCI Interface
 -------------
diff --git a/hw/misc/pvpanic.c b/hw/misc/pvpanic.c
index a4982cc5928e..246f9ae4e992 100644
--- a/hw/misc/pvpanic.c
+++ b/hw/misc/pvpanic.c
@@ -40,6 +40,11 @@ static void handle_event(int event)
         qemu_system_guest_crashloaded(NULL);
         return;
     }
+
+    if (event & PVPANIC_SHUTDOWN) {
+        qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN);
+        return;
+    }
 }
 
 /* return supported events on read */
diff --git a/include/hw/misc/pvpanic.h b/include/hw/misc/pvpanic.h
index 198047dc86ff..fa76ad93d998 100644
--- a/include/hw/misc/pvpanic.h
+++ b/include/hw/misc/pvpanic.h
@@ -23,7 +23,7 @@
 #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci"
 
 #define PVPANIC_IOPORT_PROP "ioport"
-#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED)
+#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED | PVPANIC_SHUTDOWN)
 
 /*
  * PVPanicState for any device type
diff --git a/include/standard-headers/linux/pvpanic.h b/include/standard-headers/linux/pvpanic.h
index 54b7485390d3..38e53ad45929 100644
--- a/include/standard-headers/linux/pvpanic.h
+++ b/include/standard-headers/linux/pvpanic.h
@@ -5,5 +5,6 @@
 
 #define PVPANIC_PANICKED	(1 << 0)
 #define PVPANIC_CRASH_LOADED	(1 << 1)
+#define PVPANIC_SHUTDOWN       	(1 << 2)
 
 #endif /* __PVPANIC_H__ */

-- 
2.42.0

Re: [PATCH RFC 2/3] tests/qtest/pvanic: use centralized definition of supported events

[Thomas Huth]

On 04/11/2023 12.25, Thomas Weißschuh wrote:
> Avoid the necessity to update all tests when new events are added
> to the device.
> 
> Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>
> ---
>   tests/qtest/pvpanic-pci-test.c | 5 +++--
>   tests/qtest/pvpanic-test.c     | 5 +++--
>   2 files changed, 6 insertions(+), 4 deletions(-)

FWIW:
Acked-by: Thomas Huth <thuth@redhat.com>

Re: [PATCH RFC 2/3] tests/qtest/pvanic: use centralized definition of supported events

[Marc-André Lureau]

On Sat, Nov 4, 2023 at 3:26 PM Thomas Weißschuh <thomas@t-8ch.de> wrote:
>
> Avoid the necessity to update all tests when new events are added
> to the device.
>
> Signed-off-by: Thomas Weißschuh <thomas@t-8ch.de>

In the title: pvanic -> pvpanic.

> ---
>  tests/qtest/pvpanic-pci-test.c | 5 +++--
>  tests/qtest/pvpanic-test.c     | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/tests/qtest/pvpanic-pci-test.c b/tests/qtest/pvpanic-pci-test.c
> index 2c05b376ba72..b372caf41dc0 100644
> --- a/tests/qtest/pvpanic-pci-test.c
> +++ b/tests/qtest/pvpanic-pci-test.c
> @@ -16,6 +16,7 @@
>  #include "qapi/qmp/qdict.h"
>  #include "libqos/pci.h"
>  #include "libqos/pci-pc.h"
> +#include "hw/misc/pvpanic.h"
>  #include "hw/pci/pci_regs.h"
>
>  static void test_panic_nopause(void)
> @@ -34,7 +35,7 @@ static void test_panic_nopause(void)
>      bar = qpci_iomap(dev, 0, NULL);
>
>      qpci_memread(dev, bar, 0, &val, sizeof(val));
> -    g_assert_cmpuint(val, ==, 3);
> +    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
>
>      val = 1;
>      qpci_memwrite(dev, bar, 0, &val, sizeof(val));
> @@ -67,7 +68,7 @@ static void test_panic(void)
>      bar = qpci_iomap(dev, 0, NULL);
>
>      qpci_memread(dev, bar, 0, &val, sizeof(val));
> -    g_assert_cmpuint(val, ==, 3);
> +    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
>
>      val = 1;
>      qpci_memwrite(dev, bar, 0, &val, sizeof(val));
> diff --git a/tests/qtest/pvpanic-test.c b/tests/qtest/pvpanic-test.c
> index 78f1cf8186b0..ccc603472f5d 100644
> --- a/tests/qtest/pvpanic-test.c
> +++ b/tests/qtest/pvpanic-test.c
> @@ -10,6 +10,7 @@
>  #include "qemu/osdep.h"
>  #include "libqtest.h"
>  #include "qapi/qmp/qdict.h"
> +#include "hw/misc/pvpanic.h"
>
>  static void test_panic_nopause(void)
>  {
> @@ -20,7 +21,7 @@ static void test_panic_nopause(void)
>      qts = qtest_init("-device pvpanic -action panic=none");
>
>      val = qtest_inb(qts, 0x505);
> -    g_assert_cmpuint(val, ==, 3);
> +    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
>
>      qtest_outb(qts, 0x505, 0x1);
>
> @@ -43,7 +44,7 @@ static void test_panic(void)
>      qts = qtest_init("-device pvpanic -action panic=pause");
>
>      val = qtest_inb(qts, 0x505);
> -    g_assert_cmpuint(val, ==, 3);
> +    g_assert_cmpuint(val, ==, PVPANIC_EVENTS);
>
>      qtest_outb(qts, 0x505, 0x1);
>
>
> --
> 2.42.0
>
>


-- 
Marc-André Lureau