[PATCH 0/5] xen/arm64: Branch predictor hardening (XSA-254 variant 2)

[PATCH 0/5] xen/arm64: Branch predictor hardening (XSA-254 variant 2)

[Julien Grall]

Hi all,

This series provides a framework for mitigating branch predictor hardening on
Arm64 on exception entry.

It also implements a dummy PSCI "VERSION" call as the hook for affected
Cortex-A CPUs. This will invalidate the predictor state with the latest
Arm Trusted Firmware patches which will appear at [1] and SoC vendors
with affected CPUs are strongly encouraged to update. We plan to switch to a
more efficient, special-purpose call when it is available and the PSCI spec
has been updated accordingly.

For more information about the impact of this issue and the software mitigations
for Arm processors, please see http://www.arm.com/security-update.

[1] https://github.com/ARM-software/arm-trusted-firmware/wiki/ARM-Trusted-Firmware-Security-Advisory-TFV-6

Julien Grall (5):
  xen/arm: Introduce enable callback to enable a capabilities on each
    online CPU
  xen/arm64: Add missing MIDR values for Cortex-A72, A73 and A75
  xen/arm: cpuerrata: Add MIDR_ALL_VERSIONS
  xen/arm64: Add skeleton to harden the branch predictor aliasing
    attacks
  xen/arm64: Implement branch predictor hardening for affected Cortex-A
    CPUs

 xen/arch/arm/Kconfig             |  20 ++++
 xen/arch/arm/arm64/Makefile      |   1 +
 xen/arch/arm/arm64/bpi.S         |  89 +++++++++++++++++
 xen/arch/arm/cpuerrata.c         | 203 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/cpufeature.c        |  29 ++++++
 xen/arch/arm/setup.c             |   1 +
 xen/arch/arm/traps.c             |   5 +-
 xen/include/asm-arm/cpuerrata.h  |   2 +
 xen/include/asm-arm/cpufeature.h |   6 +-
 xen/include/asm-arm/processor.h  |  11 ++-
 10 files changed, 363 insertions(+), 4 deletions(-)
 create mode 100644 xen/arch/arm/arm64/bpi.S

-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Julien Grall]

Once Xen knows what features/workarounds present on the platform, it
might be necessary to configure each online CPU.

Introduce a new callback "enable" that will be called on each online CPU to
configure the "capability".

The code is based on Linux v4.14 (where cpufeature.c comes from), the
explanation of why using stop_machine_run is kept as we have similar
problem in the future.

Lastly introduce enable_errata_workaround that will be called once CPUs
have booted and before the hardware domain is created.

This is part of XSA-254.

Signed-of-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/cpuerrata.c         |  6 ++++++
 xen/arch/arm/cpufeature.c        | 29 +++++++++++++++++++++++++++++
 xen/arch/arm/setup.c             |  1 +
 xen/include/asm-arm/cpuerrata.h  |  1 +
 xen/include/asm-arm/cpufeature.h |  3 +++
 5 files changed, 40 insertions(+)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index fe9e9facbe..772587c05a 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -64,6 +64,12 @@ void check_local_cpu_errata(void)
 {
     update_cpu_capabilities(arm_errata, "enabled workaround for");
 }
+
+void __init enable_errata_workarounds(void)
+{
+    enable_cpu_capabilities(arm_errata);
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 479c9fb011..525b45e22f 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -19,6 +19,7 @@
 #include <xen/types.h>
 #include <xen/init.h>
 #include <xen/smp.h>
+#include <xen/stop_machine.h>
 #include <asm/cpufeature.h>
 
 DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
@@ -40,6 +41,34 @@ void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
 }
 
 /*
+ * Run through the enabled capabilities and enable() it on all active
+ * CPUs.
+ */
+void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
+{
+    for ( ; caps->matches; caps++ )
+    {
+        if ( !cpus_have_cap(caps->capability) )
+            continue;
+
+        if ( caps->enable )
+        {
+            int ret;
+
+            /*
+             * Use stop_machine_run() as it schedules the work allowing
+             * us to modify PSTATE, instead of on_each_cpu() which uses
+             * an IPI, giving us a PSTATE that disappears when we
+             * return.
+             */
+            ret = stop_machine_run(caps->enable, (void *)caps, NR_CPUS);
+            /* stop_machine_run should never fail at this stage of the boot. */
+            BUG_ON(ret);
+        }
+    }
+}
+
+/*
  * Local variables:
  * mode: C
  * c-file-style: "BSD"
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 16a3b1be8e..032a6a882d 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -849,6 +849,7 @@ void __init start_xen(unsigned long boot_phys_offset,
      * stop_machine (tasklets initialized via an initcall).
      */
     apply_alternatives_all();
+    enable_errata_workarounds();
 
     /* Create initial domain 0. */
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 8b158429c7..7de68361ff 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -5,6 +5,7 @@
 #include <asm/alternative.h>
 
 void check_local_cpu_errata(void);
