[PATCH] Fix a gap where acpi_pcihp_find_hotplug_bus() returns a non-hotpluggable bus

[Ani Sinha]

When ACPI hotplug for the root bus is disabled, the bsel property for that
bus is not set. Please see the following commit:

3d7e78aaf ("Introduce a new flag for i440fx to disable PCI hotplug on the 
root bus").

As a result, when acpi_pcihp_find_hotplug_bus() is called
with bsel set to 0, it may return the root bus. This would be wrong since the
root bus is not hotpluggable. In general, this can potentially happen to other
buses as well.
In this patch, we fix the issue in this function by checking if the bus returned
by the function is actually hotpluggable. If not, we simply return NULL. This
avoids the scenario where we are actually returning a non-hotpluggable bus.

Signed-off-by: Ani Sinha 
---
 hw/acpi/pcihp.c | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/hw/acpi/pcihp.c b/hw/acpi/pcihp.c
index 39b1f74442..f148e73c89 100644
--- a/hw/acpi/pcihp.c
+++ b/hw/acpi/pcihp.c
@@ -147,6 +147,21 @@ static PCIBus *acpi_pcihp_find_hotplug_bus(AcpiPciHpState 
*s, int bsel)
 if (!bsel && !find.bus) {
 find.bus = s->root;
 }
+
+/*
+ * Check if find.bus is actually hotpluggable. If bsel is set to
+ * NULL for example on the root bus in order to make it
+ * non-hotpluggable, find.bus will match the root bus when bsel
+ * is 0. See acpi_pcihp_test_hotplug_bus() above. Since the
+ * bus is not hotpluggable however, we should not select the bus.
+ * Instead, we should set find.bus to NULL in that case. In the check
+ * below, we generalize this case for all buses, not just the root bus.
+ * The callers of this function check for a null return value and
+ * handle them appropriately.
+ */
+if (!qbus_is_hotpluggable(BUS(find.bus))) {
+find.bus = NULL;
+}
 return find.bus;
 }
 
-- 
2.17.1

on msys2, the generated qemu-version.h seems not valid

[Yonggang Luo]

#define QEMU_PKGVERSION ""
#define QEMU_FULL_VERSION "5.1.50"


pkgversion=$(git describe --match 'v*' --dirty | echo "")
what's echo "" for in qemu-version.sh
-- 
 此致
礼
罗勇刚
Yours
sincerely,
Yonggang Luo

[PATCH] meson: fix SDL2_image detection

[Volker Rümelin]

Configure used to probe for SDL2_image in sdl_image_probe (). Meson
should do the same.

This fixes the following error on my system:

Run-time dependency sdl2 found: YES 2.0.8
Found CMake: /usr/bin/cmake (3.17.0)
Run-time dependency sdl-image found: NO (tried pkgconfig and cmake)

../qemu-master/meson.build:256:2: ERROR: Dependency "sdl-image" not
found, tried pkgconfig and cmake

A full log can be found at /home/ruemelin/rpmbuild/BUILD
/qemu-5.1.50-build/meson-logs/meson-log.txt

ERROR: meson setup failed

Signed-off-by: Volker Rümelin 
---
 meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index 74f8ea0c2e..8204878860 100644
--- a/meson.build
+++ b/meson.build
@@ -248,7 +248,7 @@ if sdl.found()
   # work around 2.0.8 bug
   sdl = declare_dependency(compile_args: '-Wno-undef',
dependencies: sdl)
-  sdl_image = dependency('sdl-image', required: get_option('sdl_image'),
+  sdl_image = dependency('SDL2_image', required: get_option('sdl_image'),
  static: enable_static)
 else
   if get_option('sdl_image').enabled()
-- 
2.26.2

[PATCH] target/xtensa: enable all coprocessors for linux-user

[Max Filippov]

Linux userspace always sees coprocessors as enabled. CPENABLE register
and coprocessor exceptions are used internally by the kernel to manage
lazy coprocessor context switch. None of it is needed for linux-user.
Always enable all coprocessors for user emulation.

Signed-off-by: Max Filippov 
---
 target/xtensa/cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/xtensa/cpu.c b/target/xtensa/cpu.c
index 6a033e778c95..88a32268a18b 100644
--- a/target/xtensa/cpu.c
+++ b/target/xtensa/cpu.c
@@ -93,6 +93,7 @@ static void xtensa_cpu_reset(DeviceState *dev)
 !xtensa_abi_call0()) {
 env->sregs[PS] |= PS_WOE;
 }
+env->sregs[CPENABLE] = 0xff;
 #endif
 env->sregs[VECBASE] = env->config->vecbase;
 env->sregs[IBREAKENABLE] = 0;
-- 
2.20.1

Re: [PATCH v2] meson: Convert undefsym.sh to undefsym.py

[Stefan Hajnoczi]

On Fri, Aug 28, 2020 at 5:18 PM 罗勇刚(Yonggang Luo)  wrote:
>
> I am confusing, why my patch are not recognized by Patchew?

I looked at the message headers in case git-publish corrupted the
email, but I didn't notice any issues.

Some of your other patches were detected correctly by Patchew:
https://patchew.org/search?q=project%3AQEMU+from%3Aluoyonggang%40gmail.com

Maybe Paolo can check what happened to this particular patch?

Stefan

[Bug 1893040] Re: External modules retreval using Go1.15 on s390x appears to have checksum and ECDSA verification issues

[Laurent Vivier]

** Tags added: linux-user

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1893040

Title:
   External modules retreval using Go1.15 on s390x appears to have
  checksum and ECDSA verification issues

Status in QEMU:
  New

Bug description:
  We are observing issue while building go-runner image and we suspect it is 
due to QEMU version being used. As referred in below issue:
  https://github.com/golang/go/issues/40949

  We tried to build go-runner image using go1.15 and register QEMU
  (docker run --rm --privileged multiarch/qemu-user-
  static@sha256:c772ee1965aa0be9915ee1b018a0dd92ea361b4fa1bcab5bbc033517749b2af4
  --reset -p yes) as mentioned in PR
  https://github.com/kubernetes/release/pull/1499. We observed below
  failure during build:

  
-
  ERROR: executor failed running [/bin/sh -c CGO_ENABLED=0 GOOS=linux 
GOARCH=${ARCH} go build -ldflags '-s -w -buildid= -extldflags "-static"'
 -o go-runner ${package}]: buildkit-runc did not terminate successfully
  --
   > [builder 7/7] RUN CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build 
-ldflags '-s -w -buildid= -extldflags "-static"' -o go-runner .:
  --
  failed to solve: rpc error: code = Unknown desc = executor failed running 
[/bin/sh -c CGO_ENABLED=0 GOOS=linux GOARCH=${ARCH} go build -ldflags '-s 
-w -buildid= -extldflags "-static"' -o go-runner ${package}]: buildkit-runc 
did not terminate successfully
  Makefile:52: recipe for target 'container' failed
  make: *** [container] Error 1
  
-

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1893040/+subscriptions

Re: [PATCH v2 43/45] target/arm/vec_helper: Add gvec fp indexed multiply-and-add operations

[Peter Maydell]

On Sat, 29 Aug 2020 at 00:24, Richard Henderson
 wrote:
>
> On 8/28/20 11:33 AM, Peter Maydell wrote:
> > +#define float16_nop(N, M, S) (M)
> > +#define float32_nop(N, M, S) (M)
> > +#define float64_nop(N, M, S) (M)
> >
> > +DO_FMUL_IDX(gvec_fmul_idx_h, nop, float16, H2)
> > +DO_FMUL_IDX(gvec_fmul_idx_s, nop, float32, H4)
> > +DO_FMUL_IDX(gvec_fmul_idx_d, nop, float64, )
> > +
> > +/*
> > + * Non-fused multiply-accumulate operations, for Neon. NB that unlike
> > + * the fused ops below they assume accumulate both from and into Vd.
> > + */
> > +DO_FMUL_IDX(gvec_fmla_nf_idx_h, add, float16, H2)
> > +DO_FMUL_IDX(gvec_fmla_nf_idx_s, add, float32, H4)
> > +DO_FMUL_IDX(gvec_fmls_nf_idx_h, sub, float16, H2)
> > +DO_FMUL_IDX(gvec_fmls_nf_idx_s, sub, float32, H4)
> > +
> > +#undef float16_nop
> > +#undef float32_nop
> > +#undef float64_nop
>
> This floatN_nop stuff is pretty ugly.
>
> Better to pass in either floatN_mul, or the floatN_muladd_nf helpers that you
> added earlier.  Although I guess you're missing float64_muladd_nf so far.

I thought about doing that, but the float*_muladd_nf functions
don't have the same signature as float*_mul -- they take
(dest, op1, op2, stat) and float*_mul only takes (op1, op2, stat) --
so it doesn't work. You'd have to construct a wrapper for
the mul function that took and ignored the dest argument,
or split out mul entirely into its own macro rather than
using DO_FMUL_IDX for mul and muladd. The nop macros seemed
the simplest.

thanks
-- PMM

Re: [PATCH v2 44/45] target/arm: Implement fp16 for Neon VMUL, VMLA, VMLS

[Peter Maydell]

On Sat, 29 Aug 2020 at 00:38, Richard Henderson
 wrote:
>
> On 8/28/20 11:33 AM, Peter Maydell wrote:
> > Convert the Neon floating-point VMUL, VMLA and VMLS to use gvec,
> > and use this to implement fp16 support.
> >
> > Signed-off-by: Peter Maydell 
>
> Reviewed-by: Richard Henderson 
>
> > +/* a->vm is M:Vm, which encodes both register and index */
> > +idx = extract32(a->vm, a->size + 2, 2);
> > +a->vm = extract32(a->vm, 0, a->size + 2);
>
> I know this is what the current code does, but I tend to think that this is
> better done in decode.

Yeah, I thought that too as I was writing it, but I didn't
want to mess with the decode in this patchset, especially
given it would have meant I needed to touch all the non-fp
scalar-indexed operations too...

-- PMM

Re: [PATCH v2 24/45] target/arm: Implement fp16 for Neon VRECPE, VRSQRTE using gvec

[Peter Maydell]

On Fri, 28 Aug 2020 at 23:53, Richard Henderson
 wrote:
>
> On 8/28/20 2:40 PM, Peter Maydell wrote:
> > The other approach would be to standardize on "the decodetree pattern
> > always converts the size to the data-type size, regardless of how
> > it's encoded in the insn fields", and then you could check against
> > MO_16 here. Would that be better ?
>
> That might be clearer, yes.  Otherwise it's hard to tell what "size" means
> without looking at the manual for each instance.

Do you mind if I do that as a separate patchset after this one?
I feel that will be easier than trying to weave the change into
this series...

-- PMM

[Bug 1893010] Re: qemu linux-user doesn't support OFD fcntl locks

[Laurent Vivier]

Please check qemu-5.1.0.

This has been fixed by:

  2d92c6827ca0 ("linux-user: implement OFD locks")
  https://git.qemu.org/?p=qemu.git;a=commitdiff;h=2d92c6827ca0

perhaps you can send a patch to the qemu-devel ML to add the strace
part.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1893010

Title:
  qemu linux-user doesn't support OFD fcntl locks

Status in QEMU:
  New

Bug description:
  "Open file description locks (non-POSIX)", as they are described in
  fcntl(2) man page, aren't supported by qemu-user  and attempting to
  use those results in EINVAL. I'm on Gentoo with latest QEMU version
  currently available (5.0.0-r2), and trying to emulate ppc64 and s390x
  on x86_64.

  Looking at linux-user/syscall.c, I'm guessing the issue is in (at
  least) `target_to_host_fcntl_cmd` where switch reaches the default
  clause as there're no cases for F_OFD_SETLK / F_OFD_SETLKW /
  F_OFD_GETLK.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1893010/+subscriptions

Re: [PATCH] softfloat: Define comparison operations for bfloat16

[LIU Zhiwei]

On 2020/8/29 1:53, Richard Henderson wrote:

These operations were missed in Zhiwei's bfloat16 implementation.

Signed-off-by: Richard Henderson 
---
  include/fpu/softfloat.h | 41 +
  1 file changed, 41 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 1233f98014..78ad5ca738 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -479,6 +479,47 @@ static inline bfloat16 bfloat16_set_sign(bfloat16 a, int 
sign)
  return (a & 0x7fff) | (sign << 15);
  }
  
+static inline bool bfloat16_eq(bfloat16 a, bfloat16 b, float_status *s)

+{
+return bfloat16_compare(a, b, s) == float_relation_equal;
+}
+
+static inline bool bfloat16_le(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) <= float_relation_equal;
+}
+
+static inline bool bfloat16_lt(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) < float_relation_equal;
+}
+
+static inline bool bfloat16_unordered(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) == float_relation_unordered;
+}
+
+static inline bool bfloat16_eq_quiet(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) == float_relation_equal;
+}
+
+static inline bool bfloat16_le_quiet(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) <= float_relation_equal;
+}
+
+static inline bool bfloat16_lt_quiet(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) < float_relation_equal;
+}
+
+static inline bool bfloat16_unordered_quiet(bfloat16 a, bfloat16 b,
+   float_status *s)

Indentation.

+{
+return bfloat16_compare_quiet(a, b, s) == float_relation_unordered;
+}
+

Hi Richard,

If you have already applied the bfloat16 patch set,   I am afraid you 
have to remove these lines.


-int bfloat16_unordered_quiet(bfloat16, bfloat16, float_status *status);
-int bfloat16_le(bfloat16, bfloat16, float_status *status);
-int bfloat16_lt(bfloat16, bfloat16, float_status *status);
-int bfloat16_eq_quiet(bfloat16, bfloat16, float_status *status);

The corresponding float16 interfaces have been removed in the master 
branch when I sent the bfloat16 patch set.

So I deleted the implementations. But I forgot to remove the declarations.

I see you have applied float16 comparison interfaces from Kito, and the 
corresponding bfloat16
interfaces have all been defined here. After remove the redundant 
declarations,


Reviewed-by: LIU Zhiwei 


  #define bfloat16_zero 0
  #define bfloat16_half 0x3f00
  #define bfloat16_one 0x3f80

[PATCH v2 03/16] target/riscv: cpu: Set reset vector based on the configured property value

[Bin Meng]

From: Bin Meng 

Now that we have the newly introduced 'resetvec' property in the
RISC-V CPU and HART, instead of hard-coding the reset vector addr
in the CPU's instance_init(), move that to riscv_cpu_realize()
based on the configured property value from the RISC-V machines.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 hw/riscv/opentitan.c | 1 +
 hw/riscv/sifive_e.c  | 1 +
 hw/riscv/sifive_u.c  | 2 ++
 target/riscv/cpu.c   | 7 ++-
 4 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 23ba3b4..0531bd8 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -111,6 +111,7 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc, 
Error **errp)
 &error_abort);
 object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
 &error_abort);
+object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x8090, 
&error_abort);
 sysbus_realize(SYS_BUS_DEVICE(&s->cpus), &error_abort);
 
 /* Boot ROM */
diff --git a/hw/riscv/sifive_e.c b/hw/riscv/sifive_e.c
index ca55cc4..cd7560d 100644
--- a/hw/riscv/sifive_e.c
+++ b/hw/riscv/sifive_e.c
@@ -177,6 +177,7 @@ static void sifive_e_soc_init(Object *obj)
 object_initialize_child(obj, "cpus", &s->cpus, TYPE_RISCV_HART_ARRAY);
 object_property_set_int(OBJECT(&s->cpus), "num-harts", ms->smp.cpus,
 &error_abort);
+object_property_set_int(OBJECT(&s->cpus), "resetvec", 0x1004, 
&error_abort);
 object_initialize_child(obj, "riscv.sifive.e.gpio0", &s->gpio,
 TYPE_SIFIVE_GPIO);
 }
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index a48046c..404d5e6 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -611,6 +611,7 @@ static void sifive_u_soc_instance_init(Object *obj)
 qdev_prop_set_uint32(DEVICE(&s->e_cpus), "num-harts", 1);
 qdev_prop_set_uint32(DEVICE(&s->e_cpus), "hartid-base", 0);
 qdev_prop_set_string(DEVICE(&s->e_cpus), "cpu-type", SIFIVE_E_CPU);
+qdev_prop_set_uint64(DEVICE(&s->e_cpus), "resetvec", 0x1004);
 
 object_initialize_child(obj, "u-cluster", &s->u_cluster, TYPE_CPU_CLUSTER);
 qdev_prop_set_uint32(DEVICE(&s->u_cluster), "cluster-id", 1);
@@ -620,6 +621,7 @@ static void sifive_u_soc_instance_init(Object *obj)
 qdev_prop_set_uint32(DEVICE(&s->u_cpus), "num-harts", ms->smp.cpus - 1);
 qdev_prop_set_uint32(DEVICE(&s->u_cpus), "hartid-base", 1);
 qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type", SIFIVE_U_CPU);
+qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", 0x1004);
 
 object_initialize_child(obj, "prci", &s->prci, TYPE_SIFIVE_U_PRCI);
 object_initialize_child(obj, "otp", &s->otp, TYPE_SIFIVE_U_OTP);
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 8067a26..bd41286 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -128,7 +128,6 @@ static void riscv_any_cpu_init(Object *obj)
 CPURISCVState *env = &RISCV_CPU(obj)->env;
 set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVU);
 set_priv_version(env, PRIV_VERSION_1_11_0);
-set_resetvec(env, DEFAULT_RSTVEC);
 }
 
 static void riscv_base_cpu_init(Object *obj)
@@ -136,7 +135,6 @@ static void riscv_base_cpu_init(Object *obj)
 CPURISCVState *env = &RISCV_CPU(obj)->env;
 /* We set this in the realise function */
 set_misa(env, 0);
-set_resetvec(env, DEFAULT_RSTVEC);
 }
 
 static void rvxx_sifive_u_cpu_init(Object *obj)
@@ -144,7 +142,6 @@ static void rvxx_sifive_u_cpu_init(Object *obj)
 CPURISCVState *env = &RISCV_CPU(obj)->env;
 set_misa(env, RVXLEN | RVI | RVM | RVA | RVF | RVD | RVC | RVS | RVU);
 set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, 0x1004);
 }
 
 static void rvxx_sifive_e_cpu_init(Object *obj)
@@ -152,7 +149,6 @@ static void rvxx_sifive_e_cpu_init(Object *obj)
 CPURISCVState *env = &RISCV_CPU(obj)->env;
 set_misa(env, RVXLEN | RVI | RVM | RVA | RVC | RVU);
 set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, 0x1004);
 qdev_prop_set_bit(DEVICE(obj), "mmu", false);
 }
 
@@ -163,7 +159,6 @@ static void rv32_ibex_cpu_init(Object *obj)
 CPURISCVState *env = &RISCV_CPU(obj)->env;
 set_misa(env, RV32 | RVI | RVM | RVC | RVU);
 set_priv_version(env, PRIV_VERSION_1_10_0);
-set_resetvec(env, 0x8090);
 qdev_prop_set_bit(DEVICE(obj), "mmu", false);
 }
 
@@ -373,6 +368,8 @@ static void riscv_cpu_realize(DeviceState *dev, Error 
**errp)
 set_feature(env, RISCV_FEATURE_PMP);
 }
 
