[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 3/3] xen/arm: Add sb instruction support


  • To: Julien Grall <julien@xxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Mon, 9 May 2022 10:49:38 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=UxVaQMx+lv0ITeN6nkEB0ETAt3MqVsfGx9fx5K+EEsI=; b=Wtj6VQhX+Cq37viFaceI1i6hpibrNscAAXYhGG/jcMxpK+9UNxToAvSEhVZ86unmCiyTINbZg+jb+6oWk52wxLBwFYtOef75mVSvNSTqJBZxrMozOyje1nPb5S4D/z9ZCz7MhusEf1INDgTOS9gQHYPQZTp9RnJFhOhx+5Rysupdm+G8EAYtkVbPIJyQKlKTKjO8yomiNfEJJItvNYndRA88VVrlxtKakOZa3kgi/lRnBtpDPDve/RVQ4ZNu8w2XlctBuIKXepg/ETlZFlupcbKkRQUJrCvo/quZ90f1MWTGFWNxeTKmqNZOXslSHg4mLXTnlVIdTvGgyGqKlFmKzg==
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=UxVaQMx+lv0ITeN6nkEB0ETAt3MqVsfGx9fx5K+EEsI=; b=fVK+HPkcE6aZpp+KzIHXZGCwAfCjhnZloCdazJ9QJalQqARRwo/eF0+NG3UxfBPOvpnTwD5IxJEeiz9Ir2V6Np8mylyavEAIDA0ts0mQ2PAxo5QHEFT+6/lnNmYAC+HWyZtb1Gseew8xHABl4qzubOvNMbPgdvue95VQspKpK881NsMbeHloSVszVqqUiyfFzoCRMIGXXivGuwYOROKTjVBxnxHXEIXpF25HrQ35GlHe3+JoIhAuQxORLeJPkm3JzhEDPN9NtH5lcvMdvod66wov70Vm87H4XMPvAJPdTLWP+iQohHD3H7gMFUKKuHLBD6833p9JdXMgIujcwrF2zA==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=XuGlKkjAAAShlHyDPRN2Bx5ZOy3UbugYEUcDFSvSmiQ1zV3udJBhGdvd0WlgXwW6rMgBDnpinC7ZRJViYzxua9az8gYag31yPNOXJQEj8Dj+lUoY4TPZeqZ8TGLqH/C2o/A48kxMEK/l2Et7GDocOHVqfsxcNhs3mELT/Or7LRRh95pNZTeJSGDC/9upwOmpyVOwkBOYX1rXRDpR18QiIdAh6C/prAj1CghkmGMbv+H5xbuOdyIcgT3qBXhXxQjqBS2WvYhcK8pZmnuiO9yIk3508OAV4r5fXwT/g+oFkIRIgvzch941t/7EMPqsPIi8AmA4nvBKDbSPvmonf4WasQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=MimHVU/747JlH8fqkjmImlwkT4MTgKTqzwegGtwA5RjQP1sDRjnfX5GhQLSSsopQSg2JADl2A3pRu1x2XPjQQ/0hztgKd2DfVWqTt76x3o6JjuHnb6k5n/Hv/uBHAmJnL+ZJ9HxYmM1ChECxPrhnXBuEXfh27CguLsiAQ577UE5qOzCtyozio1wZ+jlCLZ+epMYLUzMeTjJ7wevxoekzd84hIyhSsgc2jis2OqJHKBF4R0PxwMKePMxX2ZPqSfeOYp4rsiTzHIuH1uoPBOaQu7f7+OVpkf89QCQh6Ts1HbqAWYYIopEC8SiCWPjBSfPMX07YdtGj5KwJ7L+1fTsH/Q==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
  • Delivery-date: Mon, 09 May 2022 10:50:00 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHYXtGbLw8lUArxPEOQFl634Avmjq0NftKAgADToQCAAAuUAIAH/dMAgAAGbICAAAULAA==
  • Thread-topic: [PATCH 3/3] xen/arm: Add sb instruction support

