[v8,3/8] hw/misc/pvpanic: centralize definition of supported events

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.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
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 | 4 ++++
 4 files changed, 7 insertions(+), 6 deletions(-)

On Mon, May 27, 2024 at 08:27:49AM +0200, Thomas Weißschuh wrote:
> 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.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
> 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 | 4 ++++
>  4 files changed, 7 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 83be95d0d249..603c5c7600da 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"


This part is wrong. PVPANIC_PANICKED and PVPANIC_CRASH_LOADED
are still used in pvpanic.c directly, so we should
include standard-headers/linux/pvpanic.h to avoid depending
on which header includes which.

I fixed up the patch.


>  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..947468b81b1a 100644
> --- a/include/hw/misc/pvpanic.h
> +++ b/include/hw/misc/pvpanic.h
> @@ -18,6 +18,10 @@
>  #include "exec/memory.h"
>  #include "qom/object.h"
>  
> +#include "standard-headers/linux/pvpanic.h"
> +
> +#define PVPANIC_EVENTS (PVPANIC_PANICKED | PVPANIC_CRASH_LOADED)
> +
>  #define TYPE_PVPANIC_ISA_DEVICE "pvpanic"
>  #define TYPE_PVPANIC_PCI_DEVICE "pvpanic-pci"
>  
> 
> -- 
> 2.45.1

On 2024-06-21 06:26:19+0000, Michael S. Tsirkin wrote:
> On Mon, May 27, 2024 at 08:27:49AM +0200, Thomas Weißschuh wrote:

<snip>

> > 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"
> 
> 
> This part is wrong. PVPANIC_PANICKED and PVPANIC_CRASH_LOADED
> are still used in pvpanic.c directly, so we should
> include standard-headers/linux/pvpanic.h to avoid depending
> on which header includes which.

Ack.

> I fixed up the patch.

Thanks!

<snip>


Thomas