On Wed, 29 Jan 2020 at 23:56, 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>
> ---
> v5: Drop unioning in ARMCPRegInfo with bank_fieldoffsets[].
> ---
> + for (i = 0; i < ARRAY_SIZE(aliases); i++) {
> + const struct E2HAlias *a = &aliases[i];
> + ARMCPRegInfo *src_reg, *dst_reg;
> +
> + if (a->feature && !a->feature(&cpu->isar)) {
> + continue;
> + }
> +
> + src_reg = g_hash_table_lookup(cpu->cp_regs, &a->src_key);
> + dst_reg = g_hash_table_lookup(cpu->cp_regs, &a->dst_key);
> + g_assert(src_reg != NULL);
> + g_assert(dst_reg != NULL);
> +
> + /* Cross-compare names to detect typos in the keys. */
> + g_assert(strcmp(src_reg->name, a->src_name) == 0);
> + g_assert(strcmp(dst_reg->name, a->dst_name) == 0);
> +
> + /* None of the core system registers use opaque; we will. */
> + g_assert(src_reg->opaque == NULL);
> +
> + /* Create alias before redirection so we dup the right data. */
> + if (a->new_key) {
> + ARMCPRegInfo *new_reg = g_memdup(src_reg, sizeof(ARMCPRegInfo));
> + uint32_t *new_key = g_memdup(&a->new_key, sizeof(uint32_t));
> + bool ok;
> +
> + new_reg->name = a->new_name;
> + new_reg->type |= ARM_CP_ALIAS;
> + /* Remove PL1/PL0 access, leaving PL2/PL3 R/W in place. */
> + new_reg->access &= 0xf0;
That seems like it would be more clear written as
new_reg->access &= (PL2_RW | PL3_RW);
(strictly there the PL3_RW is useless as PL2_RW implies it but
it captures the intent better I think)
> +
> + ok = g_hash_table_insert(cpu->cp_regs, new_key, new_reg);
> + g_assert(ok);
> + }
> +
> + src_reg->opaque = dst_reg;
> + src_reg->orig_readfn = src_reg->readfn ?: raw_read;
> + src_reg->orig_writefn = src_reg->writefn ?: raw_write;
> + if (!src_reg->raw_readfn) {
> + src_reg->raw_readfn = raw_read;
> + }
> + if (!src_reg->raw_writefn) {
> + src_reg->raw_writefn = raw_write;
> + }
> + src_reg->readfn = el2_e2h_read;
> + src_reg->writefn = el2_e2h_write;
> + }
> +}
> +#endif
Otherwise
Reviewed-by: Peter Maydell <peter.mayd...@linaro.org>
thanks
-- PMM
