[PATCH v1 11/22] hw/misc/aspeed_hace: Add trace-events for better debugging

Jamin Lin via posted 22 patches 10 months, 3 weeks ago
There is a newer version of this series
[PATCH v1 11/22] hw/misc/aspeed_hace: Add trace-events for better debugging
Posted by Jamin Lin via 10 months, 3 weeks ago
Introduced "trace_aspeed_hace_addr", "trace_aspeed_hace_sg",
"trace_aspeed_hace_read", and "trace_aspeed_hace_write" trace events.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 hw/misc/aspeed_hace.c | 8 ++++++++
 hw/misc/trace-events  | 6 ++++++
 2 files changed, 14 insertions(+)

diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
index 53b3b390e3..b8e473ee3f 100644
--- a/hw/misc/aspeed_hace.c
+++ b/hw/misc/aspeed_hace.c
@@ -18,6 +18,7 @@
 #include "crypto/hash.h"
 #include "hw/qdev-properties.h"
 #include "hw/irq.h"
+#include "trace.h"
 
 #define R_CRYPT_CMD     (0x10 / 4)
 
@@ -186,6 +187,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             if (ahc->has_dma64) {
                 src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
             }
+            trace_aspeed_hace_addr("src", src);
             src += i * SG_LIST_ENTRY_SIZE;
 
             len = address_space_ldl_le(&s->dram_as, src,
@@ -194,6 +196,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
             sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
                                            MEMTXATTRS_UNSPECIFIED, NULL);
             sg_addr &= SG_LIST_ADDR_MASK;