+set_resetvec(env, cpu->cfg.resetvec);
+
 /* If misa isn't set (rv32 and rv64 machines) set it here */
 if (!env->misa) {
 /* Do some ISA extension error checking */
-- 
2.7.4

[PATCH v2 02/16] hw/riscv: hart: Add a new 'resetvec' property

[Bin Meng]

From: Bin Meng 

RISC-V machines do not instantiate RISC-V CPUs directly, instead
they do that via the hart array. Add a new property for the reset
vector address to allow the value to be passed to the CPU, before
CPU is realized.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 include/hw/riscv/riscv_hart.h | 1 +
 hw/riscv/riscv_hart.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/include/hw/riscv/riscv_hart.h b/include/hw/riscv/riscv_hart.h
index c75856f..77aa4bc 100644
--- a/include/hw/riscv/riscv_hart.h
+++ b/include/hw/riscv/riscv_hart.h
@@ -37,6 +37,7 @@ typedef struct RISCVHartArrayState {
 uint32_t num_harts;
 uint32_t hartid_base;
 char *cpu_type;
+uint64_t resetvec;
 RISCVCPU *harts;
 } RISCVHartArrayState;
 
diff --git a/hw/riscv/riscv_hart.c b/hw/riscv/riscv_hart.c
index f59fe52..613ea2a 100644
--- a/hw/riscv/riscv_hart.c
+++ b/hw/riscv/riscv_hart.c
@@ -31,6 +31,8 @@ static Property riscv_harts_props[] = {
 DEFINE_PROP_UINT32("num-harts", RISCVHartArrayState, num_harts, 1),
 DEFINE_PROP_UINT32("hartid-base", RISCVHartArrayState, hartid_base, 0),
 DEFINE_PROP_STRING("cpu-type", RISCVHartArrayState, cpu_type),
+DEFINE_PROP_UINT64("resetvec", RISCVHartArrayState, resetvec,
+   DEFAULT_RSTVEC),
 DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -44,6 +46,7 @@ static bool riscv_hart_realize(RISCVHartArrayState *s, int 
idx,
char *cpu_type, Error **errp)
 {
 object_initialize_child(OBJECT(s), "harts[*]", &s->harts[idx], cpu_type);
+qdev_prop_set_uint64(DEVICE(&s->harts[idx]), "resetvec", s->resetvec);
 s->harts[idx].env.mhartid = s->hartid_base + idx;
 qemu_register_reset(riscv_harts_cpu_reset, &s->harts[idx]);
 return qdev_realize(DEVICE(&s->harts[idx]), NULL, errp);
-- 
2.7.4

[PATCH v2 00/16] hw/riscv: Add Microchip PolarFire SoC Icicle Kit board support

[Bin Meng]

From: Bin Meng 

This adds support for Microchip PolarFire SoC Icicle Kit board.
The Icicle Kit board integrates a PolarFire SoC, with one SiFive's
E51 plus four U54 cores and many on-chip peripherals and an FPGA.

For more details about Microchip PolarFire SoC, please see:
https://www.microsemi.com/product-directory/soc-fpgas/5498-polarfire-soc-fpga

The Icicle Kit board information can be found here:
https://www.microsemi.com/existing-parts/parts/152514

Unlike SiFive FU540, the RISC-V core resect vector is at 0x2022.
The RISC-V CPU and HART codes has been updated to set the core's
reset vector based on a configurable property from machine codes.

The following perepherals are created as an unimplemented device:

- Bus Error Uint 0/1/2/3/4
- L2 cache controller
- SYSREG
- MPUCFG
- IOSCBCFG
- GPIO

The following perepherals are emulated:
- SiFive CLINT
- SiFive PLIC
- PolarFire SoC Multi-Mode UART
- SiFive PDMA
- Cadence eMMC/SDHCI controller
- Cadence Gigabit Ethernet MAC

The BIOS image used by this machine is hss.bin, aka Hart Software
Services, which can be built from:
https://github.com/polarfire-soc/hart-software-services

To launch this machine:
$ qemu-system-riscv64 -M microchip-icicle-kit -smp 5 \
-bios path/to/hss.bin -sd path/to/sdcard.img \
-nic tap,ifname=tap,script=no,model=cadence_gem \
-display none -serial stdio \
-chardev socket,id=serial1,path=serial1.sock,server,wait \
-serial chardev:serial1

The memory is set to 1 GiB by default to match the hardware.
A sanity check on ram size is performed in the machine init routine
to prompt user to increase the RAM size to > 1 GiB when less than
1 GiB ram is detected.

HSS output is on the first serial port (stdio) and U-Boot/Linux
outputs on the 2nd serial port. OpenSBI outputs on a random serial
port due to the lottery mechanism used during the multi-core boot.

Please check the QEMU WiKi page for the target specific information:
https://wiki.qemu.org/Documentation/Platforms/RISCV#Microchip_PolarFire_SoC_Icicle_Kit

Changes in v2:
- change to update hw/char/meson.build
- add impl.min_access_size and impl.max_access_size as part of
  MemoryRegionOps and remove the allignment check
- change to update hw/sd/meson.build
- change the name to "generic-sdhci" when calling object_initialize_child()
- add a container MR to simplify out-of-bounds access checks
- do not initialize TYPE_SYSBUS_SDHCI in the SoC instance_init(),
  instead move that to the cadence_sdhci model
- do not access generic-sdhci's state directly,
  instead move that to the cadence_sdhci model
- change to update hw/dma/meson.build
- rename the file names to sifive_pdma.[c|h]
- update irq number to 8 per the SiFive FU540 manual
- fix the register offset for channel 1/2/3 in the read/write ops
- connect 8 IRQs to the PLIC
- change "phy-addr" default value to BOARD_PHY_ADDRESS

Bin Meng (16):
  target/riscv: cpu: Add a new 'resetvec' property
  hw/riscv: hart: Add a new 'resetvec' property
  target/riscv: cpu: Set reset vector based on the configured property
value
  hw/riscv: Initial support for Microchip PolarFire SoC Icicle Kit board
  hw/char: Add Microchip PolarFire SoC MMUART emulation
  hw/riscv: microchip_pfsoc: Connect 5 MMUARTs
  hw/sd: Add Cadence SDHCI emulation
  hw/riscv: microchip_pfsoc: Connect a Cadence SDHCI controller and an
SD card
  hw/dma: Add SiFive platform DMA controller emulation
  hw/riscv: microchip_pfsoc: Connect a DMA controller
  hw/net: cadence_gem: Add a new 'phy-addr' property
  hw/arm: xlnx: Set all boards' GEM 'phy-addr' property value to 23
  hw/riscv: microchip_pfsoc: Connect 2 Cadence GEMs
  hw/riscv: microchip_pfsoc: Hook GPIO controllers
  hw/riscv: clint: Avoid using hard-coded timebase frequency
  hw/riscv: sifive_u: Connect a DMA controller

 default-configs/riscv64-softmmu.mak |   1 +
 include/hw/char/mchp_pfsoc_mmuart.h |  61 +
 include/hw/dma/sifive_pdma.h|  57 +
 include/hw/net/cadence_gem.h|   2 +
 include/hw/riscv/microchip_pfsoc.h  | 133 +++
 include/hw/riscv/riscv_hart.h   |   1 +
 include/hw/riscv/sifive_clint.h |   4 +-
 include/hw/riscv/sifive_u.h |  11 +
 include/hw/sd/cadence_sdhci.h   |  47 
 target/riscv/cpu.h  |   7 +-
 hw/arm/xilinx_zynq.c|   1 +
 hw/arm/xlnx-versal.c|   1 +
 hw/arm/xlnx-zynqmp.c|   1 +
 hw/char/mchp_pfsoc_mmuart.c |  86 +++
 hw/dma/sifive_pdma.c| 313 ++
 hw/net/cadence_gem.c|   7 +-
 hw/riscv/microchip_pfsoc.c  | 437 
 hw/riscv/opentitan.c|   1 +
 hw/riscv/riscv_hart.c   |   3 +
 hw/riscv/sifive_clint.c |  26 ++-
 hw/riscv/sifive_e.c |   4 +-
 hw/riscv/sifive_u.c |  35 ++-
 hw/riscv/spike.c|   3 +-
 hw/riscv/virt.c |   3 +-
 hw/sd/cadence_s

[PATCH v2 10/16] hw/riscv: microchip_pfsoc: Connect a DMA controller

[Bin Meng]

From: Bin Meng 

On the Icicle Kit board, the HSS firmware utilizes the on-chip DMA
controller to move the 2nd stage bootloader in the system memory.
Let's connect a DMA controller to Microchip PolarFire SoC.

Signed-off-by: Bin Meng 

---

Changes in v2:
- connect 8 IRQs to the PLIC

 include/hw/riscv/microchip_pfsoc.h | 11 +++
 hw/riscv/microchip_pfsoc.c | 15 +++
 hw/riscv/Kconfig   |  1 +
 3 files changed, 27 insertions(+)

diff --git a/include/hw/riscv/microchip_pfsoc.h 
b/include/hw/riscv/microchip_pfsoc.h
index d810ee8..63e7860 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -23,6 +23,7 @@
 #define HW_MICROCHIP_PFSOC_H
 
 #include "hw/char/mchp_pfsoc_mmuart.h"
+#include "hw/dma/sifive_pdma.h"
 #include "hw/sd/cadence_sdhci.h"
 
 typedef struct MicrochipPFSoCState {
@@ -40,6 +41,7 @@ typedef struct MicrochipPFSoCState {
 MchpPfSoCMMUartState *serial2;
 MchpPfSoCMMUartState *serial3;
 MchpPfSoCMMUartState *serial4;
+SiFivePDMAState dma;
 CadenceSDHCIState sdhci;
 } MicrochipPFSoCState;
 
@@ -71,6 +73,7 @@ enum {
 MICROCHIP_PFSOC_BUSERR_UNIT4,
 MICROCHIP_PFSOC_CLINT,
 MICROCHIP_PFSOC_L2CC,
+MICROCHIP_PFSOC_DMA,
 MICROCHIP_PFSOC_L2LIM,
 MICROCHIP_PFSOC_PLIC,
 MICROCHIP_PFSOC_MMUART0,
@@ -88,6 +91,14 @@ enum {
 };
 
 enum {
+MICROCHIP_PFSOC_DMA_IRQ0 = 5,
+MICROCHIP_PFSOC_DMA_IRQ1 = 6,
+MICROCHIP_PFSOC_DMA_IRQ2 = 7,
+MICROCHIP_PFSOC_DMA_IRQ3 = 8,
+MICROCHIP_PFSOC_DMA_IRQ4 = 9,
+MICROCHIP_PFSOC_DMA_IRQ5 = 10,
+MICROCHIP_PFSOC_DMA_IRQ6 = 11,
+MICROCHIP_PFSOC_DMA_IRQ7 = 12,
 MICROCHIP_PFSOC_EMMC_SD_IRQ = 88,
 MICROCHIP_PFSOC_MMUART0_IRQ = 90,
 MICROCHIP_PFSOC_MMUART1_IRQ = 91,
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 0b2e9ca..d8ec973 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -13,6 +13,7 @@
  * 2) eNVM (Embedded Non-Volatile Memory)
  * 3) MMUARTs (Multi-Mode UART)
  * 4) Cadence eMMC/SDHC controller and an SD card connected to it
+ * 5) SiFive Platform DMA (Direct Memory Access Controller)
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -71,6 +72,7 @@ static const struct MemmapEntry {
 [MICROCHIP_PFSOC_BUSERR_UNIT4] ={  0x1704000, 0x1000 },
 [MICROCHIP_PFSOC_CLINT] =   {  0x200,0x1 },
 [MICROCHIP_PFSOC_L2CC] ={  0x201, 0x1000 },
+[MICROCHIP_PFSOC_DMA] = {  0x300,   0x10 },
 [MICROCHIP_PFSOC_L2LIM] =   {  0x800,  0x200 },
 [MICROCHIP_PFSOC_PLIC] ={  0xc00,  0x400 },
 [MICROCHIP_PFSOC_MMUART0] = { 0x2000, 0x1000 },
@@ -114,6 +116,9 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
  TYPE_RISCV_CPU_SIFIVE_U54);
 qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", RESET_VECTOR);
 
+object_initialize_child(obj, "dma-controller", &s->dma,
+TYPE_SIFIVE_PDMA);
+
 object_initialize_child(obj, "sd-controller", &s->sdhci,
 TYPE_CADENCE_SDHCI);
 }
@@ -218,6 +223,16 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 memmap[MICROCHIP_PFSOC_PLIC].size);
 g_free(plic_hart_config);
 
+/* DMA */
+sysbus_realize(SYS_BUS_DEVICE(&s->dma), errp);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->dma), 0,
+memmap[MICROCHIP_PFSOC_DMA].base);
+for (i = 0; i < SIFIVE_PDMA_IRQS; i++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma), i,
+   qdev_get_gpio_in(DEVICE(s->plic),
+MICROCHIP_PFSOC_DMA_IRQ0 + i));
+}
+
 /* SYSREG */
 create_unimplemented_device("microchip.pfsoc.sysreg",
 memmap[MICROCHIP_PFSOC_SYSREG].base,
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 7412db9..9032cb0 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -55,4 +55,5 @@ config MICROCHIP_PFSOC
 select SIFIVE
 select UNIMP
 select MCHP_PFSOC_MMUART
+select SIFIVE_PDMA
 select CADENCE_SDHCI
-- 
2.7.4

[PATCH v2 04/16] hw/riscv: Initial support for Microchip PolarFire SoC Icicle Kit board

[Bin Meng]

From: Bin Meng 

This is an initial support for Microchip PolarFire SoC Icicle Kit.
The Icicle Kit board integrates a PolarFire SoC, with one SiFive's
E51 plus four U54 cores and many on-chip peripherals and an FPGA.

For more details about Microchip PolarFire Soc, please see:
https://www.microsemi.com/product-directory/soc-fpgas/5498-polarfire-soc-fpga

Unlike SiFive FU540, the RISC-V core resect vector is at 0x2022.
The following perepherals are created as an unimplemented device:

- Bus Error Uint 0/1/2/3/4
- L2 cache controller
- SYSREG
- MPUCFG
- IOSCBCFG

More devices will be added later.

The BIOS image used by this machine is hss.bin, aka Hart Software
Services, which can be built from:
https://github.com/polarfire-soc/hart-software-services

To launch this machine:
$ qemu-system-riscv64 -nographic -M microchip-icicle-kit

The memory is set to 1 GiB by default to match the hardware.
A sanity check on ram size is performed in the machine init routine
to prompt user to increase the RAM size to > 1 GiB when less than
1 GiB ram is detected.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 default-configs/riscv64-softmmu.mak |   1 +
 include/hw/riscv/microchip_pfsoc.h  |  88 ++
 hw/riscv/microchip_pfsoc.c  | 312 
 MAINTAINERS |   7 +
 hw/riscv/Kconfig|   6 +
 hw/riscv/meson.build|   1 +
 6 files changed, 415 insertions(+)
 create mode 100644 include/hw/riscv/microchip_pfsoc.h
 create mode 100644 hw/riscv/microchip_pfsoc.c

diff --git a/default-configs/riscv64-softmmu.mak 
b/default-configs/riscv64-softmmu.mak
index aaf6d73..76b6195 100644
--- a/default-configs/riscv64-softmmu.mak
+++ b/default-configs/riscv64-softmmu.mak
@@ -10,3 +10,4 @@ CONFIG_SPIKE=y
 CONFIG_SIFIVE_E=y
 CONFIG_SIFIVE_U=y
 CONFIG_RISCV_VIRT=y
+CONFIG_MICROCHIP_PFSOC=y
diff --git a/include/hw/riscv/microchip_pfsoc.h 
b/include/hw/riscv/microchip_pfsoc.h
new file mode 100644
index 000..1953ef1
--- /dev/null
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -0,0 +1,88 @@
+/*
+ * Microchip PolarFire SoC machine interface
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see .
+ */
+
+#ifndef HW_MICROCHIP_PFSOC_H
+#define HW_MICROCHIP_PFSOC_H
+
+typedef struct MicrochipPFSoCState {
+/*< private >*/
+DeviceState parent_obj;
+
+/*< public >*/
+CPUClusterState e_cluster;
+CPUClusterState u_cluster;
+RISCVHartArrayState e_cpus;
+RISCVHartArrayState u_cpus;
+DeviceState *plic;
+} MicrochipPFSoCState;
+
+#define TYPE_MICROCHIP_PFSOC"microchip.pfsoc"
+#define MICROCHIP_PFSOC(obj) \
+OBJECT_CHECK(MicrochipPFSoCState, (obj), TYPE_MICROCHIP_PFSOC)
+
+typedef struct MicrochipIcicleKitState {
+/*< private >*/
+MachineState parent_obj;
+
+/*< public >*/
+MicrochipPFSoCState soc;
+} MicrochipIcicleKitState;
+
+#define TYPE_MICROCHIP_ICICLE_KIT_MACHINE \
+MACHINE_TYPE_NAME("microchip-icicle-kit")
+#define MICROCHIP_ICICLE_KIT_MACHINE(obj) \
+OBJECT_CHECK(MicrochipIcicleKitState, (obj), \
+ TYPE_MICROCHIP_ICICLE_KIT_MACHINE)
+
+enum {
+MICROCHIP_PFSOC_DEBUG,
+MICROCHIP_PFSOC_E51_DTIM,
+MICROCHIP_PFSOC_BUSERR_UNIT0,
+MICROCHIP_PFSOC_BUSERR_UNIT1,
+MICROCHIP_PFSOC_BUSERR_UNIT2,
+MICROCHIP_PFSOC_BUSERR_UNIT3,
+MICROCHIP_PFSOC_BUSERR_UNIT4,
+MICROCHIP_PFSOC_CLINT,
+MICROCHIP_PFSOC_L2CC,
+MICROCHIP_PFSOC_L2LIM,
+MICROCHIP_PFSOC_PLIC,
+MICROCHIP_PFSOC_SYSREG,
+MICROCHIP_PFSOC_MPUCFG,
+MICROCHIP_PFSOC_ENVM_CFG,
+MICROCHIP_PFSOC_ENVM_DATA,
+MICROCHIP_PFSOC_IOSCB_CFG,
+MICROCHIP_PFSOC_DRAM,
+};
+
+#define MICROCHIP_PFSOC_MANAGEMENT_CPU_COUNT1
+#define MICROCHIP_PFSOC_COMPUTE_CPU_COUNT   4
+
+#define MICROCHIP_PFSOC_PLIC_HART_CONFIG"MS"
+#define MICROCHIP_PFSOC_PLIC_NUM_SOURCES185
+#define MICROCHIP_PFSOC_PLIC_NUM_PRIORITIES 7
+#define MICROCHIP_PFSOC_PLIC_PRIORITY_BASE  0x04
+#define MICROCHIP_PFSOC_PLIC_PENDING_BASE   0x1000
+#define MICROCHIP_PFSOC_PLIC_ENABLE_BASE0x2000
+#define MICROCHIP_PFSOC_PLIC_ENABLE_STRIDE  0x80
+#define MICROCHIP_PFSOC_PLIC_CONTEXT_BASE   0x20
+#define MICROCHIP_PFSOC_PLIC_CONTEXT_STRIDE 0x1000
+
+#endif /* HW_MICROCHIP_PFSOC_H */
diff --git a/hw/riscv

[PATCH v2 09/16] hw/dma: Add SiFive platform DMA controller emulation

[Bin Meng]

From: Bin Meng 

Microchip PolarFire SoC integrates a DMA engine that supports:
* Independent concurrent DMA transfers using 4 DMA channels
* Generation of interrupts on various conditions during execution
which is actually an IP reused from the SiFive FU540 chip.

This creates a model to support both polling and interrupt modes.

Signed-off-by: Bin Meng 

---

Changes in v2:
- change to update hw/dma/meson.build
- rename the file names to sifive_pdma.[c|h]
- update irq number to 8 per the SiFive FU540 manual
- fix the register offset for channel 1/2/3 in the read/write ops

 include/hw/dma/sifive_pdma.h |  57 
 hw/dma/sifive_pdma.c | 313 +++
 hw/dma/Kconfig   |   3 +
 hw/dma/meson.build   |   1 +
 4 files changed, 374 insertions(+)
 create mode 100644 include/hw/dma/sifive_pdma.h
 create mode 100644 hw/dma/sifive_pdma.c

diff --git a/include/hw/dma/sifive_pdma.h b/include/hw/dma/sifive_pdma.h
new file mode 100644
index 000..fb09383
--- /dev/null
+++ b/include/hw/dma/sifive_pdma.h
@@ -0,0 +1,57 @@
+/*
+ * SiFive Platform DMA emulation
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#ifndef SIFIVE_PDMA_H
+#define SIFIVE_PDMA_H
+
+struct sifive_pdma_chan {
+uint32_t control;
+uint32_t next_config;
+uint64_t next_bytes;
+uint64_t next_dst;
+uint64_t next_src;
+uint32_t exec_config;
+uint64_t exec_bytes;
+uint64_t exec_dst;
+uint64_t exec_src;
+int state;
+};
+
+#define SIFIVE_PDMA_CHANS   4
+#define SIFIVE_PDMA_IRQS(SIFIVE_PDMA_CHANS * 2)
+#define SIFIVE_PDMA_REG_SIZE0x10
+#define SIFIVE_PDMA_CHAN_NO(reg)((reg & (SIFIVE_PDMA_REG_SIZE - 1)) >> 12)
+
+typedef struct SiFivePDMAState {
+SysBusDevice parent;
+MemoryRegion iomem;
+qemu_irq irq[SIFIVE_PDMA_IRQS];
+
+struct sifive_pdma_chan chan[SIFIVE_PDMA_CHANS];
+} SiFivePDMAState;
+
+#define TYPE_SIFIVE_PDMA"sifive.pdma"
+
+#define MCHP_PFSOC_DMA(obj) \
+OBJECT_CHECK(SiFivePDMAState, (obj), TYPE_SIFIVE_PDMA)
+
+#endif /* SIFIVE_PDMA_H */
diff --git a/hw/dma/sifive_pdma.c b/hw/dma/sifive_pdma.c
new file mode 100644
index 000..365bad9
--- /dev/null
+++ b/hw/dma/sifive_pdma.c
@@ -0,0 +1,313 @@
+/*
+ * SiFive Platform DMA emulation
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "hw/hw.h"
+#include "hw/irq.h"
+#include "hw/qdev-properties.h"
+#include "hw/sysbus.h"
+#include "migration/vmstate.h"
+#include "sysemu/dma.h"
+#include "hw/dma/sifive_pdma.h"
+
+#define DMA_CONTROL 0x000
+#define   CONTROL_CLAIM BIT(0)
+#define   CONTROL_RUN   BIT(1)
+#define   CONTROL_DONE_IE   BIT(14)
+#define   CONTROL_ERR_IEBIT(15)
+#define   CONTROL_DONE  BIT(30)
+#define   CONTROL_ERR   BIT(31)
+
+#define DMA_NEXT_CONFIG 0x004
+#define   CONFIG_REPEAT BIT(2)
+#define   CONFIG_ORDER  BIT(3)
+#define   CONFIG_WRSZ_SHIFT 24
+#define   CONFIG_RDSZ_SHIFT 28
+#define   CONFIG_SZ_MASK0xf
+
+#define DMA_NEXT_BYTES  0x008
+#define DMA_NEXT_DST0x010
+#define DMA_NEXT_SRC0x018
+#define DMA_EXEC_CONFIG 0x104
+#define DMA_EXEC_BYTES  0x108
+#define DMA_EXEC_DST0x110
+#define DMA_EXEC_SRC0x118
+
+enum dma_chan_state {
+DMA_CHAN_STATE_IDLE,
+DMA_CHAN_STATE_STARTED,
+DMA_CHAN_STATE_ERROR,
+DMA_CHAN_STATE_DONE
+};
+
+static void sifive_pdma_run(SiFivePDMAState *s, int ch)
+{
+uint64_t bytes = s->chan[ch].next_bytes;
+uint64_t dst = s->chan

[PATCH v2 01/16] target/riscv: cpu: Add a new 'resetvec' property

[Bin Meng]

From: Bin Meng 

Currently the reset vector address is hard-coded in a RISC-V CPU's
instance_init() routine. In a real world we can have 2 exact same
CPUs except for the reset vector address, which is pretty common in
the RISC-V core IP licensing business.

Normally reset vector address is a configurable parameter. Let's
create a 64-bit property to store the reset vector address which
covers both 32-bit and 64-bit CPUs.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 target/riscv/cpu.h | 1 +
 target/riscv/cpu.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 383808b..dc350f0 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -291,6 +291,7 @@ typedef struct RISCVCPU {
 uint16_t elen;
 bool mmu;
 bool pmp;
+uint64_t resetvec;
 } cfg;
 } RISCVCPU;
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 228b9bd..8067a26 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -518,6 +518,7 @@ static Property riscv_cpu_properties[] = {
 DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
 DEFINE_PROP_BOOL("mmu", RISCVCPU, cfg.mmu, true),
 DEFINE_PROP_BOOL("pmp", RISCVCPU, cfg.pmp, true),
+DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec, DEFAULT_RSTVEC),
 DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.7.4

[PATCH v2 16/16] hw/riscv: sifive_u: Connect a DMA controller

[Bin Meng]

From: Bin Meng 

SiFive FU540 SoC integrates a platform DMA controller with 4 DMA
channels. This connects the exsiting SiFive PDMA model to the SoC,
and adds its device tree data as well.

Signed-off-by: Bin Meng 
---

(no changes since v1)

 include/hw/riscv/sifive_u.h | 11 +++
 hw/riscv/sifive_u.c | 30 ++
 hw/riscv/Kconfig|  1 +
 3 files changed, 42 insertions(+)

diff --git a/include/hw/riscv/sifive_u.h b/include/hw/riscv/sifive_u.h
index d3c0c00..793000a 100644
--- a/include/hw/riscv/sifive_u.h
+++ b/include/hw/riscv/sifive_u.h
@@ -19,6 +19,7 @@
 #ifndef HW_SIFIVE_U_H
 #define HW_SIFIVE_U_H
 
+#include "hw/dma/sifive_pdma.h"
 #include "hw/net/cadence_gem.h"
 #include "hw/riscv/riscv_hart.h"
 #include "hw/riscv/sifive_cpu.h"
@@ -43,6 +44,7 @@ typedef struct SiFiveUSoCState {
 SiFiveUPRCIState prci;
 SIFIVEGPIOState gpio;
 SiFiveUOTPState otp;
+SiFivePDMAState dma;
 CadenceGEMState gem;
 
 uint32_t serial;
@@ -72,6 +74,7 @@ enum {
 SIFIVE_U_MROM,
 SIFIVE_U_CLINT,
 SIFIVE_U_L2CC,
+SIFIVE_U_PDMA,
 SIFIVE_U_L2LIM,
 SIFIVE_U_PLIC,
 SIFIVE_U_PRCI,
@@ -108,6 +111,14 @@ enum {
 SIFIVE_U_GPIO_IRQ13 = 20,
 SIFIVE_U_GPIO_IRQ14 = 21,
 SIFIVE_U_GPIO_IRQ15 = 22,
+SIFIVE_U_PDMA_IRQ0 = 23,
+SIFIVE_U_PDMA_IRQ1 = 24,
+SIFIVE_U_PDMA_IRQ2 = 25,
+SIFIVE_U_PDMA_IRQ3 = 26,
+SIFIVE_U_PDMA_IRQ4 = 27,
+SIFIVE_U_PDMA_IRQ5 = 28,
+SIFIVE_U_PDMA_IRQ6 = 29,
+SIFIVE_U_PDMA_IRQ7 = 30,
 SIFIVE_U_GEM_IRQ = 0x35
 };
 
diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c
index 2bc3992..7997537 100644
--- a/hw/riscv/sifive_u.c
+++ b/hw/riscv/sifive_u.c
@@ -14,6 +14,7 @@
  * 4) GPIO (General Purpose Input/Output Controller)
  * 5) OTP (One-Time Programmable) memory with stored serial number
  * 6) GEM (Gigabit Ethernet Controller) and management block
+ * 7) DMA (Direct Memory Access Controller)
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -73,6 +74,7 @@ static const struct MemmapEntry {
 [SIFIVE_U_MROM] = { 0x1000, 0xf000 },
 [SIFIVE_U_CLINT] ={  0x200,0x1 },
 [SIFIVE_U_L2CC] = {  0x201, 0x1000 },
+[SIFIVE_U_PDMA] = {  0x300,   0x10 },
 [SIFIVE_U_L2LIM] ={  0x800,  0x200 },
 [SIFIVE_U_PLIC] = {  0xc00,  0x400 },
 [SIFIVE_U_PRCI] = { 0x1000, 0x1000 },
@@ -303,6 +305,22 @@ static void create_fdt(SiFiveUState *s, const struct 
MemmapEntry *memmap,
 qemu_fdt_setprop_string(fdt, nodename, "compatible", "gpio-restart");
 g_free(nodename);
 
+nodename = g_strdup_printf("/soc/dma@%lx",
+(long)memmap[SIFIVE_U_PDMA].base);
+qemu_fdt_add_subnode(fdt, nodename);
+qemu_fdt_setprop_cell(fdt, nodename, "#dma-cells", 1);
+qemu_fdt_setprop_cells(fdt, nodename, "interrupts",
+SIFIVE_U_PDMA_IRQ0, SIFIVE_U_PDMA_IRQ1, SIFIVE_U_PDMA_IRQ2,
+SIFIVE_U_PDMA_IRQ3, SIFIVE_U_PDMA_IRQ4, SIFIVE_U_PDMA_IRQ5,
+SIFIVE_U_PDMA_IRQ6, SIFIVE_U_PDMA_IRQ7);
+qemu_fdt_setprop_cell(fdt, nodename, "interrupt-parent", plic_phandle);
+qemu_fdt_setprop_cells(fdt, nodename, "reg",
+0x0, memmap[SIFIVE_U_PDMA].base,
+0x0, memmap[SIFIVE_U_PDMA].size);
+qemu_fdt_setprop_string(fdt, nodename, "compatible",
+"sifive,fu540-c000-pdma");
+g_free(nodename);
+
 nodename = g_strdup_printf("/soc/cache-controller@%lx",
 (long)memmap[SIFIVE_U_L2CC].base);
 qemu_fdt_add_subnode(fdt, nodename);
@@ -627,6 +645,7 @@ static void sifive_u_soc_instance_init(Object *obj)
 object_initialize_child(obj, "otp", &s->otp, TYPE_SIFIVE_U_OTP);
 object_initialize_child(obj, "gem", &s->gem, TYPE_CADENCE_GEM);
 object_initialize_child(obj, "gpio", &s->gpio, TYPE_SIFIVE_GPIO);
+object_initialize_child(obj, "pdma", &s->dma, TYPE_SIFIVE_PDMA);
 }
 
 static void sifive_u_soc_realize(DeviceState *dev, Error **errp)
@@ -730,6 +749,17 @@ static void sifive_u_soc_realize(DeviceState *dev, Error 
**errp)
 SIFIVE_U_GPIO_IRQ0 + i));
 }
 
+/* PDMA */
+sysbus_realize(SYS_BUS_DEVICE(&s->dma), errp);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->dma), 0, memmap[SIFIVE_U_PDMA].base);
+
+/* Connect PDMA interrupts to the PLIC */
+for (i = 0; i < SIFIVE_PDMA_IRQS; i++) {
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->dma), i,
+   qdev_get_gpio_in(DEVICE(s->plic),
+SIFIVE_U_PDMA_IRQ0 + i));
+}
+
 qdev_prop_set_uint32(DEVICE(&s->otp), "serial", s->serial);
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->otp), errp)) {
 return;
diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
index 9032cb0..e53ab1e 100644
--- a/hw/riscv/Kconfig
+++ b/hw/riscv/Kconfig
@@ -22,6 +22,7 @@ config SIFIVE_U
 select CADENCE
 sel

[PATCH v2 05/16] hw/char: Add Microchip PolarFire SoC MMUART emulation

[Bin Meng]

From: Bin Meng 

Microchip PolarFire SoC MMUART is ns16550 compatible, with some
additional registers. Create a simple MMUART model built on top
of the existing ns16550 model.

Signed-off-by: Bin Meng 

---

Changes in v2:
- change to update hw/char/meson.build
- add impl.min_access_size and impl.max_access_size as part of
  MemoryRegionOps and remove the allignment check

 include/hw/char/mchp_pfsoc_mmuart.h | 61 ++
 hw/char/mchp_pfsoc_mmuart.c | 86 +
 MAINTAINERS |  2 +
 hw/char/Kconfig |  3 ++
 hw/char/meson.build |  1 +
 5 files changed, 153 insertions(+)
 create mode 100644 include/hw/char/mchp_pfsoc_mmuart.h
 create mode 100644 hw/char/mchp_pfsoc_mmuart.c

diff --git a/include/hw/char/mchp_pfsoc_mmuart.h 
b/include/hw/char/mchp_pfsoc_mmuart.h
new file mode 100644
index 000..f619902
--- /dev/null
+++ b/include/hw/char/mchp_pfsoc_mmuart.h
@@ -0,0 +1,61 @@
+/*
+ * Microchip PolarFire SoC MMUART emulation
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef HW_MCHP_PFSOC_MMUART_H
+#define HW_MCHP_PFSOC_MMUART_H
+
+#include "hw/char/serial.h"
+
+#define MCHP_PFSOC_MMUART_REG_SIZE  52
+
+typedef struct MchpPfSoCMMUartState {
+MemoryRegion iomem;
+hwaddr base;
+qemu_irq irq;
+
+SerialMM *serial;
+
+uint32_t reg[MCHP_PFSOC_MMUART_REG_SIZE / sizeof(uint32_t)];
+} MchpPfSoCMMUartState;
+
+/**
+ * mchp_pfsoc_mmuart_create - Create a Microchip PolarFire SoC MMUART
+ *
+ * This is a helper routine for board to create a MMUART device that is
+ * compatible with Microchip PolarFire SoC.
+ *
+ * @sysmem: system memory region to map
+ * @base: base address of the MMUART registers
+ * @irq: IRQ number of the MMUART device
+ * @chr: character device to associate to
+ *
+ * @return: a pointer to the device specific control structure
+ */
+MchpPfSoCMMUartState *mchp_pfsoc_mmuart_create(MemoryRegion *sysmem,
+hwaddr base, qemu_irq irq, Chardev *chr);
+
+#endif /* HW_MCHP_PFSOC_MMUART_H */
diff --git a/hw/char/mchp_pfsoc_mmuart.c b/hw/char/mchp_pfsoc_mmuart.c
new file mode 100644
index 000..8a002b0
--- /dev/null
+++ b/hw/char/mchp_pfsoc_mmuart.c
@@ -0,0 +1,86 @@
+/*
+ * Microchip PolarFire SoC MMUART emulation
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/log.h"
+#include "chardev/char.h"
+#include "exec/address-spaces.h"
+#include "hw/char/mchp_pfsoc_mmuart.h"
+
+static uint64_t mchp_pfsoc_mmuart_read(void *opaque, hwaddr addr, unsigned 
size)
+{
+MchpPfSoCMMUartState *s = opaque;
+
+if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
+qemu_log_mask(LOG_GUEST_ERROR, "%s: read: addr=0x%" HWADDR_PRIx "\n",
+  __func__, addr);
+return 0;
+}
+
+return s->reg[addr / sizeof(uint32_t)];
+}
+
+static void mchp_pfsoc_mmuart_write(void *opaque, hwaddr addr,
+uint64_t value, unsigned size)
+{
+MchpPfSoCMMUartState *s = opaque;
+uint32_t val32 = (uint32_t)value;
+
+if (addr >= MCHP_PFSOC_MMUART_REG_SIZE) {
+qemu_log_mask(LOG_GUEST_ERROR,

[PATCH v2 06/16] hw/riscv: microchip_pfsoc: Connect 5 MMUARTs

[Bin Meng]

From: Bin Meng 

Microchip PolarFire SoC has 5 MMUARTs, and the Icicle Kit board
wires 4 of them out. Let's connect all 5 MMUARTs.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 include/hw/riscv/microchip_pfsoc.h | 20 
 hw/riscv/microchip_pfsoc.c | 30 ++
 hw/riscv/Kconfig   |  1 +
 3 files changed, 51 insertions(+)

diff --git a/include/hw/riscv/microchip_pfsoc.h 
b/include/hw/riscv/microchip_pfsoc.h
index 1953ef1..a5efa1d 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -22,6 +22,8 @@
 #ifndef HW_MICROCHIP_PFSOC_H
 #define HW_MICROCHIP_PFSOC_H
 
+#include "hw/char/mchp_pfsoc_mmuart.h"
+
 typedef struct MicrochipPFSoCState {
 /*< private >*/
 DeviceState parent_obj;
@@ -32,6 +34,11 @@ typedef struct MicrochipPFSoCState {
 RISCVHartArrayState e_cpus;
 RISCVHartArrayState u_cpus;
 DeviceState *plic;
+MchpPfSoCMMUartState *serial0;
+MchpPfSoCMMUartState *serial1;
+MchpPfSoCMMUartState *serial2;
+MchpPfSoCMMUartState *serial3;
+MchpPfSoCMMUartState *serial4;
 } MicrochipPFSoCState;
 
 #define TYPE_MICROCHIP_PFSOC"microchip.pfsoc"
@@ -64,14 +71,27 @@ enum {
 MICROCHIP_PFSOC_L2CC,
 MICROCHIP_PFSOC_L2LIM,
 MICROCHIP_PFSOC_PLIC,
+MICROCHIP_PFSOC_MMUART0,
 MICROCHIP_PFSOC_SYSREG,
 MICROCHIP_PFSOC_MPUCFG,
+MICROCHIP_PFSOC_MMUART1,
+MICROCHIP_PFSOC_MMUART2,
+MICROCHIP_PFSOC_MMUART3,
+MICROCHIP_PFSOC_MMUART4,
 MICROCHIP_PFSOC_ENVM_CFG,
 MICROCHIP_PFSOC_ENVM_DATA,
 MICROCHIP_PFSOC_IOSCB_CFG,
 MICROCHIP_PFSOC_DRAM,
 };
 
+enum {
+MICROCHIP_PFSOC_MMUART0_IRQ = 90,
+MICROCHIP_PFSOC_MMUART1_IRQ = 91,
+MICROCHIP_PFSOC_MMUART2_IRQ = 92,
+MICROCHIP_PFSOC_MMUART3_IRQ = 93,
+MICROCHIP_PFSOC_MMUART4_IRQ = 94,
+};
+
 #define MICROCHIP_PFSOC_MANAGEMENT_CPU_COUNT1
 #define MICROCHIP_PFSOC_COMPUTE_CPU_COUNT   4
 
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index d946b2a..cee959a 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -11,6 +11,7 @@
  * 0) CLINT (Core Level Interruptor)
  * 1) PLIC (Platform Level Interrupt Controller)
  * 2) eNVM (Embedded Non-Volatile Memory)
