All of lore.kernel.org
 help / color / mirror / Atom feed
* Query regarding ERRATUM_1418040
@ 2020-06-17  8:55 Neeraj Upadhyay
  2020-06-17 11:19 ` Marc Zyngier
  0 siblings, 1 reply; 11+ messages in thread
From: Neeraj Upadhyay @ 2020-06-17  8:55 UTC (permalink / raw)
  To: marc.zyngier, will; +Cc: linux-arm-kernel

Hi,

I have query regarding the errata 1418040 [1]. Here, on kernel exit to 
EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN; and 
override any other erratas, which might require EL0 direct vct access to 
be disabled for 64 bit also? Also, this errata applies mitigation for 
all CPUs (on return to 32 bit EL0), even if, not all cpus are impacted 
by the errata?




Thanks
Neeraj


[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/kernel/entry.S?h=v5.8-rc1#n334

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member of the Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-06-17  8:55 Query regarding ERRATUM_1418040 Neeraj Upadhyay
@ 2020-06-17 11:19 ` Marc Zyngier
  2020-06-17 11:25   ` Will Deacon
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-06-17 11:19 UTC (permalink / raw)
  To: Neeraj Upadhyay; +Cc: will, linux-arm-kernel

[Thanks Will for pointing me to this email]

Hi Neeraj,

In the future, please use my kernel.org address, as I no longer work for 
ARM.

On 2020-06-17 09:55, Neeraj Upadhyay wrote:
> Hi,
> 
> I have query regarding the errata 1418040 [1]. Here, on kernel exit to
> EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN;
> and override any other erratas, which might require EL0 direct vct
> access to be disabled for 64 bit also?

So far, I am not aware of any erratum that would require traps of 
CNTVCT_EL0 to EL1 when running AArch64 userspace for CPUs that are 
affected by ARM-1418040. If such an erratum exists, it would have to be 
handled separately.

> Also, this errata applies
> mitigation for all CPUs (on return to 32 bit EL0), even if, not all
> cpus are impacted by the errata?

Indeed. There isn't much we can do to avoid it here, unless you want to 
read some per-CPU variable that tells you whether the CPU is affected. 
This would add to the cost of the mitigation , and isn't an appealing 
prospect.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-06-17 11:19 ` Marc Zyngier
@ 2020-06-17 11:25   ` Will Deacon
  2020-06-17 13:00     ` Neeraj Upadhyay
  2020-06-17 14:29     ` Marc Zyngier
  0 siblings, 2 replies; 11+ messages in thread
From: Will Deacon @ 2020-06-17 11:25 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Neeraj Upadhyay, linux-arm-kernel

On Wed, Jun 17, 2020 at 12:19:16PM +0100, Marc Zyngier wrote:
> On 2020-06-17 09:55, Neeraj Upadhyay wrote:
> > I have query regarding the errata 1418040 [1]. Here, on kernel exit to
> > EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN;
> > and override any other erratas, which might require EL0 direct vct
> > access to be disabled for 64 bit also?
> 
> So far, I am not aware of any erratum that would require traps of CNTVCT_EL0
> to EL1 when running AArch64 userspace for CPUs that are affected by
> ARM-1418040. If such an erratum exists, it would have to be handled
> separately.
> 
> > Also, this errata applies
> > mitigation for all CPUs (on return to 32 bit EL0), even if, not all
> > cpus are impacted by the errata?
> 
> Indeed. There isn't much we can do to avoid it here, unless you want to read
> some per-CPU variable that tells you whether the CPU is affected. This would
> add to the cost of the mitigation , and isn't an appealing prospect.

Hmm, but in conjunction with the previous point, doesn't this mean if
some CPUs are affected by an erratum which requires CNTVCT trapping for
AArch64 and others are affected by 1418040, then the former won't actually
be trapped?

Maybe we should preserve ARCH_TIMER_USR_VCT_ACCESS_EN for AArch64 tasks
instead of setting it unconditionally?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-06-17 11:25   ` Will Deacon
@ 2020-06-17 13:00     ` Neeraj Upadhyay
  2020-06-17 14:29     ` Marc Zyngier
  1 sibling, 0 replies; 11+ messages in thread
