diff mbox series

[v5,19/20] hw/isa: derive CPUState from MemTxAttrs in apm_ioport_writeb

Message ID 20221111182535.64844-20-alex.bennee@linaro.org
State New
Headers show
Series use MemTxAttrs to avoid current_cpu in hw/ | expand

Commit Message

Alex Bennée Nov. 11, 2022, 6:25 p.m. UTC 
Some of the callbacks need a CPUState so extend the interface so we
can pass that down rather than relying on current_cpu hacks.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 include/hw/isa/apm.h |  2 +-
 hw/acpi/ich9.c       |  1 -
 hw/acpi/piix4.c      |  2 +-
 hw/isa/apm.c         | 21 +++++++++++++++++----
 hw/isa/lpc_ich9.c    |  5 ++---
 5 files changed, 21 insertions(+), 10 deletions(-)

Comments

Richard Henderson Nov. 12, 2022, 6:04 a.m. UTC | #1 
On 11/12/22 04:25, Alex Bennée wrote:
> Some of the callbacks need a CPUState so extend the interface so we
> can pass that down rather than relying on current_cpu hacks.
> 
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> ---
>   include/hw/isa/apm.h |  2 +-
>   hw/acpi/ich9.c       |  1 -
>   hw/acpi/piix4.c      |  2 +-
>   hw/isa/apm.c         | 21 +++++++++++++++++----
>   hw/isa/lpc_ich9.c    |  5 ++---
>   5 files changed, 21 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Philippe Mathieu-Daudé Nov. 13, 2022, 8:04 p.m. UTC | #2 
On 11/11/22 19:25, Alex Bennée wrote:
> Some of the callbacks need a CPUState so extend the interface so we
> can pass that down rather than relying on current_cpu hacks.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   include/hw/isa/apm.h |  2 +-
>   hw/acpi/ich9.c       |  1 -
>   hw/acpi/piix4.c      |  2 +-
>   hw/isa/apm.c         | 21 +++++++++++++++++----
>   hw/isa/lpc_ich9.c    |  5 ++---
>   5 files changed, 21 insertions(+), 10 deletions(-)


> -static void apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
> -                              unsigned size)
> +static MemTxResult apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
> +                                     unsigned size, MemTxAttrs attrs)
>   {
>       APMState *apm = opaque;
> +    CPUState *cs;
> +
> +    if (attrs.requester_type != MTRT_CPU) {
> +        qemu_log_mask(LOG_UNIMP | LOG_GUEST_ERROR,
> +                      "%s: saw non-CPU transaction", __func__);
> +        return MEMTX_ACCESS_ERROR;

Are you sure it is illegal?

> +    }
> +    cs = qemu_get_cpu(attrs.requester_id);
> +
>       addr &= 1;
>   
>       trace_apm_io_write(addr, val);
> @@ -41,11 +52,13 @@ static void apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
>           apm->apmc = val;
>   
>           if (apm->callback) {
> -            (apm->callback)(val, apm->arg);
> +            (apm->callback)(cs, val, apm->arg);
>           }
>       } else {
>           apm->apms = val;
>       }
> +
> +    return MEMTX_OK;
>   }
diff mbox series

Patch

diff --git a/include/hw/isa/apm.h b/include/hw/isa/apm.h
index b6e070c00e..eb952e1c1c 100644
--- a/include/hw/isa/apm.h
+++ b/include/hw/isa/apm.h
@@ -6,7 +6,7 @@ 
 #define APM_CNT_IOPORT  0xb2
 #define ACPI_PORT_SMI_CMD APM_CNT_IOPORT
 