+ * 3) MMUARTs (Multi-Mode UART)
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -38,6 +39,7 @@
 #include "hw/irq.h"
 #include "hw/loader.h"
 #include "hw/sysbus.h"
+#include "chardev/char.h"
 #include "hw/cpu/cluster.h"
 #include "target/riscv/cpu.h"
 #include "hw/misc/unimp.h"
@@ -46,6 +48,7 @@
 #include "hw/riscv/sifive_clint.h"
 #include "hw/riscv/sifive_plic.h"
 #include "hw/riscv/microchip_pfsoc.h"
+#include "sysemu/sysemu.h"
 
 /*
  * The BIOS image used by this machine is called Hart Software Services (HSS).
@@ -69,8 +72,13 @@ static const struct MemmapEntry {
 [MICROCHIP_PFSOC_L2CC] ={  0x201, 0x1000 },
 [MICROCHIP_PFSOC_L2LIM] =   {  0x800,  0x200 },
 [MICROCHIP_PFSOC_PLIC] ={  0xc00,  0x400 },
+[MICROCHIP_PFSOC_MMUART0] = { 0x2000, 0x1000 },
 [MICROCHIP_PFSOC_SYSREG] =  { 0x20002000, 0x2000 },
 [MICROCHIP_PFSOC_MPUCFG] =  { 0x20005000, 0x1000 },
+[MICROCHIP_PFSOC_MMUART1] = { 0x2010, 0x1000 },
+[MICROCHIP_PFSOC_MMUART2] = { 0x20102000, 0x1000 },
+[MICROCHIP_PFSOC_MMUART3] = { 0x20104000, 0x1000 },
+[MICROCHIP_PFSOC_MMUART4] = { 0x20106000, 0x1000 },
 [MICROCHIP_PFSOC_ENVM_CFG] ={ 0x2020, 0x1000 },
 [MICROCHIP_PFSOC_ENVM_DATA] =   { 0x2022,0x2 },
 [MICROCHIP_PFSOC_IOSCB_CFG] =   { 0x3708, 0x1000 },
@@ -215,6 +223,28 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 memmap[MICROCHIP_PFSOC_MPUCFG].base,
 memmap[MICROCHIP_PFSOC_MPUCFG].size);
 
+/* MMUARTs */
+s->serial0 = mchp_pfsoc_mmuart_create(system_memory,
+memmap[MICROCHIP_PFSOC_MMUART0].base,
+qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_MMUART0_IRQ),
+serial_hd(0));
+s->serial1 = mchp_pfsoc_mmuart_create(system_memory,
+memmap[MICROCHIP_PFSOC_MMUART1].base,
+qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_MMUART1_IRQ),
+serial_hd(1));
+s->serial2 = mchp_pfsoc_mmuart_create(system_memory,
+memmap[MICROCHIP_PFSOC_MMUART2].base,
+qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_MMUART2_IRQ),
+serial_hd(2));
+s->serial3 = mchp_pfsoc_mmuart_create(system_memory,
+memmap[MICROCHIP_PFSOC_MMUART3].base,
+qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_MMUART3_IRQ),
+serial_hd(3));
+s->serial4 = mchp_pfsoc_mmuart_create(system_memory,
+memmap[MICROCHIP_PFSOC_MMUART4].base,
+qdev_ge

[PATCH v2 12/16] hw/arm: xlnx: Set all boards' GEM 'phy-addr' property value to 23

[Bin Meng]

From: Bin Meng 

When cadence_gem model was created for Xilinx boards, the PHY address
was hard-coded to 23 in the GEM model. Now that we have introduced a
property we can use that to tell GEM model what our PHY address is.
Change all boards' GEM 'phy-addr' property value to 23, and set the
PHY address default value to 0 in the GEM model.

Signed-off-by: Bin Meng 
---

(no changes since v1)

 hw/arm/xilinx_zynq.c | 1 +
 hw/arm/xlnx-versal.c | 1 +
 hw/arm/xlnx-zynqmp.c | 1 +
 hw/net/cadence_gem.c | 6 +++---
 4 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 969ef07..9ffcc56 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -119,6 +119,7 @@ static void gem_init(NICInfo *nd, uint32_t base, qemu_irq 
irq)
 qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
 qdev_set_nic_properties(dev, nd);
 }
+object_property_set_int(OBJECT(dev), "phy-addr", 23, &error_abort);
 s = SYS_BUS_DEVICE(dev);
 sysbus_realize_and_unref(s, &error_fatal);
 sysbus_mmio_map(s, 0, base);
diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index e3aa4bd..12ba6c4 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -165,6 +165,7 @@ static void versal_create_gems(Versal *s, qemu_irq *pic)
 qemu_check_nic_model(nd, "cadence_gem");
 qdev_set_nic_properties(dev, nd);
 }
+object_property_set_int(OBJECT(dev), "phy-addr", 23, &error_abort);
 object_property_set_int(OBJECT(dev), "num-priority-queues", 2,
 &error_abort);
 object_property_set_link(OBJECT(dev), "dma", OBJECT(&s->mr_ps),
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index c435b9d..a85ef4b 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -460,6 +460,7 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error 
**errp)
 }
 object_property_set_int(OBJECT(&s->gem[i]), "revision", GEM_REVISION,
 &error_abort);
+object_property_set_int(OBJECT(dev), "phy-addr", 23, &error_abort);
 object_property_set_int(OBJECT(&s->gem[i]), "num-priority-queues", 2,
 &error_abort);
 if (!sysbus_realize(SYS_BUS_DEVICE(&s->gem[i]), errp)) {
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index d80096b..7a53469 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -250,7 +250,7 @@
 #define GEM_PHYMNTNC_REG_SHIFT 18
 
 /* Marvell PHY definitions */
-#define BOARD_PHY_ADDRESS23 /* PHY address we will emulate a device at */
+#define BOARD_PHY_ADDRESS0 /* PHY address we will emulate a device at */
 
 #define PHY_REG_CONTROL  0
 #define PHY_REG_STATUS   1
@@ -1446,7 +1446,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, 
unsigned size)
 uint32_t phy_addr, reg_num;
 
 phy_addr = (retval & GEM_PHYMNTNC_ADDR) >> GEM_PHYMNTNC_ADDR_SHFT;