+void enable_errata_workarounds(void);
 
 #ifdef CONFIG_HAS_ALTERNATIVE
 
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index f00b6dbd39..21c65e198c 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -74,6 +74,7 @@ struct arm_cpu_capabilities {
     const char *desc;
     u16 capability;
     bool (*matches)(const struct arm_cpu_capabilities *);
+    int (*enable)(void *); /* Called on every active CPUs */
     union {
         struct {    /* To be used for eratum handling only */
             u32 midr_model;
@@ -85,6 +86,8 @@ struct arm_cpu_capabilities {
 void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
                              const char *info);
 
+void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
+
 #endif /* __ASSEMBLY__ */
 
 #endif
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Stefano Stabellini]

On Tue, 16 Jan 2018, Julien Grall wrote:
> Once Xen knows what features/workarounds present on the platform, it
> might be necessary to configure each online CPU.
> 
> Introduce a new callback "enable" that will be called on each online CPU to
> configure the "capability".
> 
> The code is based on Linux v4.14 (where cpufeature.c comes from), the
> explanation of why using stop_machine_run is kept as we have similar
> problem in the future.
> 
> Lastly introduce enable_errata_workaround that will be called once CPUs
> have booted and before the hardware domain is created.
> 
> This is part of XSA-254.
> 
> Signed-of-by: Julien Grall <julien.grall@linaro.org>

If you took the code from Linux, you need to add the original
Signed-off-by from the Linux commit. Aside from that:

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


> ---
>  xen/arch/arm/cpuerrata.c         |  6 ++++++
>  xen/arch/arm/cpufeature.c        | 29 +++++++++++++++++++++++++++++
>  xen/arch/arm/setup.c             |  1 +
>  xen/include/asm-arm/cpuerrata.h  |  1 +
>  xen/include/asm-arm/cpufeature.h |  3 +++
>  5 files changed, 40 insertions(+)
> 
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index fe9e9facbe..772587c05a 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -64,6 +64,12 @@ void check_local_cpu_errata(void)
>  {
>      update_cpu_capabilities(arm_errata, "enabled workaround for");
>  }
> +
> +void __init enable_errata_workarounds(void)
> +{
> +    enable_cpu_capabilities(arm_errata);
> +}
> +
>  /*
>   * Local variables:
>   * mode: C
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 479c9fb011..525b45e22f 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -19,6 +19,7 @@
>  #include <xen/types.h>
>  #include <xen/init.h>
>  #include <xen/smp.h>
> +#include <xen/stop_machine.h>
>  #include <asm/cpufeature.h>
>  
>  DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
> @@ -40,6 +41,34 @@ void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>  }
>  
>  /*
> + * Run through the enabled capabilities and enable() it on all active
> + * CPUs.
> + */
> +void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
> +{
> +    for ( ; caps->matches; caps++ )
> +    {
> +        if ( !cpus_have_cap(caps->capability) )
> +            continue;
> +
> +        if ( caps->enable )
> +        {
> +            int ret;
> +
> +            /*
> +             * Use stop_machine_run() as it schedules the work allowing
> +             * us to modify PSTATE, instead of on_each_cpu() which uses
> +             * an IPI, giving us a PSTATE that disappears when we
> +             * return.
> +             */
> +            ret = stop_machine_run(caps->enable, (void *)caps, NR_CPUS);
> +            /* stop_machine_run should never fail at this stage of the boot. */
> +            BUG_ON(ret);
> +        }
> +    }
> +}
> +
> +/*
>   * Local variables:
>   * mode: C
>   * c-file-style: "BSD"
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 16a3b1be8e..032a6a882d 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -849,6 +849,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>       * stop_machine (tasklets initialized via an initcall).
>       */
>      apply_alternatives_all();
> +    enable_errata_workarounds();
>  
>      /* Create initial domain 0. */
>      /* The vGIC for DOM0 is exactly emulating the hardware GIC */
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index 8b158429c7..7de68361ff 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -5,6 +5,7 @@
>  #include <asm/alternative.h>
>  
>  void check_local_cpu_errata(void);
> +void enable_errata_workarounds(void);
>  
>  #ifdef CONFIG_HAS_ALTERNATIVE
>  
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index f00b6dbd39..21c65e198c 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -74,6 +74,7 @@ struct arm_cpu_capabilities {
>      const char *desc;
>      u16 capability;
>      bool (*matches)(const struct arm_cpu_capabilities *);
> +    int (*enable)(void *); /* Called on every active CPUs */
>      union {
>          struct {    /* To be used for eratum handling only */
>              u32 midr_model;
> @@ -85,6 +86,8 @@ struct arm_cpu_capabilities {
>  void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>                               const char *info);
>  
> +void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Julien Grall]

Hi Stefano,

On 16/01/18 23:55, Stefano Stabellini wrote:
> On Tue, 16 Jan 2018, Julien Grall wrote:
>> Once Xen knows what features/workarounds present on the platform, it
>> might be necessary to configure each online CPU.
>>
>> Introduce a new callback "enable" that will be called on each online CPU to
>> configure the "capability".
>>
>> The code is based on Linux v4.14 (where cpufeature.c comes from), the
>> explanation of why using stop_machine_run is kept as we have similar
>> problem in the future.
>>
>> Lastly introduce enable_errata_workaround that will be called once CPUs
>> have booted and before the hardware domain is created.
>>
>> This is part of XSA-254.
>>
>> Signed-of-by: Julien Grall <julien.grall@linaro.org>
> 
> If you took the code from Linux, you need to add the original
> Signed-off-by from the Linux commit. Aside from that:

There are multiple commits touching this function. So I followed what we 
did in similar situation. By that I mean, mentioning the code was taken 
from Linux and not gathered the signed-off-by.

If you really want, I can gather all the signed-off-by of the commit 
touching this function.

Cheers,

> 
> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> 
> 
>> ---
>>   xen/arch/arm/cpuerrata.c         |  6 ++++++
>>   xen/arch/arm/cpufeature.c        | 29 +++++++++++++++++++++++++++++
>>   xen/arch/arm/setup.c             |  1 +
>>   xen/include/asm-arm/cpuerrata.h  |  1 +
>>   xen/include/asm-arm/cpufeature.h |  3 +++
>>   5 files changed, 40 insertions(+)
>>
>> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
>> index fe9e9facbe..772587c05a 100644
>> --- a/xen/arch/arm/cpuerrata.c
>> +++ b/xen/arch/arm/cpuerrata.c
>> @@ -64,6 +64,12 @@ void check_local_cpu_errata(void)
>>   {
>>       update_cpu_capabilities(arm_errata, "enabled workaround for");
>>   }
>> +
>> +void __init enable_errata_workarounds(void)
>> +{
>> +    enable_cpu_capabilities(arm_errata);
>> +}
>> +
>>   /*
>>    * Local variables:
>>    * mode: C
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index 479c9fb011..525b45e22f 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -19,6 +19,7 @@
>>   #include <xen/types.h>
>>   #include <xen/init.h>
>>   #include <xen/smp.h>
>> +#include <xen/stop_machine.h>
>>   #include <asm/cpufeature.h>
>>   
>>   DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>> @@ -40,6 +41,34 @@ void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>>   }
>>   
>>   /*
>> + * Run through the enabled capabilities and enable() it on all active
>> + * CPUs.
>> + */
>> +void __init enable_cpu_capabilities(const struct arm_cpu_capabilities *caps)
>> +{
>> +    for ( ; caps->matches; caps++ )
>> +    {
>> +        if ( !cpus_have_cap(caps->capability) )
>> +            continue;
>> +
>> +        if ( caps->enable )
>> +        {
>> +            int ret;
>> +
>> +            /*
>> +             * Use stop_machine_run() as it schedules the work allowing
>> +             * us to modify PSTATE, instead of on_each_cpu() which uses
>> +             * an IPI, giving us a PSTATE that disappears when we
>> +             * return.
>> +             */
>> +            ret = stop_machine_run(caps->enable, (void *)caps, NR_CPUS);
>> +            /* stop_machine_run should never fail at this stage of the boot. */
>> +            BUG_ON(ret);
>> +        }
>> +    }
>> +}
>> +
>> +/*
>>    * Local variables:
>>    * mode: C
>>    * c-file-style: "BSD"
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 16a3b1be8e..032a6a882d 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -849,6 +849,7 @@ void __init start_xen(unsigned long boot_phys_offset,
>>        * stop_machine (tasklets initialized via an initcall).
>>        */
>>       apply_alternatives_all();
>> +    enable_errata_workarounds();
>>   
>>       /* Create initial domain 0. */
>>       /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
>> index 8b158429c7..7de68361ff 100644
>> --- a/xen/include/asm-arm/cpuerrata.h
>> +++ b/xen/include/asm-arm/cpuerrata.h
>> @@ -5,6 +5,7 @@
>>   #include <asm/alternative.h>
>>   
>>   void check_local_cpu_errata(void);
>> +void enable_errata_workarounds(void);
>>   
>>   #ifdef CONFIG_HAS_ALTERNATIVE
>>   
>> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
>> index f00b6dbd39..21c65e198c 100644
>> --- a/xen/include/asm-arm/cpufeature.h
>> +++ b/xen/include/asm-arm/cpufeature.h
>> @@ -74,6 +74,7 @@ struct arm_cpu_capabilities {
>>       const char *desc;
>>       u16 capability;
>>       bool (*matches)(const struct arm_cpu_capabilities *);
>> +    int (*enable)(void *); /* Called on every active CPUs */
>>       union {
>>           struct {    /* To be used for eratum handling only */
>>               u32 midr_model;
>> @@ -85,6 +86,8 @@ struct arm_cpu_capabilities {
>>   void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>>                                const char *info);
>>   
>> +void enable_cpu_capabilities(const struct arm_cpu_capabilities *caps);
>> +
>>   #endif /* __ASSEMBLY__ */
>>   
>>   #endif
>> -- 
>> 2.11.0
>>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Lars Kurth]

[-- Attachment #1.1: Type: text/plain, Size: 1476 bytes --]



> On 17 Jan 2018, at 10:31, Julien Grall <julien.grall@linaro.org> wrote:
> 
> Hi Stefano,
> 
> On 16/01/18 23:55, Stefano Stabellini wrote:
>> On Tue, 16 Jan 2018, Julien Grall wrote:
>>> Once Xen knows what features/workarounds present on the platform, it
>>> might be necessary to configure each online CPU.
>>> 
>>> Introduce a new callback "enable" that will be called on each online CPU to
>>> configure the "capability".
>>> 
>>> The code is based on Linux v4.14 (where cpufeature.c comes from), the
>>> explanation of why using stop_machine_run is kept as we have similar
>>> problem in the future.
>>> 
>>> Lastly introduce enable_errata_workaround that will be called once CPUs
>>> have booted and before the hardware domain is created.
>>> 
>>> This is part of XSA-254.
>>> 
>>> Signed-of-by: Julien Grall <julien.grall@linaro.org>
>> If you took the code from Linux, you need to add the original
>> Signed-off-by from the Linux commit. Aside from that:
> 
> There are multiple commits touching this function. So I followed what we did in similar situation. By that I mean, mentioning the code was taken from Linux and not gathered the signed-off-by.
> 
> If you really want, I can gather all the signed-off-by of the commit touching this function.

If there are a lot of then, you may want to consider adding a README.source file instead. There are a few examples in tree and they are also mentioned in CONTRIBUTING files
Lars


[-- Attachment #1.2: Type: text/html, Size: 5917 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Julien Grall]

Hi Lars,

On 17/01/18 12:23, Lars Kurth wrote:
> 
> 
>> On 17 Jan 2018, at 10:31, Julien Grall <julien.grall@linaro.org 
>> <mailto:julien.grall@linaro.org>> wrote:
>>
>> Hi Stefano,
>>
>> On 16/01/18 23:55, Stefano Stabellini wrote:
>>> On Tue, 16 Jan 2018, Julien Grall wrote:
>>>> Once Xen knows what features/workarounds present on the platform, it
>>>> might be necessary to configure each online CPU.
>>>>
>>>> Introduce a new callback "enable" that will be called on each online 
>>>> CPU to
>>>> configure the "capability".
>>>>
>>>> The code is based on Linux v4.14 (where cpufeature.c comes from), the
>>>> explanation of why using stop_machine_run is kept as we have similar
>>>> problem in the future.
>>>>
>>>> Lastly introduce enable_errata_workaround that will be called once CPUs
>>>> have booted and before the hardware domain is created.
>>>>
>>>> This is part of XSA-254.
>>>>
>>>> Signed-of-by: Julien Grall <julien.grall@linaro.org 
>>>> <mailto:julien.grall@linaro.org>>
>>> If you took the code from Linux, you need to add the original
>>> Signed-off-by from the Linux commit. Aside from that:
>>
>> There are multiple commits touching this function. So I followed what 
>> we did in similar situation. By that I mean, mentioning the code was 
>> taken from Linux and not gathered the signed-off-by.
>>
>> If you really want, I can gather all the signed-off-by of the commit 
>> touching this function.
> 
> If there are a lot of then, you may want to consider adding 
> a README.source file instead. There are a few examples in tree and they 
> are also mentioned in CONTRIBUTING files

I am not sure to understand your suggestion here. I spotted only one 
CONTRIBUTING file and it only list the license.

Regarding README.source, this is covering file and contain the same 
mention as in the commit message. As this is a single function. Isn't 
the commit message enough?

Lastly, we do have quite a bit of code in Xen coming from Linux (or 
other project). A lot of them are not listed in 
README.source/CONTRIBUTING. But only mention in the commit message (not 
necessarily with Signed-off-by tag).

So I am quite interested to know what is the normal procedure here.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Lars Kurth]

[-- Attachment #1.1: Type: text/plain, Size: 2293 bytes --]

Hi Julien,

> On 17 Jan 2018, at 12:31, Julien Grall <julien.grall@linaro.org> wrote:
> 
>>>>> 
>>>> If you took the code from Linux, you need to add the original
>>>> Signed-off-by from the Linux commit. Aside from that:
>>> 
>>> There are multiple commits touching this function. So I followed what we did in similar situation. By that I mean, mentioning the code was taken from Linux and not gathered the signed-off-by.
>>> 
>>> If you really want, I can gather all the signed-off-by of the commit touching this function.
>> If there are a lot of then, you may want to consider adding a README.source file instead. There are a few examples in tree and they are also mentioned in CONTRIBUTING files
> 
> I am not sure to understand your suggestion here. I spotted only one CONTRIBUTING file and it only list the license.

I was suggesting to create a README.source in the xen arm tree seomwhere where you can list imports.

> Regarding README.source, this is covering file and contain the same mention as in the commit message. As this is a single function. Isn't the commit message enough?

From a legal viewpoint it is enough.
The reason why we created the README.source file is because it is very easy to miss code imports when they are mentioned in commit messages.

Normally this isn't a problem: only if we ever have to relicense the code or if someone does code archeology 
When we relicensed the ACPI builder, it created all sorts of problems as outlined in https://www.slideshare.net/xen_com_mgr/ossa17-mixed-license-foss-projects <https://www.slideshare.net/xen_com_mgr/ossa17-mixed-license-foss-projects>

> Lastly, we do have quite a bit of code in Xen coming from Linux (or other project). A lot of them are not listed in README.source/CONTRIBUTING. But only mention in the commit message (not necessarily with Signed-off-by tag).

That is true: which is why I have started fixing these, whenever I found them and moved such information to README.source. 

> So I am quite interested to know what is the normal procedure here.

I think:
* Doing this via Signed-off-by tag is sufficient for a one-off-import
* I would prefer if people added import related info README.source files (and also in the commit message)

Does this make sense?

Lars

[-- Attachment #1.2: Type: text/html, Size: 3625 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Stefano Stabellini]

On Wed, 17 Jan 2018, Lars Kurth wrote:
>       Regarding README.source, this is covering file and contain the same mention as in the commit message. As this is a single function. Isn't the commit message
>       enough?
> 
> 
> From a legal viewpoint it is enough.

If that is enough from a legal viewpoint, then it is enough for me.

However, from a legal viewpoint, I thought we needed to explicitly
mention all the original signed-off-bys because Julien is not actually
the copyright holder for that function, hence, we need to add the
signed-off-bys of all the missing copyright holders.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1289 bytes --]

On Wed, 17 Jan 2018, Stefano Stabellini wrote:
> On Wed, 17 Jan 2018, Lars Kurth wrote:
> >       Regarding README.source, this is covering file and contain the same mention as in the commit message. As this is a single function. Isn't the commit message
> >       enough?
> > 
> > 
> > From a legal viewpoint it is enough.
> 
> If that is enough from a legal viewpoint, then it is enough for me.
> 
> However, from a legal viewpoint, I thought we needed to explicitly
> mention all the original signed-off-bys because Julien is not actually
> the copyright holder for that function, hence, we need to add the
> signed-off-bys of all the missing copyright holders.

Actually, reading again the Developer’s Certificate of Origin, it
states:

"The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file"

so I think Lars is right. In that case, there is no need to resubmit
this series, I'll commit to staging as is. If tests go well, I'll
backport it to the stable trees.

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Julien Grall]

(+ Security team)

Hi Stefano,

On 17/01/18 21:47, Stefano Stabellini wrote:
> On Wed, 17 Jan 2018, Stefano Stabellini wrote:
>> On Wed, 17 Jan 2018, Lars Kurth wrote:
>>>        Regarding README.source, this is covering file and contain the same mention as in the commit message. As this is a single function. Isn't the commit message
>>>        enough?
>>>
>>>
>>>  From a legal viewpoint it is enough.
>>
>> If that is enough from a legal viewpoint, then it is enough for me.
>>
>> However, from a legal viewpoint, I thought we needed to explicitly
>> mention all the original signed-off-bys because Julien is not actually
>> the copyright holder for that function, hence, we need to add the
>> signed-off-bys of all the missing copyright holders.
> 
> Actually, reading again the Developer’s Certificate of Origin, it
> states:
> 
> "The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file"
> 
> so I think Lars is right. In that case, there is no need to resubmit
> this series, I'll commit to staging as is. If tests go well, I'll
> backport it to the stable trees.
Thank you! I have created branches with patches backported up to Xen 
4.8. With minor changes:

    - Xen 4.10: No changes
    - Xen 4.9:
	* minor conflict in some files
	* compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
    - Xen 4.8:
	* conflict in some files (one medium as the number of "features" is 
different)
	* compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
	
The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX 
is the version of Xen.

Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure 
and will require backport. The only difficulty here should be finding 
the list of commits required.

Also, we probably want to update the XSA pointing to the patches. So if 
someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?

Cheers,

[1] https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3020 bytes --]

On Thu, 18 Jan 2018, Julien Grall wrote:
> (+ Security team)
> 
> Hi Stefano,
> 
> On 17/01/18 21:47, Stefano Stabellini wrote:
> > On Wed, 17 Jan 2018, Stefano Stabellini wrote:
> > > On Wed, 17 Jan 2018, Lars Kurth wrote:
> > > >        Regarding README.source, this is covering file and contain the
> > > > same mention as in the commit message. As this is a single function.
> > > > Isn't the commit message
> > > >        enough?
> > > > 
> > > > 
> > > >  From a legal viewpoint it is enough.
> > > 
> > > If that is enough from a legal viewpoint, then it is enough for me.
> > > 
> > > However, from a legal viewpoint, I thought we needed to explicitly
> > > mention all the original signed-off-bys because Julien is not actually
> > > the copyright holder for that function, hence, we need to add the
> > > signed-off-bys of all the missing copyright holders.
> > 
> > Actually, reading again the Developer’s Certificate of Origin, it
> > states:
> > 
> > "The contribution is based upon previous work that, to the best of my
> > knowledge, is covered under an appropriate open source license and I have
> > the right under that license to submit that work with modifications, whether
> > created in whole or in part by me, under the same open source license
> > (unless I am permitted to submit under a different license), as indicated in
> > the file"
> > 
> > so I think Lars is right. In that case, there is no need to resubmit
> > this series, I'll commit to staging as is. If tests go well, I'll
> > backport it to the stable trees.
> Thank you! I have created branches with patches backported up to Xen 4.8. With
> minor changes:
> 
>    - Xen 4.10: No changes
>    - Xen 4.9:
> 	* minor conflict in some files
> 	* compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>    - Xen 4.8:
> 	* conflict in some files (one medium as the number of "features" is
> different)
> 	* compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
> 	
> The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX is the
> version of Xen.
> 
> Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure and will
> require backport. The only difficulty here should be finding the list of
> commits required.
> 
> Also, we probably want to update the XSA pointing to the patches. So if
> someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?

Thank you, Julien. Ideally, I would like to do the backports after
OSSTest passes its tests on those changes. In practice, for the sake of
mitigating SP2 as soon as possible, tomorrow (Friday) I might do the
backports anyway, if OSSTest is still behind on other problems.

I don't think that backporting cpufeature/cpuerrata to 4.7 should be too
convoluted, I'll give that a go as well.

Once done, I'll provide the list of commits to the xen security list so
that the XSA advisory can be updated appropriately.

Cheers,

Stefano


> Cheers,
> 
> [1] https://xenbits.xen.org/git-http/people/julieng/xen-unstable.git

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Jan Beulich]

>>> On 18.01.18 at 21:28, <sstabellini@kernel.org> wrote:
> Thank you, Julien. Ideally, I would like to do the backports after
> OSSTest passes its tests on those changes. In practice, for the sake of
> mitigating SP2 as soon as possible, tomorrow (Friday) I might do the
> backports anyway, if OSSTest is still behind on other problems.

Please avoid touching the 4.8 tree for the moment, until 4.8.3 has
been tagged.

Thanks, Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Stefano Stabellini]

On Fri, 19 Jan 2018, Jan Beulich wrote:
> >>> On 18.01.18 at 21:28, <sstabellini@kernel.org> wrote:
> > Thank you, Julien. Ideally, I would like to do the backports after
> > OSSTest passes its tests on those changes. In practice, for the sake of
> > mitigating SP2 as soon as possible, tomorrow (Friday) I might do the
> > backports anyway, if OSSTest is still behind on other problems.
> 
> Please avoid touching the 4.8 tree for the moment, until 4.8.3 has
> been tagged.

Many thanks for the head's up, I'll wait.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3558 bytes --]

On Thu, 18 Jan 2018, Julien Grall wrote:
> (+ Security team)
> 
> Hi Stefano,
> 
> On 17/01/18 21:47, Stefano Stabellini wrote:
> > On Wed, 17 Jan 2018, Stefano Stabellini wrote:
> > > On Wed, 17 Jan 2018, Lars Kurth wrote:
> > > >        Regarding README.source, this is covering file and contain the
> > > > same mention as in the commit message. As this is a single function.
> > > > Isn't the commit message
> > > >        enough?
> > > > 
> > > > 
> > > >  From a legal viewpoint it is enough.
> > > 
> > > If that is enough from a legal viewpoint, then it is enough for me.
> > > 
> > > However, from a legal viewpoint, I thought we needed to explicitly
> > > mention all the original signed-off-bys because Julien is not actually
> > > the copyright holder for that function, hence, we need to add the
> > > signed-off-bys of all the missing copyright holders.
> > 
> > Actually, reading again the Developer’s Certificate of Origin, it
> > states:
> > 
> > "The contribution is based upon previous work that, to the best of my
> > knowledge, is covered under an appropriate open source license and I have
> > the right under that license to submit that work with modifications, whether
> > created in whole or in part by me, under the same open source license
> > (unless I am permitted to submit under a different license), as indicated in
> > the file"
> > 
> > so I think Lars is right. In that case, there is no need to resubmit
> > this series, I'll commit to staging as is. If tests go well, I'll
> > backport it to the stable trees.
> Thank you! I have created branches with patches backported up to Xen 4.8. With
> minor changes:
> 
>    - Xen 4.10: No changes
>    - Xen 4.9:
> 	* minor conflict in some files
> 	* compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>    - Xen 4.8:
> 	* conflict in some files (one medium as the number of "features" is
> different)
> 	* compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
> 	
> The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX is the
> version of Xen.
> 
> Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure and will
> require backport. The only difficulty here should be finding the list of
> commits required.
> 
> Also, we probably want to update the XSA pointing to the patches. So if
> someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?

These are the commits for the XSA 254 mitigation for the arm64
architecture:

staging-4.10
b829d42829c1ff626a02756acae4dd482fc20c9a
0f7a4faafb2d79920cc63457cfca3e03990af4cc
d1f4283a1d8405a480b4121e1efcfaec8bbdbffa
cae6e1572f39a1906be0fc3bdaf49fe514c6a9c0
928112900e5b4a92ccebb2eea11665fd76aa0f0d
728fadb586a2a14a244dabd70463bcc1654ecc85

staging-4.9
2ec7ccbffc6b788f65e55498e4347c1ee3a44b01
50450c1f33dc72f2138a671d738934f796be3318
3790833ef16b95653424ec9b145e460ec1a56d16
fba48eff18c02d716c95b92df804a755620be82e
9f79e8d846e8413c828f5fc7cc6ac733728dff00
a2567d6b54b7b187ecc0165021b6dd07dafaf06a

staging-4.8
946dd2eefae2faeecbeb9662e66935c8070f64f5
85990bf53addcdb0ce8e458a3d8fad199710ac59
cf0b584c8c5030588bc47a3614ad860af7482c53
44139fed7c794eb4e47a9bb93061e325bd57fe8c
6f6786ef0d7f7025860d360f6b1267193ffd1b27

For staging-4.7, I made the backports and tested them as well. They look
correct. However, given that it was more complex than initially though,
I would appreciate if you could give it a look as well (I haven't pushed
it staging-4.7 yet):

  git://xenbits.xen.org/people/sstabellini/xen-unstable.git staging-4.7-xsa254

Thanks!

- Stefano

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Julien Grall]

Hi Stefano,

On 24 January 2018 at 22:14, Stefano Stabellini <sstabellini@kernel.org> wrote:
> On Thu, 18 Jan 2018, Julien Grall wrote:
>> (+ Security team)
>>
>> Hi Stefano,
>>
>> On 17/01/18 21:47, Stefano Stabellini wrote:
>> > On Wed, 17 Jan 2018, Stefano Stabellini wrote:
>> > > On Wed, 17 Jan 2018, Lars Kurth wrote:
>> > > >        Regarding README.source, this is covering file and contain the
>> > > > same mention as in the commit message. As this is a single function.
>> > > > Isn't the commit message
>> > > >        enough?
>> > > >
>> > > >
>> > > >  From a legal viewpoint it is enough.
>> > >
>> > > If that is enough from a legal viewpoint, then it is enough for me.
>> > >
>> > > However, from a legal viewpoint, I thought we needed to explicitly
>> > > mention all the original signed-off-bys because Julien is not actually
>> > > the copyright holder for that function, hence, we need to add the
>> > > signed-off-bys of all the missing copyright holders.
>> >
>> > Actually, reading again the Developer’s Certificate of Origin, it
>> > states:
>> >
>> > "The contribution is based upon previous work that, to the best of my
>> > knowledge, is covered under an appropriate open source license and I have
>> > the right under that license to submit that work with modifications, whether
>> > created in whole or in part by me, under the same open source license
>> > (unless I am permitted to submit under a different license), as indicated in
>> > the file"
>> >
>> > so I think Lars is right. In that case, there is no need to resubmit
>> > this series, I'll commit to staging as is. If tests go well, I'll
>> > backport it to the stable trees.
>> Thank you! I have created branches with patches backported up to Xen 4.8. With
>> minor changes:
>>
>>    - Xen 4.10: No changes
>>    - Xen 4.9:
>>       * minor conflict in some files
>>       * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>>    - Xen 4.8:
>>       * conflict in some files (one medium as the number of "features" is
>> different)
>>       * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>>
>> The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX is the
>> version of Xen.
>>
>> Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure and will
>> require backport. The only difficulty here should be finding the list of
>> commits required.
>>
>> Also, we probably want to update the XSA pointing to the patches. So if
>> someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?
>
> These are the commits for the XSA 254 mitigation for the arm64
> architecture:
>
> staging-4.10
> b829d42829c1ff626a02756acae4dd482fc20c9a
> 0f7a4faafb2d79920cc63457cfca3e03990af4cc
> d1f4283a1d8405a480b4121e1efcfaec8bbdbffa
> cae6e1572f39a1906be0fc3bdaf49fe514c6a9c0
> 928112900e5b4a92ccebb2eea11665fd76aa0f0d
> 728fadb586a2a14a244dabd70463bcc1654ecc85
>
> staging-4.9
> 2ec7ccbffc6b788f65e55498e4347c1ee3a44b01
> 50450c1f33dc72f2138a671d738934f796be3318
> 3790833ef16b95653424ec9b145e460ec1a56d16
> fba48eff18c02d716c95b92df804a755620be82e
> 9f79e8d846e8413c828f5fc7cc6ac733728dff00
> a2567d6b54b7b187ecc0165021b6dd07dafaf06a
>
> staging-4.8
> 946dd2eefae2faeecbeb9662e66935c8070f64f5
> 85990bf53addcdb0ce8e458a3d8fad199710ac59
> cf0b584c8c5030588bc47a3614ad860af7482c53
> 44139fed7c794eb4e47a9bb93061e325bd57fe8c
> 6f6786ef0d7f7025860d360f6b1267193ffd1b27

Something looks quite odd. The commit message have two cherry-pick commit ID.

Why didn't you just merged the branches I provided?

>
> For staging-4.7, I made the backports and tested them as well. They look
> correct. However, given that it was more complex than initially though,
> I would appreciate if you could give it a look as well (I haven't pushed
> it staging-4.7 yet):
>
>   git://xenbits.xen.org/people/sstabellini/xen-unstable.git staging-4.7-xsa254

I will have a look.

Cheers,

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4486 bytes --]

On Wed, 24 Jan 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 24 January 2018 at 22:14, Stefano Stabellini <sstabellini@kernel.org> wrote:
> > On Thu, 18 Jan 2018, Julien Grall wrote:
> >> (+ Security team)
> >>
> >> Hi Stefano,
> >>
> >> On 17/01/18 21:47, Stefano Stabellini wrote:
> >> > On Wed, 17 Jan 2018, Stefano Stabellini wrote:
> >> > > On Wed, 17 Jan 2018, Lars Kurth wrote:
> >> > > >        Regarding README.source, this is covering file and contain the
> >> > > > same mention as in the commit message. As this is a single function.
> >> > > > Isn't the commit message
> >> > > >        enough?
> >> > > >
> >> > > >
> >> > > >  From a legal viewpoint it is enough.
> >> > >
> >> > > If that is enough from a legal viewpoint, then it is enough for me.
> >> > >
> >> > > However, from a legal viewpoint, I thought we needed to explicitly
> >> > > mention all the original signed-off-bys because Julien is not actually
> >> > > the copyright holder for that function, hence, we need to add the
> >> > > signed-off-bys of all the missing copyright holders.
> >> >
> >> > Actually, reading again the Developer’s Certificate of Origin, it
> >> > states:
> >> >
> >> > "The contribution is based upon previous work that, to the best of my
> >> > knowledge, is covered under an appropriate open source license and I have
> >> > the right under that license to submit that work with modifications, whether
> >> > created in whole or in part by me, under the same open source license
> >> > (unless I am permitted to submit under a different license), as indicated in
> >> > the file"
> >> >
> >> > so I think Lars is right. In that case, there is no need to resubmit
> >> > this series, I'll commit to staging as is. If tests go well, I'll
> >> > backport it to the stable trees.
> >> Thank you! I have created branches with patches backported up to Xen 4.8. With
> >> minor changes:
> >>
> >>    - Xen 4.10: No changes
> >>    - Xen 4.9:
> >>       * minor conflict in some files
> >>       * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
> >>    - Xen 4.8:
> >>       * conflict in some files (one medium as the number of "features" is
> >> different)
> >>       * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
> >>
> >> The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX is the
> >> version of Xen.
> >>
> >> Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure and will
> >> require backport. The only difficulty here should be finding the list of
> >> commits required.
> >>
> >> Also, we probably want to update the XSA pointing to the patches. So if
> >> someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?
> >
> > These are the commits for the XSA 254 mitigation for the arm64
> > architecture:
> >
> > staging-4.10
> > b829d42829c1ff626a02756acae4dd482fc20c9a
> > 0f7a4faafb2d79920cc63457cfca3e03990af4cc
> > d1f4283a1d8405a480b4121e1efcfaec8bbdbffa
> > cae6e1572f39a1906be0fc3bdaf49fe514c6a9c0
> > 928112900e5b4a92ccebb2eea11665fd76aa0f0d
> > 728fadb586a2a14a244dabd70463bcc1654ecc85
> >
> > staging-4.9
> > 2ec7ccbffc6b788f65e55498e4347c1ee3a44b01
> > 50450c1f33dc72f2138a671d738934f796be3318
> > 3790833ef16b95653424ec9b145e460ec1a56d16
> > fba48eff18c02d716c95b92df804a755620be82e
> > 9f79e8d846e8413c828f5fc7cc6ac733728dff00
> > a2567d6b54b7b187ecc0165021b6dd07dafaf06a
> >
> > staging-4.8
> > 946dd2eefae2faeecbeb9662e66935c8070f64f5
> > 85990bf53addcdb0ce8e458a3d8fad199710ac59
> > cf0b584c8c5030588bc47a3614ad860af7482c53
> > 44139fed7c794eb4e47a9bb93061e325bd57fe8c
> > 6f6786ef0d7f7025860d360f6b1267193ffd1b27
> 
> Something looks quite odd. The commit message have two cherry-pick commit ID.
> 
> Why didn't you just merged the branches I provided?

Basically I did the backports on my own, then I double-checked that they
matched your own version of the backports. I did it for safety: this way
we can be quite sure that the backports are good, or both of us did
exactly the same mistakes :-)
It was very helpful to have branches to compare against, thank you for
that.


