1. Patchew
  2. QEMU
  3. [RFC PATCH v2 0/4] Use ACPI PCI hot-plug for q35
  4. View patch

[RFC PATCH v2 1/4] hw/acpi/ich9: Trace ich9_gpe_readb()/writeb()

Julia Suvorova posted 4 patches 5 years, 5 months ago
Maintainers: Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Aurelien Jarno <aurelien@aurel32.net>, "Philippe Mathieu-Daudé" <philmd@redhat.com>, Richard Henderson <rth@twiddle.net>, Paolo Bonzini <pbonzini@redhat.com>, Igor Mammedov <imammedo@redhat.com>, Aleksandar Markovic <aleksandar.qemu.devel@gmail.com>, Eduardo Habkost <ehabkost@redhat.com>
[RFC PATCH v2 1/4] hw/acpi/ich9: Trace ich9_gpe_readb()/writeb()
Posted by Julia Suvorova 5 years, 5 months ago 
Add trace events similar to piix4_gpe_readb() to check gpe status.

Signed-off-by: Julia Suvorova <jusual@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/ich9.c       | 7 ++++++-
 hw/acpi/trace-events | 4 ++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 6a19070cec..a2a1742aa6 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -36,6 +36,7 @@
 #include "hw/acpi/acpi.h"
 #include "hw/acpi/tco.h"
 #include "exec/address-spaces.h"
+#include "trace.h"
 
 #include "hw/i386/ich9.h"
 #include "hw/mem/pc-dimm.h"
@@ -59,13 +60,17 @@ static void ich9_pm_update_sci_fn(ACPIREGS *regs)
 static uint64_t ich9_gpe_readb(void *opaque, hwaddr addr, unsigned width)
 {
     ICH9LPCPMRegs *pm = opaque;
-    return acpi_gpe_ioport_readb(&pm->acpi_regs, addr);
+    uint64_t val = acpi_gpe_ioport_readb(&pm->acpi_regs, addr);
+
+    trace_ich9_gpe_readb(addr, width, val);
+    return val;
 }
 
 static void ich9_gpe_writeb(void *opaque, hwaddr addr, uint64_t val,
                             unsigned width)
 {
     ICH9LPCPMRegs *pm = opaque;
+    trace_ich9_gpe_writeb(addr, width, val);
     acpi_gpe_ioport_writeb(&pm->acpi_regs, addr, val);
     acpi_update_sci(&pm->acpi_regs, pm->irq);
 }
diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
index afbc77de1c..b9f4827afc 100644
--- a/hw/acpi/trace-events
+++ b/hw/acpi/trace-events
@@ -32,6 +32,10 @@ cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
 cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
 cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
 
+# ich9.c
+ich9_gpe_readb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d ==> 0x%" PRIx64
+ich9_gpe_writeb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d <== 0x%" PRIx64
+
 # pcihp.c
 acpi_pci_eject_slot(unsigned bsel, unsigned slot) "bsel: %u slot: %u"
 acpi_pci_unplug(int bsel, int slot) "bsel: %d slot: %d"
-- 
2.25.4
Re: [RFC PATCH v2 1/4] hw/acpi/ich9: Trace ich9_gpe_readb()/writeb()
Posted by Philippe Mathieu-Daudé 5 years, 5 months ago 
On 8/18/20 11:52 PM, Julia Suvorova wrote:
> Add trace events similar to piix4_gpe_readb() to check gpe status.
> 
> Signed-off-by: Julia Suvorova <jusual@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/acpi/ich9.c       | 7 ++++++-
>  hw/acpi/trace-events | 4 ++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 6a19070cec..a2a1742aa6 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -36,6 +36,7 @@
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/tco.h"
>  #include "exec/address-spaces.h"
> +#include "trace.h"
>  
>  #include "hw/i386/ich9.h"
>  #include "hw/mem/pc-dimm.h"
> @@ -59,13 +60,17 @@ static void ich9_pm_update_sci_fn(ACPIREGS *regs)
>  static uint64_t ich9_gpe_readb(void *opaque, hwaddr addr, unsigned width)
>  {
>      ICH9LPCPMRegs *pm = opaque;
> -    return acpi_gpe_ioport_readb(&pm->acpi_regs, addr);
> +    uint64_t val = acpi_gpe_ioport_readb(&pm->acpi_regs, addr);
> +
> +    trace_ich9_gpe_readb(addr, width, val);
> +    return val;
>  }
>  
>  static void ich9_gpe_writeb(void *opaque, hwaddr addr, uint64_t val,
>                              unsigned width)
>  {
>      ICH9LPCPMRegs *pm = opaque;
> +    trace_ich9_gpe_writeb(addr, width, val);
>      acpi_gpe_ioport_writeb(&pm->acpi_regs, addr, val);
>      acpi_update_sci(&pm->acpi_regs, pm->irq);
>  }
> diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> index afbc77de1c..b9f4827afc 100644
> --- a/hw/acpi/trace-events
> +++ b/hw/acpi/trace-events
> @@ -32,6 +32,10 @@ cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
>  cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
>  cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
>  
> +# ich9.c
> +ich9_gpe_readb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d ==> 0x%" PRIx64
> +ich9_gpe_writeb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d <== 0x%" PRIx64