-if (phy_addr == s->phy_addr || phy_addr == 0) {
+if (phy_addr == s->phy_addr) {
 reg_num = (retval & GEM_PHYMNTNC_REG) >> 
GEM_PHYMNTNC_REG_SHIFT;
 retval &= 0x;
 retval |= gem_phy_read(s, reg_num);
@@ -1569,7 +1569,7 @@ static void gem_write(void *opaque, hwaddr offset, 
uint64_t val,
 uint32_t phy_addr, reg_num;
 
 phy_addr = (val & GEM_PHYMNTNC_ADDR) >> GEM_PHYMNTNC_ADDR_SHFT;
-if (phy_addr == s->phy_addr || phy_addr == 0) {
+if (phy_addr == s->phy_addr) {
 reg_num = (val & GEM_PHYMNTNC_REG) >> GEM_PHYMNTNC_REG_SHIFT;
 gem_phy_write(s, reg_num, val);
 }
-- 
2.7.4

[PATCH v2 14/16] hw/riscv: microchip_pfsoc: Hook GPIO controllers

[Bin Meng]

From: Bin Meng 

Microchip PolarFire SoC integrates 3 GPIOs controllers. It seems
enough to create unimplemented devices to cover their register
spaces at this point.

With this commit, QEMU can boot to U-Boot (2nd stage bootloader)
all the way to the Linux shell login prompt, with a modified HSS
(1st stage bootloader).

For detailed instructions on how to create images for the Icicle
Kit board, please check QEMU RISC-V WiKi page at:
https://wiki.qemu.org/Documentation/Platforms/RISCV

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 include/hw/riscv/microchip_pfsoc.h |  3 +++
 hw/riscv/microchip_pfsoc.c | 14 ++
 2 files changed, 17 insertions(+)

diff --git a/include/hw/riscv/microchip_pfsoc.h 
b/include/hw/riscv/microchip_pfsoc.h
index 6d20853..8bfc7e1 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -89,6 +89,9 @@ enum {
 MICROCHIP_PFSOC_MMUART4,
 MICROCHIP_PFSOC_GEM0,
 MICROCHIP_PFSOC_GEM1,
+MICROCHIP_PFSOC_GPIO0,
+MICROCHIP_PFSOC_GPIO1,
+MICROCHIP_PFSOC_GPIO2,
 MICROCHIP_PFSOC_ENVM_CFG,
 MICROCHIP_PFSOC_ENVM_DATA,
 MICROCHIP_PFSOC_IOSCB_CFG,
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 7f25609..11ebdd1 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -89,6 +89,9 @@ static const struct MemmapEntry {
 [MICROCHIP_PFSOC_MMUART4] = { 0x20106000, 0x1000 },
 [MICROCHIP_PFSOC_GEM0] ={ 0x2011, 0x2000 },
 [MICROCHIP_PFSOC_GEM1] ={ 0x20112000, 0x2000 },
+[MICROCHIP_PFSOC_GPIO0] =   { 0x2012, 0x1000 },
+[MICROCHIP_PFSOC_GPIO1] =   { 0x20121000, 0x1000 },
+[MICROCHIP_PFSOC_GPIO2] =   { 0x20122000, 0x1000 },
 [MICROCHIP_PFSOC_ENVM_CFG] ={ 0x2020, 0x1000 },
 [MICROCHIP_PFSOC_ENVM_DATA] =   { 0x2022,0x2 },
 [MICROCHIP_PFSOC_IOSCB_CFG] =   { 0x3708, 0x1000 },
@@ -311,6 +314,17 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem1), 0,
 qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_GEM1_IRQ));
 
+/* GPIOs */
+create_unimplemented_device("microchip.pfsoc.gpio0",
+memmap[MICROCHIP_PFSOC_GPIO0].base,
+memmap[MICROCHIP_PFSOC_GPIO0].size);
+create_unimplemented_device("microchip.pfsoc.gpio1",
+memmap[MICROCHIP_PFSOC_GPIO1].base,
+memmap[MICROCHIP_PFSOC_GPIO1].size);
+create_unimplemented_device("microchip.pfsoc.gpio2",
+memmap[MICROCHIP_PFSOC_GPIO2].base,
+memmap[MICROCHIP_PFSOC_GPIO2].size);
+
 /* eNVM */
 memory_region_init_rom(envm_data, OBJECT(dev), "microchip.pfsoc.envm.data",
memmap[MICROCHIP_PFSOC_ENVM_DATA].size,
-- 
2.7.4

[PATCH v2 07/16] hw/sd: Add Cadence SDHCI emulation

[Bin Meng]

From: Bin Meng 

Cadence SD/SDIO/eMMC Host Controller (SD4HC) is an SDHCI compatible
controller. The SDHCI compatible registers start from offset 0x200,
which are called Slot Register Set (SRS) in its datasheet.

This creates a Cadence SDHCI model built on top of the existing
generic SDHCI model. Cadence specific Host Register Set (HRS) is
implemented to make guest software happy.

Signed-off-by: Bin Meng 
Acked-by: Philippe Mathieu-Daudé 

---

Changes in v2:
- change to update hw/sd/meson.build
- change the name to "generic-sdhci" when calling object_initialize_child()
- add a container MR to simplify out-of-bounds access checks

 include/hw/sd/cadence_sdhci.h |  47 ++
 hw/sd/cadence_sdhci.c | 193 ++
 hw/sd/Kconfig |   4 +
 hw/sd/meson.build |   1 +
 4 files changed, 245 insertions(+)
 create mode 100644 include/hw/sd/cadence_sdhci.h
 create mode 100644 hw/sd/cadence_sdhci.c

diff --git a/include/hw/sd/cadence_sdhci.h b/include/hw/sd/cadence_sdhci.h
new file mode 100644
index 000..cd8288b
--- /dev/null
+++ b/include/hw/sd/cadence_sdhci.h
@@ -0,0 +1,47 @@
+/*
+ * Cadence SDHCI emulation
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#ifndef CADENCE_SDHCI_H
+#define CADENCE_SDHCI_H
+
+#include "hw/sd/sdhci.h"
+
+#define CADENCE_SDHCI_REG_SIZE  0x100
+#define CADENCE_SDHCI_NUM_REGS  (CADENCE_SDHCI_REG_SIZE / sizeof(uint32_t))
+
+typedef struct CadenceSDHCIState {
+SysBusDevice parent;
+
+MemoryRegion container;
+MemoryRegion iomem;
+BusState *bus;
+
+uint32_t regs[CADENCE_SDHCI_NUM_REGS];
+
+SDHCIState sdhci;
+} CadenceSDHCIState;
+
+#define TYPE_CADENCE_SDHCI  "cadence.sdhci"
+#define CADENCE_SDHCI(obj)  OBJECT_CHECK(CadenceSDHCIState, (obj), \
+ TYPE_CADENCE_SDHCI)
+
+#endif /* CADENCE_SDHCI_H */
diff --git a/hw/sd/cadence_sdhci.c b/hw/sd/cadence_sdhci.c
new file mode 100644
index 000..0b371c8
--- /dev/null
+++ b/hw/sd/cadence_sdhci.c
@@ -0,0 +1,193 @@
+/*
+ * Cadence SDHCI emulation
+ *
+ * Copyright (c) 2020 Wind River Systems, Inc.
+ *
+ * Author:
+ *   Bin Meng 
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 or
+ * (at your option) version 3 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/bitops.h"
+#include "qemu/error-report.h"
+#include "qemu/log.h"
+#include "qapi/error.h"
+#include "migration/vmstate.h"
+#include "hw/irq.h"
+#include "hw/sd/cadence_sdhci.h"
+#include "sdhci-internal.h"
+
+/* HRS - Host Register Set (specific to Cadence) */
+
+#define CADENCE_SDHCI_HRS00 0x00/* general information */
+#define CADENCE_SDHCI_HRS00_SWR BIT(0)
+#define CADENCE_SDHCI_HRS00_POR_VAL 0x0001
+
+#define CADENCE_SDHCI_HRS04 0x10/* PHY access port */
+#define CADENCE_SDHCI_HRS04_WR  BIT(24)
+#define CADENCE_SDHCI_HRS04_RD  BIT(25)
+#define CADENCE_SDHCI_HRS04_ACK BIT(26)
+
+#define CADENCE_SDHCI_HRS06 0x18/* eMMC control */
+#define CADENCE_SDHCI_HRS06_TUNE_UP BIT(15)
+
+/* SRS - Slot Register Set (SDHCI-compatible) */
+
+#define CADENCE_SDHCI_SRS_BASE  0x200
+
+#define TO_REG(addr)((addr) / sizeof(uint32_t))
+
+static void cadence_sdhci_instance_init(Object *obj)
+{
+CadenceSDHCIState *s = CADENCE_SDHCI(obj);
+
+object_initialize_child(OBJECT(s), "generic-sdhci",
+&s->sdhci, TYPE_SYSBUS_SDHCI);
+}
+
+static void cadence_sdhci_reset(DeviceState *dev)
+{
+CadenceSDHCIState *s = CADENCE_SDHCI(dev);
+
+memset(s->regs, 0, CADENCE_SDHCI_REG_SIZE);
+s->regs[TO_REG(CADENCE_SDHCI_HRS00)] = CADENCE_SDHCI_HRS00_POR_VAL;
+
+device_cold_reset(DEVIC

[PATCH v2 15/16] hw/riscv: clint: Avoid using hard-coded timebase frequency

[Bin Meng]

From: Bin Meng 

At present the CLINT timestamp is using a hard-coded timebase
frequency value SIFIVE_CLINT_TIMEBASE_FREQ. This might not be
true for all boards.

Add a new 'timebase-freq' property to the CLINT device, and
update various functions to accept this as a parameter.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 

---

(no changes since v1)

 include/hw/riscv/sifive_clint.h |  4 +++-
 target/riscv/cpu.h  |  6 --
 hw/riscv/microchip_pfsoc.c  |  6 +-
 hw/riscv/sifive_clint.c | 26 +++---
 hw/riscv/sifive_e.c |  3 ++-
 hw/riscv/sifive_u.c |  3 ++-
 hw/riscv/spike.c|  3 ++-
 hw/riscv/virt.c |  3 ++-
 target/riscv/cpu_helper.c   |  4 +++-
 target/riscv/csr.c  |  4 ++--
 10 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/include/hw/riscv/sifive_clint.h b/include/hw/riscv/sifive_clint.h
index 9f5fb3d..a30be0f 100644
--- a/include/hw/riscv/sifive_clint.h
+++ b/include/hw/riscv/sifive_clint.h
@@ -39,11 +39,13 @@ typedef struct SiFiveCLINTState {
 uint32_t timecmp_base;
 uint32_t time_base;
 uint32_t aperture_size;
+uint32_t timebase_freq;
 } SiFiveCLINTState;
 
 DeviceState *sifive_clint_create(hwaddr addr, hwaddr size,
 uint32_t hartid_base, uint32_t num_harts, uint32_t sip_base,
-uint32_t timecmp_base, uint32_t time_base, bool provide_rdtime);
+uint32_t timecmp_base, uint32_t time_base, uint32_t timebase_freq,
+bool provide_rdtime);
 
 enum {
 SIFIVE_SIP_BASE = 0x0,
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index dc350f0..a003d83 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -219,7 +219,8 @@ struct CPURISCVState {
 pmp_table_t pmp_state;
 
 /* machine specific rdtime callback */
-uint64_t (*rdtime_fn)(void);
+uint64_t (*rdtime_fn)(uint32_t);
+uint32_t rdtime_fn_arg;
 
 /* True if in debugger mode.  */
 bool debugger;
@@ -349,7 +350,8 @@ void riscv_cpu_swap_hypervisor_regs(CPURISCVState *env);
 int riscv_cpu_claim_interrupts(RISCVCPU *cpu, uint32_t interrupts);
 uint32_t riscv_cpu_update_mip(RISCVCPU *cpu, uint32_t mask, uint32_t value);
 #define BOOL_TO_MASK(x) (-!!(x)) /* helper for riscv_cpu_update_mip value */
-void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(void));
+void riscv_cpu_set_rdtime_fn(CPURISCVState *env, uint64_t (*fn)(uint32_t),
+ uint32_t arg);
 #endif
 void riscv_cpu_set_mode(CPURISCVState *env, target_ulong newpriv);
 
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index 11ebdd1..da6bd29 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -60,6 +60,9 @@
 #define BIOS_FILENAME   "hss.bin"
 #define RESET_VECTOR0x2022
 
+/* CLINT timebase frequency */
+#define CLINT_TIMEBASE_FREQ 100
+
 /* GEM version */
 #define GEM_REVISION0x0107010c
 
@@ -187,7 +190,8 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 /* CLINT */
 sifive_clint_create(memmap[MICROCHIP_PFSOC_CLINT].base,
 memmap[MICROCHIP_PFSOC_CLINT].size, 0, ms->smp.cpus,
-SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE, false);
+SIFIVE_SIP_BASE, SIFIVE_TIMECMP_BASE, SIFIVE_TIME_BASE,
+CLINT_TIMEBASE_FREQ, false);
 
 /* L2 cache controller */
 create_unimplemented_device("microchip.pfsoc.l2cc",
diff --git a/hw/riscv/sifive_clint.c b/hw/riscv/sifive_clint.c
index 15e13d5..fa1ddf2 100644
--- a/hw/riscv/sifive_clint.c
+++ b/hw/riscv/sifive_clint.c
@@ -29,22 +29,23 @@
 #include "hw/riscv/sifive_clint.h"
 #include "qemu/timer.h"
 
-static uint64_t cpu_riscv_read_rtc(void)
+static uint64_t cpu_riscv_read_rtc(uint32_t timebase_freq)
 {
 return muldiv64(qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL),
-SIFIVE_CLINT_TIMEBASE_FREQ, NANOSECONDS_PER_SECOND);
+timebase_freq, NANOSECONDS_PER_SECOND);
 }
 
 /*
  * Called when timecmp is written to update the QEMU timer or immediately
  * trigger timer interrupt if mtimecmp <= current timer value.
  */
-static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value)
+static void sifive_clint_write_timecmp(RISCVCPU *cpu, uint64_t value,
+   uint32_t timebase_freq)
 {
 uint64_t next;
 uint64_t diff;
 
-uint64_t rtc_r = cpu_riscv_read_rtc();
+uint64_t rtc_r = cpu_riscv_read_rtc(timebase_freq);
 
 cpu->env.timecmp = value;
 if (cpu->env.timecmp <= rtc_r) {
@@ -59,7 +60,7 @@ static void sifive_clint_write_timecmp(RISCVCPU *cpu, 
uint64_t value)
 diff = cpu->env.timecmp - rtc_r;
 /* back to ns (note args switched in muldiv64) */
 next = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) +
-muldiv64(diff, NANOSECONDS_PER_SECOND, SIFIVE_CLINT_TIMEBASE_FREQ);
+muldiv64(diff, NANOSECONDS_PER_SECOND, timebase_freq);
 timer_mod(cpu->env.timer, next);
 }
 
@@ -112,10 +113,10 @@ static uint64_t sifive

[PATCH v2 08/16] hw/riscv: microchip_pfsoc: Connect a Cadence SDHCI controller and an SD card

[Bin Meng]

From: Bin Meng 

Microchip PolarFire SoC integrates one Cadence SDHCI controller.
On the Icicle Kit board, one eMMC chip and an external SD card
connect to this controller depending on different configuration.

As QEMU does not support eMMC yet, we just emulate the SD card
configuration. To test this, the Hart Software Services (HSS)
should choose the SD card configuration:

$ cp boards/icicle-kit-es/def_config.sdcard .config
$ make BOARD=icicle-kit-es

The SD card image can be built from the Yocto BSP at:
https://github.com/polarfire-soc/meta-polarfire-soc-yocto-bsp

Note the generated SD card image should be resized before use:
$ qemu-img resize /path/to/sdcard.img 4G

Launch QEMU with the following command:
$ qemu-system-riscv64 -nographic -M microchip-icicle-kit -sd sdcard.img

Signed-off-by: Bin Meng 

---

Changes in v2:
- do not initialize TYPE_SYSBUS_SDHCI in the SoC instance_init(),
  instead move that to the cadence_sdhci model
- do not access generic-sdhci's state directly,
  instead move that to the cadence_sdhci model

 include/hw/riscv/microchip_pfsoc.h |  4 
 hw/riscv/microchip_pfsoc.c | 23 +++
 hw/riscv/Kconfig   |  1 +
 3 files changed, 28 insertions(+)

diff --git a/include/hw/riscv/microchip_pfsoc.h 
b/include/hw/riscv/microchip_pfsoc.h
index a5efa1d..d810ee8 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -23,6 +23,7 @@
 #define HW_MICROCHIP_PFSOC_H
 
 #include "hw/char/mchp_pfsoc_mmuart.h"
+#include "hw/sd/cadence_sdhci.h"
 
 typedef struct MicrochipPFSoCState {
 /*< private >*/
@@ -39,6 +40,7 @@ typedef struct MicrochipPFSoCState {
 MchpPfSoCMMUartState *serial2;
 MchpPfSoCMMUartState *serial3;
 MchpPfSoCMMUartState *serial4;
+CadenceSDHCIState sdhci;
 } MicrochipPFSoCState;
 
 #define TYPE_MICROCHIP_PFSOC"microchip.pfsoc"
@@ -74,6 +76,7 @@ enum {
 MICROCHIP_PFSOC_MMUART0,
 MICROCHIP_PFSOC_SYSREG,
 MICROCHIP_PFSOC_MPUCFG,
+MICROCHIP_PFSOC_EMMC_SD,
 MICROCHIP_PFSOC_MMUART1,
 MICROCHIP_PFSOC_MMUART2,
 MICROCHIP_PFSOC_MMUART3,
@@ -85,6 +88,7 @@ enum {
 };
 
 enum {
+MICROCHIP_PFSOC_EMMC_SD_IRQ = 88,
 MICROCHIP_PFSOC_MMUART0_IRQ = 90,
 MICROCHIP_PFSOC_MMUART1_IRQ = 91,
 MICROCHIP_PFSOC_MMUART2_IRQ = 92,
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index cee959a..0b2e9ca 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -12,6 +12,7 @@
  * 1) PLIC (Platform Level Interrupt Controller)
  * 2) eNVM (Embedded Non-Volatile Memory)
  * 3) MMUARTs (Multi-Mode UART)
+ * 4) Cadence eMMC/SDHC controller and an SD card connected to it
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -75,6 +76,7 @@ static const struct MemmapEntry {
 [MICROCHIP_PFSOC_MMUART0] = { 0x2000, 0x1000 },
 [MICROCHIP_PFSOC_SYSREG] =  { 0x20002000, 0x2000 },
 [MICROCHIP_PFSOC_MPUCFG] =  { 0x20005000, 0x1000 },
+[MICROCHIP_PFSOC_EMMC_SD] = { 0x20008000, 0x1000 },
 [MICROCHIP_PFSOC_MMUART1] = { 0x2010, 0x1000 },
 [MICROCHIP_PFSOC_MMUART2] = { 0x20102000, 0x1000 },
 [MICROCHIP_PFSOC_MMUART3] = { 0x20104000, 0x1000 },
@@ -111,6 +113,9 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
 qdev_prop_set_string(DEVICE(&s->u_cpus), "cpu-type",
  TYPE_RISCV_CPU_SIFIVE_U54);
 qdev_prop_set_uint64(DEVICE(&s->u_cpus), "resetvec", RESET_VECTOR);
+
+object_initialize_child(obj, "sd-controller", &s->sdhci,
+TYPE_CADENCE_SDHCI);
 }
 
 static void microchip_pfsoc_soc_realize(DeviceState *dev, Error **errp)
@@ -223,6 +228,13 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 memmap[MICROCHIP_PFSOC_MPUCFG].base,
 memmap[MICROCHIP_PFSOC_MPUCFG].size);
 
+/* SDHCI */
+sysbus_realize(SYS_BUS_DEVICE(&s->sdhci), errp);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->sdhci), 0,
+memmap[MICROCHIP_PFSOC_EMMC_SD].base);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->sdhci), 0,
+qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_EMMC_SD_IRQ));
+
 /* MMUARTs */
 s->serial0 = mchp_pfsoc_mmuart_create(system_memory,
 memmap[MICROCHIP_PFSOC_MMUART0].base,
@@ -290,6 +302,7 @@ static void microchip_icicle_kit_machine_init(MachineState 
*machine)
 MicrochipIcicleKitState *s = MICROCHIP_ICICLE_KIT_MACHINE(machine);
 MemoryRegion *system_memory = get_system_memory();
 MemoryRegion *main_mem = g_new(MemoryRegion, 1);
+DriveInfo *dinfo = drive_get_next(IF_SD);
 
 /* Sanity check on RAM size */
 if (machine->ram_size < mc->default_ram_size) {
@@ -312,6 +325,16 @@ static void microchip_icicle_kit_machine_init(MachineState 
*machine)
 
 /* Load the firmware */
 riscv_find_and_load_firmwar

[PATCH v2 11/16] hw/net: cadence_gem: Add a new 'phy-addr' property

[Bin Meng]

From: Bin Meng 

At present the PHY address of the PHY connected to GEM is hard-coded
to either 23 (BOARD_PHY_ADDRESS) or 0. This might not be the case for
all boards. Add a new 'phy-addr' property so that board can specify
the PHY address for each GEM instance.

Signed-off-by: Bin Meng 

---

Changes in v2:
- change "phy-addr" default value to BOARD_PHY_ADDRESS

 include/hw/net/cadence_gem.h | 2 ++
 hw/net/cadence_gem.c | 5 +++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 54e646f..01c6189 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -73,6 +73,8 @@ typedef struct CadenceGEMState {
 /* Mask of register bits which are write 1 to clear */
 uint32_t regs_w1c[CADENCE_GEM_MAXREG];
 
+/* PHY address */
+uint8_t phy_addr;
 /* PHY registers backing store */
 uint16_t phy_regs[32];
 
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index a93b5c0..d80096b 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1446,7 +1446,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, 
unsigned size)
 uint32_t phy_addr, reg_num;
 
 phy_addr = (retval & GEM_PHYMNTNC_ADDR) >> GEM_PHYMNTNC_ADDR_SHFT;
-if (phy_addr == BOARD_PHY_ADDRESS || phy_addr == 0) {
+if (phy_addr == s->phy_addr || phy_addr == 0) {
 reg_num = (retval & GEM_PHYMNTNC_REG) >> 
GEM_PHYMNTNC_REG_SHIFT;
 retval &= 0x;
 retval |= gem_phy_read(s, reg_num);
@@ -1569,7 +1569,7 @@ static void gem_write(void *opaque, hwaddr offset, 
uint64_t val,
 uint32_t phy_addr, reg_num;
 
 phy_addr = (val & GEM_PHYMNTNC_ADDR) >> GEM_PHYMNTNC_ADDR_SHFT;
-if (phy_addr == BOARD_PHY_ADDRESS || phy_addr == 0) {
+if (phy_addr == s->phy_addr || phy_addr == 0) {
 reg_num = (val & GEM_PHYMNTNC_REG) >> GEM_PHYMNTNC_REG_SHIFT;
 gem_phy_write(s, reg_num, val);
 }
@@ -1682,6 +1682,7 @@ static Property gem_properties[] = {
 DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
 DEFINE_PROP_UINT32("revision", CadenceGEMState, revision,
GEM_MODID_VALUE),
+DEFINE_PROP_UINT8("phy-addr", CadenceGEMState, phy_addr, 
BOARD_PHY_ADDRESS),
 DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
   num_priority_queues, 1),
 DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState,
-- 
2.7.4

[PATCH v2 13/16] hw/riscv: microchip_pfsoc: Connect 2 Cadence GEMs

[Bin Meng]

From: Bin Meng 

Microchip PolarFire SoC integrates 2 Candence GEMs to provide
IEEE 802.3 standard-compliant 10/100/1000 Mbps ethernet interface.

On the Icicle Kit board, GEM0 connects to a PHY at address 8 while
GEM1 connects to a PHY at address 9.

The 2nd stage bootloader (U-Boot) is using GEM1 by default, so we
must specify 2 '-nic' options from the command line in order to get
a working ethernet.

Signed-off-by: Bin Meng 
Reviewed-by: Alistair Francis 
---

(no changes since v1)

 include/hw/riscv/microchip_pfsoc.h |  7 +++
 hw/riscv/microchip_pfsoc.c | 39 ++
 2 files changed, 46 insertions(+)

diff --git a/include/hw/riscv/microchip_pfsoc.h 
b/include/hw/riscv/microchip_pfsoc.h
index 63e7860..6d20853 100644
--- a/include/hw/riscv/microchip_pfsoc.h
+++ b/include/hw/riscv/microchip_pfsoc.h
@@ -24,6 +24,7 @@
 
 #include "hw/char/mchp_pfsoc_mmuart.h"
 #include "hw/dma/sifive_pdma.h"
+#include "hw/net/cadence_gem.h"
 #include "hw/sd/cadence_sdhci.h"
 
 typedef struct MicrochipPFSoCState {
@@ -42,6 +43,8 @@ typedef struct MicrochipPFSoCState {
 MchpPfSoCMMUartState *serial3;
 MchpPfSoCMMUartState *serial4;
 SiFivePDMAState dma;
+CadenceGEMState gem0;
+CadenceGEMState gem1;
 CadenceSDHCIState sdhci;
 } MicrochipPFSoCState;
 
@@ -84,6 +87,8 @@ enum {
 MICROCHIP_PFSOC_MMUART2,
 MICROCHIP_PFSOC_MMUART3,
 MICROCHIP_PFSOC_MMUART4,
+MICROCHIP_PFSOC_GEM0,
+MICROCHIP_PFSOC_GEM1,
 MICROCHIP_PFSOC_ENVM_CFG,
 MICROCHIP_PFSOC_ENVM_DATA,
 MICROCHIP_PFSOC_IOSCB_CFG,
@@ -99,6 +104,8 @@ enum {
 MICROCHIP_PFSOC_DMA_IRQ5 = 10,
 MICROCHIP_PFSOC_DMA_IRQ6 = 11,
 MICROCHIP_PFSOC_DMA_IRQ7 = 12,
+MICROCHIP_PFSOC_GEM0_IRQ = 64,
+MICROCHIP_PFSOC_GEM1_IRQ = 70,
 MICROCHIP_PFSOC_EMMC_SD_IRQ = 88,
 MICROCHIP_PFSOC_MMUART0_IRQ = 90,
 MICROCHIP_PFSOC_MMUART1_IRQ = 91,
diff --git a/hw/riscv/microchip_pfsoc.c b/hw/riscv/microchip_pfsoc.c
index d8ec973..7f25609 100644
--- a/hw/riscv/microchip_pfsoc.c
+++ b/hw/riscv/microchip_pfsoc.c
@@ -14,6 +14,7 @@
  * 3) MMUARTs (Multi-Mode UART)
  * 4) Cadence eMMC/SDHC controller and an SD card connected to it
  * 5) SiFive Platform DMA (Direct Memory Access Controller)
+ * 6) GEM (Gigabit Ethernet MAC Controller)
  *
  * This board currently generates devicetree dynamically that indicates at 
least
  * two harts and up to five harts.
@@ -59,6 +60,9 @@
 #define BIOS_FILENAME   "hss.bin"
 #define RESET_VECTOR0x2022
 
+/* GEM version */
+#define GEM_REVISION0x0107010c
+
 static const struct MemmapEntry {
 hwaddr base;
 hwaddr size;
@@ -83,6 +87,8 @@ static const struct MemmapEntry {
 [MICROCHIP_PFSOC_MMUART2] = { 0x20102000, 0x1000 },
 [MICROCHIP_PFSOC_MMUART3] = { 0x20104000, 0x1000 },
 [MICROCHIP_PFSOC_MMUART4] = { 0x20106000, 0x1000 },
+[MICROCHIP_PFSOC_GEM0] ={ 0x2011, 0x2000 },
+[MICROCHIP_PFSOC_GEM1] ={ 0x20112000, 0x2000 },
 [MICROCHIP_PFSOC_ENVM_CFG] ={ 0x2020, 0x1000 },
 [MICROCHIP_PFSOC_ENVM_DATA] =   { 0x2022,0x2 },
 [MICROCHIP_PFSOC_IOSCB_CFG] =   { 0x3708, 0x1000 },
@@ -119,6 +125,9 @@ static void microchip_pfsoc_soc_instance_init(Object *obj)
 object_initialize_child(obj, "dma-controller", &s->dma,
 TYPE_SIFIVE_PDMA);
 
+object_initialize_child(obj, "gem0", &s->gem0, TYPE_CADENCE_GEM);
+object_initialize_child(obj, "gem1", &s->gem1, TYPE_CADENCE_GEM);
+
 object_initialize_child(obj, "sd-controller", &s->sdhci,
 TYPE_CADENCE_SDHCI);
 }
@@ -134,6 +143,7 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 MemoryRegion *envm_data = g_new(MemoryRegion, 1);
 char *plic_hart_config;
 size_t plic_hart_config_len;
+NICInfo *nd;
 int i;
 
 sysbus_realize(SYS_BUS_DEVICE(&s->e_cpus), &error_abort);
@@ -272,6 +282,35 @@ static void microchip_pfsoc_soc_realize(DeviceState *dev, 
Error **errp)
 qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_MMUART4_IRQ),
 serial_hd(4));
 
+/* GEMs */
+
+nd = &nd_table[0];
+if (nd->used) {
+qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
+qdev_set_nic_properties(DEVICE(&s->gem0), nd);
+}
+nd = &nd_table[1];
+if (nd->used) {
+qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
+qdev_set_nic_properties(DEVICE(&s->gem1), nd);
+}
+
+object_property_set_int(OBJECT(&s->gem0), "revision", GEM_REVISION, errp);
+object_property_set_int(OBJECT(&s->gem0), "phy-addr", 8, errp);
+sysbus_realize(SYS_BUS_DEVICE(&s->gem0), errp);
+sysbus_mmio_map(SYS_BUS_DEVICE(&s->gem0), 0,
+memmap[MICROCHIP_PFSOC_GEM0].base);
+sysbus_connect_irq(SYS_BUS_DEVICE(&s->gem0), 0,
+qdev_get_gpio_in(DEVICE(s->plic), MICROCHIP_PFSOC_GEM0_IRQ));
+
+object_property_set_int(OBJECT

Re: [PATCH 6/7] hw/arm/virt: spe: Add SPE fdt binding for virt machine

[Auger Eric]

Hi Haibo,

On 8/7/20 10:10 AM, Haibo Xu wrote:
> Add a virtual SPE device for virt machine while using PPI 
> 5 for SPE overflow interrupt number.
> 
> Signed-off-by: Haibo Xu 
> ---
>  hw/arm/virt-acpi-build.c|  3 +++ 
>  hw/arm/virt.c   | 42 +
>  include/hw/acpi/acpi-defs.h |  1 + 
>  include/hw/arm/virt.h   |  1 + 
>  target/arm/cpu.c|  2 ++
>  target/arm/cpu.h|  2 ++
>  target/arm/kvm.c|  6 ++
>  7 files changed, 57 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 91f0df7b13..5073ba22a5 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -666,6 +666,9 @@ build_madt(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>  if (arm_feature(&armcpu->env, ARM_FEATURE_PMU)) {
>  gicc->performance_interrupt = cpu_to_le32(PPI(VIRTUAL_PMU_IRQ));
>  }
> +if (arm_feature(&armcpu->env, ARM_FEATURE_SPE)) {
> +gicc->spe_interrupt = cpu_to_le32(PPI(VIRTUAL_SPE_IRQ));
> +}
>  if (vms->virt) {
>  gicc->vgic_interrupt = cpu_to_le32(PPI(ARCH_GIC_MAINT_IRQ));
>  }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index ecfee362a1..c40819705d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -555,6 +555,42 @@ static void fdt_add_pmu_nodes(const VirtMachineState 
> *vms)
>  }
>  }
> 
> +static void fdt_add_spe_nodes(const VirtMachineState *vms)
> +{
> +CPUState *cpu;
> +ARMCPU *armcpu;
> +uint32_t irqflags = GIC_FDT_IRQ_FLAGS_LEVEL_HI;
> +
> +CPU_FOREACH(cpu) {
> +armcpu = ARM_CPU(cpu);
> +if (!arm_feature(&armcpu->env, ARM_FEATURE_SPE)) {
> +return;
> +}
> +if (kvm_enabled()) {
> +if (kvm_irqchip_in_kernel()) {
> +kvm_arm_spe_set_irq(cpu, PPI(VIRTUAL_SPE_IRQ));
> +}
> +kvm_arm_spe_init(cpu);
> +}
> +}
> +
> +if (vms->gic_version == VIRT_GIC_VERSION_2) {
> +irqflags = deposit32(irqflags, GIC_FDT_IRQ_PPI_CPU_START,
> + GIC_FDT_IRQ_PPI_CPU_WIDTH,
> + (1 << vms->smp_cpus) - 1);
> +}
> +
> +armcpu = ARM_CPU(qemu_get_cpu(0));
> +qemu_fdt_add_subnode(vms->fdt, "/spe");
> +if (arm_feature(&armcpu->env, ARM_FEATURE_V8)) {
> +const char compat[] = "arm,statistical-profiling-extension-v1";
> +qemu_fdt_setprop(vms->fdt, "/spe", "compatible",
> + compat, sizeof(compat));
> +qemu_fdt_setprop_cells(vms->fdt, "/spe", "interrupts",
> +   GIC_FDT_IRQ_TYPE_PPI, VIRTUAL_SPE_IRQ, 
> irqflags);
> +}
> +}
> +
>  static inline DeviceState *create_acpi_ged(VirtMachineState *vms)
>  {
>  DeviceState *dev;
> @@ -727,6 +763,10 @@ static void create_gic(VirtMachineState *vms)
>  qdev_get_gpio_in(vms->gic, ppibase
>   + VIRTUAL_PMU_IRQ));
> 
> +qdev_connect_gpio_out_named(cpudev, "spe-interrupt", 0,
> +qdev_get_gpio_in(vms->gic, ppibase
> + + VIRTUAL_SPE_IRQ));
> +
>  sysbus_connect_irq(gicbusdev, i, qdev_get_gpio_in(cpudev, 
> ARM_CPU_IRQ));
>  sysbus_connect_irq(gicbusdev, i + smp_cpus,
> qdev_get_gpio_in(cpudev, ARM_CPU_FIQ));
> @@ -1915,6 +1955,8 @@ static void machvirt_init(MachineState *machine)
> 
>  fdt_add_pmu_nodes(vms);
> 
> +fdt_add_spe_nodes(vms);
> +
>  create_uart(vms, VIRT_UART, sysmem, serial_hd(0));
> 
>  if (vms->secure) {
> diff --git a/include/hw/acpi/acpi-defs.h b/include/hw/acpi/acpi-defs.h
> index 38a42f409a..56a7f38ae4 100644
> --- a/include/hw/acpi/acpi-defs.h
> +++ b/include/hw/acpi/acpi-defs.h
> @@ -302,6 +302,7 @@ struct AcpiMadtGenericCpuInterface {
>  uint32_t vgic_interrupt;
>  uint64_t gicr_base_address;
>  uint64_t arm_mpidr;
> +uint16_t spe_interrupt; /* ACPI 6.3 */
This does not work for me.
You miss 2 uint8_t fields inbetween arm_mpdir and spe_interrupt:
Processor Power Efficiency Class and Reserved.

At the moment arm_spe_acpi_register_device() silently fails on guest
side since
gicc->header.length < ACPI_MADT_GICC_SPE

Thanks

Eric

>  } QEMU_PACKED;
> 
>  typedef struct AcpiMadtGenericCpuInterface AcpiMadtGenericCpuInterface;
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index dff67e1bef..56c83224d2 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -49,6 +49,7 @@
>  #define ARCH_TIMER_NS_EL1_IRQ 14
>  #define ARCH_TIMER_NS_EL2_IRQ 10
> 
> +#define VIRTUAL_SPE_IRQ 5
>  #define VIRTUAL_PMU_IRQ 7
> 
>  #define PPI(irq) ((irq) + 16)
> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
> index 40768b4d19..67ab0089fd 100644
> --- a/target/arm/cpu.c
> +++ b/target/arm/cpu.c
> @@ 

Re: [PATCH v2 24/45] target/arm: Implement fp16 for Neon VRECPE, VRSQRTE using gvec

[Richard Henderson]

On 8/29/20 6:53 AM, Peter Maydell wrote:
> On Fri, 28 Aug 2020 at 23:53, Richard Henderson
>  wrote:
>>
>> On 8/28/20 2:40 PM, Peter Maydell wrote:
>>> The other approach would be to standardize on "the decodetree pattern
>>> always converts the size to the data-type size, regardless of how
>>> it's encoded in the insn fields", and then you could check against
>>> MO_16 here. Would that be better ?
>>
>> That might be clearer, yes.  Otherwise it's hard to tell what "size" means
>> without looking at the manual for each instance.
> 
> Do you mind if I do that as a separate patchset after this one?
> I feel that will be easier than trying to weave the change into
> this series...

Yes, certainly.


r~

Re: [PATCH 62/77] target/microblaze: Try to keep imm and delay slot together

[Richard Henderson]

On 8/27/20 12:17 PM, Edgar E. Iglesias wrote:
> On Tue, Aug 25, 2020 at 01:59:35PM -0700, Richard Henderson wrote:
>> If the last insn on a page is imm, or a branch with delay slot,
>> then end a tb early if this has not begun the tb.  If it has
>> begun the tb, then we can allow the tb to span two pages as if
>> the imm plus its consumer, or branch plus delay, or imm plus
>> branch plus delay, are a single insn.
>>
>> If the insn in the delay slot faults, then the exception handler
>> will have reset the PC to the beginning of this sequence anyway,
>> via the stored D_FLAG and BIMM_FLAG bits.
>>
>> Disable all of this when single-stepping.
> 
> 
> Hi Richard,
> 
> We've got a Linux boot that fails after applying this patch.
> It goes from always working to only working like 1 out of 3 times.
> It fails deep in user-space so I don't have a good log for it.

I think I've found it: do_rti, do_rtb, do_rte are acting as normal jumps.  They
need to set DISAS_UPDATE so that they return to the main loop to re-evaluate
interrupts.

I'll come back to this after we deal with the other 76 patches.


r~

Re: [PATCH] meson: move zlib detection to meson

[Marc-André Lureau]

On Fri, Aug 28, 2020 at 9:38 PM Paolo Bonzini  wrote:
>
> Meson includes the same logic that tries to look for -lz if
> pkg-config (and cmake) cannot find zlib.  The undocumented
> --disable-zlib-test option becomes a no-op.
>
> Signed-off-by: Paolo Bonzini 

Nice! (special treatment since 0.54),
Reviewed-by: Marc-André Lureau 

> ---
>  Makefile|  2 +-
>  configure   | 32 +---
>  meson.build |  6 +-
>  3 files changed, 3 insertions(+), 37 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 81794d5c34..1520a2eab5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -132,7 +132,7 @@ configure: ;
>
>  $(call set-vpath, $(SRC_PATH))
>
> -LIBS+=-lz $(LIBS_TOOLS)
> +LIBS+=$(LIBS_TOOLS)
>
>  SUBDIR_MAKEFLAGS=$(if $(V),,--no-print-directory --quiet) 
> BUILD_DIR=$(BUILD_DIR)
>
> diff --git a/configure b/configure
> index 6ecaff429b..e035eb2868 100755
> --- a/configure
> +++ b/configure
> @@ -501,7 +501,6 @@ opengl=""
>  opengl_dmabuf="no"
>  cpuid_h="no"
>  avx2_opt=""
> -zlib="yes"
>  capstone=""
>  lzo=""
>  snappy=""
> @@ -1423,7 +1422,7 @@ for opt do
>;;
>--enable-usb-redir) usb_redir="yes"
>;;
> -  --disable-zlib-test) zlib="no"
> +  --disable-zlib-test)
>;;
>--disable-lzo) lzo="no"
>;;
> @@ -3898,30 +3897,6 @@ if ! compile_prog "$glib_cflags -Werror" "$glib_libs" 
> ; then
>  fi
>  fi
>
> -#
> -# zlib check
> -
> -if test "$zlib" != "no" ; then
> -if $pkg_config --exists zlib; then
> -zlib_cflags=$($pkg_config --cflags zlib)
> -zlib_libs=$($pkg_config --libs zlib)
> -QEMU_CFLAGS="$zlib_cflags $QEMU_CFLAGS"
> -LIBS="$zlib_libs $LIBS"
> -else
> -cat > $TMPC << EOF
> -#include 
> -int main(void) { zlibVersion(); return 0; }
> -EOF
> -if compile_prog "" "-lz" ; then
> -zlib_libs=-lz
> -LIBS="$LIBS $zlib_libs"
> -else
> -error_exit "zlib check failed" \
> -"Make sure to have the zlib libs and headers installed."
> -fi
> -fi
> -fi
> -
>  ##
>  # SHA command probe for modules
>  if test "$modules" = yes; then
> @@ -7129,11 +7104,6 @@ fi
>  if test "$posix_memalign" = "yes" ; then
>echo "CONFIG_POSIX_MEMALIGN=y" >> $config_host_mak
>  fi
> -if test "$zlib" != "no" ; then
> -echo "CONFIG_ZLIB=y" >> $config_host_mak
> -echo "ZLIB_CFLAGS=$zlib_cflags" >> $config_host_mak
> -echo "ZLIB_LIBS=$zlib_libs" >> $config_host_mak
> -fi
>  if test "$spice" = "yes" ; then
>echo "CONFIG_SPICE=y" >> $config_host_mak
>echo "SPICE_CFLAGS=$spice_cflags" >> $config_host_mak
> diff --git a/meson.build b/meson.build
> index 74f8ea0c2e..35e6f8688d 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -134,11 +134,7 @@ if 'CONFIG_AUTH_PAM' in config_host
>pam = cc.find_library('pam')
>  endif
>  libaio = cc.find_library('aio', required: false)
> -zlib = not_found
> -if 'CONFIG_ZLIB' in config_host
> -  zlib = declare_dependency(compile_args: config_host['ZLIB_CFLAGS'].split(),
> -link_args: config_host['ZLIB_LIBS'].split())
> -endif
> +zlib = dependency('zlib', required: true)
>  linux_io_uring = not_found
>  if 'CONFIG_LINUX_IO_URING' in config_host
>linux_io_uring = declare_dependency(compile_args: 
> config_host['LINUX_IO_URING_CFLAGS'].split(),
> --
> 2.26.2
>

Re: [PATCH] target/xtensa: enable all coprocessors for linux-user

[Richard Henderson]

On 8/29/20 3:47 AM, Max Filippov wrote:
> Linux userspace always sees coprocessors as enabled. CPENABLE register
> and coprocessor exceptions are used internally by the kernel to manage
> lazy coprocessor context switch. None of it is needed for linux-user.
> Always enable all coprocessors for user emulation.
> 
> Signed-off-by: Max Filippov 

Reviewed-by: Richard Henderson 


r~

Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

[Richard Henderson]

On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
> The S24/TCX datasheet is listed as "Unable to locate" on [1].
> 
> However the NetBSD revision 1.32 of the driver introduced
> 64-bit accesses to the stippler and blitter [2]. It is safe
> to assume these memory regions are 64-bit accessible.
> QEMU implementation is 32-bit, so fill the 'impl' fields.
> 
> [1] 
> http://web.archive.org/web/20111209011516/http://wikis.sun.com/display/FOSSdocs/Home
> [2] 
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/sbus/tcx.c.diff?r1=1.31&r2=1.32
> 
> Reported-by: Andreas Gustafsson 
> Buglink: https://bugs.launchpad.net/bugs/1892540
> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Since v1:
> - added missing uncommitted staged changes... (tcx_blit_ops)
> ---
>  hw/display/tcx.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PATCH v2] memory: Directly dispatch alias accesses on origin memory region

[Richard Henderson]

On 8/22/20 9:14 AM, Philippe Mathieu-Daudé wrote:
> There is an issue when accessing an alias memory region via the
> memory_region_dispatch_read() / memory_region_dispatch_write()
> calls.
> 
> Since commit 2cdfcf272d ("memory: assign MemoryRegionOps to all
> regions") we assign &unassigned_mem_ops to all MemoryRegions.
> 
> The memory_region_init_alias() flow is:
> 
>   memory_region_init_alias()
>   -> memory_region_init()
>  -> object_initialize(TYPE_MEMORY_REGION)
> -> memory_region_initfn()
>-> mr->ops = &unassigned_mem_ops;
> 
> When accessing the alias using memory_region_dispatch_read(),
> flow is:
> 
>   memory_region_dispatch_read()
>   -> memory_region_access_valid(mr)
>  -> mr->ops->valid.accepts()
> -> unassigned_mem_accepts()
> <- false
>  <- false
><- MEMTX_DECODE_ERROR
> 
> The caller gets a MEMTX_DECODE_ERROR while the access is OK.
> 
> Fix by directly dispatching aliases accesses to its origin region.
> 
> Fixes: 2cdfcf272d ("memory: assign MemoryRegionOps to all regions")
> Signed-off-by: Philippe Mathieu-Daudé 
> ---
> Since v1:
> - reword as not a bugfix (Paolo)
> - access the aliases recursively (Paolo)
> ---
>  softmmu/memory.c | 8 
>  1 file changed, 8 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 07/70] target/riscv: rvv-1.0: remove rvv related codes from fcsr registers

[Richard Henderson]

On 8/17/20 1:48 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> * Remove VXRM and VXSAT fields from FCSR register as they are only
>   presented in VCSR register.
> * Remove RVV loose check in fs() predicate function.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/csr.c | 13 -
>  1 file changed, 13 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [PULL 00/18] riscv-to-apply queue

[LIU Zhiwei]

On 2020/8/26 6:30, Alistair Francis wrote:

On Tue, Aug 25, 2020 at 2:50 PM Peter Maydell  wrote:

On Tue, 25 Aug 2020 at 22:32, Alistair Francis  wrote:

On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell  wrote:

The hypervisor related patches don't seem to have any
reviewed-by tags, which seems a shame for a fairly significant
chunk of work. Is there really nobody who can review them
for you ?

Unfortunately not. They have been on the list since April and haven't
received any feedback.

There isn't a lot of people reviewing the RISC-V patches unfortunately.

:-(   I'd hoped it was a more active target than that.

There are lots of active contributors, we are just short on reviewers.

Richard and Philippe review patches and some of the RISC-V patches get
reviewed by the RISC-V community. The main problem (which is a common
problem in open source) is that large technical patch series just get
ignored.

Hi Alistair,

It's really a pity.

I will review every patch that CC me in no more than a week if no other 
people reviewed this patch.


So if there too many patches, just ease to CC me.


Best Regards,
Zhiwei


Alistair


-- PMM

Re: [RFC v4 12/70] target/riscv: rvv-1.0: add fractional LMUL

[Richard Henderson]

On 8/17/20 1:48 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Introduce the concepts of fractional LMUL for RVV 1.0.
> In RVV 1.0, LMUL bits are contiguous in vtype register.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/cpu.h   | 15 ---
>  target/riscv/translate.c | 16 ++--
>  target/riscv/vector_helper.c | 16 ++--
>  3 files changed, 36 insertions(+), 11 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

[Philippe Mathieu-Daudé]

Le sam. 29 août 2020 18:14, Michael  a écrit :

> Hello,
>
> since I wrote the NetBSD code in question, here are my 2 cent:
>
> On Sat, 29 Aug 2020 08:41:43 -0700
> Richard Henderson  wrote:
>
> > On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
> > > The S24/TCX datasheet is listed as "Unable to locate" on [1].
>
> I don't have it either, but someone did a lot of reverse engineering
> and gave me his notes. The hardware isn't that complicated, but quite
> weird.
>
> > > However the NetBSD revision 1.32 of the driver introduced
> > > 64-bit accesses to the stippler and blitter [2]. It is safe
> > > to assume these memory regions are 64-bit accessible.
> > > QEMU implementation is 32-bit, so fill the 'impl' fields.
>
> IIRC the real hardware *requires* 64bit accesses for stipple and
> blitter operations to work. For stipples you write a 64bit word into
> STIP space, the address defines where in the framebuffer you want to
> draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> BLIT space works similarly, the 64bit word contains an offset were to
> read pixels from, and how many you want to copy.
>

Thanks Michael for this information!
If you don't mind I'll amend it to the commit description so there is a
reference for posterity.

I'm waiting for *Andreas Gustafsson to test it then will repost.*


> have fun
> Michael
>

Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

[Michael]

Hello,

since I wrote the NetBSD code in question, here are my 2 cent:

On Sat, 29 Aug 2020 08:41:43 -0700
Richard Henderson  wrote:

> On 8/22/20 7:21 AM, Philippe Mathieu-Daudé wrote:
> > The S24/TCX datasheet is listed as "Unable to locate" on [1].

I don't have it either, but someone did a lot of reverse engineering
and gave me his notes. The hardware isn't that complicated, but quite
weird.

> > However the NetBSD revision 1.32 of the driver introduced
> > 64-bit accesses to the stippler and blitter [2]. It is safe
> > to assume these memory regions are 64-bit accessible.
> > QEMU implementation is 32-bit, so fill the 'impl' fields.

IIRC the real hardware *requires* 64bit accesses for stipple and
blitter operations to work. For stipples you write a 64bit word into
STIP space, the address defines where in the framebuffer you want to
draw, the data contain a 32bit bitmask, foreground colour and a ROP.
BLIT space works similarly, the 64bit word contains an offset were to
read pixels from, and how many you want to copy.

have fun
Michael

Re: [PULL 00/18] riscv-to-apply queue

[Alistair Francis]

On Sat, Aug 29, 2020 at 8:50 AM LIU Zhiwei  wrote:
>
>
>
> On 2020/8/26 6:30, Alistair Francis wrote:
> > On Tue, Aug 25, 2020 at 2:50 PM Peter Maydell  
> > wrote:
> >> On Tue, 25 Aug 2020 at 22:32, Alistair Francis  
> >> wrote:
> >>> On Tue, Aug 25, 2020 at 2:24 PM Peter Maydell  
> >>> wrote:
>  The hypervisor related patches don't seem to have any
>  reviewed-by tags, which seems a shame for a fairly significant
>  chunk of work. Is there really nobody who can review them
>  for you ?
> >>> Unfortunately not. They have been on the list since April and haven't
> >>> received any feedback.
> >>>
> >>> There isn't a lot of people reviewing the RISC-V patches unfortunately.
> >> :-(   I'd hoped it was a more active target than that.
> > There are lots of active contributors, we are just short on reviewers.
> >
> > Richard and Philippe review patches and some of the RISC-V patches get
> > reviewed by the RISC-V community. The main problem (which is a common
> > problem in open source) is that large technical patch series just get
> > ignored.
> Hi Alistair,
>
> It's really a pity.
>
> I will review every patch that CC me in no more than a week if no other
> people reviewed this patch.
>
> So if there too many patches, just ease to CC me.

Thanks!

If you want to review more it's also a good idea to sign up to the
RISC-V QEMU mailing list. That way you can keep an eye on all patches
and start with reviewing ones that are interesting to you.

Alistair

>
>
> Best Regards,
> Zhiwei
> >
> > Alistair
> >
> >> -- PMM
>

Re: [RFC v4 14/70] target/riscv: rvv-1.0: update check functions

[Richard Henderson]

On 8/17/20 1:48 AM, frank.ch...@sifive.com wrote:
> +static inline bool is_aligned(const uint8_t val, const uint8_t pos)
> +{
> +return pos ? (val & (pos - 1)) == 0 : true;
> +}

The truncation to uint8_t from int is odd.  Can we drop all of that and just
use int?

Looking at the uses, I think that you should pass lmul directly instead of
requiring the callers to all compute 1 << lmul, and also verify that lmul is
positive.

That change makes this function look like

return lmul <= 0 || extract32(val, 0, lmul) == 0;


> +static inline bool is_overlapped(const uint8_t astart, uint8_t asize,
> + const uint8_t bstart, uint8_t bsize)
> +{
> +asize = asize == 0 ? 1 : asize;
> +bsize = bsize == 0 ? 1 : bsize;

This looks odd.  Again, I think passing in lmul would be better than size.
Then compute size here locally:

int asize = amul <= 0 ? 1 : 1 << amul;

> +
> +const int aend = astart + asize;
> +const int bend = bstart + bsize;
> +
> +return MAX(aend, bend) - MIN(astart, bstart) < asize + bsize;
> +}
> +
> +static inline bool is_overlapped_widen(const uint8_t astart, uint8_t asize,
> +   const uint8_t bstart, uint8_t bsize)

This needs more comments, I think.  It's not obvious why this is (or needs to
be) different from is_overlapped.

I think you're trying to implement the

  * destination eew smaller than source eew,
and overlap is allowed at the beginning.
  * destination eew larger than source eew,
and overlap is allowed at the end.

rule from section 5.2.  But since you're not comparing asize vs bsize, that's
not what you're doing.

Anyway, I think all of these rules can be put into require_noover, and there
need not be a separate require_noover_widen.

> +static bool require_rvv(DisasContext *s)
> +{
> +if (s->mstatus_vs == 0) {
> +return false;
> +}
> +return true;

return s->mstatus_vs != 0;

> +static bool vext_check_sss(DisasContext *s, int vd, int vs1,
> +   int vs2, int vm, bool is_vs1)
> +{
> +bool ret = require_vm(vm, vd);
> +if (s->lmul > 0) {
> +ret &= require_align(vd, 1 << s->lmul) &&
> +   require_align(vs2, 1 << s->lmul);
> +if (is_vs1) {
> +ret &= require_align(vs1, 1 << s->lmul);
> +}
> +}
> +return ret;
> +}

I think this (and similar function taking is_vs1) should be split.  All callers
pass a constant value, and thus can just as easily call a different function.

Perhaps

static bool vext_check_ss(DisasContext *s, int vd,
  int vs2, int vm)
{
return (require_vm(vm, vd) &&
require_align(vd, s->lmul) &&
require_align(vs2, s->lmul));
}

static bool vext_check_sss(DisasContext *s, int vd, int vs1,
   int vs2, int vm)
{
return (vext_check_ss(s, vd, vs2, vm) &&
require_align(vs1, s->lmul));
}

> +/*
> + * Check function for maskable vector instruction with format:
> + * single-width result and single-width sources (SEW = SEW op SEW)
> + *
> + * is_vs1: indicates whether insn[19:15] is a vs1 field or not.
> + *
> + * Rules to be checked here:
> + *   1. Source (vs2, vs1) vector register number are multiples of LMUL.
> + *  (Section 3.3.2)
> + *   2. Destination vector register cannot overlap a source vector
> + *  register (vs2, vs1) group.
> + *  (Section 5.2)
> + */
> +static bool vext_check_mss(DisasContext *s, int vd, int vs1,
> +   int vs2, bool is_vs1)
>  {
> +bool ret = require_align(vs2, 1 << s->lmul);
> +if (vd != vs2) {
> +ret &= require_noover(vd, 1, vs2, 1 << s->lmul);
> +}
> +if (is_vs1) {
> +if (vd != vs1) {
> +ret &= require_noover(vd, 1, vs1, 1 << s->lmul);
> +}
> +ret &= require_align(vs1, 1 << s->lmul);
> +}
> +return ret;
> +}

If require_noover implements all of the overlap rules, as suggested, this
simplifies to

static bool vext_check_ms(DisasContext *s, int vd, int vs2)
{
return (require_align(vs2, s->lmul) &&
require_noover(vd, 0, vs2, s->lmul);
}

static bool vext_check_mss(DisasContext *s, int vd,
   int vs1, int vs2)
{
return (vext_check_ms(s, vd, vs2) &&
require_align(vs1, s->lmul) &&
require_noover(vd, 0, vs1, s->lmul));
}

> +/*
> + * Common check function for vector widening instructions
> + * of double-width result (2*SEW).
> + *
> + * Rules to be checked here:
> + *   1. The largest vector register group used by an instruction
> + *  can not be greater than 8 vector registers (Section 5.2):
> + *  => LMUL < 8.
> + *  => SEW < 64.
> + *   2. Destination vector register number is multiples of 2 * LMUL.
> + *  (Section 3.3.2, 11.2)
> + *   3. Destination vector register group for a masked vector
> + *  instruction cannot overlap the source mask register (v0).
> + *

Re: [RFC v4 15/70] target/riscv: introduce more imm value modes in translator functions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Immediate value in translator function is extended not only
> zero-extended and sign-extended but with more modes to be applicable
> with multiple formats of vector instructions.
> 
> * IMM_ZX: Zero-extended
> * IMM_SX: Sign-extended
> * IMM_TRUNC_SEW:  Truncate to log(SEW) bit
> * IMM_TRUNC_2SEW: Truncate to log(2*SEW) bit
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 115 ++--
>  1 file changed, 66 insertions(+), 49 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [RFC v4 16/70] target/riscv: rvv:1.0: add translation-time nan-box helper function

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> * Add fp16 nan-box check generator function, if a 16-bit input is not
>   properly nanboxed, then the input is replaced with the default qnan.
> * Add do_nanbox() helper function to utilize gen_check_nanbox_X() to
>   generate the NaN-boxed floating-point values based on SEW setting.
> * Apply nanbox helper in opfvf_trans
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 35 -
>  target/riscv/translate.c| 10 +++
>  2 files changed, 44 insertions(+), 1 deletion(-)

Reviewed-by: Richard Henderson 

r~

Re: [RFC v4 18/70] target/riscv: rvv-1.0: stride load and store instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
>  /*
> + * Vector unit-stride, strided, unit-stride segment, strided segment
> + * store check function.
> + *
> + * Rules to be checked here:
> + *   1. EMUL must within the range: 1/8 <= EMUL <= 8. (Section 7.3)
> + *   2. Destination vector register number is multiples of EMUL.
> + *  (Section 3.3.2, 7.3)
> + *   3. The EMUL setting must be such that EMUL * NFIELDS ≤ 8. (Section 7.8)
> + *   4. Vector register numbers accessed by the segment load or store
> + *  cannot increment past 31. (Section 7.8)
> + */
> +static bool vext_check_store(DisasContext *s, int vd, int nf, uint8_t eew)
> +{
> +int8_t emul = ctzl(eew) - (s->sew + 3) + s->lmul;

I think eew should be passed as log, as MemOp and MO_{8,16,32,64} constants.
This is already the scale used by sew.

> +#define GEN_VEXT_TRANS(NAME, EEW, SEQ, ARGTYPE, OP, CHECK)   \
> +static bool trans_##NAME(DisasContext *s, arg_##ARGTYPE * a) \
> +{\
> +if (CHECK(s, a, EEW)) {  \
> +return OP(s, a, SEQ);\
> +}\
> +return false;\

That also makes EEW and SEQ identical, so that...

> +GEN_VEXT_TRANS(vlse8_v,  8,  0, rnfvm, ld_stride_op, ld_stride_check)
> +GEN_VEXT_TRANS(vlse16_v, 16, 1, rnfvm, ld_stride_op, ld_stride_check)
> +GEN_VEXT_TRANS(vlse32_v, 32, 2, rnfvm, ld_stride_op, ld_stride_check)
> +GEN_VEXT_TRANS(vlse64_v, 64, 3, rnfvm, ld_stride_op, ld_stride_check)

... this does not need to replicate those constants.

> -#define GEN_VEXT_LD_ELEM(NAME, MTYPE, ETYPE, H, LDSUF) \
> +#define GEN_VEXT_LD_ELEM(NAME, ETYPE, H, LDSUF)\
>  static void NAME(CPURISCVState *env, abi_ptr addr, \
>   uint32_t idx, void *vd, uintptr_t retaddr)\
>  {  \
> -MTYPE data;\
> +ETYPE data;\
>  ETYPE *cur = ((ETYPE *)vd + H(idx));   \
>  data = cpu_##LDSUF##_data_ra(env, addr, retaddr);  \
>  *cur = data;   \
>  }  \

Since there's no extension to be done between MTYPE and ETYPE anymore, you can
also drop the "data" variable completely and store directly into *cur.


r~

Re: [RFC v4 19/70] target/riscv: rvv-1.0: index load and store instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +static bool vext_check_st_index(DisasContext *s, int vd, int vs2, int nf,
> +uint8_t eew)
>  {
> -return !s->vill;
> +uint8_t lmul_r = s->lmul < 0 ? 0 : s->lmul;
> +int8_t emul = ctzl(eew) - (s->sew + 3) + s->lmul;
> +return (emul >= -3 && emul <= 3) &&
> +require_align(vs2, 1 << emul) &&
> +require_align(vd, 1 << s->lmul) &&
> +((nf << lmul_r) <= (NVPR / 4) &&
> + (vd + (nf << lmul_r)) <= NVPR);
> +}

Same comments wrt EEW as the last patch.

It wouldn't hurt to extract something like

static bool require_nf(int vd, int nf, int lmul)
{
int size = nf << MAX(lmul, 0);
return size <= 8 && vd + size <= 32;
}

in the previous patch as well.


r~

Re: [RFC v4 20/70] target/riscv: rvv-1.0: fix address index overflow bug of indexed load/store insns

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Replace ETYPE from signed int to unsigned int to prevent index overflow
> issue, which would lead to wrong index address.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/vector_helper.c | 8 
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [RFC v4 21/70] target/riscv: rvv-1.0: fault-only-first unit stride load

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/helper.h   | 27 +++-
>  target/riscv/insn32.decode  | 14 +++
>  target/riscv/insn_trans/trans_rvv.inc.c | 31 --
>  target/riscv/vector_helper.c| 56 +
>  4 files changed, 38 insertions(+), 90 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [RFC v4 22/70] target/riscv: rvv-1.0: amo operations

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +static bool vext_check_amo(DisasContext *s, int vd, int vs2,
> +   int wd, int vm, uint8_t eew)
> +{
> +int8_t emul = ctzl(eew) - (s->sew + 3) + s->lmul;
> +bool ret = has_ext(s, RVA) &&
> +   (1 << s->sew >= 4) &&
> +   (1 << s->sew <= sizeof(target_ulong)) &&
> +   (eew <= (sizeof(target_ulong) << 3))  &&
> +   require_align(vd, 1 << s->lmul) &&
> +   require_align(vs2, 1 << emul) &&
> +   (emul >= -3 && emul <= 3);
> +if (wd) {
> +ret &= require_vm(vm, vd);
> +if (eew > (1 << (s->sew + 3))) {
> +if (vd != vs2) {
> +ret &= require_noover(vd, 1 << s->lmul, vs2, 1 << emul);
> +}
> +} else if (eew < (1 << (s->sew + 3))) {
> +if (emul < 0) {
> +ret &= require_noover(vd, 1 << s->lmul, vs2, 1 << emul);
> +} else {
> +ret &= require_noover_widen(vd, 1 << s->lmul, vs2, 1 << 
> emul);
> +}
> +}
> +}
> +return ret;
> +}

Same comments for EEW and require_noover.

r~

Re: [RFC v4 23/70] target/riscv: rvv-1.0: load/store whole register instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +/*
> + * load and store whole register instructions ignore vtype and vl setting.
> + * Thus, we don't need to check vill bit. (Section 7.9)
> + */
> +#define GEN_LDST_WHOLE_TRANS(NAME, EEW, ARGTYPE, ARG_NF, IS_STORE) \
> +static bool trans_##NAME(DisasContext *s, arg_##ARGTYPE * a)   \
> +{  \
> +if (require_rvv(s) &&  \
> +QEMU_IS_ALIGNED(a->rd, ARG_NF)) {  \
> +uint32_t data = 0; \
> +bool ret;  \
> +data = FIELD_DP32(data, VDATA, NF, ARG_NF);\
> +ret = ldst_whole_trans(a->rd, a->rs1, data, gen_helper_##NAME, \
> +   s, IS_STORE);   \
> +return ret;\
> +}  \
> +return false;  \
> +}

Decodetree is intentionally organized such that ARGTYPE = NAME.  There's no
point in duplicating that.  Move everything besides the call to
ldst_whole_trans into ldst_whole_trans.


r~

Re: [RFC v4 24/70] target/riscv: rvv-1.0: update vext_max_elems() for load/store insns

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +static uint8_t vext_get_emul(DisasContext *s, uint8_t eew)
> +{
> +int8_t emul = ctzl(eew) - (s->sew + 3) + s->lmul;
> +return emul < 0 ? 0 : emul;
> +}

Same comment about EEW being encoded as MemOp to match SEW.


r~

Re: [RFC v4 25/70] target/riscv: rvv-1.0: take fractional LMUL into vector max elements calculation

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Update vext_get_vlmax() and MAXSZ() to take fractional LMUL into
> calculation for RVV 1.0.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/cpu.h  | 43 ++---
>  target/riscv/insn_trans/trans_rvv.inc.c | 12 ++-
>  2 files changed, 42 insertions(+), 13 deletions(-)

Reviewed-by: Richard Henderson 

r~

Re: [RFC v4 34/70] target/riscv: rvv-1.0: register gather instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +static bool vrgatherei16_vv_check(DisasContext *s, arg_rmrr *a)
> +{
> +int8_t emul = 4 - (s->sew + 3) + s->lmul;

Similarly for EEW elsewhere.  This should be

int emul = MO_16 - s->sew + s->lmul;

> @@ -3339,7 +3353,8 @@ static bool trans_vrgather_vx(DisasContext *s, arg_rmrr 
> *a)
>  }
>  
>  if (a->vm && s->vl_eq_vlmax) {
> -int vlmax = s->vlen;
> +int scale = s->lmul - (s->sew + 3);
> +int vlmax = scale < 0 ? s->vlen >> -scale : s->vlen << scale;

Isn't this what s->vlen is already computed as?


> +#define GEN_VEXT_VRGATHER_VV(NAME, TS1, TS2, HS1, HS2)\
>  void HELPER(NAME)(void *vd, void *v0, void *vs1, void *vs2,   \
>CPURISCVState *env, uint32_t desc)  \
>  { \
> -uint32_t vlmax = vext_max_elems(desc, ctzl(sizeof(ETYPE)));   \
> +uint32_t vlmax = vext_max_elems(desc, ctzl(sizeof(TS1))); \

Surely you can't just use TS1?  For vrgatherei16_vv_[wd], the maximum number of
elements is still bound by TS2.


r~

Re: [RFC v4 36/70] target/riscv: rvv-1.0: floating-point move instruction

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
>  static bool trans_vfmv_v_f(DisasContext *s, arg_vfmv_v_f *a)
>  {
>  if (require_rvv(s) &&
> +has_ext(s, RVF) &&
>  vext_check_isa_ill(s) &&
>  require_align(a->rd, 1 << s->lmul) &&
>  (s->sew != 0)) {
> +TCGv_i64 t1 = tcg_temp_local_new_i64();
> +/* NaN-box f[rs1] */
> +do_nanbox(s, t1, cpu_fpr[a->rs1]);

Don't you need to check

  s->sew == MO_64 ? has_ext(s, RVD) : has_ext(s, RVF)

?

It might be worth folding that into it's own helper function, which also
incorporates the s->sew != 0 check.  E.g.

static bool require_rvf(Disascontext *s)
{
switch (s->sew) {
case MO_16:
case MO_32:
return has_ext(s, RVF);
case MO_64:
return has_ext(s, RVD);
default:
return false;
}
}

> +TCGv_i64 t1 = tcg_temp_local_new_i64();
> +/* NaN-box f[rs1] */
> +do_nanbox(s, t1, cpu_fpr[a->rs1]);
> +
>  if (s->vl_eq_vlmax) {
>  tcg_gen_gvec_dup_i64(s->sew, vreg_ofs(s, a->rd),
> - MAXSZ(s), MAXSZ(s), cpu_fpr[a->rs1]);
> + MAXSZ(s), MAXSZ(s), t1);
>  mark_vs_dirty(s);
>  } else {
>  TCGv_ptr dest;

Recall that local temps get written to the stack at branches.

You should avoid the local temp by computing do_nanbox on both arms of this IF.
 In the else branch, do_nanbox should be after the brcond.


r~

Re: [RFC v4 36/70] target/riscv: rvv-1.0: floating-point move instruction

[Richard Henderson]

On 8/29/20 1:00 PM, Richard Henderson wrote:
> static bool require_rvf(Disascontext *s)
> {
> switch (s->sew) {
> case MO_16:
> case MO_32:
> return has_ext(s, RVF);
> case MO_64:
> return has_ext(s, RVD);
> default:
> return false;
> }
> }

Oh, and check mstatus_fs here too.  Which is missing in trans_vfmv_v_f, btw.


r~

Re: [RFC v4 37/70] target/riscv: rvv-1.0: floating-point scalar move instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> NaN-boxed the scalar floating-point register based on RVV 1.0's rules.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn32.decode  |  4 +--
>  target/riscv/insn_trans/trans_rvv.inc.c | 42 ++---
>  2 files changed, 25 insertions(+), 21 deletions(-)

Needs require_rvf(), as discussed vs the previous patch,

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 38/70] target/riscv: rvv-1.0: whole register move instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Add the following instructions:
> 
> * vmv1r.v
> * vmv2r.v
> * vmv4r.v
> * vmv8r.v
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn32.decode  |  4 
>  target/riscv/insn_trans/trans_rvv.inc.c | 25 +
>  2 files changed, 29 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 40/70] target/riscv: rvv-1.0: single-width averaging add and subtract instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Add the following instructions:
> 
> * vaaddu.vv
> * vaaddu.vx
> * vasubu.vv
> * vasubu.vx
> 
> Remove the following instructions:
> 
> * vadd.vi
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/helper.h   | 16 ++
>  target/riscv/insn32.decode  | 13 +++--
>  target/riscv/insn_trans/trans_rvv.inc.c |  5 +-
>  target/riscv/vector_helper.c| 74 +
>  4 files changed, 102 insertions(+), 6 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 42/70] target/riscv: rvv-1.0: integer add-with-carry/subtract-with-borrow

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Clear tail elements only if VTA is agnostic.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn32.decode  | 20 ++--
>  target/riscv/insn_trans/trans_rvv.inc.c |  2 +-
>  target/riscv/vector_helper.c| 14 --
>  3 files changed, 15 insertions(+), 21 deletions(-)

The patch description is wrong, but the code is correct.

One nit:

> -uint8_t carry = vext_elem_mask(v0, i);\
> +uint8_t carry = !vm ? vext_elem_mask(v0, i) : 0;  \

ETYPE carry = !vm && vext_elem_mask(v0, i);


r~

Re: [RFC v4 45/70] target/riscv: rvv-1.0: add Zvqmac extension

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/cpu.c   | 1 +
>  target/riscv/cpu.h   | 1 +
>  target/riscv/translate.c | 2 ++
>  3 files changed, 4 insertions(+)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 45/70] target/riscv: rvv-1.0: add Zvqmac extension

[Richard Henderson]

On 8/29/20 1:17 PM, Richard Henderson wrote:
> On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
>> From: Frank Chang 
>>
>> Signed-off-by: Frank Chang 
>> ---
>>  target/riscv/cpu.c   | 1 +
>>  target/riscv/cpu.h   | 1 +
>>  target/riscv/translate.c | 2 ++
>>  3 files changed, 4 insertions(+)
> 
> Reviewed-by: Richard Henderson 

On second thought, the current manual says

# This is only a proposal for a future extension after v1.0 and might change
substantially before ratifcation.

and does not even include the opcodes for these.

I think we should focus on 1.0 and drop this extension for now.


r~

Re: [RFC v4 47/70] target/riscv: rvv-1.0: single-width saturating add and subtract instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Sign-extend vsaddu.vi immediate value.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 46/70] target/riscv: rvv-1.0: quad-widening integer multiply-add instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Add the following instructions:
> 
> * vqmaccu.vv
> * vqmaccu.vx
> * vqmacc.vv
> * vqmacc.vx
> * vqmaccsu.vv
> * vqmaccsu.vx
> * vqmaccus.vx
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/helper.h   |  15 
>  target/riscv/insn32.decode  |   7 ++
>  target/riscv/insn_trans/trans_rvv.inc.c | 109 
>  target/riscv/vector_helper.c|  40 +
>  4 files changed, 171 insertions(+)

As mentioned, let's table this one for now.


r~

Re: [RFC v4 48/70] target/riscv: rvv-1.0: integer comparison instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> * Sign-extend vmselu.vi and vmsgtu.vi immediate values.
> * Remove "set tail elements to zeros" as tail elements can be unchanged
>   for either VTA to have undisturbed or agnostic setting.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 4 ++--
>  target/riscv/vector_helper.c| 8 
>  2 files changed, 2 insertions(+), 10 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 50/70] target/riscv: rvv-1.0: floating-point compare instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/vector_helper.c | 8 
>  1 file changed, 8 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 51/70] target/riscv: rvv-1.0: mask-register logical instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 3 ++-
>  target/riscv/vector_helper.c| 4 
>  2 files changed, 2 insertions(+), 5 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 52/70] target/riscv: rvv-1.0: slide instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> * Remove clear function from helper functions as the tail elements
>   are unchanged in RVV 1.0.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/vector_helper.c | 19 ---
>  1 file changed, 12 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 53/70] target/riscv: rvv-1.0: floating-point slide instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +DEF_HELPER_6(vfslide1up_vf_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vfslide1up_vf_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vfslide1up_vf_d, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vfslide1down_vf_h, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vfslide1down_vf_w, void, ptr, ptr, i64, ptr, env, i32)
> +DEF_HELPER_6(vfslide1down_vf_d, void, ptr, ptr, i64, ptr, env, i32)

You shouldn't need any new helpers.

The fp scalar argument can be passed to the existing helpers.

The manual doesn't explicitly say, but I assume the fp scalar needs to be
nan-boxed (for sew > flen), or checked (for sew < flen), just like the scalar
move instructions.


r~

Re: [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

[Michael]

Hello,

On Sat, 29 Aug 2020 18:45:06 +0200
Philippe Mathieu-Daudé  wrote:

> > > > However the NetBSD revision 1.32 of the driver introduced
> > > > 64-bit accesses to the stippler and blitter [2]. It is safe
> > > > to assume these memory regions are 64-bit accessible.
> > > > QEMU implementation is 32-bit, so fill the 'impl' fields.  
> >
> > IIRC the real hardware *requires* 64bit accesses for stipple and
> > blitter operations to work. For stipples you write a 64bit word into
> > STIP space, the address defines where in the framebuffer you want to
> > draw, the data contain a 32bit bitmask, foreground colour and a ROP.
> > BLIT space works similarly, the 64bit word contains an offset were to
> > read pixels from, and how many you want to copy.
> >  
> 
> Thanks Michael for this information!
> If you don't mind I'll amend it to the commit description so there is a
> reference for posterity.

One more thing since there seems to be some confusion - 64bit accesses
on the framebuffer are fine as well. TCX/S24 is *not* an SBus device,
even though its node says it is.
S24 is a card that plugs into a special slot on the SS5 mainboard,
which is shared with an SBus slot and looks a lot like a horizontal UPA
slot. Both S24 and TCX are accessed through the Micro/TurboSPARC's AFX
bus which is 64bit wide and intended for graphics.
Early FFB docs even mentioned connecting to both AFX and UPA, no idea
if that was ever realized in hardware though.

have fun
Michael

[REPORT] Nightly Performance Tests - Saturday, August 29, 2020

[Ahmed Karaman]

Host CPU : Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
Host Memory  : 15.49 GB

Start Time (UTC) : 2020-08-29 22:35:01
End Time (UTC)   : 2020-08-29 23:07:59
Execution Time   : 0:32:57.786998

Status   : SUCCESS

Note:
Changes denoted by '-' are less than 0.01%.


SUMMARY REPORT - COMMIT 39335fab

AVERAGE RESULTS

Target  Instructions  Latest  v5.1.0
--    --  --
aarch642 158 345 257   - +1.693%
alpha  1 914 965 210   - +3.524%
arm8 076 411 724   - +2.304%
hppa   4 261 660 282   - +3.162%
m68k   2 690 286 019   - +7.133%
mips   1 862 042 108   - +2.494%
mipsel 2 008 215 481   - +2.674%
mips64 1 918 638 076   - +2.818%
mips64el   2 051 558 104   - +3.026%
ppc2 480 149 009   - +3.107%
ppc64  2 576 685 090   -  +3.14%
ppc64le2 558 841 352   - +3.172%
riscv641 406 710 751   -  +2.65%
s390x  3 158 131 112   - +3.118%
sh42 364 461 215   - +3.331%
sparc643 318 816 026   - +3.861%
x86_64 1 775 796 489   - +2.155%


   DETAILED RESULTS

Test Program: dijkstra_double

Target  Instructions  Latest  v5.1.0
--    --  --
aarch643 062 564 802   - +1.423%
alpha  3 191 863 094   - +3.696%
arm   16 357 176 719   - +2.347%
hppa   7 228 376 670   - +3.086%
m68k   4 294 055 120   - +9.693%
mips   3 051 470 521   - +2.428%
mipsel 3 231 521 199   - +2.869%
mips64 3 245 828 100   - +2.596%
mips64el   3 414 220 121   - +3.021%
ppc4 914 534 259   -  +4.74%
ppc64  5 098 122 090   - +4.565%
ppc64le5 082 391 080   - +4.579%
riscv642 192 297 538   - +1.956%
s390x  4 584 592 888   - +2.898%
sh43 949 055 211   - +3.464%
sparc644 586 220 801   - +4.238%
x86_64 2 484 100 236   - +1.751%


Test Program: dijkstra_int32

Target  Instructions  Latest  v5.1.0
--    --  --
aarch642 210 194 213   - +1.493%
alpha  1 494 131 792   -  +2.15%
arm8 262 944 317   - +2.665%
hppa   5 207 293 281   - +3.046%
m68k   1 725 873 574   - +2.528%
mips   1 495 245 241   - +1.493%
mipsel 1 497 140 812   - +1.479%
mips64 1 715 422 394   - +1.894%
mips64el   1 695 211 434   - +1.909%
ppc2 014 572 096   -  +1.82%
ppc64  2 206 239 628   - +2.137%
ppc64le2 197 989 010   - +2.146%
riscv641 354 919 707   - +2.396%
s390x  2 916 086 767   - +1.236%
sh41 990 548 527   -  +2.67%
sparc642 874 257 044  +0.07% +3.831%
x86_64 1 553 996 279   - +2.121%


Test Program: matmult_double

Target  Instructions  Latest  v5.1.0
--    --  --
aarch641 412 251 151   -   +0.3%
alpha  3 233 987 397   - +7.473%
arm8 545 187 751   - +1.088%
hppa   3 483 519 295   - +4.466%
m68k   3 919 104 247   -+18.432%
mips   2 344 782 089   - +4.091%
mipsel 3 329 894 022   - +5.177%
mips64 2 359 038 211   - +4.076%
mips

Re: [RFC v4 55/70] target/riscv: rvv-1.0: single-width floating-point reduction

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/vector_helper.c | 12 ++--
>  1 file changed, 6 insertions(+), 6 deletions(-)

The one sentence in the manual is quite poor for this, and needs improvement:

> Floating-point max and min reductions should return the same final> value and 
> raise the same exception flags regardless of operation
> order.

It doesn't actually say anything about the way in which we've implemented the
*_noprop functions.

Anyway,
Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 56/70] target/riscv: rvv-1.0: widening floating-point reduction instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 57/70] target/riscv: rvv-1.0: single-width scaling shift instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> log(SEW) truncate vssra.vi immediate value.
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/insn_trans/trans_rvv.inc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 55/70] target/riscv: rvv-1.0: single-width floating-point reduction

