Re: [PATCH v2] x86/hvm: Disallow CR0.PG 1->0 transitions when CS.L=1

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/04/2023 11:31 am, Roger Pau Monné wrote:
> On Thu, Apr 13, 2023 at 04:00:09PM +0100, Andrew Cooper wrote:
>> The Long Mode consistency checks exist to "ensure that the processor does not
>> enter an undefined mode or state that results in unpredictable behavior".  
>> APM
>> Vol2 Table 14-5 "Long-Mode Consistency Checks" lists them, but there is no 
>> row
>> preventing the OS from trying to exit Long mode while in 64bit mode.  This
>> could leave the CPU in Protected Mode with an %rip above the 4G boundary.
>>
>> Experimentally, AMD CPUs really do permit this state transition.  An OS which
>> tries it hits an instant SHUTDOWN, even in cases where the truncation I 
>> expect
>> to be going on behind the scenes ought to result in sane continued execution.
>>
>> Furthermore, right from the very outset, the APM Vol2 14.7 "Leaving Long 
>> Mode"
>> section instructs peoples to switch to a compatibility mode segment first
>> before clearing CR0.PG, which does clear out the upper bits in %rip.  This is
>> further backed up by Vol2 Figure 1-6 "Operating Modes of the AMD64
>> Architecture".
>>
>> Either way, this appears to have been a genuine oversight in the AMD64 spec.
>>
>> Intel, on the other hand, rejects this state transition with #GP.
>>
>> Between revision 71 (Nov 2019) and 72 (May 2020) of SDM Vol3, a footnote to
>> 4.1.2 "Paging-Mode Enable" was altered from:
>>
>>   If CR4.PCIDE= 1, an attempt to clear CR0.PG causes a general-protection
>>   exception (#GP); software should clear CR4.PCIDE before attempting to
>>   disable paging.
>>
>> to
>>
>>   If the logical processor is in 64-bit mode or if CR4.PCIDE= 1, an attempt 
>> to
>>   clear CR0.PG causes a general-protection exception (#GP). Software should
>>   transition to compatibility mode and clear CR4.PCIDE before attempting to
>>   disable paging.
>>
>> which acknowledges this corner case, but there doesn't appear to be any other
>> discussion even in the relevant Long Mode sections.
>>
>> So it appears that Intel spotted and addressed the corner case in IA-32e 
>> mode,
>> but were 15 years late to document it.
>>
>> Xen was written to the AMD spec, and misses the check.  Follow the Intel
>> behaviour, because it is more sensible and avoids hitting a VMEntry failure.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> ---
>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Wei Liu <wl@xxxxxxx>
>>
>> v2:
>>  * Restrict to when Long Mode is enabled.
> Maybe the subject also needs to be slightly edited to mention CS.L=1
> and Long Mode enabled?  Or just mention long mode instead of the code
> segment status?
>
> Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks.  I think "Disallow CR0.PG 1->0 transitions in 64bit mode" is the
most concise way of tweaking the subject.

~Andrew