Re: [PATCH v8 3/8] hw/misc/pvpanic: centralize definition of supported events
On 2024-06-21 06:26:19+, Michael S. Tsirkin wrote: > On Mon, May 27, 2024 at 08:27:49AM +0200, Thomas Weißschuh wrote: > > 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! Thomas
Re: [PATCH v8 3/8] hw/misc/pvpanic: centralize definition of supported events
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 > Reviewed-by: Cornelia Huck > Signed-off-by: Thomas Weißschuh > --- > 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
[PATCH 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 Reviewed-by: Cornelia Huck Signed-off-by: Thomas Weißschuh --- 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" 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