From: "Philippe Mathieu-Daudé" <f4bug@amsat.org>
To: Peter Maydell <peter.maydell@linaro.org>,
qemu-arm@nongnu.org, qemu-devel@nongnu.org
Cc: Kumar Gala <kumar.gala@linaro.org>,
Kevin Townsend <kevin.townsend@linaro.org>
Subject: Re: [PATCH 3/3] hw/arm/mps2-tz: Implement AN524 memory remapping via machine property
Date: Tue, 13 Apr 2021 18:51:05 +0200 [thread overview]
Message-ID: <d2029650-d5ab-f412-2cca-086b3666d26e@amsat.org> (raw)
In-Reply-To: <20210412134317.12501-4-peter.maydell@linaro.org>
On 4/12/21 3:43 PM, Peter Maydell wrote:
> The AN524 FPGA image supports two memory maps, which differ
> in where the QSPI and BRAM are. In the default map, the BRAM
> is at 0x0000_0000, and the QSPI at 0x2800_0000. In the second
> map, they are the other way around.
>
> In hardware, the initial mapping can be selected by the user
> by writing either "REMAP: BRAM" (the default) or "REMAP: QSPI"
> in the board configuration file. The guest can also dynamically
> change the mapping via the SCC CFG_REG0 register.
>
> Implement this functionality for QEMU, using a machine property
> "remap" with valid values "BRAM" and "QSPI" to allow the user to set
> the initial mapping, in the same way they can on the FPGA, and
> wiring up the bit from the SCC register to also switch the mapping.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> docs/system/arm/mps2.rst | 10 ++++
> hw/arm/mps2-tz.c | 106 ++++++++++++++++++++++++++++++++++++++-
> 2 files changed, 115 insertions(+), 1 deletion(-)
>
> diff --git a/docs/system/arm/mps2.rst b/docs/system/arm/mps2.rst
> index f83b1517871..8a75beb3a08 100644
> --- a/docs/system/arm/mps2.rst
> +++ b/docs/system/arm/mps2.rst
> @@ -45,3 +45,13 @@ Differences between QEMU and real hardware:
> flash, but only as simple ROM, so attempting to rewrite the flash
> from the guest will fail
> - QEMU does not model the USB controller in MPS3 boards
> +
> +Machine-specific options
> +""""""""""""""""""""""""
> +
> +The following machine-specific options are supported:
> +
> +remap
> + Supported for ``mps3-an524`` only.
> + Set ``BRAM``/``QSPI`` to select the initial memory mapping. The
> + default is ``BRAM``.
> diff --git a/hw/arm/mps2-tz.c b/hw/arm/mps2-tz.c
> index 25016e464d9..e9806779326 100644
> --- a/hw/arm/mps2-tz.c
> +++ b/hw/arm/mps2-tz.c
> @@ -55,6 +55,7 @@
> #include "hw/boards.h"
> #include "exec/address-spaces.h"
> #include "sysemu/sysemu.h"
> +#include "sysemu/reset.h"
> #include "hw/misc/unimp.h"
> #include "hw/char/cmsdk-apb-uart.h"
> #include "hw/timer/cmsdk-apb-timer.h"
> @@ -72,6 +73,7 @@
> #include "hw/core/split-irq.h"
> #include "hw/qdev-clock.h"
> #include "qom/object.h"
> +#include "hw/irq.h"
>
> #define MPS2TZ_NUMIRQ_MAX 96
> #define MPS2TZ_RAM_MAX 5
> @@ -153,6 +155,9 @@ struct MPS2TZMachineState {
> SplitIRQ cpu_irq_splitter[MPS2TZ_NUMIRQ_MAX];
> Clock *sysclk;
> Clock *s32kclk;
> +
> + int remap;
Maybe bool, ...
> + qemu_irq remap_irq;
> };
>
> #define TYPE_MPS2TZ_MACHINE "mps2tz"
> @@ -228,6 +233,10 @@ static const RAMInfo an505_raminfo[] = { {
> },
> };
>
> +/*
> + * Note that the addresses and MPC numbering here should match up
> + * with those used in remap_memory(), which can swap the BRAM and QSPI.
> + */
> static const RAMInfo an524_raminfo[] = { {
> .name = "bram",
> .base = 0x00000000,
> @@ -457,6 +466,7 @@ static MemoryRegion *make_scc(MPS2TZMachineState *mms, void *opaque,
>
> object_initialize_child(OBJECT(mms), "scc", scc, TYPE_MPS2_SCC);
> sccdev = DEVICE(scc);
> + qdev_prop_set_uint32(sccdev, "scc-cfg0", mms->remap);
... and here:
qdev_prop_set_uint32(sccdev, "scc-cfg0", mms->remap ? 1 : 0);
as remap is a bit and scc-cfg0 could have other bits set in the future.
> qdev_prop_set_uint32(sccdev, "scc-cfg4", 0x2);
> qdev_prop_set_uint32(sccdev, "scc-aid", 0x00200008);
> qdev_prop_set_uint32(sccdev, "scc-id", mmc->scc_id);
> @@ -573,6 +583,50 @@ static MemoryRegion *make_mpc(MPS2TZMachineState *mms, void *opaque,
> return sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 0);
> }
>
> +static hwaddr boot_mem_base(MPS2TZMachineState *mms)
> +{
> + /*
> + * Return the canonical address of the block which will be mapped
> + * at address 0x0 (i.e. where the vector table is).
> + * This is usually 0, but if the AN524 alternate memory map is
> + * enabled it will be the base address of the QSPI block.
> + */
> + return mms->remap ? 0x28000000 : 0;
> +}
> +
> +static void remap_memory(MPS2TZMachineState *mms, int map)
> +{
> + /*
> + * Remap the memory for the AN524. 'map' is the value of
> + * SCC CFG_REG0 bit 0, i.e. 0 for the default map and 1
> + * for the "option 1" mapping where QSPI is at address 0.
> + *
> + * Effectively we need to swap around the "upstream" ends of
> + * MPC 0 and MPC 1.
> + */
> + MPS2TZMachineClass *mmc = MPS2TZ_MACHINE_GET_CLASS(mms);
> + int i;
> +
> + if (mmc->fpga_type != FPGA_AN524) {
> + return;
> + }
> +
This is done in the machine_reset() handler and during QDev realization,
so probably not important, but since it is reachable by an IRQ handler,
maybe it is better (thinking about code copy/pasting) to wrap this with:
memory_region_transaction_begin();
> + for (i = 0; i < 2; i++) {
> + TZMPC *mpc = &mms->mpc[i];
> + MemoryRegion *upstream = sysbus_mmio_get_region(SYS_BUS_DEVICE(mpc), 1);
> + hwaddr addr = (i ^ map) ? 0x28000000 : 0;
> +
> + memory_region_set_address(upstream, addr);
> + }
memory_region_transaction_commit();
> +}
> +
> +static void remap_irq_fn(void *opaque, int n, int level)
> +{
> + MPS2TZMachineState *mms = opaque;
> +
> + remap_memory(mms, level);
> +}
Otherwise:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
next prev parent reply other threads:[~2021-04-13 16:51 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-12 13:43 [PATCH 0/3] mps3-an524: support memory remapping Peter Maydell
2021-04-12 13:43 ` [PATCH 1/3] hw/misc/mps2-scc: Add "QEMU interface" comment Peter Maydell
2021-04-12 13:43 ` [PATCH 2/3] hw/misc/mps2-scc: Support using CFG0 bit 0 for remapping Peter Maydell
2021-04-13 16:30 ` Philippe Mathieu-Daudé
2021-04-12 13:43 ` [PATCH 3/3] hw/arm/mps2-tz: Implement AN524 memory remapping via machine property Peter Maydell
2021-04-13 16:51 ` Philippe Mathieu-Daudé [this message]
2021-04-12 14:37 ` [PATCH 0/3] mps3-an524: support memory remapping Philippe Mathieu-Daudé
2021-04-12 14:48 ` Peter Maydell
2021-04-13 16:29 ` Philippe Mathieu-Daudé
2021-04-13 16:33 ` Philippe Mathieu-Daudé
2021-04-12 18:39 ` Richard Henderson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d2029650-d5ab-f412-2cca-086b3666d26e@amsat.org \
--to=f4bug@amsat.org \
--cc=kevin.townsend@linaro.org \
--cc=kumar.gala@linaro.org \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).