[PATCH v2 0/3] xen/arm: Warn user on cpu errata 832075

[PATCH v2 0/3] xen/arm: Warn user on cpu errata 832075

[Bertrand Marquis]

This serie is a v2 after a discussion of [1] to introduce several
changes to handle properly the warning for Arm cpu errata 832075:
- use printk_once instead of warning add
- introduce a tainted type "Unsecure"

The last patch is adding the warning and flags A57 core affected as
unsupported.

[1] https://lists.xenproject.org/archives/html/xen-devel/2020-10/msg00896.html

Bertrand Marquis (3):
  xen/arm: use printk_once for errata warning prints
  xen: Add an unsecure Taint type
  xen/arm: Warn user on cpu errata 832075

 SUPPORT.md               |  1 +
 xen/arch/arm/cpuerrata.c | 23 +++++++++++++++--------
 xen/common/kernel.c      |  4 +++-
 xen/include/xen/lib.h    |  1 +
 4 files changed, 20 insertions(+), 9 deletions(-)

-- 
2.17.1

[PATCH v2 1/3] xen/arm: use printk_once for errata warning prints

[Bertrand Marquis]

Replace usage of warning_add by printk_once with a **** prefix and
suffix for errata related warnings.

This prevents the need for the assert which is not secure enough to
protect this print against wrong usage.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
 xen/arch/arm/cpuerrata.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 0c63dfa779..0430069a84 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -157,7 +157,6 @@ extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
 static int enable_smccc_arch_workaround_1(void *data)
 {
     struct arm_smccc_res res;
-    static bool warned = false;
     const struct arm_cpu_capabilities *entry = data;
 
     /*
@@ -182,13 +181,8 @@ static int enable_smccc_arch_workaround_1(void *data)
                                      "call ARM_SMCCC_ARCH_WORKAROUND_1");
 
 warn:
-    if ( !warned )
-    {
-        ASSERT(system_state < SYS_STATE_active);
-        warning_add("No support for ARM_SMCCC_ARCH_WORKAROUND_1.\n"
-                    "Please update your firmware.\n");
-        warned = true;
-    }
+    printk_once("**** No support for ARM_SMCCC_ARCH_WORKAROUND_1. ****\n"
+                "**** Please update your firmware.                ****\n");
 
     return 0;
 }
-- 
2.17.1

[PATCH v2 2/3] xen: Add an unsecure Taint type

[Bertrand Marquis]

Define a new Unsecure taint type to be used to signal a system tainted
due to an unsecure configuration or hardware feature/errata.

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
 xen/common/kernel.c   | 4 +++-
 xen/include/xen/lib.h | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/common/kernel.c b/xen/common/kernel.c
index c3a943f077..7a345ae45e 100644
--- a/xen/common/kernel.c
+++ b/xen/common/kernel.c
@@ -326,6 +326,7 @@ unsigned int tainted;
  *  'E' - An error (e.g. a machine check exceptions) has been injected.
  *  'H' - HVM forced emulation prefix is permitted.
  *  'M' - Machine had a machine check experience.
+ *  'U' - Platform is unsecure (usually due to an errata on the platform).
  *
  *      The string is overwritten by the next call to print_taint().
  */
@@ -333,7 +334,8 @@ char *print_tainted(char *str)
 {
     if ( tainted )
     {
-        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c",
+        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c",
+                 tainted & TAINT_MACHINE_UNSECURE ? 'U' : ' ',
                  tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
                  tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
                  tainted & TAINT_ERROR_INJECT ? 'E' : ' ',
diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
index 1983bd6b86..a9679c913d 100644
--- a/xen/include/xen/lib.h
+++ b/xen/include/xen/lib.h
@@ -193,6 +193,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
 #define TAINT_MACHINE_CHECK             (1u << 1)
 #define TAINT_ERROR_INJECT              (1u << 2)
 #define TAINT_HVM_FEP                   (1u << 3)
+#define TAINT_MACHINE_UNSECURE          (1u << 4)
 extern unsigned int tainted;
 #define TAINT_STRING_MAX_LEN            20
 extern char *print_tainted(char *str);
-- 
2.17.1

[PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Bertrand Marquis]

When a Cortex A57 processor is affected by CPU errata 832075, a guest
not implementing the workaround for it could deadlock the system.
Add a warning during boot informing the user that only trusted guests
should be executed on the system.
An equivalent warning is already given to the user by KVM on cores
affected by this errata.

Also taint the hypervisor as unsecure when this errata applies and
mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md

Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
---
 SUPPORT.md               |  1 +
 xen/arch/arm/cpuerrata.c | 13 +++++++++++++
 2 files changed, 14 insertions(+)

diff --git a/SUPPORT.md b/SUPPORT.md
index 5fbe5fc444..f7a3b046b0 100644
--- a/SUPPORT.md
+++ b/SUPPORT.md
@@ -38,6 +38,7 @@ supported in this document.
 ### ARM v8
 
     Status: Supported
+    Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075)
 
 ## Host hardware support
 
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 0430069a84..b35e8cd0b9 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -503,6 +503,19 @@ void check_local_cpu_errata(void)
 void __init enable_errata_workarounds(void)
 {
     enable_cpu_capabilities(arm_errata);
+
+#ifdef CONFIG_ARM64_ERRATUM_832075
+    if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) )
+    {
+        printk_once("**** This CPU is affected by the errata 832075. ****\n"
+                    "**** Guests without CPU erratum workarounds     ****\n"
+                    "**** can deadlock the system!                   ****\n"
+                    "**** Only trusted guests should be used.        ****\n");
+
+        /* Taint the machine has being insecure */
+        add_taint(TAINT_MACHINE_UNSECURE);
+    }
+#endif
 }
 
 static int cpu_errata_callback(struct notifier_block *nfb,
-- 
2.17.1

Re: [PATCH v2 1/3] xen/arm: use printk_once for errata warning prints

[Stefano Stabellini]

On Mon, 26 Oct 2020, Bertrand Marquis wrote:
> Replace usage of warning_add by printk_once with a **** prefix and
> suffix for errata related warnings.
> 
> This prevents the need for the assert which is not secure enough to
> protect this print against wrong usage.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/arch/arm/cpuerrata.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 0c63dfa779..0430069a84 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -157,7 +157,6 @@ extern char __smccc_workaround_1_smc_start[], __smccc_workaround_1_smc_end[];
>  static int enable_smccc_arch_workaround_1(void *data)
>  {
>      struct arm_smccc_res res;
> -    static bool warned = false;
>      const struct arm_cpu_capabilities *entry = data;
>  
>      /*
> @@ -182,13 +181,8 @@ static int enable_smccc_arch_workaround_1(void *data)
>                                       "call ARM_SMCCC_ARCH_WORKAROUND_1");
>  
>  warn:
> -    if ( !warned )
> -    {
> -        ASSERT(system_state < SYS_STATE_active);
> -        warning_add("No support for ARM_SMCCC_ARCH_WORKAROUND_1.\n"
> -                    "Please update your firmware.\n");
> -        warned = true;
> -    }
> +    printk_once("**** No support for ARM_SMCCC_ARCH_WORKAROUND_1. ****\n"
> +                "**** Please update your firmware.                ****\n");
>  
>      return 0;
>  }
> -- 
> 2.17.1
> 

Re: [PATCH v2 2/3] xen: Add an unsecure Taint type

[Stefano Stabellini]

On Mon, 26 Oct 2020, Bertrand Marquis wrote:
> Define a new Unsecure taint type to be used to signal a system tainted
> due to an unsecure configuration or hardware feature/errata.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
>  xen/common/kernel.c   | 4 +++-
>  xen/include/xen/lib.h | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/common/kernel.c b/xen/common/kernel.c
> index c3a943f077..7a345ae45e 100644
> --- a/xen/common/kernel.c
> +++ b/xen/common/kernel.c
> @@ -326,6 +326,7 @@ unsigned int tainted;
>   *  'E' - An error (e.g. a machine check exceptions) has been injected.
>   *  'H' - HVM forced emulation prefix is permitted.
>   *  'M' - Machine had a machine check experience.
> + *  'U' - Platform is unsecure (usually due to an errata on the platform).
>   *
>   *      The string is overwritten by the next call to print_taint().
>   */
> @@ -333,7 +334,8 @@ char *print_tainted(char *str)
>  {
>      if ( tainted )
>      {
> -        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c",
> +        snprintf(str, TAINT_STRING_MAX_LEN, "Tainted: %c%c%c%c%c",
> +                 tainted & TAINT_MACHINE_UNSECURE ? 'U' : ' ',
>                   tainted & TAINT_MACHINE_CHECK ? 'M' : ' ',
>                   tainted & TAINT_SYNC_CONSOLE ? 'C' : ' ',
>                   tainted & TAINT_ERROR_INJECT ? 'E' : ' ',
> diff --git a/xen/include/xen/lib.h b/xen/include/xen/lib.h
> index 1983bd6b86..a9679c913d 100644
> --- a/xen/include/xen/lib.h
> +++ b/xen/include/xen/lib.h
> @@ -193,6 +193,7 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c);
>  #define TAINT_MACHINE_CHECK             (1u << 1)
>  #define TAINT_ERROR_INJECT              (1u << 2)
>  #define TAINT_HVM_FEP                   (1u << 3)
> +#define TAINT_MACHINE_UNSECURE          (1u << 4)
>  extern unsigned int tainted;
>  #define TAINT_STRING_MAX_LEN            20
>  extern char *print_tainted(char *str);
> -- 
> 2.17.1
> 

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Stefano Stabellini]

On Mon, 26 Oct 2020, Bertrand Marquis wrote:
> When a Cortex A57 processor is affected by CPU errata 832075, a guest
> not implementing the workaround for it could deadlock the system.
> Add a warning during boot informing the user that only trusted guests
> should be executed on the system.
> An equivalent warning is already given to the user by KVM on cores
> affected by this errata.
> 
> Also taint the hypervisor as unsecure when this errata applies and
> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> ---
>  SUPPORT.md               |  1 +
>  xen/arch/arm/cpuerrata.c | 13 +++++++++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/SUPPORT.md b/SUPPORT.md
> index 5fbe5fc444..f7a3b046b0 100644
> --- a/SUPPORT.md
> +++ b/SUPPORT.md
> @@ -38,6 +38,7 @@ supported in this document.
>  ### ARM v8
>  
>      Status: Supported
> +    Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075)
>  
>  ## Host hardware support
>  
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 0430069a84..b35e8cd0b9 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -503,6 +503,19 @@ void check_local_cpu_errata(void)
>  void __init enable_errata_workarounds(void)
>  {
>      enable_cpu_capabilities(arm_errata);
> +
> +#ifdef CONFIG_ARM64_ERRATUM_832075
> +    if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) )
> +    {
> +        printk_once("**** This CPU is affected by the errata 832075. ****\n"
> +                    "**** Guests without CPU erratum workarounds     ****\n"
> +                    "**** can deadlock the system!                   ****\n"
> +                    "**** Only trusted guests should be used.        ****\n");

These can be on 2 lines, no need to be on 4 lines.


I know that Julien wrote about printing the warning from
enable_errata_workarounds but to me it looks more natural if we did it
from the .enable function specific to ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE.

That said, I don't feel strongly about it, I am fine either way. Julien,
do you have a preference?


Other than that, it is fine.


> +        /* Taint the machine has being insecure */
> +        add_taint(TAINT_MACHINE_UNSECURE);
> +    }
> +#endif

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Bertrand Marquis]

> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Mon, 26 Oct 2020, Bertrand Marquis wrote:
>> When a Cortex A57 processor is affected by CPU errata 832075, a guest
>> not implementing the workaround for it could deadlock the system.
>> Add a warning during boot informing the user that only trusted guests
>> should be executed on the system.
>> An equivalent warning is already given to the user by KVM on cores
>> affected by this errata.
>> 
>> Also taint the hypervisor as unsecure when this errata applies and
>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
>> 
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>> ---
>> SUPPORT.md               |  1 +
>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++
>> 2 files changed, 14 insertions(+)
>> 
>> diff --git a/SUPPORT.md b/SUPPORT.md
>> index 5fbe5fc444..f7a3b046b0 100644
>> --- a/SUPPORT.md
>> +++ b/SUPPORT.md
>> @@ -38,6 +38,7 @@ supported in this document.
>> ### ARM v8
>> 
>>     Status: Supported
>> +    Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075)
>> 
>> ## Host hardware support
>> 
>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
>> index 0430069a84..b35e8cd0b9 100644
>> --- a/xen/arch/arm/cpuerrata.c
>> +++ b/xen/arch/arm/cpuerrata.c
>> @@ -503,6 +503,19 @@ void check_local_cpu_errata(void)
>> void __init enable_errata_workarounds(void)
>> {
>>     enable_cpu_capabilities(arm_errata);
>> +
>> +#ifdef CONFIG_ARM64_ERRATUM_832075
>> +    if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) )
>> +    {
>> +        printk_once("**** This CPU is affected by the errata 832075. ****\n"
>> +                    "**** Guests without CPU erratum workarounds     ****\n"
>> +                    "**** can deadlock the system!                   ****\n"
>> +                    "**** Only trusted guests should be used.        ****\n");
> 
> These can be on 2 lines, no need to be on 4 lines.

I can fix that in a v3.

> 
> 
> I know that Julien wrote about printing the warning from
> enable_errata_workarounds but to me it looks more natural if we did it
> from the .enable function specific to ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE.

I have no preference either here but i kind of like this way because if we had more warnings
they would allow be at the same place.

I will wait for Julien answer on this before sending a v3 for this patch.

Cheers
Bertrand

> 
> That said, I don't feel strongly about it, I am fine either way. Julien,
> do you have a preference?
> 
> 
> Other than that, it is fine.
> 
> 
>> +        /* Taint the machine has being insecure */
>> +        add_taint(TAINT_MACHINE_UNSECURE);
>> +    }
>> +#endif

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[George Dunlap]