From: Neeraj Upadhyay @ 2020-06-17 13:00 UTC (permalink / raw)
  To: Will Deacon, Marc Zyngier; +Cc: linux-arm-kernel

Hi Marc,

Sorry for the wrong mail address.

On 6/17/2020 4:55 PM, Will Deacon wrote:
> On Wed, Jun 17, 2020 at 12:19:16PM +0100, Marc Zyngier wrote:
>> On 2020-06-17 09:55, Neeraj Upadhyay wrote:
>>> I have query regarding the errata 1418040 [1]. Here, on kernel exit to
>>> EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN;
>>> and override any other erratas, which might require EL0 direct vct
>>> access to be disabled for 64 bit also?
>>
>> So far, I am not aware of any erratum that would require traps of CNTVCT_EL0
>> to EL1 when running AArch64 userspace for CPUs that are affected by
>> ARM-1418040. If such an erratum exists, it would have to be handled
>> separately.
>>
>>> Also, this errata applies
>>> mitigation for all CPUs (on return to 32 bit EL0), even if, not all
>>> cpus are impacted by the errata?
>>
>> Indeed. There isn't much we can do to avoid it here, unless you want to read
>> some per-CPU variable that tells you whether the CPU is affected. This would
>> add to the cost of the mitigation , and isn't an appealing prospect.
> 
> Hmm, but in conjunction with the previous point, doesn't this mean if
> some CPUs are affected by an erratum which requires CNTVCT trapping for
> AArch64 and others are affected by 1418040, then the former won't actually
> be trapped?
> 
> Maybe we should preserve ARCH_TIMER_USR_VCT_ACCESS_EN for AArch64 tasks
> instead of setting it unconditionally?
> 
> Will
> 

What is the reason for errata being applied on every kernel exit to EL0
and not at process switch point?


Thanks
Neeraj

-- 
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a 
member of the Code Aurora Forum, hosted by The Linux Foundation

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-06-17 11:25   ` Will Deacon
  2020-06-17 13:00     ` Neeraj Upadhyay
@ 2020-06-17 14:29     ` Marc Zyngier
  2020-07-01 16:27       ` Will Deacon
  1 sibling, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-06-17 14:29 UTC (permalink / raw)
  To: Will Deacon; +Cc: Neeraj Upadhyay, linux-arm-kernel

On 2020-06-17 12:25, Will Deacon wrote:
> On Wed, Jun 17, 2020 at 12:19:16PM +0100, Marc Zyngier wrote:
>> On 2020-06-17 09:55, Neeraj Upadhyay wrote:
>> > I have query regarding the errata 1418040 [1]. Here, on kernel exit to
>> > EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN;
>> > and override any other erratas, which might require EL0 direct vct
>> > access to be disabled for 64 bit also?
>> 
>> So far, I am not aware of any erratum that would require traps of 
>> CNTVCT_EL0
>> to EL1 when running AArch64 userspace for CPUs that are affected by
>> ARM-1418040. If such an erratum exists, it would have to be handled
>> separately.
>> 
>> > Also, this errata applies
>> > mitigation for all CPUs (on return to 32 bit EL0), even if, not all
>> > cpus are impacted by the errata?
>> 
>> Indeed. There isn't much we can do to avoid it here, unless you want 
>> to read
>> some per-CPU variable that tells you whether the CPU is affected. This 
>> would
>> add to the cost of the mitigation , and isn't an appealing prospect.
> 
> Hmm, but in conjunction with the previous point, doesn't this mean if
> some CPUs are affected by an erratum which requires CNTVCT trapping for
> AArch64 and others are affected by 1418040, then the former won't 
> actually
> be trapped?

Indeed. Having CPUs that require opposite workarounds is one of the many 
fascinating aspects of BL systems... :-/ Does such a system exist today?

