All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 0/3]  A few RISC-V fixes
@ 2020-06-30 20:12 ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, palmer, alistair23


This series has a few fixes for RISC-V.



Alistair Francis (3):
  hw/char: Convert the Ibex UART to use the qdev Clock model
  hw/riscv: Allow 64 bit access to SiFive CLINT
  target/riscv: Regen floating point rounding mode in dynamic mode

 include/hw/char/ibex_uart.h |  2 ++
 hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
 hw/riscv/sifive_clint.c     |  2 +-
 target/riscv/translate.c    |  2 +-
 4 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v1 0/3]  A few RISC-V fixes
@ 2020-06-30 20:12 ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: palmer, alistair.francis, alistair23


This series has a few fixes for RISC-V.



Alistair Francis (3):
  hw/char: Convert the Ibex UART to use the qdev Clock model
  hw/riscv: Allow 64 bit access to SiFive CLINT
  target/riscv: Regen floating point rounding mode in dynamic mode

 include/hw/char/ibex_uart.h |  2 ++
 hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
 hw/riscv/sifive_clint.c     |  2 +-
 target/riscv/translate.c    |  2 +-
 4 files changed, 22 insertions(+), 3 deletions(-)

-- 
2.27.0



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
  2020-06-30 20:12 ` Alistair Francis
@ 2020-06-30 20:12   ` Alistair Francis
  -1 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, palmer, alistair23

Conver the Ibex UART to use the recently added qdev-clock functions.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/char/ibex_uart.h |  2 ++
 hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
index 2bec772615..322bfffd8b 100644
--- a/include/hw/char/ibex_uart.h
+++ b/include/hw/char/ibex_uart.h
@@ -101,6 +101,8 @@ typedef struct {
     uint32_t uart_val;
     uint32_t uart_timeout_ctrl;
 
+    Clock *f_clk;
+
     CharBackend chr;
     qemu_irq tx_watermark;
     qemu_irq rx_watermark;
diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index 45cd724998..f967e6919a 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -28,6 +28,7 @@
 #include "qemu/osdep.h"
 #include "hw/char/ibex_uart.h"
 #include "hw/irq.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
@@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
         }
         if (value & UART_CTRL_NCO) {
             uint64_t baud = ((value & UART_CTRL_NCO) >> 16);
-            baud *= 1000;
+            baud *= clock_get_hz(s->f_clk);
             baud >>= 20;
 
             s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
@@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
     }
 }
 
+static void ibex_uart_clk_update(void *opaque)
+{
+    IbexUartState *s = opaque;
+
+    /* recompute uart's speed on clock change */
+    uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
+    baud *= clock_get_hz(s->f_clk);
+    baud >>= 20;
+
+    s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
+}
+
 static void fifo_trigger_update(void *opaque)
 {
     IbexUartState *s = opaque;
@@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj)
 {
     IbexUartState *s = IBEX_UART(obj);
 
+    s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
+                                  ibex_uart_clk_update, s);
+    clock_set_hz(s->f_clk, 50000000);
+
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
@ 2020-06-30 20:12   ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: palmer, alistair.francis, alistair23

Conver the Ibex UART to use the recently added qdev-clock functions.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 include/hw/char/ibex_uart.h |  2 ++
 hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
index 2bec772615..322bfffd8b 100644
--- a/include/hw/char/ibex_uart.h
+++ b/include/hw/char/ibex_uart.h
@@ -101,6 +101,8 @@ typedef struct {
     uint32_t uart_val;
     uint32_t uart_timeout_ctrl;
 
+    Clock *f_clk;
+
     CharBackend chr;
     qemu_irq tx_watermark;
     qemu_irq rx_watermark;
diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
index 45cd724998..f967e6919a 100644
--- a/hw/char/ibex_uart.c
+++ b/hw/char/ibex_uart.c
@@ -28,6 +28,7 @@
 #include "qemu/osdep.h"
 #include "hw/char/ibex_uart.h"
 #include "hw/irq.h"
+#include "hw/qdev-clock.h"
 #include "hw/qdev-properties.h"
 #include "migration/vmstate.h"
 #include "qemu/log.h"
@@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
         }
         if (value & UART_CTRL_NCO) {
             uint64_t baud = ((value & UART_CTRL_NCO) >> 16);
-            baud *= 1000;
+            baud *= clock_get_hz(s->f_clk);
             baud >>= 20;
 
             s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
@@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
     }
 }
 
