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

Re: [PATCH v2 0/7] Implement byteswap and update references


  • To: Lin Liu <lin.liu@xxxxxxxxxx>, <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
  • Date: Fri, 22 Oct 2021 12:50:14 +0100
  • 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=H9bfAxHq9C4djbYaVFzfI7/T3GJLqJyrIdorviNKW4s=; b=CrdN8ryuUEAAUSs3lM43jqmOSM7UTx9H5d8Y6vddYh6sHDbEsR3bL+2XKf/0rIC7B+sNSwzxU1rAauhpEm3bpPX4sV0O8ilVW//GpZ7cz3cNk8Q8oKYkBYimzI+6PvHwtAdApOKFVDIBaBwrjFeRFScthWghq/opRJ8Y5/0DqpjfIQ78bqEn+Vh80X+h9AFGPwNTEgqiKvJs/yMsaRbDNNeKN0wV8t7jbD2E8NZNRCnYwISmixBw47xkdJlhHThtG1vuFWZQK+UPp3XQ/DKku/ZwzUP2+LSaIpTjf0VuBIUvU4c8vQ+FIKpOdqCxbCqTT/CRHzTuDJmjwwFX4yyLbQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Gtv+TO+hW8jL2ZjnDuHKXGaLKDIopgkoLfYgZnMc5wr3BiJQKuSrdj9OTA+eqvMJ5jE0Aj0y7ll7WTwoMmixnoBin2EnKlKdfSIlV7WAknHDzARmFZuA1cgxEt+o7jLdTc7tMK98F2yU1+ZgHAV8PBTukg7y+f9JcBWXRu+O6KZDl88AtFuoieWenZGRR07HoJ9i46C3lMhCD450X9Qw/RbIgIZ47RLH6+T+oSFiz45sogh12SI9VpEHBS2hfd9IOFNnSZEOcXXo8dmsqhAuqIiFt04Sgm5tFvgf+F+1U4EEK15nUT1M/BC3z0ghKs6gFxqGQEQfIRs05puWj6aCvg==
  • Authentication-results: esa3.hc3370-68.iphmx.com; dkim=pass (signature verified) header.i=@citrix.onmicrosoft.com
  • Cc: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Julien Grall <julien@xxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Fri, 22 Oct 2021 11:50:41 +0000
  • Ironport-data: A9a23:9CLL0K/Zbcwtzqa96d02DrUDjXiTJUtcMsCJ2f8bNWPcYEJGY0x3y mQWC2HVPviJNGv3fIoiYIq08k0O7JaHydViHQs+rS88E34SpcT7XtnIdU2Y0wF+jyHgoOCLy +1EN7Es+ehtFie0Si9AttENlFEkvU2ybuOU5NXsZ2YhGmeIdA970Ug6wrZj2NYy6TSEK1jlV e3a8pW31GCNg1aYAkpMg05UgEoy1BhakGpwUm0WPZinjneH/5UmJMt3yZWKB2n5WuFp8tuSH I4v+l0bElTxpH/BAvv9+lryn9ZjrrT6ZWBigVIOM0Sub4QrSoXfHc/XOdJFAXq7hQllkPhg6 YoSvLPrbD4YL73BtfQtAjkFLwBxaPguFL/veRBTsOSWxkzCNXDt3+9vHAc9OohwFuRfWD8Us 6ZCcXZUM07F17neLLGTE4GAguwKKsXxMZxZkXZn1TzDVt4tQIzZQrWM7thdtNs1rp0TTaqAN ppHAdZpRArvTh5qBkxGNMI/l+Sx3Uf5az4E8U3A8MLb5ECMlVcsgdABKuH9e8OIbdVYmF6Co WDL9Hi/BQsVXPSdwzeY9nOnhsfUgDj2HokVEdWQ+uZxhVyPxkQaEBAMSUaguv69l1K/XNREb UcT/0IGvaU0sUCmUNT5dxm5u2Kf+A4RXcJKFO834x3LzbDbiy6VCWQJQSRIQMA3v88xAzox3 xmGmM2BLQJotLqZWHeM7IC+pDm5OTUWBWIabCpCRgwAi/H8pKkjgxSJScxseIaulcH8Ezz0x zGMrQA9iq8VgMpN0L+0lXjFnjatq57hXgMzoALNUQqN7QR/foepIZOp7Vvz6uxJJ4KUCFKGu RA5d9O2tb5US8vXzWrUHbtLTOrBC+u53CP0kH8wQJAOrBGUym+zcK1A0GBeeXtIPZNREdP2W 3P7tQRU7Z5VGXKla65rfo68Y/gXIbjc+cfNDa+MMIIfCnRlXErepns2PB/Pt4z4uBF0yflXB HuNTSq74Z/244xcxz2qW/xV77Yvwi0vrY84bcGml0r5uVZyiXj8dFvkDLdsRrxmhE9niF+Mm zq6Cyds408DOAEZSnKPmbP/1XhQcRAG6Wne8qS7jNKrLAt8A30GAPTM274ncIENt/0LzbuYo ijiChcDlQuXaZj7xeOiMCwLhFTHBs4XkJ7GFXZ0YQbAN4YLMe5DE5vzh7NoJOJ6pYSPPNZ/T uUfetXoPxi8Ym+vxtjpVrGk9NYKXE3y3WqmZnP5CBBiL88Ib1GYobfMI1qwnBTi+wLq7KPSV ZX7jViFKXfCLiw/ZPvrhAWHlgLo4yJFwb8qAyMl4LB7IS3RzWSjEASo5tcfKMAQMxTTgDyc0 gedGxADoufR5YQy9bH0aWqs9u9FysNyQRhXGXf197GzOXWI92av29YYAu2JYSrcRCX//6D7P bdZyPT1MfsmmldWstUjT+Y3nPxmv9a/9aVHyglEHWnQawj5AL1XPXTbj9JEsbdAx+EFtFLuC F6P4NRTJZ6AJNjhTAwKPAMgY+najaMUlzDe4O4bOkL/4CMrrrOLXV8LZ0uHiTBHLaszO4Qgm L9ztMkT4g25qxwrLtfZ0XwEqzXSdiQNCvx1uIsbDYnnjhsQ5mtDOZGMWDXr5JyvaslXNhV4K DGjm6ef1a9XwVDPciRvGCGVj/Zdn5kHpDtD0EQGewaSgtPAi/I6gE9R/DAwQlgHxxlLybsua G1iNkkzLqSS5TZ4wsNEWjn0SQ1GARSY/G33ykcIyzKFHxX5CDSVIT1vI/uJ8WAY73lYL2pS8 7yvwWr4VSrnIZPq1SwoVE858/HuQLSdLOEZdBxLyyhdI6QHXA==
  • Ironport-hdrordr: A9a23:fsDzPqqHVYYzd/XqvSxb+rwaV5oGeYIsimQD101hICG9E/bo7f xG+c5xvyMc5wx8ZJheo6HkBEDtex7hHOdOkO0s1O6ZLW7bUQiTXfpfBPXZrQEIcheWntK1s5 0QC5SWYOeAdGSS5vyU3ODXKbcdKM7uytHKuQ6n9RdQpPZRGsNdBzESMHf8LnFL
  • Ironport-sdr: ZQYD7joum40Hn9lLF8T0+iBYNKsH2NoTsbMUjvT1NCjzlctiHZidO1H5B4ph+Kkc2NlRVikgcF 7pDICL3R74D72WBIbzLgRGuCPhXiN+pclvRAXFmNcY6mVDfwRb2u73v9/JdsEe6/4EjSQeBAZW ZF3PLV3rpBkEIFrYRgqnDfXVnCRed5aGDUEtWBMkHhX/dhsYNbjNTv0lbBleqqyeCOHbSv8UBQ cpCSkKlv9f7M95wRzyNeDzHvxBbaL2Xo0j0YYp0w6sjYfXXwYDqSsNiNCwXj0SsHsy3I0FNl/d 9loWuYF+f/rGTcE9g/yz23Z5
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 22/10/2021 11:47, Lin Liu wrote:
> The swab() is massively over complicated
> Simplify it with compiler builtins and fallback to plain C function
> if undefined.
> Update components to switch to this new swap bytes.
>
> <snip>
>  34 files changed, 150 insertions(+), 646 deletions(-)