> >
> > For staging-4.7, I made the backports and tested them as well. They look
> > correct. However, given that it was more complex than initially though,
> > I would appreciate if you could give it a look as well (I haven't pushed
> > it staging-4.7 yet):
> >
> >   git://xenbits.xen.org/people/sstabellini/xen-unstable.git staging-4.7-xsa254
> 
> I will have a look.

Thanks again!

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Julien Grall]

Hi,

On 24/01/18 22:43, Stefano Stabellini wrote:
> On Wed, 24 Jan 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 24 January 2018 at 22:14, Stefano Stabellini <sstabellini@kernel.org> wrote:
>>> On Thu, 18 Jan 2018, Julien Grall wrote:
>>>> (+ Security team)
>>>>
>>>> Hi Stefano,
>>>>
>>>> On 17/01/18 21:47, Stefano Stabellini wrote:
>>>>> On Wed, 17 Jan 2018, Stefano Stabellini wrote:
>>>>>> On Wed, 17 Jan 2018, Lars Kurth wrote:
>>>>>>>         Regarding README.source, this is covering file and contain the
>>>>>>> same mention as in the commit message. As this is a single function.
>>>>>>> Isn't the commit message
>>>>>>>         enough?
>>>>>>>
>>>>>>>
>>>>>>>   From a legal viewpoint it is enough.
>>>>>>
>>>>>> If that is enough from a legal viewpoint, then it is enough for me.
>>>>>>
>>>>>> However, from a legal viewpoint, I thought we needed to explicitly
>>>>>> mention all the original signed-off-bys because Julien is not actually
>>>>>> the copyright holder for that function, hence, we need to add the
>>>>>> signed-off-bys of all the missing copyright holders.
>>>>>
>>>>> Actually, reading again the Developer’s Certificate of Origin, it
>>>>> states:
>>>>>
>>>>> "The contribution is based upon previous work that, to the best of my
>>>>> knowledge, is covered under an appropriate open source license and I have
>>>>> the right under that license to submit that work with modifications, whether
>>>>> created in whole or in part by me, under the same open source license
>>>>> (unless I am permitted to submit under a different license), as indicated in
>>>>> the file"
>>>>>
>>>>> so I think Lars is right. In that case, there is no need to resubmit
>>>>> this series, I'll commit to staging as is. If tests go well, I'll
>>>>> backport it to the stable trees.
>>>> Thank you! I have created branches with patches backported up to Xen 4.8. With
>>>> minor changes:
>>>>
>>>>     - Xen 4.10: No changes
>>>>     - Xen 4.9:
>>>>        * minor conflict in some files
>>>>        * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>>>>     - Xen 4.8:
>>>>        * conflict in some files (one medium as the number of "features" is
>>>> different)
>>>>        * compilation failure in cpuerrata.c (__virt_to_mfn does not exist)
>>>>
>>>> The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX is the
>>>> version of Xen.
>>>>
>>>> Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure and will
>>>> require backport. The only difficulty here should be finding the list of
>>>> commits required.
>>>>
>>>> Also, we probably want to update the XSA pointing to the patches. So if
>>>> someone wants to backport to Xen 4.7 (or earlier) they can. Any opinions?
>>>
>>> These are the commits for the XSA 254 mitigation for the arm64
>>> architecture:
>>>
>>> staging-4.10
>>> b829d42829c1ff626a02756acae4dd482fc20c9a
>>> 0f7a4faafb2d79920cc63457cfca3e03990af4cc
>>> d1f4283a1d8405a480b4121e1efcfaec8bbdbffa
>>> cae6e1572f39a1906be0fc3bdaf49fe514c6a9c0
>>> 928112900e5b4a92ccebb2eea11665fd76aa0f0d
>>> 728fadb586a2a14a244dabd70463bcc1654ecc85
>>>
>>> staging-4.9
>>> 2ec7ccbffc6b788f65e55498e4347c1ee3a44b01
>>> 50450c1f33dc72f2138a671d738934f796be3318
>>> 3790833ef16b95653424ec9b145e460ec1a56d16
>>> fba48eff18c02d716c95b92df804a755620be82e
>>> 9f79e8d846e8413c828f5fc7cc6ac733728dff00
>>> a2567d6b54b7b187ecc0165021b6dd07dafaf06a
>>>
>>> staging-4.8
>>> 946dd2eefae2faeecbeb9662e66935c8070f64f5
>>> 85990bf53addcdb0ce8e458a3d8fad199710ac59
>>> cf0b584c8c5030588bc47a3614ad860af7482c53
>>> 44139fed7c794eb4e47a9bb93061e325bd57fe8c
>>> 6f6786ef0d7f7025860d360f6b1267193ffd1b27
>>
>> Something looks quite odd. The commit message have two cherry-pick commit ID.
>>
>> Why didn't you just merged the branches I provided?
> 
> Basically I did the backports on my own, then I double-checked that they
> matched your own version of the backports. I did it for safety: this way
> we can be quite sure that the backports are good, or both of us did
> exactly the same mistakes :-)
> It was very helpful to have branches to compare against, thank you for
> that.