> Maybe we should preserve ARCH_TIMER_USR_VCT_ACCESS_EN for AArch64 tasks
> instead of setting it unconditionally?

We'd still need something when switching from an AArch32 task to an 
AArch64 one. I guess we'd either need to re-enable it on entry from a 
32bit task, or implement some sort of per-CPU, per-ISA state to be 
restored on return to userspace.

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-06-17 14:29     ` Marc Zyngier
@ 2020-07-01 16:27       ` Will Deacon
  2020-07-02  8:19         ` Marc Zyngier
  0 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2020-07-01 16:27 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Neeraj Upadhyay, linux-arm-kernel

On Wed, Jun 17, 2020 at 03:29:46PM +0100, Marc Zyngier wrote:
> On 2020-06-17 12:25, Will Deacon wrote:
> > On Wed, Jun 17, 2020 at 12:19:16PM +0100, Marc Zyngier wrote:
> > > On 2020-06-17 09:55, Neeraj Upadhyay wrote:
> > > > I have query regarding the errata 1418040 [1]. Here, on kernel exit to
> > > > EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN;
> > > > and override any other erratas, which might require EL0 direct vct
> > > > access to be disabled for 64 bit also?
> > > 
> > > So far, I am not aware of any erratum that would require traps of
> > > CNTVCT_EL0
> > > to EL1 when running AArch64 userspace for CPUs that are affected by
> > > ARM-1418040. If such an erratum exists, it would have to be handled
> > > separately.
> > > 
> > > > Also, this errata applies
> > > > mitigation for all CPUs (on return to 32 bit EL0), even if, not all
> > > > cpus are impacted by the errata?
> > > 
> > > Indeed. There isn't much we can do to avoid it here, unless you want
> > > to read
> > > some per-CPU variable that tells you whether the CPU is affected.
> > > This would
> > > add to the cost of the mitigation , and isn't an appealing prospect.
> > 
> > Hmm, but in conjunction with the previous point, doesn't this mean if
> > some CPUs are affected by an erratum which requires CNTVCT trapping for
> > AArch64 and others are affected by 1418040, then the former won't
> > actually
> > be trapped?
> 
> Indeed. Having CPUs that require opposite workarounds is one of the many
> fascinating aspects of BL systems... :-/ Does such a system exist today?

I don't know, but it feels like we should either address the issue of scream
loudly if we detect it!

> > Maybe we should preserve ARCH_TIMER_USR_VCT_ACCESS_EN for AArch64 tasks
> > instead of setting it unconditionally?
> 
> We'd still need something when switching from an AArch32 task to an AArch64
> one. I guess we'd either need to re-enable it on entry from a 32bit task, or
> implement some sort of per-CPU, per-ISA state to be restored on return to
> userspace.

I think re-enabling on entry from a 32-bit task would be the easiest thing to
do. Since you're playing with 32-bit timer bugs atm, do you fancy taking a
look ;)

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-07-01 16:27       ` Will Deacon
@ 2020-07-02  8:19         ` Marc Zyngier
  2020-07-06 12:09           ` Will Deacon
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-07-02  8:19 UTC (permalink / raw)
  To: Will Deacon; +Cc: Neeraj Upadhyay, linux-arm-kernel

