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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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, Roger.