I also double checked it yesterday because I wasn't sure what you did :).

> 
> 
>>>
>>> For staging-4.7, I made the backports and tested them as well. They look
>>> correct. However, given that it was more complex than initially though,
>>> I would appreciate if you could give it a look as well (I haven't pushed
>>> it staging-4.7 yet):
>>>
>>>    git://xenbits.xen.org/people/sstabellini/xen-unstable.git staging-4.7-xsa254
>>
>> I will have a look.
> 
> Thanks again!

This looks good to me. Thank you for backporting them to 4.7.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: XSA-254 SP2 for ARM (was Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU)

[Stefano Stabellini]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 6177 bytes --]

On Thu, 25 Jan 2018, Julien Grall wrote:
> Hi,
> 
> On 24/01/18 22:43, Stefano Stabellini wrote:
> > On Wed, 24 Jan 2018, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 24 January 2018 at 22:14, Stefano Stabellini <sstabellini@kernel.org>
> > > wrote:
> > > > On Thu, 18 Jan 2018, Julien Grall wrote:
> > > > > (+ Security team)
> > > > > 
> > > > > Hi Stefano,
> > > > > 
> > > > > On 17/01/18 21:47, Stefano Stabellini wrote:
> > > > > > On Wed, 17 Jan 2018, Stefano Stabellini wrote:
> > > > > > > On Wed, 17 Jan 2018, Lars Kurth wrote:
> > > > > > > >         Regarding README.source, this is covering file and
> > > > > > > > contain the
> > > > > > > > same mention as in the commit message. As this is a single
> > > > > > > > function.
> > > > > > > > Isn't the commit message
> > > > > > > >         enough?
> > > > > > > > 
> > > > > > > > 
> > > > > > > >   From a legal viewpoint it is enough.
> > > > > > > 
> > > > > > > If that is enough from a legal viewpoint, then it is enough for
> > > > > > > me.
> > > > > > > 
> > > > > > > However, from a legal viewpoint, I thought we needed to explicitly
> > > > > > > mention all the original signed-off-bys because Julien is not
> > > > > > > actually
> > > > > > > the copyright holder for that function, hence, we need to add the
> > > > > > > signed-off-bys of all the missing copyright holders.
> > > > > > 
> > > > > > Actually, reading again the Developer’s Certificate of Origin, it
> > > > > > states:
> > > > > > 
> > > > > > "The contribution is based upon previous work that, to the best of
> > > > > > my
> > > > > > knowledge, is covered under an appropriate open source license and I
> > > > > > have
> > > > > > the right under that license to submit that work with modifications,
> > > > > > whether
> > > > > > created in whole or in part by me, under the same open source
> > > > > > license
> > > > > > (unless I am permitted to submit under a different license), as
> > > > > > indicated in
> > > > > > the file"
> > > > > > 
> > > > > > so I think Lars is right. In that case, there is no need to resubmit
> > > > > > this series, I'll commit to staging as is. If tests go well, I'll
> > > > > > backport it to the stable trees.
> > > > > Thank you! I have created branches with patches backported up to Xen
> > > > > 4.8. With
> > > > > minor changes:
> > > > > 
> > > > >     - Xen 4.10: No changes
> > > > >     - Xen 4.9:
> > > > >        * minor conflict in some files
> > > > >        * compilation failure in cpuerrata.c (__virt_to_mfn does not
> > > > > exist)
> > > > >     - Xen 4.8:
> > > > >        * conflict in some files (one medium as the number of
> > > > > "features" is
> > > > > different)
> > > > >        * compilation failure in cpuerrata.c (__virt_to_mfn does not
> > > > > exist)
> > > > > 
> > > > > The branches can be found on xenbits [1] : xsa-254-sp2-X.XX where X.XX
> > > > > is the
> > > > > version of Xen.
> > > > > 
> > > > > Xen 4.7 and earlier does not have cpufeature/cpuerrata infrastructure
> > > > > and will
> > > > > require backport. The only difficulty here should be finding the list
> > > > > of
> > > > > commits required.
> > > > > 
> > > > > Also, we probably want to update the XSA pointing to the patches. So
> > > > > if
> > > > > someone wants to backport to Xen 4.7 (or earlier) they can. Any
> > > > > opinions?
> > > > 
> > > > These are the commits for the XSA 254 mitigation for the arm64
> > > > architecture:
> > > > 
> > > > staging-4.10
> > > > b829d42829c1ff626a02756acae4dd482fc20c9a
> > > > 0f7a4faafb2d79920cc63457cfca3e03990af4cc
> > > > d1f4283a1d8405a480b4121e1efcfaec8bbdbffa
> > > > cae6e1572f39a1906be0fc3bdaf49fe514c6a9c0
> > > > 928112900e5b4a92ccebb2eea11665fd76aa0f0d
> > > > 728fadb586a2a14a244dabd70463bcc1654ecc85
> > > > 
> > > > staging-4.9
> > > > 2ec7ccbffc6b788f65e55498e4347c1ee3a44b01
> > > > 50450c1f33dc72f2138a671d738934f796be3318
> > > > 3790833ef16b95653424ec9b145e460ec1a56d16
> > > > fba48eff18c02d716c95b92df804a755620be82e
> > > > 9f79e8d846e8413c828f5fc7cc6ac733728dff00
> > > > a2567d6b54b7b187ecc0165021b6dd07dafaf06a
> > > > 
> > > > staging-4.8
> > > > 946dd2eefae2faeecbeb9662e66935c8070f64f5
> > > > 85990bf53addcdb0ce8e458a3d8fad199710ac59
> > > > cf0b584c8c5030588bc47a3614ad860af7482c53
> > > > 44139fed7c794eb4e47a9bb93061e325bd57fe8c
> > > > 6f6786ef0d7f7025860d360f6b1267193ffd1b27
> > > 
> > > Something looks quite odd. The commit message have two cherry-pick commit
> > > ID.
> > > 
> > > Why didn't you just merged the branches I provided?
> > 
> > Basically I did the backports on my own, then I double-checked that they
> > matched your own version of the backports. I did it for safety: this way
> > we can be quite sure that the backports are good, or both of us did
> > exactly the same mistakes :-)
> > It was very helpful to have branches to compare against, thank you for
> > that.
> 
> I also double checked it yesterday because I wasn't sure what you did :).
> 
> > 
> > 
> > > > 
> > > > For staging-4.7, I made the backports and tested them as well. They look
> > > > correct. However, given that it was more complex than initially though,
> > > > I would appreciate if you could give it a look as well (I haven't pushed
> > > > it staging-4.7 yet):
> > > > 
> > > >    git://xenbits.xen.org/people/sstabellini/xen-unstable.git
> > > > staging-4.7-xsa254
> > > 
> > > I will have a look.
> > 
> > Thanks again!
> 
> This looks good to me. Thank you for backporting them to 4.7.

