On Mon, Oct 14, 2024 at 8:08 PM 阎明铸 <[email protected]> wrote:
>
> Thank you for your reply and I'm sorry that I didn't explain it clearly.
>
> - ACT is an official riscv test suite to check the riscv support of the 
> DUT(device under test).

It's probably worth including this in the commit message.

> - Currently ACT support using 
> [sail-riscv](https://github.com/riscv/sail-riscv)(default) or 
> [spike](https://github.com/riscv-software-src/riscv-isa-sim)
> - QEMU is not supported yet,but someone made a commit: 
> [commit](https://github.com/qemu/qemu/commit/66247edc8b6fb36d6b905babcd795068ea989ad5)
>
> But there are still problems, so I'm trying to fix it. After debugging, I 
> found that it's a htif problem, and the idea of fixing it is referenced from 
> the sail-riscv implementation

It would be good to reference the sail implementation and the
justification for the change there

Alistair

>
> "Alistair Francis" &lt;[email protected]&gt;写道:
> &gt; On Fri, Sep 27, 2024 at 11:26 PM MingZhu Yan <[email protected]> wrote:
> &gt; &gt;
> &gt; &gt; Applications sometimes only write the lower 32-bit payload bytes, 
> this is used
> &gt; &gt; in ACT tests. As a workaround, this refers to the solution of 
> sail-riscv.
> &gt;
> &gt; I'm not sure what ACT is, but this feels like a guest bug, not a QEMU 
> issue.
> &gt;
> &gt; Alistair
> &gt;
> &gt; &gt; if the payload is written a few times with the same value, we 
> process the whole
> &gt; &gt; htif command anyway.
> &gt; &gt;
> &gt; &gt; Signed-off-by: MingZhu Yan <[email protected]>
> &gt; &gt; ---
> &gt; &gt;  hw/char/riscv_htif.c | 35 +++++++++++++++++++----------------
> &gt; &gt;  1 file changed, 19 insertions(+), 16 deletions(-)
> &gt; &gt;
> &gt; &gt; diff --git a/hw/char/riscv_htif.c b/hw/char/riscv_htif.c
> &gt; &gt; index 9bef60def1..d74cce3bef 100644
> &gt; &gt; --- a/hw/char/riscv_htif.c
> &gt; &gt; +++ b/hw/char/riscv_htif.c
> &gt; &gt; @@ -65,16 +65,8 @@ void htif_symbol_callback(const char *st_name, 
> int st_info, uint64_t st_value,
> &gt; &gt;  {
> &gt; &gt;      if (strcmp("fromhost", st_name) == 0) {
> &gt; &gt;          fromhost_addr = st_value;
> &gt; &gt; -        if (st_size != 8) {
> &gt; &gt; -            error_report("HTIF fromhost must be 8 bytes");
> &gt; &gt; -            exit(1);
> &gt; &gt; -        }
> &gt; &gt;      } else if (strcmp("tohost", st_name) == 0) {
> &gt; &gt;          tohost_addr = st_value;
> &gt; &gt; -        if (st_size != 8) {
> &gt; &gt; -            error_report("HTIF tohost must be 8 bytes");
> &gt; &gt; -            exit(1);
> &gt; &gt; -        }
> &gt; &gt;      } else if (strcmp("begin_signature", st_name) == 0) {
> &gt; &gt;          begin_sig_addr = st_value;
> &gt; &gt;      } else if (strcmp("end_signature", st_name) == 0) {
> &gt; &gt; @@ -290,18 +282,26 @@ static void htif_mm_write(void *opaque, 
> hwaddr addr,
> &gt; &gt;                            uint64_t value, unsigned size)
> &gt; &gt;  {
> &gt; &gt;      HTIFState *s = opaque;
> &gt; &gt; -    if (addr == TOHOST_OFFSET1) {
> &gt; &gt; -        if (s-&gt;tohost == 0x0) {
> &gt; &gt; -            s-&gt;allow_tohost = 1;
> &gt; &gt; -            s-&gt;tohost = value &amp; 0xFFFFFFFF;
> &gt; &gt; +    int htif_cmd_write = 0;
> &gt; &gt; +    if (size == 8 &amp;&amp; addr == TOHOST_OFFSET1) {
> &gt; &gt; +        htif_cmd_write = 1;
> &gt; &gt; +        s-&gt;tohost = value;
> &gt; &gt; +        htif_handle_tohost_write(s, s-&gt;tohost);
> &gt; &gt; +    } else if (size == 4 &amp;&amp; addr == TOHOST_OFFSET1) {
> &gt; &gt; +        if ((value) == (s-&gt;tohost &amp; 0xFFFF)) {
> &gt; &gt; +            s-&gt;allow_tohost = s-&gt;allow_tohost + 1;
> &gt; &gt;          } else {
> &gt; &gt;              s-&gt;allow_tohost = 0;
> &gt; &gt;          }
> &gt; &gt; -    } else if (addr == TOHOST_OFFSET2) {
> &gt; &gt; -        if (s-&gt;allow_tohost) {
> &gt; &gt; -            s-&gt;tohost |= value &lt;&lt; 32;
> &gt; &gt; -            htif_handle_tohost_write(s, s-&gt;tohost);
> &gt; &gt; +        s-&gt;tohost = deposit64(s-&gt;tohost, 0, 32, value);
> &gt; &gt; +    } else if (size == 4 &amp;&amp; addr == TOHOST_OFFSET2) {
> &gt; &gt; +        if ((value &amp; 0xFF) == (s-&gt;tohost &amp; 0xFF00)) {
> &gt; &gt; +            s-&gt;allow_tohost = s-&gt;allow_tohost + 1;
> &gt; &gt; +        } else {
> &gt; &gt; +            s-&gt;allow_tohost = 1;
> &gt; &gt;          }
> &gt; &gt; +        htif_cmd_write = 1;
> &gt; &gt; +        s-&gt;tohost = deposit64(s-&gt;tohost, 32, 32, value);
> &gt; &gt;      } else if (addr == FROMHOST_OFFSET1) {
> &gt; &gt;          s-&gt;fromhost_inprogress = 1;
> &gt; &gt;          s-&gt;fromhost = value &amp; 0xFFFFFFFF;
> &gt; &gt; @@ -312,6 +312,9 @@ static void htif_mm_write(void *opaque, hwaddr 
> addr,
> &gt; &gt;          qemu_log("Invalid htif write: address %016" PRIx64 "\n",
> &gt; &gt;              (uint64_t)addr);
> &gt; &gt;      }
> &gt; &gt; +    if ((s-&gt;tohost == 1 &amp;&amp; htif_cmd_write) || 
> s-&gt;allow_tohost &gt; 2) {
> &gt; &gt; +        htif_handle_tohost_write(s, s-&gt;tohost);
> &gt; &gt; +    }
> &gt; &gt;  }
> &gt; &gt;
> &gt; &gt;  static const MemoryRegionOps htif_mm_ops = {
> &gt; &gt; --
> &gt; &gt; 2.34.1
> &gt; &gt;
> &gt; &gt;
> </[email protected]></[email protected]>

Reply via email to