Re: [RFC 10/16] xen/arm: vcpreg: Add wrappers to handle co-proc access trapped by HCR_EL2.TVM

[Stefano Stabellini <sstabellini@kernel.org>]

On Mon, 8 Oct 2018, Julien Grall wrote:
> A follow-up patch will require to emulate some accesses to some
> co-processors registers trapped by HCR_EL2.TVM. When set, all NS EL1 writes
> to the virtual memory control registers will be trapped to the hypervisor.
> 
> This patch adds the infrastructure to passthrough the access to host
> registers. For convenience a bunch of helpers have been added to
> generate the different helpers.
> 
> Note that HCR_EL2.TVM will be set in a follow-up patch dynamically.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/vcpreg.c        | 144 +++++++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/cpregs.h |   1 +
>  2 files changed, 145 insertions(+)
> 
> diff --git a/xen/arch/arm/vcpreg.c b/xen/arch/arm/vcpreg.c
> index b04d996fd3..49529b97cd 100644
> --- a/xen/arch/arm/vcpreg.c
> +++ b/xen/arch/arm/vcpreg.c
> @@ -24,6 +24,122 @@
>  #include <asm/traps.h>
>  #include <asm/vtimer.h>
>  
> +/*
> + * Macros to help generating helpers for registers trapped when
> + * HCR_EL2.TVM is set.
> + *
> + * Note that it only traps NS write access from EL1.
> + *
> + *  - TVM_REG() should not be used outside of the macros. It is there to
> + *    help defining TVM_REG32() and TVM_REG64()
> + *  - TVM_REG32(regname, xreg) and TVM_REG64(regname, xreg) are used to
> + *    resp. generate helper accessing 32-bit and 64-bit register. "regname"
> + *    been the Arm32 name and "xreg" the Arm64 name.
         ^ is

Please add that we use the Arm64 reg name to call WRITE_SYSREG in the
Xen source code even on Arm32 in general


> + *  - UPDATE_REG32_COMBINED(lowreg, hireg, xreg) are used to generate a

TVM_REG32_COMBINED


> + *  pair of registers share the same Arm32 registers. "lowreg" and
> + *  "higreg" been resp. the Arm32 name and "xreg" the Arm64 name. "lowreg"
> + *  will use xreg[31:0] and "hireg" will use xreg[63:32].

Please add that xreg is unused in the Arm32 case.


> + */
> +
> +/* The name is passed from the upper macro to workaround macro expansion. */
> +#define TVM_REG(sz, func, reg...)                                           \
> +static bool func(struct cpu_user_regs *regs, uint##sz##_t *r, bool read)    \
> +{                                                                           \
> +    GUEST_BUG_ON(read);                                                     \
> +    WRITE_SYSREG##sz(*r, reg);                                              \
> +                                                                            \
> +    return true;                                                            \
> +}
> +
> +#define TVM_REG32(regname, xreg) TVM_REG(32, vreg_emulate_##regname, xreg)
> +#define TVM_REG64(regname, xreg) TVM_REG(64, vreg_emulate_##regname, xreg)
> +
> +#ifdef CONFIG_ARM_32
> +#define TVM_REG32_COMBINED(lowreg, hireg, xreg)                     \
> +    /* Use TVM_REG directly to workaround macro expansion. */       \
> +    TVM_REG(32, vreg_emulate_##lowreg, lowreg)                      \
> +    TVM_REG(32, vreg_emulate_##hireg, hireg)
> +
> +#else /* CONFIG_ARM_64 */
> +#define TVM_REG32_COMBINED(lowreg, hireg, xreg)                             \
> +static bool vreg_emulate_##xreg(struct cpu_user_regs *regs, uint32_t *r,    \
> +                                bool read, bool hi)                         \
> +{                                                                           \
> +    register_t reg = READ_SYSREG(xreg);                                     \
> +                                                                            \
> +    GUEST_BUG_ON(read);                                                     \
> +    if ( hi ) /* reg[63:32] is AArch32 register hireg */                    \
> +    {                                                                       \
> +        reg &= GENMASK(31, 0);                                              \

Move GENMASK before the if? It's the same regardless


> +        reg |= ((uint64_t)*r) << 32;                                        \
> +    }                                                                       \
> +    else /* reg[31:0] is AArch32 register lowreg. */                        \
> +    {                                                                       \
> +        reg &= GENMASK(31, 0);                                              \
> +        reg |= *r;                                                          \
> +    }                                                                       \
> +    WRITE_SYSREG(reg, xreg);                                                \
> +                                                                            \
> +    return true;                                                            \
> +}                                                                           \
> +                                                                            \
> +static bool vreg_emulate_##lowreg(struct cpu_user_regs *regs, uint32_t *r,  \
> +                                  bool read)                                \
> +{                                                                           \
> +    return vreg_emulate_##xreg(regs, r, read, false);                       \
> +}                                                                           \
> +                                                                            \
> +static bool vreg_emulate_##hireg(struct cpu_user_regs *regs, uint32_t *r,   \
> +                                 bool read)                                 \
> +{                                                                           \
> +    return vreg_emulate_##xreg(regs, r, read, true);                        \
> +}
> +#endif
> +
> +/* Defining helpers for emulating co-processor registers. */
> +TVM_REG32(SCTLR, SCTLR_EL1)
> +/*
> + * AArch32 provides two way to access TTBR* depending on the access
> + * size, whilst AArch64 provides one way.
> + *
> + * When using AArch32, for simplicity, use the same access size as the
> + * guest.
> + */
> +#ifdef CONFIG_ARM_32
> +TVM_REG32(TTBR0_32, TTBR0_32)
> +TVM_REG32(TTBR1_32, TTBR1_32)
> +#else
> +TVM_REG32(TTBR0_32, TTBR0_EL1)
> +TVM_REG32(TTBR1_32, TTBR1_EL1)
> +#endif
> +TVM_REG64(TTBR0, TTBR0_EL1)
> +TVM_REG64(TTBR1, TTBR1_EL1)
> +/* AArch32 registers TTBCR and TTBCR2 share AArch64 register TCR_EL1. */
> +TVM_REG32_COMBINED(TTBCR, TTBCR2, TCR_EL1)
> +TVM_REG32(DACR, DACR32_EL2)
> +TVM_REG32(DFSR, ESR_EL1)
> +TVM_REG32(IFSR, IFSR32_EL2)
> +/* AArch32 registers DFAR and IFAR shares AArch64 register FAR_EL1. */
> +TVM_REG32_COMBINED(DFAR, IFAR, FAR_EL1)
> +TVM_REG32(ADFSR, AFSR0_EL1)
> +TVM_REG32(AIFSR, AFSR1_EL1)
> +/* AArch32 registers MAIR0 and MAIR1 share AArch64 register MAIR_EL1. */
> +TVM_REG32_COMBINED(MAIR0, MAIR1, MAIR_EL1)
> +/* AArch32 registers AMAIR0 and AMAIR1 share AArch64 register AMAIR_EL1. */
> +TVM_REG32_COMBINED(AMAIR0, AMAIR1, AMAIR_EL1)
> +TVM_REG32(CONTEXTIDR, CONTEXTIDR_EL1)
> +
> +/* Macro to generate easily case for co-processor emulation. */
> +#define GENERATE_CASE(reg, sz)                                      \
> +    case HSR_CPREG##sz(reg):                                        \
> +    {                                                               \
> +        bool res;                                                   \
> +                                                                    \
> +        res = vreg_emulate_cp##sz(regs, hsr, vreg_emulate_##reg);   \
> +        ASSERT(res);                                                \
> +        break;                                                      \
> +    }
> +
>  void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>  {
>      const struct hsr_cp32 cp32 = hsr.cp32;
> @@ -64,6 +180,31 @@ void do_cp15_32(struct cpu_user_regs *regs, const union hsr hsr)
>          break;
>  
>      /*
> +     * HCR_EL2.TVM
> +     *
> +     * ARMv8 (DDI 0487B.b): Table D1-37

In 0487D.a is D1-99


> +     */
> +    GENERATE_CASE(SCTLR, 32)
> +    GENERATE_CASE(TTBR0_32, 32)
> +    GENERATE_CASE(TTBR1_32, 32)
> +    GENERATE_CASE(TTBCR, 32)
> +    GENERATE_CASE(TTBCR2, 32)
> +    GENERATE_CASE(DACR, 32)
> +    GENERATE_CASE(DFSR, 32)
> +    GENERATE_CASE(IFSR, 32)
> +    GENERATE_CASE(DFAR, 32)
> +    GENERATE_CASE(IFAR, 32)
> +    GENERATE_CASE(ADFSR, 32)
> +    GENERATE_CASE(AIFSR, 32)
> +    /* AKA PRRR */
> +    GENERATE_CASE(MAIR0, 32)
> +    /* AKA NMRR */
> +    GENERATE_CASE(MAIR1, 32)
> +    GENERATE_CASE(AMAIR0, 32)
> +    GENERATE_CASE(AMAIR1, 32)
> +    GENERATE_CASE(CONTEXTIDR, 32)
> +
> +    /*
>       * MDCR_EL2.TPM
>       *
>       * ARMv7 (DDI 0406C.b): B1.14.17
> @@ -192,6 +333,9 @@ void do_cp15_64(struct cpu_user_regs *regs, const union hsr hsr)
>              return inject_undef_exception(regs, hsr);
>          break;
>  
> +    GENERATE_CASE(TTBR0, 64)
> +    GENERATE_CASE(TTBR1, 64)
> +
>      /*
>       * CPTR_EL2.T{0..9,12..13}
>       *
> diff --git a/xen/include/asm-arm/cpregs.h b/xen/include/asm-arm/cpregs.h
> index 07e5791983..f1cbac5e5d 100644
> --- a/xen/include/asm-arm/cpregs.h
> +++ b/xen/include/asm-arm/cpregs.h
> @@ -142,6 +142,7 @@
>  
>  /* CP15 CR2: Translation Table Base and Control Registers */
>  #define TTBCR           p15,0,c2,c0,2   /* Translation Table Base Control Register */
> +#define TTBCR2          p15,0,c2,c0,3   /* Translation Table Base Control Register 2 */
>  #define TTBR0           p15,0,c2        /* Translation Table Base Reg. 0 */
>  #define TTBR1           p15,1,c2        /* Translation Table Base Reg. 1 */
>  #define HTTBR           p15,4,c2        /* Hyp. Translation Table Base Register */
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel