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

Re: [Xen-devel] [PATCH SpectreV1+L1TF v4 03/11] config: introduce L1TF_LFENCE option


  • To: Jan Beulich <JBeulich@xxxxxxxx>
  • From: Norbert Manthey <nmanthey@xxxxxxxxx>
  • Date: Mon, 28 Jan 2019 11:07:32 +0100
  • Autocrypt: addr=nmanthey@xxxxxxxxx; prefer-encrypt=mutual; keydata= mQINBFoJQc0BEADM8Z7hB7AnW6ErbSMsYkKh4HLAPfoM+wt7Fd7axHurcOgFJEBOY2gz0isR /EDiGxYyTgxt5PZHJIfra0OqXRbWuLltbjhJACbu35eaAo8UM4/awgtYx3O1UCbIlvHGsYDg kXjF8bBrVjPu0+g55XizX6ot/YPAgmWTdH8qXoLYVZVWJilKlTqpYEVvarSn/BVgCbIsQIps K93sOTN9eJKDSqHvbkgKl9XG3WsZ703431egIpIZpfN0zZtzumdZONb7LiodcFHJ717vvd89 3Hv2bYv8QLSfYsZcSnyU0NVzbPhb1WtaduwXwNmnX1qHJuExzr8EnRT1pyhVSqouxt+xkKbV QD9r+cWLChumg3g9bDLzyrOTlEfAUNxIqbzSt03CRR43dWgfgGiLDcrqC2b1QR886WDpz4ok xX3fdLaqN492s/3c59qCGNG30ebAj8AbV+v551rsfEba+IWTvvoQnbstc6vKJCc2uG8rom5o eHG/bP1Ug2ht6m/0uWRyFq9C27fpU9+FDhb0ZsT4UwOCbthe35/wBZUg72zDpT/h5lm64G6C 0TRqYRgYcltlP705BJafsymmAXOZ1nTCuXnYAB9G9LzZcKKq5q0rP0kp7KRDbniirCUfp7jK VpPCOUEc3tS1RdCCSeWNuVgzLnJdR8W2h9StuEbb7hW4aFhwRQARAQABtCROb3JiZXJ0IE1h bnRoZXkgPG5tYW50aGV5QGFtYXpvbi5kZT6JAj0EEwEIACcFAloJQc0CGyMFCQlmAYAFCwkI BwIGFQgJCgsCBBYCAwECHgECF4AACgkQZ+8yS8zN62ajmQ/6AlChoY5UlnUaH/jgcabyAfUC XayHgCcpL1SoMKvc2rCA8PF0fza3Ep2Sw0idLqC/LyAYbI6gMYavSZsLcsvY6KYAZKeaEriG 7R6cSdrbmRcKpPjwvv4iY6G0DBTeaqfNjGe1ECY8u522LprDQVquysJIf3YaEyxoK/cLSb0c kjzpqI1P9Vh+8BQb5H9gWpakbhFIwbRGHdAF1roT7tezmEshFS0IURJ2ZFEI+ZgWgtl1MBwN sBt65im7x5gDo25h8A5xC9gLXTc4j3tk+3huaZjUJ9mCbtI12djVtspjNvDyUPQ5Mxw2Jwar C3/ZC+Nkb+VlymmErpnEUZNltcq8gsdYND4TlNbZ2JhD0ibiYFQPkyuCVUiVtimXfh6po9Yt OkE0DIgEngxMYfTTx01Zf6iwrbi49eHd/eQQw3zG5nn+yZsEG8UcP1SCrUma8p93KiKOedoL n43kTg4RscdZMjj4v6JkISBcGTR4uotMYP4M0zwjklnFXPmrZ6/E5huzUpH9B7ZIe/SUu8Ur xww/4dN6rfqbNzMxmya8VGlEQZgUMWcck+cPrRLB09ZOk4zq9i/yaHDEZA1HNOfQ9UCevXV5 7seXSX7PCY6WDAdsT3+FuaoQ7UoWN3rdpb+064QKZ0FsHeGzUd7MZtlgU4EKrh25mTSNZYRs nTz2zT/J33e5Ag0EWglBzQEQAKioD1gSELj3Y47NE11oPkzWWdxKZdVr8B8VMu6nVAAGFRSf Dms4ZmwGY27skMmOH2srnZyTfm9FaTKr8RI+71Fh9nfB9PMmwzA7OIY9nD73/HqPywzTTleG MlALmnuY6xFRSDmqmvxDHgWyzB4TgPWt8+hW3+TJKCx2RgLAdSuULZla4lia+NlS8WNRUDGK sFJCCB3BW5I/cocfpBEUqLbbmnPuD9UKpEnFcYWD9YaDNcBTjSc7iDsvtpdrBXg5VETOz/TQ /CmVs9h/5zug8O4bXxHEEJpCAxs4cGKxowBqx/XJfkwdWeo/LdaeR+LRbXvq4A32HSkyj9sV vygwt2OFEk493JGik8qtAA/oPvuqVPJGacxmZ7zKR12c0mnKCHiexFJzFbC7MSiUhhe8nNiM p6Sl6EZmsTUXhV2bd2M12Bqcss3TTJ1AcW04T4HYHVCSxwl0dVfcf3TIaH0BSPiwFxz0FjMk 10umoRvUhYYoYpPFCz8dujXBlfB8q2tnHltEfoi/EIptt1BMNzTYkHKArj8Fwjf6K+nQ3a8p 1cWfkYpA5bRqbhbplzpa0u1Ex0hZk6pka0qcVgqmH31O2OcSsqeKfUfHkzj3Q6dmuwm1je/f HWH9N1gDPEp1RB5bIxPnOG1Z4SNl9oVQJhc4qoJiqbvkciivYcH7u2CBkboFABEBAAGJAiUE GAEIAA8FAloJQc0CGwwFCQlmAYAACgkQZ+8yS8zN62YU9Q//WTnN28aBX1EhDidVho80Ql2b tV1cDRh/vWTcM4qoM8vzW4+F/Ive6wDVAJ7zwAv8F8WPzy+acxtHLkyYk14M6VZ1eSy0kV0+ RZQdQ+nPtlb1MoDKw2N5zhvs8A+WD8xjDIA9i21hQ/BNILUBINuYKyR19448/41szmYIEhuJ R2fHoLzNdXNKWQnN3/NPTuvpjcrkXKJm2k32qfiys9KBcZX8/GpuMCc9hMuymzOr+YlXo4z4 1xarEJoPOQOXnrmxN4Y30/qmf70KHLZ0GQccIm/o/XSOvNGluaYv0ZVJXHoCnYvTbi0eYvz5 OfOcndqLOfboq9kVHC6Yye1DLNGjIVoShJGSsphxOx2ryGjHwhzqDrLiRkV82gh6dUHKxBWd DXfirT8a4Gz/tY9PMxan67aSxQ5ONpXe7g7FrfrAMe91XRTf50G3rHb8+AqZfxZJFrBn+06i p1cthq7rJSlYCqna2FedTUT+tK1hU9O0aK4ZYYcRzuTRxjd4gKAWDzJ1F/MQ12ftrfCAvs7U sVbXv2TndGIleMnheYv1pIrXEm0+sdz5v91l2/TmvkyyWT8s2ksuZis9luh+OubeLxHq090C hfavI9WxhitfYVsfo2kr3EotGG1MnW+cOkCIX68w+3ZS4nixZyJ/TBa7RcTDNr+gjbiGMtd9 pEddsOqYwOs=
  • Cc: Tim Deegan <tim@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>, Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>, Dario Faggioli <dfaggioli@xxxxxxxx>, Martin Pohlack <mpohlack@xxxxxxxxx>, Julien Grall <julien.grall@xxxxxxx>, David Woodhouse <dwmw@xxxxxxxxxxxx>, "Martin Mazein\(amazein\)" <amazein@xxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>, Julian Stecklina <jsteckli@xxxxxxxxx>, Bjoern Doebel <doebel@xxxxxxxxx>
  • Delivery-date: Mon, 28 Jan 2019 10:08:28 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Openpgp: preference=signencrypt

