ACPI controller cleanup

[PATCH 6/7] hw/acpi: Trace GPE access in all device models, not just PIIX4

Posted by Bernhard Beschow 2 years, 5 months ago

Signed-off-by: Bernhard Beschow <shentey@gmail.com>
---
 hw/acpi/core.c       | 5 +++++
 hw/acpi/piix4.c      | 3 ---
 hw/acpi/trace-events | 8 ++++----
 3 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 6da275c599..a33e410e69 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -32,6 +32,7 @@
 #include "qemu/module.h"
 #include "qemu/option.h"
 #include "sysemu/runstate.h"
+#include "trace.h"
 
 struct acpi_table_header {
     uint16_t _length;         /* our length, not actual part of the hdr */
@@ -686,6 +687,8 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
 {
     uint8_t *cur;
 
+    trace_acpi_gpe_ioport_writeb(addr, val);
+
     cur = acpi_gpe_ioport_get_ptr(ar, addr);
     if (addr < ar->gpe.len / 2) {
         /* GPE_STS */
@@ -709,6 +712,8 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
         val = *cur;
     }
 
+    trace_acpi_gpe_ioport_readb(addr, val);
+
     return val;
 }
 
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 836f9026b1..b65ddb8e44 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -45,7 +45,6 @@
 #include "hw/acpi/acpi_dev_interface.h"
 #include "migration/vmstate.h"
 #include "hw/core/cpu.h"
-#include "trace.h"
 #include "qom/object.h"
 
 #define GPE_BASE 0xafe0
@@ -510,7 +509,6 @@ static uint64_t gpe_readb(void *opaque, hwaddr addr, unsigned width)
     PIIX4PMState *s = opaque;
     uint32_t val = acpi_gpe_ioport_readb(&s->ar, addr);
 
-    trace_piix4_gpe_readb(addr, width, val);
     return val;
 }
 