Thank you! I pushed the branch, these are the relevant commits for 4.7:

fd884d6 xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs
50c68df xen/arm64: Add skeleton to harden the branch predictor aliasing attacks
1bdcc9f xen/arm: cpuerrata: Add MIDR_ALL_VERSIONS
2914ef5 xen/arm64: Add missing MIDR values for Cortex-A72, A73 and A75
62b9706 xen/arm: Introduce enable callback to enable a capabilities on each online CPU
624abdc xen/arm: Detect silicon revision and set cap bits accordingly
d7b73ed xen/arm: cpufeature: Provide an helper to check if a capability is supported
112c49c xen/arm: Add cpu_hwcap bitmap
a5b0fa4 xen/arm: Add macros to handle the MIDR

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Julien Grall]

On 17/01/18 14:31, Lars Kurth wrote:
> Hi Julien,

Hi Lars,

>> On 17 Jan 2018, at 12:31, Julien Grall <julien.grall@linaro.org 
>> <mailto:julien.grall@linaro.org>> wrote:
>>
>>>>>>
>>>>> If you took the code from Linux, you need to add the original
>>>>> Signed-off-by from the Linux commit. Aside from that:
>>>>
>>>> There are multiple commits touching this function. So I followed 
>>>> what we did in similar situation. By that I mean, mentioning the 
>>>> code was taken from Linux and not gathered the signed-off-by.
>>>>
>>>> If you really want, I can gather all the signed-off-by of the commit 
>>>> touching this function.
>>> If there are a lot of then, you may want to consider adding 
>>> a README.source file instead. There are a few examples in tree and 
>>> they are also mentioned in CONTRIBUTING files
>>
>> I am not sure to understand your suggestion here. I spotted only one 
>> CONTRIBUTING file and it only list the license.
> 
> I was suggesting to create a README.source in the xen arm tree seomwhere 
> where you can list imports.
> 
>> Regarding README.source, this is covering file and contain the same 
>> mention as in the commit message. As this is a single function. Isn't 
>> the commit message enough?
> 
>  From a legal viewpoint it is enough.
> The reason why we created the README.source file is because it is very 
> easy to miss code imports when they are mentioned in commit messages.

I understand that.

> 
> Normally this isn't a problem: only if we ever have to relicense the 
> code or if someone does code archeology
> When we relicensed the ACPI builder, it created all sorts of problems as 
> outlined in 
> https://www.slideshare.net/xen_com_mgr/ossa17-mixed-license-foss-projects
> 
>> Lastly, we do have quite a bit of code in Xen coming from Linux (or 
>> other project). A lot of them are not listed in 
>> README.source/CONTRIBUTING. But only mention in the commit message 
>> (not necessarily with Signed-off-by tag).
> 
> That is true: which is why I have started fixing these, whenever I found 
> them and moved such information to README.source.
> 
>> So I am quite interested to know what is the normal procedure here.
> 
> I think:
> * Doing this via Signed-off-by tag is sufficient for a one-off-import
> * I would prefer if people added import related info README.source files 
> (and also in the commit message)
> 
> Does this make sense?

I think it makes sense. Do you expect to see the README.source 
per-directory? Or one at top level?

Cheers,

> 
> Lars

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 1/5] xen/arm: Introduce enable callback to enable a capabilities on each online CPU

[Lars Kurth]

[-- Attachment #1.1: Type: text/plain, Size: 1473 bytes --]

Hi Julien,

> On 18 Jan 2018, at 10:56, Julien Grall <julien.grall@linaro.org> wrote:
> 
> On 17/01/18 14:31, Lars Kurth wrote:
>> Hi Julien,
> 
> Hi Lars,
> 
... 
>> Normally this isn't a problem: only if we ever have to relicense the code or if someone does code archeology
>> When we relicensed the ACPI builder, it created all sorts of problems as outlined in https://www.slideshare.net/xen_com_mgr/ossa17-mixed-license-foss-projects <https://www.slideshare.net/xen_com_mgr/ossa17-mixed-license-foss-projects>
>>> Lastly, we do have quite a bit of code in Xen coming from Linux (or other project). A lot of them are not listed in README.source/CONTRIBUTING. But only mention in the commit message (not necessarily with Signed-off-by tag).
>> That is true: which is why I have started fixing these, whenever I found them and moved such information to README.source.
>>> So I am quite interested to know what is the normal procedure here.
>> I think:
>> * Doing this via Signed-off-by tag is sufficient for a one-off-import
>> * I would prefer if people added import related info README.source files (and also in the commit message)
>> Does this make sense?
> 
> I think it makes sense. Do you expect to see the README.source per-directory? Or one at top level?

I think one somewhere at the top level of the ARM tree is sufficient. It depends on how often you import code. It shouldn't block the series. Just something to do at some point
Lars


[-- Attachment #1.2: Type: text/html, Size: 5747 bytes --]

[-- Attachment #2: Type: text/plain, Size: 157 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 2/5] xen/arm64: Add missing MIDR values for Cortex-A72, A73 and A75

[Julien Grall]

Cortex-A72, A73 and A75 MIDR will be used to a follow-up for hardening
the branch predictor.

This is part of XSA-254.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/include/asm-arm/processor.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 65eb1071e1..3edab1b893 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -47,10 +47,16 @@
 #define ARM_CPU_PART_CORTEX_A15     0xC0F
 #define ARM_CPU_PART_CORTEX_A53     0xD03
 #define ARM_CPU_PART_CORTEX_A57     0xD07
+#define ARM_CPU_PART_CORTEX_A72     0xD08
+#define ARM_CPU_PART_CORTEX_A73     0xD09
+#define ARM_CPU_PART_CORTEX_A75     0xD0A
 
 #define MIDR_CORTEX_A15 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A15)
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
 #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
+#define MIDR_CORTEX_A72 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A72)
+#define MIDR_CORTEX_A73 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A73)
+#define MIDR_CORTEX_A75 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A75)
 
 /* MPIDR Multiprocessor Affinity Register */
 #define _MPIDR_UP           (30)
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/5] xen/arm64: Add missing MIDR values for Cortex-A72, A73 and A75

[Stefano Stabellini]

On Tue, 16 Jan 2018, Julien Grall wrote:
> Cortex-A72, A73 and A75 MIDR will be used to a follow-up for hardening
> the branch predictor.
> 
> This is part of XSA-254.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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


> ---
>  xen/include/asm-arm/processor.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 65eb1071e1..3edab1b893 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -47,10 +47,16 @@
>  #define ARM_CPU_PART_CORTEX_A15     0xC0F
>  #define ARM_CPU_PART_CORTEX_A53     0xD03
>  #define ARM_CPU_PART_CORTEX_A57     0xD07
> +#define ARM_CPU_PART_CORTEX_A72     0xD08
> +#define ARM_CPU_PART_CORTEX_A73     0xD09
> +#define ARM_CPU_PART_CORTEX_A75     0xD0A
>  
>  #define MIDR_CORTEX_A15 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A15)
>  #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
>  #define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
> +#define MIDR_CORTEX_A72 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A72)
> +#define MIDR_CORTEX_A73 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A73)
> +#define MIDR_CORTEX_A75 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A75)
>  
>  /* MPIDR Multiprocessor Affinity Register */
>  #define _MPIDR_UP           (30)
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 3/5] xen/arm: cpuerrata: Add MIDR_ALL_VERSIONS

[Julien Grall]

Introduce a new macro MIDR_ALL_VERSIONS to match all variant/revision of a
given CPU model.

This is part of XSA-254.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/cpuerrata.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 772587c05a..c50d3331f2 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -7,6 +7,12 @@
     .midr_range_min = min,              \
     .midr_range_max = max
 
+#define MIDR_ALL_VERSIONS(model)        \
+    .matches = is_affected_midr_range,  \
+    .midr_model = model,                \
+    .midr_range_min = 0,                \
+    .midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK)
+
 static bool __maybe_unused
 is_affected_midr_range(const struct arm_cpu_capabilities *entry)
 {
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 3/5] xen/arm: cpuerrata: Add MIDR_ALL_VERSIONS

[Stefano Stabellini]

On Tue, 16 Jan 2018, Julien Grall wrote:
> Introduce a new macro MIDR_ALL_VERSIONS to match all variant/revision of a
> given CPU model.
> 
> This is part of XSA-254.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>

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


> ---
>  xen/arch/arm/cpuerrata.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 772587c05a..c50d3331f2 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -7,6 +7,12 @@
>      .midr_range_min = min,              \
>      .midr_range_max = max
>  
> +#define MIDR_ALL_VERSIONS(model)        \
> +    .matches = is_affected_midr_range,  \
> +    .midr_model = model,                \
> +    .midr_range_min = 0,                \
> +    .midr_range_max = (MIDR_VARIANT_MASK | MIDR_REVISION_MASK)
> +
>  static bool __maybe_unused
>  is_affected_midr_range(const struct arm_cpu_capabilities *entry)
>  {
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 4/5] xen/arm64: Add skeleton to harden the branch predictor aliasing attacks

[Julien Grall]

Aliasing attacked against CPU branch predictors can allow an attacker to
redirect speculative control flow on some CPUs and potentially divulge
information from one context to another.

This patch adds initial skeleton code behind a new Kconfig option to
enable implementation-specific mitigations against these attacks for
CPUs that are affected.

Most of the mitigations will have to be applied when entering to the
hypervisor from the guest context. For safety, it is applied at every
exception entry. So there are potential for optimizing when receiving
an exception at the same level.

Because the attack is against branch predictor, it is not possible to
safely use branch instruction before the mitigation is applied.
Therefore, this has to be done in the vector entry before jump to the
helper handling a given exception.

On Arm64, each vector can hold 32 instructions. This leave us 31
instructions for the mitigation. The last one is the branch instruction
to the helper.

Because a platform may have CPUs with different micro-architectures,
per-CPU vector table needs to be provided. Realistically, only a few
different mitigations will be necessary. So provide a small set of
vector tables. They will be re-used and patch with the mitigations
on-demand.

This is based on the work done in Linux (see [1]).

