Re: [PATCH v4 1/1] hw/riscv: Add signature dump function for spike to run ACT tests

[Alistair Francis]

On Thu, Apr 6, 2023 at 11:02 AM liweiwei <liweiwei@iscas.ac.cn> wrote:
>
>
> On 2023/4/6 08:36, Alistair Francis wrote:
> > On Wed, Apr 5, 2023 at 7:58 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> >> Add signature and signature-granularity properties in spike to specify the 
> >> target
> >> signatrue file and the line size for signature data.
> >>
> >> Recgonize the signature section between begin_signature and end_signature 
> >> symbols
> >> when loading elf of ACT tests. Then dump signature data in signature 
> >> section just
> >> before the ACT tests exit.
> >>
> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> >> Reviewed-by: LIU Zhiwei <zhiwei_liu@linux.alibaba.com>
> >> ---
> >>   hw/char/riscv_htif.c         | 44 +++++++++++++++++++++++++++++++++++-
> >>   hw/riscv/spike.c             | 13 +++++++++++
> >>   include/hw/char/riscv_htif.h |  3 +++
> >>   3 files changed, 59 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> >> index 098de50e35..37d3ccc76b 100644
> >> --- a/hw/char/riscv_htif.c
> >> +++ b/hw/char/riscv_htif.c
> >> @@ -29,6 +29,8 @@
> >>   #include "chardev/char-fe.h"
> >>   #include "qemu/timer.h"
> >>   #include "qemu/error-report.h"
> >> +#include "exec/address-spaces.h"
> >> +#include "sysemu/dma.h"
> >>
> >>   #define RISCV_DEBUG_HTIF 0
> >>   #define HTIF_DEBUG(fmt, ...)                                             
> >>       \
> >> @@ -51,7 +53,10 @@
> >>   /* PK system call number */
> >>   #define PK_SYS_WRITE            64
> >>
> >> -static uint64_t fromhost_addr, tohost_addr;
> >> +const char *sig_file;
> >> +uint8_t line_size = 16;
> >> +
> >> +static uint64_t fromhost_addr, tohost_addr, begin_sig_addr, end_sig_addr;
> >>
> >>   void htif_symbol_callback(const char *st_name, int st_info, uint64_t 
> >> st_value,
> >>                             uint64_t st_size)
> >> @@ -68,6 +73,10 @@ void htif_symbol_callback(const char *st_name, int 
> >> st_info, uint64_t st_value,
> >>               error_report("HTIF tohost must be 8 bytes");
> >>               exit(1);
> >>           }
> >> +    } else if (strcmp("begin_signature", st_name) == 0) {
> >> +        begin_sig_addr = st_value;
> >> +    } else if (strcmp("end_signature", st_name) == 0) {
> >> +        end_sig_addr = st_value;
> >>       }
> >>   }
> >>
> >> @@ -163,6 +172,39 @@ static void htif_handle_tohost_write(HTIFState *s, 
> >> uint64_t val_written)
> >>               if (payload & 0x1) {
> >>                   /* exit code */
> >>                   int exit_code = payload >> 1;
> >> +
> >> +                /*
> >> +                 * Dump signature data if sig_file is specified and
> >> +                 * begin/end_signature symbols exist.
> >> +                 */
> >> +                if (sig_file && begin_sig_addr && end_sig_addr) {
> > There is no guarantee that these are initalised to zero, so this isn't
> > really checking anything is it?
>
> I think the static global variable will be  initialized to zero by default.

Ah, yes you are right. static variables are initalised to zero as per
the C99 standard.

In which case:

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

>
> If not,  fromhost_addr and tohost_addr may have the same problem.
>
> Regards,
>
> Weiwei Li
>
> >