It is worth saying a couple of things.

x86's ___arch__swab64 is wrong.  Well - it was mostly ok for 32bit
builds of Xen, and is not ok for 64bit builds.  As a consequence, this
series nets an improvement of:

$ ../scripts/bloat-o-meter xen-syms-before xen-syms-after
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-54 (-54)
Function                                     old     new   delta
elf_access_unsigned                          173     151     -22
unlzo                                       1128    1096     -32
Total: Before=3803328, After=3803274, chg -0.00%

because the code generation for bswap64 goes from:

ffff82d0402059b0 <_bswap64>:
ffff82d0402059b0:    48 89 f8                 mov    %rdi,%rax
ffff82d0402059b3:    48 c1 e8 20              shr    $0x20,%rax
ffff82d0402059b7:    0f cf                    bswap  %edi
ffff82d0402059b9:    0f c8                    bswap  %eax
ffff82d0402059bb:    97                       xchg   %eax,%edi
ffff82d0402059bc:    48 89 c2                 mov    %rax,%rdx
ffff82d0402059bf:    89 f8                    mov    %edi,%eax
ffff82d0402059c1:    48 c1 e2 20              shl    $0x20,%rdx
ffff82d0402059c5:    48 09 d0                 or     %rdx,%rax
ffff82d0402059c8:    c3                       retq 

to

ffff82d0402059b0 <_bswap64>:
ffff82d0402059b0:    48 89 f8                 mov    %rdi,%rax
ffff82d0402059b3:    48 0f c8                 bswap  %rax
ffff82d0402059b6:    c3                       retq 

Almost all byteswapping is done on 32bit quantities, not 64, which is
why the delta is so small.

However, it also drops 500 lines of code, which is a demonstration of
how silly the swab() infrastructure was.  It also removes the need for
per-arch code to do any of this.

I'd say its safe to go into 4.16, but I'll understand if others want to
push back on that at this point in the release.

~Andrew

 


Rackspace

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