Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Fri, Aug 21, 2020 at 11:51 AM Catalin Marinas
wrote:
>
> On Fri, Aug 21, 2020 at 06:02:39PM +0100, Marc Zyngier wrote:
> > On 2020-08-21 15:05, Catalin Marinas wrote:
> > > On Fri, Aug 21, 2020 at 01:45:40PM +0100, Marc Zyngier wrote:
> > > > On 2020-08-21 13:26, Catalin Marinas wrote:
> > > > > On Fri, Aug 21, 2020 at 01:12:10PM +0100, Will Deacon wrote:
> > > > > > On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> > > > > > > On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > > > > > > > @@ -979,6 +980,14 @@
> > > > > > > > write_sysreg(__scs_new, sysreg);
> > > > > > > > \
> > > > > > > > } while (0)
> > > > > > > >
> > > > > > > > +#define read_sysreg_par() ({
> > > > > > > > \
> > > > > > > > + u64 par;
> > > > > > > > \
> > > > > > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > > > > > > \
> > > > > > > > + par = read_sysreg(par_el1);
> > > > > > > > \
> > > > > > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > > > > > > \
> > > > > > > > + par;
> > > > > > > > \
> > > > > > > > +})
> > > > > > >
> > > > > > > I was about to queue this up but one more point to clarify: can
> > > > > > > we get
> > > > > > > an interrupt at either side of the PAR_EL1 read and the handler
> > > > > > > do a
> > > > > > > device read, triggering the erratum? Do we need a DMB at exception
> > > > > > > entry/return?
> > > > > >
> > > > > > Disabling irqs around the PAR access would be simpler, I think
> > > > > > (assuming
> > > > > > this is needed).
> > > > >
> > > > > This wouldn't work if it interrupts a guest.
> > > >
> > > > If we take an interrupt either side of the PAR_EL1 read and that we
> > > > fully exit, the saving of PAR_EL1 on the way out solves the problem.
> > > >
> > > > If we don't fully exit, but instead reenter the guest immediately
> > > > (fixup_guest_exit() returns true), we'd need a DMB at that point,
> > > > at least because of the GICv2 proxying code which performs device
> > > > accesses on the guest's behalf.
> > >
> > > If you are ok with the diff below, I can fold it in:
> > >
> > > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > > b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > > index ca88ea416176..8770cf7ccd42 100644
> > > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > > @@ -420,7 +420,7 @@ static inline bool fixup_guest_exit(struct
> > > kvm_vcpu *vcpu, u64 *exit_code)
> > > if (cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_TX2_219_TVM) &&
> > > kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 &&
> > > handle_tx2_tvm(vcpu))
> > > - return true;
> > > + goto guest;
> > >
> > > /*
> > > * We trap the first access to the FP/SIMD to save the host context
> > > @@ -430,13 +430,13 @@ static inline bool fixup_guest_exit(struct
> > > kvm_vcpu *vcpu, u64 *exit_code)
> > > * Similarly for trapped SVE accesses.
> > > */
> > > if (__hyp_handle_fpsimd(vcpu))
> > > - return true;
> > > + goto guest;
> > >
> > > if (__hyp_handle_ptrauth(vcpu))
> > > - return true;
> > > + goto guest;
> > >
> > > if (!__populate_fault_info(vcpu))
> > > - return true;
> > > + goto guest;
> > >
> > > if (static_branch_unlikely(_v2_cpuif_trap)) {
> > > bool valid;
> > > @@ -451,7 +451,7 @@ static inline bool fixup_guest_exit(struct
> > > kvm_vcpu *vcpu, u64 *exit_code)
> > > int ret = __vgic_v2_perform_cpuif_access(vcpu);
> > >
> > > if (ret == 1)
> > > - return true;
> > > + goto guest;
> > >
> > > /* Promote an illegal access to an SError.*/
> > > if (ret == -1)
> > > @@ -467,12 +467,17 @@ static inline bool fixup_guest_exit(struct
> > > kvm_vcpu *vcpu, u64 *exit_code)
> > > int ret = __vgic_v3_perform_cpuif_access(vcpu);
> > >
> > > if (ret == 1)
> > > - return true;
> > > + goto guest;
> > > }
> > >
> > > exit:
> > > /* Return to the host kernel and handle the exit */
> > > return false;
> > > +
> > > +guest:
> > > + /* Re-enter the guest */
> > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > + return true;
> > > }
> > >
> > > static inline bool __needs_ssbd_off(struct kvm_vcpu *vcpu)
> >
> > Looks good to me!
>
> Thanks Marc. Since it needs the local_irq_save() around the PAR_EL1
> access in read_sysreg_par(), I'll wait for Rob to update the patches.
> Rob also asked the hardware guys for clarification on this scenario, so
> let's see what they reply.
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Fri, Aug 21, 2020 at 06:02:39PM +0100, Marc Zyngier wrote:
> On 2020-08-21 15:05, Catalin Marinas wrote:
> > On Fri, Aug 21, 2020 at 01:45:40PM +0100, Marc Zyngier wrote:
> > > On 2020-08-21 13:26, Catalin Marinas wrote:
> > > > On Fri, Aug 21, 2020 at 01:12:10PM +0100, Will Deacon wrote:
> > > > > On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> > > > > > On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > > > > > > @@ -979,6 +980,14 @@
> > > > > > > write_sysreg(__scs_new, sysreg);
> > > > > > > \
> > > > > > > } while (0)
> > > > > > >
> > > > > > > +#define read_sysreg_par() ({
> > > > > > > \
> > > > > > > + u64 par;
> > > > > > > \
> > > > > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > > > > > \
> > > > > > > + par = read_sysreg(par_el1);
> > > > > > > \
> > > > > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > > > > > \
> > > > > > > + par;
> > > > > > > \
> > > > > > > +})
> > > > > >
> > > > > > I was about to queue this up but one more point to clarify: can we
> > > > > > get
> > > > > > an interrupt at either side of the PAR_EL1 read and the handler do a
> > > > > > device read, triggering the erratum? Do we need a DMB at exception
> > > > > > entry/return?
> > > > >
> > > > > Disabling irqs around the PAR access would be simpler, I think
> > > > > (assuming
> > > > > this is needed).
> > > >
> > > > This wouldn't work if it interrupts a guest.
> > >
> > > If we take an interrupt either side of the PAR_EL1 read and that we
> > > fully exit, the saving of PAR_EL1 on the way out solves the problem.
> > >
> > > If we don't fully exit, but instead reenter the guest immediately
> > > (fixup_guest_exit() returns true), we'd need a DMB at that point,
> > > at least because of the GICv2 proxying code which performs device
> > > accesses on the guest's behalf.
> >
> > If you are ok with the diff below, I can fold it in:
> >
> > diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > index ca88ea416176..8770cf7ccd42 100644
> > --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> > +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> > @@ -420,7 +420,7 @@ static inline bool fixup_guest_exit(struct
> > kvm_vcpu *vcpu, u64 *exit_code)
> > if (cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_TX2_219_TVM) &&
> > kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 &&
> > handle_tx2_tvm(vcpu))
> > - return true;
> > + goto guest;
> >
> > /*
> > * We trap the first access to the FP/SIMD to save the host context
> > @@ -430,13 +430,13 @@ static inline bool fixup_guest_exit(struct
> > kvm_vcpu *vcpu, u64 *exit_code)
> > * Similarly for trapped SVE accesses.
> > */
> > if (__hyp_handle_fpsimd(vcpu))
> > - return true;
> > + goto guest;
> >
> > if (__hyp_handle_ptrauth(vcpu))
> > - return true;
> > + goto guest;
> >
> > if (!__populate_fault_info(vcpu))
> > - return true;
> > + goto guest;
> >
> > if (static_branch_unlikely(_v2_cpuif_trap)) {
> > bool valid;
> > @@ -451,7 +451,7 @@ static inline bool fixup_guest_exit(struct
> > kvm_vcpu *vcpu, u64 *exit_code)
> > int ret = __vgic_v2_perform_cpuif_access(vcpu);
> >
> > if (ret == 1)
> > - return true;
> > + goto guest;
> >
> > /* Promote an illegal access to an SError.*/
> > if (ret == -1)
> > @@ -467,12 +467,17 @@ static inline bool fixup_guest_exit(struct
> > kvm_vcpu *vcpu, u64 *exit_code)
> > int ret = __vgic_v3_perform_cpuif_access(vcpu);
> >
> > if (ret == 1)
> > - return true;
> > + goto guest;
> > }
> >
> > exit:
> > /* Return to the host kernel and handle the exit */
> > return false;
> > +
> > +guest:
> > + /* Re-enter the guest */
> > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > + return true;
> > }
> >
> > static inline bool __needs_ssbd_off(struct kvm_vcpu *vcpu)
>
> Looks good to me!
Thanks Marc. Since it needs the local_irq_save() around the PAR_EL1
access in read_sysreg_par(), I'll wait for Rob to update the patches.
Rob also asked the hardware guys for clarification on this scenario, so
let's see what they reply.
--
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On 2020-08-21 15:05, Catalin Marinas wrote:
On Fri, Aug 21, 2020 at 01:45:40PM +0100, Marc Zyngier wrote:
On 2020-08-21 13:26, Catalin Marinas wrote:
> On Fri, Aug 21, 2020 at 01:12:10PM +0100, Will Deacon wrote:
> > On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> > > On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > > > @@ -979,6 +980,14 @@
> > > > write_sysreg(__scs_new, sysreg);\
> > > > } while (0)
> > > >
> > > > +#define read_sysreg_par() ({
\
> > > > + u64 par;\
> > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
\
> > > > + par = read_sysreg(par_el1); \
> > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
\
> > > > + par;\
> > > > +})
> > >
> > > I was about to queue this up but one more point to clarify: can we get
> > > an interrupt at either side of the PAR_EL1 read and the handler do a
> > > device read, triggering the erratum? Do we need a DMB at exception
> > > entry/return?
> >
> > Disabling irqs around the PAR access would be simpler, I think
> > (assuming
> > this is needed).
>
> This wouldn't work if it interrupts a guest.
If we take an interrupt either side of the PAR_EL1 read and that we
fully exit, the saving of PAR_EL1 on the way out solves the problem.
If we don't fully exit, but instead reenter the guest immediately
(fixup_guest_exit() returns true), we'd need a DMB at that point,
at least because of the GICv2 proxying code which performs device
accesses on the guest's behalf.
If you are ok with the diff below, I can fold it in:
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h
b/arch/arm64/kvm/hyp/include/hyp/switch.h
index ca88ea416176..8770cf7ccd42 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -420,7 +420,7 @@ static inline bool fixup_guest_exit(struct
kvm_vcpu *vcpu, u64 *exit_code)
if (cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_TX2_219_TVM) &&
kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 &&
handle_tx2_tvm(vcpu))
- return true;
+ goto guest;
/*
* We trap the first access to the FP/SIMD to save the host context
@@ -430,13 +430,13 @@ static inline bool fixup_guest_exit(struct
kvm_vcpu *vcpu, u64 *exit_code)
* Similarly for trapped SVE accesses.
*/
if (__hyp_handle_fpsimd(vcpu))
- return true;
+ goto guest;
if (__hyp_handle_ptrauth(vcpu))
- return true;
+ goto guest;
if (!__populate_fault_info(vcpu))
- return true;
+ goto guest;
if (static_branch_unlikely(_v2_cpuif_trap)) {
bool valid;
@@ -451,7 +451,7 @@ static inline bool fixup_guest_exit(struct
kvm_vcpu *vcpu, u64 *exit_code)
int ret = __vgic_v2_perform_cpuif_access(vcpu);
if (ret == 1)
- return true;
+ goto guest;
/* Promote an illegal access to an SError.*/
if (ret == -1)
@@ -467,12 +467,17 @@ static inline bool fixup_guest_exit(struct
kvm_vcpu *vcpu, u64 *exit_code)
int ret = __vgic_v3_perform_cpuif_access(vcpu);
if (ret == 1)
- return true;
+ goto guest;
}
exit:
/* Return to the host kernel and handle the exit */
return false;
+
+guest:
+ /* Re-enter the guest */
+ asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
+ return true;
}
static inline bool __needs_ssbd_off(struct kvm_vcpu *vcpu)
Looks good to me!
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Fri, Aug 21, 2020 at 01:45:40PM +0100, Marc Zyngier wrote:
> On 2020-08-21 13:26, Catalin Marinas wrote:
> > On Fri, Aug 21, 2020 at 01:12:10PM +0100, Will Deacon wrote:
> > > On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> > > > On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > > > > @@ -979,6 +980,14 @@
> > > > > write_sysreg(__scs_new, sysreg);
> > > > > \
> > > > > } while (0)
> > > > >
> > > > > +#define read_sysreg_par() ({
> > > > > \
> > > > > + u64 par;
> > > > > \
> > > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > > > \
> > > > > + par = read_sysreg(par_el1);
> > > > > \
> > > > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
> > > > > \
> > > > > + par;
> > > > > \
> > > > > +})
> > > >
> > > > I was about to queue this up but one more point to clarify: can we get
> > > > an interrupt at either side of the PAR_EL1 read and the handler do a
> > > > device read, triggering the erratum? Do we need a DMB at exception
> > > > entry/return?
> > >
> > > Disabling irqs around the PAR access would be simpler, I think
> > > (assuming
> > > this is needed).
> >
> > This wouldn't work if it interrupts a guest.
>
> If we take an interrupt either side of the PAR_EL1 read and that we
> fully exit, the saving of PAR_EL1 on the way out solves the problem.
>
> If we don't fully exit, but instead reenter the guest immediately
> (fixup_guest_exit() returns true), we'd need a DMB at that point,
> at least because of the GICv2 proxying code which performs device
> accesses on the guest's behalf.
If you are ok with the diff below, I can fold it in:
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h
b/arch/arm64/kvm/hyp/include/hyp/switch.h
index ca88ea416176..8770cf7ccd42 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -420,7 +420,7 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu,
u64 *exit_code)
if (cpus_have_final_cap(ARM64_WORKAROUND_CAVIUM_TX2_219_TVM) &&
kvm_vcpu_trap_get_class(vcpu) == ESR_ELx_EC_SYS64 &&
handle_tx2_tvm(vcpu))
- return true;
+ goto guest;
/*
* We trap the first access to the FP/SIMD to save the host context
@@ -430,13 +430,13 @@ static inline bool fixup_guest_exit(struct kvm_vcpu
*vcpu, u64 *exit_code)
* Similarly for trapped SVE accesses.
*/
if (__hyp_handle_fpsimd(vcpu))
- return true;
+ goto guest;
if (__hyp_handle_ptrauth(vcpu))
- return true;
+ goto guest;
if (!__populate_fault_info(vcpu))
- return true;
+ goto guest;
if (static_branch_unlikely(_v2_cpuif_trap)) {
bool valid;
@@ -451,7 +451,7 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu,
u64 *exit_code)
int ret = __vgic_v2_perform_cpuif_access(vcpu);
if (ret == 1)
- return true;
+ goto guest;
/* Promote an illegal access to an SError.*/
if (ret == -1)
@@ -467,12 +467,17 @@ static inline bool fixup_guest_exit(struct kvm_vcpu
*vcpu, u64 *exit_code)
int ret = __vgic_v3_perform_cpuif_access(vcpu);
if (ret == 1)
- return true;
+ goto guest;
}
exit:
/* Return to the host kernel and handle the exit */
return false;
+
+guest:
+ /* Re-enter the guest */
+ asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));
+ return true;
}
static inline bool __needs_ssbd_off(struct kvm_vcpu *vcpu)
--
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On 2020-08-21 13:26, Catalin Marinas wrote:
On Fri, Aug 21, 2020 at 01:12:10PM +0100, Will Deacon wrote:
On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > @@ -979,6 +980,14 @@
> > write_sysreg(__scs_new, sysreg);\
> > } while (0)
> >
> > +#define read_sysreg_par() ({ \
> > + u64 par;\
> > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> > + par = read_sysreg(par_el1); \
> > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> > + par;\
> > +})
>
> I was about to queue this up but one more point to clarify: can we get
> an interrupt at either side of the PAR_EL1 read and the handler do a
> device read, triggering the erratum? Do we need a DMB at exception
> entry/return?
Disabling irqs around the PAR access would be simpler, I think
(assuming
this is needed).
This wouldn't work if it interrupts a guest.
If we take an interrupt either side of the PAR_EL1 read and that we
fully exit, the saving of PAR_EL1 on the way out solves the problem.
If we don't fully exit, but instead reenter the guest immediately
(fixup_guest_exit() returns true), we'd need a DMB at that point,
at least because of the GICv2 proxying code which performs device
accesses on the guest's behalf.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Fri, Aug 21, 2020 at 01:12:10PM +0100, Will Deacon wrote:
> On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> > On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > > @@ -979,6 +980,14 @@
> > > write_sysreg(__scs_new, sysreg);\
> > > } while (0)
> > >
> > > +#define read_sysreg_par() ({
> > > \
> > > + u64 par;\
> > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> > > + par = read_sysreg(par_el1); \
> > > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> > > + par;\
> > > +})
> >
> > I was about to queue this up but one more point to clarify: can we get
> > an interrupt at either side of the PAR_EL1 read and the handler do a
> > device read, triggering the erratum? Do we need a DMB at exception
> > entry/return?
>
> Disabling irqs around the PAR access would be simpler, I think (assuming
> this is needed).
This wouldn't work if it interrupts a guest.
--
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Fri, Aug 21, 2020 at 01:07:00PM +0100, Catalin Marinas wrote:
> On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> > @@ -979,6 +980,14 @@
> > write_sysreg(__scs_new, sysreg);\
> > } while (0)
> >
> > +#define read_sysreg_par() ({
> > \
> > + u64 par;\
> > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> > + par = read_sysreg(par_el1); \
> > + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> > + par;\
> > +})
>
> I was about to queue this up but one more point to clarify: can we get
> an interrupt at either side of the PAR_EL1 read and the handler do a
> device read, triggering the erratum? Do we need a DMB at exception
> entry/return?
Disabling irqs around the PAR access would be simpler, I think (assuming
this is needed).
Will
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
Re: [PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Mon, Aug 03, 2020 at 01:31:27PM -0600, Rob Herring wrote:
> @@ -979,6 +980,14 @@
> write_sysreg(__scs_new, sysreg);\
> } while (0)
>
> +#define read_sysreg_par() ({ \
> + u64 par;\
> + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> + par = read_sysreg(par_el1); \
> + asm(ALTERNATIVE("nop", "dmb sy", ARM64_WORKAROUND_1508412));\
> + par;\
> +})
I was about to queue this up but one more point to clarify: can we get
an interrupt at either side of the PAR_EL1 read and the handler do a
device read, triggering the erratum? Do we need a DMB at exception
entry/return?
--
Catalin
___
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
[PATCH v4 3/3] arm64: Add workaround for Arm Cortex-A77 erratum 1508412
On Cortex-A77 r0p0 and r1p0, a sequence of a non-cacheable or device load and a store exclusive or PAR_EL1 read can cause a deadlock. The workaround requires a DMB SY before and after a PAR_EL1 register read. A deadlock is still possible with the workaround as KVM guests must also have the workaround. IOW, a malicious guest can deadlock an affected systems. This workaround also depends on a firmware counterpart to enable the h/w to insert DMB SY after load and store exclusive instructions. See the errata document SDEN-1152370 v10 [1] for more information. [1] https://static.docs.arm.com/101992/0010/Arm_Cortex_A77_MP074_Software_Developer_Errata_Notice_v10.pdf Cc: Catalin Marinas Cc: James Morse Cc: Suzuki K Poulose Cc: Will Deacon Cc: Marc Zyngier Cc: Julien Thierry Cc: kvmarm@lists.cs.columbia.edu Signed-off-by: Rob Herring --- v4: - Move read_sysreg_par out of KVM code to sysreg.h to share - Also use read_sysreg_par in fault.c and kvm/sys_regs.c - Use alternative f/w for dmbs around PAR read - Use cpus_have_final_cap instead of cpus_have_const_cap - Add note about speculation of PAR read v3: - Add dmbs around PAR reads in KVM code - Clean-up 'work-around' and 'errata' v2: - Don't disable KVM, just print warning --- Documentation/arm64/silicon-errata.rst | 2 ++ arch/arm64/Kconfig | 20 arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/include/asm/sysreg.h| 9 + arch/arm64/kernel/cpu_errata.c | 10 ++ arch/arm64/kvm/arm.c | 3 ++- arch/arm64/kvm/hyp/switch.c| 7 --- arch/arm64/kvm/hyp/sysreg-sr.c | 2 +- arch/arm64/kvm/sys_regs.c | 2 +- arch/arm64/mm/fault.c | 2 +- 10 files changed, 52 insertions(+), 8 deletions(-) diff --git a/Documentation/arm64/silicon-errata.rst b/Documentation/arm64/silicon-errata.rst index 936cf2a59ca4..716b279e3b33 100644 --- a/Documentation/arm64/silicon-errata.rst +++ b/Documentation/arm64/silicon-errata.rst @@ -90,6 +90,8 @@ stable kernels. ++-+-+-+ | ARM| Cortex-A76 | #1463225| ARM64_ERRATUM_1463225 | ++-+-+-+ +| ARM| Cortex-A77 | #1508412| ARM64_ERRATUM_1508412 | +++-+-+-+ | ARM| Neoverse-N1 | #1188873,1418040| ARM64_ERRATUM_1418040 | ++-+-+-+ | ARM| Neoverse-N1 | #1349291| N/A | diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index a4a094bedcb2..53dc281fd1eb 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -626,6 +626,26 @@ config ARM64_ERRATUM_1542419 If unsure, say Y. +config ARM64_ERRATUM_1508412 + bool "Cortex-A77: 1508412: workaround deadlock on sequence of NC/Device load and store exclusive or PAR read" + default y + help + This option adds a workaround for Arm Cortex-A77 erratum 1508412. + + Affected Cortex-A77 cores (r0p0, r1p0) could deadlock on a sequence + of a store-exclusive or read of PAR_EL1 and a load with device or + non-cacheable memory attributes. The workaround depends on a firmware + counterpart. + + KVM guests must also have the workaround implemented or they can + deadlock the system. + + Work around the issue by inserting DMB SY barriers around PAR_EL1 + register reads and warning KVM users. The DMB barrier is sufficient + to prevent a speculative PAR_EL1 read. + + If unsure, say Y. + config CAVIUM_ERRATUM_22375 bool "Cavium erratum 22375, 24313" default y diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index d7b3bb0cb180..2a2cdb4ced8b 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -62,7 +62,8 @@ #define ARM64_HAS_GENERIC_AUTH 52 #define ARM64_HAS_32BIT_EL153 #define ARM64_BTI 54 +#define ARM64_WORKAROUND_1508412 55 -#define ARM64_NCAPS55 +#define ARM64_NCAPS56 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index 463175f80341..17c80d701ae4 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -898,6 +898,7 @@ #include #include +#include #define __DEFINE_MRS_MSR_S_REGNUM \ " .irp num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30\n" \ @@ -979,6 +980,14 @@ write_sysreg(__scs_new,