> On Oct 28, 2020, at 8:43 AM, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
> 
> 
> 
>> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote:
>> 
>> On Mon, 26 Oct 2020, Bertrand Marquis wrote:
>>> When a Cortex A57 processor is affected by CPU errata 832075, a guest
>>> not implementing the workaround for it could deadlock the system.
>>> Add a warning during boot informing the user that only trusted guests
>>> should be executed on the system.
>>> An equivalent warning is already given to the user by KVM on cores
>>> affected by this errata.
>>> 
>>> Also taint the hypervisor as unsecure when this errata applies and
>>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
>>> 
>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>> ---
>>> SUPPORT.md               |  1 +
>>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++
>>> 2 files changed, 14 insertions(+)
>>> 
>>> diff --git a/SUPPORT.md b/SUPPORT.md
>>> index 5fbe5fc444..f7a3b046b0 100644
>>> --- a/SUPPORT.md
>>> +++ b/SUPPORT.md
>>> @@ -38,6 +38,7 @@ supported in this document.
>>> ### ARM v8
>>> 
>>>    Status: Supported
>>> +    Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075)

I think this should be:

8<—

    Status, Cortex A57 r0p0-r1p1: Supported, not security supported

For the Cortex A57 r0p0 - r1p1, see Errata 832075.

—>8

 -George

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Bertrand Marquis]

