Xen project Mailing List

Re: [PATCH v3 1/6] xen: implement byteswap

To: Julien Grall <julien@xxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>

From: Lin Liu （刘林） <lin.liu@xxxxxxxxxx>

Date: Tue, 10 May 2022 12:10:26 +0000

Accept-language: zh-CN, en-US

Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none

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=PFKuD9etltBqEaM+5XMhtGOpO/FTPxSZggd8NgZ9YC8=; b=ZE61uJIRHrvuhBGpVdEE7zr1JKeCoUGwt/0HLzQ9EP2OnP/HuAtWfJG1wB4cV2ri1Ckn8xu2wz8jp3wEyPwARjoUqdRbQ87W2leaDPnaG0GQaVHHjYBU0UVQGPRSdLhQkg1gEmEl9IQcd63AqFNdV/NOTqihbbaFBfNQQSUS6KI2LIv5E8XhN5nwYZ2SAyZVG6BpcbEpJ8Axv12CTyr02SexiQFKWd1rNKdFF7I7YRrQN0ZDjQQOOeHpCzHKPE8txSH+v7a4y+mEl7Os1nlPY5MgjuvJITr4rQBgfMNsoEnV6/HFpIA2nqmcR9ohA/X968rdwlk746Rac+zy0UfuIA==

Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=juRWu/OIiMwGb2kOYrN/OSp0qwuFFDZuF6y6uQQCFT/SBDRJaD0jeuyXs1UpP8lM38m2FXFxO7JQd/Q0gBCueXLTfnsnecYC6x1KYpAPiXgGqtvZzp6XhoG+Rt7nBPmIU4sEHBNNAjVHHX5Ypsmb7Xvr3QMJoajkOQFO830IEdoySoAe5RYpvbM/xNMlTMwyT6HfaGUN6tY3a+F5ENie2unngjpS6n0y/wjgLTDTdIEOwh77iBT3kbkPFrqqrtS7RVXn4Q5qds4vzw5PvspuKBjCbHBLbgfJ22LJgcHtUwl0QfAFXJSRBO1GyVi7ki0VQzN/QR8VNrSLSvqxJ43urg==

Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;

Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monne <roger.pau@xxxxxxxxxx>

Delivery-date: Tue, 10 May 2022 12:10:39 +0000

Ironport-data: A9a23:AaKl/KjOypke1SvRwQtLkLNWX161qBEKZh0ujC45NGQN5FlHY01je htvUD3VOayIMzP1fIwlbIq0o0pQvZKGxtEySVE9rytgEiMb9cadCdqndUqhZCn6wu8v7a5EA 2fyTvGacajYm1eF/k/F3oDJ9CU6jefSLlbFILas1hpZHGeIcw98z0M68wIFqtQw24LhXlrV4 YmaT/D3YzdJ5RYlagr41IrbwP9flKyaVOQw5wFWiVhj5TcyplFNZH4tDfjZw0jQG+G4KtWSV efbpIxVy0uCl/sb5nFJpZ6gGqECaua60QFjERO6UYD66vRJjnRaPqrWqJPwwKqY4tmEt4kZ9 TlDiXC/YQd4Z53W3+00aTJjTHh9Lath4ZbpHmfq5KR/z2WeG5ft69NHKRlqeKgnoKNwC2wI8 uEEIjcQaBzFn/ix3L+wVuhrgIIkMdXvO4Qc/HpnyFk1D95/GcyFH/qMuoQehWhv7ixNNa+2i 84xbTtpKhDBfjVEO0sNCYJ4l+Ct7pX6W2IB8w3O+vFti4TV5F1Q4J/yYeuOQMPJZdh0s2iR5 UvMzU2sV3n2M/Tak1Jp6EmEhPLLnC79cJIfEvu/7PECqE2ewCkfBQMbUXO/oOKlkQiuVtRHM UsW9yEy668o+ySDVcLhVhe1pHqFuB80WNdKFeA+rgaXxcL86QuDGnINSDIHbdU8rdI3XhQjz FrPlNTsbRRRt7mSRWOY562jhzq4MigILkcPfSYBCwAC5rHLoos+kxbORdZLC7Oug5v+HjSY6 y+OhDgzgfMUl8Fj6kmg1VXOgjbpo4eTSAcwvlXTRjj8sVw/Y5O5bYu171Sd9exHMIuSUliGu j4DhtSa6+cNS5qKkURhXdkwIV1g3N7dWBW0vLKlN8BJG+iFk5J7Qb1t3Q==