-typedef void (*apm_ctrl_changed_t)(uint32_t val, void *arg);
+typedef void (*apm_ctrl_changed_t)(CPUState *cs, uint32_t val, void *arg);
 
 typedef struct APMState {
     uint8_t apmc;
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index bd9bbade70..70ad1cd1ff 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -30,7 +30,6 @@ 
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
 #include "qemu/timer.h"
-#include "hw/core/cpu.h"
 #include "sysemu/reset.h"
 #include "sysemu/runstate.h"
 #include "hw/acpi/acpi.h"
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 0a81f1ad93..43b78ef8f9 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -70,7 +70,7 @@  static void pm_tmr_timer(ACPIREGS *ar)
     acpi_update_sci(&s->ar, s->irq);
 }
 
-static void apm_ctrl_changed(uint32_t val, void *arg)
+static void apm_ctrl_changed(CPUState *cs, uint32_t val, void *arg)
 {
     PIIX4PMState *s = arg;
     PCIDevice *d = PCI_DEVICE(s);
diff --git a/hw/isa/apm.c b/hw/isa/apm.c
index dfe9020d30..95efbf2457 100644
--- a/hw/isa/apm.c
+++ b/hw/isa/apm.c
@@ -21,6 +21,8 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "hw/core/cpu.h"
 #include "hw/isa/apm.h"
 #include "hw/pci/pci.h"
 #include "migration/vmstate.h"
@@ -30,10 +32,19 @@ 
 /* fixed I/O location */
 #define APM_STS_IOPORT  0xb3
 
-static void apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
-                              unsigned size)
+static MemTxResult apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
+                                     unsigned size, MemTxAttrs attrs)
 {
     APMState *apm = opaque;
+    CPUState *cs;
+
+    if (attrs.requester_type != MTRT_CPU) {
+        qemu_log_mask(LOG_UNIMP | LOG_GUEST_ERROR,
+                      "%s: saw non-CPU transaction", __func__);
+        return MEMTX_ACCESS_ERROR;
+    }
+    cs = qemu_get_cpu(attrs.requester_id);
+
     addr &= 1;
 
     trace_apm_io_write(addr, val);
@@ -41,11 +52,13 @@  static void apm_ioport_writeb(void *opaque, hwaddr addr, uint64_t val,
         apm->apmc = val;
 
         if (apm->callback) {
-            (apm->callback)(val, apm->arg);
+            (apm->callback)(cs, val, apm->arg);
         }
     } else {
         apm->apms = val;
     }
+
+    return MEMTX_OK;
 }
 
 static uint64_t apm_ioport_readb(void *opaque, hwaddr addr, unsigned size)
@@ -77,7 +90,7 @@  const VMStateDescription vmstate_apm = {
 
 static const MemoryRegionOps apm_ops = {
     .read = apm_ioport_readb,
-    .write = apm_ioport_writeb,
+    .write_with_attrs = apm_ioport_writeb,
     .impl = {
         .min_access_size = 1,
         .max_access_size = 1,
diff --git a/hw/isa/lpc_ich9.c b/hw/isa/lpc_ich9.c
index 0b0a83e080..2700a18a65 100644
--- a/hw/isa/lpc_ich9.c
+++ b/hw/isa/lpc_ich9.c
@@ -443,7 +443,7 @@  void ich9_lpc_pm_init(PCIDevice *lpc_pci, bool smm_enabled)
 
 /* APM */
 
-static void ich9_apm_ctrl_changed(uint32_t val, void *arg)
+static void ich9_apm_ctrl_changed(CPUState *cs, uint32_t val, void *arg)
 {
     ICH9LPCState *lpc = arg;
 
@@ -459,12 +459,11 @@  static void ich9_apm_ctrl_changed(uint32_t val, void *arg)
     if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) {
         if (lpc->smi_negotiated_features &
             (UINT64_C(1) << ICH9_LPC_SMI_F_BROADCAST_BIT)) {
-            CPUState *cs;
             CPU_FOREACH(cs) {
                 cpu_interrupt(cs, CPU_INTERRUPT_SMI);
             }
         } else {
-            cpu_interrupt(current_cpu, CPU_INTERRUPT_SMI);
+            cpu_interrupt(cs, CPU_INTERRUPT_SMI);
         }
     }
 }