+static void ibex_uart_clk_update(void *opaque)
+{
+    IbexUartState *s = opaque;
+
+    /* recompute uart's speed on clock change */
+    uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
+    baud *= clock_get_hz(s->f_clk);
+    baud >>= 20;
+
+    s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
+}
+
 static void fifo_trigger_update(void *opaque)
 {
     IbexUartState *s = opaque;
@@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj)
 {
     IbexUartState *s = IBEX_UART(obj);
 
+    s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
+                                  ibex_uart_clk_update, s);
+    clock_set_hz(s->f_clk, 50000000);
+
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark);
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty);
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
  2020-06-30 20:12 ` Alistair Francis
@ 2020-06-30 20:12   ` Alistair Francis
  -1 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, palmer, alistair23

Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
"memory: Revert "memory: accept mismatching sizes in
memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
accesses to the CLINT and QEMU would trigger a fault. Fix this failure
by allowing 8 byte accesses.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_clint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index b11ffa0edc..669c21adc2 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
-        .max_access_size = 4
+        .max_access_size = 8
     }
 };
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
@ 2020-06-30 20:12   ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: palmer, alistair.francis, alistair23

Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
"memory: Revert "memory: accept mismatching sizes in
memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
accesses to the CLINT and QEMU would trigger a fault. Fix this failure
by allowing 8 byte accesses.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 hw/riscv/sifive_clint.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index b11ffa0edc..669c21adc2 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
     .endianness = DEVICE_LITTLE_ENDIAN,
     .valid = {
         .min_access_size = 4,
-        .max_access_size = 4
+        .max_access_size = 8
     }
 };
 
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
  2020-06-30 20:12 ` Alistair Francis
@ 2020-06-30 20:12   ` Alistair Francis
  -1 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: alistair.francis, palmer, alistair23