Ironport-hdrordr: A9a23:UcR9cK+ELgkxq9lSyR1uk+Ftdb1zdoMgy1knxilNoENuH/Bwxv rFoB1E73TJYW4qKQodcdDpAtjifZquz+8O3WBxB8bpYOCCggeVxe5ZnOzfKlHbehEWs9QtrZ uIEJIOReEYb2IK6/oSiTPQe7lP/DDEytHQuQ609QYOcegeUdAF0+4PMHf/LqQZfml7LKt8MK DZyttMpjKmd3hSRN+8HGM5U+/KoMCOvI76YDYdbiRXpzWmvHeN0vrXAhKY1hARX3dk2rE561 XIlAT/++GKr+y78BnBzGXehq4m1ucJi+EzRfBkuPJlaQkEuTzYJriJnIfy+QzdldvfqGrCVu O85yvIcf4DrE85NVvF3CcFkzOQrArGrUWShWNwyEGT3vDRVXY0DdFMipledQac4008vMtk2K YOxG6BsYFLZCmw1RgVyuK4IC2CrHDE10bKUNRj/EB3QM8bcvtcvIYf9ERaHNMJGz/78pkuFK 1rANvH7PhbfFuGZzSB11Mfi+CETzA2BFOLU0ICssua33xfm2141VIRwIgakm0b/JwwRpFY76 DPM7hulrtJUsgKBJgNTNspUI+yECjAUBjMOGWdLRDuE7wGIWvEr9rt7LA89IiRCeg1JVsJ6e H8uX9jxB8PkhjVeLOzNbVwg2DwaXT4WyjxwcdD4JU8sqHgRdPQQF++dGw=

List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Thread-index: AQHYZFb0Ik5G7uZNNE+lQn3ZaONJQ60X9FWAgAAQFI4=

Thread-topic: [PATCH v3 1/6] xen: implement byteswap

Hi,

On 10/05/2022 11:15, Lin Liu wrote:
> swab() is massively over complicated and can be simplified by builtins.

NIT: "by builtins" -> "by re-implementing using compiler builtins".

> The compilers provide builtin function to swap bytes.
> * gcc:   https://nam04.safelinks.protection.outlook.com/?url="">
> * clang: https://nam04.safelinks.protection.outlook.com/?url="">
> This patch simplify swab() with builtins and fallback for old compilers.
>
> Signed-off-by: Lin Liu <lin.liu@xxxxxxxxxx>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien@xxxxxxx>
> Cc: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> Cc: Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Cc: George Dunlap <george.dunlap@xxxxxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Wei Liu <wl@xxxxxxx>
> Cc: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
> Changes in v3:
> - Check __has_builtin instead of GNUC version
>
> Changes in v2:
> - Add fallback for compilers without __builtin_bswap
> - Implement with plain C instead of macros
> ---
>   xen/arch/arm/include/asm/byteorder.h | 14 ++-----
>   xen/arch/x86/include/asm/byteorder.h | 34 ++---------------
>   xen/include/xen/byteorder.h          | 56 ++++++++++++++++++++++++++++
>   xen/include/xen/byteswap.h           | 44 ++++++++++++++++++++++
>   xen/include/xen/compiler.h           | 12 ++++++
>   5 files changed, 120 insertions(+), 40 deletions(-)
>   create mode 100644 xen/include/xen/byteorder.h
>   create mode 100644 xen/include/xen/byteswap.h
>
> diff --git a/xen/arch/arm/include/asm/byteorder.h b/xen/arch/arm/include/asm/byteorder.h
> index 9c712c4788..622eeaba07 100644
> --- a/xen/arch/arm/include/asm/byteorder.h
> +++ b/xen/arch/arm/include/asm/byteorder.h
> @@ -1,16 +1,10 @@
>   #ifndef __ASM_ARM_BYTEORDER_H__
>   #define __ASM_ARM_BYTEORDER_H__
>
> -#define __BYTEORDER_HAS_U64__
> +#ifndef __BYTE_ORDER__
> +   #define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__
> +#endif
>
> -#include <xen/byteorder/little_endian.h>
> +#include <xen/byteorder.h>
>
>   #endif /* __ASM_ARM_BYTEORDER_H__ */
> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */

>> This change looks unrelated to this patch. Can you explain it?

Do you mean following code block? Yes, it is unrelated, I removed it as I found some files does not include such field.

Will revert such field in V4.

> -/*
> - * Local variables:
> - * mode: C
> - * c-file-style: "BSD"
> - * c-basic-offset: 4
> - * indent-tabs-mode: nil
> - * End:
> - */