Hi George,

> On 28 Oct 2020, at 09:43, George Dunlap <george.dunlap@citrix.com> wrote:
> 
> 
> 
>> On Oct 28, 2020, at 8:43 AM, Bertrand Marquis <Bertrand.Marquis@arm.com> wrote:
>> 
>> 
>> 
>>> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> 
>>> On Mon, 26 Oct 2020, Bertrand Marquis wrote:
>>>> When a Cortex A57 processor is affected by CPU errata 832075, a guest
>>>> not implementing the workaround for it could deadlock the system.
>>>> Add a warning during boot informing the user that only trusted guests
>>>> should be executed on the system.
>>>> An equivalent warning is already given to the user by KVM on cores
>>>> affected by this errata.
>>>> 
>>>> Also taint the hypervisor as unsecure when this errata applies and
>>>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
>>>> 
>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>>> ---
>>>> SUPPORT.md               |  1 +
>>>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++
>>>> 2 files changed, 14 insertions(+)
>>>> 
>>>> diff --git a/SUPPORT.md b/SUPPORT.md
>>>> index 5fbe5fc444..f7a3b046b0 100644
>>>> --- a/SUPPORT.md
>>>> +++ b/SUPPORT.md
>>>> @@ -38,6 +38,7 @@ supported in this document.
>>>> ### ARM v8
>>>> 
>>>>   Status: Supported
>>>> +    Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075)
> 
> I think this should be:
> 
> 8<—
> 
>    Status, Cortex A57 r0p0-r1p1: Supported, not security supported
> 
> For the Cortex A57 r0p0 - r1p1, see Errata 832075.
> 
> —>8
> 

