Re: [Xen-devel] [PATCH] x86: clear RDRAND CPUID bit on AMD family 15h/16h

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 29/08/2019 13:39, Juergen Gross wrote:
> On 29.08.19 14:27, Jan Beulich wrote:
>> On 29.08.2019 14:01, Juergen Gross wrote:
>>> On 29.08.19 13:42, Andrew Cooper wrote:
>>>> On 29/08/2019 10:28, Jan Beulich wrote:
>>>>> Inspired by Linux commit c49a0a80137c7ca7d6ced4c812c9e07a949f6f24:
>>>>>
>>>>>       There have been reports of RDRAND issues after resuming from
>>>>> suspend on
>>>>>       some AMD family 15h and family 16h systems. This issue stems
>>>>> from a BIOS
>>>>>       not performing the proper steps during resume to ensure
>>>>> RDRAND continues
>>>>>       to function properly.
>>>>>
>>>>>       Update the CPU initialization to clear the RDRAND CPUID bit
>>>>> for any family
>>>>>       15h and 16h processor that supports RDRAND. If it is known
>>>>> that the family
>>>>>       15h or family 16h system does not have an RDRAND resume
>>>>> issue or that the
>>>>>       system will not be placed in suspend, the "cpuid=rdrand"
>>>>> kernel parameter
>>>>>       can be used to stop the clearing of the RDRAND CPUID bit.
>>>>>
>>>>>       Note, that clearing the RDRAND CPUID bit does not prevent a
>>>>> processor
>>>>>       that normally supports the RDRAND instruction from executing
>>>>> it. So any
>>>>>       code that determined the support based on family and model
>>>>> won't #UD.
>>>
>>> Yeah, but it will no longer see random numbers after resume.
>>
>> That's the implication; note that I've taken the text from the
>> Linux commit.
>>
>>>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>>>> ---
>>>>> Slightly RFC, in particular because of the change to
>>>>> parse_xen_cpuid():
>>>>> Alternative approach suggestions are welcome.
>>>>
>>>> This issue was on my radar, but I hadn't got around to starting a
>>>> patch yet.
>>>>
>>>> I was wondering if we could perhaps default it to whether S3 was
>>>> available, but looking at the code which prints "ACPI sleep modes:
>>>> S3",
>>>> it doesn't appear to be related to any real ACPI information.
>>>
>>> Wouldn't it make more sense to inhibit suspend/resume instead?
>>
>> That's an alternative option. But if anything I'd see us providing
>> both, not the one you suggest instead of what the patch here does.
>>
>>> I'm quite sure a machine running Xen is very rarely put to S3.
>>
>> I'm not at all sure about this.
>
> Suspend/resume in Xen was broken for more than 3 months in the late 4.12
> development phase and nobody noticed it. Only when I started my core
> scheduling series I came across the issue.

OpenXT and Qubes will come after you with sticks...

For traditional server scenarios, S3 is rare to non-existent, but Xen
also runs in a number of well known cases where S3 is very important. 
The reason S3 breaks frequently (see also Jan's IRQ adjustments broke it
for a while in staging), is because none of the developers who work in
these areas use it.

I now have a test scenario using a CoffeeLake system sat on my desk, but
it only gets testing when I'm poking the low level code, or when I
suspect a change might have an impact.

~Andrew

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