@@ -519,7 +517,6 @@ static void gpe_writeb(void *opaque, hwaddr addr, uint64_t val,
 {
     PIIX4PMState *s = opaque;
 
-    trace_piix4_gpe_writeb(addr, width, val);
     acpi_gpe_ioport_writeb(&s->ar, addr, val);
     acpi_update_sci(&s->ar, s->irq);
 }
diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
index 78e0a8670e..159937ddb9 100644
--- a/hw/acpi/trace-events
+++ b/hw/acpi/trace-events
@@ -17,6 +17,10 @@ mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event"
 mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
 mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete failed"
 
+# core.c
+acpi_gpe_ioport_readb(uint32_t addr, uint8_t val) "addr: 0x%" PRIx32 " ==> 0x%" PRIx8
+acpi_gpe_ioport_writeb(uint32_t addr, uint8_t val) "addr: 0x%" PRIx32 " <== 0x%" PRIx8
+
 # cpu.c
 cpuhp_acpi_invalid_idx_selected(uint32_t idx) "0x%"PRIx32
 cpuhp_acpi_read_flags(uint32_t idx, uint8_t flags) "idx[0x%"PRIx32"] flags: 0x%"PRIx8
@@ -48,10 +52,6 @@ acpi_pci_sel_read(uint32_t val) "%" PRIu32
 acpi_pci_ej_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64
 acpi_pci_sel_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64
 
-# piix4.c
-piix4_gpe_readb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d ==> 0x%" PRIx64
-piix4_gpe_writeb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d <== 0x%" PRIx64
-
 # tco.c
 tco_timer_reload(int ticks, int msec) "ticks=%d (%d ms)"
 tco_timer_expired(int timeouts_no, bool strap, bool no_reboot) "timeouts_no=%d no_reboot=%d/%d"
-- 
2.39.1

Re: [PATCH 6/7] hw/acpi: Trace GPE access in all device models, not just PIIX4

Posted by Igor Mammedov 2 years, 5 months ago

On Sun, 22 Jan 2023 18:07:23 +0100
Bernhard Beschow <shentey@gmail.com> wrote:

> Signed-off-by: Bernhard Beschow <shentey@gmail.com>

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/acpi/core.c       | 5 +++++
>  hw/acpi/piix4.c      | 3 ---
>  hw/acpi/trace-events | 8 ++++----
>  3 files changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 6da275c599..a33e410e69 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -32,6 +32,7 @@
>  #include "qemu/module.h"
>  #include "qemu/option.h"
>  #include "sysemu/runstate.h"
> +#include "trace.h"
>  
>  struct acpi_table_header {
>      uint16_t _length;         /* our length, not actual part of the hdr */
> @@ -686,6 +687,8 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
>  {
>      uint8_t *cur;
>  
> +    trace_acpi_gpe_ioport_writeb(addr, val);
> +
>      cur = acpi_gpe_ioport_get_ptr(ar, addr);
>      if (addr < ar->gpe.len / 2) {
>          /* GPE_STS */
> @@ -709,6 +712,8 @@ uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
>          val = *cur;
>      }
>  
> +    trace_acpi_gpe_ioport_readb(addr, val);
> +
>      return val;
>  }
>  
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index 836f9026b1..b65ddb8e44 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -45,7 +45,6 @@
>  #include "hw/acpi/acpi_dev_interface.h"
>  #include "migration/vmstate.h"
>  #include "hw/core/cpu.h"
> -#include "trace.h"
>  #include "qom/object.h"
>  
>  #define GPE_BASE 0xafe0
> @@ -510,7 +509,6 @@ static uint64_t gpe_readb(void *opaque, hwaddr addr, unsigned width)
>      PIIX4PMState *s = opaque;
>      uint32_t val = acpi_gpe_ioport_readb(&s->ar, addr);
>  
> -    trace_piix4_gpe_readb(addr, width, val);
>      return val;
>  }
>  
> @@ -519,7 +517,6 @@ static void gpe_writeb(void *opaque, hwaddr addr, uint64_t val,
>  {
>      PIIX4PMState *s = opaque;
>  
> -    trace_piix4_gpe_writeb(addr, width, val);
>      acpi_gpe_ioport_writeb(&s->ar, addr, val);
>      acpi_update_sci(&s->ar, s->irq);
>  }
> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> index 78e0a8670e..159937ddb9 100644
> --- a/hw/acpi/trace-events
> +++ b/hw/acpi/trace-events
> @@ -17,6 +17,10 @@ mhp_acpi_clear_remove_evt(uint32_t slot) "slot[0x%"PRIx32"] clear remove event"
>  mhp_acpi_pc_dimm_deleted(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm deleted"
>  mhp_acpi_pc_dimm_delete_failed(uint32_t slot) "slot[0x%"PRIx32"] pc-dimm delete failed"
>  
> +# core.c
> +acpi_gpe_ioport_readb(uint32_t addr, uint8_t val) "addr: 0x%" PRIx32 " ==> 0x%" PRIx8
> +acpi_gpe_ioport_writeb(uint32_t addr, uint8_t val) "addr: 0x%" PRIx32 " <== 0x%" PRIx8
> +
>  # cpu.c
>  cpuhp_acpi_invalid_idx_selected(uint32_t idx) "0x%"PRIx32
>  cpuhp_acpi_read_flags(uint32_t idx, uint8_t flags) "idx[0x%"PRIx32"] flags: 0x%"PRIx8
> @@ -48,10 +52,6 @@ acpi_pci_sel_read(uint32_t val) "%" PRIu32
>  acpi_pci_ej_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64
>  acpi_pci_sel_write(uint64_t addr, uint64_t data) "0x%" PRIx64 " <== %" PRIu64
>  
> -# piix4.c
> -piix4_gpe_readb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d ==> 0x%" PRIx64
> -piix4_gpe_writeb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d <== 0x%" PRIx64
> -
>  # tco.c
>  tco_timer_reload(int ticks, int msec) "ticks=%d (%d ms)"
>  tco_timer_expired(int timeouts_no, bool strap, bool no_reboot) "timeouts_no=%d no_reboot=%d/%d"

Re: [PATCH 6/7] hw/acpi: Trace GPE access in all device models, not just PIIX4

Posted by Philippe Mathieu-Daudé 2 years, 5 months ago

On 22/1/23 18:07, Bernhard Beschow wrote:
> Signed-off-by: Bernhard Beschow <shentey@gmail.com>
> ---
>   hw/acpi/core.c       | 5 +++++
>   hw/acpi/piix4.c      | 3 ---
>   hw/acpi/trace-events | 8 ++++----
>   3 files changed, 9 insertions(+), 7 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

[PATCH 1/7] hw/acpi/{ich9, piix4}: Reuse existing attributes for QOM properties
[PATCH 2/7] hw/acpi/ich9: Remove unneeded assignments
[PATCH 3/7] hw/acpi/{ich9, piix4}: Resolve redundant io_base address attributes
[PATCH 4/7] hw/acpi/ich9: Use ICH9_PMIO_GPE0_STS just once
[PATCH 5/7] hw/acpi/piix4: Fix offset of GPE0 registers
[PATCH 6/7] hw/acpi: Trace GPE access in all device models, not just PIIX4
[PATCH 7/7] hw/acpi/core: Trace enable and status registers of GPE separately