When a guest specificies the the rounding mode should be dynamic 0b111
then we want to re-caclulate the rounding mode on each instruction. The
gen_helper_set_rounding_mode() function will correctly check the
rounding mode and handle a dynamic rounding, we just need to make sure
it's always called if dynamic rounding is selected.

Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index ce71ca7a92..a39eba679a 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -490,7 +490,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
 {
     TCGv_i32 t0;
 
-    if (ctx->frm == rm) {
+    if (ctx->frm == rm && rm != 7) {
         return;
     }
     ctx->frm = rm;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
@ 2020-06-30 20:12   ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-06-30 20:12 UTC (permalink / raw)
  To: qemu-devel, qemu-riscv; +Cc: palmer, alistair.francis, alistair23

When a guest specificies the the rounding mode should be dynamic 0b111
then we want to re-caclulate the rounding mode on each instruction. The
gen_helper_set_rounding_mode() function will correctly check the
rounding mode and handle a dynamic rounding, we just need to make sure
it's always called if dynamic rounding is selected.

Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index ce71ca7a92..a39eba679a 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -490,7 +490,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
 {
     TCGv_i32 t0;
 
-    if (ctx->frm == rm) {
+    if (ctx->frm == rm && rm != 7) {
         return;
     }
     ctx->frm = rm;
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
  2020-06-30 20:12   ` Alistair Francis
@ 2020-06-30 21:51     ` Richard Henderson
  -1 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2020-06-30 21:51 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, palmer

On 6/30/20 1:12 PM, Alistair Francis wrote:
> When a guest specificies the the rounding mode should be dynamic 0b111
> then we want to re-caclulate the rounding mode on each instruction. The
> gen_helper_set_rounding_mode() function will correctly check the
> rounding mode and handle a dynamic rounding, we just need to make sure
> it's always called if dynamic rounding is selected.
> 
> Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index ce71ca7a92..a39eba679a 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -490,7 +490,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
>  {
>      TCGv_i32 t0;
>  
> -    if (ctx->frm == rm) {
> +    if (ctx->frm == rm && rm != 7) {
>          return;

This should not be necessary.

It was my understanding that after the set to the csr, that we would end the
TB.  That's certainly what I see in RISCV_OP_CSR_POST.

The next TB will begin wiht ctx->frm = -1, so we will reset the rounding mode
with 7.  It would be good to understand what's really going on here.


r~


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
@ 2020-06-30 21:51     ` Richard Henderson
  0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2020-06-30 21:51 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: palmer, alistair23

On 6/30/20 1:12 PM, Alistair Francis wrote:
> When a guest specificies the the rounding mode should be dynamic 0b111
> then we want to re-caclulate the rounding mode on each instruction. The
> gen_helper_set_rounding_mode() function will correctly check the
> rounding mode and handle a dynamic rounding, we just need to make sure
> it's always called if dynamic rounding is selected.
> 
> Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index ce71ca7a92..a39eba679a 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -490,7 +490,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
>  {
>      TCGv_i32 t0;
>  
> -    if (ctx->frm == rm) {
> +    if (ctx->frm == rm && rm != 7) {
>          return;

This should not be necessary.

It was my understanding that after the set to the csr, that we would end the
TB.  That's certainly what I see in RISCV_OP_CSR_POST.

The next TB will begin wiht ctx->frm = -1, so we will reset the rounding mode
with 7.  It would be good to understand what's really going on here.


r~


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
  2020-06-30 21:51     ` Richard Henderson
  (?)
@ 2020-07-01  0:17     ` LIU Zhiwei
  -1 siblings, 0 replies; 27+ messages in thread
From: LIU Zhiwei @ 2020-07-01  0:17 UTC (permalink / raw)
  To: Richard Henderson, Alistair Francis, qemu-devel, qemu-riscv
  Cc: alistair23, palmer



On 2020/7/1 5:51, Richard Henderson wrote:
> On 6/30/20 1:12 PM, Alistair Francis wrote:
>> When a guest specificies the the rounding mode should be dynamic 0b111
>> then we want to re-caclulate the rounding mode on each instruction. The
>> gen_helper_set_rounding_mode() function will correctly check the
>> rounding mode and handle a dynamic rounding, we just need to make sure
>> it's always called if dynamic rounding is selected.
>>
>> Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
>> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
>> ---
>>   target/riscv/translate.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
>> index ce71ca7a92..a39eba679a 100644
>> --- a/target/riscv/translate.c
>> +++ b/target/riscv/translate.c
>> @@ -490,7 +490,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
>>   {
>>       TCGv_i32 t0;
>>   
>> -    if (ctx->frm == rm) {
>> +    if (ctx->frm == rm && rm != 7) {
>>           return;
> This should not be necessary.
>
> It was my understanding that after the set to the csr, that we would end the
> TB.  That's certainly what I see in RISCV_OP_CSR_POST.
>
> The next TB will begin wiht ctx->frm = -1, so we will reset the rounding mode
> with 7.  It would be good to understand what's really going on here.
Agree. I think the 'bug '  is false positive.
Although the round mode process code is  confusing, it it right.

Zhiwei
>
> r~



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
  2020-06-30 20:12   ` Alistair Francis
@ 2020-07-01  0:19     ` LIU Zhiwei
  -1 siblings, 0 replies; 27+ messages in thread
From: LIU Zhiwei @ 2020-07-01  0:19 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, palmer



On 2020/7/1 4:12, Alistair Francis wrote:
> Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> "memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> by allowing 8 byte accesses.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   hw/riscv/sifive_clint.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> index b11ffa0edc..669c21adc2 100644
> --- a/hw/riscv/sifive_clint.c
> +++ b/hw/riscv/sifive_clint.c
> @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .valid = {
>           .min_access_size = 4,
> -        .max_access_size = 4
> +        .max_access_size = 8
>       }
>   };
>   

Reviewed-by: LIU Zhiwei<zhiwei_liu@c-sky.com>




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
@ 2020-07-01  0:19     ` LIU Zhiwei
  0 siblings, 0 replies; 27+ messages in thread
From: LIU Zhiwei @ 2020-07-01  0:19 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: palmer, alistair23



On 2020/7/1 4:12, Alistair Francis wrote:
> Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> "memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> by allowing 8 byte accesses.
>
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>   hw/riscv/sifive_clint.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> index b11ffa0edc..669c21adc2 100644
> --- a/hw/riscv/sifive_clint.c
> +++ b/hw/riscv/sifive_clint.c
> @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
>       .endianness = DEVICE_LITTLE_ENDIAN,
>       .valid = {
>           .min_access_size = 4,
> -        .max_access_size = 4
> +        .max_access_size = 8
>       }
>   };
>   

Reviewed-by: LIU Zhiwei<zhiwei_liu@c-sky.com>




^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
  2020-07-01  0:19     ` LIU Zhiwei
@ 2020-07-02  3:33       ` Alistair Francis
  -1 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-07-02  3:33 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: open list:RISC-V, Palmer Dabbelt, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Tue, Jun 30, 2020 at 5:19 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
>
>
> On 2020/7/1 4:12, Alistair Francis wrote:
> > Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> > "memory: Revert "memory: accept mismatching sizes in
> > memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> > accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> > by allowing 8 byte accesses.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >   hw/riscv/sifive_clint.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> > index b11ffa0edc..669c21adc2 100644
> > --- a/hw/riscv/sifive_clint.c
> > +++ b/hw/riscv/sifive_clint.c
> > @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
> >       .endianness = DEVICE_LITTLE_ENDIAN,
> >       .valid = {
> >           .min_access_size = 4,
> > -        .max_access_size = 4
> > +        .max_access_size = 8
> >       }
> >   };
> >
>
> Reviewed-by: LIU Zhiwei<zhiwei_liu@c-sky.com>

Thanks for the review. As most RISC-V machines are broken without this
patch I have applied it to my next PR.

Alistair

>
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
@ 2020-07-02  3:33       ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-07-02  3:33 UTC (permalink / raw)
  To: LIU Zhiwei
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Palmer Dabbelt

On Tue, Jun 30, 2020 at 5:19 PM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
>
>
> On 2020/7/1 4:12, Alistair Francis wrote:
> > Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> > "memory: Revert "memory: accept mismatching sizes in
> > memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> > accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> > by allowing 8 byte accesses.
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >   hw/riscv/sifive_clint.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> > index b11ffa0edc..669c21adc2 100644
> > --- a/hw/riscv/sifive_clint.c
> > +++ b/hw/riscv/sifive_clint.c
> > @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
> >       .endianness = DEVICE_LITTLE_ENDIAN,
> >       .valid = {
> >           .min_access_size = 4,
> > -        .max_access_size = 4
> > +        .max_access_size = 8
> >       }
> >   };
> >
>
> Reviewed-by: LIU Zhiwei<zhiwei_liu@c-sky.com>

Thanks for the review. As most RISC-V machines are broken without this
patch I have applied it to my next PR.

Alistair

>
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
  2020-06-30 20:12   ` Alistair Francis
@ 2020-07-03  1:24     ` Bin Meng
  -1 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2020-07-03  1:24 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Palmer Dabbelt, open list:RISC-V,
	qemu-devel@nongnu.org Developers, Alistair Francis

On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> When a guest specificies the the rounding mode should be dynamic 0b111
> then we want to re-caclulate the rounding mode on each instruction. The
> gen_helper_set_rounding_mode() function will correctly check the
> rounding mode and handle a dynamic rounding, we just need to make sure
> it's always called if dynamic rounding is selected.
>
> Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")

I can't find this commit id.

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Regards,
Bin


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
@ 2020-07-03  1:24     ` Bin Meng
  0 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2020-07-03  1:24 UTC (permalink / raw)
  To: Alistair Francis
  Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
	Palmer Dabbelt, Alistair Francis

On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis
<alistair.francis@wdc.com> wrote:
>
> When a guest specificies the the rounding mode should be dynamic 0b111
> then we want to re-caclulate the rounding mode on each instruction. The
> gen_helper_set_rounding_mode() function will correctly check the
> rounding mode and handle a dynamic rounding, we just need to make sure
> it's always called if dynamic rounding is selected.
>
> Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")

I can't find this commit id.

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  target/riscv/translate.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Regards,
Bin


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
  2020-06-30 20:12   ` Alistair Francis
@ 2020-07-03  7:42     ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-03  7:42 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: Damien Hedde, alistair23, palmer

+Damien

On 6/30/20 10:12 PM, Alistair Francis wrote:
> Conver the Ibex UART to use the recently added qdev-clock functions.

Yeah! This is our first user \o/

> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/char/ibex_uart.h |  2 ++
>  hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
> index 2bec772615..322bfffd8b 100644
> --- a/include/hw/char/ibex_uart.h
> +++ b/include/hw/char/ibex_uart.h
> @@ -101,6 +101,8 @@ typedef struct {
>      uint32_t uart_val;
>      uint32_t uart_timeout_ctrl;
>  
> +    Clock *f_clk;
> +
>      CharBackend chr;
>      qemu_irq tx_watermark;
>      qemu_irq rx_watermark;
> diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> index 45cd724998..f967e6919a 100644
> --- a/hw/char/ibex_uart.c
> +++ b/hw/char/ibex_uart.c
> @@ -28,6 +28,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/char/ibex_uart.h"
>  #include "hw/irq.h"
> +#include "hw/qdev-clock.h"
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
>  #include "qemu/log.h"
> @@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
>          }
>          if (value & UART_CTRL_NCO) {
>              uint64_t baud = ((value & UART_CTRL_NCO) >> 16);

UART_CTRL_NCO is defined as:

  #define UART_CTRL_NCO           (0xFFFF << 16)

Note for later, convert to the clearer registerfields API?

> -            baud *= 1000;
> +            baud *= clock_get_hz(s->f_clk);
>              baud >>= 20;
>  
>              s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> @@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
>      }
>  }
>  
> +static void ibex_uart_clk_update(void *opaque)
> +{
> +    IbexUartState *s = opaque;
> +
> +    /* recompute uart's speed on clock change */
> +    uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
> +    baud *= clock_get_hz(s->f_clk);
> +    baud >>= 20;

Maybe worth to extract:

  uint64_t ibex_uart_get_baud(IbexUartState *s)
  {
       uint64_t baud;

       baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
       baud *= clock_get_hz(s->f_clk);
       baud >>= 20;

       return baud;
  }

> +
> +    s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> +}
> +
>  static void fifo_trigger_update(void *opaque)
>  {
>      IbexUartState *s = opaque;
> @@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj)
>  {
>      IbexUartState *s = IBEX_UART(obj);
>  
> +    s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
> +                                  ibex_uart_clk_update, s);
> +    clock_set_hz(s->f_clk, 50000000);

Can you add a definition for this 50 MHz value:

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark);
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark);
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty);
> 



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
@ 2020-07-03  7:42     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-03  7:42 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: palmer, alistair23, Damien Hedde