[Richard Henderson]

On 8/29/20 4:50 PM, Richard Henderson wrote:
> It doesn't actually say anything about the way in which we've implemented the
> *_noprop functions.

Ho hum.  It seems v2.2 has changed fmin/fmax for the fpu as well.

We really should fix that too...


r~

Re: [RFC v4 61/70] target/riscv: rvv-1.0: floating-point min/max instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/vector_helper.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 62/70] target/riscv: introduce floating-point rounding mode enum

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/fpu_helper.c   | 12 ++--
>  target/riscv/insn_trans/trans_rvv.inc.c | 18 +-
>  target/riscv/internals.h|  9 +
>  3 files changed, 24 insertions(+), 15 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 63/70] target/riscv: rvv-1.0: floating-point/integer type-convert instructions

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +DEF_HELPER_5(vfcvt_rtz_xu_f_v_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfcvt_rtz_xu_f_v_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfcvt_rtz_xu_f_v_d, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfcvt_rtz_x_f_v_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfcvt_rtz_x_f_v_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfcvt_rtz_x_f_v_d, void, ptr, ptr, ptr, env, i32)

You do not need any of these new helpers, because...

> +GEN_OPFV_TRANS(vfcvt_rtz_xu_f_v, opfv_check, FRM_RTZ)
> +GEN_OPFV_TRANS(vfcvt_rtz_x_f_v, opfv_check, FRM_RTZ)

