diff mbox series

[2/3] xen/arm: Advertise workaround 1 if we apply 3

Message ID 8119538cce93516f1e78d37d578996a195686f89.1651570561.git.bertrand.marquis@arm.com (mailing list archive)
State Superseded
Headers show
Series Spectre BHB follow up | expand

Commit Message

Bertrand Marquis May 3, 2022, 9:38 a.m. UTC
SMCC_WORKAROUND_3 is handling both Spectre v2 and spectre BHB.
So when a guest is asking if we support workaround 1, tell yes if we
apply workaround 3 on exception entry as it handles it.

This will allow guests not supporting Spectre BHB but impacted by
spectre v2 to still handle it correctly.
The modified behaviour is coherent with what the Linux kernel does in
KVM for guests.

While there use ARM_SMCCC_SUCCESS instead of 0 for the return code value
for workaround detection to be coherent with Workaround 2 handling.

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

Comments

Julien Grall May 3, 2022, 6:17 p.m. UTC | #1
Hi Bertrand,

On 03/05/2022 10:38, Bertrand Marquis wrote:
> SMCC_WORKAROUND_3 is handling both Spectre v2 and spectre BHB.
> So when a guest is asking if we support workaround 1, tell yes if we
> apply workaround 3 on exception entry as it handles it.
> 
> This will allow guests not supporting Spectre BHB but impacted by
> spectre v2 to still handle it correctly.
> The modified behaviour is coherent with what the Linux kernel does in
> KVM for guests.
> 
> While there use ARM_SMCCC_SUCCESS instead of 0 for the return code value
> for workaround detection to be coherent with Workaround 2 handling.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>

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

I think we should also consider for backport.

Cheers,
Bertrand Marquis May 4, 2022, 7:25 a.m. UTC | #2
Hi Julien,

> On 3 May 2022, at 19:17, Julien Grall <julien@xen.org> wrote:
> 
> Hi Bertrand,
> 
> On 03/05/2022 10:38, Bertrand Marquis wrote:
>> SMCC_WORKAROUND_3 is handling both Spectre v2 and spectre BHB.
>> So when a guest is asking if we support workaround 1, tell yes if we
>> apply workaround 3 on exception entry as it handles it.
>> This will allow guests not supporting Spectre BHB but impacted by
>> spectre v2 to still handle it correctly.
>> The modified behaviour is coherent with what the Linux kernel does in
>> KVM for guests.
>> While there use ARM_SMCCC_SUCCESS instead of 0 for the return code value
>> for workaround detection to be coherent with Workaround 2 handling.
>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks

> 
> I think we should also consider for backport.

Agree.

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall
Julien Grall May 5, 2022, 10:51 a.m. UTC | #3
On 04/05/2022 08:25, Bertrand Marquis wrote:
> Hi Julien,

Hi Bertrand,

>> On 3 May 2022, at 19:17, Julien Grall <julien@xen.org> wrote:
>>
>> Hi Bertrand,
>>
>> On 03/05/2022 10:38, Bertrand Marquis wrote:
>>> SMCC_WORKAROUND_3 is handling both Spectre v2 and spectre BHB.
>>> So when a guest is asking if we support workaround 1, tell yes if we
>>> apply workaround 3 on exception entry as it handles it.
>>> This will allow guests not supporting Spectre BHB but impacted by
>>> spectre v2 to still handle it correctly.
>>> The modified behaviour is coherent with what the Linux kernel does in
>>> KVM for guests.
>>> While there use ARM_SMCCC_SUCCESS instead of 0 for the return code value
>>> for workaround detection to be coherent with Workaround 2 handling.
>>> Signed-off-by: Bertrand Marquis <bertrand.marquis@arm.com>
>>
>> Acked-by: Julien Grall <jgrall@amazon.com>
> 
> Thanks
> 
>>
>> I think we should also consider for backport.
> 
> Agree.

I have committed this patch and added to my list of backport candidate.

Cheers,
diff mbox series

Patch

diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c
index b633ff2fe8..676740ef15 100644
--- a/xen/arch/arm/vsmc.c
+++ b/xen/arch/arm/vsmc.c
@@ -104,8 +104,13 @@  static bool handle_arch(struct cpu_user_regs *regs)
         switch ( arch_func_id )
         {
         case ARM_SMCCC_ARCH_WORKAROUND_1_FID:
-            if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) )
-                ret = 0;
+            /*
+             * Workaround 3 is also mitigating spectre v2 so advertise that we
+             * support Workaround 1 if we do Workaround 3 on exception entry.
+             */
+            if ( cpus_have_cap(ARM_HARDEN_BRANCH_PREDICTOR) ||
+                 cpus_have_cap(ARM_WORKAROUND_BHB_SMCC_3) )
+                ret = ARM_SMCCC_SUCCESS;
             break;
         case ARM_SMCCC_ARCH_WORKAROUND_2_FID:
             switch ( get_ssbd_state() )
@@ -126,7 +131,7 @@  static bool handle_arch(struct cpu_user_regs *regs)
             break;
         case ARM_SMCCC_ARCH_WORKAROUND_3_FID:
             if ( cpus_have_cap(ARM_WORKAROUND_BHB_SMCC_3) )
-                ret = 0;
+                ret = ARM_SMCCC_SUCCESS;
             break;
         }