Hi Julien,,

> On 9 May 2022, at 11:31, Julien Grall <julien@xxxxxxx> wrote:
> 
> 
> 
> On 09/05/2022 11:08, Bertrand Marquis wrote:
>> Hi Julien,
> 
> Hi,
> 
>>> On 4 May 2022, at 09:06, Julien Grall <julien@xxxxxxx> wrote:
>>> 
>>> 
>>> 
>>> On 04/05/2022 08:24, Bertrand Marquis wrote:
>>>> Hi Julien,
>>> 
>>> Hi Bertrand,
>>> 
>>>>> On 3 May 2022, at 19:47, Julien Grall <julien@xxxxxxx> wrote:
>>>>>> A new cpuerrata capability is introduced to enable the alternative
>>>>> 
>>>>> 'sb' is definitely not an erratum. Errata are for stuff that are meant to 
>>>>> be specific to one (or multiple) CPU and they are not part of the 
>>>>> architecture.
>>>>> 
>>>>> This is the first time we introduce a feature in Xen. So we need to add a 
>>>>> new array in cpufeature.c that will cover 'SB' for now. In future we 
>>>>> could add feature like pointer auth, LSE atomics...
>>>> I am not quite sure why you would want to do that.
>>>> Using the sb instruction is definitely something to do to solve erratas, 
>>>> if a CPU is not impacted by those erratas, why would you want to use this ?
>>> 
>>> I agree that SB is used to solve errata but the instruction itself is not a 
>>> workaround (it may be part of it though). Instead, this is a more efficient 
>>> way to prevent speculation and will replace dsb/isb.
>>> 
>>> Speculation is never going to disappear from processor. So, in the future, 
>>> there might be valid reason for us to say "We don't want the processor to 
>>> speculate". This would mean using SB.
>> If the need arise then we will check depending on that how we can support it 
>> but in the current status as there is no use case I don’t think we need that.
> 
> It is not clear how I should read this answer... If you add SB in 
> cpuerrata.c, then a user will start to see message like:
> 
> "enabled workaround for Speculation Barrier".
> 
> Which is completely bogus. Replacing "dsb; isb" with "sb" is mostly an 
> optimization and none of the current use will end up to be architecturaly 
> executed.

So ultimately something like this is what you are looking for ?

diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index e744abe800..7c3e5141a6 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -681,9 +681,12 @@ static const struct arm_cpu_capabilities arm_errata[] = {
         .capability = ARM64_WORKAROUND_AT_SPECULATE,
         MIDR_ALL_VERSIONS(MIDR_CORTEX_A55),
     },
+};
+
+static const struct arm_cpu_capabilities arm_features[] = {
 #ifdef CONFIG_ARM_64
     {
-        .desc = "Speculation barrier (SB)",
+        .desc = "Speculation barrier instruction (SB)",
         .capability = ARM64_HAS_SB,
         .matches = has_sb_instruction,
     },
@@ -694,6 +697,7 @@ static const struct arm_cpu_capabilities arm_errata[] = {
 void check_local_cpu_errata(void)
 {
     update_cpu_capabilities(arm_errata, "enabled workaround for");
+    update_cpu_capabilities(arm_features, "enabled support for");
 }

> 
> I appreciate this is more work to add cpufeature.c. However, AFAIK, there are 
> no rush to get this optimization in (see why above) and muddy (even 
> temporarily) the logs.

The upper I am ok to do but if we want to design something new to handle 
possible features and move this to cpufeature, we will need to step back and 
check more possible use cases and how we want to handle them.

This is the part which I do not want to handle in this serie.
Point here is to enable the use of the proper instruction when possible on new 
processors (namely Neoverse N2 at the moment).

Is doing it like this (maybe with a TODO to say that this should be moved to 
cpufeature) ok for you ?

Cheers
Bertrand

> 
> Cheers,
> 
> -- 
> Julien Grall


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.