You get to set round-to-zero mode here.  Of course, the macro used will need to
change...


r~

Re: [RFC v4 64/70] target/riscv: rvv-1.0: widening floating-point/integer type-convert

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> @@ -995,12 +995,18 @@ DEF_HELPER_5(vfwcvt_xu_f_v_h, void, ptr, ptr, ptr, env, 
> i32)
>  DEF_HELPER_5(vfwcvt_xu_f_v_w, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_x_f_v_h, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_x_f_v_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfwcvt_f_xu_v_b, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_f_xu_v_h, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_f_xu_v_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfwcvt_f_x_v_b, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_f_x_v_h, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_f_x_v_w, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_f_f_v_h, void, ptr, ptr, ptr, env, i32)
>  DEF_HELPER_5(vfwcvt_f_f_v_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfwcvt_rtz_xu_f_v_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfwcvt_rtz_xu_f_v_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfwcvt_rtz_x_f_v_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfwcvt_rtz_x_f_v_w, void, ptr, ptr, ptr, env, i32)

Similarly, these last 4 helpers are not required.


r~

Re: [RFC v4 65/70] target/riscv: add "set round to odd" rounding mode helper function

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> helper_set_rounding_mode() is responsible for SIGILL, and "round to odd"
> should be an interface private to translation, so add a new independent
> helper_set_rod_rounding_mode().
> 
> Signed-off-by: Frank Chang 
> ---
>  target/riscv/fpu_helper.c | 5 +
>  target/riscv/helper.h | 1 +
>  target/riscv/internals.h  | 1 +
>  target/riscv/translate.c  | 5 +
>  4 files changed, 12 insertions(+)
...
> @@ -44,6 +44,7 @@ enum {
>  FRM_RUP = 3,/* Round Up */
>  FRM_RMM = 4,/* Round to Nearest, ties to Max Magnitude */
>  FRM_DYN = 7,/* Dynamic rounding mode */
> +FRM_ROD = 8,/* Round to Odd */
>  };

This needs a comment, or a rename, to indicate that it is private to qemu.

Otherwise,
Reviewed-by: Richard Henderson 


r~