+Damien

On 6/30/20 10:12 PM, Alistair Francis wrote:
> Conver the Ibex UART to use the recently added qdev-clock functions.

Yeah! This is our first user \o/

> 
> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  include/hw/char/ibex_uart.h |  2 ++
>  hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
>  2 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
> index 2bec772615..322bfffd8b 100644
> --- a/include/hw/char/ibex_uart.h
> +++ b/include/hw/char/ibex_uart.h
> @@ -101,6 +101,8 @@ typedef struct {
>      uint32_t uart_val;
>      uint32_t uart_timeout_ctrl;
>  
> +    Clock *f_clk;
> +
>      CharBackend chr;
>      qemu_irq tx_watermark;
>      qemu_irq rx_watermark;
> diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> index 45cd724998..f967e6919a 100644
> --- a/hw/char/ibex_uart.c
> +++ b/hw/char/ibex_uart.c
> @@ -28,6 +28,7 @@
>  #include "qemu/osdep.h"
>  #include "hw/char/ibex_uart.h"
>  #include "hw/irq.h"
> +#include "hw/qdev-clock.h"
>  #include "hw/qdev-properties.h"
>  #include "migration/vmstate.h"
>  #include "qemu/log.h"
> @@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
>          }
>          if (value & UART_CTRL_NCO) {
>              uint64_t baud = ((value & UART_CTRL_NCO) >> 16);

UART_CTRL_NCO is defined as:

  #define UART_CTRL_NCO           (0xFFFF << 16)

Note for later, convert to the clearer registerfields API?

> -            baud *= 1000;
> +            baud *= clock_get_hz(s->f_clk);
>              baud >>= 20;
>  
>              s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> @@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
>      }
>  }
>  
> +static void ibex_uart_clk_update(void *opaque)
> +{
> +    IbexUartState *s = opaque;
> +
> +    /* recompute uart's speed on clock change */
> +    uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
> +    baud *= clock_get_hz(s->f_clk);
> +    baud >>= 20;

Maybe worth to extract:

  uint64_t ibex_uart_get_baud(IbexUartState *s)
  {
       uint64_t baud;

       baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
       baud *= clock_get_hz(s->f_clk);
       baud >>= 20;

       return baud;
  }

> +
> +    s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> +}
> +
>  static void fifo_trigger_update(void *opaque)
>  {
>      IbexUartState *s = opaque;
> @@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj)
>  {
>      IbexUartState *s = IBEX_UART(obj);
>  
> +    s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
> +                                  ibex_uart_clk_update, s);
> +    clock_set_hz(s->f_clk, 50000000);