+            trace_aspeed_hace_sg(i, sg_addr, len);
             /*
              * Ideally, sg_addr should be 64-bit for the AST2700, using the
              * following program to obtain the 64-bit sg_addr and convert it
@@ -237,6 +240,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
     } else {
         plen = s->regs[R_HASH_SRC_LEN];
         src = deposit64(src, 0, 32, s->regs[R_HASH_SRC]);
+        trace_aspeed_hace_addr("src", src);
         if (ahc->has_dma64) {
             src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
         }
@@ -299,6 +303,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
     if (ahc->has_dma64) {
         digest_addr = deposit64(digest_addr, 32, 32, s->regs[R_HASH_DEST_HI]);
     }
+    trace_aspeed_hace_addr("digest", digest_addr);
     if (address_space_write(&s->dram_as, digest_addr,
                             MEMTXATTRS_UNSPECIFIED,
                             digest_buf, digest_len)) {
@@ -326,6 +331,7 @@ static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
         return 0;
     }
 
+    trace_aspeed_hace_read(addr << 2, s->regs[addr]);
     return s->regs[addr];
 }
 
@@ -344,6 +350,8 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
         return;
     }
 
+    trace_aspeed_hace_write(addr << 2, data);
+
     switch (addr) {
     case R_STATUS:
         if (data & HASH_IRQ) {
diff --git a/hw/misc/trace-events b/hw/misc/trace-events
index 4383808d7a..cf96e68cfa 100644
--- a/hw/misc/trace-events
+++ b/hw/misc/trace-events
@@ -302,6 +302,12 @@ aspeed_peci_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%"
 aspeed_peci_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
 aspeed_peci_raise_interrupt(uint32_t ctrl, uint32_t status) "ctrl 0x%" PRIx32 " status 0x%" PRIx32
 
+# aspeed_hace.c
+aspeed_hace_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_hace_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
+aspeed_hace_sg(int index, uint64_t addr, uint32_t len) "%d: addr 0x%" PRIx64 " len 0x%" PRIx32
+aspeed_hace_addr(const char *s, uint64_t addr) "%s: 0x%" PRIx64
+
 # bcm2835_property.c
 bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
 
-- 
2.43.0
Re: [PATCH v1 11/22] hw/misc/aspeed_hace: Add trace-events for better debugging
Posted by Cédric Le Goater 10 months, 1 week ago
On 3/21/25 10:26, Jamin Lin wrote:
> Introduced "trace_aspeed_hace_addr", "trace_aspeed_hace_sg",
> "trace_aspeed_hace_read", and "trace_aspeed_hace_write" trace events.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   hw/misc/aspeed_hace.c | 8 ++++++++
>   hw/misc/trace-events  | 6 ++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/hw/misc/aspeed_hace.c b/hw/misc/aspeed_hace.c
> index 53b3b390e3..b8e473ee3f 100644
> --- a/hw/misc/aspeed_hace.c
> +++ b/hw/misc/aspeed_hace.c
> @@ -18,6 +18,7 @@
>   #include "crypto/hash.h"
>   #include "hw/qdev-properties.h"
>   #include "hw/irq.h"
> +#include "trace.h"
>   
>   #define R_CRYPT_CMD     (0x10 / 4)
>   
> @@ -186,6 +187,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
>               if (ahc->has_dma64) {
>                   src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
>               }
> +            trace_aspeed_hace_addr("src", src);
>               src += i * SG_LIST_ENTRY_SIZE;
>   
>               len = address_space_ldl_le(&s->dram_as, src,
> @@ -194,6 +196,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
>               sg_addr = address_space_ldl_le(&s->dram_as, src + SG_LIST_LEN_SIZE,
>                                              MEMTXATTRS_UNSPECIFIED, NULL);
>               sg_addr &= SG_LIST_ADDR_MASK;
> +            trace_aspeed_hace_sg(i, sg_addr, len);
>               /*
>                * Ideally, sg_addr should be 64-bit for the AST2700, using the
>                * following program to obtain the 64-bit sg_addr and convert it
> @@ -237,6 +240,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
>       } else {
>           plen = s->regs[R_HASH_SRC_LEN];
>           src = deposit64(src, 0, 32, s->regs[R_HASH_SRC]);
> +        trace_aspeed_hace_addr("src", src);
>           if (ahc->has_dma64) {
>               src = deposit64(src, 32, 32, s->regs[R_HASH_SRC_HI]);
>           }
> @@ -299,6 +303,7 @@ static void do_hash_operation(AspeedHACEState *s, int algo, bool sg_mode,
>       if (ahc->has_dma64) {
>           digest_addr = deposit64(digest_addr, 32, 32, s->regs[R_HASH_DEST_HI]);
>       }
> +    trace_aspeed_hace_addr("digest", digest_addr);
>       if (address_space_write(&s->dram_as, digest_addr,
>                               MEMTXATTRS_UNSPECIFIED,
>                               digest_buf, digest_len)) {

I would use the prefix 'trace_aspeed_hace_hash_addr' since the trace
events are called from do_hash_operation()


Thanks,

C.



> @@ -326,6 +331,7 @@ static uint64_t aspeed_hace_read(void *opaque, hwaddr addr, unsigned int size)
>           return 0;
>       }
>   
> +    trace_aspeed_hace_read(addr << 2, s->regs[addr]);
>       return s->regs[addr];
>   }
>   
> @@ -344,6 +350,8 @@ static void aspeed_hace_write(void *opaque, hwaddr addr, uint64_t data,
>           return;
>       }
>   
> +    trace_aspeed_hace_write(addr << 2, data);
> +
>       switch (addr) {
>       case R_STATUS:
>           if (data & HASH_IRQ) {
> diff --git a/hw/misc/trace-events b/hw/misc/trace-events
> index 4383808d7a..cf96e68cfa 100644
> --- a/hw/misc/trace-events
> +++ b/hw/misc/trace-events
> @@ -302,6 +302,12 @@ aspeed_peci_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%"
>   aspeed_peci_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
>   aspeed_peci_raise_interrupt(uint32_t ctrl, uint32_t status) "ctrl 0x%" PRIx32 " status 0x%" PRIx32
>   
> +# aspeed_hace.c
> +aspeed_hace_read(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
> +aspeed_hace_write(uint64_t offset, uint64_t data) "offset 0x%" PRIx64 " data 0x%" PRIx64
> +aspeed_hace_sg(int index, uint64_t addr, uint32_t len) "%d: addr 0x%" PRIx64 " len 0x%" PRIx32
> +aspeed_hace_addr(const char *s, uint64_t addr) "%s: 0x%" PRIx64
> +
>   # bcm2835_property.c
>   bcm2835_mbox_property(uint32_t tag, uint32_t bufsize, size_t resplen) "mbox property tag:0x%08x in_sz:%u out_sz:%zu"
>