On 1/28/19 09:24, Jan Beulich wrote:
>>>> On 28.01.19 at 08:56, <nmanthey@xxxxxxxxx> wrote:
>> On 1/28/19 08:35, Jan Beulich wrote:
>>>>>> On 27.01.19 at 21:28, <nmanthey@xxxxxxxxx> wrote:
>>>> On 1/25/19 14:09, Jan Beulich wrote:
>>>>>>>> On 25.01.19 at 11:50, <nmanthey@xxxxxxxxx> wrote:
>>>>>> On 1/25/19 11:14, Jan Beulich wrote:
>>>>>>>>>> On 24.01.19 at 22:29, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>>>>>> Worse is the "evaluate condition, stash result, fence, use variable"
>>>>>>>> option, which is almost completely useless.  If you work out the
>>>>>>>> resulting instruction stream, you'll have a conditional expression
>>>>>>>> calculated down into a register, then a fence, then a test register and
>>>>>>>> conditional jump into one of two basic blocks.  This takes the perf 
>>>>>>>> hit,
>>>>>>>> and doesn't protect either of the basic blocks for speculative
>>>>>>>> mis-execution.
>>>>>>> How does it not protect anything? It shrinks the speculation window
>>>>>>> to just the register test and conditional branch, which ought to be
>>>>>>> far smaller than that behind a memory access which fails to hit any
>>>>>>> of the caches (and perhaps even any of the TLBs). This is the more
>>>>>>> that LFENCE does specifically not prevent insn fetching from
>>>>>>> continuing.
>>>>>>>
>>>>>>> That said I agree that the LFENCE would better sit between the
>>>>>>> register test and the conditional branch, but as we've said so many
>>>>>>> times before - this can't be achieved without compiler support. It's
>>>>>>> said enough that the default "cc" clobber of asm()-s on x86 alone
>>>>>>> prevents this from possibly working, while my over four year old
>>>>>>> patch to add a means to avoid this has not seen sufficient
>>>>>>> comments to get it into some hopefully acceptable shape, but also
>>>>>>> has not been approved as is.
>>>>>>>
>>>>>>> Then again, following an earlier reply of mine on another sub-
>>>>>>> thread, nothing really prevents the compiler from moving ahead
>>>>>>> and folding the two LFENCEs of the "both branches" model into
>>>>>>> one. It just so happens that apparently right now this never
>>>>>>> occurs (assuming Norbert has done full generated code analysis
>>>>>>> to confirm the intended placement).
>>>>>> I am happy to jump back to my earlier version without a configuration
>>>>>> option to protect both branches with a lfence instruction, using logic
>>>>>> operators.
>>>>> I don't understand this, I'm afraid: What I've said was to support
>>>>> my thinking of the && + || variant being identical in code and risk
>>>>> to that using ?: . I.e. I'm not asking you to switch back.
>>>> I understand that you did not ask. However, Andrew raised concerns, and
>>>> I analyzed the binary output for the variant with logical operators.
>>>> Hence, I'd like to keep that variant with the logical operators.
>>> But didn't you say earlier that there was no difference in generated
>>> code between the two variants?
>> Yes, for the current commit, and for the 1 compiler I used. Personally,
>> I prefer the logic operand variant. You seem to prefer the ternary
>> variant, and Andrew at least raised concerns there. I would really like
>> to move forward somehow, but currently it does not look really clear how
>> to achieve that.
> Well, being able to move forward implies getting a response to my
> reply suggesting that both variants are equivalent in risk. If there
> are convincing arguments that the (imo) worse (simply from a
> readability pov) is indeed better from a risk (of the compiler not
> doing what we want it to do) pov, I'd certainly give up my opposition.
I understand the readability concern. The C standard makes similar
promises about the semantics (left-to-right, and using sequence points).
The implementation in the end seems to be up to the compiler. The risk
is that future compilers treat the conditional operator differently than
the one I used today. I'm fine with what we have right now. Once I'm
done with a v5 candidate, I'll look into this comparison one more time.
>
>> I try to apply majority vote for each hunk that has been commented and
>> create a v5 of the series. I even think about separating the
>> introduction of eval_nospec and the arch_nospec_barrier macro into
>> another series to move faster with the array_index_nospec-based changes
>> first. Guidance is very welcome.
> I have no problem picking patches out of order for committing.
> For example I'd commit patches 10 and 11 of v4 as is once it
> has the necessary release manager ack. I notice only now that
> you didn't even Cc Jürgen. I guess I'll reply to the cover letter
> asking for his opinion on the series as a whole.

To be able to merge these patched independently, I will bring back the
patch that was listed in the XSA,
xsa289/0005-nospec-introduce-method-for-static-arrays.patch, as that
function is required by patch 10 and 11.

Best,
Norbert

>
> Jan
>




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrer: Christian Schlaeger, Ralf Herbrich
Ust-ID: DE 289 237 879
Eingetragen am Amtsgericht Charlottenburg HRB 149173 B

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

 


Rackspace

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