| Message ID | 20230413110513.243326-9-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Permission Indirection Extension | expand |
On Thu, 13 Apr 2023 12:05:02 +0100, Joey Gouly <joey.gouly@arm.com> wrote: > > Define the new system register TCR2_EL1 and context switch it. > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > Cc: Marc Zyngier <maz@kernel.org> > Cc: Oliver Upton <oliver.upton@linux.dev> > Cc: James Morse <james.morse@arm.com> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > Cc: Zenghui Yu <yuzenghui@huawei.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will@kernel.org> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > --- > arch/arm64/include/asm/kvm_host.h | 1 + > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 4 ++++ > arch/arm64/kvm/sys_regs.c | 1 + > 3 files changed, 6 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index bcd774d74f34..e1137832a01f 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -269,6 +269,7 @@ enum vcpu_sysreg { > TTBR0_EL1, /* Translation Table Base Register 0 */ > TTBR1_EL1, /* Translation Table Base Register 1 */ > TCR_EL1, /* Translation Control Register */ > + TCR2_EL1, /* Extended Translation Control Register */ > ESR_EL1, /* Exception Syndrome Register */ > AFSR0_EL1, /* Auxiliary Fault Status Register 0 */ > AFSR1_EL1, /* Auxiliary Fault Status Register 1 */ > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > index 699ea1f8d409..16199a107a47 100644 > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > @@ -44,6 +44,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) > ctxt_sys_reg(ctxt, TTBR0_EL1) = read_sysreg_el1(SYS_TTBR0); > ctxt_sys_reg(ctxt, TTBR1_EL1) = read_sysreg_el1(SYS_TTBR1); > ctxt_sys_reg(ctxt, TCR_EL1) = read_sysreg_el1(SYS_TCR); > + if (cpus_have_final_cap(ARM64_HAS_TCR2)) > + ctxt_sys_reg(ctxt, TCR2_EL1) = read_sysreg_el1(SYS_TCR2); > ctxt_sys_reg(ctxt, ESR_EL1) = read_sysreg_el1(SYS_ESR); > ctxt_sys_reg(ctxt, AFSR0_EL1) = read_sysreg_el1(SYS_AFSR0); > ctxt_sys_reg(ctxt, AFSR1_EL1) = read_sysreg_el1(SYS_AFSR1); > @@ -114,6 +116,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) > write_sysreg_el1(ctxt_sys_reg(ctxt, CPACR_EL1), SYS_CPACR); > write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR0_EL1), SYS_TTBR0); > write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR1_EL1), SYS_TTBR1); > + if (cpus_have_final_cap(ARM64_HAS_TCR2)) > + write_sysreg_el1(ctxt_sys_reg(ctxt, TCR2_EL1), SYS_TCR2); > write_sysreg_el1(ctxt_sys_reg(ctxt, ESR_EL1), SYS_ESR); > write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR0_EL1), SYS_AFSR0); > write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR1_EL1), SYS_AFSR1); > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 53749d3a0996..5e7e4a433035 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -1871,6 +1871,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, > { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, > { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 }, > + { SYS_DESC(SYS_TCR2_EL1), NULL, reset_unknown, TCR2_EL1 }, I'm not convinced reset_unknown is the right thing, at least for the bits that are defined as "If EL2 and EL3 is not implemented, this bit resets to 0b0 on a reset." Given that an EL1 guest isn't in control of EL2, I'm a bit wary that we start execution of the guest in a context that isn't well defined. My strong preference would be to reset TCR2 just like TCR, unless you can provide a explanation of why UNKNOWN is actually more correct. Thanks, M.
Hi, On Thu, Apr 20, 2023 at 10:13:38AM +0100, Marc Zyngier wrote: > On Thu, 13 Apr 2023 12:05:02 +0100, > Joey Gouly <joey.gouly@arm.com> wrote: > > > > Define the new system register TCR2_EL1 and context switch it. > > > > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > > Cc: Marc Zyngier <maz@kernel.org> > > Cc: Oliver Upton <oliver.upton@linux.dev> > > Cc: James Morse <james.morse@arm.com> > > Cc: Suzuki K Poulose <suzuki.poulose@arm.com> > > Cc: Zenghui Yu <yuzenghui@huawei.com> > > Cc: Catalin Marinas <catalin.marinas@arm.com> > > Cc: Will Deacon <will@kernel.org> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > > --- > > arch/arm64/include/asm/kvm_host.h | 1 + > > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 4 ++++ > > arch/arm64/kvm/sys_regs.c | 1 + > > 3 files changed, 6 insertions(+) > > > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index bcd774d74f34..e1137832a01f 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -269,6 +269,7 @@ enum vcpu_sysreg { > > TTBR0_EL1, /* Translation Table Base Register 0 */ > > TTBR1_EL1, /* Translation Table Base Register 1 */ > > TCR_EL1, /* Translation Control Register */ > > + TCR2_EL1, /* Extended Translation Control Register */ > > ESR_EL1, /* Exception Syndrome Register */ > > AFSR0_EL1, /* Auxiliary Fault Status Register 0 */ > > AFSR1_EL1, /* Auxiliary Fault Status Register 1 */ > > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > > index 699ea1f8d409..16199a107a47 100644 > > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > > @@ -44,6 +44,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) > > ctxt_sys_reg(ctxt, TTBR0_EL1) = read_sysreg_el1(SYS_TTBR0); > > ctxt_sys_reg(ctxt, TTBR1_EL1) = read_sysreg_el1(SYS_TTBR1); > > ctxt_sys_reg(ctxt, TCR_EL1) = read_sysreg_el1(SYS_TCR); > > + if (cpus_have_final_cap(ARM64_HAS_TCR2)) > > + ctxt_sys_reg(ctxt, TCR2_EL1) = read_sysreg_el1(SYS_TCR2); > > ctxt_sys_reg(ctxt, ESR_EL1) = read_sysreg_el1(SYS_ESR); > > ctxt_sys_reg(ctxt, AFSR0_EL1) = read_sysreg_el1(SYS_AFSR0); > > ctxt_sys_reg(ctxt, AFSR1_EL1) = read_sysreg_el1(SYS_AFSR1); > > @@ -114,6 +116,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) > > write_sysreg_el1(ctxt_sys_reg(ctxt, CPACR_EL1), SYS_CPACR); > > write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR0_EL1), SYS_TTBR0); > > write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR1_EL1), SYS_TTBR1); > > + if (cpus_have_final_cap(ARM64_HAS_TCR2)) > > + write_sysreg_el1(ctxt_sys_reg(ctxt, TCR2_EL1), SYS_TCR2); > > write_sysreg_el1(ctxt_sys_reg(ctxt, ESR_EL1), SYS_ESR); > > write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR0_EL1), SYS_AFSR0); > > write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR1_EL1), SYS_AFSR1); > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 53749d3a0996..5e7e4a433035 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -1871,6 +1871,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, > > { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, > > { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 }, > > + { SYS_DESC(SYS_TCR2_EL1), NULL, reset_unknown, TCR2_EL1 }, > > I'm not convinced reset_unknown is the right thing, at least for the > bits that are defined as "If EL2 and EL3 is not implemented, this bit > resets to 0b0 on a reset." > > Given that an EL1 guest isn't in control of EL2, I'm a bit wary that > we start execution of the guest in a context that isn't well defined. > My strong preference would be to reset TCR2 just like TCR, unless you > can provide a explanation of why UNKNOWN is actually more correct. You're right, I have changed this to reset to 0 like TCR_EL1. Thanks, Joey
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index bcd774d74f34..e1137832a01f 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -269,6 +269,7 @@ enum vcpu_sysreg { TTBR0_EL1, /* Translation Table Base Register 0 */ TTBR1_EL1, /* Translation Table Base Register 1 */ TCR_EL1, /* Translation Control Register */ + TCR2_EL1, /* Extended Translation Control Register */ ESR_EL1, /* Exception Syndrome Register */ AFSR0_EL1, /* Auxiliary Fault Status Register 0 */ AFSR1_EL1, /* Auxiliary Fault Status Register 1 */ diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h index 699ea1f8d409..16199a107a47 100644 --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h @@ -44,6 +44,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) ctxt_sys_reg(ctxt, TTBR0_EL1) = read_sysreg_el1(SYS_TTBR0); ctxt_sys_reg(ctxt, TTBR1_EL1) = read_sysreg_el1(SYS_TTBR1); ctxt_sys_reg(ctxt, TCR_EL1) = read_sysreg_el1(SYS_TCR); + if (cpus_have_final_cap(ARM64_HAS_TCR2)) + ctxt_sys_reg(ctxt, TCR2_EL1) = read_sysreg_el1(SYS_TCR2); ctxt_sys_reg(ctxt, ESR_EL1) = read_sysreg_el1(SYS_ESR); ctxt_sys_reg(ctxt, AFSR0_EL1) = read_sysreg_el1(SYS_AFSR0); ctxt_sys_reg(ctxt, AFSR1_EL1) = read_sysreg_el1(SYS_AFSR1); @@ -114,6 +116,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) write_sysreg_el1(ctxt_sys_reg(ctxt, CPACR_EL1), SYS_CPACR); write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR0_EL1), SYS_TTBR0); write_sysreg_el1(ctxt_sys_reg(ctxt, TTBR1_EL1), SYS_TTBR1); + if (cpus_have_final_cap(ARM64_HAS_TCR2)) + write_sysreg_el1(ctxt_sys_reg(ctxt, TCR2_EL1), SYS_TCR2); write_sysreg_el1(ctxt_sys_reg(ctxt, ESR_EL1), SYS_ESR); write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR0_EL1), SYS_AFSR0); write_sysreg_el1(ctxt_sys_reg(ctxt, AFSR1_EL1), SYS_AFSR1); diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c index 53749d3a0996..5e7e4a433035 100644 --- a/arch/arm64/kvm/sys_regs.c +++ b/arch/arm64/kvm/sys_regs.c @@ -1871,6 +1871,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { { SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 }, { SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 }, { SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 }, + { SYS_DESC(SYS_TCR2_EL1), NULL, reset_unknown, TCR2_EL1 }, PTRAUTH_KEY(APIA), PTRAUTH_KEY(APIB),