Can you add a definition for this 50 MHz value:

Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark);
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark);
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty);
> 



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
  2020-06-30 20:12   ` Alistair Francis
@ 2020-07-03  7:44     ` Philippe Mathieu-Daudé
  -1 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-03  7:44 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: alistair23, palmer

On 6/30/20 10:12 PM, Alistair Francis wrote:
> Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> "memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> by allowing 8 byte accesses.
> 

Fixes: 5d971f9e67 (Revert "accept mismatching sizes in access_valid")
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/sifive_clint.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> index b11ffa0edc..669c21adc2 100644
> --- a/hw/riscv/sifive_clint.c
> +++ b/hw/riscv/sifive_clint.c
> @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> +        .max_access_size = 8
>      }
>  };
>  
> 



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT
@ 2020-07-03  7:44     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-07-03  7:44 UTC (permalink / raw)
  To: Alistair Francis, qemu-devel, qemu-riscv; +Cc: palmer, alistair23

On 6/30/20 10:12 PM, Alistair Francis wrote:
> Commit 5d971f9e672507210e77d020d89e0e89165c8fc9
> "memory: Revert "memory: accept mismatching sizes in
> memory_region_access_valid"" broke most RISC-V boards as they do 64 bit
> accesses to the CLINT and QEMU would trigger a fault. Fix this failure
> by allowing 8 byte accesses.
> 