Re: [RFC v4 66/70] target/riscv: rvv-1.0: narrowing floating-point/integer type-convert

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +DEF_HELPER_5(vfncvt_rod_f_f_w_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rod_f_f_w_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rtz_xu_f_w_b, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rtz_xu_f_w_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rtz_xu_f_w_w, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rtz_x_f_w_b, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rtz_x_f_w_h, void, ptr, ptr, ptr, env, i32)
> +DEF_HELPER_5(vfncvt_rtz_x_f_w_w, void, ptr, ptr, ptr, env, i32)

These helpers are redundant with the ones that don't specify rounding mode,
because you already handled that in the translator.


r~

Re: [RFC v4 67/70] target/riscv: rvv-1.0: relax RV_VLEN_MAX to 512-bits

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Frank Chang 
> 
> As GVEC only supports MAXSZ and OPRSZ in the range of: [8..256] bytes
> and LMUL could be a fractional number. The maximum vector size can be
> operated might be less than 8 bytes or larger than 256 bytes.
> Skip to use GVEC if maximum vector size <= 8 or >= 256 bytes.
> 
> Signed-off-by: Frank Chang 
> 
> --
> Maybe to relax the limitations of MAXSZ or OPRSZ would be a better
> approach.

I would definitely like to improve gvec to handle any actual vector length that
you need.  With VLEN=512 (bits) and LMUL=8, that gives you 512 byte vectors.
Is that the limit of what you need, or did you want to go higher?

There will have to be some maximum supported by tcg, though.
It's probably worth having an assert somewhere.

Perhaps something like

/*
 * RV_LEN_MAX (bits) / 8 (bits-per-byte) * 8 (LMUL)
 * = RV_LEN_MAX (bytes)
 *
 * should be less than the number of bytes supported by gvec.
 */
QEMU_BUILD_BUG_ON(RV_VLEN_MAX > (8 << SIMD_MAXSZ_BITS));

Perhaps placed in in vector_helper.c, so that cpu.h does not have to include
"tcg/tcg-gvec-desc.h".

However... simply increasing the number of bits in SIMD_MAXSZ_BITS and
SIMD_OPRSZ_BITS will break Arm SVE -- we need 20 bits in simd_data(), and
that's exactly what we have at present.

If we can come up with a more compact encoding of oprsz/maxsz, that would be
ideal.  Otherwise, I need to compress the data currently stored in simd_data().

-

I suppose one point here is that for RISC-V, oprsz always equals maxsz.  So
we've effectively wasted 5 bits.  Moreover, that's also true for Arm SVE.

However, Arm AdvSIMD, the older vector isa, will have oprsz == 8 or oprsz ==
16.  Since the vector registers overlap, maxsz is the SVE vector length, and
the area in between oprsz and maxsz is cleared.

If we ever merge the x86_64 AVX2 patches from last year's GSoC, and then expand
on that to implement AVX512, then we would have oprsz == 16 or oprsz == 32,
with maxsz == 64.

Perhaps we could reduce the generality of oprsz, and compress it into 2 bits:

  0b00 -> 8
  0b01 -> 16
  0b10 -> 32
  0b11 -> maxsz

Now we have 3 bits we can move over to the maxsz field, which will let us
represent 8 * 256 or 2048 byte vectors.

Thoughts?


r~

Re: [RFC v4 69/70] target/riscv: gdb: support vector registers for rv64

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> +++ b/gdb-xml/riscv-64bit-csr.xml
> @@ -248,4 +248,11 @@
>
>
>
> +  
> +  
> +  
> +  
> +  
> +  
> +  

Just because these are csr's doesn't mean they're unrelated to RVV.  I would
think that ideally they would be in the (generated) RVV-related xml file.

But I'm certainly not a gdb expert.  So if that doesn't work, fine, leave it as 
is.

However, if you leave these in the csr section, I think the next patch has to
be folded in, because you've already included

>  #if defined(TARGET_RISCV32)
>  gdb_register_coprocessor(cs, riscv_gdb_get_csr, riscv_gdb_set_csr,
> - 241, "riscv-32bit-csr.xml", 0);
> + 248, "riscv-32bit-csr.xml", 0);

... this.

That said, the actual dynamic xml looks fine.


r~

Re: [RFC v4 70/70] target/riscv: gdb: support vector registers for rv32

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Greentime Hu 
> 
> This patch adds vector support for rv32 gdb. It allows gdb client to access
> vector registers correctly.
> 
> Signed-off-by: Greentime Hu 
> Signed-off-by: Frank Chang 
> ---
>  gdb-xml/riscv-32bit-csr.xml | 7 +++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/gdb-xml/riscv-32bit-csr.xml b/gdb-xml/riscv-32bit-csr.xml
> index 3d2031da7dc..bb98b927995 100644
> --- a/gdb-xml/riscv-32bit-csr.xml
> +++ b/gdb-xml/riscv-32bit-csr.xml
> @@ -248,4 +248,11 @@
>
>
>
> +  
> +  
> +  
> +  
> +  
> +  
> +  
>  
> 

As per comments in the previous patch, needs squashing.


r~

Re: [RFC v4 68/70] target/riscv: gdb: modify gdb csr xml file to align with csr register map

[Richard Henderson]

On 8/17/20 1:49 AM, frank.ch...@sifive.com wrote:
> From: Hsiangkai Wang 
> 
> Signed-off-by: Hsiangkai Wang 
> Signed-off-by: Frank Chang 
> ---
>  gdb-xml/riscv-32bit-csr.xml | 11 ++-
>  gdb-xml/riscv-64bit-csr.xml | 11 ++-
>  target/riscv/gdbstub.c  |  4 ++--
>  3 files changed, 14 insertions(+), 12 deletions(-)

Looks ok, from a quick glance.

I do think that this could stand to be auto-generated as well, from an extended
csr_ops table.  Extended in that you'd want to include the name and the isa bit
to which the register applies.

I'll let Alistair weigh in on this.  But either way,
Acked-by: Richard Henderson 


r~

[PULL 0/7] softfloat patch queue

[Richard Henderson]

The following changes since commit ea1bb830cb021cca2e361091cf728aaabc8c0654:

  Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20200828' 
into staging (2020-08-28 15:14:40 +0100)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-sf-20200829

for you to fetch changes up to c53b1079334c41b342a8ad3b7ccfd51bf5427f5a:

  softfloat: Define comparison operations for bfloat16 (2020-08-29 19:25:42 
-0700)


* float16 comparison wrappers
* float16 conversions to/from 8-bit integers
* bfloat16 support


Frank Chang (1):
  softfloat: Add fp16 and uint8/int8 conversion functions

Kito Cheng (1):
  softfloat: Implement the full set of comparisons for float16

LIU Zhiwei (3):
  softfloat: Define operations for bfloat16
  softfloat: Define convert operations for bfloat16
  softfloat: Define misc operations for bfloat16

Richard Henderson (1):
  softfloat: Define comparison operations for bfloat16

Stephen Long (1):
  softfloat: Add float16_is_normal

 include/fpu/softfloat-types.h  |   5 +
 include/fpu/softfloat.h| 235 +++
 fpu/softfloat.c| 425 +
 target/riscv/vector_helper.c   |  25 ---
 fpu/softfloat-specialize.c.inc |  38 
 5 files changed, 703 insertions(+), 25 deletions(-)

[PULL 3/7] softfloat: Add float16_is_normal

[Richard Henderson]

From: Stephen Long 

This float16 predicate was missing from the normal set.

Signed-off-by: Stephen Long 
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 0a5a5e5d0b..863dfc6151 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -272,6 +272,11 @@ static inline bool float16_is_zero_or_denormal(float16 a)
 return (float16_val(a) & 0x7c00) == 0;
 }
 
+static inline bool float16_is_normal(float16 a)
+{
+return (((float16_val(a) >> 10) + 1) & 0x1f) >= 2;
+}
+
 static inline float16 float16_abs(float16 a)
 {
 /* Note that abs does *not* handle NaN specially, nor does
-- 
2.25.1

[PULL 1/7] softfloat: Implement the full set of comparisons for float16

[Richard Henderson]

From: Kito Cheng 

Implement them in softfloat and remove the local versions in riscv.

Signed-off-by: Kito Cheng 
Signed-off-by: Chih-Min Chao 
Acked-by: Alex Bennée 
Reviewed-by: Alistair Francis 
Message-Id: <1596102747-20226-2-git-send-email-chihmin.c...@sifive.com>
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat.h  | 41 
 target/riscv/vector_helper.c | 25 --
 2 files changed, 41 insertions(+), 25 deletions(-)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 659218b5c7..573fce99bc 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -285,6 +285,47 @@ static inline float16 float16_set_sign(float16 a, int sign)
 return make_float16((float16_val(a) & 0x7fff) | (sign << 15));
 }
 
+static inline bool float16_eq(float16 a, float16 b, float_status *s)
+{
+return float16_compare(a, b, s) == float_relation_equal;
+}
+
+static inline bool float16_le(float16 a, float16 b, float_status *s)
+{
+return float16_compare(a, b, s) <= float_relation_equal;
+}
+
+static inline bool float16_lt(float16 a, float16 b, float_status *s)
+{
+return float16_compare(a, b, s) < float_relation_equal;
+}
+
+static inline bool float16_unordered(float16 a, float16 b, float_status *s)
+{
+return float16_compare(a, b, s) == float_relation_unordered;
+}
+
+static inline bool float16_eq_quiet(float16 a, float16 b, float_status *s)
+{
+return float16_compare_quiet(a, b, s) == float_relation_equal;
+}
+
+static inline bool float16_le_quiet(float16 a, float16 b, float_status *s)
+{
+return float16_compare_quiet(a, b, s) <= float_relation_equal;
+}
+
+static inline bool float16_lt_quiet(float16 a, float16 b, float_status *s)
+{
+return float16_compare_quiet(a, b, s) < float_relation_equal;
+}
+
+static inline bool float16_unordered_quiet(float16 a, float16 b,
+   float_status *s)
+{
+return float16_compare_quiet(a, b, s) == float_relation_unordered;
+}
+
 #define float16_zero make_float16(0)
 #define float16_half make_float16(0x3800)
 #define float16_one make_float16(0x3c00)
diff --git a/target/riscv/vector_helper.c b/target/riscv/vector_helper.c
index 793af99067..a156573d28 100644
--- a/target/riscv/vector_helper.c
+++ b/target/riscv/vector_helper.c
@@ -3955,12 +3955,6 @@ void HELPER(NAME)(void *vd, void *v0, void *vs1, void 
*vs2,   \
 } \
 }
 
-static bool float16_eq_quiet(uint16_t a, uint16_t b, float_status *s)
-{
-FloatRelation compare = float16_compare_quiet(a, b, s);
-return compare == float_relation_equal;
-}
-
 GEN_VEXT_CMP_VV_ENV(vmfeq_vv_h, uint16_t, H2, float16_eq_quiet)
 GEN_VEXT_CMP_VV_ENV(vmfeq_vv_w, uint32_t, H4, float32_eq_quiet)
 GEN_VEXT_CMP_VV_ENV(vmfeq_vv_d, uint64_t, H8, float64_eq_quiet)
@@ -4017,12 +4011,6 @@ GEN_VEXT_CMP_VF(vmfne_vf_h, uint16_t, H2, vmfne16)
 GEN_VEXT_CMP_VF(vmfne_vf_w, uint32_t, H4, vmfne32)
 GEN_VEXT_CMP_VF(vmfne_vf_d, uint64_t, H8, vmfne64)
 
-static bool float16_lt(uint16_t a, uint16_t b, float_status *s)
-{
-FloatRelation compare = float16_compare(a, b, s);
-return compare == float_relation_less;
-}
-
 GEN_VEXT_CMP_VV_ENV(vmflt_vv_h, uint16_t, H2, float16_lt)
 GEN_VEXT_CMP_VV_ENV(vmflt_vv_w, uint32_t, H4, float32_lt)
 GEN_VEXT_CMP_VV_ENV(vmflt_vv_d, uint64_t, H8, float64_lt)
@@ -4030,13 +4018,6 @@ GEN_VEXT_CMP_VF(vmflt_vf_h, uint16_t, H2, float16_lt)
 GEN_VEXT_CMP_VF(vmflt_vf_w, uint32_t, H4, float32_lt)
 GEN_VEXT_CMP_VF(vmflt_vf_d, uint64_t, H8, float64_lt)
 
-static bool float16_le(uint16_t a, uint16_t b, float_status *s)
-{
-FloatRelation compare = float16_compare(a, b, s);
-return compare == float_relation_less ||
-   compare == float_relation_equal;
-}
-
 GEN_VEXT_CMP_VV_ENV(vmfle_vv_h, uint16_t, H2, float16_le)
 GEN_VEXT_CMP_VV_ENV(vmfle_vv_w, uint32_t, H4, float32_le)
 GEN_VEXT_CMP_VV_ENV(vmfle_vv_d, uint64_t, H8, float64_le)
@@ -4091,12 +4072,6 @@ GEN_VEXT_CMP_VF(vmfge_vf_h, uint16_t, H2, vmfge16)
 GEN_VEXT_CMP_VF(vmfge_vf_w, uint32_t, H4, vmfge32)
 GEN_VEXT_CMP_VF(vmfge_vf_d, uint64_t, H8, vmfge64)
 
-static bool float16_unordered_quiet(uint16_t a, uint16_t b, float_status *s)
-{
-FloatRelation compare = float16_compare_quiet(a, b, s);
-return compare == float_relation_unordered;
-}
-
 GEN_VEXT_CMP_VV_ENV(vmford_vv_h, uint16_t, H2, !float16_unordered_quiet)
 GEN_VEXT_CMP_VV_ENV(vmford_vv_w, uint32_t, H4, !float32_unordered_quiet)
 GEN_VEXT_CMP_VV_ENV(vmford_vv_d, uint64_t, H8, !float64_unordered_quiet)
-- 
2.25.1

[PULL 7/7] softfloat: Define comparison operations for bfloat16

[Richard Henderson]

Reviewed-by: LIU Zhiwei 
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat.h | 41 +
 1 file changed, 41 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 1233f98014..78ad5ca738 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -479,6 +479,47 @@ static inline bfloat16 bfloat16_set_sign(bfloat16 a, int 
sign)
 return (a & 0x7fff) | (sign << 15);
 }
 
+static inline bool bfloat16_eq(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) == float_relation_equal;
+}
+
+static inline bool bfloat16_le(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) <= float_relation_equal;
+}
+
+static inline bool bfloat16_lt(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) < float_relation_equal;
+}
+
+static inline bool bfloat16_unordered(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare(a, b, s) == float_relation_unordered;
+}
+
+static inline bool bfloat16_eq_quiet(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) == float_relation_equal;
+}
+
+static inline bool bfloat16_le_quiet(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) <= float_relation_equal;
+}
+
+static inline bool bfloat16_lt_quiet(bfloat16 a, bfloat16 b, float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) < float_relation_equal;
+}
+
+static inline bool bfloat16_unordered_quiet(bfloat16 a, bfloat16 b,
+   float_status *s)
+{
+return bfloat16_compare_quiet(a, b, s) == float_relation_unordered;
+}
+
 #define bfloat16_zero 0
 #define bfloat16_half 0x3f00
 #define bfloat16_one 0x3f80
-- 
2.25.1

[PULL 2/7] softfloat: Add fp16 and uint8/int8 conversion functions

[Richard Henderson]

From: Frank Chang 

Reviewed-by: Alex Bennée 
Signed-off-by: Frank Chang 
Message-Id: <1596102747-20226-4-git-send-email-chihmin.c...@sifive.com>
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat.h |  8 
 fpu/softfloat.c | 34 ++
 2 files changed, 42 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 573fce99bc..0a5a5e5d0b 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -136,9 +136,11 @@ float16 uint16_to_float16_scalbn(uint16_t a, int, 
float_status *status);
 float16 uint32_to_float16_scalbn(uint32_t a, int, float_status *status);
 float16 uint64_to_float16_scalbn(uint64_t a, int, float_status *status);
 
+float16 int8_to_float16(int8_t a, float_status *status);
 float16 int16_to_float16(int16_t a, float_status *status);
 float16 int32_to_float16(int32_t a, float_status *status);
 float16 int64_to_float16(int64_t a, float_status *status);
+float16 uint8_to_float16(uint8_t a, float_status *status);
 float16 uint16_to_float16(uint16_t a, float_status *status);
 float16 uint32_to_float16(uint32_t a, float_status *status);
 float16 uint64_to_float16(uint64_t a, float_status *status);
@@ -187,10 +189,13 @@ float32 float16_to_float32(float16, bool ieee, 
float_status *status);
 float16 float64_to_float16(float64 a, bool ieee, float_status *status);
 float64 float16_to_float64(float16 a, bool ieee, float_status *status);
 
+int8_t  float16_to_int8_scalbn(float16, FloatRoundMode, int,
+   float_status *status);
 int16_t float16_to_int16_scalbn(float16, FloatRoundMode, int, float_status *);
 int32_t float16_to_int32_scalbn(float16, FloatRoundMode, int, float_status *);
 int64_t float16_to_int64_scalbn(float16, FloatRoundMode, int, float_status *);
 
+int8_t  float16_to_int8(float16, float_status *status);
 int16_t float16_to_int16(float16, float_status *status);
 int32_t float16_to_int32(float16, float_status *status);
 int64_t float16_to_int64(float16, float_status *status);
@@ -199,6 +204,8 @@ int16_t float16_to_int16_round_to_zero(float16, 
float_status *status);
 int32_t float16_to_int32_round_to_zero(float16, float_status *status);
 int64_t float16_to_int64_round_to_zero(float16, float_status *status);
 
+uint8_t float16_to_uint8_scalbn(float16 a, FloatRoundMode,
+int, float_status *status);
 uint16_t float16_to_uint16_scalbn(float16 a, FloatRoundMode,
   int, float_status *status);
 uint32_t float16_to_uint32_scalbn(float16 a, FloatRoundMode,
@@ -206,6 +213,7 @@ uint32_t float16_to_uint32_scalbn(float16 a, FloatRoundMode,
 uint64_t float16_to_uint64_scalbn(float16 a, FloatRoundMode,
   int, float_status *status);
 
+uint8_t  float16_to_uint8(float16 a, float_status *status);
 uint16_t float16_to_uint16(float16 a, float_status *status);
 uint32_t float16_to_uint32(float16 a, float_status *status);
 uint64_t float16_to_uint64(float16 a, float_status *status);
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 33abc8207b..64de0b40f7 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -2109,6 +2109,13 @@ static int64_t round_to_int_and_pack(FloatParts in, 
FloatRoundMode rmode,
 }
 }
 
+int8_t float16_to_int8_scalbn(float16 a, FloatRoundMode rmode, int scale,
+  float_status *s)
+{
+return round_to_int_and_pack(float16_unpack_canonical(a, s),
+ rmode, scale, INT8_MIN, INT8_MAX, s);
+}
+
 int16_t float16_to_int16_scalbn(float16 a, FloatRoundMode rmode, int scale,
 float_status *s)
 {
@@ -2172,6 +2179,11 @@ int64_t float64_to_int64_scalbn(float64 a, 
FloatRoundMode rmode, int scale,
  rmode, scale, INT64_MIN, INT64_MAX, s);
 }
 
+int8_t float16_to_int8(float16 a, float_status *s)
+{
+return float16_to_int8_scalbn(a, s->float_rounding_mode, 0, s);
+}
+
 int16_t float16_to_int16(float16 a, float_status *s)
 {
 return float16_to_int16_scalbn(a, s->float_rounding_mode, 0, s);
@@ -2322,6 +2334,13 @@ static uint64_t round_to_uint_and_pack(FloatParts in, 
FloatRoundMode rmode,
 }
 }
 
+uint8_t float16_to_uint8_scalbn(float16 a, FloatRoundMode rmode, int scale,
+float_status *s)
+{
+return round_to_uint_and_pack(float16_unpack_canonical(a, s),
+  rmode, scale, UINT8_MAX, s);
+}
+
 uint16_t float16_to_uint16_scalbn(float16 a, FloatRoundMode rmode, int scale,
   float_status *s)
 {
@@ -2385,6 +2404,11 @@ uint64_t float64_to_uint64_scalbn(float64 a, 
FloatRoundMode rmode, int scale,
   rmode, scale, UINT64_MAX, s);
 }
 
+uint8_t float16_to_uint8(float16 a, float_status *s)
+{
+return float16_to_uint8_scalbn(a, s->float_rounding_mode, 0, s);
+}
+
 uint16_t float16_to_uint16(float16 a, float_status *s)
 {
 retu

[PULL 5/7] softfloat: Define convert operations for bfloat16

[Richard Henderson]

From: LIU Zhiwei 

Signed-off-by: LIU Zhiwei 
Reviewed-by: Richard Henderson 
Message-Id: <20200813071421.2509-3-zhiwei_...@c-sky.com>
[rth: Use FloatRoundMode for conversion functions.]
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat.h |  54 ++
 fpu/softfloat.c | 223 
 2 files changed, 277 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 68b3cf488d..95f0789a92 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -348,6 +348,60 @@ static inline bool float16_unordered_quiet(float16 a, 
float16 b,
 #define float16_three make_float16(0x4200)
 #define float16_infinity make_float16(0x7c00)
 
+/*
+| Software bfloat16 conversion routines.
+**/
+
+bfloat16 bfloat16_round_to_int(bfloat16, float_status *status);
+bfloat16 float32_to_bfloat16(float32, float_status *status);
+float32 bfloat16_to_float32(bfloat16, float_status *status);
+bfloat16 float64_to_bfloat16(float64 a, float_status *status);
+float64 bfloat16_to_float64(bfloat16 a, float_status *status);
+
+int16_t bfloat16_to_int16_scalbn(bfloat16, FloatRoundMode,
+ int, float_status *status);
+int32_t bfloat16_to_int32_scalbn(bfloat16, FloatRoundMode,
+ int, float_status *status);
+int64_t bfloat16_to_int64_scalbn(bfloat16, FloatRoundMode,
+ int, float_status *status);
+
+int16_t bfloat16_to_int16(bfloat16, float_status *status);
+int32_t bfloat16_to_int32(bfloat16, float_status *status);
+int64_t bfloat16_to_int64(bfloat16, float_status *status);
+
+int16_t bfloat16_to_int16_round_to_zero(bfloat16, float_status *status);
+int32_t bfloat16_to_int32_round_to_zero(bfloat16, float_status *status);
+int64_t bfloat16_to_int64_round_to_zero(bfloat16, float_status *status);
+
+uint16_t bfloat16_to_uint16_scalbn(bfloat16 a, FloatRoundMode,
+   int, float_status *status);
+uint32_t bfloat16_to_uint32_scalbn(bfloat16 a, FloatRoundMode,
+   int, float_status *status);
+uint64_t bfloat16_to_uint64_scalbn(bfloat16 a, FloatRoundMode,
+   int, float_status *status);
+
+uint16_t bfloat16_to_uint16(bfloat16 a, float_status *status);
+uint32_t bfloat16_to_uint32(bfloat16 a, float_status *status);
+uint64_t bfloat16_to_uint64(bfloat16 a, float_status *status);
+
+uint16_t bfloat16_to_uint16_round_to_zero(bfloat16 a, float_status *status);
+uint32_t bfloat16_to_uint32_round_to_zero(bfloat16 a, float_status *status);
+uint64_t bfloat16_to_uint64_round_to_zero(bfloat16 a, float_status *status);
+
+bfloat16 int16_to_bfloat16_scalbn(int16_t a, int, float_status *status);
+bfloat16 int32_to_bfloat16_scalbn(int32_t a, int, float_status *status);
+bfloat16 int64_to_bfloat16_scalbn(int64_t a, int, float_status *status);
+bfloat16 uint16_to_bfloat16_scalbn(uint16_t a, int, float_status *status);
+bfloat16 uint32_to_bfloat16_scalbn(uint32_t a, int, float_status *status);
+bfloat16 uint64_to_bfloat16_scalbn(uint64_t a, int, float_status *status);
+
+bfloat16 int16_to_bfloat16(int16_t a, float_status *status);
+bfloat16 int32_to_bfloat16(int32_t a, float_status *status);
+bfloat16 int64_to_bfloat16(int64_t a, float_status *status);
+bfloat16 uint16_to_bfloat16(uint16_t a, float_status *status);
+bfloat16 uint32_to_bfloat16(uint32_t a, float_status *status);
+bfloat16 uint64_to_bfloat16(uint64_t a, float_status *status);
+
 /*
 | Software bfloat16 operations.
 **/
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 77d81ab9b0..67cfa0fd82 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -2014,6 +2014,34 @@ float32 float64_to_float32(float64 a, float_status *s)
 return float32_round_pack_canonical(pr, s);
 }
 