Ok I will fix that.

Thanks for the review

Cheers
Bertrand

> -George

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Julien Grall]

Hi,

On 28/10/2020 08:43, Bertrand Marquis wrote:
> 
> 
>> On 27 Oct 2020, at 22:44, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>
>> On Mon, 26 Oct 2020, Bertrand Marquis wrote:
>>> When a Cortex A57 processor is affected by CPU errata 832075, a guest
>>> not implementing the workaround for it could deadlock the system.
>>> Add a warning during boot informing the user that only trusted guests
>>> should be executed on the system.
>>> An equivalent warning is already given to the user by KVM on cores
>>> affected by this errata.
>>>
>>> Also taint the hypervisor as unsecure when this errata applies and
>>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
>>>
>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>> ---
>>> SUPPORT.md               |  1 +
>>> xen/arch/arm/cpuerrata.c | 13 +++++++++++++
>>> 2 files changed, 14 insertions(+)
>>>
>>> diff --git a/SUPPORT.md b/SUPPORT.md
>>> index 5fbe5fc444..f7a3b046b0 100644
>>> --- a/SUPPORT.md
>>> +++ b/SUPPORT.md
>>> @@ -38,6 +38,7 @@ supported in this document.
>>> ### ARM v8
>>>
>>>      Status: Supported
>>> +    Status, Cortex A57 r0p0 - r1p2, not security supported (Errata 832075)
>>>
>>> ## Host hardware support
>>>
>>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
>>> index 0430069a84..b35e8cd0b9 100644
>>> --- a/xen/arch/arm/cpuerrata.c
>>> +++ b/xen/arch/arm/cpuerrata.c
>>> @@ -503,6 +503,19 @@ void check_local_cpu_errata(void)
>>> void __init enable_errata_workarounds(void)
>>> {
>>>      enable_cpu_capabilities(arm_errata);
>>> +
>>> +#ifdef CONFIG_ARM64_ERRATUM_832075
>>> +    if ( cpus_have_cap(ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE) )
>>> +    {
>>> +        printk_once("**** This CPU is affected by the errata 832075. ****\n"
>>> +                    "**** Guests without CPU erratum workarounds     ****\n"
>>> +                    "**** can deadlock the system!                   ****\n"
>>> +                    "**** Only trusted guests should be used.        ****\n");
>>
>> These can be on 2 lines, no need to be on 4 lines.
> 
> I can fix that in a v3.
> 
>>
>>
>> I know that Julien wrote about printing the warning from
>> enable_errata_workarounds but to me it looks more natural if we did it
>> from the .enable function specific to ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE.
> 
> I have no preference either here but i kind of like this way because if we had more warnings
> they would allow be at the same place.

So I add this placement in mind because the previous version was using 
warning_add() (It can't be called from non-init helper). As we are using 
printk_once() now, I don't really have a preference.

So I would stick with what you wrote.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Julien Grall]

Hi Bertrand,

On 26/10/2020 16:21, Bertrand Marquis wrote:
> When a Cortex A57 processor is affected by CPU errata 832075, a guest
> not implementing the workaround for it could deadlock the system.
> Add a warning during boot informing the user that only trusted guests
> should be executed on the system.
> An equivalent warning is already given to the user by KVM on cores
> affected by this errata.
> 
> Also taint the hypervisor as unsecure when this errata applies and
> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

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

If you don't need to resend the series, then I would be happy to fix the 
typo pointed out by George on commit.

Cheers,

-- 
Julien Grall

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Stefano Stabellini]

On Wed, 28 Oct 2020, Julien Grall wrote:
> Hi Bertrand,
> 
> On 26/10/2020 16:21, Bertrand Marquis wrote:
> > When a Cortex A57 processor is affected by CPU errata 832075, a guest
> > not implementing the workaround for it could deadlock the system.
> > Add a warning during boot informing the user that only trusted guests
> > should be executed on the system.
> > An equivalent warning is already given to the user by KVM on cores
> > affected by this errata.
> > 
> > Also taint the hypervisor as unsecure when this errata applies and
> > mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
> > 
> > Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> If you don't need to resend the series, then I would be happy to fix the typo
> pointed out by George on commit.

That's OK for me. Since you are at it, could you also condense the 4
lines of the warning into 2 lines on commit as well?

Thanks,

Stefano

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Bertrand Marquis]