Fixes: 5d971f9e67 (Revert "accept mismatching sizes in access_valid")
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> ---
>  hw/riscv/sifive_clint.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
> index b11ffa0edc..669c21adc2 100644
> --- a/hw/riscv/sifive_clint.c
> +++ b/hw/riscv/sifive_clint.c
> @@ -181,7 +181,7 @@ static const MemoryRegionOps sifive_clint_ops = {
>      .endianness = DEVICE_LITTLE_ENDIAN,
>      .valid = {
>          .min_access_size = 4,
> -        .max_access_size = 4
> +        .max_access_size = 8
>      }
>  };
>  
> 



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
  2020-07-03  7:42     ` Philippe Mathieu-Daudé
@ 2020-07-07 17:20       ` Alistair Francis
  -1 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-07-07 17:20 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Damien Hedde, open list:RISC-V, Palmer Dabbelt, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Fri, Jul 3, 2020 at 12:42 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> +Damien
>
> On 6/30/20 10:12 PM, Alistair Francis wrote:
> > Conver the Ibex UART to use the recently added qdev-clock functions.
>
> Yeah! This is our first user \o/

:)

>
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/hw/char/ibex_uart.h |  2 ++
> >  hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
> >  2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
> > index 2bec772615..322bfffd8b 100644
> > --- a/include/hw/char/ibex_uart.h
> > +++ b/include/hw/char/ibex_uart.h
> > @@ -101,6 +101,8 @@ typedef struct {
> >      uint32_t uart_val;
> >      uint32_t uart_timeout_ctrl;
> >
> > +    Clock *f_clk;
> > +
> >      CharBackend chr;
> >      qemu_irq tx_watermark;
> >      qemu_irq rx_watermark;
> > diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> > index 45cd724998..f967e6919a 100644
> > --- a/hw/char/ibex_uart.c
> > +++ b/hw/char/ibex_uart.c
> > @@ -28,6 +28,7 @@
> >  #include "qemu/osdep.h"
> >  #include "hw/char/ibex_uart.h"
> >  #include "hw/irq.h"
> > +#include "hw/qdev-clock.h"
> >  #include "hw/qdev-properties.h"
> >  #include "migration/vmstate.h"
> >  #include "qemu/log.h"
> > @@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
> >          }
> >          if (value & UART_CTRL_NCO) {
> >              uint64_t baud = ((value & UART_CTRL_NCO) >> 16);
>
> UART_CTRL_NCO is defined as:
>
>   #define UART_CTRL_NCO           (0xFFFF << 16)
>
> Note for later, convert to the clearer registerfields API?

Done, I have added a patch to do this.

>
> > -            baud *= 1000;
> > +            baud *= clock_get_hz(s->f_clk);
> >              baud >>= 20;
> >
> >              s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> > @@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
> >      }
> >  }
> >
> > +static void ibex_uart_clk_update(void *opaque)
> > +{
> > +    IbexUartState *s = opaque;
> > +
> > +    /* recompute uart's speed on clock change */
> > +    uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
> > +    baud *= clock_get_hz(s->f_clk);
> > +    baud >>= 20;
>
> Maybe worth to extract:
>
>   uint64_t ibex_uart_get_baud(IbexUartState *s)
>   {
>        uint64_t baud;
>
>        baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
>        baud *= clock_get_hz(s->f_clk);
>        baud >>= 20;
>
>        return baud;
>   }

Done.

>
> > +
> > +    s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> > +}
> > +
> >  static void fifo_trigger_update(void *opaque)
> >  {
> >      IbexUartState *s = opaque;
> > @@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj)
> >  {
> >      IbexUartState *s = IBEX_UART(obj);
> >
> > +    s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
> > +                                  ibex_uart_clk_update, s);
> > +    clock_set_hz(s->f_clk, 50000000);
>
> Can you add a definition for this 50 MHz value:

Done.

>
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks!

Alistair

>
> > +
> >      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark);
> >      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark);
> >      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty);
> >
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model
@ 2020-07-07 17:20       ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-07-07 17:20 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Palmer Dabbelt, Damien Hedde

On Fri, Jul 3, 2020 at 12:42 AM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> +Damien
>
> On 6/30/20 10:12 PM, Alistair Francis wrote:
> > Conver the Ibex UART to use the recently added qdev-clock functions.
>
> Yeah! This is our first user \o/

:)

