Re: [PATCH v7 3/3] hw/nvme: Add SPDM over DOE support
On Fri, 2024-06-14 at 11:28 +1000, Alistair Francis wrote: > From: Wilfred Mallawa > > Setup Data Object Exchance (DOE) as an extended capability for the > NVME small typo here 邏️ [s/Setup Data Object Exchance/Setup Data Object Exchange] Wilfred > controller and connect SPDM to it (CMA) to it. > > Signed-off-by: Wilfred Mallawa > Signed-off-by: Alistair Francis > Reviewed-by: Jonathan Cameron > Acked-by: Klaus Jensen > --- > docs/specs/index.rst | 1 + > docs/specs/spdm.rst | 134 > > include/hw/pci/pci_device.h | 7 ++ > include/hw/pci/pcie_doe.h | 3 + > hw/nvme/ctrl.c | 60 > 5 files changed, 205 insertions(+) > create mode 100644 docs/specs/spdm.rst > > diff --git a/docs/specs/index.rst b/docs/specs/index.rst > index 1484e3e760..e2d907959a 100644 > --- a/docs/specs/index.rst > +++ b/docs/specs/index.rst > @@ -29,6 +29,7 @@ guest hardware that is specific to QEMU. > edu > ivshmem-spec > pvpanic > + spdm > standard-vga > virt-ctlr > vmcoreinfo > diff --git a/docs/specs/spdm.rst b/docs/specs/spdm.rst > new file mode 100644 > index 00..f7de080ff0 > --- /dev/null > +++ b/docs/specs/spdm.rst > @@ -0,0 +1,134 @@ > +== > +QEMU Security Protocols and Data Models (SPDM) Support > +== > + > +SPDM enables authentication, attestation and key exchange to assist > in > +providing infrastructure security enablement. It's a standard > published > +by the `DMTF`_. > + > +QEMU supports connecting to a SPDM responder implementation. This > allows an > +external application to emulate the SPDM responder logic for an SPDM > device. > + > +Setting up a SPDM server > + > + > +When using QEMU with SPDM devices QEMU will connect to a server > which > +implements the SPDM functionality. > + > +SPDM-Utils > +-- > + > +You can use `SPDM Utils`_ to emulate a responder. This is the > simplest method. > + > +SPDM-Utils is a Linux applications to manage, test and develop > devices > +supporting DMTF Security Protocol and Data Model (SPDM). It is > written in Rust > +and utilises libspdm. > + > +To use SPDM-Utils you will need to do the following steps. Details > are included > +in the SPDM-Utils README. > + > + 1. `Build libspdm`_ > + 2. `Build SPDM Utils`_ > + 3. `Run it as a server`_ > + > +spdm-emu > + > + > +You can use `spdm emu`_ to model the > +SPDM responder. > + > +.. code-block:: shell > + > + $ cd spdm-emu > + $ git submodule init; git submodule update --recursive > + $ mkdir build; cd build > + $ cmake -DARCH=x64 -DTOOLCHAIN=GCC -DTARGET=Debug - > DCRYPTO=openssl .. > + $ make -j32 > + $ make copy_sample_key # Build certificates, required for SPDM > authentication. > + > +It is worth noting that the certificates should be in compliance > with > +PCIe r6.1 sec 6.31.3. This means you will need to add the following > to > +openssl.cnf > + > +.. code-block:: > + > + subjectAltName = > otherName:2.23.147;UTF8:Vendor=1b36:Device=0010:CC=010802:REV=02:SSVI > D=1af4:SSID=1100 > + 2.23.147 = ASN1:OID:2.23.147 > + > +and then manually regenerate some certificates with: > + > +.. code-block:: shell > + > + $ openssl req -nodes -newkey ec:param.pem -keyout > end_responder.key \ > + -out end_responder.req -sha384 -batch \ > + -subj "/CN=DMTF libspdm ECP384 responder cert" > + > + $ openssl x509 -req -in end_responder.req -out > end_responder.cert \ > + -CA inter.cert -CAkey inter.key -sha384 -days 3650 - > set_serial 3 \ > + -extensions v3_end -extfile ../openssl.cnf > + > + $ openssl asn1parse -in end_responder.cert -out > end_responder.cert.der > + > + $ cat ca.cert.der inter.cert.der end_responder.cert.der > > bundle_responder.certchain.der > + > +You can use SPDM-Utils instead as it will generate the correct > certificates > +automatically. > + > +The responder can then be launched with > + > +.. code-block:: shell > + > + $ cd bin > + $ ./spdm_responder_emu --trans PCI_DOE > + > +Connecting an SPDM NVMe device > +== > + > +Once a SPDM server is running we can start QEMU and connect to the > server. > + > +For an NVMe device first let's setup a block we can use > + > +.. code-block:: shell > + > + $ cd qemu-spdm/linux/image > + $ dd if=/dev/zero of=blknvme bs=1M count=2096 # 2GB NNMe Driv
Re: [PATCH v7 1/3] hw/pci: Add all Data Object Types defined in PCIe r6.0
Reviewed-by: Wilfred Mallawa On Fri, 2024-06-14 at 11:28 +1000, Alistair Francis wrote: > Add all of the defined protocols/features from the PCIe-SIG r6.0 > "Table 6-32 PCI-SIG defined Data Object Types (Vendor ID = 0001h)" > table. > > Signed-off-by: Alistair Francis > Reviewed-by: Jonathan Cameron > --- > include/hw/pci/pcie_doe.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/hw/pci/pcie_doe.h b/include/hw/pci/pcie_doe.h > index 87dc17dcef..15d94661f9 100644 > --- a/include/hw/pci/pcie_doe.h > +++ b/include/hw/pci/pcie_doe.h > @@ -46,6 +46,8 @@ REG32(PCI_DOE_CAP_STATUS, 0) > > /* PCI-SIG defined Data Object Types - r6.0 Table 6-32 */ > #define PCI_SIG_DOE_DISCOVERY 0x00 > +#define PCI_SIG_DOE_CMA 0x01 > +#define PCI_SIG_DOE_SECURED_CMA 0x02 > > #define PCI_DOE_DW_SIZE_MAX (1 << 18) > #define PCI_DOE_PROTOCOL_NUM_MAX 256
Re: [PATCH] tests/tcg/xtensa: add linker.ld to CLEANFILES
On Tue, 2023-03-14 at 17:08 -0700, Max Filippov wrote: > On Tue, Mar 14, 2023 at 4:41 PM Wilfred Mallawa > wrote: > > > > On Tue, 2023-03-14 at 15:08 -0700, Max Filippov wrote: > > > Linker script for xtensa tests must be preprocessed for a > > > specific > > > target, remove it as a part of make clean. > > > > > > Signed-off-by: Max Filippov > > > --- > > > tests/tcg/xtensa/Makefile.softmmu-target | 1 + > > > 1 file changed, 1 insertion(+) > > > Wilfred Mallawa > > The tag is missing, I assume you meant Reviewed-by. Ah crap, sorry! yes I did. Wilfred >
Re: [PATCH] tests/tcg/xtensa: add linker.ld to CLEANFILES
On Tue, 2023-03-14 at 15:08 -0700, Max Filippov wrote: > Linker script for xtensa tests must be preprocessed for a specific > target, remove it as a part of make clean. > > Signed-off-by: Max Filippov > --- > tests/tcg/xtensa/Makefile.softmmu-target | 1 + > 1 file changed, 1 insertion(+) Wilfred Mallawa > > diff --git a/tests/tcg/xtensa/Makefile.softmmu-target > b/tests/tcg/xtensa/Makefile.softmmu-target > index 973e55298ee4..948c0e6506bd 100644 > --- a/tests/tcg/xtensa/Makefile.softmmu-target > +++ b/tests/tcg/xtensa/Makefile.softmmu-target > @@ -26,6 +26,7 @@ ASFLAGS = -Wa,--no-absolute-literals > LDFLAGS = -Tlinker.ld -nostartfiles -nostdlib > > CRT = crt.o vectors.o > +CLEANFILES += linker.ld > > linker.ld: linker.ld.S > $(CC) $(XTENSA_INC) -E -P $< -o $@
Re: [PATCH for-8.0] hw/char/cadence_uart: Fix guards on invalid BRGR/BDIV settings
On Tue, 2023-03-14 at 17:08 +, Peter Maydell wrote:
> The cadence UART attempts to avoid allowing the guset to set invalid
> baud rate register values in the uart_write() function. However it
> does the "mask to the size of the register field" and "check for
> invalid values" in the wrong order, which means that a malicious
> guest can get a bogus value into the register by setting also some
> high bits in the value, and cause QEMU to crash by division-by-zero.
>
> Do the mask before the bounds check instead of afterwards.
>
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1493
> Signed-off-by: Peter Maydell
> ---
> hw/char/cadence_uart.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
> index c069a30842e..807e3985419 100644
> --- a/hw/char/cadence_uart.c
> +++ b/hw/char/cadence_uart.c
> @@ -450,13 +450,15 @@ static MemTxResult uart_write(void *opaque,
> hwaddr offset,
> }
> break;
> case R_BRGR: /* Baud rate generator */
> + value &= 0x;
> if (value >= 0x01) {
> - s->r[offset] = value & 0x;
> + s->r[offset] = value;
> }
> break;
> case R_BDIV: /* Baud rate divider */
> + value &= 0xff;
> if (value >= 0x04) {
> - s->r[offset] = value & 0xFF;
> + s->r[offset] = value;
> }
> break;
> default:
Re: [PATCH] include/blcok: fixup typos
On Mon, 2023-03-13 at 10:01 +, Peter Maydell wrote: > On Mon, 13 Mar 2023 at 00:26, Wilfred Mallawa > wrote: > > > > From: Wilfred Mallawa > > > > Fixup a few minor typos > > Typo in patch subject line: should be 'block' :-) Ha! already sent a V2 for this :) > > > Signed-off-by: Wilfred Mallawa > > --- > > Otherwise > Reviewed-by: Peter Maydell > > thanks > -- PMM
[PATCH v2] include/block: fixup typos
From: Wilfred Mallawa Fixup a few minor typos Signed-off-by: Wilfred Mallawa --- v2: - Fixup typo in commit msg. include/block/aio-wait.h | 2 +- include/block/block_int-common.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h index da13357bb8..6e43e3b7bb 100644 --- a/include/block/aio-wait.h +++ b/include/block/aio-wait.h @@ -63,7 +63,7 @@ extern AioWait global_aio_wait; * @ctx: the aio context, or NULL if multiple aio contexts (for which the * caller does not hold a lock) are involved in the polling condition. * @cond: wait while this conditional expression is true - * @unlock: whether to unlock and then lock again @ctx. This apples + * @unlock: whether to unlock and then lock again @ctx. This applies * only when waiting for another AioContext from the main loop. * Otherwise it's ignored. * diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index d419017328..ce51c1f7f9 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -1254,7 +1254,7 @@ extern QemuOptsList bdrv_create_opts_simple; /* * Common functions that are neither I/O nor Global State. * - * See include/block/block-commmon.h for more information about + * See include/block/block-common.h for more information about * the Common API. */ -- 2.39.2
[PATCH] include/blcok: fixup typos
From: Wilfred Mallawa Fixup a few minor typos Signed-off-by: Wilfred Mallawa --- include/block/aio-wait.h | 2 +- include/block/block_int-common.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/block/aio-wait.h b/include/block/aio-wait.h index da13357bb8..6e43e3b7bb 100644 --- a/include/block/aio-wait.h +++ b/include/block/aio-wait.h @@ -63,7 +63,7 @@ extern AioWait global_aio_wait; * @ctx: the aio context, or NULL if multiple aio contexts (for which the * caller does not hold a lock) are involved in the polling condition. * @cond: wait while this conditional expression is true - * @unlock: whether to unlock and then lock again @ctx. This apples + * @unlock: whether to unlock and then lock again @ctx. This applies * only when waiting for another AioContext from the main loop. * Otherwise it's ignored. * diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index d419017328..ce51c1f7f9 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -1254,7 +1254,7 @@ extern QemuOptsList bdrv_create_opts_simple; /* * Common functions that are neither I/O nor Global State. * - * See include/block/block-commmon.h for more information about + * See include/block/block-common.h for more information about * the Common API. */ -- 2.39.2
Re: [PATCH v2 6/6] monitor: convert monitor_cleanup() to AIO_WAIT_WHILE_UNLOCKED()
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote: > monitor_cleanup() is called from the main loop thread. Calling > AIO_WAIT_WHILE(qemu_get_aio_context(), ...) from the main loop thread > is > equivalent to AIO_WAIT_WHILE_UNLOCKED(NULL, ...) because neither > unlocks > the AioContext and the latter's assertion that we're in the main loop > succeeds. > > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé > Reviewed-by: Markus Armbruster > Reviewed-by: Kevin Wolf > Signed-off-by: Stefan Hajnoczi > --- > monitor/monitor.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Wilfred Mallawa > > diff --git a/monitor/monitor.c b/monitor/monitor.c > index 8dc96f6af9..602535696c 100644 > --- a/monitor/monitor.c > +++ b/monitor/monitor.c > @@ -666,7 +666,7 @@ void monitor_cleanup(void) > * We need to poll both qemu_aio_context and iohandler_ctx to > make > * sure that the dispatcher coroutine keeps making progress and > * eventually terminates. qemu_aio_context is automatically > - * polled by calling AIO_WAIT_WHILE on it, but we must poll > + * polled by calling AIO_WAIT_WHILE_UNLOCKED on it, but we must > poll > * iohandler_ctx manually. > * > * Letting the iothread continue while shutting down the > dispatcher > @@ -679,7 +679,7 @@ void monitor_cleanup(void) > aio_co_wake(qmp_dispatcher_co); > } > > - AIO_WAIT_WHILE(qemu_get_aio_context(), > + AIO_WAIT_WHILE_UNLOCKED(NULL, > (aio_poll(iohandler_get_aio_context(), false), > qatomic_mb_read(_dispatcher_co_busy))); >
Re: [PATCH v2 5/6] hmp: convert handle_hmp_command() to AIO_WAIT_WHILE_UNLOCKED()
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote: > The HMP monitor runs in the main loop thread. Calling > AIO_WAIT_WHILE(qemu_get_aio_context(), ...) from the main loop thread > is > equivalent to AIO_WAIT_WHILE_UNLOCKED(NULL, ...) because neither > unlocks > the AioContext and the latter's assertion that we're in the main loop > succeeds. > > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé > Reviewed-by: Markus Armbruster > Reviewed-by: Kevin Wolf > Signed-off-by: Stefan Hajnoczi > --- > monitor/hmp.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Wilfred Mallawa > > diff --git a/monitor/hmp.c b/monitor/hmp.c > index fee410362f..5cab56d355 100644 > --- a/monitor/hmp.c > +++ b/monitor/hmp.c > @@ -1167,7 +1167,7 @@ void handle_hmp_command(MonitorHMP *mon, const > char *cmdline) > Coroutine *co = qemu_coroutine_create(handle_hmp_command_co, > ); > monitor_set_cur(co, >common); > aio_co_enter(qemu_get_aio_context(), co); > - AIO_WAIT_WHILE(qemu_get_aio_context(), !data.done); > + AIO_WAIT_WHILE_UNLOCKED(NULL, !data.done); > } > > qobject_unref(qdict);
Re: [PATCH v2 4/6] block: convert bdrv_drain_all_begin() to AIO_WAIT_WHILE_UNLOCKED()
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote:
> Since the AioContext argument was already NULL, AIO_WAIT_WHILE() was
> never going to unlock the AioContext. Therefore it is possible to
> replace AIO_WAIT_WHILE() with AIO_WAIT_WHILE_UNLOCKED().
>
> Reviewed-by: Philippe Mathieu-Daudé
> Tested-by: Philippe Mathieu-Daudé
> Reviewed-by: Kevin Wolf
> Signed-off-by: Stefan Hajnoczi
> ---
> block/io.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/block/io.c b/block/io.c
> index 8974d46941..db438c7657 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -520,7 +520,7 @@ void bdrv_drain_all_begin(void)
> bdrv_drain_all_begin_nopoll();
>
> /* Now poll the in-flight requests */
> - AIO_WAIT_WHILE(NULL, bdrv_drain_all_poll());
> + AIO_WAIT_WHILE_UNLOCKED(NULL, bdrv_drain_all_poll());
>
> while ((bs = bdrv_next_all_states(bs))) {
> bdrv_drain_assert_idle(bs);
Re: [PATCH v2 3/6] block: convert bdrv_graph_wrlock() to AIO_WAIT_WHILE_UNLOCKED()
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote:
> The following conversion is safe and does not change behavior:
>
> GLOBAL_STATE_CODE();
> ...
> - AIO_WAIT_WHILE(qemu_get_aio_context(), ...);
> + AIO_WAIT_WHILE_UNLOCKED(NULL, ...);
>
> Since we're in GLOBAL_STATE_CODE(), qemu_get_aio_context() is our
> home
> thread's AioContext. Thus AIO_WAIT_WHILE() does not unlock the
> AioContext:
>
> if (ctx_ && in_aio_context_home_thread(ctx_)) { \
> while ((cond)) { \
> aio_poll(ctx_, true); \
> waited_ = true; \
> } \
>
> And that means AIO_WAIT_WHILE_UNLOCKED(NULL, ...) can be substituted.
>
> Reviewed-by: Philippe Mathieu-Daudé
> Tested-by: Philippe Mathieu-Daudé
> Reviewed-by: Kevin Wolf
> Signed-off-by: Stefan Hajnoczi
> ---
> block/graph-lock.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/block/graph-lock.c b/block/graph-lock.c
> index 454c31e691..639526608f 100644
> --- a/block/graph-lock.c
> +++ b/block/graph-lock.c
> @@ -127,7 +127,7 @@ void bdrv_graph_wrlock(void)
> * reader lock.
> */
> qatomic_set(_writer, 0);
> - AIO_WAIT_WHILE(qemu_get_aio_context(), reader_count() >= 1);
> + AIO_WAIT_WHILE_UNLOCKED(NULL, reader_count() >= 1);
> qatomic_set(_writer, 1);
>
> /*
Re: [PATCH v2 2/6] block: convert blk_exp_close_all_type() to AIO_WAIT_WHILE_UNLOCKED()
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote: > There is no change in behavior. Switch to AIO_WAIT_WHILE_UNLOCKED() > instead of AIO_WAIT_WHILE() to document that this code has already > been > audited and converted. The AioContext argument is already NULL so > aio_context_release() is never called anyway. > > Reviewed-by: Philippe Mathieu-Daudé > Tested-by: Philippe Mathieu-Daudé > Reviewed-by: Kevin Wolf > Signed-off-by: Stefan Hajnoczi > --- > block/export/export.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Wilfred Mallawa > > diff --git a/block/export/export.c b/block/export/export.c > index 28a91c9c42..e3fee60611 100644 > --- a/block/export/export.c > +++ b/block/export/export.c > @@ -306,7 +306,7 @@ void blk_exp_close_all_type(BlockExportType type) > blk_exp_request_shutdown(exp); > } > > - AIO_WAIT_WHILE(NULL, blk_exp_has_type(type)); > + AIO_WAIT_WHILE_UNLOCKED(NULL, blk_exp_has_type(type)); > } > > void blk_exp_close_all(void)
Re: [PATCH v2 1/6] block: don't acquire AioContext lock in bdrv_drain_all()
On Thu, 2023-03-09 at 14:08 -0500, Stefan Hajnoczi wrote:
> There is no need for the AioContext lock in bdrv_drain_all() because
> nothing in AIO_WAIT_WHILE() needs the lock and the condition is
> atomic.
>
> AIO_WAIT_WHILE_UNLOCKED() has no use for the AioContext parameter
> other
> than performing a check that is nowadays already done by the
> GLOBAL_STATE_CODE()/IO_CODE() macros. Set the ctx argument to NULL
> here
> to help us keep track of all converted callers. Eventually all
> callers
> will have been converted and then the argument can be dropped
> entirely.
>
> Reviewed-by: Kevin Wolf
> Signed-off-by: Stefan Hajnoczi
> ---
> block/block-backend.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/block/block-backend.c b/block/block-backend.c
> index 278b04ce69..d2b6b3652d 100644
> --- a/block/block-backend.c
> +++ b/block/block-backend.c
> @@ -1835,14 +1835,8 @@ void blk_drain_all(void)
> bdrv_drain_all_begin();
>
> while ((blk = blk_all_next(blk)) != NULL) {
> - AioContext *ctx = blk_get_aio_context(blk);
> -
> - aio_context_acquire(ctx);
> -
> /* We may have -ENOMEDIUM completions in flight */
> - AIO_WAIT_WHILE(ctx, qatomic_mb_read(>in_flight) > 0);
> -
> - aio_context_release(ctx);
> + AIO_WAIT_WHILE_UNLOCKED(NULL, qatomic_mb_read(
> >in_flight) > 0);
> }
>
> bdrv_drain_all_end();
Re: [PATCH 04/45] target/riscv: Refactor some of the generic vector functionality
On Fri, 2023-03-10 at 09:11 +, Lawrence Hunter wrote:
> From: Kiran Ostrolenk
>
> This refactoring ensures these functions/macros can be used by both
> vector and vector-crypto helpers (latter implemented in proceeding
> commit).
>
> Signed-off-by: Kiran Ostrolenk
> ---
> target/riscv/vector_helper.c | 36 ---
> --
> target/riscv/vector_internals.c | 24 ++
> target/riscv/vector_internals.h | 16 +++
> 3 files changed, 40 insertions(+), 36 deletions(-)
>
Reviewed-by: Wilfred Mallawa
> diff --git a/target/riscv/vector_helper.c
> b/target/riscv/vector_helper.c
> index 823aa8eb08..09b790653e 100644
> --- a/target/riscv/vector_helper.c
> +++ b/target/riscv/vector_helper.c
> @@ -721,8 +721,6 @@ GEN_VEXT_VV(vsub_vv_h, 2)
> GEN_VEXT_VV(vsub_vv_w, 4)
> GEN_VEXT_VV(vsub_vv_d, 8)
>
> -typedef void opivx2_fn(void *vd, target_long s1, void *vs2, int i);
> -
> /*
> * (T1)s1 gives the real operator type.
> * (TX1)(T1)s1 expands the operator type of widen or narrow
> operations.
> @@ -747,40 +745,6 @@ RVVCALL(OPIVX2, vrsub_vx_h, OP_SSS_H, H2, H2,
> DO_RSUB)
> RVVCALL(OPIVX2, vrsub_vx_w, OP_SSS_W, H4, H4, DO_RSUB)
> RVVCALL(OPIVX2, vrsub_vx_d, OP_SSS_D, H8, H8, DO_RSUB)
>
> -static void do_vext_vx(void *vd, void *v0, target_long s1, void
> *vs2,
> - CPURISCVState *env, uint32_t desc,
> - opivx2_fn fn, uint32_t esz)
> -{
> - uint32_t vm = vext_vm(desc);
> - uint32_t vl = env->vl;
> - uint32_t total_elems = vext_get_total_elems(env, desc, esz);
> - uint32_t vta = vext_vta(desc);
> - uint32_t vma = vext_vma(desc);
> - uint32_t i;
> -
> - for (i = env->vstart; i < vl; i++) {
> - if (!vm && !vext_elem_mask(v0, i)) {
> - /* set masked-off elements to 1s */
> - vext_set_elems_1s(vd, vma, i * esz, (i + 1) * esz);
> - continue;
> - }
> - fn(vd, s1, vs2, i);
> - }
> - env->vstart = 0;
> - /* set tail elements to 1s */
> - vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
> -}
> -
> -/* generate the helpers for OPIVX */
> -#define GEN_VEXT_VX(NAME, ESZ) \
> -void HELPER(NAME)(void *vd, void *v0, target_ulong s1, \
> - void *vs2, CPURISCVState *env, \
> - uint32_t desc) \
> -{ \
> - do_vext_vx(vd, v0, s1, vs2, env, desc, \
> - do_##NAME, ESZ); \
> -}
> -
> GEN_VEXT_VX(vadd_vx_b, 1)
> GEN_VEXT_VX(vadd_vx_h, 2)
> GEN_VEXT_VX(vadd_vx_w, 4)
> diff --git a/target/riscv/vector_internals.c
> b/target/riscv/vector_internals.c
> index 95efaa79cb..9cf5c17cde 100644
> --- a/target/riscv/vector_internals.c
> +++ b/target/riscv/vector_internals.c
> @@ -55,3 +55,27 @@ void do_vext_vv(void *vd, void *v0, void *vs1,
> void *vs2,
> /* set tail elements to 1s */
> vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
> }
> +
> +void do_vext_vx(void *vd, void *v0, target_long s1, void *vs2,
> + CPURISCVState *env, uint32_t desc,
> + opivx2_fn fn, uint32_t esz)
> +{
> + uint32_t vm = vext_vm(desc);
> + uint32_t vl = env->vl;
> + uint32_t total_elems = vext_get_total_elems(env, desc, esz);
> + uint32_t vta = vext_vta(desc);
> + uint32_t vma = vext_vma(desc);
> + uint32_t i;
> +
> + for (i = env->vstart; i < vl; i++) {
> + if (!vm && !vext_elem_mask(v0, i)) {
> + /* set masked-off elements to 1s */
> + vext_set_elems_1s(vd, vma, i * esz, (i + 1) * esz);
> + continue;
> + }
> + fn(vd, s1, vs2, i);
> + }
> + env->vstart = 0;
> + /* set tail elements to 1s */
> + vext_set_elems_1s(vd, vta, vl * esz, total_elems * esz);
> +}
> diff --git a/target/riscv/vector_internals.h
> b/target/riscv/vector_internals.h
> index 1d26ff9514..90500e5df6 100644
> --- a/target/riscv/vector_internals.h
> +++ b/target/riscv/vector_internals.h
> @@ -115,4 +115,20 @@ void HELPER(NAME)(void *vd, void *v0, void
> *vs1, \
> do_##NAME, ESZ); \
> }
>
> +typedef void opivx2_fn(void *vd, target_long s1, void *vs2, int i);
> +
> +void do_vext_vx(void *vd, void *v0, target_long s1, void *vs2,
> + CPURISCVState *env, uint32_t desc,
> + opivx2_fn fn, uint32_t esz);
> +
> +/* generate the helpers for OPIVX */
&
Re: [PATCH v2 6/6] gitlab-ci.d/crossbuilds: Drop the 32-bit arm system emulation jobs
On Thu, 2023-03-02 at 17:31 +0100, Thomas Huth wrote: > Hardly anybody still uses 32-bit arm environments for running QEMU, > so let's stop wasting our scarce CI minutes with these jobs. > > Signed-off-by: Thomas Huth > --- > .gitlab-ci.d/crossbuilds.yml | 14 -- > 1 file changed, 14 deletions(-) Reviewed-by: Wilfred Mallawa > > diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab- > ci.d/crossbuilds.yml > index 3ce51adf77..419b0c2fe1 100644 > --- a/.gitlab-ci.d/crossbuilds.yml > +++ b/.gitlab-ci.d/crossbuilds.yml > @@ -1,13 +1,6 @@ > include: > - local: '/.gitlab-ci.d/crossbuild-template.yml' > > -cross-armel-system: > - extends: .cross_system_build_job > - needs: > - job: armel-debian-cross-container > - variables: > - IMAGE: debian-armel-cross > - > cross-armel-user: > extends: .cross_user_build_job > needs: > @@ -15,13 +8,6 @@ cross-armel-user: > variables: > IMAGE: debian-armel-cross > > -cross-armhf-system: > - extends: .cross_system_build_job > - needs: > - job: armhf-debian-cross-container > - variables: > - IMAGE: debian-armhf-cross > - > cross-armhf-user: > extends: .cross_user_build_job > needs:
Re: [PATCH v2 5/6] docs/about/deprecated: Deprecate 32-bit arm hosts
On Thu, 2023-03-02 at 17:31 +0100, Thomas Huth wrote: > For running QEMU in system emulation mode, the user needs a rather > strong host system, i.e. not only an embedded low-frequency > controller. > All recent beefy arm host machines should support 64-bit now, it's > unlikely that anybody is still seriously using QEMU on a 32-bit arm > CPU, so we deprecate the 32-bit arm hosts here to finally save use > some time and precious CI minutes. > > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 9 + > 1 file changed, 9 insertions(+) Reviewed-by: Wilfred Mallawa > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 21ce70b5c9..c7113a7510 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -229,6 +229,15 @@ discontinue it. Since all recent x86 hardware > from the past >10 years > is capable of the 64-bit x86 extensions, a corresponding 64-bit OS > should be used instead. > > +System emulation on 32-bit arm hosts (since 8.0) > + > + > +Since QEMU needs a strong host machine for running full system > emulation, and > +all recent powerful arm hosts support 64-bit, the QEMU project > deprecates the > +support for running any system emulation on 32-bit arm hosts in > general. Use > +64-bit arm hosts for system emulation instead. (Note: "user" mode > emulation > +continuous to be supported on 32-bit arm hosts, too) > + > > QEMU API (QAPI) events > --
Re: [PATCH v2 4/6] docs/about/deprecated: Deprecate the qemu-system-arm binary
On Thu, 2023-03-02 at 17:31 +0100, Thomas Huth wrote: > qemu-system-aarch64 is a proper superset of qemu-system-arm, > and the latter was mainly still required for 32-bit KVM support. > But this 32-bit KVM arm support has been dropped in the Linux > kernel a couple of years ago already, so we don't really need > qemu-system-arm anymore, thus deprecated it now. > > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 10 ++ > 1 file changed, 10 insertions(+) Reviewed-by: Wilfred Mallawa > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index a30aa8dfdf..21ce70b5c9 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -45,6 +45,16 @@ run 32-bit guests by selecting a 32-bit CPU model, > including KVM support > on x86_64 hosts. Thus users are recommended to reconfigure their > systems > to use the ``qemu-system-x86_64`` binary instead. > > +``qemu-system-arm`` binary (since 8.0) > +'' > + > +``qemu-system-aarch64`` is a proper superset of ``qemu-system-arm``. > The > +latter was mainly a requirement for running KVM on 32-bit arm hosts, > but > +this 32-bit KVM support has been removed some years ago already > (see: > + > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/com > mit/?id=541ad0150ca4 > +). Thus the QEMU project will drop the ``qemu-system-arm`` binary in > a > +future release. Use ``qemu-system-aarch64`` instead. > + > > System emulator command line arguments > --
Re: [PATCH v2 3/6] gitlab-ci.d/crossbuilds: Drop the i386 jobs
On Thu, 2023-03-02 at 17:31 +0100, Thomas Huth wrote: > Hardly anybody still uses 32-bit x86 environments for running QEMU, > so let's stop wasting our scarce CI minutes with these jobs. > > Signed-off-by: Thomas Huth > --- > .gitlab-ci.d/crossbuilds.yml | 16 > 1 file changed, 16 deletions(-) Reviewed-by: Wilfred Mallawa > > diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab- > ci.d/crossbuilds.yml > index 101416080c..3ce51adf77 100644 > --- a/.gitlab-ci.d/crossbuilds.yml > +++ b/.gitlab-ci.d/crossbuilds.yml > @@ -43,22 +43,6 @@ cross-arm64-user: > variables: > IMAGE: debian-arm64-cross > > -cross-i386-system: > - extends: .cross_system_build_job > - needs: > - job: i386-fedora-cross-container > - variables: > - IMAGE: fedora-i386-cross > - MAKE_CHECK_ARGS: check-qtest > - > -cross-i386-user: > - extends: .cross_user_build_job > - needs: > - job: i386-fedora-cross-container > - variables: > - IMAGE: fedora-i386-cross > - MAKE_CHECK_ARGS: check > - > cross-i386-tci: > extends: .cross_accel_build_job > timeout: 60m
Re: [PATCH v2 2/6] docs/about/deprecated: Deprecate 32-bit x86 hosts
On Thu, 2023-03-02 at 17:31 +0100, Thomas Huth wrote: > Hardly anybody still uses 32-bit x86 hosts today, so we should start > deprecating them to stop wasting our time and CI minutes here. > For example, there are also still some unresolved problems with > these: > When emulating 64-bit binaries in user mode, TCG does not honor > atomicity > for 64-bit accesses, which is "perhaps worse than not working at all" > (quoting Richard). Let's simply make it clear that people should use > 64-bit x86 hosts nowadays and we do not intend to fix/maintain the > old > 32-bit stuff. > > Signed-off-by: Thomas Huth > --- > docs/about/deprecated.rst | 12 ++++ > 1 file changed, 12 insertions(+) Reviewed-by: Wilfred Mallawa > > diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst > index 11700adac9..a30aa8dfdf 100644 > --- a/docs/about/deprecated.rst > +++ b/docs/about/deprecated.rst > @@ -208,6 +208,18 @@ CI coverage support may bitrot away before the > deprecation process > completes. The little endian variants of MIPS (both 32 and 64 bit) > are > still a supported host architecture. > > +32-bit x86 hosts (since 8.0) > + > + > +Support for 32-bit x86 host deployments is increasingly uncommon in > +mainstream OS distributions given the widespread availability of 64- > bit > +x86 hardware. The QEMU project no longer considers 32-bit x86 > support > +to be an effective use of its limited resources, and thus intends to > +discontinue it. Since all recent x86 hardware from the past >10 > years > +is capable of the 64-bit x86 extensions, a corresponding 64-bit OS > +should be used instead. > + > + > QEMU API (QAPI) events > -- >
Re: [PATCH v2 1/6] docs/about/deprecated: Deprecate the qemu-system-i386 binary
On Thu, 2023-03-02 at 18:05 +, Daniel P. Berrangé wrote: > On Thu, Mar 02, 2023 at 05:31:01PM +0100, Thomas Huth wrote: > > Hardly anybody really requires the i386 binary anymore, since the > > qemu-system-x86_64 binary is a proper superset. So let's deprecate > > the 32-bit variant now, so that we can finally stop wasting our > > time > > and CI minutes with this. > > The first sentence isn't quite true wrt to KVM. Change slightly to: > > Aside from not supporting KVM on 32-bit hosts, the qemu-system-x86_64 > binary is a proper superset of the qemu-system-i386 binary. With the > 32-bit host support being deprecated, it is now also possible to > deprecate the qemu-system-i386 binary. > +1 > > With regards to 32-bit KVM support in the x86 Linux kernel, > > the developers confirmed that they do not need a recent > > qemu-system-i386 binary here: > > > > https://lore.kernel.org/kvm/y%2ffkts5ajfy0h...@google.com/ > > > > Signed-off-by: Thomas Huth > > --- > > docs/about/deprecated.rst | 12 ++++ > > 1 file changed, 12 insertions(+) > > Reviewed-by: Daniel P. Berrangé Reviewed-by: Wilfred Mallawa > > > With regards, > Daniel
[PATCH v2] include/hw/riscv/opentitan: update opentitan IRQs
From: Wilfred Mallawa
Updates the opentitan IRQs to match the latest supported commit of
Opentitan from TockOS.
OPENTITAN_SUPPORTED_SHA := 565e4af39760a123c59a184aa2f5812a961fde47
Memory layout as per [1]
[1]
https://github.com/lowRISC/opentitan/blob/565e4af39760a123c59a184aa2f5812a961fde47/hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h
Signed-off-by: Wilfred Mallawa
---
Changes in v2:
- Updated the MMIO register layout/size
- Bumped the supported commit sha
- Added link to OT register layout for reference in the commit
msg
hw/riscv/opentitan.c | 80 ++--
include/hw/riscv/opentitan.h | 14 +++
2 files changed, 47 insertions(+), 47 deletions(-)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 64d5d435b9..353f030d80 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -31,47 +31,47 @@
/*
* This version of the OpenTitan machine currently supports
* OpenTitan RTL version:
- *
+ *
*
* MMIO mapping as per (specified commit):
* lowRISC/opentitan: hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h
*/
static const MemMapEntry ibex_memmap[] = {
-[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
-[IBEX_DEV_RAM] ={ 0x1000, 0x2 },
-[IBEX_DEV_FLASH] = { 0x2000, 0x10 },
-[IBEX_DEV_UART] = { 0x4000, 0x1000 },
-[IBEX_DEV_GPIO] = { 0x4004, 0x1000 },
-[IBEX_DEV_SPI_DEVICE] = { 0x4005, 0x1000 },
-[IBEX_DEV_I2C] ={ 0x4008, 0x1000 },
-[IBEX_DEV_PATTGEN] ={ 0x400e, 0x1000 },
-[IBEX_DEV_TIMER] = { 0x4010, 0x1000 },
-[IBEX_DEV_OTP_CTRL] = { 0x4013, 0x4000 },
-[IBEX_DEV_LC_CTRL] ={ 0x4014, 0x1000 },
-[IBEX_DEV_ALERT_HANDLER] = { 0x4015, 0x1000 },
-[IBEX_DEV_SPI_HOST0] = { 0x4030, 0x1000 },
-[IBEX_DEV_SPI_HOST1] = { 0x4031, 0x1000 },
-[IBEX_DEV_USBDEV] = { 0x4032, 0x1000 },
-[IBEX_DEV_PWRMGR] = { 0x4040, 0x1000 },
-[IBEX_DEV_RSTMGR] = { 0x4041, 0x1000 },
-[IBEX_DEV_CLKMGR] = { 0x4042, 0x1000 },
-[IBEX_DEV_PINMUX] = { 0x4046, 0x1000 },
-[IBEX_DEV_AON_TIMER] = { 0x4047, 0x1000 },
-[IBEX_DEV_SENSOR_CTRL] ={ 0x4049, 0x1000 },
-[IBEX_DEV_FLASH_CTRL] = { 0x4100, 0x1000 },
-[IBEX_DEV_AES] ={ 0x4110, 0x1000 },
-[IBEX_DEV_HMAC] = { 0x4111, 0x1000 },
-[IBEX_DEV_KMAC] = { 0x4112, 0x1000 },
-[IBEX_DEV_OTBN] = { 0x4113, 0x1 },
-[IBEX_DEV_KEYMGR] = { 0x4114, 0x1000 },
-[IBEX_DEV_CSRNG] = { 0x4115, 0x1000 },
-[IBEX_DEV_ENTROPY] ={ 0x4116, 0x1000 },
-[IBEX_DEV_EDNO] = { 0x4117, 0x1000 },
-[IBEX_DEV_EDN1] = { 0x4118, 0x1000 },
-[IBEX_DEV_NMI_GEN] ={ 0x411c, 0x1000 },
-[IBEX_DEV_PERI] = { 0x411f, 0x1 },
-[IBEX_DEV_PLIC] = { 0x4800, 0x4005000 },
-[IBEX_DEV_FLASH_VIRTUAL] = { 0x8000, 0x8 },
+[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
+[IBEX_DEV_RAM] ={ 0x1000, 0x2 },
+[IBEX_DEV_FLASH] = { 0x2000, 0x10},
+[IBEX_DEV_UART] = { 0x4000, 0x40},
+[IBEX_DEV_GPIO] = { 0x4004, 0x40},
+[IBEX_DEV_SPI_DEVICE] = { 0x4005, 0x2000 },
+[IBEX_DEV_I2C] ={ 0x4008, 0x80},
+[IBEX_DEV_PATTGEN] ={ 0x400e, 0x40},
+[IBEX_DEV_TIMER] = { 0x4010, 0x200 },
+[IBEX_DEV_OTP_CTRL] = { 0x4013, 0x2000 },
+[IBEX_DEV_LC_CTRL] ={ 0x4014, 0x100 },
+[IBEX_DEV_ALERT_HANDLER] = { 0x4015, 0x800 },
+[IBEX_DEV_SPI_HOST0] = { 0x4030, 0x40},
+[IBEX_DEV_SPI_HOST1] = { 0x4031, 0x40},
+[IBEX_DEV_USBDEV] = { 0x4032, 0x1000 },
+[IBEX_DEV_PWRMGR] = { 0x4040, 0x80},
+[IBEX_DEV_RSTMGR] = { 0x4041, 0x80},
+[IBEX_DEV_CLKMGR] = { 0x4042, 0x80},
+[IBEX_DEV_PINMUX] = { 0x4046, 0x1000 },
+[IBEX_DEV_AON_TIMER] = { 0x4047, 0x40},
+[IBEX_DEV_SENSOR_CTRL] ={ 0x4049, 0x40},
+[IBEX_DEV_FLASH_CTRL] = { 0x4100, 0x200 },
+[IBEX_DEV_AES] ={ 0x4110, 0x100 },
+[IBEX_DEV_HMAC] = { 0x4111, 0x1000 },
+[IBEX_DEV_KMAC] = { 0x4112, 0x1000 },
+[IBEX_DEV_OTBN] = { 0x4113, 0x1 },
+[IBEX_DEV_KEYMGR] = { 0x4114
[PATCH] include/hw/riscv/opentitan: update opentitan IRQs
From: Wilfred Mallawa
Updates the opentitan IRQs to match the latest supported commit of
Opentitan from TockOS.
OPENTITAN_SUPPORTED_SHA := 565e4af39760a123c59a184aa2f5812a961fde47
Signed-off-by: Wilfred Mallawa
---
include/hw/riscv/opentitan.h | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 7659d1bc5b..235728b9cc 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -108,11 +108,11 @@ enum {
IBEX_UART0_RX_BREAK_ERR_IRQ = 6,
IBEX_UART0_RX_TIMEOUT_IRQ = 7,
IBEX_UART0_RX_PARITY_ERR_IRQ = 8,
-IBEX_TIMER_TIMEREXPIRED0_0= 127,
-IBEX_SPI_HOST0_ERR_IRQ= 134,
-IBEX_SPI_HOST0_SPI_EVENT_IRQ = 135,
-IBEX_SPI_HOST1_ERR_IRQ= 136,
-IBEX_SPI_HOST1_SPI_EVENT_IRQ = 137,
+IBEX_TIMER_TIMEREXPIRED0_0= 124,
+IBEX_SPI_HOST0_ERR_IRQ= 131,
+IBEX_SPI_HOST0_SPI_EVENT_IRQ = 132,
+IBEX_SPI_HOST1_ERR_IRQ= 133,
+IBEX_SPI_HOST1_SPI_EVENT_IRQ = 134,
};
#endif
--
2.39.0
Re: [PULL v2 03/45] hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro
On Wed, 2023-01-04 at 22:30 +1000, Alistair Francis wrote:
> On Thu, Dec 22, 2022 at 8:40 AM Alistair Francis
> wrote:
> >
> > From: Wilfred Mallawa
> >
> > use the `FIELD32_1CLEAR` macro to implement register
> > `rw1c` functionality to `ibex_spi`.
> >
> > This change was tested by running the `SPI_HOST` from TockOS.
> >
> > Signed-off-by: Wilfred Mallawa
> > Reviewed-by: Alistair Francis
> > Message-Id:
> > <20221017054950.317584-3-wilfred.mall...@opensource.wdc.com>
> > Signed-off-by: Alistair Francis
> > ---
> > hw/ssi/ibex_spi_host.c | 21 +
> > 1 file changed, 9 insertions(+), 12 deletions(-)
> >
> > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > index 57df462e3c..0a456cd1ed 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -342,7 +342,7 @@ static void ibex_spi_host_write(void *opaque,
> > hwaddr addr,
> > {
> > IbexSPIHostState *s = opaque;
> > uint32_t val32 = val64;
> > - uint32_t shift_mask = 0xff, status = 0, data = 0;
> > + uint32_t shift_mask = 0xff, status = 0;
> > uint8_t txqd_len;
> >
> > trace_ibex_spi_host_write(addr, size, val64);
> > @@ -355,12 +355,11 @@ static void ibex_spi_host_write(void *opaque,
> > hwaddr addr,
> > case IBEX_SPI_HOST_INTR_STATE:
> > /* rw1c status register */
> > if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
> > - data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > INTR_STATE, ERROR);
>
> It seems that this change doesn't build on Windows
> (https://cirrus-ci.com/task/697832247296?logs=main#L2163)
>
> Maybe ERROR is reserved? Either way I'll have to drop this commit.
> Maybe just drop this change and keep the rest?
>
> Alistair
Odd! yea I'll do that and send a new patch.
Wilfred
>
> > }
> > if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
> > - data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > INTR_STATE, SPI_EVENT);
> > }
> > - s->regs[addr] = data;
> > break;
> > case IBEX_SPI_HOST_INTR_ENABLE:
> > s->regs[addr] = val32;
> > @@ -505,27 +504,25 @@ static void ibex_spi_host_write(void *opaque,
> > hwaddr addr,
> > * When an error occurs, the corresponding bit must be
> > cleared
> > * here before issuing any further commands
> > */
> > - status = s->regs[addr];
> > /* rw1c status register */
> > if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, CMDBUSY);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, OVERFLOW);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, UNDERFLOW);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, CMDINVAL);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, CSIDINVAL);
> > }
> > if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
> > - status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL,
> > 0);
> > + s->regs[addr] = FIELD32_1CLEAR(s->regs[addr],
> > ERROR_STATUS, ACCESSINVAL);
> > }
> > - s->regs[addr] = status;
> > break;
> > case IBEX_SPI_HOST_EVENT_ENABLE:
> > /* Controls which classes of SPI events raise an interrupt. */
> > --
> > 2.38.1
> >
Re: [PATCH 0/5] cpus: Remove system reset() API from user emulation
On Tue, 2022-12-20 at 15:56 +0100, Philippe Mathieu-Daudé wrote: > In user emulation, threads -- implemented as CPU -- are > created/destroyed, but never reset. There is no point in > allowing the user emulation access the sysemu/reset API. > > Philippe Mathieu-Daudé (5): > target/i386: Restrict qapi/qapi-events-run-state.h to system > emulation > target/i386: Restrict sysemu/reset.h to system emulation > target/loongarch: Restrict sysemu/reset.h to system emulation > target/s390x: Restrict sysemu/reset.h to system emulation > hw/core: Only build CPU reset handlers with system emulation > Reviewed-by: Wilfred Mallawa > hw/core/meson.build | 2 +- > target/i386/cpu.c | 2 +- > target/i386/helper.c | 2 +- > target/loongarch/cpu.c | 2 ++ > target/s390x/cpu.c | 4 +++- > 5 files changed, 8 insertions(+), 4 deletions(-) >
Re: [PATCH v2 14/16] hw/intc: sifive_plic: Change "priority-base" to start from interrupt source 0
On Wed, 2022-12-07 at 18:03 +0800, Bin Meng wrote:
> At present the SiFive PLIC model "priority-base" expects interrupt
> priority register base starting from source 1 instead source 0,
> that's why on most platforms "priority-base" is set to 0x04 except
> 'opentitan' machine. 'opentitan' should have set "priority-base"
> to 0x04 too.
>
> Note the irq number calculation in sifive_plic_{read,write} is
> correct as the codes make up for the irq number by adding 1.
>
> Let's simply update "priority-base" to start from interrupt source
> 0 and add a comment to make it crystal clear.
>
> Signed-off-by: Bin Meng
> Reviewed-by: Alistair Francis
Reviewed-by: Wilfred Mallawa
Wilfred
> ---
>
> (no changes since v1)
>
> include/hw/riscv/microchip_pfsoc.h | 2 +-
> include/hw/riscv/shakti_c.h | 2 +-
> include/hw/riscv/sifive_e.h | 2 +-
> include/hw/riscv/sifive_u.h | 2 +-
> include/hw/riscv/virt.h | 2 +-
> hw/intc/sifive_plic.c | 5 +++--
> 6 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/hw/riscv/microchip_pfsoc.h
> b/include/hw/riscv/microchip_pfsoc.h
> index 577efad0c4..e65ffeb02d 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -155,7 +155,7 @@ enum {
>
> #define MICROCHIP_PFSOC_PLIC_NUM_SOURCES 187
> #define MICROCHIP_PFSOC_PLIC_NUM_PRIORITIES 7
> -#define MICROCHIP_PFSOC_PLIC_PRIORITY_BASE 0x04
> +#define MICROCHIP_PFSOC_PLIC_PRIORITY_BASE 0x00
> #define MICROCHIP_PFSOC_PLIC_PENDING_BASE 0x1000
> #define MICROCHIP_PFSOC_PLIC_ENABLE_BASE 0x2000
> #define MICROCHIP_PFSOC_PLIC_ENABLE_STRIDE 0x80
> diff --git a/include/hw/riscv/shakti_c.h
> b/include/hw/riscv/shakti_c.h
> index daf0aae13f..539fe1156d 100644
> --- a/include/hw/riscv/shakti_c.h
> +++ b/include/hw/riscv/shakti_c.h
> @@ -65,7 +65,7 @@ enum {
> #define SHAKTI_C_PLIC_NUM_SOURCES 28
> /* Excluding Priority 0 */
> #define SHAKTI_C_PLIC_NUM_PRIORITIES 2
> -#define SHAKTI_C_PLIC_PRIORITY_BASE 0x04
> +#define SHAKTI_C_PLIC_PRIORITY_BASE 0x00
> #define SHAKTI_C_PLIC_PENDING_BASE 0x1000
> #define SHAKTI_C_PLIC_ENABLE_BASE 0x2000
> #define SHAKTI_C_PLIC_ENABLE_STRIDE 0x80
> diff --git a/include/hw/riscv/sifive_e.h
> b/include/hw/riscv/sifive_e.h
> index 9e58247fd8..b824a79e2d 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -89,7 +89,7 @@ enum {
> */
> #define SIFIVE_E_PLIC_NUM_SOURCES 53
> #define SIFIVE_E_PLIC_NUM_PRIORITIES 7
> -#define SIFIVE_E_PLIC_PRIORITY_BASE 0x04
> +#define SIFIVE_E_PLIC_PRIORITY_BASE 0x00
> #define SIFIVE_E_PLIC_PENDING_BASE 0x1000
> #define SIFIVE_E_PLIC_ENABLE_BASE 0x2000
> #define SIFIVE_E_PLIC_ENABLE_STRIDE 0x80
> diff --git a/include/hw/riscv/sifive_u.h
> b/include/hw/riscv/sifive_u.h
> index 8f63a183c4..e680d61ece 100644
> --- a/include/hw/riscv/sifive_u.h
> +++ b/include/hw/riscv/sifive_u.h
> @@ -158,7 +158,7 @@ enum {
>
> #define SIFIVE_U_PLIC_NUM_SOURCES 54
> #define SIFIVE_U_PLIC_NUM_PRIORITIES 7
> -#define SIFIVE_U_PLIC_PRIORITY_BASE 0x04
> +#define SIFIVE_U_PLIC_PRIORITY_BASE 0x00
> #define SIFIVE_U_PLIC_PENDING_BASE 0x1000
> #define SIFIVE_U_PLIC_ENABLE_BASE 0x2000
> #define SIFIVE_U_PLIC_ENABLE_STRIDE 0x80
> diff --git a/include/hw/riscv/virt.h b/include/hw/riscv/virt.h
> index e1ce0048af..3407c9e8dd 100644
> --- a/include/hw/riscv/virt.h
> +++ b/include/hw/riscv/virt.h
> @@ -98,7 +98,7 @@ enum {
> #define VIRT_IRQCHIP_MAX_GUESTS_BITS 3
> #define VIRT_IRQCHIP_MAX_GUESTS ((1U <<
> VIRT_IRQCHIP_MAX_GUESTS_BITS) - 1U)
>
> -#define VIRT_PLIC_PRIORITY_BASE 0x04
> +#define VIRT_PLIC_PRIORITY_BASE 0x00
> #define VIRT_PLIC_PENDING_BASE 0x1000
> #define VIRT_PLIC_ENABLE_BASE 0x2000
> #define VIRT_PLIC_ENABLE_STRIDE 0x80
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index 1edeb1e1ed..1a792cc3f5 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -140,7 +140,7 @@ static uint64_t sifive_plic_read(void *opaque,
> hwaddr addr, unsigned size)
> SiFivePLICState *plic = opaque;
>
> if (addr_between(addr, plic->priority_base, plic->num_sources <<
> 2)) {
> - uint32_t irq = ((addr - plic->priority_base) >> 2) + 1;
> + uint32_t irq = (addr - plic->priority_base) >> 2;
>
> return plic->source_priority[irq];
> } else if (addr_between(addr, plic->pending_base, plic-
> >num_sources >> 3)) {
> @@ -187,7 +187,7 @@ static void sifive_plic_write(void *opaque,
> hwaddr addr, uint64_t value,
> SiF
Re: [PATCH v2 01/16] hw/riscv: Select MSI_NONBROKEN in SIFIVE_PLIC
On Wed, 2022-12-07 at 18:03 +0800, Bin Meng wrote: > hw/pci/Kconfig says MSI_NONBROKEN should be selected by interrupt > controllers regardless of how MSI is implemented. msi_nonbroken is > initialized to true in sifive_plic_realize(). > > Let SIFIVE_PLIC select MSI_NONBROKEN and drop the selection from > RISC-V machines. > > Signed-off-by: Bin Meng > Reviewed-by: Alistair Francis Reviewed-by: Wilfred Mallawa Wilfred > --- > > (no changes since v1) > > hw/intc/Kconfig | 1 + > hw/riscv/Kconfig | 5 - > 2 files changed, 1 insertion(+), 5 deletions(-) > > diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig > index ecd2883ceb..1d4573e803 100644 > --- a/hw/intc/Kconfig > +++ b/hw/intc/Kconfig > @@ -78,6 +78,7 @@ config RISCV_IMSIC > > config SIFIVE_PLIC > bool > + select MSI_NONBROKEN > > config GOLDFISH_PIC > bool > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig > index 79ff61c464..167dc4cca6 100644 > --- a/hw/riscv/Kconfig > +++ b/hw/riscv/Kconfig > @@ -11,7 +11,6 @@ config MICROCHIP_PFSOC > select MCHP_PFSOC_IOSCB > select MCHP_PFSOC_MMUART > select MCHP_PFSOC_SYSREG > - select MSI_NONBROKEN > select RISCV_ACLINT > select SIFIVE_PDMA > select SIFIVE_PLIC > @@ -37,7 +36,6 @@ config RISCV_VIRT > imply TPM_TIS_SYSBUS > select RISCV_NUMA > select GOLDFISH_RTC > - select MSI_NONBROKEN > select PCI > select PCI_EXPRESS_GENERIC_BRIDGE > select PFLASH_CFI01 > @@ -53,7 +51,6 @@ config RISCV_VIRT > > config SIFIVE_E > bool > - select MSI_NONBROKEN > select RISCV_ACLINT > select SIFIVE_GPIO > select SIFIVE_PLIC > @@ -64,7 +61,6 @@ config SIFIVE_E > config SIFIVE_U > bool > select CADENCE > - select MSI_NONBROKEN > select RISCV_ACLINT > select SIFIVE_GPIO > select SIFIVE_PDMA > @@ -82,6 +78,5 @@ config SPIKE > bool > select RISCV_NUMA > select HTIF > - select MSI_NONBROKEN > select RISCV_ACLINT > select SIFIVE_PLIC
Re: [PATCH v2 04/16] hw/riscv: Sort machines Kconfig options in alphabetical order
On Wed, 2022-12-07 at 18:03 +0800, Bin Meng wrote: > SHAKTI_C machine Kconfig option was inserted in disorder. Fix it. > > Signed-off-by: Bin Meng > Reviewed-by: Alistair Francis Reviewed-by: Wilfred Mallawa Wilfred > --- > > (no changes since v1) > > hw/riscv/Kconfig | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) > > diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig > index 1e4b58024f..4550b3b938 100644 > --- a/hw/riscv/Kconfig > +++ b/hw/riscv/Kconfig > @@ -4,6 +4,8 @@ config RISCV_NUMA > config IBEX > bool > > +# RISC-V machines in alphabetical order > + > config MICROCHIP_PFSOC > bool > select CADENCE_SDHCI > @@ -22,13 +24,6 @@ config OPENTITAN > select SIFIVE_PLIC > select UNIMP > > -config SHAKTI_C > - bool > - select UNIMP > - select SHAKTI_UART > - select RISCV_ACLINT > - select SIFIVE_PLIC > - > config RISCV_VIRT > bool > imply PCI_DEVICES > @@ -50,6 +45,13 @@ config RISCV_VIRT > select FW_CFG_DMA > select PLATFORM_BUS > > +config SHAKTI_C > + bool > + select RISCV_ACLINT > + select SHAKTI_UART > + select SIFIVE_PLIC > + select UNIMP > + > config SIFIVE_E > bool > select RISCV_ACLINT
Re: [PATCH 1/2] target/riscv: Simplify helper_sret() a little bit
On Wed, 2022-12-07 at 17:00 +0800, Bin Meng wrote:
> There are 2 paths in helper_sret() and the same mstatus update codes
> are replicated. Extract the common parts to simplify it a little bit.
>
> Signed-off-by: Bin Meng
Reviewed-by: Wilfred Mallawa
Wilfred
> ---
>
> target/riscv/op_helper.c | 20 ++--
> 1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index d7af7f056b..a047d38152 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -149,21 +149,21 @@ target_ulong helper_sret(CPURISCVState *env)
> }
>
> mstatus = env->mstatus;
> + prev_priv = get_field(mstatus, MSTATUS_SPP);
> + mstatus = set_field(mstatus, MSTATUS_SIE,
> + get_field(mstatus, MSTATUS_SPIE));
> + mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> + mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> + env->mstatus = mstatus;
>
> if (riscv_has_ext(env, RVH) && !riscv_cpu_virt_enabled(env)) {
> /* We support Hypervisor extensions and virtulisation is
> disabled */
> target_ulong hstatus = env->hstatus;
>
> - prev_priv = get_field(mstatus, MSTATUS_SPP);
> prev_virt = get_field(hstatus, HSTATUS_SPV);
>
> hstatus = set_field(hstatus, HSTATUS_SPV, 0);
> - mstatus = set_field(mstatus, MSTATUS_SPP, 0);
> - mstatus = set_field(mstatus, SSTATUS_SIE,
> - get_field(mstatus, SSTATUS_SPIE));
> - mstatus = set_field(mstatus, SSTATUS_SPIE, 1);
>
> - env->mstatus = mstatus;
> env->hstatus = hstatus;
>
> if (prev_virt) {
> @@ -171,14 +171,6 @@ target_ulong helper_sret(CPURISCVState *env)
> }
>
> riscv_cpu_set_virt_enabled(env, prev_virt);
> - } else {
> - prev_priv = get_field(mstatus, MSTATUS_SPP);
> -
> - mstatus = set_field(mstatus, MSTATUS_SIE,
> - get_field(mstatus, MSTATUS_SPIE));
> - mstatus = set_field(mstatus, MSTATUS_SPIE, 1);
> - mstatus = set_field(mstatus, MSTATUS_SPP, PRV_U);
> - env->mstatus = mstatus;
> }
>
> riscv_cpu_set_mode(env, prev_priv);
Re: [PATCH] target/riscv: Fix mret exception cause when no pmp rule is configured
On Mon, 2022-12-05 at 14:53 +0800, Bin Meng wrote:
> The priv spec v1.12 says:
>
> If no PMP entry matches an M-mode access, the access succeeds. If
> no PMP entry matches an S-mode or U-mode access, but at least one
> PMP entry is implemented, the access fails. Failed accesses
> generate
> an instruction, load, or store access-fault exception.
>
> At present the exception cause is set to 'illegal instruction' but
> should have been 'instruction access fault'.
>
> Fixes: d102f19a2085 ("target/riscv/pmp: Raise exception if no PMP
> entry is configured")
> Signed-off-by: Bin Meng
> ---
>
> target/riscv/op_helper.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/target/riscv/op_helper.c b/target/riscv/op_helper.c
> index 09f1f5185d..d7af7f056b 100644
> --- a/target/riscv/op_helper.c
> +++ b/target/riscv/op_helper.c
> @@ -202,7 +202,7 @@ target_ulong helper_mret(CPURISCVState *env)
>
> if (riscv_feature(env, RISCV_FEATURE_PMP) &&
> !pmp_get_num_rules(env) && (prev_priv != PRV_M)) {
> - riscv_raise_exception(env, RISCV_EXCP_ILLEGAL_INST,
> GETPC());
> + riscv_raise_exception(env, RISCV_EXCP_INST_ACCESS_FAULT,
> GETPC());
> }
>
> target_ulong prev_virt = get_field(env->mstatus, MSTATUS_MPV);
Re: [PATCH 15/15] hw/intc: sifive_plic: Fix the pending register range check
On Mon, 2022-12-05 at 16:21 +0800, Bin Meng wrote:
> On Fri, Dec 2, 2022 at 8:28 AM Wilfred Mallawa
> wrote:
> >
> > On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote:
> > > The pending register upper limit is currently set to
> > > plic->num_sources >> 3, which is wrong, e.g.: considering
> > > plic->num_sources is 7, the upper limit becomes 0 which fails
> > > the range check if reading the pending register at pending_base.
> > >
> > > Fixes: 1e24429e40df ("SiFive RISC-V PLIC Block")
> > > Signed-off-by: Bin Meng
> > >
> > > ---
> > >
> > > hw/intc/sifive_plic.c | 5 +++--
> > > 1 file changed, 3 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> > > index 7a6a358c57..a3fc8222c7 100644
> > > --- a/hw/intc/sifive_plic.c
> > > +++ b/hw/intc/sifive_plic.c
> > > @@ -143,7 +143,8 @@ static uint64_t sifive_plic_read(void
> > > *opaque,
> > > hwaddr addr, unsigned size)
> > > uint32_t irq = (addr - plic->priority_base) >> 2;
> > >
> > > return plic->source_priority[irq];
> > > - } else if (addr_between(addr, plic->pending_base, plic-
> > > > num_sources >> 3)) {
> > > + } else if (addr_between(addr, plic->pending_base,
> > > + (plic->num_sources + 31) >> 3)) {
> > why does adding specifically 31 work here?
> >
>
> Each pending register is 32-bit for 32 interrupt sources. Adding 31
> is
> to round up to next pending register offset.
>
Ah I see, thanks for that.
Regards,
Wilfred
> Regards,
> Bin
Re: [PATCH 0/3] python: testing fixes
On Fri, 2022-12-02 at 19:52 -0500, John Snow wrote: > A few tiny touchups needed for cutting edge 'flake8' tooling, a minor > type touchup in iotests, and extending the python tests to cover the > recently released Python 3.11. > > John Snow (3): > Python: fix flake8 config > iotests/check: Fix typing for sys.exit() value > python: add 3.11 to supported list > > python/setup.cfg | 6 -- > tests/qemu-iotests/check | 2 +- > 2 files changed, 5 insertions(+), 3 deletions(-) > > -- > 2.38.1 > I see you've left Westeros! xD Reviewed-by: Wilfred Mallawa > >
Re: [PATCH] docs/acpi/bits: document BITS_DEBUG environment variable
On Sat, 2022-12-03 at 13:23 +, Ani Sinha wrote: > Debug specific actions can be enabled in bios bits acpi tests by > passing > BITS_DEBUG in the environment variable while running the test. > Document that. > > CC: qemu-triv...@nongnu.org > Signed-off-by: Ani Sinha > --- > docs/devel/acpi-bits.rst | 3 +++ > 1 file changed, 3 insertions(+) Reviewed-by: Wilfred Mallawa > > diff --git a/docs/devel/acpi-bits.rst b/docs/devel/acpi-bits.rst > index 4a94c7d83d..9eb4b9e666 100644 > --- a/docs/devel/acpi-bits.rst > +++ b/docs/devel/acpi-bits.rst > @@ -52,6 +52,9 @@ Under ``tests/avocado/`` as the root we have: > for their tests. In order to enable debugging, you can set > **V=1** > environment variable. This enables verbose mode for the test and > also dumps > the entire log from bios bits and more information in case > failure happens. > + You can also set **BITS_DEBUG=1** to turn on debug mode. It will > enable > + verbose logs and also retain the temporary work directory the > test used for > + you to inspect and run the specific commands manually. > > In order to run this test, please perform the following steps > from the QEMU > build directory:
Re: [PATCH 15/15] hw/intc: sifive_plic: Fix the pending register range check
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote:
> The pending register upper limit is currently set to
> plic->num_sources >> 3, which is wrong, e.g.: considering
> plic->num_sources is 7, the upper limit becomes 0 which fails
> the range check if reading the pending register at pending_base.
>
> Fixes: 1e24429e40df ("SiFive RISC-V PLIC Block")
> Signed-off-by: Bin Meng
>
> ---
>
> hw/intc/sifive_plic.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index 7a6a358c57..a3fc8222c7 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -143,7 +143,8 @@ static uint64_t sifive_plic_read(void *opaque,
> hwaddr addr, unsigned size)
> uint32_t irq = (addr - plic->priority_base) >> 2;
>
> return plic->source_priority[irq];
> - } else if (addr_between(addr, plic->pending_base, plic-
> >num_sources >> 3)) {
> + } else if (addr_between(addr, plic->pending_base,
> + (plic->num_sources + 31) >> 3)) {
why does adding specifically 31 work here?
Wilfred,
> uint32_t word = (addr - plic->pending_base) >> 2;
>
> return plic->pending[word];
> @@ -202,7 +203,7 @@ static void sifive_plic_write(void *opaque,
> hwaddr addr, uint64_t value,
> sifive_plic_update(plic);
> }
> } else if (addr_between(addr, plic->pending_base,
> - plic->num_sources >> 3)) {
> + (plic->num_sources + 31) >> 3)) {
> qemu_log_mask(LOG_GUEST_ERROR,
> "%s: invalid pending write: 0x%" HWADDR_PRIx
> "",
> __func__, addr);
Re: [PATCH 14/15] hw/riscv: opentitan: Drop "hartid-base" and "priority-base" initialization
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote: > "hartid-base" and "priority-base" are zero by default. There is no > need to initialize them to zero again. > > Signed-off-by: Bin Meng > --- > > hw/riscv/opentitan.c | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Wilfred Mallawa > > diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c > index be7ff1eea0..da73aa51f5 100644 > --- a/hw/riscv/opentitan.c > +++ b/hw/riscv/opentitan.c > @@ -165,10 +165,8 @@ static void lowrisc_ibex_soc_realize(DeviceState > *dev_soc, Error **errp) > > /* PLIC */ > qdev_prop_set_string(DEVICE(>plic), "hart-config", "M"); > - qdev_prop_set_uint32(DEVICE(>plic), "hartid-base", 0); > qdev_prop_set_uint32(DEVICE(>plic), "num-sources", 180); > qdev_prop_set_uint32(DEVICE(>plic), "num-priorities", 3); > - qdev_prop_set_uint32(DEVICE(>plic), "priority-base", 0x00); > qdev_prop_set_uint32(DEVICE(>plic), "pending-base", 0x1000); > qdev_prop_set_uint32(DEVICE(>plic), "enable-base", 0x2000); > qdev_prop_set_uint32(DEVICE(>plic), "enable-stride", 32);
Re: [PATCH 11/15] hw/riscv: sifive_u: Avoid using magic number for "riscv, ndev"
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote: > At present magic number is used to create "riscv,ndev" property > in the dtb. Let's use the macro SIFIVE_U_PLIC_NUM_SOURCES that > is used to instantiate the PLIC model instead. > > Signed-off-by: Bin Meng > --- > > hw/riscv/sifive_u.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > Reviewed-by: Wilfred Mallawa > diff --git a/hw/riscv/sifive_u.c b/hw/riscv/sifive_u.c > index b139824aab..b40a4767e2 100644 > --- a/hw/riscv/sifive_u.c > +++ b/hw/riscv/sifive_u.c > @@ -287,7 +287,8 @@ static void create_fdt(SiFiveUState *s, const > MemMapEntry *memmap, > qemu_fdt_setprop_cells(fdt, nodename, "reg", > 0x0, memmap[SIFIVE_U_DEV_PLIC].base, > 0x0, memmap[SIFIVE_U_DEV_PLIC].size); > - qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", 0x35); > + qemu_fdt_setprop_cell(fdt, nodename, "riscv,ndev", > + SIFIVE_U_PLIC_NUM_SOURCES - 1); > qemu_fdt_setprop_cell(fdt, nodename, "phandle", plic_phandle); > plic_phandle = qemu_fdt_get_phandle(fdt, nodename); > g_free(cells);
Re: [PATCH 10/15] hw/riscv: sifive_e: Fix the number of interrupt sources of PLIC
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote:
> Per chapter 10 in Freedom E310 manuals [1][2][3], E310 G002 and G003
> supports 52 interrupt sources while G000 supports 51 interrupt
> sources.
>
> We use the value of G002 and G003, so it is 53 (including source 0).
>
> [1] G000 manual:
> https://sifive.cdn.prismic.io/sifive/4faf3e34-4a42-4c2f-be9e-c77baa4928c7_fe310-g000-manual-v3p2.pdf
>
> [2] G002 manual:
> https://sifive.cdn.prismic.io/sifive/034760b5-ac6a-4b1c-911c-f4148bb2c4a5_fe310-g002-v1p5.pdf
>
> [3] G003 manual:
> https://sifive.cdn.prismic.io/sifive/3af39c59-6498-471e-9dab-5355a0d539eb_fe310-g003-manual.pdf
>
> Fixes: eb637edb1241 ("SiFive Freedom E Series RISC-V Machine")
> Signed-off-by: Bin Meng
> ---
>
> include/hw/riscv/sifive_e.h | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
Reviewed-by: Wilfred Mallawa
> diff --git a/include/hw/riscv/sifive_e.h
> b/include/hw/riscv/sifive_e.h
> index d738745925..9e58247fd8 100644
> --- a/include/hw/riscv/sifive_e.h
> +++ b/include/hw/riscv/sifive_e.h
> @@ -82,7 +82,12 @@ enum {
> };
>
> #define SIFIVE_E_PLIC_HART_CONFIG "M"
> -#define SIFIVE_E_PLIC_NUM_SOURCES 127
> +/*
> + * Freedom E310 G002 and G003 supports 52 interrupt sources while
> + * Freedom E310 G000 supports 51 interrupt sources. We use the value
> + * of G002 and G003, so it is 53 (including interrupt source 0).
> + */
> +#define SIFIVE_E_PLIC_NUM_SOURCES 53
> #define SIFIVE_E_PLIC_NUM_PRIORITIES 7
> #define SIFIVE_E_PLIC_PRIORITY_BASE 0x04
> #define SIFIVE_E_PLIC_PENDING_BASE 0x1000
Re: [PATCH 09/15] hw/riscv: microchip_pfsoc: Fix the number of interrupt sources of PLIC
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote:
> Per chapter 6.5.2 in [1], the number of interupt sources including
> interrupt source 0 should be 187.
>
> [1] PolarFire SoC MSS TRM:
> https://ww1.microchip.com/downloads/aemDocuments/documents/FPGA/ProductDocuments/ReferenceManuals/PolarFire_SoC_FPGA_MSS_Technical_Reference_Manual_VC.pdf
>
> Fixes: 56f6e31e7b7e ("hw/riscv: Initial support for Microchip
> PolarFire SoC Icicle Kit board")
> Signed-off-by: Bin Meng
> ---
>
> include/hw/riscv/microchip_pfsoc.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Wilfred Mallawa
> diff --git a/include/hw/riscv/microchip_pfsoc.h
> b/include/hw/riscv/microchip_pfsoc.h
> index a757b240e0..9720bac2d5 100644
> --- a/include/hw/riscv/microchip_pfsoc.h
> +++ b/include/hw/riscv/microchip_pfsoc.h
> @@ -150,7 +150,7 @@ enum {
> #define MICROCHIP_PFSOC_MANAGEMENT_CPU_COUNT 1
> #define MICROCHIP_PFSOC_COMPUTE_CPU_COUNT 4
>
> -#define MICROCHIP_PFSOC_PLIC_NUM_SOURCES 185
> +#define MICROCHIP_PFSOC_PLIC_NUM_SOURCES 187
> #define MICROCHIP_PFSOC_PLIC_NUM_PRIORITIES 7
> #define MICROCHIP_PFSOC_PLIC_PRIORITY_BASE 0x04
> #define MICROCHIP_PFSOC_PLIC_PENDING_BASE 0x1000
Re: [PATCH 06/15] hw/intc: sifive_plic: Drop PLICMode_H
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote:
> H-mode has been removed since priv spec 1.10. Drop it.
>
> Signed-off-by: Bin Meng
> ---
>
> include/hw/intc/sifive_plic.h | 1 -
> hw/intc/sifive_plic.c | 1 -
> 2 files changed, 2 deletions(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/include/hw/intc/sifive_plic.h
> b/include/hw/intc/sifive_plic.h
> index 134cf39a96..d3f45ec248 100644
> --- a/include/hw/intc/sifive_plic.h
> +++ b/include/hw/intc/sifive_plic.h
> @@ -33,7 +33,6 @@ DECLARE_INSTANCE_CHECKER(SiFivePLICState,
> SIFIVE_PLIC,
> typedef enum PLICMode {
> PLICMode_U,
> PLICMode_S,
> - PLICMode_H,
> PLICMode_M
> } PLICMode;
>
> diff --git a/hw/intc/sifive_plic.c b/hw/intc/sifive_plic.c
> index 1cf156cf85..3f6ffb1d70 100644
> --- a/hw/intc/sifive_plic.c
> +++ b/hw/intc/sifive_plic.c
> @@ -42,7 +42,6 @@ static PLICMode char_to_mode(char c)
> switch (c) {
> case 'U': return PLICMode_U;
> case 'S': return PLICMode_S;
> - case 'H': return PLICMode_H;
> case 'M': return PLICMode_M;
> default:
> error_report("plic: invalid mode '%c'", c);
Re: [PATCH 05/15] hw/riscv: spike: Remove misleading comments
On Thu, 2022-12-01 at 22:08 +0800, Bin Meng wrote: > PLIC is not included in the 'spike' machine. > > Signed-off-by: Bin Meng > --- > > hw/riscv/spike.c | 1 - > 1 file changed, 1 deletion(-) > Reviewed-by: Wilfred Mallawa > diff --git a/hw/riscv/spike.c b/hw/riscv/spike.c > index 1e1d752c00..13946acf0d 100644 > --- a/hw/riscv/spike.c > +++ b/hw/riscv/spike.c > @@ -8,7 +8,6 @@ > * > * 0) HTIF Console and Poweroff > * 1) CLINT (Timer and IPI) > - * 2) PLIC (Platform Level Interrupt Controller) > * > * This program is free software; you can redistribute it and/or > modify it > * under the terms and conditions of the GNU General Public License,
Re: [PATCH 03/15] hw/riscv: Fix opentitan dependency to SIFIVE_PLIC
On Thu, 2022-12-01 at 22:07 +0800, Bin Meng wrote:
> Since commit ef6310064820 ("hw/riscv: opentitan: Update to the latest
> build")
> the IBEX PLIC model was replaced with the SiFive PLIC model in the
> 'opentitan' machine but we forgot the add the dependency there.
>
> Signed-off-by: Bin Meng
> ---
>
> hw/riscv/Kconfig | 1 +
> 1 file changed, 1 insertion(+)
>
Reviewed-by: Wilfred Mallawa
> diff --git a/hw/riscv/Kconfig b/hw/riscv/Kconfig
> index 167dc4cca6..1e4b58024f 100644
> --- a/hw/riscv/Kconfig
> +++ b/hw/riscv/Kconfig
> @@ -19,6 +19,7 @@ config MICROCHIP_PFSOC
> config OPENTITAN
> bool
> select IBEX
> + select SIFIVE_PLIC
> select UNIMP
>
> config SHAKTI_C
Re: [PATCH-for-8.0 1/3] tcg/s390x: Fix coding style
On Wed, 2022-11-30 at 14:26 +0100, Philippe Mathieu-Daudé wrote:
> We are going to modify this code, so fix its style first to avoid:
>
> ERROR: spaces required around that '*' (ctx:VxV)
> #281: FILE: tcg/s390x/tcg-target.c.inc:1224:
> + uintptr_t mask = ~(0xull << i*16);
> ^
>
> Signed-off-by: Philippe Mathieu-Daudé
> ---
> tcg/s390x/tcg-target.c.inc | 20 ++--
> 1 file changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Wilfred Mallawa
>
> diff --git a/tcg/s390x/tcg-target.c.inc b/tcg/s390x/tcg-target.c.inc
> index 33becd7694..f1d3907cd8 100644
> --- a/tcg/s390x/tcg-target.c.inc
> +++ b/tcg/s390x/tcg-target.c.inc
> @@ -802,9 +802,9 @@ static bool maybe_out_small_movi(TCGContext *s,
> TCGType type,
> }
>
> for (i = 0; i < 4; i++) {
> - tcg_target_long mask = 0xull << i*16;
> + tcg_target_long mask = 0xull << i * 16;
> if ((uval & mask) == uval) {
> - tcg_out_insn_RI(s, lli_insns[i], ret, uval >> i*16);
> + tcg_out_insn_RI(s, lli_insns[i], ret, uval >> i * 16);
> return true;
> }
> }
> @@ -1221,9 +1221,9 @@ static void tgen_andi(TCGContext *s, TCGType
> type, TCGReg dest, uint64_t val)
>
> /* Try all 32-bit insns that can perform it in one go. */
> for (i = 0; i < 4; i++) {
> - tcg_target_ulong mask = ~(0xull << i*16);
> + tcg_target_ulong mask = ~(0xull << i * 16);
> if (((val | ~valid) & mask) == mask) {
> - tcg_out_insn_RI(s, ni_insns[i], dest, val >> i*16);
> + tcg_out_insn_RI(s, ni_insns[i], dest, val >> i * 16);
> return;
> }
> }
> @@ -1231,9 +1231,9 @@ static void tgen_andi(TCGContext *s, TCGType
> type, TCGReg dest, uint64_t val)
> /* Try all 48-bit insns that can perform it in one go. */
> if (HAVE_FACILITY(EXT_IMM)) {
> for (i = 0; i < 2; i++) {
> - tcg_target_ulong mask = ~(0xull << i*32);
> + tcg_target_ulong mask = ~(0xull << i * 32);
> if (((val | ~valid) & mask) == mask) {
> - tcg_out_insn_RIL(s, nif_insns[i], dest, val >>
> i*32);
> + tcg_out_insn_RIL(s, nif_insns[i], dest, val >> i *
> 32);
> return;
> }
> }
> @@ -1279,9 +1279,9 @@ static void tgen_ori(TCGContext *s, TCGType
> type, TCGReg dest, uint64_t val)
>
> /* Try all 32-bit insns that can perform it in one go. */
> for (i = 0; i < 4; i++) {
> - tcg_target_ulong mask = (0xull << i*16);
> + tcg_target_ulong mask = (0xull << i * 16);
> if ((val & mask) != 0 && (val & ~mask) == 0) {
> - tcg_out_insn_RI(s, oi_insns[i], dest, val >> i*16);
> + tcg_out_insn_RI(s, oi_insns[i], dest, val >> i * 16);
> return;
> }
> }
> @@ -1289,9 +1289,9 @@ static void tgen_ori(TCGContext *s, TCGType
> type, TCGReg dest, uint64_t val)
> /* Try all 48-bit insns that can perform it in one go. */
> if (HAVE_FACILITY(EXT_IMM)) {
> for (i = 0; i < 2; i++) {
> - tcg_target_ulong mask = (0xull << i*32);
> + tcg_target_ulong mask = (0xull << i * 32);
> if ((val & mask) != 0 && (val & ~mask) == 0) {
> - tcg_out_insn_RIL(s, oif_insns[i], dest, val >>
> i*32);
> + tcg_out_insn_RIL(s, oif_insns[i], dest, val >> i *
> 32);
> return;
> }
> }
Re: [PATCH-for-8.0 2/2] hw: Reduce "qemu/accel.h" inclusion
On Wed, 2022-11-30 at 14:56 +0100, Philippe Mathieu-Daudé wrote: > Move "qemu/accel.h" include from the heavily included > "hw/boards.h" to hw/core/machine.c, the single file using > the AccelState definition. > > Signed-off-by: Philippe Mathieu-Daudé > --- > hw/core/machine.c | 1 + > include/hw/boards.h | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Wilfred Mallawa > diff --git a/hw/core/machine.c b/hw/core/machine.c > index 8d34caa31d..42fc6f1e84 100644 > --- a/hw/core/machine.c > +++ b/hw/core/machine.c > @@ -12,6 +12,7 @@ > > #include "qemu/osdep.h" > #include "qemu/option.h" > +#include "qemu/accel.h" > #include "qapi/qmp/qerror.h" > #include "sysemu/replay.h" > #include "qemu/units.h" > diff --git a/include/hw/boards.h b/include/hw/boards.h > index 90f1dd3aeb..f00f74c5f4 100644 > --- a/include/hw/boards.h > +++ b/include/hw/boards.h > @@ -6,7 +6,6 @@ > #include "exec/memory.h" > #include "sysemu/hostmem.h" > #include "sysemu/blockdev.h" > -#include "qemu/accel.h" > #include "qapi/qapi-types-machine.h" > #include "qemu/module.h" > #include "qom/object.h"
Re: [PATCH for-7.2] replay: Fix declaration of replay_read_next_clock
On Mon, 2022-11-28 at 17:05 -0800, Richard Henderson wrote:
> Fixes the build with gcc 13:
>
> replay/replay-time.c:34:6: error: conflicting types for \
> 'replay_read_next_clock' due to enum/integer mismatch; \
> have 'void(ReplayClockKind)' [-Werror=enum-int-mismatch]
> 34 | void replay_read_next_clock(ReplayClockKind kind)
> | ^~
> In file included from ../qemu/replay/replay-time.c:14:
> replay/replay-internal.h:139:6: note: previous declaration of \
> 'replay_read_next_clock' with type 'void(unsigned int)'
> 139 | void replay_read_next_clock(unsigned int kind);
> | ^~
>
> Fixes: 8eda206e090 ("replay: recording and replaying clock ticks")
> Signed-off-by: Richard Henderson
> ---
> replay/replay-internal.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Wilfred Mallawa
> diff --git a/replay/replay-internal.h b/replay/replay-internal.h
> index 89e377be90..b6836354ac 100644
> --- a/replay/replay-internal.h
> +++ b/replay/replay-internal.h
> @@ -136,7 +136,7 @@ bool replay_next_event_is(int event);
> /*! Reads next clock value from the file.
> If clock kind read from the file is different from the
> parameter,
> the value is not used. */
> -void replay_read_next_clock(unsigned int kind);
> +void replay_read_next_clock(ReplayClockKind kind);
>
> /* Asynchronous events queue */
>
Re: [PATCH] MAINTAINERS: Add 9p test client to section "virtio-9p"
On Mon, 2022-11-28 at 18:12 +0100, Christian Schoenebeck wrote: > The 9p test cases use a dedicated, lite-weight 9p client > implementation > (using virtio transport) under tests/qtest/libqos/ to communicate > with > QEMU's 9p server. > > It's already there for a long time. Let's officially assign it to 9p > maintainers. > > Signed-off-by: Christian Schoenebeck > --- > MAINTAINERS | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Wilfred Mallawa > diff --git a/MAINTAINERS b/MAINTAINERS > index cf24910249..4f156a99f1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -2036,6 +2036,7 @@ X: hw/9pfs/xen-9p* > F: fsdev/ > F: docs/tools/virtfs-proxy-helper.rst > F: tests/qtest/virtio-9p-test.c > +F: tests/qtest/libqos/virtio-9p* > T: git https://gitlab.com/gkurz/qemu.git 9p-next > T: git https://github.com/cschoenebeck/qemu.git 9p.next >
Re: [PATCH v6 2/9] target/riscv: add support for Zca extension
On Tue, 2022-11-29 at 09:38 +0800, weiwei wrote:
>
> On 2022/11/29 07:06, Wilfred Mallawa wrote:
>
> > On Mon, 2022-11-28 at 20:29 +0800, Weiwei Li wrote:
> >
> > > Modify the check for C extension to Zca (C implies Zca)
> > >
> > > Signed-off-by: Weiwei Li
> > > Signed-off-by: Junqiang Wang
> > > Reviewed-by: Richard Henderson
> > > Reviewed-by: Alistair Francis
> > > ---
> > > target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> > > target/riscv/translate.c | 8 ++--
> > > 2 files changed, 8 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> > > b/target/riscv/insn_trans/trans_rvi.c.inc
> > > index 4496f21266..ef7c3002b0 100644
> > > --- a/target/riscv/insn_trans/trans_rvi.c.inc
> > > +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> > > @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx,
> > > arg_jalr
> > > *a)
> > > tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
> > >
> > > gen_set_pc(ctx, cpu_pc);
> > > - if (!has_ext(ctx, RVC)) {
> > > + if (!ctx->cfg_ptr->ext_zca) {
> > > TCGv t0 = tcg_temp_new();
> > >
> > > misaligned = gen_new_label();
> > > @@ -178,7 +178,7 @@ static bool gen_branch(DisasContext *ctx,
> > > arg_b
> > > *a, TCGCond cond)
> > >
> > > gen_set_label(l); /* branch taken */
> > >
> > > - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) &
> > > 0x3))
> > > {
> > > + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm)
> > > &
> > > 0x3)) {
> > > /* misaligned */
> > > gen_exception_inst_addr_mis(ctx);
> > > } else {
> > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> > > index 2ab8772ebe..ee24b451e3 100644
> > > --- a/target/riscv/translate.c
> > > +++ b/target/riscv/translate.c
> > > @@ -557,7 +557,7 @@ static void gen_jal(DisasContext *ctx, int
> > > rd,
> > > target_ulong imm)
> > >
> > > /* check misaligned: */
> > > next_pc = ctx->base.pc_next + imm;
> > > - if (!has_ext(ctx, RVC)) {
> > > + if (!ctx->cfg_ptr->ext_zca) {
> > > if ((next_pc & 0x3) != 0) {
> > > gen_exception_inst_addr_mis(ctx);
> > > return;
> > > @@ -1097,7 +1097,11 @@ static void decode_opc(CPURISCVState *env,
> > > DisasContext *ctx, uint16_t opcode)
> > > ctx->virt_inst_excp = false;
> > > /* Check for compressed insn */
> > > if (insn_len(opcode) == 2) {
> > > - if (!has_ext(ctx, RVC)) {
> > > + /*
> > > + * Zca support all of the existing C extension,
> > > excluding
> > > all
> > > + * compressed floating point loads and stores
> > > + */
> > Look like a typo: *`supports` and *`C extensions`
> Thanks a lot!
> Yeah, it should be 'supports' here (and it's 'is' here in original
> Zc* 0.70.1 spec).
> Maybe we can use the new description from newest spec here:
> "The Zca extension is added as way to refer to instructions in the C
> extension that do not i
> nclude the floating-point loads and stores."
Yea, that sounds good!
> By the way, why do you think it should be 'C extensions' ?
Ah crap, sorry! I misread it, it looks correct.
Regards,
Wilfred
> Regards,
> Weiwei Li
>
> >
> > > + if (!ctx->cfg_ptr->ext_zca) {
> > > gen_exception_illegal(ctx);
> > > } else {
> > > ctx->opcode = opcode;
> > otherwise,
> > Reviewed-by: Wilfred Mallawa
> >
> > Wilfred
>
Re: [PATCH v6 2/9] target/riscv: add support for Zca extension
On Mon, 2022-11-28 at 20:29 +0800, Weiwei Li wrote:
> Modify the check for C extension to Zca (C implies Zca)
>
> Signed-off-by: Weiwei Li
> Signed-off-by: Junqiang Wang
> Reviewed-by: Richard Henderson
> Reviewed-by: Alistair Francis
> ---
> target/riscv/insn_trans/trans_rvi.c.inc | 4 ++--
> target/riscv/translate.c | 8 ++--
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/insn_trans/trans_rvi.c.inc
> b/target/riscv/insn_trans/trans_rvi.c.inc
> index 4496f21266..ef7c3002b0 100644
> --- a/target/riscv/insn_trans/trans_rvi.c.inc
> +++ b/target/riscv/insn_trans/trans_rvi.c.inc
> @@ -56,7 +56,7 @@ static bool trans_jalr(DisasContext *ctx, arg_jalr
> *a)
> tcg_gen_andi_tl(cpu_pc, cpu_pc, (target_ulong)-2);
>
> gen_set_pc(ctx, cpu_pc);
> - if (!has_ext(ctx, RVC)) {
> + if (!ctx->cfg_ptr->ext_zca) {
> TCGv t0 = tcg_temp_new();
>
> misaligned = gen_new_label();
> @@ -178,7 +178,7 @@ static bool gen_branch(DisasContext *ctx, arg_b
> *a, TCGCond cond)
>
> gen_set_label(l); /* branch taken */
>
> - if (!has_ext(ctx, RVC) && ((ctx->base.pc_next + a->imm) & 0x3))
> {
> + if (!ctx->cfg_ptr->ext_zca && ((ctx->base.pc_next + a->imm) &
> 0x3)) {
> /* misaligned */
> gen_exception_inst_addr_mis(ctx);
> } else {
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 2ab8772ebe..ee24b451e3 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -557,7 +557,7 @@ static void gen_jal(DisasContext *ctx, int rd,
> target_ulong imm)
>
> /* check misaligned: */
> next_pc = ctx->base.pc_next + imm;
> - if (!has_ext(ctx, RVC)) {
> + if (!ctx->cfg_ptr->ext_zca) {
> if ((next_pc & 0x3) != 0) {
> gen_exception_inst_addr_mis(ctx);
> return;
> @@ -1097,7 +1097,11 @@ static void decode_opc(CPURISCVState *env,
> DisasContext *ctx, uint16_t opcode)
> ctx->virt_inst_excp = false;
> /* Check for compressed insn */
> if (insn_len(opcode) == 2) {
> - if (!has_ext(ctx, RVC)) {
> + /*
> + * Zca support all of the existing C extension, excluding
> all
> + * compressed floating point loads and stores
> + */
Look like a typo: *`supports` and *`C extensions`
> + if (!ctx->cfg_ptr->ext_zca) {
> gen_exception_illegal(ctx);
> } else {
> ctx->opcode = opcode;
otherwise,
Reviewed-by: Wilfred Mallawa
Wilfred
[PATCH v1 1/2] hw/riscv/opentitan: bump opentitan
From: Wilfred Mallawa
This patch updates the OpenTitan model to match
the specified register layout as per [1]. Which is also the latest
commit of OpenTitan supported by TockOS.
Note: Pinmux and Padctrl has been merged into Pinmux [2][3], this patch removes
any references to Padctrl. Note: OpenTitan doc [2] has not yet specified
much detail regarding this, except for a note that states `TODO: this
section needs to be updated to reflect the pinmux/padctrl merger`
[1]
https://github.com/lowRISC/opentitan/blob/d072ac505f82152678d6e04be95c72b728a347b8/hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h
[2] https://docs.opentitan.org/hw/top_earlgrey/doc/design/
[3] https://docs.opentitan.org/hw/ip/pinmux/doc/#overview
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Reviewed-by: Bin Meng
---
hw/riscv/opentitan.c | 21 +
include/hw/riscv/opentitan.h | 9 -
2 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index be7ff1eea0..92493c629d 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -28,8 +28,16 @@
#include "qemu/units.h"
#include "sysemu/sysemu.h"
+/*
+ * This version of the OpenTitan machine currently supports
+ * OpenTitan RTL version:
+ *
+ *
+ * MMIO mapping as per (specified commit):
+ * lowRISC/opentitan: hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h
+ */
static const MemMapEntry ibex_memmap[] = {
-[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
+[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
[IBEX_DEV_RAM] ={ 0x1000, 0x2 },
[IBEX_DEV_FLASH] = { 0x2000, 0x10 },
[IBEX_DEV_UART] = { 0x4000, 0x1000 },
@@ -38,17 +46,17 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_I2C] ={ 0x4008, 0x1000 },
[IBEX_DEV_PATTGEN] ={ 0x400e, 0x1000 },
[IBEX_DEV_TIMER] = { 0x4010, 0x1000 },
-[IBEX_DEV_SENSOR_CTRL] ={ 0x4011, 0x1000 },
[IBEX_DEV_OTP_CTRL] = { 0x4013, 0x4000 },
[IBEX_DEV_LC_CTRL] ={ 0x4014, 0x1000 },
-[IBEX_DEV_USBDEV] = { 0x4015, 0x1000 },
+[IBEX_DEV_ALERT_HANDLER] = { 0x4015, 0x1000 },
[IBEX_DEV_SPI_HOST0] = { 0x4030, 0x1000 },
[IBEX_DEV_SPI_HOST1] = { 0x4031, 0x1000 },
+[IBEX_DEV_USBDEV] = { 0x4032, 0x1000 },
[IBEX_DEV_PWRMGR] = { 0x4040, 0x1000 },
[IBEX_DEV_RSTMGR] = { 0x4041, 0x1000 },
[IBEX_DEV_CLKMGR] = { 0x4042, 0x1000 },
[IBEX_DEV_PINMUX] = { 0x4046, 0x1000 },
-[IBEX_DEV_PADCTRL] ={ 0x4047, 0x1000 },
+[IBEX_DEV_SENSOR_CTRL] ={ 0x4049, 0x1000 },
[IBEX_DEV_FLASH_CTRL] = { 0x4100, 0x1000 },
[IBEX_DEV_AES] ={ 0x4110, 0x1000 },
[IBEX_DEV_HMAC] = { 0x4111, 0x1000 },
@@ -59,10 +67,9 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_ENTROPY] ={ 0x4116, 0x1000 },
[IBEX_DEV_EDNO] = { 0x4117, 0x1000 },
[IBEX_DEV_EDN1] = { 0x4118, 0x1000 },
-[IBEX_DEV_ALERT_HANDLER] = { 0x411b, 0x1000 },
[IBEX_DEV_NMI_GEN] ={ 0x411c, 0x1000 },
[IBEX_DEV_PERI] = { 0x411f, 0x1 },
-[IBEX_DEV_PLIC] = { 0x4800, 0x4005000 },
+[IBEX_DEV_PLIC] = { 0x4800, 0x4005000 },
[IBEX_DEV_FLASH_VIRTUAL] = { 0x8000, 0x8 },
};
@@ -265,8 +272,6 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
memmap[IBEX_DEV_CLKMGR].base, memmap[IBEX_DEV_CLKMGR].size);
create_unimplemented_device("riscv.lowrisc.ibex.pinmux",
memmap[IBEX_DEV_PINMUX].base, memmap[IBEX_DEV_PINMUX].size);
-create_unimplemented_device("riscv.lowrisc.ibex.padctrl",
-memmap[IBEX_DEV_PADCTRL].base, memmap[IBEX_DEV_PADCTRL].size);
create_unimplemented_device("riscv.lowrisc.ibex.usbdev",
memmap[IBEX_DEV_USBDEV].base, memmap[IBEX_DEV_USBDEV].size);
create_unimplemented_device("riscv.lowrisc.ibex.flash_ctrl",
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 6665cd5794..1fc055cdff 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -81,7 +81,6 @@ enum {
IBEX_DEV_RSTMGR,
IBEX_DEV_CLKMGR,
IBEX_DEV_PINMUX,
-IBEX_DEV_PADCTRL,
IBEX_DEV_USBDEV,
IBEX_DEV_FLASH_CTRL,
IBEX_DEV_PLIC,
@@ -109,10 +108,10 @@ enum {
IBEX_UART0_RX_TIMEOUT_IRQ = 7,
IBEX_UART0_RX_PARITY_ERR_IRQ = 8,
IBEX_TIMER_TIMEREXPIRED0_0= 127,
-IBEX_SPI_HOST0_ERR_IRQ= 151,
-IBEX_SPI_HOST0_SPI_EVENT_IRQ = 152,
-IBEX_SPI_HOST1_ERR_IRQ= 153,
-IBEX_SPI_HOST1_SP
[PATCH v1 2/2] hw/riscv/opentitan: add aon_timer base unimpl
From: Wilfred Mallawa
Adds the updated `aon_timer` base as an unimplemented device. This is
used by TockOS, patch ensures the guest doesn't hit load faults.
Signed-off-by: Wilfred Mallawa
Reviewed-by: Bin Meng
Reviewed-by: Alistair Francis
---
hw/riscv/opentitan.c | 3 +++
include/hw/riscv/opentitan.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 92493c629d..78f895d773 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -56,6 +56,7 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_RSTMGR] = { 0x4041, 0x1000 },
[IBEX_DEV_CLKMGR] = { 0x4042, 0x1000 },
[IBEX_DEV_PINMUX] = { 0x4046, 0x1000 },
+[IBEX_DEV_AON_TIMER] = { 0x4047, 0x1000 },
[IBEX_DEV_SENSOR_CTRL] ={ 0x4049, 0x1000 },
[IBEX_DEV_FLASH_CTRL] = { 0x4100, 0x1000 },
[IBEX_DEV_AES] ={ 0x4110, 0x1000 },
@@ -272,6 +273,8 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
memmap[IBEX_DEV_CLKMGR].base, memmap[IBEX_DEV_CLKMGR].size);
create_unimplemented_device("riscv.lowrisc.ibex.pinmux",
memmap[IBEX_DEV_PINMUX].base, memmap[IBEX_DEV_PINMUX].size);
+create_unimplemented_device("riscv.lowrisc.ibex.aon_timer",
+memmap[IBEX_DEV_AON_TIMER].base, memmap[IBEX_DEV_AON_TIMER].size);
create_unimplemented_device("riscv.lowrisc.ibex.usbdev",
memmap[IBEX_DEV_USBDEV].base, memmap[IBEX_DEV_USBDEV].size);
create_unimplemented_device("riscv.lowrisc.ibex.flash_ctrl",
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 1fc055cdff..7659d1bc5b 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -81,6 +81,7 @@ enum {
IBEX_DEV_RSTMGR,
IBEX_DEV_CLKMGR,
IBEX_DEV_PINMUX,
+IBEX_DEV_AON_TIMER,
IBEX_DEV_USBDEV,
IBEX_DEV_FLASH_CTRL,
IBEX_DEV_PLIC,
--
2.37.3
[PATCH v1 0/2] hw/riscv/opentitan: bump opentitan version
From: Wilfred Mallawa This patch provides updates to the OpenTitan model to bump to RTL version . A unique change here is the merger of hwip `padctrl` into `pinmux`, to reflect this change, any references to `padctrl` are removed. Additionally, an unimplemented device for `aon_timer` is added and IRQ numbers are updated. Patch was tested by running the latest master of TockOS as of this patch. Changelog V1: - Added a comment specifying what git SHA of OT we are targeting in [1/2]. Wilfred Mallawa (2): hw/riscv/opentitan: bump opentitan hw/riscv/opentitan: add aon_timer base unimpl hw/riscv/opentitan.c | 24 include/hw/riscv/opentitan.h | 10 +- 2 files changed, 21 insertions(+), 13 deletions(-) -- 2.37.3
[PATCH v0 2/2] hw/riscv/opentitan: add aon_timer base unimpl
From: Wilfred Mallawa
Adds the updated `aon_timer` base as an unimplemented device. This is
used by TockOS, patch ensures the guest doesn't hit load faults.
Signed-off-by: Wilfred Mallawa
---
hw/riscv/opentitan.c | 3 +++
include/hw/riscv/opentitan.h | 1 +
2 files changed, 4 insertions(+)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 373fed36b6..50452f792a 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -48,6 +48,7 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_RSTMGR] = { 0x4041, 0x1000 },
[IBEX_DEV_CLKMGR] = { 0x4042, 0x1000 },
[IBEX_DEV_PINMUX] = { 0x4046, 0x1000 },
+[IBEX_DEV_AON_TIMER] = { 0x4047, 0x1000 },
[IBEX_DEV_SENSOR_CTRL] ={ 0x4049, 0x1000 },
[IBEX_DEV_FLASH_CTRL] = { 0x4100, 0x1000 },
[IBEX_DEV_AES] ={ 0x4110, 0x1000 },
@@ -264,6 +265,8 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
memmap[IBEX_DEV_CLKMGR].base, memmap[IBEX_DEV_CLKMGR].size);
create_unimplemented_device("riscv.lowrisc.ibex.pinmux",
memmap[IBEX_DEV_PINMUX].base, memmap[IBEX_DEV_PINMUX].size);
+create_unimplemented_device("riscv.lowrisc.ibex.aon_timer",
+memmap[IBEX_DEV_AON_TIMER].base, memmap[IBEX_DEV_AON_TIMER].size);
create_unimplemented_device("riscv.lowrisc.ibex.usbdev",
memmap[IBEX_DEV_USBDEV].base, memmap[IBEX_DEV_USBDEV].size);
create_unimplemented_device("riscv.lowrisc.ibex.flash_ctrl",
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 1fc055cdff..7659d1bc5b 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -81,6 +81,7 @@ enum {
IBEX_DEV_RSTMGR,
IBEX_DEV_CLKMGR,
IBEX_DEV_PINMUX,
+IBEX_DEV_AON_TIMER,
IBEX_DEV_USBDEV,
IBEX_DEV_FLASH_CTRL,
IBEX_DEV_PLIC,
--
2.37.3
[PATCH v0 1/2] hw/riscv/opentitan: bump opentitan
From: Wilfred Mallawa
This patch updates the OpenTitan model to match
the specified register layout as per [1]. Which is also the latest
commit of OpenTitan supported by TockOS.
Note: Pinmux and Padctrl has been merged into Pinmux [2][3], this patch removes
any references to Padctrl. Note: OpenTitan doc [2] has not yet specified
much detail regarding this, except for a note that states `TODO: this
section needs to be updated to reflect the pinmux/padctrl merger`
[1]
https://github.com/lowRISC/opentitan/blob/d072ac505f82152678d6e04be95c72b728a347b8/hw/top_earlgrey/sw/autogen/top_earlgrey_memory.h
[2] https://docs.opentitan.org/hw/top_earlgrey/doc/design/
[3] https://docs.opentitan.org/hw/ip/pinmux/doc/#overview
Signed-off-by: Wilfred Mallawa
---
hw/riscv/opentitan.c | 13 +
include/hw/riscv/opentitan.h | 9 -
2 files changed, 9 insertions(+), 13 deletions(-)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index be7ff1eea0..373fed36b6 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -29,7 +29,7 @@
#include "sysemu/sysemu.h"
static const MemMapEntry ibex_memmap[] = {
-[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
+[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
[IBEX_DEV_RAM] ={ 0x1000, 0x2 },
[IBEX_DEV_FLASH] = { 0x2000, 0x10 },
[IBEX_DEV_UART] = { 0x4000, 0x1000 },
@@ -38,17 +38,17 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_I2C] ={ 0x4008, 0x1000 },
[IBEX_DEV_PATTGEN] ={ 0x400e, 0x1000 },
[IBEX_DEV_TIMER] = { 0x4010, 0x1000 },
-[IBEX_DEV_SENSOR_CTRL] ={ 0x4011, 0x1000 },
[IBEX_DEV_OTP_CTRL] = { 0x4013, 0x4000 },
[IBEX_DEV_LC_CTRL] ={ 0x4014, 0x1000 },
-[IBEX_DEV_USBDEV] = { 0x4015, 0x1000 },
+[IBEX_DEV_ALERT_HANDLER] = { 0x4015, 0x1000 },
[IBEX_DEV_SPI_HOST0] = { 0x4030, 0x1000 },
[IBEX_DEV_SPI_HOST1] = { 0x4031, 0x1000 },
+[IBEX_DEV_USBDEV] = { 0x4032, 0x1000 },
[IBEX_DEV_PWRMGR] = { 0x4040, 0x1000 },
[IBEX_DEV_RSTMGR] = { 0x4041, 0x1000 },
[IBEX_DEV_CLKMGR] = { 0x4042, 0x1000 },
[IBEX_DEV_PINMUX] = { 0x4046, 0x1000 },
-[IBEX_DEV_PADCTRL] ={ 0x4047, 0x1000 },
+[IBEX_DEV_SENSOR_CTRL] ={ 0x4049, 0x1000 },
[IBEX_DEV_FLASH_CTRL] = { 0x4100, 0x1000 },
[IBEX_DEV_AES] ={ 0x4110, 0x1000 },
[IBEX_DEV_HMAC] = { 0x4111, 0x1000 },
@@ -59,10 +59,9 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_ENTROPY] ={ 0x4116, 0x1000 },
[IBEX_DEV_EDNO] = { 0x4117, 0x1000 },
[IBEX_DEV_EDN1] = { 0x4118, 0x1000 },
-[IBEX_DEV_ALERT_HANDLER] = { 0x411b, 0x1000 },
[IBEX_DEV_NMI_GEN] ={ 0x411c, 0x1000 },
[IBEX_DEV_PERI] = { 0x411f, 0x1 },
-[IBEX_DEV_PLIC] = { 0x4800, 0x4005000 },
+[IBEX_DEV_PLIC] = { 0x4800, 0x4005000 },
[IBEX_DEV_FLASH_VIRTUAL] = { 0x8000, 0x8 },
};
@@ -265,8 +264,6 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
memmap[IBEX_DEV_CLKMGR].base, memmap[IBEX_DEV_CLKMGR].size);
create_unimplemented_device("riscv.lowrisc.ibex.pinmux",
memmap[IBEX_DEV_PINMUX].base, memmap[IBEX_DEV_PINMUX].size);
-create_unimplemented_device("riscv.lowrisc.ibex.padctrl",
-memmap[IBEX_DEV_PADCTRL].base, memmap[IBEX_DEV_PADCTRL].size);
create_unimplemented_device("riscv.lowrisc.ibex.usbdev",
memmap[IBEX_DEV_USBDEV].base, memmap[IBEX_DEV_USBDEV].size);
create_unimplemented_device("riscv.lowrisc.ibex.flash_ctrl",
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 6665cd5794..1fc055cdff 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -81,7 +81,6 @@ enum {
IBEX_DEV_RSTMGR,
IBEX_DEV_CLKMGR,
IBEX_DEV_PINMUX,
-IBEX_DEV_PADCTRL,
IBEX_DEV_USBDEV,
IBEX_DEV_FLASH_CTRL,
IBEX_DEV_PLIC,
@@ -109,10 +108,10 @@ enum {
IBEX_UART0_RX_TIMEOUT_IRQ = 7,
IBEX_UART0_RX_PARITY_ERR_IRQ = 8,
IBEX_TIMER_TIMEREXPIRED0_0= 127,
-IBEX_SPI_HOST0_ERR_IRQ= 151,
-IBEX_SPI_HOST0_SPI_EVENT_IRQ = 152,
-IBEX_SPI_HOST1_ERR_IRQ= 153,
-IBEX_SPI_HOST1_SPI_EVENT_IRQ = 154,
+IBEX_SPI_HOST0_ERR_IRQ= 134,
+IBEX_SPI_HOST0_SPI_EVENT_IRQ = 135,
+IBEX_SPI_HOST1_ERR_IRQ= 136,
+IBEX_SPI_HOST1_SPI_EVENT_IRQ = 137,
};
#endif
--
2.37.3
[PATCH v0 0/2] hw/riscv/opentitan: bump opentitan version
From: Wilfred Mallawa This patch provides updates to the OpenTitan model to bump to RTL version . A unique change here is the merger of hwip `padctrl` into `pinmux`, to reflect this change, any references to `padctrl` are removed. Additionally, an unimplemented device for `aon_timer` is added and IRQ numbers are updated. Patch was tested by running the latest master of TockOS as of this patch. Wilfred Mallawa (2): hw/riscv/opentitan: bump opentitan hw/riscv/opentitan: add aon_timer base unimpl hw/riscv/opentitan.c | 16 include/hw/riscv/opentitan.h | 10 +- 2 files changed, 13 insertions(+), 13 deletions(-) -- 2.37.3
[PATCH v3 1/2] hw/registerfields: add `FIELDx_1CLEAR()` macro
From: Wilfred Mallawa
Adds a helper macro that implements the register `w1c`
functionality.
Ex:
uint32_t data = FIELD32_1CLEAR(val, REG, FIELD);
If ANY bits of the specified `FIELD` is set
then the respective field is cleared and returned to `data`.
If the field is cleared (0), then no change and
val is returned.
Signed-off-by: Wilfred Mallawa
---
include/hw/registerfields.h | 22 ++
1 file changed, 22 insertions(+)
diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index 1330ca77de..0b8404c2f7 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -115,6 +115,28 @@
R_ ## reg ## _ ## field ## _LENGTH, _v.v); \
_d; })
+/*
+ * Clear the specified field in storage if
+ * any field bits are set, else no changes made. Implements
+ * single/multi-bit `w1c`
+ *
+ */
+#define FIELD8_1CLEAR(storage, reg, field)\
+(FIELD_EX8(storage, reg, field) ? \
+FIELD_DP8(storage, reg, field, 0x00) : storage)
+
+#define FIELD16_1CLEAR(storage, reg, field) \
+(FIELD_EX16(storage, reg, field) ?\
+FIELD_DP16(storage, reg, field, 0x00) : storage)
+
+#define FIELD32_1CLEAR(storage, reg, field) \
+(FIELD_EX32(storage, reg, field) ?\
+FIELD_DP32(storage, reg, field, 0x00) : storage)
+
+#define FIELD64_1CLEAR(storage, reg, field) \
+(FIELD_EX64(storage, reg, field) ?\
+FIELD_DP64(storage, reg, field, 0x00) : storage)
+
#define FIELD_SDP8(storage, reg, field, val) ({ \
struct { \
signed int v:R_ ## reg ## _ ## field ## _LENGTH; \
--
2.37.3
[PATCH v3 2/2] hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro
From: Wilfred Mallawa
use the `FIELD32_1CLEAR` macro to implement register
`rw1c` functionality to `ibex_spi`.
This change was tested by running the `SPI_HOST` from TockOS.
Signed-off-by: Wilfred Mallawa
---
hw/ssi/ibex_spi_host.c | 21 +
1 file changed, 9 insertions(+), 12 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 57df462e3c..0a456cd1ed 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -342,7 +342,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
{
IbexSPIHostState *s = opaque;
uint32_t val32 = val64;
-uint32_t shift_mask = 0xff, status = 0, data = 0;
+uint32_t shift_mask = 0xff, status = 0;
uint8_t txqd_len;
trace_ibex_spi_host_write(addr, size, val64);
@@ -355,12 +355,11 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
case IBEX_SPI_HOST_INTR_STATE:
/* rw1c status register */
if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
-data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], INTR_STATE, ERROR);
}
if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
-data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], INTR_STATE,
SPI_EVENT);
}
-s->regs[addr] = data;
break;
case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
@@ -505,27 +504,25 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
-status = s->regs[addr];
/* rw1c status register */
if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
-status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
CMDBUSY);
}
if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
-status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
OVERFLOW);
}
if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
-status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
UNDERFLOW);
}
if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
-status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
CMDINVAL);
}
if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
-status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
CSIDINVAL);
}
if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
-status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
+s->regs[addr] = FIELD32_1CLEAR(s->regs[addr], ERROR_STATUS,
ACCESSINVAL);
}
-s->regs[addr] = status;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
--
2.37.3
[PATCH v3 0/2] implement `FIELDx_1CLEAR() macro
From: Wilfred Mallawa This patch series implements a `FIELDx_1CLEAR()` macro and implements it in the `hw/ssi/ibex_spi.c` model. *** Changelog *** Since v2: - change the macro arguments name to match the existing macros. (reg_val, reg, field) -> (storage, reg, field) - Add the use of this macro to `ibex_spi` Since v1: - Instead of needing all field bits to be set we clear the field if any are set. If the field is 0/clear then no change. Wilfred Mallawa (2): hw/registerfields: add `FIELDx_1CLEAR()` macro hw/ssi/ibex_spi: implement `FIELD32_1CLEAR` macro hw/ssi/ibex_spi_host.c | 21 + include/hw/registerfields.h | 22 ++ 2 files changed, 31 insertions(+), 12 deletions(-) -- 2.37.3
Re: [RFC v2] hw/registerfields: add `FIELDx_1CLEAR()` macro
On Mon, 2022-10-10 at 11:29 +1000, Alistair Francis wrote: > On Tue, Sep 27, 2022 at 10:58 AM Wilfred Mallawa > wrote: > > > > From: Wilfred Mallawa > > > > Changes from V1: > > * Instead of needing all field bits to be set > > we clear the field if any are set. If the field is > > 0/clear then no change. > > The changelog should go > > > > > Adds a helper macro that implements the register `w1c` > > functionality. > > > > Ex: > > uint32_t data = FIELD32_1CLEAR(val, REG, FIELD); > > > > If ANY bits of the specified `FIELD` is set > > then the respective field is cleared and returned to `data`. > > > > If the field is cleared (0), then no change and > > val is returned. > > > > Signed-off-by: Wilfred Mallawa > > --- > > Below this line. > > Otherwise the patch looks good. Do you mind adding it to a series > that > converts the OT SPI to use these macros? > Yep, that sounds good. I'll send a new series with this + SPI changes with the macro implemented. > > include/hw/registerfields.h | 22 ++ > > 1 file changed, 22 insertions(+) > > > > diff --git a/include/hw/registerfields.h > > b/include/hw/registerfields.h > > index 1330ca77de..4a6a228339 100644 > > --- a/include/hw/registerfields.h > > +++ b/include/hw/registerfields.h > > @@ -115,6 +115,28 @@ > > R_ ## reg ## _ ## field ## _LENGTH, > > _v.v); \ > > _d; }) > > > > +/* > > + * Clear the specified field in reg_val if > > + * any field bits are set, else no changes made. Implements > > + * single/multi-bit `w1c` > > + * > > + */ > > +#define FIELD8_1CLEAR(reg_val, reg, > > field) \ > > These should probably match the other macros with: > > (storage, reg, field) > Will do! > Alistair Wilferd
[PATCH v5 2/2] hw/ssi: ibex_spi: fixup/add rw1c functionality
From: Wilfred Mallawa
This patch adds the `rw1c` functionality to the respective
registers. The status fields are cleared when the respective
field is set.
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
---
hw/ssi/ibex_spi_host.c | 36 +++---
include/hw/ssi/ibex_spi_host.h | 4 ++--
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d145a4cdbd..57df462e3c 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -342,7 +342,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
{
IbexSPIHostState *s = opaque;
uint32_t val32 = val64;
-uint32_t shift_mask = 0xff, status = 0;
+uint32_t shift_mask = 0xff, status = 0, data = 0;
uint8_t txqd_len;
trace_ibex_spi_host_write(addr, size, val64);
@@ -352,7 +352,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
switch (addr) {
/* Skipping any R/O registers */
-case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE:
+case IBEX_SPI_HOST_INTR_STATE:
+/* rw1c status register */
+if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
+data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+}
+if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
+data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+}
+s->regs[addr] = data;
+break;
+case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
break;
case IBEX_SPI_HOST_INTR_TEST:
@@ -495,7 +505,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
-s->regs[addr] = val32;
+status = s->regs[addr];
+/* rw1c status register */
+if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
+}
+s->regs[addr] = status;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
diff --git a/include/hw/ssi/ibex_spi_host.h b/include/hw/ssi/ibex_spi_host.h
index 3fedcb6805..1f6d077766 100644
--- a/include/hw/ssi/ibex_spi_host.h
+++ b/include/hw/ssi/ibex_spi_host.h
@@ -40,7 +40,7 @@
OBJECT_CHECK(IbexSPIHostState, (obj), TYPE_IBEX_SPI_HOST)
/* SPI Registers */
-#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw */
+#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw1c */
#define IBEX_SPI_HOST_INTR_ENABLE(0x04 / 4) /* rw */
#define IBEX_SPI_HOST_INTR_TEST (0x08 / 4) /* wo */
#define IBEX_SPI_HOST_ALERT_TEST (0x0c / 4) /* wo */
@@ -54,7 +54,7 @@
#define IBEX_SPI_HOST_TXDATA (0x28 / 4)
#define IBEX_SPI_HOST_ERROR_ENABLE (0x2c / 4) /* rw */
-#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw */
+#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw1c */
#define IBEX_SPI_HOST_EVENT_ENABLE (0x34 / 4) /* rw */
/* FIFO Len in Bytes */
--
2.37.3
[PATCH v5 1/2] hw/ssi: ibex_spi: fixup coverity issue
From: Wilfred Mallawa
This patch addresses the coverity issues specified in [1],
as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
implemented to clean up the code.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
Fixes: Coverity CID 1488107
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Reviewed-by: Andrew Jones
---
hw/ssi/ibex_spi_host.c | 132 +
1 file changed, 68 insertions(+), 64 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 94d7da9cc2..d145a4cdbd 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -108,18 +108,22 @@ static inline uint8_t div4_round_up(uint8_t dividend)
static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the RX FIFO and assert RXEMPTY */
fifo8_reset(>rx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, RXFULL, 0);
+data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the TX FIFO and assert TXEMPTY */
fifo8_reset(>tx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, TXFULL, 0);
+data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +166,38 @@ static void ibex_spi_host_reset(DeviceState *dev)
*/
static void ibex_spi_host_irq(IbexSPIHostState *s)
{
-bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_ERROR_MASK;
-bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_SPI_EVENT_MASK;
-bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_ERROR_MASK;
-bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_SPI_EVENT_MASK;
+uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST];
+uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
+uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE];
+
+uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
+uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE];
+uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
+uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS];
+
+
+bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR);
+bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT);
+bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR);
+bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT);
+
int err_irq = 0, event_irq = 0;
/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
+if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) {
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDBUSY_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDBUSY_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CMDBUSY)) {
/* Wrote to COMMAND when not READY */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDINVAL) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CMDINVAL)) {
/* Invalid command segment */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CSIDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CSIDINVAL) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CSIDINVAL)) {
/* Invalid value for CSID */
err_irq = 1;
}
@@ -204,22 +209,19 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
/* Event IRQ Enabled and Event IR
[PATCH v5 0/2] hw/ssi/ibex_spi: bug fixes
From: Wilfred Mallawa The remaining patches in this series address: - Coverity issues for `ibex_spi` - Adds rw1c functionality Changes since V4: - Fixup compiler warning for unused variable `data` in [1/2] Wilfred Mallawa (2): hw/ssi: ibex_spi: fixup coverity issue hw/ssi: ibex_spi: fixup/add rw1c functionality hw/ssi/ibex_spi_host.c | 166 - include/hw/ssi/ibex_spi_host.h | 4 +- 2 files changed, 102 insertions(+), 68 deletions(-) -- 2.37.3
[PATCH v1 2/2] riscv/opentitan: connect lifecycle controller
From: Wilfred Mallawa
Connects the ibex lifecycle controller with opentitan,
with this change, we can now get past the lifecycle checks
in the boot rom.
Signed-off-by: Wilfred Mallawa
---
hw/riscv/opentitan.c | 10 --
include/hw/riscv/opentitan.h | 2 ++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index be7ff1eea0..73a5cef694 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -122,6 +122,8 @@ static void lowrisc_ibex_soc_init(Object *obj)
object_initialize_child(obj, "timer", >timer, TYPE_IBEX_TIMER);
+object_initialize_child(obj, "lifetime_ctrl", >lc, TYPE_IBEX_LC_CTRL);
+
for (int i = 0; i < OPENTITAN_NUM_SPI_HOSTS; i++) {
object_initialize_child(obj, "spi_host[*]", >spi_host[i],
TYPE_IBEX_SPI_HOST);
@@ -243,6 +245,12 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
}
}
+/* Life-Cycle Control */
+if (!sysbus_realize(SYS_BUS_DEVICE(>lc), errp)) {
+return;
+}
+sysbus_mmio_map(SYS_BUS_DEVICE(>lc), 0, memmap[IBEX_DEV_LC_CTRL].base);
+
create_unimplemented_device("riscv.lowrisc.ibex.gpio",
memmap[IBEX_DEV_GPIO].base, memmap[IBEX_DEV_GPIO].size);
create_unimplemented_device("riscv.lowrisc.ibex.spi_device",
@@ -255,8 +263,6 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
memmap[IBEX_DEV_SENSOR_CTRL].base, memmap[IBEX_DEV_SENSOR_CTRL].size);
create_unimplemented_device("riscv.lowrisc.ibex.otp_ctrl",
memmap[IBEX_DEV_OTP_CTRL].base, memmap[IBEX_DEV_OTP_CTRL].size);
-create_unimplemented_device("riscv.lowrisc.ibex.lc_ctrl",
-memmap[IBEX_DEV_LC_CTRL].base, memmap[IBEX_DEV_LC_CTRL].size);
create_unimplemented_device("riscv.lowrisc.ibex.pwrmgr",
memmap[IBEX_DEV_PWRMGR].base, memmap[IBEX_DEV_PWRMGR].size);
create_unimplemented_device("riscv.lowrisc.ibex.rstmgr",
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 6665cd5794..64b7f21339 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -24,6 +24,7 @@
#include "hw/char/ibex_uart.h"
#include "hw/timer/ibex_timer.h"
#include "hw/ssi/ibex_spi_host.h"
+#include "hw/misc/ibex_lc_ctrl.h"
#include "qom/object.h"
#define TYPE_RISCV_IBEX_SOC "riscv.lowrisc.ibex.soc"
@@ -44,6 +45,7 @@ struct LowRISCIbexSoCState {
SiFivePLICState plic;
IbexUartState uart;
IbexTimerState timer;
+IbexLCState lc;
IbexSPIHostState spi_host[OPENTITAN_NUM_SPI_HOSTS];
uint32_t resetvec;
--
2.37.3
[PATCH v1 1/2] hw/misc: add ibex lifecycle controller
From: Wilfred Mallawa Device model for the OpenTitan lifecycle controller as per [1]. Addition of this model is the first of many steps to adding `boot_rom` support for OpenTitan. The OpenTitan `boot_rom` needs to access the lifecycle controller during the init/test sequence before it jumps to flash. With this model, we can get past the lifecycle controller stages in boot. Currently the supported functionality is limited. [1] https://docs.opentitan.org/hw/ip/lc_ctrl/doc/ Signed-off-by: Wilfred Mallawa --- hw/misc/ibex_lc_ctrl.c | 287 + hw/misc/meson.build| 3 + hw/misc/trace-events | 5 + include/hw/misc/ibex_lc_ctrl.h | 121 ++ 4 files changed, 416 insertions(+) create mode 100644 hw/misc/ibex_lc_ctrl.c create mode 100644 include/hw/misc/ibex_lc_ctrl.h diff --git a/hw/misc/ibex_lc_ctrl.c b/hw/misc/ibex_lc_ctrl.c new file mode 100644 index 00..f034a92a9c --- /dev/null +++ b/hw/misc/ibex_lc_ctrl.c @@ -0,0 +1,287 @@ +/* + * QEMU model of the Ibex Life Cycle Controller + * SPEC Reference: https://docs.opentitan.org/hw/ip/lc_ctrl/doc/ + * + * Copyright (C) 2022 Western Digital + * + * 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. + */ + +#include "qemu/osdep.h" +#include "qemu/log.h" +#include "qemu/module.h" +#include "hw/misc/ibex_lc_ctrl.h" +#include "hw/irq.h" +#include "hw/qdev-properties.h" +#include "hw/qdev-properties-system.h" +#include "migration/vmstate.h" +#include "trace.h" + +REG32(ALERT_TEST, 0x00) +FIELD(ALERT_TEST, FETAL_PROG_ERR, 0, 1) +FIELD(ALERT_TEST, FETAL_STATE_ERR, 1, 1) +FIELD(ALERT_TEST, FETAL_BUS_INTEG_ERR, 2, 1) +REG32(CTRL_STATUS, 0x04) +FIELD(CTRL_STATUS, READY, 0, 1) +FIELD(CTRL_STATUS, TRANSITION_SUCCESSFUL, 0, 1) +FIELD(CTRL_STATUS, TRANSITION_COUNT_ERROR, 1, 1) +FIELD(CTRL_STATUS, TRANSITION_ERROR, 2, 1) +FIELD(CTRL_STATUS, TOKEN_ERROR, 3, 1) +FIELD(CTRL_STATUS, FLASH_RMA_ERROR, 4, 1) +FIELD(CTRL_STATUS, OTP_ERROR, 5, 1) +FIELD(CTRL_STATUS, STATE_ERROR, 6, 1) +FIELD(CTRL_STATUS, BUS_INTEG_ERROR, 7, 1) +FIELD(CTRL_STATUS, OTP_PARTITION_ERROR, 8, 1) +REG32(CLAIM_TRANSITION_IF, 0x08) + FIELD(CLAIM_TRANSITION_IF, MUTEX, 0, 8) +REG32(TRANSITION_REGWEN , 0x0C) + FIELD(TRANSITION_REGWEN , TRANSITION_REGWEN, 0, 1) +REG32(TRANSITION_CMD , 0x10) + FIELD(TRANSITION_CMD , START, 0, 1) +REG32(TRANSITION_CTRL , 0x14) + FIELD(TRANSITION_CTRL , EXT_CLOCK_EN, 0, 1) +REG32(TRANSITION_TOKEN_0 , 0x18) + FIELD(TRANSITION_TOKEN_0 , TRANSITION_TOKEN_0, 0, 32) +REG32(TRANSITION_TOKEN_1 , 0x1C) + FIELD(TRANSITION_TOKEN_1 , TRANSITION_TOKEN_1, 0, 32) +REG32(TRANSITION_TOKEN_2 , 0x20) + FIELD(TRANSITION_TOKEN_2 , TRANSITION_TOKEN_2, 0, 32) +REG32(TRANSITION_TOKEN_3 , 0x24) + FIELD(TRANSITION_TOKEN_3 , TRANSITION_TOKEN_3, 0, 32) +REG32(TRANSITION_TARGET , 0x28) + FIELD(TRANSITION_TARGET , STATE, 0, 30) +REG32(OTP_VENDOR_TEST_CTRL , 0x2C) + FIELD(OTP_VENDOR_TEST_CTRL , OTP_VENDOR_TEST_CTRL, 0, 32) +REG32(OTP_VENDOR_TEST_STATUS , 0x30) + FIELD(OTP_VENDOR_TEST_STATUS , OTP_VENDOR_TEST_STATUS, 0, 32) +REG32(LC_STATE , 0x34) + FIELD(LC_STATE , STATE, 0, 30) +REG32(LC_TRANSITION_CNT , 0x38) + FIELD(LC_TRANSITION_CNT , CNT, 0, 5) +REG32(LC_ID_STATE , 0x3C) + FIELD(LC_ID_STATE , STATE, 0, 32) +REG32(HW_REV , 0x40) + FIELD(HW_REV , CHIP_REV, 0, 16) + FIELD(HW_REV , CHIP_GEN, 16, 16) +REG32(DEVICE_ID_0 , 0x44) + FIELD(DEVICE_ID_0 , DEVICE_ID_0, 0, 32) +REG32(DEVICE_ID_1 , 0x48) + FIELD(DEVICE_ID_1 , DEVICE_ID_2, 0, 32) +REG32(DEVICE_ID_2 , 0x4C) + FIELD(DEVICE_ID_2 , DEVICE_ID_2, 0, 32) +REG32(DEVICE_ID_3 , 0x50) + FIELD(DEVICE_ID_3 , DEVICE_ID_3, 0, 32) +REG32(DEVICE_ID_4 , 0x54) + FIELD(DEVICE_ID_4 , DEVICE_ID_4, 0, 32) +REG32(DEVICE_ID_5 , 0x58) +
[PATCH v1 0/2] Add OpenTitan lifecycle controller
From: Wilfred Mallawa This series of patches: - Add OpenTitan lifecycle controller with basic functionality - Connects it to OpenTitan Currently in OpenTitan, we skip the `boot_rom` since is has become more complex and we do not have all the support in QEMU to use it. One of the missing pieces to getting the `boot_rom` working is the lifecycle controller. There's a particular `magic_value` that is kept in the `LC_STATE` register which is checked by the `boot_rom`. With this basic implementation of the device, we can get past the lifecycle controller in the `boot_rom` check stage. Testing was done using TockOS (running QEMU with `-d in_asm`) to see how far in the boot rom we get. End goal is to add support to all device models in QEMU that are required by the OpenTitan `boot_rom` such that we can use it as the `bios`. Wilfred Mallawa (2): hw/misc: add ibex lifecycle controller riscv/opentitan: connect lifecycle controller hw/misc/ibex_lc_ctrl.c | 287 + hw/misc/meson.build| 3 + hw/misc/trace-events | 5 + hw/riscv/opentitan.c | 10 +- include/hw/misc/ibex_lc_ctrl.h | 121 ++ include/hw/riscv/opentitan.h | 2 + 6 files changed, 426 insertions(+), 2 deletions(-) create mode 100644 hw/misc/ibex_lc_ctrl.c create mode 100644 include/hw/misc/ibex_lc_ctrl.h -- 2.37.3
[RFC v2] hw/registerfields: add `FIELDx_1CLEAR()` macro
From: Wilfred Mallawa
Changes from V1:
* Instead of needing all field bits to be set
we clear the field if any are set. If the field is
0/clear then no change.
Adds a helper macro that implements the register `w1c`
functionality.
Ex:
uint32_t data = FIELD32_1CLEAR(val, REG, FIELD);
If ANY bits of the specified `FIELD` is set
then the respective field is cleared and returned to `data`.
If the field is cleared (0), then no change and
val is returned.
Signed-off-by: Wilfred Mallawa
---
include/hw/registerfields.h | 22 ++
1 file changed, 22 insertions(+)
diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index 1330ca77de..4a6a228339 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -115,6 +115,28 @@
R_ ## reg ## _ ## field ## _LENGTH, _v.v); \
_d; })
+/*
+ * Clear the specified field in reg_val if
+ * any field bits are set, else no changes made. Implements
+ * single/multi-bit `w1c`
+ *
+ */
+#define FIELD8_1CLEAR(reg_val, reg, field)\
+(FIELD_EX8(reg_val, reg, field) ? \
+FIELD_DP8(reg_val, reg, field, 0x00) : reg_val)
+
+#define FIELD16_1CLEAR(reg_val, reg, field) \
+(FIELD_EX16(reg_val, reg, field) ?\
+FIELD_DP16(reg_val, reg, field, 0x00) : reg_val)
+
+#define FIELD32_1CLEAR(reg_val, reg, field) \
+(FIELD_EX32(reg_val, reg, field) ?\
+FIELD_DP32(reg_val, reg, field, 0x00) : reg_val)
+
+#define FIELD64_1CLEAR(reg_val, reg, field) \
+(FIELD_EX64(reg_val, reg, field) ?\
+FIELD_DP64(reg_val, reg, field, 0x00) : reg_val)
+
#define FIELD_SDP8(storage, reg, field, val) ({ \
struct { \
signed int v:R_ ## reg ## _ ## field ## _LENGTH; \
--
2.37.3
Re: [PATCH 1/3] target/riscv: Set the CPU resetvec directly
On Wed, 2022-09-14 at 12:11 +0200, Alistair Francis via wrote:
> Instead of using our properties to set a config value which then
> might
> be used to set the resetvec (depending on your timing), let's instead
> just set the resetvec directly in the env struct.
>
> This allows us to set the reset vec from the command line with:
> -global
> driver=riscv.hart_array,property=resetvec,value=0x2400
>
> Signed-off-by: Alistair Francis
> ---
> target/riscv/cpu.h | 3 +--
> target/riscv/cpu.c | 13 +++--
> target/riscv/machine.c | 6 +++---
> 3 files changed, 7 insertions(+), 15 deletions(-)
>
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 06751e1e3e..22344a620b 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -190,7 +190,7 @@ struct CPUArchState {
> /* This contains QEMU specific information about the virt state.
> */
> target_ulong virt;
> target_ulong geilen;
> - target_ulong resetvec;
> + uint64_t resetvec;
>
> target_ulong mhartid;
> /*
> @@ -474,7 +474,6 @@ struct RISCVCPUConfig {
> bool pmp;
> bool epmp;
> bool debug;
> - uint64_t resetvec;
>
> bool short_isa_string;
> };
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index aee14a239a..b29c88b9f0 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -228,13 +228,6 @@ static void set_vext_version(CPURISCVState *env,
> int vext_ver)
> env->vext_ver = vext_ver;
> }
>
> -static void set_resetvec(CPURISCVState *env, target_ulong resetvec)
> -{
> -#ifndef CONFIG_USER_ONLY
> - env->resetvec = resetvec;
> -#endif
> -}
> -
> static void riscv_any_cpu_init(Object *obj)
> {
> CPURISCVState *env = _CPU(obj)->env;
> @@ -336,7 +329,6 @@ static void rv32_imafcu_nommu_cpu_init(Object
> *obj)
>
> set_misa(env, MXL_RV32, RVI | RVM | RVA | RVF | RVC | RVU);
> set_priv_version(env, PRIV_VERSION_1_10_0);
> - set_resetvec(env, DEFAULT_RSTVEC);
> cpu->cfg.mmu = false;
> }
> #endif
> @@ -676,7 +668,6 @@ static void riscv_cpu_realize(DeviceState *dev,
> Error **errp)
> riscv_set_feature(env, RISCV_FEATURE_DEBUG);
> }
>
> - set_resetvec(env, cpu->cfg.resetvec);
>
> #ifndef CONFIG_USER_ONLY
> if (cpu->cfg.ext_sstc) {
> @@ -1079,7 +1070,9 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_UINT64("marchid", RISCVCPU, cfg.marchid,
> RISCV_CPU_MARCHID),
> DEFINE_PROP_UINT64("mimpid", RISCVCPU, cfg.mimpid,
> RISCV_CPU_MIMPID),
>
> - DEFINE_PROP_UINT64("resetvec", RISCVCPU, cfg.resetvec,
> DEFAULT_RSTVEC),
> +#ifndef CONFIG_USER_ONLY
> + DEFINE_PROP_UINT64("resetvec", RISCVCPU, env.resetvec,
> DEFAULT_RSTVEC),
> +#endif
>
> DEFINE_PROP_BOOL("short-isa-string", RISCVCPU,
> cfg.short_isa_string, false),
>
> diff --git a/target/riscv/machine.c b/target/riscv/machine.c
> index 41098f6ad0..c4e6b3bba4 100644
> --- a/target/riscv/machine.c
> +++ b/target/riscv/machine.c
> @@ -308,8 +308,8 @@ static const VMStateDescription
> vmstate_pmu_ctr_state = {
>
> const VMStateDescription vmstate_riscv_cpu = {
> .name = "cpu",
> - .version_id = 4,
> - .minimum_version_id = 4,
> + .version_id = 5,
> + .minimum_version_id = 5,
> .post_load = riscv_cpu_post_load,
> .fields = (VMStateField[]) {
> VMSTATE_UINTTL_ARRAY(env.gpr, RISCVCPU, 32),
> @@ -331,7 +331,7 @@ const VMStateDescription vmstate_riscv_cpu = {
> VMSTATE_UINT32(env.features, RISCVCPU),
> VMSTATE_UINTTL(env.priv, RISCVCPU),
> VMSTATE_UINTTL(env.virt, RISCVCPU),
> - VMSTATE_UINTTL(env.resetvec, RISCVCPU),
> + VMSTATE_UINT64(env.resetvec, RISCVCPU),
> VMSTATE_UINTTL(env.mhartid, RISCVCPU),
> VMSTATE_UINT64(env.mstatus, RISCVCPU),
> VMSTATE_UINT64(env.mip, RISCVCPU),
Reviewed by: Wilfred Mallawa
Re: [RFC] hw/registerfields: add `FIELDx_1CLEAR()` macro
Ping!
https://lore.kernel.org/qemu-devel/20220901010220.495112-1-wilfred.mall...@opensource.wdc.com/
Wilfred
On Fri, 2022-09-02 at 01:18 +0200, Philippe Mathieu-Daudé wrote:
> On 1/9/22 07:32, Richard Henderson wrote:
> > On 9/1/22 02:02, Wilfred Mallawa wrote:
> > > From: Wilfred Mallawa
> > >
> > > Adds a helper macro that implements the `rw1c`
> > > behaviour.
> > >
> > > Ex:
> > > uint32_t data = FIELD32_1CLEAR(val, REG, FIELD);
> > >
> > > if the specified `FIELD` is set (single/multi bit all fields)
> > > then the respective field is cleared and returned to `data`.
> > >
> > > If ALL bits of the bitfield are not set, then no change and
> > > val is returned.
> > >
> > > Signed-off-by: Wilfred Mallawa
> >
> > Why do these operations need to go into hw/registerfields.h?
> > It's not a common operation, since we've never needed it so far.
>
> I suggested it to improve readability of this patch:
> https://lore.kernel.org/qemu-devel/c33257a3-645f-9386-52e5-21a15ef0e...@amsat.org/
>
> > > ---
> > > include/hw/registerfields.h | 28
> > > 1 file changed, 28 insertions(+)
> > >
> > > diff --git a/include/hw/registerfields.h
> > > b/include/hw/registerfields.h
> > > index 1330ca77de..5a804f72e3 100644
> > > --- a/include/hw/registerfields.h
> > > +++ b/include/hw/registerfields.h
> > > @@ -115,6 +115,34 @@
> > > R_ ## reg ## _ ## field ## _LENGTH,
> > > _v.v); \
> > > _d; })
> > > +/* Get the max value (uint) discribed by `num_bits` bits */
> > > +#define MAX_N_BITS(num_bits) ((1 << (num_bits)) - 1)
> > > +
> > > +/*
> > > + * Clear the specified field in reg_val if
> > > + * all field bits are set, else no changes made. Implements
> > > + * single/multi-bit `rw1c`
> > > + */
> > > +#define FIELD8_1CLEAR(reg_val, reg,
> > > field) \
> > > + ((FIELD_EX8(reg_val, reg, field)
> > > == \
> > > + MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH))
> > > ? \
> > > + FIELD_DP8(reg_val, reg, field, 0x00) : reg_val)
> > > +
> > > +#define FIELD16_1CLEAR(reg_val, reg,
> > > field) \
> > > + ((FIELD_EX16(reg_val, reg, field)
> > > == \
> > > + MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH))
> > > ? \
> > > + FIELD_DP16(reg_val, reg, field, 0x00) : reg_val)
> > > +
> > > +#define FIELD32_1CLEAR(reg_val, reg,
> > > field) \
> > > + ((FIELD_EX32(reg_val, reg, field)
> > > == \
> > > + MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH))
> > > ? \
> > > + FIELD_DP32(reg_val, reg, field, 0x00) : reg_val)
> > > +
> > > +#define FIELD64_1CLEAR(reg_val, reg,
> > > field) \
> > > + ((FIELD_EX64(reg_val, reg, field)
> > > == \
> > > + MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH))
> > > ? \
> > > + FIELD_DP64(reg_val, reg, field, 0x00) : reg_val)
> > > +
> > > #define FIELD_SDP8(storage, reg, field, val)
> > > ({ \
> > > struct
> > > { \
> > > signed int v:R_ ## reg ## _ ## field ##
> > > _LENGTH; \
> >
> >
>
Re: [PATCH 3/3] hw/riscv: opentitan: Expose the resetvec as a SoC property
On Wed, 2022-09-14 at 12:11 +0200, Alistair Francis via wrote:
> On the OpenTitan hardware the resetvec is fixed at the start of ROM.
> In
> QEMU we don't run the ROM code and instead just jump to the next
> stage.
> This means we need to be a little more flexible about what the
> resetvec
> is.
>
> This patch allows us to set the resetvec from the command line with
> something like this:
> -global
> driver=riscv.lowrisc.ibex.soc,property=resetvec,value=0x2400
>
> This way as the next stage changes we can update the resetvec.
>
> Signed-off-by: Alistair Francis
> ---
> include/hw/riscv/opentitan.h | 2 ++
> hw/riscv/opentitan.c | 8 +++-
> 2 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/riscv/opentitan.h
> b/include/hw/riscv/opentitan.h
> index 26d960f288..6665cd5794 100644
> --- a/include/hw/riscv/opentitan.h
> +++ b/include/hw/riscv/opentitan.h
> @@ -46,6 +46,8 @@ struct LowRISCIbexSoCState {
> IbexTimerState timer;
> IbexSPIHostState spi_host[OPENTITAN_NUM_SPI_HOSTS];
>
> + uint32_t resetvec;
> +
> MemoryRegion flash_mem;
> MemoryRegion rom;
> MemoryRegion flash_alias;
> diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
> index 45c92c9bbc..be7ff1eea0 100644
> --- a/hw/riscv/opentitan.c
> +++ b/hw/riscv/opentitan.c
> @@ -142,7 +142,7 @@ static void lowrisc_ibex_soc_realize(DeviceState
> *dev_soc, Error **errp)
> _abort);
> object_property_set_int(OBJECT(>cpus), "num-harts", ms-
> >smp.cpus,
> _abort);
> - object_property_set_int(OBJECT(>cpus), "resetvec",
> 0x2400,
> + object_property_set_int(OBJECT(>cpus), "resetvec", s-
> >resetvec,
> _abort);
> sysbus_realize(SYS_BUS_DEVICE(>cpus), _fatal);
>
> @@ -297,10 +297,16 @@ static void
> lowrisc_ibex_soc_realize(DeviceState *dev_soc, Error **errp)
> memmap[IBEX_DEV_PERI].base, memmap[IBEX_DEV_PERI].size);
> }
>
> +static Property lowrisc_ibex_soc_props[] = {
> + DEFINE_PROP_UINT32("resetvec", LowRISCIbexSoCState, resetvec,
> 0x2400),
> + DEFINE_PROP_END_OF_LIST()
> +};
> +
> static void lowrisc_ibex_soc_class_init(ObjectClass *oc, void *data)
> {
> DeviceClass *dc = DEVICE_CLASS(oc);
>
> + device_class_set_props(dc, lowrisc_ibex_soc_props);
> dc->realize = lowrisc_ibex_soc_realize;
> /* Reason: Uses serial_hds in realize function, thus can't be
> used twice */
> dc->user_creatable = false;
Nice! I tested this on https://github.com/tock/tock/pull/3056 , with
the addition of `global
driver=riscv.lowrisc.ibex.soc,property=resetvec,value=0x2450 `
Alot more convienient with this patch for when the entry point changes,
will look into parsing the manifest to dynamically set it!
Reviewed by: Wilfred Mallawa
Re: [PATCH 2/3] hw/riscv: opentitan: Fixup resetvec
On Wed, 2022-09-14 at 12:11 +0200, Alistair Francis via wrote: > The resetvec for the OpenTitan machine ended up being set to an out > of > date value, so let's fix that and bump it to the correct start > address > (after the boot ROM) > > Fixes: bf8803c64d75 "hw/riscv: opentitan: bump opentitan version" > Signed-off-by: Alistair Francis > --- > hw/riscv/opentitan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c > index af13dbe3b1..45c92c9bbc 100644 > --- a/hw/riscv/opentitan.c > +++ b/hw/riscv/opentitan.c > @@ -142,7 +142,7 @@ static void lowrisc_ibex_soc_realize(DeviceState > *dev_soc, Error **errp) > _abort); > object_property_set_int(OBJECT(>cpus), "num-harts", ms- > >smp.cpus, > _abort); > - object_property_set_int(OBJECT(>cpus), "resetvec", > 0x2490, > + object_property_set_int(OBJECT(>cpus), "resetvec", > 0x2400, > _abort); > sysbus_realize(SYS_BUS_DEVICE(>cpus), _fatal); > Reviewed by: Wilfred Mallawa
[RFC] hw/registerfields: add `FIELDx_1CLEAR()` macro
From: Wilfred Mallawa
Adds a helper macro that implements the `rw1c`
behaviour.
Ex:
uint32_t data = FIELD32_1CLEAR(val, REG, FIELD);
if the specified `FIELD` is set (single/multi bit all fields)
then the respective field is cleared and returned to `data`.
If ALL bits of the bitfield are not set, then no change and
val is returned.
Signed-off-by: Wilfred Mallawa
---
include/hw/registerfields.h | 28
1 file changed, 28 insertions(+)
diff --git a/include/hw/registerfields.h b/include/hw/registerfields.h
index 1330ca77de..5a804f72e3 100644
--- a/include/hw/registerfields.h
+++ b/include/hw/registerfields.h
@@ -115,6 +115,34 @@
R_ ## reg ## _ ## field ## _LENGTH, _v.v); \
_d; })
+/* Get the max value (uint) discribed by `num_bits` bits */
+#define MAX_N_BITS(num_bits) ((1 << (num_bits)) - 1)
+
+/*
+ * Clear the specified field in reg_val if
+ * all field bits are set, else no changes made. Implements
+ * single/multi-bit `rw1c`
+ */
+#define FIELD8_1CLEAR(reg_val, reg, field)\
+((FIELD_EX8(reg_val, reg, field) == \
+ MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) ? \
+ FIELD_DP8(reg_val, reg, field, 0x00) : reg_val)
+
+#define FIELD16_1CLEAR(reg_val, reg, field) \
+((FIELD_EX16(reg_val, reg, field) == \
+ MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) ? \
+ FIELD_DP16(reg_val, reg, field, 0x00) : reg_val)
+
+#define FIELD32_1CLEAR(reg_val, reg, field) \
+((FIELD_EX32(reg_val, reg, field) == \
+ MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) ? \
+ FIELD_DP32(reg_val, reg, field, 0x00) : reg_val)
+
+#define FIELD64_1CLEAR(reg_val, reg, field) \
+((FIELD_EX64(reg_val, reg, field) == \
+ MAX_N_BITS(R_ ## reg ## _ ## field ## _LENGTH)) ? \
+ FIELD_DP64(reg_val, reg, field, 0x00) : reg_val)
+
#define FIELD_SDP8(storage, reg, field, val) ({ \
struct { \
signed int v:R_ ## reg ## _ ## field ## _LENGTH; \
--
2.37.2
[PATCH v4 4/4] hw/ssi: ibex_spi: update reg addr
From: Wilfred Mallawa Updates the `EVENT_ENABLE` register to offset `0x34` as per OpenTitan spec [1]. [1] https://docs.opentitan.org/hw/ip/spi_host/doc/#Reg_event_enable Signed-off-by: Wilfred Mallawa Reviewed-by: Alistair Francis --- hw/ssi/ibex_spi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index 40d401ad47..0becad34e0 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30) FIELD(ERROR_STATUS, CMDINVAL, 3, 1) FIELD(ERROR_STATUS, CSIDINVAL, 4, 1) FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1) -REG32(EVENT_ENABLE, 0x30) +REG32(EVENT_ENABLE, 0x34) FIELD(EVENT_ENABLE, RXFULL, 0, 1) FIELD(EVENT_ENABLE, TXEMPTY, 1, 1) FIELD(EVENT_ENABLE, RXWM, 2, 1) -- 2.37.2
[PATCH v4 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
From: Wilfred Mallawa
This patch adds the `rw1c` functionality to the respective
registers. The status fields are cleared when the respective
field is set.
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
---
hw/ssi/ibex_spi_host.c | 34 --
include/hw/ssi/ibex_spi_host.h | 4 ++--
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d52b193a1a..40d401ad47 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -352,7 +352,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
switch (addr) {
/* Skipping any R/O registers */
-case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE:
+case IBEX_SPI_HOST_INTR_STATE:
+/* rw1c status register */
+if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
+data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+}
+if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
+data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+}
+s->regs[addr] = data;
+break;
+case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
break;
case IBEX_SPI_HOST_INTR_TEST:
@@ -495,7 +505,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
-s->regs[addr] = val32;
+status = s->regs[addr];
+/* rw1c status register */
+if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
+}
+s->regs[addr] = status;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
diff --git a/include/hw/ssi/ibex_spi_host.h b/include/hw/ssi/ibex_spi_host.h
index 3fedcb6805..1f6d077766 100644
--- a/include/hw/ssi/ibex_spi_host.h
+++ b/include/hw/ssi/ibex_spi_host.h
@@ -40,7 +40,7 @@
OBJECT_CHECK(IbexSPIHostState, (obj), TYPE_IBEX_SPI_HOST)
/* SPI Registers */
-#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw */
+#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw1c */
#define IBEX_SPI_HOST_INTR_ENABLE(0x04 / 4) /* rw */
#define IBEX_SPI_HOST_INTR_TEST (0x08 / 4) /* wo */
#define IBEX_SPI_HOST_ALERT_TEST (0x0c / 4) /* wo */
@@ -54,7 +54,7 @@
#define IBEX_SPI_HOST_TXDATA (0x28 / 4)
#define IBEX_SPI_HOST_ERROR_ENABLE (0x2c / 4) /* rw */
-#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw */
+#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw1c */
#define IBEX_SPI_HOST_EVENT_ENABLE (0x34 / 4) /* rw */
/* FIFO Len in Bytes */
--
2.37.2
[PATCH v4 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host
From: Wilfred Mallawa
This patch fixes up minor typos in ibex_spi_host
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Reviewed-by: Andrew Jones
---
hw/ssi/ibex_spi_host.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d14580b409..601041d719 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -172,7 +172,7 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
& R_INTR_STATE_SPI_EVENT_MASK;
int err_irq = 0, event_irq = 0;
-/* Error IRQ enabled and Error IRQ Cleared*/
+/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
@@ -434,7 +434,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
case IBEX_SPI_HOST_TXDATA:
/*
* This is a hardware `feature` where
- * the first word written TXDATA after init is omitted entirely
+ * the first word written to TXDATA after init is omitted entirely
*/
if (s->init_status) {
s->init_status = false;
@@ -487,7 +487,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
break;
case IBEX_SPI_HOST_ERROR_STATUS:
/*
- * Indicates that any errors that have occurred.
+ * Indicates any errors that have occurred.
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
--
2.37.2
[PATCH v4 2/4] hw/ssi: ibex_spi: fixup coverity issue
From: Wilfred Mallawa
This patch addresses the coverity issues specified in [1],
as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
implemented to clean up the code.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
Fixes: Coverity CID 1488107
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Reviewed-by: Andrew Jones
---
hw/ssi/ibex_spi_host.c | 132 +
1 file changed, 68 insertions(+), 64 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 601041d719..d52b193a1a 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -108,18 +108,22 @@ static inline uint8_t div4_round_up(uint8_t dividend)
static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the RX FIFO and assert RXEMPTY */
fifo8_reset(>rx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, RXFULL, 0);
+data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the TX FIFO and assert TXEMPTY */
fifo8_reset(>tx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, TXFULL, 0);
+data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +166,38 @@ static void ibex_spi_host_reset(DeviceState *dev)
*/
static void ibex_spi_host_irq(IbexSPIHostState *s)
{
-bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_ERROR_MASK;
-bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_SPI_EVENT_MASK;
-bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_ERROR_MASK;
-bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_SPI_EVENT_MASK;
+uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST];
+uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
+uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE];
+
+uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
+uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE];
+uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
+uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS];
+
+
+bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR);
+bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT);
+bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR);
+bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT);
+
int err_irq = 0, event_irq = 0;
/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
+if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) {
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDBUSY_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDBUSY_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CMDBUSY)) {
/* Wrote to COMMAND when not READY */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDINVAL) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CMDINVAL)) {
/* Invalid command segment */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CSIDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CSIDINVAL) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CSIDINVAL)) {
/* Invalid value for CSID */
err_irq = 1;
}
@@ -204,22 +209,19 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
/* Event IRQ Enabled and Event IR
[PATCH v4 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs
From: Wilfred Mallawa Patch V4 fixes up: - Fixup missing register field clearing on tx/rx_fifo_reset() in [2/4] Testing: - Tested with Opentitan unit tests for TockOS...[OK] Wilfred Mallawa (4): hw/ssi: ibex_spi: fixup typos in ibex_spi_host hw/ssi: ibex_spi: fixup coverity issue hw/ssi: ibex_spi: fixup/add rw1c functionality hw/ssi: ibex_spi: update reg addr hw/ssi/ibex_spi_host.c | 174 - include/hw/ssi/ibex_spi_host.h | 4 +- 2 files changed, 106 insertions(+), 72 deletions(-) -- 2.37.2
Re: [PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue
On Mon, 2022-08-22 at 13:42 +1000, Alistair Francis wrote:
> On Mon, Aug 22, 2022 at 9:53 AM Wilfred Mallawa
> wrote:
> >
> > From: Wilfred Mallawa
> >
> > This patch addresses the coverity issues specified in [1],
> > as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> > implemented to clean up the code.
> >
> > [1]
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
> >
> > Fixes: Coverity CID 1488107
> >
> > Signed-off-by: Wilfred Mallawa
> > Reviewed-by: Andrew Jones
> > ---
> > hw/ssi/ibex_spi_host.c | 130 +
> >
> > 1 file changed, 66 insertions(+), 64 deletions(-)
> >
> > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > index 601041d719..1298664d2b 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t
> > dividend)
> >
> > static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
> > {
> > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > /* Empty the RX FIFO and assert RXEMPTY */
> > fifo8_reset(>rx_fifo);
> > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> > + data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
>
> Doesn't this no longer clear the RXFULL bits?
>
> > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > }
> >
> > static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
> > {
> > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > /* Empty the TX FIFO and assert TXEMPTY */
> > fifo8_reset(>tx_fifo);
> > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> > + data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
>
> Same here about TXFULL
>
> Otherwise the patch looks good
>
> Alistair
>
Good catch! I'll fix these up.
Wilfred
> > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > }
> >
> > static void ibex_spi_host_reset(DeviceState *dev)
> > @@ -162,37 +164,38 @@ static void ibex_spi_host_reset(DeviceState
> > *dev)
> > */
> > static void ibex_spi_host_irq(IbexSPIHostState *s)
> > {
> > - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > - & R_INTR_ENABLE_ERROR_MASK;
> > - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > - & R_INTR_ENABLE_SPI_EVENT_MASK;
> > - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > - & R_INTR_STATE_ERROR_MASK;
> > - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > - & R_INTR_STATE_SPI_EVENT_MASK;
> > + uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST];
> > + uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
> > + uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE];
> > +
> > + uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
> > + uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE];
> > + uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
> > + uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS];
> > +
> > +
> > + bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR);
> > + bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE,
> > SPI_EVENT);
> > + bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE,
> > ERROR);
> > + bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE,
> > SPI_EVENT);
> > +
> > int err_irq = 0, event_irq = 0;
> >
> > /* Error IRQ enabled and Error IRQ Cleared */
> > if (error_en && !err_pending) {
> > /* Event enabled, Interrupt Test Error */
> > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] &
> > R_INTR_TEST_ERROR_MASK) {
> > + if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) {
> > err_irq = 1;
> > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> > - & R_ERROR_ENABLE_CMDBUSY_MASK) &&
> > - s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> > - & R_ERROR_STATUS_CMDBUSY_MASK) {
> > + } else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY)
> > &&
> > + FIELD_EX32(err_status_reg, ERROR_ST
[PATCH v3 2/4] hw/ssi: ibex_spi: fixup coverity issue
From: Wilfred Mallawa
This patch addresses the coverity issues specified in [1],
as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
implemented to clean up the code.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
Fixes: Coverity CID 1488107
Signed-off-by: Wilfred Mallawa
Reviewed-by: Andrew Jones
---
hw/ssi/ibex_spi_host.c | 130 +
1 file changed, 66 insertions(+), 64 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 601041d719..1298664d2b 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend)
static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the RX FIFO and assert RXEMPTY */
fifo8_reset(>rx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the TX FIFO and assert TXEMPTY */
fifo8_reset(>tx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +164,38 @@ static void ibex_spi_host_reset(DeviceState *dev)
*/
static void ibex_spi_host_irq(IbexSPIHostState *s)
{
-bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_ERROR_MASK;
-bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_SPI_EVENT_MASK;
-bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_ERROR_MASK;
-bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_SPI_EVENT_MASK;
+uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST];
+uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
+uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE];
+
+uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
+uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE];
+uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
+uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS];
+
+
+bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR);
+bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT);
+bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR);
+bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT);
+
int err_irq = 0, event_irq = 0;
/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
+if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) {
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDBUSY_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDBUSY_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CMDBUSY)) {
/* Wrote to COMMAND when not READY */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDINVAL) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CMDINVAL)) {
/* Invalid command segment */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CSIDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CSIDINVAL) &&
+ FIELD_EX32(err_status_reg, ERROR_STATUS, CSIDINVAL)) {
/* Invalid value for CSID */
err_irq = 1;
}
@@ -204,22 +207,19 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
/* Event IRQ Enabled and Event IRQ Cleared */
if (event_en && !status_pending) {
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] &
[PATCH v3 4/4] hw/ssi: ibex_spi: update reg addr
From: Wilfred Mallawa Updates the `EVENT_ENABLE` register to offset `0x34` as per OpenTitan spec [1]. [1] https://docs.opentitan.org/hw/ip/spi_host/doc/#Reg_event_enable Signed-off-by: Wilfred Mallawa Reviewed-by: Alistair Francis --- hw/ssi/ibex_spi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index 3809febb0c..bafff8ca1f 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30) FIELD(ERROR_STATUS, CMDINVAL, 3, 1) FIELD(ERROR_STATUS, CSIDINVAL, 4, 1) FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1) -REG32(EVENT_ENABLE, 0x30) +REG32(EVENT_ENABLE, 0x34) FIELD(EVENT_ENABLE, RXFULL, 0, 1) FIELD(EVENT_ENABLE, TXEMPTY, 1, 1) FIELD(EVENT_ENABLE, RXWM, 2, 1) -- 2.37.2
[PATCH v3 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs
From: Wilfred Mallawa This patch series cleans up the ibex_spi driver, fixes the specified coverity issue, implements register rw1c functionality and updates an incorrect register offset. Patch V3 fixes up: - Style errors (excess indentation on multi-line) - Remove patch note from commit message in [2/4] Testing: - Tested with Opentitan unit tests for TockOS...[OK] Wilfred Mallawa (4): hw/ssi: ibex_spi: fixup typos in ibex_spi_host hw/ssi: ibex_spi: fixup coverity issue hw/ssi: ibex_spi: fixup/add rw1c functionality hw/ssi: ibex_spi: update reg addr hw/ssi/ibex_spi_host.c | 172 +++-- include/hw/ssi/ibex_spi_host.h | 4 +- 2 files changed, 104 insertions(+), 72 deletions(-) -- 2.37.2
[PATCH v3 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
From: Wilfred Mallawa
This patch adds the `rw1c` functionality to the respective
registers. The status fields are cleared when the respective
field is set.
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
---
hw/ssi/ibex_spi_host.c | 34 --
include/hw/ssi/ibex_spi_host.h | 4 ++--
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 1298664d2b..3809febb0c 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -350,7 +350,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
switch (addr) {
/* Skipping any R/O registers */
-case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE:
+case IBEX_SPI_HOST_INTR_STATE:
+/* rw1c status register */
+if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
+data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+}
+if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
+data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+}
+s->regs[addr] = data;
+break;
+case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
break;
case IBEX_SPI_HOST_INTR_TEST:
@@ -493,7 +503,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
-s->regs[addr] = val32;
+status = s->regs[addr];
+/* rw1c status register */
+if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
+}
+s->regs[addr] = status;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
diff --git a/include/hw/ssi/ibex_spi_host.h b/include/hw/ssi/ibex_spi_host.h
index 3fedcb6805..1f6d077766 100644
--- a/include/hw/ssi/ibex_spi_host.h
+++ b/include/hw/ssi/ibex_spi_host.h
@@ -40,7 +40,7 @@
OBJECT_CHECK(IbexSPIHostState, (obj), TYPE_IBEX_SPI_HOST)
/* SPI Registers */
-#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw */
+#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw1c */
#define IBEX_SPI_HOST_INTR_ENABLE(0x04 / 4) /* rw */
#define IBEX_SPI_HOST_INTR_TEST (0x08 / 4) /* wo */
#define IBEX_SPI_HOST_ALERT_TEST (0x0c / 4) /* wo */
@@ -54,7 +54,7 @@
#define IBEX_SPI_HOST_TXDATA (0x28 / 4)
#define IBEX_SPI_HOST_ERROR_ENABLE (0x2c / 4) /* rw */
-#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw */
+#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw1c */
#define IBEX_SPI_HOST_EVENT_ENABLE (0x34 / 4) /* rw */
/* FIFO Len in Bytes */
--
2.37.2
[PATCH v3 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host
From: Wilfred Mallawa
This patch fixes up minor typos in ibex_spi_host
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Reviewed-by: Andrew Jones
---
hw/ssi/ibex_spi_host.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d14580b409..601041d719 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -172,7 +172,7 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
& R_INTR_STATE_SPI_EVENT_MASK;
int err_irq = 0, event_irq = 0;
-/* Error IRQ enabled and Error IRQ Cleared*/
+/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
@@ -434,7 +434,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
case IBEX_SPI_HOST_TXDATA:
/*
* This is a hardware `feature` where
- * the first word written TXDATA after init is omitted entirely
+ * the first word written to TXDATA after init is omitted entirely
*/
if (s->init_status) {
s->init_status = false;
@@ -487,7 +487,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
break;
case IBEX_SPI_HOST_ERROR_STATUS:
/*
- * Indicates that any errors that have occurred.
+ * Indicates any errors that have occurred.
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
--
2.37.2
[PATCH v2 4/4] hw/ssi: ibex_spi: update reg addr
From: Wilfred Mallawa Updates the `EVENT_ENABLE` register to offset `0x34` as per OpenTitan spec [1]. [1] https://docs.opentitan.org/hw/ip/spi_host/doc/#Reg_event_enable Signed-off-by: Wilfred Mallawa --- hw/ssi/ibex_spi_host.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c index 19dd094d76..3b8dcbce05 100644 --- a/hw/ssi/ibex_spi_host.c +++ b/hw/ssi/ibex_spi_host.c @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30) FIELD(ERROR_STATUS, CMDINVAL, 3, 1) FIELD(ERROR_STATUS, CSIDINVAL, 4, 1) FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1) -REG32(EVENT_ENABLE, 0x30) +REG32(EVENT_ENABLE, 0x34) FIELD(EVENT_ENABLE, RXFULL, 0, 1) FIELD(EVENT_ENABLE, TXEMPTY, 1, 1) FIELD(EVENT_ENABLE, RXWM, 2, 1) -- 2.37.1
[PATCH v2 3/4] hw/ssi: ibex_spi: fixup/add rw1c functionality
From: Wilfred Mallawa
This patch adds the `rw1c` functionality to the respective
registers. The status fields are cleared when the respective
field is set.
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
---
hw/ssi/ibex_spi_host.c | 34 --
include/hw/ssi/ibex_spi_host.h | 4 ++--
2 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d377f1100c..19dd094d76 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -350,7 +350,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
switch (addr) {
/* Skipping any R/O registers */
-case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE:
+case IBEX_SPI_HOST_INTR_STATE:
+/* rw1c status register */
+if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
+data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+}
+if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
+data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+}
+s->regs[addr] = data;
+break;
+case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
break;
case IBEX_SPI_HOST_INTR_TEST:
@@ -493,7 +503,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
-s->regs[addr] = val32;
+status = s->regs[addr];
+/* rw1c status register */
+if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDBUSY, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, OVERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
+status = FIELD_DP32(status, ERROR_STATUS, UNDERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CMDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, CSIDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
+status = FIELD_DP32(status, ERROR_STATUS, ACCESSINVAL, 0);
+}
+s->regs[addr] = status;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
diff --git a/include/hw/ssi/ibex_spi_host.h b/include/hw/ssi/ibex_spi_host.h
index 3fedcb6805..1f6d077766 100644
--- a/include/hw/ssi/ibex_spi_host.h
+++ b/include/hw/ssi/ibex_spi_host.h
@@ -40,7 +40,7 @@
OBJECT_CHECK(IbexSPIHostState, (obj), TYPE_IBEX_SPI_HOST)
/* SPI Registers */
-#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw */
+#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw1c */
#define IBEX_SPI_HOST_INTR_ENABLE(0x04 / 4) /* rw */
#define IBEX_SPI_HOST_INTR_TEST (0x08 / 4) /* wo */
#define IBEX_SPI_HOST_ALERT_TEST (0x0c / 4) /* wo */
@@ -54,7 +54,7 @@
#define IBEX_SPI_HOST_TXDATA (0x28 / 4)
#define IBEX_SPI_HOST_ERROR_ENABLE (0x2c / 4) /* rw */
-#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw */
+#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw1c */
#define IBEX_SPI_HOST_EVENT_ENABLE (0x34 / 4) /* rw */
/* FIFO Len in Bytes */
--
2.37.1
[PATCH v2 2/4] hw/ssi: ibex_spi: fixup coverity issue
From: Wilfred Mallawa
This patch addresses the coverity issues specified in [1],
as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
implemented to clean up the code.
Patch V2: Style changes have been made as suggested by
Andrew Jones, to promote code readability.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
Fixes: Coverity CID 1488107
Signed-off-by: Wilfred Mallawa
---
hw/ssi/ibex_spi_host.c | 128 +
1 file changed, 65 insertions(+), 63 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 601041d719..d377f1100c 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend)
static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the RX FIFO and assert RXEMPTY */
fifo8_reset(>rx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the TX FIFO and assert TXEMPTY */
fifo8_reset(>tx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +164,38 @@ static void ibex_spi_host_reset(DeviceState *dev)
*/
static void ibex_spi_host_irq(IbexSPIHostState *s)
{
-bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_ERROR_MASK;
-bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_SPI_EVENT_MASK;
-bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_ERROR_MASK;
-bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_SPI_EVENT_MASK;
+uint32_t intr_test_reg = s->regs[IBEX_SPI_HOST_INTR_TEST];
+uint32_t intr_en_reg = s->regs[IBEX_SPI_HOST_INTR_ENABLE];
+uint32_t intr_state_reg = s->regs[IBEX_SPI_HOST_INTR_STATE];
+
+uint32_t err_en_reg = s->regs[IBEX_SPI_HOST_ERROR_ENABLE];
+uint32_t event_en_reg = s->regs[IBEX_SPI_HOST_EVENT_ENABLE];
+uint32_t err_status_reg = s->regs[IBEX_SPI_HOST_ERROR_STATUS];
+uint32_t status_reg = s->regs[IBEX_SPI_HOST_STATUS];
+
+
+bool error_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, ERROR);
+bool event_en = FIELD_EX32(intr_en_reg, INTR_ENABLE, SPI_EVENT);
+bool err_pending = FIELD_EX32(intr_state_reg, INTR_STATE, ERROR);
+bool status_pending = FIELD_EX32(intr_state_reg, INTR_STATE, SPI_EVENT);
+
int err_irq = 0, event_irq = 0;
/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
+if (FIELD_EX32(intr_test_reg, INTR_TEST, ERROR)) {
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDBUSY_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDBUSY_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDBUSY) &&
+FIELD_EX32(err_status_reg, ERROR_STATUS, CMDBUSY)) {
/* Wrote to COMMAND when not READY */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CMDINVAL) &&
+FIELD_EX32(err_status_reg, ERROR_STATUS, CMDINVAL)) {
/* Invalid command segment */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CSIDINVAL_MASK) {
+} else if (FIELD_EX32(err_en_reg, ERROR_ENABLE, CSIDINVAL) &&
+FIELD_EX32(err_status_reg, ERROR_STATUS, CSIDINVAL)) {
/* Invalid value for CSID */
err_irq = 1;
}
@@ -204,22 +207,19 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
/* Event IRQ Enabled and Event IRQ Cleared */
if (event_en && !status_pendin
[PATCH v2 1/4] hw/ssi: ibex_spi: fixup typos in ibex_spi_host
From: Wilfred Mallawa
This patch fixes up minor typos in ibex_spi_host
Signed-off-by: Wilfred Mallawa
Reviewed-by: Alistair Francis
Reviewed-by: Andrew Jones
---
hw/ssi/ibex_spi_host.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d14580b409..601041d719 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -172,7 +172,7 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
& R_INTR_STATE_SPI_EVENT_MASK;
int err_irq = 0, event_irq = 0;
-/* Error IRQ enabled and Error IRQ Cleared*/
+/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
@@ -434,7 +434,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
case IBEX_SPI_HOST_TXDATA:
/*
* This is a hardware `feature` where
- * the first word written TXDATA after init is omitted entirely
+ * the first word written to TXDATA after init is omitted entirely
*/
if (s->init_status) {
s->init_status = false;
@@ -487,7 +487,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
break;
case IBEX_SPI_HOST_ERROR_STATUS:
/*
- * Indicates that any errors that have occurred.
+ * Indicates any errors that have occurred.
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
--
2.37.1
[PATCH v2 0/4] hw/ssi: ibex_spi: cleanup and fixup bugs
From: Wilfred Mallawa This patch series (note V2) cleans up the ibex_spi driver, fixes the specified coverity issue, implements register rw1c functionality and updates an incorrect register offset. In V2, the following changes are made. - New patch [4/4] to isolate the register address offset update. - Adding the `ibex_spi` tag to commits - Split the register update to a seperate patch - Address all style suggestions made, to improve readability Testing: - Tested with Opentitan unit tests for TockOS. [OK] Wilfred Mallawa (4): hw/ssi: ibex_spi: fixup typos in ibex_spi_host hw/ssi: ibex_spi: fixup coverity issue hw/ssi: ibex_spi: fixup/add rw1c functionality hw/ssi: ibex_spi: update reg addr hw/ssi/ibex_spi_host.c | 170 - include/hw/ssi/ibex_spi_host.h | 4 +- 2 files changed, 103 insertions(+), 71 deletions(-) -- 2.37.1
Re: [PATCH 2/3] hw/ssi: fixup coverity issue
On Thu, 2022-08-11 at 10:55 +0800, Bin Meng wrote:
> On Thu, Aug 11, 2022 at 8:58 AM Wilfred Mallawa
> wrote:
> >
> > From: Wilfred Mallawa
> >
> > This patch addresses the coverity issues specified in [1],
> > as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
> > implemented to clean up the code.
> >
> > Additionally, the `EVENT_ENABLE` register is correctly updated
> > to addr of `0x34`.
> >
> > [1]
> > https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
> >
> > Fixes: Coverity CID 1488107
> >
> > Signed-off-by: Wilfred Mallawa
>
> nits: please add "ibex_spi" to the tag, like hw/ssi: ibex_spi:
will add in v2 :)
>
> > ---
> > hw/ssi/ibex_spi_host.c | 141 +++--
> >
> > 1 file changed, 78 insertions(+), 63 deletions(-)
> >
> > diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
> > index 601041d719..8c35bfa95f 100644
> > --- a/hw/ssi/ibex_spi_host.c
> > +++ b/hw/ssi/ibex_spi_host.c
> > @@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
> > FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
> > FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
> > FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
> > -REG32(EVENT_ENABLE, 0x30)
> > +REG32(EVENT_ENABLE, 0x34)
> > FIELD(EVENT_ENABLE, RXFULL, 0, 1)
> > FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
> > FIELD(EVENT_ENABLE, RXWM, 2, 1)
> > @@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t
> > dividend)
> >
> > static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
> > {
> > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > /* Empty the RX FIFO and assert RXEMPTY */
> > fifo8_reset(>rx_fifo);
> > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
> > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
> > + data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
> > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > }
> >
> > static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
> > {
> > + uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
> > /* Empty the TX FIFO and assert TXEMPTY */
> > fifo8_reset(>tx_fifo);
> > - s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
> > - s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
> > + data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
> > + s->regs[IBEX_SPI_HOST_STATUS] = data;
> > }
> >
> > static void ibex_spi_host_reset(DeviceState *dev)
> > @@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState
> > *dev)
> > */
> > static void ibex_spi_host_irq(IbexSPIHostState *s)
> > {
> > - bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > - & R_INTR_ENABLE_ERROR_MASK;
> > - bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
> > - & R_INTR_ENABLE_SPI_EVENT_MASK;
> > - bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > - & R_INTR_STATE_ERROR_MASK;
> > - bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
> > - & R_INTR_STATE_SPI_EVENT_MASK;
> > + bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> > + INTR_ENABLE, ERROR);
> > +
> > + bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
> > + INTR_ENABLE, SPI_EVENT);
> > +
> > + bool err_pending = FIELD_EX32(s-
> > >regs[IBEX_SPI_HOST_INTR_STATE],
> > + INTR_STATE, ERROR);
> > +
> > + bool status_pending = FIELD_EX32(s-
> > >regs[IBEX_SPI_HOST_INTR_STATE],
> > + INTR_STATE, SPI_EVENT);
> > +
> > int err_irq = 0, event_irq = 0;
> >
> > /* Error IRQ enabled and Error IRQ Cleared */
> > if (error_en && !err_pending) {
> > /* Event enabled, Interrupt Test Error */
> > - if (s->regs[IBEX_SPI_HOST_INTR_TEST] &
> > R_INTR_TEST_ERROR_MASK) {
> > + if (FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_TEST],
> > INTR_TEST, ERROR)) {
> > err_irq = 1;
> > - } else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
> > - & R_ERROR_ENABLE_CMDBUSY_MASK) &&
> > - s->regs[IBEX_SPI_HOST_ERROR_STATUS]
> > - & R_ERROR_STATUS
[PATCH] hw/riscv: opentitan: bump opentitan version
From: Wilfred Mallawa
The following patch updates opentitan to match the new configuration,
as per, lowRISC/opentitan@217a0168ba118503c166a9587819e3811eeb0c0c
Note: with this patch we now skip the usage of the opentitan
`boot_rom`. The Opentitan boot rom contains hw verification
for devies which we are currently not supporting in qemu. As of now,
the `boot_rom` has no major significance, however, would be good to
support in the future.
Tested by running utests from the latest tock [1]
(that supports this version of OT).
[1] https://github.com/tock/tock/pull/3056
Signed-off-by: Wilfred Mallawa
---
hw/riscv/opentitan.c | 12
include/hw/riscv/opentitan.h | 11 ++-
2 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/hw/riscv/opentitan.c b/hw/riscv/opentitan.c
index 4495a2c039..af13dbe3b1 100644
--- a/hw/riscv/opentitan.c
+++ b/hw/riscv/opentitan.c
@@ -29,9 +29,9 @@
#include "sysemu/sysemu.h"
static const MemMapEntry ibex_memmap[] = {
-[IBEX_DEV_ROM] ={ 0x8000, 16 * KiB },
-[IBEX_DEV_RAM] ={ 0x1000, 0x1 },
-[IBEX_DEV_FLASH] = { 0x2000, 0x8 },
+[IBEX_DEV_ROM] ={ 0x8000, 0x8000 },
+[IBEX_DEV_RAM] ={ 0x1000, 0x2 },
+[IBEX_DEV_FLASH] = { 0x2000, 0x10 },
[IBEX_DEV_UART] = { 0x4000, 0x1000 },
[IBEX_DEV_GPIO] = { 0x4004, 0x1000 },
[IBEX_DEV_SPI_DEVICE] = { 0x4005, 0x1000 },
@@ -40,6 +40,7 @@ static const MemMapEntry ibex_memmap[] = {
[IBEX_DEV_TIMER] = { 0x4010, 0x1000 },
[IBEX_DEV_SENSOR_CTRL] ={ 0x4011, 0x1000 },
[IBEX_DEV_OTP_CTRL] = { 0x4013, 0x4000 },
+[IBEX_DEV_LC_CTRL] ={ 0x4014, 0x1000 },
[IBEX_DEV_USBDEV] = { 0x4015, 0x1000 },
[IBEX_DEV_SPI_HOST0] = { 0x4030, 0x1000 },
[IBEX_DEV_SPI_HOST1] = { 0x4031, 0x1000 },
@@ -141,7 +142,8 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
_abort);
object_property_set_int(OBJECT(>cpus), "num-harts", ms->smp.cpus,
_abort);
-object_property_set_int(OBJECT(>cpus), "resetvec", 0x8080,
_abort);
+object_property_set_int(OBJECT(>cpus), "resetvec", 0x2490,
+_abort);
sysbus_realize(SYS_BUS_DEVICE(>cpus), _fatal);
/* Boot ROM */
@@ -253,6 +255,8 @@ static void lowrisc_ibex_soc_realize(DeviceState *dev_soc,
Error **errp)
memmap[IBEX_DEV_SENSOR_CTRL].base, memmap[IBEX_DEV_SENSOR_CTRL].size);
create_unimplemented_device("riscv.lowrisc.ibex.otp_ctrl",
memmap[IBEX_DEV_OTP_CTRL].base, memmap[IBEX_DEV_OTP_CTRL].size);
+create_unimplemented_device("riscv.lowrisc.ibex.lc_ctrl",
+memmap[IBEX_DEV_LC_CTRL].base, memmap[IBEX_DEV_LC_CTRL].size);
create_unimplemented_device("riscv.lowrisc.ibex.pwrmgr",
memmap[IBEX_DEV_PWRMGR].base, memmap[IBEX_DEV_PWRMGR].size);
create_unimplemented_device("riscv.lowrisc.ibex.rstmgr",
diff --git a/include/hw/riscv/opentitan.h b/include/hw/riscv/opentitan.h
index 68892cd8e5..26d960f288 100644
--- a/include/hw/riscv/opentitan.h
+++ b/include/hw/riscv/opentitan.h
@@ -74,6 +74,7 @@ enum {
IBEX_DEV_TIMER,
IBEX_DEV_SENSOR_CTRL,
IBEX_DEV_OTP_CTRL,
+IBEX_DEV_LC_CTRL,
IBEX_DEV_PWRMGR,
IBEX_DEV_RSTMGR,
IBEX_DEV_CLKMGR,
@@ -105,11 +106,11 @@ enum {
IBEX_UART0_RX_BREAK_ERR_IRQ = 6,
IBEX_UART0_RX_TIMEOUT_IRQ = 7,
IBEX_UART0_RX_PARITY_ERR_IRQ = 8,
-IBEX_TIMER_TIMEREXPIRED0_0= 126,
-IBEX_SPI_HOST0_ERR_IRQ= 150,
-IBEX_SPI_HOST0_SPI_EVENT_IRQ = 151,
-IBEX_SPI_HOST1_ERR_IRQ= 152,
-IBEX_SPI_HOST1_SPI_EVENT_IRQ = 153,
+IBEX_TIMER_TIMEREXPIRED0_0= 127,
+IBEX_SPI_HOST0_ERR_IRQ= 151,
+IBEX_SPI_HOST0_SPI_EVENT_IRQ = 152,
+IBEX_SPI_HOST1_ERR_IRQ= 153,
+IBEX_SPI_HOST1_SPI_EVENT_IRQ = 154,
};
#endif
--
2.37.1
[PATCH 3/3] hw/ssi: fixup/add rw1c functionality
From: Wilfred Mallawa
This patch adds the `rw1c` functionality to the respective
registers. The status fields are cleared when the respective
field is set.
Signed-off-by: Wilfred Mallawa
---
hw/ssi/ibex_spi_host.c | 36 +++---
include/hw/ssi/ibex_spi_host.h | 4 ++--
2 files changed, 35 insertions(+), 5 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 8c35bfa95f..935372506c 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -352,7 +352,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
{
IbexSPIHostState *s = opaque;
uint32_t val32 = val64;
-uint32_t shift_mask = 0xff, data;
+uint32_t shift_mask = 0xff, data = 0;
uint8_t txqd_len;
trace_ibex_spi_host_write(addr, size, val64);
@@ -362,7 +362,17 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
switch (addr) {
/* Skipping any R/O registers */
-case IBEX_SPI_HOST_INTR_STATE...IBEX_SPI_HOST_INTR_ENABLE:
+case IBEX_SPI_HOST_INTR_STATE:
+/* rw1c status register */
+if (FIELD_EX32(val32, INTR_STATE, ERROR)) {
+data = FIELD_DP32(data, INTR_STATE, ERROR, 0);
+}
+if (FIELD_EX32(val32, INTR_STATE, SPI_EVENT)) {
+data = FIELD_DP32(data, INTR_STATE, SPI_EVENT, 0);
+}
+s->regs[addr] = data;
+break;
+case IBEX_SPI_HOST_INTR_ENABLE:
s->regs[addr] = val32;
break;
case IBEX_SPI_HOST_INTR_TEST:
@@ -506,7 +516,27 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
-s->regs[addr] = val32;
+data = s->regs[addr];
+/* rw1c status register */
+if (FIELD_EX32(val32, ERROR_STATUS, CMDBUSY)) {
+data = FIELD_DP32(data, ERROR_STATUS, CMDBUSY, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, OVERFLOW)) {
+data = FIELD_DP32(data, ERROR_STATUS, OVERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, UNDERFLOW)) {
+data = FIELD_DP32(data, ERROR_STATUS, UNDERFLOW, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CMDINVAL)) {
+data = FIELD_DP32(data, ERROR_STATUS, CMDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, CSIDINVAL)) {
+data = FIELD_DP32(data, ERROR_STATUS, CSIDINVAL, 0);
+}
+if (FIELD_EX32(val32, ERROR_STATUS, ACCESSINVAL)) {
+data = FIELD_DP32(data, ERROR_STATUS, ACCESSINVAL, 0);
+}
+s->regs[addr] = data;
break;
case IBEX_SPI_HOST_EVENT_ENABLE:
/* Controls which classes of SPI events raise an interrupt. */
diff --git a/include/hw/ssi/ibex_spi_host.h b/include/hw/ssi/ibex_spi_host.h
index 3fedcb6805..1f6d077766 100644
--- a/include/hw/ssi/ibex_spi_host.h
+++ b/include/hw/ssi/ibex_spi_host.h
@@ -40,7 +40,7 @@
OBJECT_CHECK(IbexSPIHostState, (obj), TYPE_IBEX_SPI_HOST)
/* SPI Registers */
-#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw */
+#define IBEX_SPI_HOST_INTR_STATE (0x00 / 4) /* rw1c */
#define IBEX_SPI_HOST_INTR_ENABLE(0x04 / 4) /* rw */
#define IBEX_SPI_HOST_INTR_TEST (0x08 / 4) /* wo */
#define IBEX_SPI_HOST_ALERT_TEST (0x0c / 4) /* wo */
@@ -54,7 +54,7 @@
#define IBEX_SPI_HOST_TXDATA (0x28 / 4)
#define IBEX_SPI_HOST_ERROR_ENABLE (0x2c / 4) /* rw */
-#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw */
+#define IBEX_SPI_HOST_ERROR_STATUS (0x30 / 4) /* rw1c */
#define IBEX_SPI_HOST_EVENT_ENABLE (0x34 / 4) /* rw */
/* FIFO Len in Bytes */
--
2.37.1
[PATCH 1/3] hw/ssi: fixup typos in ibex_spi_host
From: Wilfred Mallawa
This patch fixes up minor typos in ibex_spi_host
Signed-off-by: Wilfred Mallawa
---
hw/ssi/ibex_spi_host.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index d14580b409..601041d719 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -172,7 +172,7 @@ static void ibex_spi_host_irq(IbexSPIHostState *s)
& R_INTR_STATE_SPI_EVENT_MASK;
int err_irq = 0, event_irq = 0;
-/* Error IRQ enabled and Error IRQ Cleared*/
+/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
@@ -434,7 +434,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
case IBEX_SPI_HOST_TXDATA:
/*
* This is a hardware `feature` where
- * the first word written TXDATA after init is omitted entirely
+ * the first word written to TXDATA after init is omitted entirely
*/
if (s->init_status) {
s->init_status = false;
@@ -487,7 +487,7 @@ static void ibex_spi_host_write(void *opaque, hwaddr addr,
break;
case IBEX_SPI_HOST_ERROR_STATUS:
/*
- * Indicates that any errors that have occurred.
+ * Indicates any errors that have occurred.
* When an error occurs, the corresponding bit must be cleared
* here before issuing any further commands
*/
--
2.37.1
[PATCH 2/3] hw/ssi: fixup coverity issue
From: Wilfred Mallawa
This patch addresses the coverity issues specified in [1],
as suggested, `FIELD_DP32()`/`FIELD_EX32()` macros have been
implemented to clean up the code.
Additionally, the `EVENT_ENABLE` register is correctly updated
to addr of `0x34`.
[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg887713.html
Fixes: Coverity CID 1488107
Signed-off-by: Wilfred Mallawa
---
hw/ssi/ibex_spi_host.c | 141 +++--
1 file changed, 78 insertions(+), 63 deletions(-)
diff --git a/hw/ssi/ibex_spi_host.c b/hw/ssi/ibex_spi_host.c
index 601041d719..8c35bfa95f 100644
--- a/hw/ssi/ibex_spi_host.c
+++ b/hw/ssi/ibex_spi_host.c
@@ -93,7 +93,7 @@ REG32(ERROR_STATUS, 0x30)
FIELD(ERROR_STATUS, CMDINVAL, 3, 1)
FIELD(ERROR_STATUS, CSIDINVAL, 4, 1)
FIELD(ERROR_STATUS, ACCESSINVAL, 5, 1)
-REG32(EVENT_ENABLE, 0x30)
+REG32(EVENT_ENABLE, 0x34)
FIELD(EVENT_ENABLE, RXFULL, 0, 1)
FIELD(EVENT_ENABLE, TXEMPTY, 1, 1)
FIELD(EVENT_ENABLE, RXWM, 2, 1)
@@ -108,18 +108,20 @@ static inline uint8_t div4_round_up(uint8_t dividend)
static void ibex_spi_rxfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the RX FIFO and assert RXEMPTY */
fifo8_reset(>rx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_RXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_RXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, RXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_txfifo_reset(IbexSPIHostState *s)
{
+uint32_t data = s->regs[IBEX_SPI_HOST_STATUS];
/* Empty the TX FIFO and assert TXEMPTY */
fifo8_reset(>tx_fifo);
-s->regs[IBEX_SPI_HOST_STATUS] &= ~R_STATUS_TXFULL_MASK;
-s->regs[IBEX_SPI_HOST_STATUS] |= R_STATUS_TXEMPTY_MASK;
+data = FIELD_DP32(data, STATUS, TXEMPTY, 1);
+s->regs[IBEX_SPI_HOST_STATUS] = data;
}
static void ibex_spi_host_reset(DeviceState *dev)
@@ -162,37 +164,41 @@ static void ibex_spi_host_reset(DeviceState *dev)
*/
static void ibex_spi_host_irq(IbexSPIHostState *s)
{
-bool error_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_ERROR_MASK;
-bool event_en = s->regs[IBEX_SPI_HOST_INTR_ENABLE]
-& R_INTR_ENABLE_SPI_EVENT_MASK;
-bool err_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_ERROR_MASK;
-bool status_pending = s->regs[IBEX_SPI_HOST_INTR_STATE]
-& R_INTR_STATE_SPI_EVENT_MASK;
+bool error_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
+ INTR_ENABLE, ERROR);
+
+bool event_en = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_ENABLE],
+ INTR_ENABLE, SPI_EVENT);
+
+bool err_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
+ INTR_STATE, ERROR);
+
+bool status_pending = FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_STATE],
+ INTR_STATE, SPI_EVENT);
+
int err_irq = 0, event_irq = 0;
/* Error IRQ enabled and Error IRQ Cleared */
if (error_en && !err_pending) {
/* Event enabled, Interrupt Test Error */
-if (s->regs[IBEX_SPI_HOST_INTR_TEST] & R_INTR_TEST_ERROR_MASK) {
+if (FIELD_EX32(s->regs[IBEX_SPI_HOST_INTR_TEST], INTR_TEST, ERROR)) {
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDBUSY_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDBUSY_MASK) {
+} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
+ ERROR_ENABLE, CMDBUSY) &&
+FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
+ ERROR_STATUS, CMDBUSY)) {
/* Wrote to COMMAND when not READY */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CMDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CMDINVAL_MASK) {
+} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_ENABLE],
+ ERROR_ENABLE, CMDINVAL) &&
+FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_STATUS],
+ ERROR_STATUS, CMDINVAL)) {
/* Invalid command segment */
err_irq = 1;
-} else if ((s->regs[IBEX_SPI_HOST_ERROR_ENABLE]
-& R_ERROR_ENABLE_CSIDINVAL_MASK) &&
-s->regs[IBEX_SPI_HOST_ERROR_STATUS]
-& R_ERROR_STATUS_CSIDINVAL_MASK) {
+} else if (FIELD_EX32(s->regs[IBEX_SPI_HOST_ERROR_E
Re: [PATCH 1/2] riscv: opentitan: fixup plic stride len
On Mon, 2022-01-10 at 15:34 +0800, Bin Meng wrote: > CAUTION: This email originated from outside of Western Digital. Do > not click on links or open attachments unless you recognize the > sender and know that the content is safe. > > > On Mon, Jan 10, 2022 at 2:13 PM Alistair Francis > wrote: > > > > From: Wilfred Mallawa > > > > The following change was made to rectify incorrectly set stride > > length > > on the PLIC. Where it should be 32bit and not 24bit (0x18). This > > was > > PLIC [1] Thanks, will add this in. > > > discovered whilst attempting to fix a bug where a timer_interrupt > > was > > not serviced on TockOS-OpenTitan. > > > > [1] > https://docs.opentitan.org/hw/top_earlgrey/ip_autogen/rv_plic/doc/ > > > Signed-off-by: Wilfred Mallawa > > --- > > hw/riscv/opentitan.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > Reviewed-by: Bin Meng