This is part of XSA-254.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
branch ktpi

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/Kconfig             |  20 ++++++
 xen/arch/arm/arm64/Makefile      |   1 +
 xen/arch/arm/arm64/bpi.S         |  64 ++++++++++++++++++
 xen/arch/arm/cpuerrata.c         | 142 +++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/traps.c             |   5 +-
 xen/include/asm-arm/cpuerrata.h  |   1 +
 xen/include/asm-arm/cpufeature.h |   3 +-
 xen/include/asm-arm/processor.h  |   5 +-
 8 files changed, 237 insertions(+), 4 deletions(-)
 create mode 100644 xen/arch/arm/arm64/bpi.S

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index f58019d6ed..06fd85cc77 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -171,6 +171,26 @@ config ARM64_ERRATUM_834220
 
 endmenu
 
+config HARDEN_BRANCH_PREDICTOR
+	bool "Harden the branch predictor against aliasing attacks" if EXPERT
+	default y
+	help
+	  Speculation attacks against some high-performance processors rely on
+	  being able to manipulate the branch predictor for a victim context by
+	  executing aliasing branches in the attacker context.  Such attacks
+	  can be partially mitigated against by clearing internal branch
+	  predictor state and limiting the prediction logic in some situations.
+
+	  This config option will take CPU-specific actions to harden the
+	  branch predictor against aliasing attacks and may rely on specific
+	  instruction sequences or control bits being set by the system
+	  firmware.
+
+	  If unsure, say Y.
+
+config ARM64_HARDEN_BRANCH_PREDICTOR
+    def_bool y if ARM_64 && HARDEN_BRANCH_PREDICTOR
+
 source "common/Kconfig"
 
 source "drivers/Kconfig"
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 718fe44455..bb5c610b2a 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,6 +1,7 @@
 subdir-y += lib
 
 obj-y += cache.o
+obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
 obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
new file mode 100644
index 0000000000..6cc2f17529
--- /dev/null
+++ b/xen/arch/arm/arm64/bpi.S
@@ -0,0 +1,64 @@
+/*
+ * Contains CPU specific branch predictor invalidation sequences
+ *
+ * Copyright (C) 2018 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+.macro ventry target
+    .rept 31
+    nop
+    .endr
+    b	\target
+.endm
+
+.macro vectors target
+    ventry \target + 0x000
+    ventry \target + 0x080
+    ventry \target + 0x100
+    ventry \target + 0x180
+
+    ventry \target + 0x200
+    ventry \target + 0x280
+    ventry \target + 0x300
+    ventry \target + 0x380
+
+    ventry \target + 0x400
+    ventry \target + 0x480
+    ventry \target + 0x500
+    ventry \target + 0x580
+
+    ventry \target + 0x600
+    ventry \target + 0x680
+    ventry \target + 0x700
+    ventry \target + 0x780
+.endm
+
+/*
+ * Populate 4 vector tables. This will cover up to 4 different
+ * micro-architectures in a system.
+ */
+    .align	11
+ENTRY(__bp_harden_hyp_vecs_start)
+    .rept 4
+    vectors hyp_traps_vector
+    .endr
+ENTRY(__bp_harden_hyp_vecs_end)
+
+/*
+ * Local variables:
+ * mode: ASM
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index c50d3331f2..76d98e771d 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -1,6 +1,148 @@
+#include <xen/cpumask.h>
+#include <xen/mm.h>
+#include <xen/sizes.h>
+#include <xen/smp.h>
+#include <xen/spinlock.h>
+#include <xen/vmap.h>
 #include <asm/cpufeature.h>
 #include <asm/cpuerrata.h>
 
+/* Override macros from asm/page.h to make them work with mfn_t */
+#undef virt_to_mfn
+#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
+
+/* Hardening Branch predictor code for Arm64 */
+#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
+
+#define VECTOR_TABLE_SIZE SZ_2K
+
+/*
+ * Number of available table vectors (this should be in-sync with
+ * arch/arm64/bpi.S
+ */
+#define NR_BPI_HYP_VECS 4
+
+extern char __bp_harden_hyp_vecs_start[], __bp_harden_hyp_vecs_end[];
+
+/*
+ * Key for each slot. This is used to find whether a specific workaround
+ * had a slot assigned.
+ *
+ * The key is virtual address of the vector workaround
+ */
+static uintptr_t bp_harden_slot_key[NR_BPI_HYP_VECS];
+
+/*
+ * [hyp_vec_start, hyp_vec_end[ corresponds to the first 31 instructions
+ * of each vector. The last (i.e 32th) instruction is used to branch to
+ * the original entry.
+ *
+ * Those instructions will be copied on each vector to harden them.
+ */
+static bool copy_hyp_vect_bpi(unsigned int slot, const char *hyp_vec_start,
+                              const char *hyp_vec_end)
+{
+    void *dst_remapped;
+    const void *dst = __bp_harden_hyp_vecs_start + slot * VECTOR_TABLE_SIZE;
+    unsigned int i;
+    mfn_t dst_mfn = virt_to_mfn(dst);
+
+    BUG_ON(((hyp_vec_end - hyp_vec_start) / 4) > 31);
+
+    /*
+     * Vectors are part of the text that are mapped read-only. So re-map
+     * the vector table to be able to update vectors.
+     */
+    dst_remapped = __vmap(&dst_mfn,
+                          1UL << get_order_from_bytes(VECTOR_TABLE_SIZE),
+                          1, 1, PAGE_HYPERVISOR, VMAP_DEFAULT);
+    if ( !dst_remapped )
+        return false;
+
+    dst_remapped += (vaddr_t)dst & ~PAGE_MASK;
+
+    for ( i = 0; i < VECTOR_TABLE_SIZE; i += 0x80 )
+    {
+        memcpy(dst_remapped + i, hyp_vec_start, hyp_vec_end - hyp_vec_start);
+    }
+
+    clean_dcache_va_range(dst_remapped, VECTOR_TABLE_SIZE);
+    invalidate_icache();
+
+    vunmap(dst_remapped);
+
+    return true;
+}
+
+static bool __maybe_unused
+install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
+                         const char *hyp_vec_start,
+                         const char *hyp_vec_end)
+{
+    static int last_slot = -1;
+    static DEFINE_SPINLOCK(bp_lock);
+    unsigned int i, slot = -1;
+    bool ret = true;
+
+    /*
+     * Enable callbacks are called on every CPU based on the
+     * capabilities. So double-check whether the CPU matches the
+     * entry.
+     */
+    if ( !entry->matches(entry) )
+        return true;
+
+    /*
+     * No need to install hardened vector when the processor has
+     * ID_AA64PRF0_EL1.CSV2 set.
+     */
+    if ( cpu_data[smp_processor_id()].pfr64.csv2 )
+        return true;
+
+    spin_lock(&bp_lock);
+
+    /*
+     * Look up whether the hardening vector had a slot already
+     * assigned.
+     */
+    for ( i = 0; i < 4; i++ )
+    {
+        if ( bp_harden_slot_key[i] == (uintptr_t)hyp_vec_start )
+        {
+            slot = i;
+            break;
+        }
+    }
+
+    if ( slot == -1 )
+    {
+        last_slot++;
+        /* Check we don't overrun the number of slots available. */
+        BUG_ON(NR_BPI_HYP_VECS <= last_slot);
+
+        slot = last_slot;
+        ret = copy_hyp_vect_bpi(slot, hyp_vec_start, hyp_vec_end);
+
+        /* Only update the slot if the copy succeeded. */
+        if ( ret )
+            bp_harden_slot_key[slot] = (uintptr_t)hyp_vec_start;
+    }
+
+    if ( ret )
+    {
+        /* Install the new vector table. */
+        WRITE_SYSREG((vaddr_t)(__bp_harden_hyp_vecs_start + slot * VECTOR_TABLE_SIZE),
+                     VBAR_EL2);
+        isb();
+    }
+
+    spin_unlock(&bp_lock);
+
+    return ret;
+}
+
+#endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */
+
 #define MIDR_RANGE(model, min, max)     \
     .matches = is_affected_midr_range,  \
     .midr_model = model,                \
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 013c1600ec..a3e4919751 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -160,7 +160,10 @@ __initcall(update_serrors_cpu_caps);
 
 void init_traps(void)
 {
-    /* Setup Hyp vector base */
+    /*
+     * Setup Hyp vector base. Note they might get updated with the
+     * branch predictor hardening.
+     */
     WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
 
     /* Trap Debug and Performance Monitor accesses */
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 7de68361ff..23ebf367ea 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -1,6 +1,7 @@
 #ifndef __ARM_CPUERRATA_H__
 #define __ARM_CPUERRATA_H__
 
+#include <xen/percpu.h>
 #include <asm/cpufeature.h>
 #include <asm/alternative.h>
 
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 21c65e198c..e557a095af 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -42,8 +42,9 @@
 #define LIVEPATCH_FEATURE   4
 #define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
 #define SKIP_CTXT_SWITCH_SERROR_SYNC 6
+#define ARM_HARDEN_BRANCH_PREDICTOR 7
 
-#define ARM_NCAPS           7
+#define ARM_NCAPS           8
 
 #ifndef __ASSEMBLY__
 
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 3edab1b893..466da5da86 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -385,8 +385,9 @@ struct cpuinfo_arm {
             unsigned long fp:4;   /* Floating Point */
             unsigned long simd:4; /* Advanced SIMD */
             unsigned long gic:4;  /* GIC support */
-            unsigned long __res0:4;
-            unsigned long __res1;
+            unsigned long __res0:28;
+            unsigned long csv2:4;
+            unsigned long __res1:4;
         };
     } pfr64;
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/5] xen/arm64: Add skeleton to harden the branch predictor aliasing attacks

[Stefano Stabellini]

On Tue, 16 Jan 2018, Julien Grall wrote:
> Aliasing attacked against CPU branch predictors can allow an attacker to
> redirect speculative control flow on some CPUs and potentially divulge
> information from one context to another.
> 
> This patch adds initial skeleton code behind a new Kconfig option to
> enable implementation-specific mitigations against these attacks for
> CPUs that are affected.
> 
> Most of the mitigations will have to be applied when entering to the
> hypervisor from the guest context. For safety, it is applied at every
> exception entry. So there are potential for optimizing when receiving
> an exception at the same level.
> 
> Because the attack is against branch predictor, it is not possible to
> safely use branch instruction before the mitigation is applied.
> Therefore, this has to be done in the vector entry before jump to the
> helper handling a given exception.
> 
> On Arm64, each vector can hold 32 instructions. This leave us 31
> instructions for the mitigation. The last one is the branch instruction
> to the helper.
> 
> Because a platform may have CPUs with different micro-architectures,
> per-CPU vector table needs to be provided. Realistically, only a few
> different mitigations will be necessary. So provide a small set of
> vector tables. They will be re-used and patch with the mitigations
> on-demand.
> 
> This is based on the work done in Linux (see [1]).
> 
> This is part of XSA-254.
> 
> [1] git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
> branch ktpi

If mentioning the original commit/branch is sufficient, then OK.
Otherwise, if we need to explicitly mention all the copyright holders,
then please add all the required signed-off-bys.


> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/Kconfig             |  20 ++++++
>  xen/arch/arm/arm64/Makefile      |   1 +
>  xen/arch/arm/arm64/bpi.S         |  64 ++++++++++++++++++
>  xen/arch/arm/cpuerrata.c         | 142 +++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/traps.c             |   5 +-
>  xen/include/asm-arm/cpuerrata.h  |   1 +
>  xen/include/asm-arm/cpufeature.h |   3 +-
>  xen/include/asm-arm/processor.h  |   5 +-
>  8 files changed, 237 insertions(+), 4 deletions(-)
>  create mode 100644 xen/arch/arm/arm64/bpi.S
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index f58019d6ed..06fd85cc77 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -171,6 +171,26 @@ config ARM64_ERRATUM_834220
>  
>  endmenu
>  
> +config HARDEN_BRANCH_PREDICTOR
> +	bool "Harden the branch predictor against aliasing attacks" if EXPERT
> +	default y
> +	help
> +	  Speculation attacks against some high-performance processors rely on
> +	  being able to manipulate the branch predictor for a victim context by
> +	  executing aliasing branches in the attacker context.  Such attacks
> +	  can be partially mitigated against by clearing internal branch
> +	  predictor state and limiting the prediction logic in some situations.
> +
> +	  This config option will take CPU-specific actions to harden the
> +	  branch predictor against aliasing attacks and may rely on specific
> +	  instruction sequences or control bits being set by the system
> +	  firmware.
> +
> +	  If unsure, say Y.
> +
> +config ARM64_HARDEN_BRANCH_PREDICTOR
> +    def_bool y if ARM_64 && HARDEN_BRANCH_PREDICTOR
> +
>  source "common/Kconfig"
>  
>  source "drivers/Kconfig"
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 718fe44455..bb5c610b2a 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -1,6 +1,7 @@
>  subdir-y += lib
>  
>  obj-y += cache.o
> +obj-$(CONFIG_HARDEN_BRANCH_PREDICTOR) += bpi.o
>  obj-$(EARLY_PRINTK) += debug.o
>  obj-y += domctl.o
>  obj-y += domain.o
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> new file mode 100644
> index 0000000000..6cc2f17529
> --- /dev/null
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -0,0 +1,64 @@
> +/*
> + * Contains CPU specific branch predictor invalidation sequences
> + *
> + * Copyright (C) 2018 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +.macro ventry target
> +    .rept 31
> +    nop
> +    .endr
> +    b	\target
> +.endm
> +
> +.macro vectors target
> +    ventry \target + 0x000
> +    ventry \target + 0x080
> +    ventry \target + 0x100
> +    ventry \target + 0x180
> +
> +    ventry \target + 0x200
> +    ventry \target + 0x280
> +    ventry \target + 0x300
> +    ventry \target + 0x380
> +
> +    ventry \target + 0x400
> +    ventry \target + 0x480
> +    ventry \target + 0x500
> +    ventry \target + 0x580
> +
> +    ventry \target + 0x600
> +    ventry \target + 0x680
> +    ventry \target + 0x700
> +    ventry \target + 0x780
> +.endm
> +
> +/*
> + * Populate 4 vector tables. This will cover up to 4 different
> + * micro-architectures in a system.
> + */
> +    .align	11
> +ENTRY(__bp_harden_hyp_vecs_start)
> +    .rept 4
> +    vectors hyp_traps_vector
> +    .endr
> +ENTRY(__bp_harden_hyp_vecs_end)
> +
> +/*
> + * Local variables:
> + * mode: ASM
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index c50d3331f2..76d98e771d 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -1,6 +1,148 @@
> +#include <xen/cpumask.h>
> +#include <xen/mm.h>
> +#include <xen/sizes.h>
> +#include <xen/smp.h>
> +#include <xen/spinlock.h>
> +#include <xen/vmap.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cpuerrata.h>
>  
> +/* Override macros from asm/page.h to make them work with mfn_t */
> +#undef virt_to_mfn
> +#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +
> +/* Hardening Branch predictor code for Arm64 */
> +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
> +
> +#define VECTOR_TABLE_SIZE SZ_2K
> +
> +/*
> + * Number of available table vectors (this should be in-sync with
> + * arch/arm64/bpi.S
> + */
> +#define NR_BPI_HYP_VECS 4
> +
> +extern char __bp_harden_hyp_vecs_start[], __bp_harden_hyp_vecs_end[];
> +
> +/*
> + * Key for each slot. This is used to find whether a specific workaround
> + * had a slot assigned.
> + *
> + * The key is virtual address of the vector workaround
> + */
> +static uintptr_t bp_harden_slot_key[NR_BPI_HYP_VECS];
> +
> +/*
> + * [hyp_vec_start, hyp_vec_end[ corresponds to the first 31 instructions
> + * of each vector. The last (i.e 32th) instruction is used to branch to
> + * the original entry.
> + *
> + * Those instructions will be copied on each vector to harden them.
> + */
> +static bool copy_hyp_vect_bpi(unsigned int slot, const char *hyp_vec_start,
> +                              const char *hyp_vec_end)
> +{
> +    void *dst_remapped;
> +    const void *dst = __bp_harden_hyp_vecs_start + slot * VECTOR_TABLE_SIZE;
> +    unsigned int i;
> +    mfn_t dst_mfn = virt_to_mfn(dst);
> +
> +    BUG_ON(((hyp_vec_end - hyp_vec_start) / 4) > 31);
> +
> +    /*
> +     * Vectors are part of the text that are mapped read-only. So re-map
> +     * the vector table to be able to update vectors.
> +     */
> +    dst_remapped = __vmap(&dst_mfn,
> +                          1UL << get_order_from_bytes(VECTOR_TABLE_SIZE),
> +                          1, 1, PAGE_HYPERVISOR, VMAP_DEFAULT);
> +    if ( !dst_remapped )
> +        return false;
> +
> +    dst_remapped += (vaddr_t)dst & ~PAGE_MASK;
> +
> +    for ( i = 0; i < VECTOR_TABLE_SIZE; i += 0x80 )
> +    {
> +        memcpy(dst_remapped + i, hyp_vec_start, hyp_vec_end - hyp_vec_start);
> +    }
> +
> +    clean_dcache_va_range(dst_remapped, VECTOR_TABLE_SIZE);
> +    invalidate_icache();
> +
> +    vunmap(dst_remapped);
> +
> +    return true;
> +}
> +
> +static bool __maybe_unused
> +install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
> +                         const char *hyp_vec_start,
> +                         const char *hyp_vec_end)
> +{
> +    static int last_slot = -1;
> +    static DEFINE_SPINLOCK(bp_lock);
> +    unsigned int i, slot = -1;
> +    bool ret = true;
> +
> +    /*
> +     * Enable callbacks are called on every CPU based on the
> +     * capabilities. So double-check whether the CPU matches the
> +     * entry.
> +     */
> +    if ( !entry->matches(entry) )
> +        return true;
> +    /*
> +     * No need to install hardened vector when the processor has
> +     * ID_AA64PRF0_EL1.CSV2 set.
> +     */
> +    if ( cpu_data[smp_processor_id()].pfr64.csv2 )
> +        return true;
> +
> +    spin_lock(&bp_lock);
> +    /*
> +     * Look up whether the hardening vector had a slot already
> +     * assigned.
> +     */
> +    for ( i = 0; i < 4; i++ )
> +    {
> +        if ( bp_harden_slot_key[i] == (uintptr_t)hyp_vec_start )
> +        {
> +            slot = i;
> +            break;
> +        }
> +    }
> +
> +    if ( slot == -1 )
> +    {
> +        last_slot++;
> +        /* Check we don't overrun the number of slots available. */
> +        BUG_ON(NR_BPI_HYP_VECS <= last_slot);
> +
> +        slot = last_slot;
> +        ret = copy_hyp_vect_bpi(slot, hyp_vec_start, hyp_vec_end);
> +
> +        /* Only update the slot if the copy succeeded. */
> +        if ( ret )
> +            bp_harden_slot_key[slot] = (uintptr_t)hyp_vec_start;
> +    }
> +
> +    if ( ret )
> +    {
> +        /* Install the new vector table. */
> +        WRITE_SYSREG((vaddr_t)(__bp_harden_hyp_vecs_start + slot * VECTOR_TABLE_SIZE),
> +                     VBAR_EL2);
> +        isb();
> +    }
> +
> +    spin_unlock(&bp_lock);
> +
> +    return ret;
> +}
> +
> +#endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */
> +
>  #define MIDR_RANGE(model, min, max)     \
>      .matches = is_affected_midr_range,  \
>      .midr_model = model,                \
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 013c1600ec..a3e4919751 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -160,7 +160,10 @@ __initcall(update_serrors_cpu_caps);
>  
>  void init_traps(void)
>  {
> -    /* Setup Hyp vector base */
> +    /*
> +     * Setup Hyp vector base. Note they might get updated with the
> +     * branch predictor hardening.
> +     */
>      WRITE_SYSREG((vaddr_t)hyp_traps_vector, VBAR_EL2);
>  
>      /* Trap Debug and Performance Monitor accesses */
> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
> index 7de68361ff..23ebf367ea 100644
> --- a/xen/include/asm-arm/cpuerrata.h
> +++ b/xen/include/asm-arm/cpuerrata.h
> @@ -1,6 +1,7 @@
>  #ifndef __ARM_CPUERRATA_H__
>  #define __ARM_CPUERRATA_H__
>  
> +#include <xen/percpu.h>

This doesn't seem to be necessary?

In any case:

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


>  #include <asm/cpufeature.h>
>  #include <asm/alternative.h>
>  
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 21c65e198c..e557a095af 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -42,8 +42,9 @@
>  #define LIVEPATCH_FEATURE   4
>  #define SKIP_SYNCHRONIZE_SERROR_ENTRY_EXIT 5
>  #define SKIP_CTXT_SWITCH_SERROR_SYNC 6
> +#define ARM_HARDEN_BRANCH_PREDICTOR 7
>  
> -#define ARM_NCAPS           7
> +#define ARM_NCAPS           8
>  
>  #ifndef __ASSEMBLY__
>  
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 3edab1b893..466da5da86 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -385,8 +385,9 @@ struct cpuinfo_arm {
>              unsigned long fp:4;   /* Floating Point */
>              unsigned long simd:4; /* Advanced SIMD */
>              unsigned long gic:4;  /* GIC support */
> -            unsigned long __res0:4;
> -            unsigned long __res1;
> +            unsigned long __res0:28;
> +            unsigned long csv2:4;
> +            unsigned long __res1:4;
>          };
>      } pfr64;
>  

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 4/5] xen/arm64: Add skeleton to harden the branch predictor aliasing attacks

[Julien Grall]

Hi Stefano,

On 17/01/18 18:26, Stefano Stabellini wrote:
> On Tue, 16 Jan 2018, Julien Grall wrote:
>> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
>> index 7de68361ff..23ebf367ea 100644
>> --- a/xen/include/asm-arm/cpuerrata.h
>> +++ b/xen/include/asm-arm/cpuerrata.h
>> @@ -1,6 +1,7 @@
>>   #ifndef __ARM_CPUERRATA_H__
>>   #define __ARM_CPUERRATA_H__
>>   
>> +#include <xen/percpu.h>
> 
> This doesn't seem to be necessary?

You are right. My original plan was based on Linux which use per-cpu 
variable. But I scratched that as it was possible to simplify the code.

I will send a patch to remove it.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

[PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Julien Grall]

Cortex-A57, A72, A73 and A75 are susceptible to branch predictor
aliasing and can theoritically be attacked by malicious code.

This patch implements a PSCI-based mitigation for these CPUs when
available. The call into firmware will invalidate the branch predictor
state, preventing any malicious entries from affection other victim
contexts.

Ported from Linux git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
branch kpti.

 Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
 Signed-off-by: Will Deacon <will.deacon@arm.com>

This is part of XSA-254.

Signed-off-by: Julien Grall <julien.grall@linaro.org>
---
 xen/arch/arm/arm64/bpi.S | 25 ++++++++++++++++++++++++
 xen/arch/arm/cpuerrata.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 74 insertions(+)

diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
index 6cc2f17529..4b7f1dc21f 100644
--- a/xen/arch/arm/arm64/bpi.S
+++ b/xen/arch/arm/arm64/bpi.S
@@ -56,6 +56,31 @@ ENTRY(__bp_harden_hyp_vecs_start)
     .endr
 ENTRY(__bp_harden_hyp_vecs_end)
 