>
> >
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  include/hw/char/ibex_uart.h |  2 ++
> >  hw/char/ibex_uart.c         | 19 ++++++++++++++++++-
> >  2 files changed, 20 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/char/ibex_uart.h b/include/hw/char/ibex_uart.h
> > index 2bec772615..322bfffd8b 100644
> > --- a/include/hw/char/ibex_uart.h
> > +++ b/include/hw/char/ibex_uart.h
> > @@ -101,6 +101,8 @@ typedef struct {
> >      uint32_t uart_val;
> >      uint32_t uart_timeout_ctrl;
> >
> > +    Clock *f_clk;
> > +
> >      CharBackend chr;
> >      qemu_irq tx_watermark;
> >      qemu_irq rx_watermark;
> > diff --git a/hw/char/ibex_uart.c b/hw/char/ibex_uart.c
> > index 45cd724998..f967e6919a 100644
> > --- a/hw/char/ibex_uart.c
> > +++ b/hw/char/ibex_uart.c
> > @@ -28,6 +28,7 @@
> >  #include "qemu/osdep.h"
> >  #include "hw/char/ibex_uart.h"
> >  #include "hw/irq.h"
> > +#include "hw/qdev-clock.h"
> >  #include "hw/qdev-properties.h"
> >  #include "migration/vmstate.h"
> >  #include "qemu/log.h"
> > @@ -330,7 +331,7 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
> >          }
> >          if (value & UART_CTRL_NCO) {
> >              uint64_t baud = ((value & UART_CTRL_NCO) >> 16);
>
> UART_CTRL_NCO is defined as:
>
>   #define UART_CTRL_NCO           (0xFFFF << 16)
>
> Note for later, convert to the clearer registerfields API?

Done, I have added a patch to do this.

>
> > -            baud *= 1000;
> > +            baud *= clock_get_hz(s->f_clk);
> >              baud >>= 20;
> >
> >              s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> > @@ -385,6 +386,18 @@ static void ibex_uart_write(void *opaque, hwaddr addr,
> >      }
> >  }
> >
> > +static void ibex_uart_clk_update(void *opaque)
> > +{
> > +    IbexUartState *s = opaque;
> > +
> > +    /* recompute uart's speed on clock change */
> > +    uint64_t baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
> > +    baud *= clock_get_hz(s->f_clk);
> > +    baud >>= 20;
>
> Maybe worth to extract:
>
>   uint64_t ibex_uart_get_baud(IbexUartState *s)
>   {
>        uint64_t baud;
>
>        baud = ((s->uart_ctrl & UART_CTRL_NCO) >> 16);
>        baud *= clock_get_hz(s->f_clk);
>        baud >>= 20;
>
>        return baud;
>   }

Done.

>
> > +
> > +    s->char_tx_time = (NANOSECONDS_PER_SECOND / baud) * 10;
> > +}
> > +
> >  static void fifo_trigger_update(void *opaque)
> >  {
> >      IbexUartState *s = opaque;
> > @@ -444,6 +457,10 @@ static void ibex_uart_init(Object *obj)
> >  {
> >      IbexUartState *s = IBEX_UART(obj);
> >
> > +    s->f_clk = qdev_init_clock_in(DEVICE(obj), "f_clock",
> > +                                  ibex_uart_clk_update, s);
> > +    clock_set_hz(s->f_clk, 50000000);
>
> Can you add a definition for this 50 MHz value:

Done.

>
> Otherwise:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Thanks!

Alistair

>
> > +
> >      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_watermark);
> >      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->rx_watermark);
> >      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->tx_empty);
> >
>


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
  2020-07-03  1:24     ` Bin Meng
@ 2020-07-07 17:23       ` Alistair Francis
  -1 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-07-07 17:23 UTC (permalink / raw)
  To: Bin Meng
  Cc: open list:RISC-V, Palmer Dabbelt, Alistair Francis,
	qemu-devel@nongnu.org Developers

On Thu, Jul 2, 2020 at 6:25 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > When a guest specificies the the rounding mode should be dynamic 0b111
> > then we want to re-caclulate the rounding mode on each instruction. The
> > gen_helper_set_rounding_mode() function will correctly check the
> > rounding mode and handle a dynamic rounding, we just need to make sure
> > it's always called if dynamic rounding is selected.
> >
> > Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
>
> I can't find this commit id.

It's a launchpad bug case: https://bugs.launchpad.net/qemu/+bug/1885350

Alistair

>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/translate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Regards,
> Bin


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
@ 2020-07-07 17:23       ` Alistair Francis
  0 siblings, 0 replies; 27+ messages in thread
From: Alistair Francis @ 2020-07-07 17:23 UTC (permalink / raw)
  To: Bin Meng
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Palmer Dabbelt

On Thu, Jul 2, 2020 at 6:25 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis
> <alistair.francis@wdc.com> wrote:
> >
> > When a guest specificies the the rounding mode should be dynamic 0b111
> > then we want to re-caclulate the rounding mode on each instruction. The
> > gen_helper_set_rounding_mode() function will correctly check the
> > rounding mode and handle a dynamic rounding, we just need to make sure
> > it's always called if dynamic rounding is selected.
> >
> > Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
>
> I can't find this commit id.

It's a launchpad bug case: https://bugs.launchpad.net/qemu/+bug/1885350

Alistair

>
> > Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
> > ---
> >  target/riscv/translate.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Regards,
> Bin


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
  2020-07-07 17:23       ` Alistair Francis
