Message ID | 20201014100541.11687-1-michal.orzel@arm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xen/arm: Document the erratum #853709 related to Cortex A72 | expand |
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/
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 >
Hi Julien, I agree. You can update the commit message. Thanks for review. Michal
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,
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);
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(-)