Hi Julien,

> On 28 Oct 2020, at 18:39, Julien Grall <julien@xen.org> wrote:
> 
> Hi Bertrand,
> 
> On 26/10/2020 16:21, Bertrand Marquis wrote:
>> When a Cortex A57 processor is affected by CPU errata 832075, a guest
>> not implementing the workaround for it could deadlock the system.
>> Add a warning during boot informing the user that only trusted guests
>> should be executed on the system.
>> An equivalent warning is already given to the user by KVM on cores
>> affected by this errata.
>> Also taint the hypervisor as unsecure when this errata applies and
>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> Reviewed-by: Julien Grall <jgrall@amazon.com>

Thanks

> 
> If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit.

There is only the condensing from Stefano.
If you can handle that on commit to great but if you need me to send a v3 to make your life easier do not hesitate to tell me

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall
> 

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Stefano Stabellini]

On Thu, 29 Oct 2020, Bertrand Marquis wrote:
> Hi Julien,
> 
> > On 28 Oct 2020, at 18:39, Julien Grall <julien@xen.org> wrote:
> > 
> > Hi Bertrand,
> > 
> > On 26/10/2020 16:21, Bertrand Marquis wrote:
> >> When a Cortex A57 processor is affected by CPU errata 832075, a guest
> >> not implementing the workaround for it could deadlock the system.
> >> Add a warning during boot informing the user that only trusted guests
> >> should be executed on the system.
> >> An equivalent warning is already given to the user by KVM on cores
> >> affected by this errata.
> >> Also taint the hypervisor as unsecure when this errata applies and
> >> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
> >> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> > 
> > Reviewed-by: Julien Grall <jgrall@amazon.com>
> 
> Thanks
> 
> > 
> > If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit.
> 
> There is only the condensing from Stefano.
> If you can handle that on commit to great but if you need me to send a v3 to make your life easier do not hesitate to tell me

I have just done the committing

Re: [PATCH v2 3/3] xen/arm: Warn user on cpu errata 832075

[Bertrand Marquis]

> On 29 Oct 2020, at 23:32, Stefano Stabellini <sstabellini@kernel.org> wrote:
> 
> On Thu, 29 Oct 2020, Bertrand Marquis wrote:
>> Hi Julien,
>> 
>>> On 28 Oct 2020, at 18:39, Julien Grall <julien@xen.org> wrote:
>>> 
>>> Hi Bertrand,
>>> 
>>> On 26/10/2020 16:21, Bertrand Marquis wrote:
>>>> When a Cortex A57 processor is affected by CPU errata 832075, a guest
>>>> not implementing the workaround for it could deadlock the system.
>>>> Add a warning during boot informing the user that only trusted guests
>>>> should be executed on the system.
>>>> An equivalent warning is already given to the user by KVM on cores
>>>> affected by this errata.
>>>> Also taint the hypervisor as unsecure when this errata applies and
>>>> mention Cortex A57 r0p0 - r1p2 as not security supported in SUPPORT.md
>>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>> 
>>> Reviewed-by: Julien Grall <jgrall@amazon.com>
>> 
>> Thanks
>> 
>>> 
>>> If you don't need to resend the series, then I would be happy to fix the typo pointed out by George on commit.
>> 
>> There is only the condensing from Stefano.
>> If you can handle that on commit to great but if you need me to send a v3 to make your life easier do not hesitate to tell me
> 
> I have just done the committing

Thanks a lot :-)

Cheers
Bertrand