| Message ID | 20230405095720.75848-2-liweiwei@iscas.ac.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/riscv: Add ACT related support | expand |
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? Alistair > + uint64_t sig_len = end_sig_addr - begin_sig_addr; > + char *sig_data = g_malloc(sig_len); > + dma_memory_read(&address_space_memory, begin_sig_addr, > + sig_data, sig_len, MEMTXATTRS_UNSPECIFIED); > + FILE *signature = fopen(sig_file, "w"); > + if (signature == NULL) { > + error_report("Unable to open %s with error %s", > + sig_file, strerror(errno)); > + exit(1); > + } > + > + for (int i = 0; i < sig_len; i += line_size) { > + for (int j = line_size; j > 0; j--) { > + if (i + j <= sig_len) { > + fprintf(signature, "%02x", > + sig_data[i + j - 1] & 0xff); > + } else { > + fprintf(signature, "%02x", 0); > + } > + } > + fprintf(signature, "\n"); > + } > + > + fclose(signature); > + g_free(sig_data); > + } > + > exit(exit_code); > } else { > uint64_t syscall[8]; > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > index a584d5b3a2..2c5546560a 100644 > --- a/hw/riscv/spike.c > +++ b/hw/riscv/spike.c > @@ -332,6 +332,11 @@ static void spike_board_init(MachineState *machine) > htif_custom_base); > } > > +static void spike_set_signature(Object *obj, const char *val, Error **errp) > +{ > + sig_file = g_strdup(val); > +} > + > static void spike_machine_instance_init(Object *obj) > { > } > @@ -350,6 +355,14 @@ static void spike_machine_class_init(ObjectClass *oc, void *data) > mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id; > mc->numa_mem_supported = true; > mc->default_ram_id = "riscv.spike.ram"; > + object_class_property_add_str(oc, "signature", NULL, spike_set_signature); > + object_class_property_set_description(oc, "signature", > + "File to write ACT test signature"); > + object_class_property_add_uint8_ptr(oc, "signature-granularity", > + &line_size, OBJ_PROP_FLAG_WRITE); > + object_class_property_set_description(oc, "signature-granularity", > + "Size of each line in ACT signature " > + "file"); > } > > static const TypeInfo spike_machine_typeinfo = { > diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h > index 5958c5b986..df493fdf6b 100644 > --- a/include/hw/char/riscv_htif.h > +++ b/include/hw/char/riscv_htif.h > @@ -40,6 +40,9 @@ typedef struct HTIFState { > uint64_t pending_read; > } HTIFState; > > +extern const char *sig_file; > +extern uint8_t line_size; > + > /* HTIF symbol callback */ > void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value, > uint64_t st_size); > -- > 2.25.1 > >
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. If not, fromhost_addr and tohost_addr may have the same problem. Regards, Weiwei Li > > Alistair > >> + uint64_t sig_len = end_sig_addr - begin_sig_addr; >> + char *sig_data = g_malloc(sig_len); >> + dma_memory_read(&address_space_memory, begin_sig_addr, >> + sig_data, sig_len, MEMTXATTRS_UNSPECIFIED); >> + FILE *signature = fopen(sig_file, "w"); >> + if (signature == NULL) { >> + error_report("Unable to open %s with error %s", >> + sig_file, strerror(errno)); >> + exit(1); >> + } >> + >> + for (int i = 0; i < sig_len; i += line_size) { >> + for (int j = line_size; j > 0; j--) { >> + if (i + j <= sig_len) { >> + fprintf(signature, "%02x", >> + sig_data[i + j - 1] & 0xff); >> + } else { >> + fprintf(signature, "%02x", 0); >> + } >> + } >> + fprintf(signature, "\n"); >> + } >> + >> + fclose(signature); >> + g_free(sig_data); >> + } >> + >> exit(exit_code); >> } else { >> uint64_t syscall[8]; >> diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c >> index a584d5b3a2..2c5546560a 100644 >> --- a/hw/riscv/spike.c >> +++ b/hw/riscv/spike.c >> @@ -332,6 +332,11 @@ static void spike_board_init(MachineState *machine) >> htif_custom_base); >> } >> >> +static void spike_set_signature(Object *obj, const char *val, Error **errp) >> +{ >> + sig_file = g_strdup(val); >> +} >> + >> static void spike_machine_instance_init(Object *obj) >> { >> } >> @@ -350,6 +355,14 @@ static void spike_machine_class_init(ObjectClass *oc, void *data) >> mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id; >> mc->numa_mem_supported = true; >> mc->default_ram_id = "riscv.spike.ram"; >> + object_class_property_add_str(oc, "signature", NULL, spike_set_signature); >> + object_class_property_set_description(oc, "signature", >> + "File to write ACT test signature"); >> + object_class_property_add_uint8_ptr(oc, "signature-granularity", >> + &line_size, OBJ_PROP_FLAG_WRITE); >> + object_class_property_set_description(oc, "signature-granularity", >> + "Size of each line in ACT signature " >> + "file"); >> } >> >> static const TypeInfo spike_machine_typeinfo = { >> diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h >> index 5958c5b986..df493fdf6b 100644 >> --- a/include/hw/char/riscv_htif.h >> +++ b/include/hw/char/riscv_htif.h >> @@ -40,6 +40,9 @@ typedef struct HTIFState { >> uint64_t pending_read; >> } HTIFState; >> >> +extern const char *sig_file; >> +extern uint8_t line_size; >> + >> /* HTIF symbol callback */ >> void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value, >> uint64_t st_size); >> -- >> 2.25.1 >> >>
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 > > >
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) { + uint64_t sig_len = end_sig_addr - begin_sig_addr; + char *sig_data = g_malloc(sig_len); + dma_memory_read(&address_space_memory, begin_sig_addr, + sig_data, sig_len, MEMTXATTRS_UNSPECIFIED); + FILE *signature = fopen(sig_file, "w"); + if (signature == NULL) { + error_report("Unable to open %s with error %s", + sig_file, strerror(errno)); + exit(1); + } + + for (int i = 0; i < sig_len; i += line_size) { + for (int j = line_size; j > 0; j--) { + if (i + j <= sig_len) { + fprintf(signature, "%02x", + sig_data[i + j - 1] & 0xff); + } else { + fprintf(signature, "%02x", 0); + } + } + fprintf(signature, "\n"); + } + + fclose(signature); + g_free(sig_data); + } + exit(exit_code); } else { uint64_t syscall[8]; diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c index a584d5b3a2..2c5546560a 100644 --- a/hw/riscv/spike.c +++ b/hw/riscv/spike.c @@ -332,6 +332,11 @@ static void spike_board_init(MachineState *machine) htif_custom_base); } +static void spike_set_signature(Object *obj, const char *val, Error **errp) +{ + sig_file = g_strdup(val); +} + static void spike_machine_instance_init(Object *obj) { } @@ -350,6 +355,14 @@ static void spike_machine_class_init(ObjectClass *oc, void *data) mc->get_default_cpu_node_id = riscv_numa_get_default_cpu_node_id; mc->numa_mem_supported = true; mc->default_ram_id = "riscv.spike.ram"; + object_class_property_add_str(oc, "signature", NULL, spike_set_signature); + object_class_property_set_description(oc, "signature", + "File to write ACT test signature"); + object_class_property_add_uint8_ptr(oc, "signature-granularity", + &line_size, OBJ_PROP_FLAG_WRITE); + object_class_property_set_description(oc, "signature-granularity", + "Size of each line in ACT signature " + "file"); } static const TypeInfo spike_machine_typeinfo = { diff --git a/include/hw/char/riscv_htif.h b/include/hw/char/riscv_htif.h index 5958c5b986..df493fdf6b 100644 --- a/include/hw/char/riscv_htif.h +++ b/include/hw/char/riscv_htif.h @@ -40,6 +40,9 @@ typedef struct HTIFState { uint64_t pending_read; } HTIFState; +extern const char *sig_file; +extern uint8_t line_size; + /* HTIF symbol callback */ void htif_symbol_callback(const char *st_name, int st_info, uint64_t st_value, uint64_t st_size);