Hi Reiji,
On Thu, Jan 6, 2022 at 4:29 AM Reiji Watanabe <reijiw@xxxxxxxxxx> wrote:
>
> Add feature_config_ctrl for RAS and AMU, which are indicated in
> ID_AA64PFR0_EL1, to program configuration registers to trap
> guest's using those features when they are not exposed to the guest.
>
> Introduce trap_ras_regs() to change a behavior of guest's access to
> the registers, which is currently raz/wi, depending on the feature's
> availability for the guest (and inject undefined instruction
> exception when guest's RAS register access are trapped and RAS is
> not exposed to the guest). In order to keep the current visibility
> of the RAS registers from userspace (always visible), a visibility
> function for RAS registers is not added.
>
> No code is added for AMU's access/visibility handler because the
> current code already injects the exception for Guest's AMU register
> access unconditionally because AMU is never exposed to the guest.
I think it might be code to trap it anyway, in case AMU guest support
is added in the future.
>
> Signed-off-by: Reiji Watanabe <reijiw@xxxxxxxxxx>
> ---
> arch/arm64/kvm/sys_regs.c | 90 +++++++++++++++++++++++++++++++++++----
> 1 file changed, 82 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 33893a501475..015d67092d5e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -304,6 +304,63 @@ struct feature_config_ctrl {
> void (*trap_activate)(struct kvm_vcpu *vcpu);
> };
>
> +enum vcpu_config_reg {
> + VCPU_HCR_EL2 = 1,
> + VCPU_MDCR_EL2,
> + VCPU_CPTR_EL2,
> +};
> +
> +static void feature_trap_activate(struct kvm_vcpu *vcpu,
> + enum vcpu_config_reg cfg_reg,
> + u64 cfg_set, u64 cfg_clear)
> +{
> + u64 *reg_ptr, reg_val;
> +
> + switch (cfg_reg) {
> + case VCPU_HCR_EL2:
> + reg_ptr = &vcpu->arch.hcr_el2;
> + break;
> + case VCPU_MDCR_EL2:
> + reg_ptr = &vcpu->arch.mdcr_el2;
> + break;
> + case VCPU_CPTR_EL2:
> + reg_ptr = &vcpu->arch.cptr_el2;
> + break;
> + }
> +
> + /* Clear/Set fields that are indicated by cfg_clear/cfg_set. */
> + reg_val = (*reg_ptr & ~cfg_clear);
> + reg_val |= cfg_set;
> + *reg_ptr = reg_val;
> +}
> +
> +static void feature_ras_trap_activate(struct kvm_vcpu *vcpu)
> +{
> + feature_trap_activate(vcpu, VCPU_HCR_EL2, HCR_TERR | HCR_TEA, HCR_FIEN);
Covers all the flags for ras.
> +}
> +
> +static void feature_amu_trap_activate(struct kvm_vcpu *vcpu)
> +{
> + feature_trap_activate(vcpu, VCPU_CPTR_EL2, CPTR_EL2_TAM, 0);
Covers the CPTR flags for AMU, but as you mentioned, does not
explicitly clear HCR_AMVOFFEN.
Cheers,
/fuad
> +}
> +
> +/* For ID_AA64PFR0_EL1 */
> +static struct feature_config_ctrl ftr_ctrl_ras = {
> + .ftr_reg = SYS_ID_AA64PFR0_EL1,
> + .ftr_shift = ID_AA64PFR0_RAS_SHIFT,
> + .ftr_min = ID_AA64PFR0_RAS_V1,
> + .ftr_signed = FTR_UNSIGNED,
> + .trap_activate = feature_ras_trap_activate,
> +};
> +
> +static struct feature_config_ctrl ftr_ctrl_amu = {
> + .ftr_reg = SYS_ID_AA64PFR0_EL1,
> + .ftr_shift = ID_AA64PFR0_AMU_SHIFT,
> + .ftr_min = ID_AA64PFR0_AMU,
> + .ftr_signed = FTR_UNSIGNED,
> + .trap_activate = feature_amu_trap_activate,
> +};
> +
> struct id_reg_info {
> u32 sys_reg; /* Register ID */
> u64 sys_val; /* Sanitized system value */
> @@ -778,6 +835,11 @@ static struct id_reg_info id_aa64pfr0_el1_info = {
> .init = init_id_aa64pfr0_el1_info,
> .validate = validate_id_aa64pfr0_el1,
> .vcpu_mask = vcpu_mask_id_aa64pfr0_el1,
> + .trap_features = &(const struct feature_config_ctrl *[]) {
> + &ftr_ctrl_ras,
> + &ftr_ctrl_amu,
> + NULL,
> + },
> };
>
> static struct id_reg_info id_aa64pfr1_el1_info = {
> @@ -901,6 +963,18 @@ static inline bool vcpu_feature_is_available(struct kvm_vcpu *vcpu,
> return feature_avail(ctrl, val);
> }
>
> +static bool trap_ras_regs(struct kvm_vcpu *vcpu,
> + struct sys_reg_params *p,
> + const struct sys_reg_desc *r)
> +{
> + if (!vcpu_feature_is_available(vcpu, &ftr_ctrl_ras)) {
> + kvm_inject_undefined(vcpu);
> + return false;
> + }
> +
> + return trap_raz_wi(vcpu, p, r);
> +}
> +
> /*
> * ARMv8.1 mandates at least a trivial LORegion implementation, where all the
> * RW registers are RES0 (which we can implement as RAZ/WI). On an ARMv8.0
> @@ -2265,14 +2339,14 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> { SYS_DESC(SYS_AFSR1_EL1), access_vm_reg, reset_unknown, AFSR1_EL1 },
> { SYS_DESC(SYS_ESR_EL1), access_vm_reg, reset_unknown, ESR_EL1 },
>
> - { SYS_DESC(SYS_ERRIDR_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERRSELR_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERXFR_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERXCTLR_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERXSTATUS_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERXADDR_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERXMISC0_EL1), trap_raz_wi },
> - { SYS_DESC(SYS_ERXMISC1_EL1), trap_raz_wi },
> + { SYS_DESC(SYS_ERRIDR_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERRSELR_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERXFR_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERXCTLR_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERXSTATUS_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERXADDR_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERXMISC0_EL1), trap_ras_regs },
> + { SYS_DESC(SYS_ERXMISC1_EL1), trap_ras_regs },
>
> MTE_REG(TFSR_EL1),
> MTE_REG(TFSRE0_EL1),
> --
> 2.34.1.448.ga2b2bfdf31-goog
>
> _______________________________________________
> kvmarm mailing list
> kvmarm@xxxxxxxxxxxxxxxxxxxxx
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