Nitpick, val could be uint8_t.

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +
>  # pcihp.c
>  acpi_pci_eject_slot(unsigned bsel, unsigned slot) "bsel: %u slot: %u"
>  acpi_pci_unplug(int bsel, int slot) "bsel: %d slot: %d"
>
Re: [RFC PATCH v2 1/4] hw/acpi/ich9: Trace ich9_gpe_readb()/writeb()
Posted by Julia Suvorova 5 years, 5 months ago 
On Wed, Aug 19, 2020 at 5:14 AM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 8/18/20 11:52 PM, Julia Suvorova wrote:
> > Add trace events similar to piix4_gpe_readb() to check gpe status.
> >
> > Signed-off-by: Julia Suvorova <jusual@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/acpi/ich9.c       | 7 ++++++-
> >  hw/acpi/trace-events | 4 ++++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 6a19070cec..a2a1742aa6 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -36,6 +36,7 @@
> >  #include "hw/acpi/acpi.h"
> >  #include "hw/acpi/tco.h"
> >  #include "exec/address-spaces.h"
> > +#include "trace.h"
> >
> >  #include "hw/i386/ich9.h"
> >  #include "hw/mem/pc-dimm.h"
> > @@ -59,13 +60,17 @@ static void ich9_pm_update_sci_fn(ACPIREGS *regs)
> >  static uint64_t ich9_gpe_readb(void *opaque, hwaddr addr, unsigned width)
> >  {
> >      ICH9LPCPMRegs *pm = opaque;
> > -    return acpi_gpe_ioport_readb(&pm->acpi_regs, addr);
> > +    uint64_t val = acpi_gpe_ioport_readb(&pm->acpi_regs, addr);
> > +
> > +    trace_ich9_gpe_readb(addr, width, val);
> > +    return val;
> >  }
> >
> >  static void ich9_gpe_writeb(void *opaque, hwaddr addr, uint64_t val,
> >                              unsigned width)
> >  {
> >      ICH9LPCPMRegs *pm = opaque;
> > +    trace_ich9_gpe_writeb(addr, width, val);
> >      acpi_gpe_ioport_writeb(&pm->acpi_regs, addr, val);
> >      acpi_update_sci(&pm->acpi_regs, pm->irq);
> >  }
> > diff --git a/hw/acpi/trace-events b/hw/acpi/trace-events
> > index afbc77de1c..b9f4827afc 100644
> > --- a/hw/acpi/trace-events
> > +++ b/hw/acpi/trace-events
> > @@ -32,6 +32,10 @@ cpuhp_acpi_ejecting_cpu(uint32_t idx) "0x%"PRIx32
> >  cpuhp_acpi_write_ost_ev(uint32_t slot, uint32_t ev) "idx[0x%"PRIx32"] OST EVENT: 0x%"PRIx32
> >  cpuhp_acpi_write_ost_status(uint32_t slot, uint32_t st) "idx[0x%"PRIx32"] OST STATUS: 0x%"PRIx32
> >
> > +# ich9.c
> > +ich9_gpe_readb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d ==> 0x%" PRIx64
> > +ich9_gpe_writeb(uint64_t addr, unsigned width, uint64_t val) "addr: 0x%" PRIx64 " width: %d <== 0x%" PRIx64
>
> Nitpick, val could be uint8_t.

Since 'val' is an argument and its type is uint64_t it's better to
avoid implicit cast to a smaller type. Also, acpi_gpe_ioport_readb()
does not return uint8_t.

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.