On Tue, 3 Dec 2019 at 02:30, Richard Henderson
<richard.hender...@linaro.org> wrote:
>
> Several of the EL1/0 registers are redirected to the EL2 version when in
> EL2 and HCR_EL2.E2H is set. Many of these registers have side effects.
> Link together the two ARMCPRegInfo structures after they have been
> properly instantiated. Install common dispatch routines to all of the
> relevant registers.
>
> The same set of registers that are redirected also have additional
> EL12/EL02 aliases created to access the original register that was
> redirected.
>
> Omit the generic timer registers from redirection here, because we'll
> need multiple kinds of redirection from both EL0 and EL2.
>
> Signed-off-by: Richard Henderson <richard.hender...@linaro.org>
> ---
> target/arm/cpu.h | 44 ++++++++----
> target/arm/helper.c | 162 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 193 insertions(+), 13 deletions(-)
>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 4bd1bf915c..bb5a72520e 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2488,19 +2488,6 @@ struct ARMCPRegInfo {
> */
> ptrdiff_t fieldoffset; /* offsetof(CPUARMState, field) */
>
> - /* Offsets of the secure and non-secure fields in CPUARMState for the
> - * register if it is banked. These fields are only used during the
> static
> - * registration of a register. During hashing the bank associated
> - * with a given security state is copied to fieldoffset which is used
> from
> - * there on out.
> - *
> - * It is expected that register definitions use either fieldoffset or
> - * bank_fieldoffsets in the definition but not both. It is also expected
> - * that both bank offsets are set when defining a banked register. This
> - * use indicates that a register is banked.
> - */
> - ptrdiff_t bank_fieldoffsets[2];
> -
> /* Function for making any access checks for this register in addition to
> * those specified by the 'access' permissions bits. If NULL, no extra
> * checks required. The access check is performed at runtime, not at
> @@ -2535,6 +2522,37 @@ struct ARMCPRegInfo {
> * fieldoffset is 0 then no reset will be done.
> */
> CPResetFn *resetfn;
> +
> + union {
> + /*
> + * Offsets of the secure and non-secure fields in CPUARMState for
> + * the register if it is banked. These fields are only used during
> + * the static registration of a register. During hashing the bank
> + * associated with a given security state is copied to fieldoffset
> + * which is used from there on out.
> + *
> + * It is expected that register definitions use either fieldoffset
> + * or bank_fieldoffsets in the definition but not both. It is also
> + * expected that both bank offsets are set when defining a banked
> + * register. This use indicates that a register is banked.
> + */
> + ptrdiff_t bank_fieldoffsets[2];
> +
> + /*
> + * "Original" writefn and readfn.
> + * For ARMv8.1-VHE register aliases, we overwrite the read/write
> + * accessor functions of various EL1/EL0 to perform the runtime
> + * check for which sysreg should actually be modified, and then
> + * forwards the operation. Before overwriting the accessors,
> + * the original function is copied here, so that accesses that
> + * really do go to the EL1/EL0 version proceed normally.
> + * (The corresponding EL2 register is linked via opaque.)
> + */
> + struct {
> + CPReadFn *orig_readfn;
> + CPWriteFn *orig_writefn;
> + };
Does this really need to be a union ? It's not clear to me
why we know the two halves of it are never used at the same time.
> + };
> };
>
> /* Macros which are lvalues for the field in CPUARMState for the
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 1812588fa1..0baf188078 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -5306,6 +5306,158 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
> REGINFO_SENTINEL
> };
>
> +#ifndef CONFIG_USER_ONLY
> +/* Test if system register redirection is to occur in the current state. */
> +static bool redirect_for_e2h(CPUARMState *env)
> +{
> + return arm_current_el(env) == 2 && (arm_hcr_el2_eff(env) & HCR_E2H);
> +}
> +
> +static uint64_t el2_e2h_read(CPUARMState *env, const ARMCPRegInfo *ri)
> +{
> + CPReadFn *readfn;
> +
> + if (redirect_for_e2h(env)) {
> + /* Switch to the saved EL2 version of the register. */
> + ri = ri->opaque;
> + readfn = ri->readfn;
> + } else {
> + readfn = ri->orig_readfn;
> + }
> + if (readfn == NULL) {
> + readfn = raw_read;
> + }
> + return readfn(env, ri);
> +}
> +
> +static void el2_e2h_write(CPUARMState *env, const ARMCPRegInfo *ri,
> + uint64_t value)
> +{
> + CPWriteFn *writefn;
> +
> + if (redirect_for_e2h(env)) {
> + /* Switch to the saved EL2 version of the register. */
> + ri = ri->opaque;
> + writefn = ri->writefn;
> + } else {
> + writefn = ri->orig_writefn;
> + }
> + if (writefn == NULL) {
> + writefn = raw_write;
> + }
> + writefn(env, ri, value);
> +}
I see how this works when we have a readfn or writefn,
but how does the redirection work where the access
goes directly via .fieldoffset ?
thanks
-- PMM