On 2020-07-01 17:27, Will Deacon wrote:
> On Wed, Jun 17, 2020 at 03:29:46PM +0100, Marc Zyngier wrote:
>> On 2020-06-17 12:25, Will Deacon wrote:
>> > On Wed, Jun 17, 2020 at 12:19:16PM +0100, Marc Zyngier wrote:
>> > > On 2020-06-17 09:55, Neeraj Upadhyay wrote:
>> > > > I have query regarding the errata 1418040 [1]. Here, on kernel exit to
>> > > > EL0 64 bit mode, will it always enable ARCH_TIMER_USR_VCT_ACCESS_EN;
>> > > > and override any other erratas, which might require EL0 direct vct
>> > > > access to be disabled for 64 bit also?
>> > >
>> > > So far, I am not aware of any erratum that would require traps of
>> > > CNTVCT_EL0
>> > > to EL1 when running AArch64 userspace for CPUs that are affected by
>> > > ARM-1418040. If such an erratum exists, it would have to be handled
>> > > separately.
>> > >
>> > > > Also, this errata applies
>> > > > mitigation for all CPUs (on return to 32 bit EL0), even if, not all
>> > > > cpus are impacted by the errata?
>> > >
>> > > Indeed. There isn't much we can do to avoid it here, unless you want
>> > > to read
>> > > some per-CPU variable that tells you whether the CPU is affected.
>> > > This would
>> > > add to the cost of the mitigation , and isn't an appealing prospect.
>> >
>> > Hmm, but in conjunction with the previous point, doesn't this mean if
>> > some CPUs are affected by an erratum which requires CNTVCT trapping for
>> > AArch64 and others are affected by 1418040, then the former won't
>> > actually
>> > be trapped?
>> 
>> Indeed. Having CPUs that require opposite workarounds is one of the 
>> many
>> fascinating aspects of BL systems... :-/ Does such a system exist 
>> today?
> 
> I don't know, but it feels like we should either address the issue of 
> scream
> loudly if we detect it!

I have no idea how you plan to detect that.

>> > Maybe we should preserve ARCH_TIMER_USR_VCT_ACCESS_EN for AArch64 tasks
>> > instead of setting it unconditionally?
>> 
>> We'd still need something when switching from an AArch32 task to an 
>> AArch64
>> one. I guess we'd either need to re-enable it on entry from a 32bit 
>> task, or
>> implement some sort of per-CPU, per-ISA state to be restored on return 
>> to
>> userspace.
> 
> I think re-enabling on entry from a 32-bit task would be the easiest 
> thing to
> do. Since you're playing with 32-bit timer bugs atm, do you fancy 
> taking a
> look ;)

