[PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

[PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

[Michal Orzel]

Workaround for Cortex-A57 erratum #852523 is already
in Xen but Cortex-A72 erratum #853709 is not although
it applies to the same issue.

Signed-off-by: Michal Orzel <michal.orzel@arm.com>
---
 docs/misc/arm/silicon-errata.txt | 1 +
 xen/arch/arm/domain.c            | 6 ++++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index e15d0923e9..1f18a9df58 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -50,6 +50,7 @@ stable hypervisors.
 | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220    |
 | ARM            | Cortex-A57      | #1319537        | N/A                     |
 | ARM            | Cortex-A72      | #1319367        | N/A                     |
+| ARM            | Cortex-A72      | #853709         | N/A                     |
 | ARM            | Cortex-A76      | #1165522        | N/A                     |
 | ARM            | Neoverse-N1     | #1165522        | N/A
 | ARM            | MMU-500         | #842869         | N/A                     |
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 3b37f899b9..18cafcdda7 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -216,7 +216,8 @@ static void ctxt_switch_to(struct vcpu *n)
     WRITE_SYSREG64(n->arch.ttbr1, TTBR1_EL1);
 
     /*
-     * Erratum #852523: DACR32_EL2 must be restored before one of the
+     * Erratum #852523 (Cortex-A57) or erratum #853709 (Cortex-A72):
+     * DACR32_EL2 must be restored before one of the
      * following sysregs: SCTLR_EL1, TCR_EL1, TTBR0_EL1, TTBR1_EL1 or
      * CONTEXTIDR_EL1.
      */
@@ -245,7 +246,8 @@ static void ctxt_switch_to(struct vcpu *n)
 
     /*
      * This write to sysreg CONTEXTIDR_EL1 ensures we don't hit erratum
-     * #852523. I.e DACR32_EL2 is not correctly synchronized.
+     * #852523 (Cortex-A57) or #853709 (Cortex-A72).
+     * I.e DACR32_EL2 is not correctly synchronized.
      */
     WRITE_SYSREG(n->arch.contextidr, CONTEXTIDR_EL1);
     WRITE_SYSREG(n->arch.tpidr_el0, TPIDR_EL0);
-- 
2.28.0

Re: [PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

[Julien Grall]

Hi Michal,

On 14/10/2020 11:05, Michal Orzel wrote:
> Workaround for Cortex-A57 erratum #852523 is already
> in Xen but Cortex-A72 erratum #853709 is not although
> it applies to the same issue.

This commit message is a bit confusing because it implies that Xen 
doesn't workaround #852523. However, we do workaround it (there is no 
runtime check) but not document it.

So how about the following commit message?

"The Cortex-A72 erratum #853709 is the same as the Cortex-A57 erratum 
#852523. As the latter is already workaround, we only need to update the 
documentation."

> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Other than the commit message, I have cross-checked with the 
documentation ([1]):

Reviewed-by: Julien Grall <jgrall@amazon.com>

I can update the commit message on commit.

Cheers,

> ---
>   docs/misc/arm/silicon-errata.txt | 1 +
>   xen/arch/arm/domain.c            | 6 ++++--
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
> index e15d0923e9..1f18a9df58 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -50,6 +50,7 @@ stable hypervisors.
>   | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220    |
>   | ARM            | Cortex-A57      | #1319537        | N/A                     |
>   | ARM            | Cortex-A72      | #1319367        | N/A                     |
> +| ARM            | Cortex-A72      | #853709         | N/A                     |
>   | ARM            | Cortex-A76      | #1165522        | N/A                     |
>   | ARM            | Neoverse-N1     | #1165522        | N/A
>   | ARM            | MMU-500         | #842869         | N/A                     |
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 3b37f899b9..18cafcdda7 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -216,7 +216,8 @@ static void ctxt_switch_to(struct vcpu *n)
>       WRITE_SYSREG64(n->arch.ttbr1, TTBR1_EL1);
>   
>       /*
> -     * Erratum #852523: DACR32_EL2 must be restored before one of the
> +     * Erratum #852523 (Cortex-A57) or erratum #853709 (Cortex-A72):
> +     * DACR32_EL2 must be restored before one of the
>        * following sysregs: SCTLR_EL1, TCR_EL1, TTBR0_EL1, TTBR1_EL1 or
>        * CONTEXTIDR_EL1.
>        */
> @@ -245,7 +246,8 @@ static void ctxt_switch_to(struct vcpu *n)
>   
>       /*
>        * This write to sysreg CONTEXTIDR_EL1 ensures we don't hit erratum
> -     * #852523. I.e DACR32_EL2 is not correctly synchronized.
> +     * #852523 (Cortex-A57) or #853709 (Cortex-A72).
> +     * I.e DACR32_EL2 is not correctly synchronized.
>        */
>       WRITE_SYSREG(n->arch.contextidr, CONTEXTIDR_EL1);
>       WRITE_SYSREG(n->arch.tpidr_el0, TPIDR_EL0);
> 

[1] https://developer.arm.com/documentation/epm012079/11/

-- 
Julien Grall

Re: [PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

[Bertrand Marquis]

Hi,

> On 14 Oct 2020, at 11:05, Michal Orzel <Michal.Orzel@arm.com> wrote:
> 
> Workaround for Cortex-A57 erratum #852523 is already
> in Xen but Cortex-A72 erratum #853709 is not although
> it applies to the same issue.
> 
> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Change in commit message suggested by Julien is quite right and
should be added.

Bertrand

> ---
> docs/misc/arm/silicon-errata.txt | 1 +
> xen/arch/arm/domain.c            | 6 ++++--
> 2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
> index e15d0923e9..1f18a9df58 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -50,6 +50,7 @@ stable hypervisors.
> | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220    |
> | ARM            | Cortex-A57      | #1319537        | N/A                     |
> | ARM            | Cortex-A72      | #1319367        | N/A                     |
> +| ARM            | Cortex-A72      | #853709         | N/A                     |
> | ARM            | Cortex-A76      | #1165522        | N/A                     |
> | ARM            | Neoverse-N1     | #1165522        | N/A
> | ARM            | MMU-500         | #842869         | N/A                     |
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 3b37f899b9..18cafcdda7 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -216,7 +216,8 @@ static void ctxt_switch_to(struct vcpu *n)
>     WRITE_SYSREG64(n->arch.ttbr1, TTBR1_EL1);
> 
>     /*
> -     * Erratum #852523: DACR32_EL2 must be restored before one of the
> +     * Erratum #852523 (Cortex-A57) or erratum #853709 (Cortex-A72):
> +     * DACR32_EL2 must be restored before one of the
>      * following sysregs: SCTLR_EL1, TCR_EL1, TTBR0_EL1, TTBR1_EL1 or
>      * CONTEXTIDR_EL1.
>      */
> @@ -245,7 +246,8 @@ static void ctxt_switch_to(struct vcpu *n)
> 
>     /*
>      * This write to sysreg CONTEXTIDR_EL1 ensures we don't hit erratum
> -     * #852523. I.e DACR32_EL2 is not correctly synchronized.
> +     * #852523 (Cortex-A57) or #853709 (Cortex-A72).
> +     * I.e DACR32_EL2 is not correctly synchronized.
>      */
>     WRITE_SYSREG(n->arch.contextidr, CONTEXTIDR_EL1);
>     WRITE_SYSREG(n->arch.tpidr_el0, TPIDR_EL0);
> -- 
> 2.28.0
> 

Re: [PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

[Michal Orzel]

[-- Attachment #1: Type: text/plain, Size: 3524 bytes --]

Hi Julien,

I agree. You can update the commit message.
Thanks for review.

Michal

________________________________
From: Julien Grall <julien@xen.org>
Sent: Wednesday, October 14, 2020 12:56 PM
To: Michal Orzel <Michal.Orzel@arm.com>; xen-devel@lists.xenproject.org <xen-devel@lists.xenproject.org>
Cc: Stefano Stabellini <sstabellini@kernel.org>; Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>; Bertrand Marquis <Bertrand.Marquis@arm.com>
Subject: Re: [PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

Hi Michal,

On 14/10/2020 11:05, Michal Orzel wrote:
> Workaround for Cortex-A57 erratum #852523 is already
> in Xen but Cortex-A72 erratum #853709 is not although
> it applies to the same issue.

This commit message is a bit confusing because it implies that Xen
doesn't workaround #852523. However, we do workaround it (there is no
runtime check) but not document it.

So how about the following commit message?

"The Cortex-A72 erratum #853709 is the same as the Cortex-A57 erratum
#852523. As the latter is already workaround, we only need to update the
documentation."

> Signed-off-by: Michal Orzel <michal.orzel@arm.com>

Other than the commit message, I have cross-checked with the
documentation ([1]):

Reviewed-by: Julien Grall <jgrall@amazon.com>

I can update the commit message on commit.

Cheers,

> ---
>   docs/misc/arm/silicon-errata.txt | 1 +
>   xen/arch/arm/domain.c            | 6 ++++--
>   2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
> index e15d0923e9..1f18a9df58 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -50,6 +50,7 @@ stable hypervisors.
>   | ARM            | Cortex-A57      | #834220         | ARM64_ERRATUM_834220    |
>   | ARM            | Cortex-A57      | #1319537        | N/A                     |
>   | ARM            | Cortex-A72      | #1319367        | N/A                     |
> +| ARM            | Cortex-A72      | #853709         | N/A                     |
>   | ARM            | Cortex-A76      | #1165522        | N/A                     |
>   | ARM            | Neoverse-N1     | #1165522        | N/A
>   | ARM            | MMU-500         | #842869         | N/A                     |
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 3b37f899b9..18cafcdda7 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -216,7 +216,8 @@ static void ctxt_switch_to(struct vcpu *n)
>       WRITE_SYSREG64(n->arch.ttbr1, TTBR1_EL1);
>
>       /*
> -     * Erratum #852523: DACR32_EL2 must be restored before one of the
> +     * Erratum #852523 (Cortex-A57) or erratum #853709 (Cortex-A72):
> +     * DACR32_EL2 must be restored before one of the
>        * following sysregs: SCTLR_EL1, TCR_EL1, TTBR0_EL1, TTBR1_EL1 or
>        * CONTEXTIDR_EL1.
>        */
> @@ -245,7 +246,8 @@ static void ctxt_switch_to(struct vcpu *n)
>
>       /*
>        * This write to sysreg CONTEXTIDR_EL1 ensures we don't hit erratum
> -     * #852523. I.e DACR32_EL2 is not correctly synchronized.
> +     * #852523 (Cortex-A57) or #853709 (Cortex-A72).
> +     * I.e DACR32_EL2 is not correctly synchronized.
>        */
>       WRITE_SYSREG(n->arch.contextidr, CONTEXTIDR_EL1);
>       WRITE_SYSREG(n->arch.tpidr_el0, TPIDR_EL0);
>

[1] https://developer.arm.com/documentation/epm012079/11/

--
Julien Grall

[-- Attachment #2: Type: text/html, Size: 7454 bytes --]

Re: [PATCH] xen/arm: Document the erratum #853709 related to Cortex A72

[Julien Grall]

On 14/10/2020 12:06, Michal Orzel wrote:
> Hi Julien,
> 
> I agree. You can update the commit message.

Thanks. I have updated the commit message and committed it.

On a different topic, it looks like you are sending the e-mail with 
HTML. Would you mind to configure it to send plain text?

Cheers,

-- 
Julien Grall