Re: [PATCH 1/2] xen/*/nospec: Provide common versions of evaluate_nospec/block_speculation

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.03.2024 18:40, Julien Grall wrote:
> Hi Andrew,
> 
> On 04/03/2024 17:07, Andrew Cooper wrote:
>> On 04/03/2024 4:55 pm, Jan Beulich wrote:
>>> On 04.03.2024 17:46, Julien Grall wrote:
>>>> On 04/03/2024 16:41, Jan Beulich wrote:
>>>>> On 04.03.2024 17:31, Julien Grall wrote:
>>>>>> On 04/03/2024 16:10, Andrew Cooper wrote:
>>>>>>> It is daft to require all architectures to provide empty 
>>>>>>> implementations of
>>>>>>> this functionality.
>>>>>> Oleksii recenlty sent a similar patch [1]. This was pushed back because
>>>>>> from naming, it sounds like the helpers ought to be non-empty on every
>>>>>> architecture.
>>>>>>
>>>>>> It would be best if asm-generic provides a safe version of the helpers.
>>>>>> So my preference is to not have this patch. This can of course change if
>>>>>> I see an explanation why it is empty on Arm (I believe it should contain
>>>>>> csdb) and other arch would want the same.
>>>>> Except that there's no new asm-generic/ header here (as opposed to how
>>>>> Oleksii had it). Imo avoiding the need for empty stubs is okay this way,
>>>>> when introducing an asm-generic/ header would not have been. Of course
>>>>> if Arm wants to put something there rather sooner than later, then
>>>>> perhaps the functions better wouldn't be removed from there, just to then
>>>>> be put back pretty soon.
>>>> I am confused. I agree the patch is slightly different, but I thought
>>>> the fundamental problem was the block_speculation() implementation may
>>>> not be safe everywhere. And it was best to let each architecture decide
>>>> how they want to implement (vs Xen decide for us the default).
>>>>
>>>> Reading the original thread, I thought you had agreed with that
>>>> statement. Did I misinterpret?
>>> Yes and no. Whatever is put in asm-generic/ ought to be correct and safe
>>> by default, imo. The same doesn't apply to fallbacks put in place in
>>> headers in xen/: If an arch doesn't provide its own implementation, it
>>> indicates that the default (fallback) is good enough. Still I can easily
>>> see that other views are possible here ...
>>
>> With speculation, there's absolutely nothing we can possibly do in any
>> common code which will be safe generally.
>>
>> But we can make it less invasive until an architecture wants to
>> implement the primitives.
> 
> I understand the goal. However, I am unsure it is a good idea to provide 
> unsafe just to reduce the arch specific header by a few lines. My 
> concern is new ports may not realize that block_speculation() needs to 
> be implemented. This could end to a preventable XSA in the future.
> 
> I guess the risk could be reduced if we had some documentation 
> explaining how to port Xen to a new architecture (I am not asking you to 
> write the doc).

But that's precisely the difference I'm trying to point out between having
a stub header in asm-generic/ vs having the fallback in xen/nospec.h: This
way an arch still has to supply asm/nospec.h, and hence they can be
expected to consider what needs putting there and what can be left to the
fallbacks (whether just "for the time being" is a separate question).
Whereas allowing to simply point at the asm-generic/ header is (imo) far
more likely to have only little thought applied ("oh, there is that
generic header, let's just use it").

Yet as said, the line between the two can certainly be viewed as blurred.

Jan