I came up with this, tested in a guest only by fudging the detection
code (I don't have the offending HW at hand). The use of branches vs
NOPs is debatable, no strong opinion here.

         M.

 From 81126933d6c990dac7213d0ec66c4a9df21fe8b8 Mon Sep 17 00:00:00 2001
 From: Marc Zyngier <maz@kernel.org>
Date: Wed, 1 Jul 2020 21:29:24 +0100
Subject: [PATCH] arm64: Rework ARM_ERRATUM_1414080 handling

The current handling of erratum 1414080 has the side effect that
cntkctl_el1 can get changed for both 32 and 64bit tasks.

This isn't a problem so far, but if we ever need to mitigate another
of these errata on the 64bit side, we'd better keep the messing with
cntkctl_el1 local to 32bit tasks.

For that, make sure that on entering the kernel from a 32bit tasks,
userspace access to cntvct gets enabled, and disabled returning to
userspace, while it never gets changed for 64bit tasks.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
  arch/arm64/kernel/entry.S | 44 +++++++++++++++++++++++++--------------
  1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 5304d193c79d..357bce62c31e 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -167,6 +167,19 @@ alternative_cb_end
  	stp	x28, x29, [sp, #16 * 14]

  	.if	\el == 0
+	.if	\regsize == 32
+	// If we come back from a 32bit task on a system affected by
+	// 1418040, let's reenable userspace access to the virtual counter.
+#ifdef CONFIG_ARM64_ERRATUM_1418040
+alternative_if_not ARM64_WORKAROUND_1418040
+	b	.L__entry_wa_1418040\@
+alternative_else_nop_endif
+	mrs	x0, cntkctl_el1
+	orr	x0, x0, #2	// ARCH_TIMER_USR_VCT_ACCESS_EN
+	msr	cntkctl_el1, x0
+.L__entry_wa_1418040\@:
+#endif
+	.endif
  	clear_gp_regs
  	mrs	x21, sp_el0
  	ldr_this_cpu	tsk, __entry_task, x20
@@ -318,7 +331,21 @@ alternative_else_nop_endif
  	ldr	x23, [sp, #S_SP]		// load return stack pointer
  	msr	sp_el0, x23
  	tst	x22, #PSR_MODE32_BIT		// native task?
-	b.eq	3f
+	b.eq	4f
+
+#ifdef CONFIG_ARM64_ERRATUM_1418040
+alternative_if_not ARM64_WORKAROUND_1418040
+	b	3f
+alternative_else_nop_endif
+	/*
+	 * if (x22.mode32 == 1)
+	 *     cntkctl_el1.el0vcten = 0
+	 */
+	mrs	x1, cntkctl_el1
+	bfi	x1, xzr, #1, #1	// ARCH_TIMER_USR_VCT_ACCESS_EN
+	msr	cntkctl_el1, x1
+3:
+#endif

  #ifdef CONFIG_ARM64_ERRATUM_845719
  alternative_if ARM64_WORKAROUND_845719
@@ -330,22 +357,7 @@ alternative_if ARM64_WORKAROUND_845719
  #endif
  alternative_else_nop_endif
  #endif
-3:
-#ifdef CONFIG_ARM64_ERRATUM_1418040
-alternative_if_not ARM64_WORKAROUND_1418040
-	b	4f
-alternative_else_nop_endif
-	/*
-	 * if (x22.mode32 == cntkctl_el1.el0vcten)
-	 *     cntkctl_el1.el0vcten = ~cntkctl_el1.el0vcten
-	 */
-	mrs	x1, cntkctl_el1
-	eon	x0, x1, x22, lsr #3
-	tbz	x0, #1, 4f
-	eor	x1, x1, #2	// ARCH_TIMER_USR_VCT_ACCESS_EN
-	msr	cntkctl_el1, x1
  4:
-#endif
  	scs_save tsk, x0

  	/* No kernel C function calls after this as user keys are set. */
-- 
2.27.0


-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-07-02  8:19         ` Marc Zyngier
@ 2020-07-06 12:09           ` Will Deacon
  2020-07-06 12:27             ` Marc Zyngier
  0 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2020-07-06 12:09 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Neeraj Upadhyay, linux-arm-kernel

On Thu, Jul 02, 2020 at 09:19:04AM +0100, Marc Zyngier wrote:
> On 2020-07-01 17:27, Will Deacon wrote:
> > On Wed, Jun 17, 2020 at 03:29:46PM +0100, Marc Zyngier wrote:
> > > On 2020-06-17 12:25, Will Deacon wrote:
> > > > Hmm, but in conjunction with the previous point, doesn't this mean if
> > > > some CPUs are affected by an erratum which requires CNTVCT trapping for
> > > > AArch64 and others are affected by 1418040, then the former won't
> > > > actually
> > > > be trapped?
> > > 
> > > Indeed. Having CPUs that require opposite workarounds is one of the
> > > many
> > > fascinating aspects of BL systems... :-/ Does such a system exist
> > > today?
> > 
> > I don't know, but it feels like we should either address the issue of
> > scream
> > loudly if we detect it!
> 
> I have no idea how you plan to detect that.

You'd end up having to have workarounds know about each other and, yes,
it would be awful.

> > I think re-enabling on entry from a 32-bit task would be the easiest
> > thing to
> > do. Since you're playing with 32-bit timer bugs atm, do you fancy taking
> > a
> > look ;)
> 
> I came up with this, tested in a guest only by fudging the detection
> code (I don't have the offending HW at hand). The use of branches vs
> NOPs is debatable, no strong opinion here.
> 
>         M.
> 
> From 81126933d6c990dac7213d0ec66c4a9df21fe8b8 Mon Sep 17 00:00:00 2001
> From: Marc Zyngier <maz@kernel.org>
> Date: Wed, 1 Jul 2020 21:29:24 +0100
> Subject: [PATCH] arm64: Rework ARM_ERRATUM_1414080 handling
> 
> The current handling of erratum 1414080 has the side effect that
> cntkctl_el1 can get changed for both 32 and 64bit tasks.
> 
> This isn't a problem so far, but if we ever need to mitigate another
> of these errata on the 64bit side, we'd better keep the messing with
> cntkctl_el1 local to 32bit tasks.
> 
> For that, make sure that on entering the kernel from a 32bit tasks,
> userspace access to cntvct gets enabled, and disabled returning to
> userspace, while it never gets changed for 64bit tasks.
> 
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> ---
>  arch/arm64/kernel/entry.S | 44 +++++++++++++++++++++++++--------------
>  1 file changed, 28 insertions(+), 16 deletions(-)

Mostly looks fine, just some small nits below. Looking at it now, though,
I suspect it would make more sense to do this at context-switch. We can
do that later on.

> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 5304d193c79d..357bce62c31e 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -167,6 +167,19 @@ alternative_cb_end
>  	stp	x28, x29, [sp, #16 * 14]
> 
>  	.if	\el == 0
> +	.if	\regsize == 32
> +	// If we come back from a 32bit task on a system affected by
> +	// 1418040, let's reenable userspace access to the virtual counter.
> +#ifdef CONFIG_ARM64_ERRATUM_1418040
> +alternative_if_not ARM64_WORKAROUND_1418040
> +	b	.L__entry_wa_1418040\@
> +alternative_else_nop_endif
> +	mrs	x0, cntkctl_el1
> +	orr	x0, x0, #2	// ARCH_TIMER_USR_VCT_ACCESS_EN
> +	msr	cntkctl_el1, x0
> +.L__entry_wa_1418040\@:

nit: naming this .L__entry_skip_wa_1418040\@ would be clearer to me,
as we take the branch to jump over the workaround.

> +#endif
> +	.endif
>  	clear_gp_regs
>  	mrs	x21, sp_el0
>  	ldr_this_cpu	tsk, __entry_task, x20
> @@ -318,7 +331,21 @@ alternative_else_nop_endif
>  	ldr	x23, [sp, #S_SP]		// load return stack pointer
>  	msr	sp_el0, x23
>  	tst	x22, #PSR_MODE32_BIT		// native task?
> -	b.eq	3f
> +	b.eq	4f
> +
> +#ifdef CONFIG_ARM64_ERRATUM_1418040
> +alternative_if_not ARM64_WORKAROUND_1418040
> +	b	3f
> +alternative_else_nop_endif
> +	/*
> +	 * if (x22.mode32 == 1)
> +	 *     cntkctl_el1.el0vcten = 0
> +	 */

I think we can probably drop this comment now, as I find it more confusing
that helpful.

> +	mrs	x1, cntkctl_el1
> +	bfi	x1, xzr, #1, #1	// ARCH_TIMER_USR_VCT_ACCESS_EN

Can you use BIC here?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-07-06 12:09           ` Will Deacon
@ 2020-07-06 12:27             ` Marc Zyngier
  2020-07-06 15:16               ` Will Deacon
  0 siblings, 1 reply; 11+ messages in thread
From: Marc Zyngier @ 2020-07-06 12:27 UTC (permalink / raw)
  To: Will Deacon; +Cc: Neeraj Upadhyay, linux-arm-kernel

On 2020-07-06 13:09, Will Deacon wrote:
> On Thu, Jul 02, 2020 at 09:19:04AM +0100, Marc Zyngier wrote:
>> On 2020-07-01 17:27, Will Deacon wrote:
>> > On Wed, Jun 17, 2020 at 03:29:46PM +0100, Marc Zyngier wrote:
>> > > On 2020-06-17 12:25, Will Deacon wrote:
>> > > > Hmm, but in conjunction with the previous point, doesn't this mean if
>> > > > some CPUs are affected by an erratum which requires CNTVCT trapping for
>> > > > AArch64 and others are affected by 1418040, then the former won't
>> > > > actually
>> > > > be trapped?
>> > >
>> > > Indeed. Having CPUs that require opposite workarounds is one of the
>> > > many
>> > > fascinating aspects of BL systems... :-/ Does such a system exist
>> > > today?
>> >
>> > I don't know, but it feels like we should either address the issue of
>> > scream
>> > loudly if we detect it!
>> 
>> I have no idea how you plan to detect that.
> 
> You'd end up having to have workarounds know about each other and, yes,
> it would be awful.
> 
>> > I think re-enabling on entry from a 32-bit task would be the easiest
>> > thing to
>> > do. Since you're playing with 32-bit timer bugs atm, do you fancy taking
>> > a
>> > look ;)
>> 
>> I came up with this, tested in a guest only by fudging the detection
>> code (I don't have the offending HW at hand). The use of branches vs
>> NOPs is debatable, no strong opinion here.
>> 
>>         M.
>> 
>> From 81126933d6c990dac7213d0ec66c4a9df21fe8b8 Mon Sep 17 00:00:00 2001
>> From: Marc Zyngier <maz@kernel.org>
>> Date: Wed, 1 Jul 2020 21:29:24 +0100
>> Subject: [PATCH] arm64: Rework ARM_ERRATUM_1414080 handling
>> 
>> The current handling of erratum 1414080 has the side effect that
>> cntkctl_el1 can get changed for both 32 and 64bit tasks.
>> 
>> This isn't a problem so far, but if we ever need to mitigate another
>> of these errata on the 64bit side, we'd better keep the messing with
>> cntkctl_el1 local to 32bit tasks.
>> 
>> For that, make sure that on entering the kernel from a 32bit tasks,
>> userspace access to cntvct gets enabled, and disabled returning to
>> userspace, while it never gets changed for 64bit tasks.
>> 
>> Signed-off-by: Marc Zyngier <maz@kernel.org>
>> ---
>>  arch/arm64/kernel/entry.S | 44 
>> +++++++++++++++++++++++++--------------
>>  1 file changed, 28 insertions(+), 16 deletions(-)
> 
> Mostly looks fine, just some small nits below. Looking at it now, 
> though,
> I suspect it would make more sense to do this at context-switch. We can
> do that later on.
> 
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 5304d193c79d..357bce62c31e 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -167,6 +167,19 @@ alternative_cb_end
>>  	stp	x28, x29, [sp, #16 * 14]
>> 
>>  	.if	\el == 0
>> +	.if	\regsize == 32
>> +	// If we come back from a 32bit task on a system affected by
>> +	// 1418040, let's reenable userspace access to the virtual counter.
>> +#ifdef CONFIG_ARM64_ERRATUM_1418040
>> +alternative_if_not ARM64_WORKAROUND_1418040
>> +	b	.L__entry_wa_1418040\@
>> +alternative_else_nop_endif
>> +	mrs	x0, cntkctl_el1
>> +	orr	x0, x0, #2	// ARCH_TIMER_USR_VCT_ACCESS_EN
>> +	msr	cntkctl_el1, x0
>> +.L__entry_wa_1418040\@:
> 
> nit: naming this .L__entry_skip_wa_1418040\@ would be clearer to me,
> as we take the branch to jump over the workaround.

Indeed.

>> +#endif
>> +	.endif
>>  	clear_gp_regs
>>  	mrs	x21, sp_el0
>>  	ldr_this_cpu	tsk, __entry_task, x20
>> @@ -318,7 +331,21 @@ alternative_else_nop_endif
>>  	ldr	x23, [sp, #S_SP]		// load return stack pointer
>>  	msr	sp_el0, x23
>>  	tst	x22, #PSR_MODE32_BIT		// native task?
>> -	b.eq	3f
>> +	b.eq	4f
>> +
>> +#ifdef CONFIG_ARM64_ERRATUM_1418040
>> +alternative_if_not ARM64_WORKAROUND_1418040
>> +	b	3f
>> +alternative_else_nop_endif
>> +	/*
>> +	 * if (x22.mode32 == 1)
>> +	 *     cntkctl_el1.el0vcten = 0
>> +	 */
> 
> I think we can probably drop this comment now, as I find it more 
> confusing
> that helpful.
> 
>> +	mrs	x1, cntkctl_el1
>> +	bfi	x1, xzr, #1, #1	// ARCH_TIMER_USR_VCT_ACCESS_EN
> 
> Can you use BIC here?

You wish. Turns out that BIC is labelled as a v8.2 instruction, even
if is just yet another alias for BFM, and gas sends me packing
if I dare using it.

So the above BFI is a poor man's version of BIC. Which objdump happily
disassembles as... BIC! Yes, this is all braindead.

Anyway, I'll fix this up and repost it together with the rest of the
vdso series.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-07-06 12:27             ` Marc Zyngier
@ 2020-07-06 15:16               ` Will Deacon
  2020-07-06 16:05                 ` Marc Zyngier
  0 siblings, 1 reply; 11+ messages in thread
From: Will Deacon @ 2020-07-06 15:16 UTC (permalink / raw)
  To: Marc Zyngier; +Cc: Neeraj Upadhyay, linux-arm-kernel

On Mon, Jul 06, 2020 at 01:27:16PM +0100, Marc Zyngier wrote:
> On 2020-07-06 13:09, Will Deacon wrote:
> > On Thu, Jul 02, 2020 at 09:19:04AM +0100, Marc Zyngier wrote:
> > > +	mrs	x1, cntkctl_el1
> > > +	bfi	x1, xzr, #1, #1	// ARCH_TIMER_USR_VCT_ACCESS_EN
> > 
> > Can you use BIC here?
> 
> You wish. Turns out that BIC is labelled as a v8.2 instruction, even
> if is just yet another alias for BFM, and gas sends me packing
> if I dare using it.
> 
> So the above BFI is a poor man's version of BIC. Which objdump happily
> disassembles as... BIC! Yes, this is all braindead.

Wha.. really? What about all our other uses of the BIC alias? Are you sure
you're not mixing it up with BFC?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: Query regarding ERRATUM_1418040
  2020-07-06 15:16               ` Will Deacon
@ 2020-07-06 16:05                 ` Marc Zyngier
  0 siblings, 0 replies; 11+ messages in thread
From: Marc Zyngier @ 2020-07-06 16:05 UTC (permalink / raw)
  To: Will Deacon; +Cc: Neeraj Upadhyay, linux-arm-kernel

On 2020-07-06 16:16, Will Deacon wrote:
> On Mon, Jul 06, 2020 at 01:27:16PM +0100, Marc Zyngier wrote:
>> On 2020-07-06 13:09, Will Deacon wrote:
>> > On Thu, Jul 02, 2020 at 09:19:04AM +0100, Marc Zyngier wrote:
>> > > +	mrs	x1, cntkctl_el1
>> > > +	bfi	x1, xzr, #1, #1	// ARCH_TIMER_USR_VCT_ACCESS_EN
>> >
>> > Can you use BIC here?
>> 
>> You wish. Turns out that BIC is labelled as a v8.2 instruction, even
>> if is just yet another alias for BFM, and gas sends me packing
>> if I dare using it.
>> 
>> So the above BFI is a poor man's version of BIC. Which objdump happily
>> disassembles as... BIC! Yes, this is all braindead.
> 
> Wha.. really? What about all our other uses of the BIC alias? Are you 
> sure
> you're not mixing it up with BFC?

Of course I am. BIC is absolutely fine. I blame the lack of coffee,
or something else.

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2020-07-06 16:07 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-17  8:55 Query regarding ERRATUM_1418040 Neeraj Upadhyay
2020-06-17 11:19 ` Marc Zyngier
2020-06-17 11:25   ` Will Deacon
2020-06-17 13:00     ` Neeraj Upadhyay
2020-06-17 14:29     ` Marc Zyngier
2020-07-01 16:27       ` Will Deacon
2020-07-02  8:19         ` Marc Zyngier
2020-07-06 12:09           ` Will Deacon
2020-07-06 12:27             ` Marc Zyngier
2020-07-06 15:16               ` Will Deacon
2020-07-06 16:05                 ` Marc Zyngier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.