@ 2020-07-08  0:36         ` Bin Meng
  -1 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2020-07-08  0:36 UTC (permalink / raw)
  To: Alistair Francis
  Cc: open list:RISC-V, Palmer Dabbelt, Alistair Francis,
	qemu-devel@nongnu.org Developers

Hi Alistair,

On Wed, Jul 8, 2020 at 1:33 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Jul 2, 2020 at 6:25 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> > >
> > > When a guest specificies the the rounding mode should be dynamic 0b111
> > > then we want to re-caclulate the rounding mode on each instruction. The
> > > gen_helper_set_rounding_mode() function will correctly check the
> > > rounding mode and handle a dynamic rounding, we just need to make sure
> > > it's always called if dynamic rounding is selected.
> > >
> > > Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
> >
> > I can't find this commit id.
>
> It's a launchpad bug case: https://bugs.launchpad.net/qemu/+bug/1885350
>

My understanding is that the convention used in "Fixes" tag is for
commit id. 1885350 is not a commit id but a QEMU bug report id.

Regards,
Bin


^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode
@ 2020-07-08  0:36         ` Bin Meng
  0 siblings, 0 replies; 27+ messages in thread
From: Bin Meng @ 2020-07-08  0:36 UTC (permalink / raw)
  To: Alistair Francis
  Cc: Alistair Francis, qemu-devel@nongnu.org Developers,
	open list:RISC-V, Palmer Dabbelt

Hi Alistair,

On Wed, Jul 8, 2020 at 1:33 AM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Thu, Jul 2, 2020 at 6:25 PM Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > On Wed, Jul 1, 2020 at 4:23 AM Alistair Francis
> > <alistair.francis@wdc.com> wrote:
> > >
> > > When a guest specificies the the rounding mode should be dynamic 0b111
> > > then we want to re-caclulate the rounding mode on each instruction. The
> > > gen_helper_set_rounding_mode() function will correctly check the
> > > rounding mode and handle a dynamic rounding, we just need to make sure
> > > it's always called if dynamic rounding is selected.
> > >
> > > Fixes: 1885350 ("RISCV dynamic rounding mode is not behaving correctly")
> >
> > I can't find this commit id.
>
> It's a launchpad bug case: https://bugs.launchpad.net/qemu/+bug/1885350
>

My understanding is that the convention used in "Fixes" tag is for
commit id. 1885350 is not a commit id but a QEMU bug report id.

Regards,
Bin


^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2020-07-08  0:37 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30 20:12 [PATCH v1 0/3] A few RISC-V fixes Alistair Francis
2020-06-30 20:12 ` Alistair Francis
2020-06-30 20:12 ` [PATCH v1 1/3] hw/char: Convert the Ibex UART to use the qdev Clock model Alistair Francis
2020-06-30 20:12   ` Alistair Francis
2020-07-03  7:42   ` Philippe Mathieu-Daudé
2020-07-03  7:42     ` Philippe Mathieu-Daudé
2020-07-07 17:20     ` Alistair Francis
2020-07-07 17:20       ` Alistair Francis
2020-06-30 20:12 ` [PATCH v1 2/3] hw/riscv: Allow 64 bit access to SiFive CLINT Alistair Francis
2020-06-30 20:12   ` Alistair Francis
2020-07-01  0:19   ` LIU Zhiwei
2020-07-01  0:19     ` LIU Zhiwei
2020-07-02  3:33     ` Alistair Francis
2020-07-02  3:33       ` Alistair Francis
2020-07-03  7:44   ` Philippe Mathieu-Daudé
2020-07-03  7:44     ` Philippe Mathieu-Daudé
2020-06-30 20:12 ` [PATCH v1 3/3] target/riscv: Regen floating point rounding mode in dynamic mode Alistair Francis
2020-06-30 20:12   ` Alistair Francis
2020-06-30 21:51   ` Richard Henderson
2020-06-30 21:51     ` Richard Henderson
2020-07-01  0:17     ` LIU Zhiwei
2020-07-03  1:24   ` Bin Meng
2020-07-03  1:24     ` Bin Meng
2020-07-07 17:23     ` Alistair Francis
2020-07-07 17:23       ` Alistair Francis
2020-07-08  0:36       ` Bin Meng
2020-07-08  0:36         ` Bin Meng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.