+float32 bfloat16_to_float32(bfloat16 a, float_status *s)
+{
+FloatParts p = bfloat16_unpack_canonical(a, s);
+FloatParts pr = float_to_float(p, &float32_params, s);
+return float32_round_pack_canonical(pr, s);
+}
+
+float64 bfloat16_to_float64(bfloat16 a, float_status *s)
+{
+FloatParts p = bfloat16_unpack_canonical(a, s);
+FloatParts pr = float_to_float(p, &float64_params, s);
+return float64_round_pack_canonical(pr, s);
+}
+
+bfloat16 float32_to_bfloat16(float32 a, float_status *s)
+{
+FloatParts p = float32_unpack_canonical(a, s);
+FloatParts pr = float_to_float(p, &bfloat16_params, s);
+return bfloat16_round_pack_canonical(pr, s);
+}
+
+bfloat16 float64_to_bfloat16(float64 a, float_status *s)
+{
+FloatParts p = float64_unpack_canonical(a, s);
+FloatParts pr = float_to_float(p, &bfloat16_params, s);
+return bfloat16_round

[PULL 4/7] softfloat: Define operations for bfloat16

[Richard Henderson]

From: LIU Zhiwei 

This patch implements operations for bfloat16 except conversion and some misc
operations. We also add FloatFmt and pack/unpack interfaces for bfloat16.
As they are both static fields, we can't make a sperate patch for them.

Signed-off-by: LIU Zhiwei 
Reviewed-by: Richard Henderson 
Message-Id: <20200813071421.2509-2-zhiwei_...@c-sky.com>
[rth: Use FloatRelation for comparison operations.]
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat-types.h |   5 +
 include/fpu/softfloat.h   |  38 
 fpu/softfloat.c   | 168 ++
 3 files changed, 211 insertions(+)

diff --git a/include/fpu/softfloat-types.h b/include/fpu/softfloat-types.h
index c7ddcab8ca..8a3f20fae9 100644
--- a/include/fpu/softfloat-types.h
+++ b/include/fpu/softfloat-types.h
@@ -112,6 +112,11 @@ typedef struct {
 #define make_float128(high_, low_) ((float128) { .high = high_, .low = low_ })
 #define make_float128_init(high_, low_) { .high = high_, .low = low_ }
 
+/*
+ * Software neural-network floating-point types.
+ */
+typedef uint16_t bfloat16;
+
 /*
  * Software IEC/IEEE floating-point underflow tininess-detection mode.
  */
diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 863dfc6151..68b3cf488d 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -109,6 +109,7 @@ void float_raise(uint8_t flags, float_status *status);
 float16 float16_squash_input_denormal(float16 a, float_status *status);
 float32 float32_squash_input_denormal(float32 a, float_status *status);
 float64 float64_squash_input_denormal(float64 a, float_status *status);
+bfloat16 bfloat16_squash_input_denormal(bfloat16 a, float_status *status);
 
 /*
 | Options to indicate which negations to perform in float*_muladd()
@@ -347,6 +348,43 @@ static inline bool float16_unordered_quiet(float16 a, 
float16 b,
 #define float16_three make_float16(0x4200)
 #define float16_infinity make_float16(0x7c00)
 
+/*
+| Software bfloat16 operations.
+**/
+
+bfloat16 bfloat16_add(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_sub(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_mul(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_div(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_muladd(bfloat16, bfloat16, bfloat16, int,
+ float_status *status);
+float16 bfloat16_scalbn(bfloat16, int, float_status *status);
+bfloat16 bfloat16_min(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_max(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_minnum(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_maxnum(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_minnummag(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_maxnummag(bfloat16, bfloat16, float_status *status);
+bfloat16 bfloat16_sqrt(bfloat16, float_status *status);
+FloatRelation bfloat16_compare(bfloat16, bfloat16, float_status *status);
+FloatRelation bfloat16_compare_quiet(bfloat16, bfloat16, float_status *status);
+
+bfloat16 bfloat16_silence_nan(bfloat16, float_status *status);
+bfloat16 bfloat16_default_nan(float_status *status);
+
+static inline bfloat16 bfloat16_set_sign(bfloat16 a, int sign)
+{
+return (a & 0x7fff) | (sign << 15);
+}
+
+#define bfloat16_zero 0
+#define bfloat16_half 0x3f00
+#define bfloat16_one 0x3f80
+#define bfloat16_one_point_five 0x3fc0
+#define bfloat16_two 0x4000
+#define bfloat16_three 0x4040
+#define bfloat16_infinity 0x7f80
+
 /*
 | The pattern for a default generated half-precision NaN.
 **/
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 64de0b40f7..77d81ab9b0 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -554,6 +554,10 @@ static const FloatFmt float16_params_ahp = {
 .arm_althp = true
 };
 
+static const FloatFmt bfloat16_params = {
+FLOAT_PARAMS(8, 7)
+};
+
 static const FloatFmt float32_params = {
 FLOAT_PARAMS(8, 23)
 };
@@ -580,6 +584,11 @@ static inline FloatParts float16_unpack_raw(float16 f)
 return unpack_raw(float16_params, f);
 }
 
+static inline FloatParts bfloat16_unpack_raw(bfloat16 f)
+{
+return unpack_raw(bfloat16_params, f);
+}
+
 static inline FloatParts float32_unpack_raw(float32 f)
 {
 return unpack_raw(float32_params, f);
@@ -603,6 +612,11 @@ static inline float16 float16_pack_raw(FloatParts p)
 return make_float16(pack_raw(float16_params, p));
 }
 
+static inline bfloat16 bfloat16_pack_raw(FloatParts p)
+{
+return pack_raw(bfloat16_params, p);
+}
+
 static inline float32 float32_pack_raw(FloatParts p)
 {
 return make

[PULL 6/7] softfloat: Define misc operations for bfloat16

[Richard Henderson]

From: LIU Zhiwei 

Signed-off-by: LIU Zhiwei 
Reviewed-by: Richard Henderson 
Message-Id: <20200813071421.2509-4-zhiwei_...@c-sky.com>
[rth: Fix merge conflict with NO_SIGNALING_NANS; use bool for predicates.]
Signed-off-by: Richard Henderson 
---
 include/fpu/softfloat.h| 48 ++
 fpu/softfloat-specialize.c.inc | 38 +++
 2 files changed, 86 insertions(+)

diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h
index 95f0789a92..1233f98014 100644
--- a/include/fpu/softfloat.h
+++ b/include/fpu/softfloat.h
@@ -423,9 +423,57 @@ bfloat16 bfloat16_sqrt(bfloat16, float_status *status);
 FloatRelation bfloat16_compare(bfloat16, bfloat16, float_status *status);
 FloatRelation bfloat16_compare_quiet(bfloat16, bfloat16, float_status *status);
 
+bool bfloat16_is_quiet_nan(bfloat16, float_status *status);
+bool bfloat16_is_signaling_nan(bfloat16, float_status *status);
 bfloat16 bfloat16_silence_nan(bfloat16, float_status *status);
 bfloat16 bfloat16_default_nan(float_status *status);
 
+static inline bool bfloat16_is_any_nan(bfloat16 a)
+{
+return ((a & ~0x8000) > 0x7F80);
+}
+
+static inline bool bfloat16_is_neg(bfloat16 a)
+{
+return a >> 15;
+}
+
+static inline bool bfloat16_is_infinity(bfloat16 a)
+{
+return (a & 0x7fff) == 0x7F80;
+}
+
+static inline bool bfloat16_is_zero(bfloat16 a)
+{
+return (a & 0x7fff) == 0;
+}
+
+static inline bool bfloat16_is_zero_or_denormal(bfloat16 a)
+{
+return (a & 0x7F80) == 0;
+}
+
+static inline bool bfloat16_is_normal(bfloat16 a)
+{
+return (((a >> 7) + 1) & 0xff) >= 2;
+}
+
+static inline bfloat16 bfloat16_abs(bfloat16 a)
+{
+/* Note that abs does *not* handle NaN specially, nor does
+ * it flush denormal inputs to zero.
+ */
+return a & 0x7fff;
+}
+
+static inline bfloat16 bfloat16_chs(bfloat16 a)
+{
+/* Note that chs does *not* handle NaN specially, nor does
+ * it flush denormal inputs to zero.
+ */
+return a ^ 0x8000;
+}
+
 static inline bfloat16 bfloat16_set_sign(bfloat16 a, int sign)
 {
 return (a & 0x7fff) | (sign << 15);
diff --git a/fpu/softfloat-specialize.c.inc b/fpu/softfloat-specialize.c.inc
index dc4ea33c09..c2f87addb2 100644
--- a/fpu/softfloat-specialize.c.inc
+++ b/fpu/softfloat-specialize.c.inc
@@ -265,6 +265,25 @@ bool float16_is_quiet_nan(float16 a_, float_status *status)
 }
 }
 
+/*
+| Returns 1 if the bfloat16 value `a' is a quiet
+| NaN; otherwise returns 0.
+**/
+
+bool bfloat16_is_quiet_nan(bfloat16 a_, float_status *status)
+{
+if (no_signaling_nans(status)) {
+return bfloat16_is_any_nan(a_);
+} else {
+uint16_t a = a_;
+if (snan_bit_is_one(status)) {
+return (((a >> 6) & 0x1FF) == 0x1FE) && (a & 0x3F);
+} else {
+return ((a >> 6) & 0x1FF) == 0x1FF;
+}
+}
+}
+
 /*
 | Returns 1 if the half-precision floating-point value `a' is a signaling
 | NaN; otherwise returns 0.
@@ -284,6 +303,25 @@ bool float16_is_signaling_nan(float16 a_, float_status 
*status)
 }
 }
 
+/*
+| Returns 1 if the bfloat16 value `a' is a signaling
+| NaN; otherwise returns 0.
+**/
+
+bool bfloat16_is_signaling_nan(bfloat16 a_, float_status *status)
+{
+if (no_signaling_nans(status)) {
+return 0;
+} else {
+uint16_t a = a_;
+if (snan_bit_is_one(status)) {
+return ((a >> 6) & 0x1FF) == 0x1FF;
+} else {
+return (((a >> 6) & 0x1FF) == 0x1FE) && (a & 0x3F);
+}
+}
+}
+
 /*
 | Returns 1 if the single-precision floating-point value `a' is a quiet
 | NaN; otherwise returns 0.
-- 
2.25.1

Re: [Bug 1892540] [RFC PATCH v2] hw/display/tcx: Allow 64-bit accesses to framebuffer stippler and blitter

[mst]

On Sat, Aug 22, 2020 at 02:21:27PM -, Philippe Mathieu-Daudé wrote:
> The S24/TCX datasheet is listed as "Unable to locate" on [1].
> 
> However the NetBSD revision 1.32 of the driver introduced
> 64-bit accesses to the stippler and blitter [2]. It is safe
> to assume these memory regions are 64-bit accessible.
> QEMU implementation is 32-bit, so fill the 'impl' fields.
> 
> [1] 
> http://web.archive.org/web/20111209011516/http://wikis.sun.com/display/FOSSdocs/Home
> [2] 
> http://cvsweb.netbsd.org/bsdweb.cgi/src/sys/dev/sbus/tcx.c.diff?r1=1.31&r2=1.32
> 
> Reported-by: Andreas Gustafsson 
> Buglink: https://bugs.launchpad.net/bugs/1892540
> Fixes: 55d7bfe2293 ("tcx: Implement hardware acceleration")
> Signed-off-by: Philippe Mathieu-Daudé 

Philippe, did you submit the patch on the mailing list
normally too? I don't seem to see it there.

the patch seems to work for me:

Tested-by: Michael S. Tsirkin 


CC Nathan who reported a similar failure.

Nathan, does the patch below fix the issue for you?

> ---
> Since v1:
> - added missing uncommitted staged changes... (tcx_blit_ops)
> ---
 hw/display/tcx.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 1fb45b1aab8..96c6898b149 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -548,20 +548,28 @@ static const MemoryRegionOps tcx_stip_ops = {
 .read = tcx_stip_readl,
 .write = tcx_stip_writel,
 .endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
+.impl = {
 .min_access_size = 4,
 .max_access_size = 4,
 },
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
 };
 
 static const MemoryRegionOps tcx_rstip_ops = {
 .read = tcx_stip_readl,
 .write = tcx_rstip_writel,
 .endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
+.impl = {
 .min_access_size = 4,
 .max_access_size = 4,
 },
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
 };
 
 static uint64_t tcx_blit_readl(void *opaque, hwaddr addr,
@@ -650,10 +658,14 @@ static const MemoryRegionOps tcx_rblit_ops = {
 .read = tcx_blit_readl,
 .write = tcx_rblit_writel,
 .endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
+.impl = {
 .min_access_size = 4,
 .max_access_size = 4,
 },
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
 };
 
 static void tcx_invalidate_cursor_position(TCXState *s)


---

I think you shouldn't specify .min_access_size in impl, since
that also allows 1 and 2 byte accesses from guest.



> -- 
> 2.26.2
> 
> -- 
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/1892540
> 
> Title:
>   qemu can no longer boot NetBSD/sparc
> 
> Status in QEMU:
>   New
> 
> Bug description:
>   Booting NetBSD/sparc in qemu no longer works.  It broke between qemu
>   version 5.0.0 and 5.1.0, and a bisection identified the following as
>   the offending commit:
> 
> [5d971f9e672507210e77d020d89e0e89165c8fc9] memory: Revert "memory:
>   accept mismatching sizes in memory_region_access_valid"
> 
>   It's still broken as of 7fd51e68c34fcefdb4d6fd646ed3346f780f89f4.
> 
>   To reproduce, run
> 
> wget 
> http://ftp.netbsd.org/pub/NetBSD/NetBSD-9.0/images/NetBSD-9.0-sparc.iso
> qemu-system-sparc -nographic -cdrom NetBSD-9.0-sparc.iso -boot d
> 
>   The expected behavior is that the guest boots to the prompt
> 
> Installation medium to load the additional utilities from:
> 
>   The observed behavior is a panic:
> 
> [   1.050] system[0]: trap 0x29: pc=0xf0046b14 sfsr=0xb6 
> sfva=0x5400
> [   1.050] cpu0: data fault: pc=0xf0046b14 addr=0x5400 
> sfsr=0xb6
> [   1.050] panic: kernel fault
> [   1.050] halted
> 
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/qemu/+bug/1892540/+subscriptions

Re: [PATCH] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid"

[Michael S. Tsirkin]

On Wed, Aug 26, 2020 at 10:32:16PM -0700, Nathan Chancellor wrote:
> Hi all,
> 
> Sorry for the duplicate reply, my first one was rejected by a mailing
> list administrator for being too long so I resent it with the error logs
> as a link instead of inline.
> 
> On Wed, Jun 10, 2020 at 09:47:49AM -0400, Michael S. Tsirkin wrote:
> > Memory API documentation documents valid .min_access_size and 
> > .max_access_size
> > fields and explains that any access outside these boundaries is blocked.
> > 
> > This is what devices seem to assume.
> > 
> > However this is not what the implementation does: it simply
> > ignores the boundaries unless there's an "accepts" callback.
> > 
> > Naturally, this breaks a bunch of devices.
> > 
> > Revert to the documented behaviour.
> > 
> > Devices that want to allow any access can just drop the valid field,
> > or add the impl field to have accesses converted to appropriate
> > length.
> > 
> > Cc: qemu-sta...@nongnu.org
> > Reviewed-by: Richard Henderson 
> > Fixes: CVE-2020-13754
> > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1842363
> > Fixes: a014ed07bd5a ("memory: accept mismatching sizes in 
> > memory_region_access_valid")
> > Signed-off-by: Michael S. Tsirkin 
> > ---
> >  memory.c | 29 +
> >  1 file changed, 9 insertions(+), 20 deletions(-)
> > 
> > diff --git a/memory.c b/memory.c
> > index 91ceaf9fcf..3e9388fb74 100644
> > --- a/memory.c
> > +++ b/memory.c
> > @@ -1352,35 +1352,24 @@ bool memory_region_access_valid(MemoryRegion *mr,
> >  bool is_write,
> >  MemTxAttrs attrs)
> >  {
> > -int access_size_min, access_size_max;
> > -int access_size, i;
> > +if (mr->ops->valid.accepts
> > +&& !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, 
> > attrs)) {
> > +return false;
> > +}
> >  
> >  if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
> >  return false;
> >  }
> >  
> > -if (!mr->ops->valid.accepts) {
> > +/* Treat zero as compatibility all valid */
> > +if (!mr->ops->valid.max_access_size) {
> >  return true;
> >  }
> >  
> > -access_size_min = mr->ops->valid.min_access_size;
> > -if (!mr->ops->valid.min_access_size) {
> > -access_size_min = 1;
> > +if (size > mr->ops->valid.max_access_size
> > +|| size < mr->ops->valid.min_access_size) {
> > +return false;
> >  }
> > -
> > -access_size_max = mr->ops->valid.max_access_size;
> > -if (!mr->ops->valid.max_access_size) {
> > -access_size_max = 4;
> > -}
> > -
> > -access_size = MAX(MIN(size, access_size_max), access_size_min);
> > -for (i = 0; i < size; i += access_size) {
> > -if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size,
> > -is_write, attrs)) {
> > -return false;
> > -}
> > -}
> > -
> >  return true;
> >  }
> >  
> > -- 
> > MST
> > 
> > 
> 
> I just ran into a regression with booting RISC-V kernels due to this
> commit. I can reproduce it with QEMU 5.1.0 and latest tip of tree
> (25f6dc28a3a8dd231c2c092a0e65bd796353c769 at the time of initially
> writing this).
> 
> The error message, commands, and bisect logs are available here:
> 
> https://gist.githubusercontent.com/nathanchance/c106dd22ec0c0e00f6a25daba106a1b9/raw/d929f2fff6da9126ded156affb0f19f359e9f693/qemu-5.1.0-issue-terminal-log.txt
> 
> I have attached the rootfs and kernel image used for these tests. If for
> some reason there is a problem receiving them, the kernel is just an
> arch/riscv/configs/defconfig kernel at Linux 5.9-rc2 and the rootfs is
> available here:
> 
> https://github.com/ClangBuiltLinux/boot-utils/blob/3b21a5b71451742866349ba4f18638c5a754e660/images/riscv/rootfs.cpio.zst
> 
> Please let me know if I can provide any follow up information or if I am
> doing something wrong.
> 
> Cheers,
> Nathan


The following patch was proposed to fix the issue:

---
 hw/display/tcx.c | 18 +++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/hw/display/tcx.c b/hw/display/tcx.c
index 1fb45b1aab8..96c6898b149 100644
--- a/hw/display/tcx.c
+++ b/hw/display/tcx.c
@@ -548,20 +548,28 @@ static const MemoryRegionOps tcx_stip_ops = {
 .read = tcx_stip_readl,
 .write = tcx_stip_writel,
 .endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
+.impl = {
 .min_access_size = 4,
 .max_access_size = 4,
 },
+.valid = {
+.min_access_size = 4,
+.max_access_size = 8,
+},
 };
 
 static const MemoryRegionOps tcx_rstip_ops = {
 .read = tcx_stip_readl,
 .write = tcx_rstip_writel,
 .endianness = DEVICE_NATIVE_ENDIAN,
-.valid = {
+.impl = {
 .min_access_size = 4,
 .max_access_size = 4,
 },
+.valid = {
+.min_access_size = 4,
+.max_access_

Re: [PATCH] memory: Revert "memory: accept mismatching sizes in memory_region_access_valid"

[Nathan Chancellor]

On Sun, Aug 30, 2020 at 02:20:38AM -0400, Michael S. Tsirkin wrote:
> On Wed, Aug 26, 2020 at 10:32:16PM -0700, Nathan Chancellor wrote:
> > Hi all,
> > 
> > Sorry for the duplicate reply, my first one was rejected by a mailing
> > list administrator for being too long so I resent it with the error logs
> > as a link instead of inline.
> > 
> > On Wed, Jun 10, 2020 at 09:47:49AM -0400, Michael S. Tsirkin wrote:
> > > Memory API documentation documents valid .min_access_size and 
> > > .max_access_size
> > > fields and explains that any access outside these boundaries is blocked.
> > > 
> > > This is what devices seem to assume.
> > > 
> > > However this is not what the implementation does: it simply
> > > ignores the boundaries unless there's an "accepts" callback.
> > > 
> > > Naturally, this breaks a bunch of devices.
> > > 
> > > Revert to the documented behaviour.
> > > 
> > > Devices that want to allow any access can just drop the valid field,
> > > or add the impl field to have accesses converted to appropriate
> > > length.
> > > 
> > > Cc: qemu-sta...@nongnu.org
> > > Reviewed-by: Richard Henderson 
> > > Fixes: CVE-2020-13754
> > > Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1842363
> > > Fixes: a014ed07bd5a ("memory: accept mismatching sizes in 
> > > memory_region_access_valid")
> > > Signed-off-by: Michael S. Tsirkin 
> > > ---
> > >  memory.c | 29 +
> > >  1 file changed, 9 insertions(+), 20 deletions(-)
> > > 
> > > diff --git a/memory.c b/memory.c
> > > index 91ceaf9fcf..3e9388fb74 100644
> > > --- a/memory.c
> > > +++ b/memory.c
> > > @@ -1352,35 +1352,24 @@ bool memory_region_access_valid(MemoryRegion *mr,
> > >  bool is_write,
> > >  MemTxAttrs attrs)
> > >  {
> > > -int access_size_min, access_size_max;
> > > -int access_size, i;
> > > +if (mr->ops->valid.accepts
> > > +&& !mr->ops->valid.accepts(mr->opaque, addr, size, is_write, 
> > > attrs)) {
> > > +return false;
> > > +}
> > >  
> > >  if (!mr->ops->valid.unaligned && (addr & (size - 1))) {
> > >  return false;
> > >  }
> > >  
> > > -if (!mr->ops->valid.accepts) {
> > > +/* Treat zero as compatibility all valid */
> > > +if (!mr->ops->valid.max_access_size) {
> > >  return true;
> > >  }
> > >  
> > > -access_size_min = mr->ops->valid.min_access_size;
> > > -if (!mr->ops->valid.min_access_size) {
> > > -access_size_min = 1;
> > > +if (size > mr->ops->valid.max_access_size
> > > +|| size < mr->ops->valid.min_access_size) {
> > > +return false;
> > >  }
> > > -
> > > -access_size_max = mr->ops->valid.max_access_size;
> > > -if (!mr->ops->valid.max_access_size) {
> > > -access_size_max = 4;
> > > -}
> > > -
> > > -access_size = MAX(MIN(size, access_size_max), access_size_min);
> > > -for (i = 0; i < size; i += access_size) {
> > > -if (!mr->ops->valid.accepts(mr->opaque, addr + i, access_size,
> > > -is_write, attrs)) {
> > > -return false;
> > > -}
> > > -}
> > > -
> > >  return true;
> > >  }
> > >  
> > > -- 
> > > MST
> > > 
> > > 
> > 
> > I just ran into a regression with booting RISC-V kernels due to this
> > commit. I can reproduce it with QEMU 5.1.0 and latest tip of tree
> > (25f6dc28a3a8dd231c2c092a0e65bd796353c769 at the time of initially
> > writing this).
> > 
> > The error message, commands, and bisect logs are available here:
> > 
> > https://gist.githubusercontent.com/nathanchance/c106dd22ec0c0e00f6a25daba106a1b9/raw/d929f2fff6da9126ded156affb0f19f359e9f693/qemu-5.1.0-issue-terminal-log.txt
> > 
> > I have attached the rootfs and kernel image used for these tests. If for
> > some reason there is a problem receiving them, the kernel is just an
> > arch/riscv/configs/defconfig kernel at Linux 5.9-rc2 and the rootfs is
> > available here:
> > 
> > https://github.com/ClangBuiltLinux/boot-utils/blob/3b21a5b71451742866349ba4f18638c5a754e660/images/riscv/rootfs.cpio.zst
> > 
> > Please let me know if I can provide any follow up information or if I am
> > doing something wrong.
> > 
> > Cheers,
> > Nathan
> 
> 
> The following patch was proposed to fix the issue:
> 
> ---
>  hw/display/tcx.c | 18 +++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/display/tcx.c b/hw/display/tcx.c
> index 1fb45b1aab8..96c6898b149 100644
> --- a/hw/display/tcx.c
> +++ b/hw/display/tcx.c
> @@ -548,20 +548,28 @@ static const MemoryRegionOps tcx_stip_ops = {
>  .read = tcx_stip_readl,
>  .write = tcx_stip_writel,
>  .endianness = DEVICE_NATIVE_ENDIAN,
> -.valid = {
> +.impl = {
>  .min_access_size = 4,
>  .max_access_size = 4,
>  },
> +.valid = {
> +.min_access_size = 4,
> +.max_access_siz