+ENTRY(__psci_hyp_bp_inval_start)
+    sub     sp, sp, #(8 * 18)
+    stp     x16, x17, [sp, #(16 * 0)]
+    stp     x14, x15, [sp, #(16 * 1)]
+    stp     x12, x13, [sp, #(16 * 2)]
+    stp     x10, x11, [sp, #(16 * 3)]
+    stp     x8, x9, [sp, #(16 * 4)]
+    stp     x6, x7, [sp, #(16 * 5)]
+    stp     x4, x5, [sp, #(16 * 6)]
+    stp     x2, x3, [sp, #(16 * 7)]
+    stp     x0, x1, [sp, #(16 * 8)]
+    mov     x0, #0x84000000
+    smc     #0
+    ldp     x16, x17, [sp, #(16 * 0)]
+    ldp     x14, x15, [sp, #(16 * 1)]
+    ldp     x12, x13, [sp, #(16 * 2)]
+    ldp     x10, x11, [sp, #(16 * 3)]
+    ldp     x8, x9, [sp, #(16 * 4)]
+    ldp     x6, x7, [sp, #(16 * 5)]
+    ldp     x4, x5, [sp, #(16 * 6)]
+    ldp     x2, x3, [sp, #(16 * 7)]
+    ldp     x0, x1, [sp, #(16 * 8)]
+    add     sp, sp, #(8 * 18)
+ENTRY(__psci_hyp_bp_inval_end)
+
 /*
  * Local variables:
  * mode: ASM
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 76d98e771d..f1ea7f3c5b 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -4,8 +4,10 @@
 #include <xen/smp.h>
 #include <xen/spinlock.h>
 #include <xen/vmap.h>
+#include <xen/warning.h>
 #include <asm/cpufeature.h>
 #include <asm/cpuerrata.h>
+#include <asm/psci.h>
 
 /* Override macros from asm/page.h to make them work with mfn_t */
 #undef virt_to_mfn
@@ -141,6 +143,31 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
     return ret;
 }
 
+extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
+
+static int enable_psci_bp_hardening(void *data)
+{
+    bool ret = true;
+    static bool warned = false;
+
+    /*
+     * The mitigation is using PSCI version function to invalidate the
+     * branch predictor. This function is only available with PSCI 0.2
+     * and later.
+     */
+    if ( psci_ver >= PSCI_VERSION(0, 2) )
+        ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
+                                       __psci_hyp_bp_inval_end);
+    else if ( !warned )
+    {
+        ASSERT(system_state < SYS_STATE_active);
+        warning_add("PSCI 0.2 or later is required for the branch predictor hardening.\n");
+        warned = true;
+    }
+
+    return !ret;
+}
+
 #endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */
 
 #define MIDR_RANGE(model, min, max)     \
@@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] = {
                    (1 << MIDR_VARIANT_SHIFT) | 2),
     },
 #endif
+#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
+        .enable = enable_psci_bp_hardening,
+    },
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
+        .enable = enable_psci_bp_hardening,
+    },
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
+        .enable = enable_psci_bp_hardening,
+    },
+    {
+        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
+        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
+        .enable = enable_psci_bp_hardening,
+    },
+#endif
     {},
 };
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Stefano Stabellini]

On Tue, 16 Jan 2018, Julien Grall wrote:
> Cortex-A57, A72, A73 and A75 are susceptible to branch predictor
> aliasing and can theoritically be attacked by malicious code.
> 
> This patch implements a PSCI-based mitigation for these CPUs when
> available. The call into firmware will invalidate the branch predictor
> state, preventing any malicious entries from affection other victim
> contexts.
> 
> Ported from Linux git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git
> branch kpti.
> 
>  Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>
>  Signed-off-by: Will Deacon <will.deacon@arm.com>
> 
> This is part of XSA-254.
> 
> Signed-off-by: Julien Grall <julien.grall@linaro.org>
> ---
>  xen/arch/arm/arm64/bpi.S | 25 ++++++++++++++++++++++++
>  xen/arch/arm/cpuerrata.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 74 insertions(+)
> 
> diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S
> index 6cc2f17529..4b7f1dc21f 100644
> --- a/xen/arch/arm/arm64/bpi.S
> +++ b/xen/arch/arm/arm64/bpi.S
> @@ -56,6 +56,31 @@ ENTRY(__bp_harden_hyp_vecs_start)
>      .endr
>  ENTRY(__bp_harden_hyp_vecs_end)
>  
> +ENTRY(__psci_hyp_bp_inval_start)
> +    sub     sp, sp, #(8 * 18)
> +    stp     x16, x17, [sp, #(16 * 0)]
> +    stp     x14, x15, [sp, #(16 * 1)]
> +    stp     x12, x13, [sp, #(16 * 2)]
> +    stp     x10, x11, [sp, #(16 * 3)]
> +    stp     x8, x9, [sp, #(16 * 4)]
> +    stp     x6, x7, [sp, #(16 * 5)]
> +    stp     x4, x5, [sp, #(16 * 6)]
> +    stp     x2, x3, [sp, #(16 * 7)]
> +    stp     x0, x1, [sp, #(16 * 8)]
> +    mov     x0, #0x84000000
> +    smc     #0
> +    ldp     x16, x17, [sp, #(16 * 0)]
> +    ldp     x14, x15, [sp, #(16 * 1)]
> +    ldp     x12, x13, [sp, #(16 * 2)]
> +    ldp     x10, x11, [sp, #(16 * 3)]
> +    ldp     x8, x9, [sp, #(16 * 4)]
> +    ldp     x6, x7, [sp, #(16 * 5)]
> +    ldp     x4, x5, [sp, #(16 * 6)]
> +    ldp     x2, x3, [sp, #(16 * 7)]
> +    ldp     x0, x1, [sp, #(16 * 8)]
> +    add     sp, sp, #(8 * 18)
> +ENTRY(__psci_hyp_bp_inval_end)
> +
>  /*
>   * Local variables:
>   * mode: ASM
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 76d98e771d..f1ea7f3c5b 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -4,8 +4,10 @@
>  #include <xen/smp.h>
>  #include <xen/spinlock.h>
>  #include <xen/vmap.h>
> +#include <xen/warning.h>
>  #include <asm/cpufeature.h>
>  #include <asm/cpuerrata.h>
> +#include <asm/psci.h>
>  
>  /* Override macros from asm/page.h to make them work with mfn_t */
>  #undef virt_to_mfn
> @@ -141,6 +143,31 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry,
>      return ret;
>  }
>  
> +extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[];
> +
> +static int enable_psci_bp_hardening(void *data)
> +{
> +    bool ret = true;
> +    static bool warned = false;
> +
> +    /*
> +     * The mitigation is using PSCI version function to invalidate the
> +     * branch predictor. This function is only available with PSCI 0.2
> +     * and later.
> +     */
> +    if ( psci_ver >= PSCI_VERSION(0, 2) )
> +        ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start,
> +                                       __psci_hyp_bp_inval_end);
> +    else if ( !warned )
> +    {
> +        ASSERT(system_state < SYS_STATE_active);
> +        warning_add("PSCI 0.2 or later is required for the branch predictor hardening.\n");
> +        warned = true;
> +    }
> +
> +    return !ret;
> +}
> +
>  #endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */
>  
>  #define MIDR_RANGE(model, min, max)     \
> @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] = {
>                     (1 << MIDR_VARIANT_SHIFT) | 2),
>      },
>  #endif
> +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
> +    {
> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
> +        .enable = enable_psci_bp_hardening,
> +    },
> +    {
> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
> +        .enable = enable_psci_bp_hardening,
> +    },
> +    {
> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
> +        .enable = enable_psci_bp_hardening,
> +    },
> +    {
> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
> +        .enable = enable_psci_bp_hardening,
> +    },

We need to add a basic description in the desc field as it is printed by
update_cpu_capabilities.


> +#endif
>      {},
>  };
>  
> -- 
> 2.11.0
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Julien Grall]

Hi Stefano,

On 17/01/18 00:42, Stefano Stabellini wrote:
> On Tue, 16 Jan 2018, Julien Grall wrote:
>>   #define MIDR_RANGE(model, min, max)     \
>> @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] = {
>>                      (1 << MIDR_VARIANT_SHIFT) | 2),
>>       },
>>   #endif
>> +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
>> +    {
>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
>> +        .enable = enable_psci_bp_hardening,
>> +    },
>> +    {
>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
>> +        .enable = enable_psci_bp_hardening,
>> +    },
>> +    {
>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
>> +        .enable = enable_psci_bp_hardening,
>> +    },
>> +    {
>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
>> +        .enable = enable_psci_bp_hardening,
>> +    },
> 
> We need to add a basic description in the desc field as it is printed by
> update_cpu_capabilities.

desc field is not mandatory, and in that case I think the print would be 
confusing. At the difference of the other errata, we have more check in 
install_bp_hardening_vec that may result to skip the hardening.

The errata here is covering all variant/revision of A75 models for 
safety reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to 
tell whether a branch predictor trained in one context will affect 
speculative execution in another context. This field is checked in 
install_bp_hardening_vec so you avoid to harden the vector tables and 
small performance hit.

IHMO, it would be better to have a per-CPU message in 
install_bp_hardening_vec announcing whether the vector tables has been 
harden and the kind of hardening.

What do you think?

Cheers,

> 
> 
>> +#endif
>>       {},
>>   };
>>   
>> -- 
>> 2.11.0
>>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Stefano Stabellini]

On Wed, 17 Jan 2018, Julien Grall wrote:
> Hi Stefano,
> 
> On 17/01/18 00:42, Stefano Stabellini wrote:
> > On Tue, 16 Jan 2018, Julien Grall wrote:
> > >   #define MIDR_RANGE(model, min, max)     \
> > > @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[]
> > > = {
> > >                      (1 << MIDR_VARIANT_SHIFT) | 2),
> > >       },
> > >   #endif
> > > +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > > +    {
> > > +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
> > > +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
> > > +        .enable = enable_psci_bp_hardening,
> > > +    },
> > 
> > We need to add a basic description in the desc field as it is printed by
> > update_cpu_capabilities.
> 
> desc field is not mandatory, and in that case I think the print would be
> confusing. At the difference of the other errata, we have more check in
> install_bp_hardening_vec that may result to skip the hardening.
> 
> The errata here is covering all variant/revision of A75 models for safety
> reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a
> branch predictor trained in one context will affect speculative execution in
> another context. This field is checked in install_bp_hardening_vec so you
> avoid to harden the vector tables and small performance hit.
> 
> IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec
> announcing whether the vector tables has been harden and the kind of
> hardening.
> 
> What do you think?

I understand what you are saying and I agree with you. Maybe we should
change update_cpu_capabilities to print "detected need for workaround:
blah" but you don't have to do it in this series.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 5/5] xen/arm64: Implement branch predictor hardening for affected Cortex-A CPUs

[Julien Grall]

Hi,

On 17/01/18 17:11, Stefano Stabellini wrote:
> On Wed, 17 Jan 2018, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 17/01/18 00:42, Stefano Stabellini wrote:
>>> On Tue, 16 Jan 2018, Julien Grall wrote:
>>>>    #define MIDR_RANGE(model, min, max)     \
>>>> @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[]
>>>> = {
>>>>                       (1 << MIDR_VARIANT_SHIFT) | 2),
>>>>        },
>>>>    #endif
>>>> +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR
>>>> +    {
>>>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>>>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A57),
>>>> +        .enable = enable_psci_bp_hardening,
>>>> +    },
>>>> +    {
>>>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>>>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A72),
>>>> +        .enable = enable_psci_bp_hardening,
>>>> +    },
>>>> +    {
>>>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>>>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A73),
>>>> +        .enable = enable_psci_bp_hardening,
>>>> +    },
>>>> +    {
>>>> +        .capability = ARM_HARDEN_BRANCH_PREDICTOR,
>>>> +        MIDR_ALL_VERSIONS(MIDR_CORTEX_A75),
>>>> +        .enable = enable_psci_bp_hardening,
>>>> +    },
>>>
>>> We need to add a basic description in the desc field as it is printed by
>>> update_cpu_capabilities.
>>
>> desc field is not mandatory, and in that case I think the print would be
>> confusing. At the difference of the other errata, we have more check in
>> install_bp_hardening_vec that may result to skip the hardening.
>>
>> The errata here is covering all variant/revision of A75 models for safety
>> reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a
>> branch predictor trained in one context will affect speculative execution in
>> another context. This field is checked in install_bp_hardening_vec so you
>> avoid to harden the vector tables and small performance hit.
>>
>> IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec
>> announcing whether the vector tables has been harden and the kind of
>> hardening.
>>
>> What do you think?
> 
> I understand what you are saying and I agree with you. Maybe we should
> change update_cpu_capabilities to print "detected need for workaround:
> blah" but you don't have to do it in this series.

"need for workaround..." is not entirely true for the branch predictor 
hardening. It is more a "may need". I still prefer the per-CPU message
"CPU%u will %s on guest exit"

%u is the CPU number. %s will be the name of the call/instruction to 
execute.

The rationale behind is branch predictor hardening may be different on 
each CPU (think big.LITTLE) so code executed will be different. This 
differ from the other errata that will be applied for all CPUs.

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel