Re: [Xen-devel] [PATCH v2] x86/PV: fix/generalize guest nul selector handling

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

On Thu, Sep 28, 2017 at 08:41:28AM +0000, Jan Beulich wrote:
> Segment bases (and limits) aren't being cleared by the loading of a nul
> selector into a segment register on AMD CPUs. Therefore, if an
> outgoing vCPU has a non-zero base in FS or GS and the subsequent
> incoming vCPU has a non-zero but nul selector in the respective
> register(s), the selector value(s) would be loaded without clearing the
> segment base(s) in the hidden register portion.
> 
> Since the ABI states "zero" in its description of the fs and gs fields,
> it is worth noting that the chosen approach to fix this alters the
> written down ABI. I consider this preferrable over enforcing the
> previously written down behavior, as nul selectors are far more likely
> to be what was meant from the beginning.
> 
> The adjustments also eliminate an inconsistency between FS and GS
> handling: Old code had an extra pointless (gs_base_user was always zero
> when DIRTY_GS was set) conditional for GS. The old bitkeeper changeset
> has no explanation for this asymmetry.
> 
> Inspired by Linux commit e137a4d8f4dd2e277e355495b6b2cb241a8693c3.
> 
> Additionally for DS and ES a flat selector is being loaded prior to the
> loading of a nul one on AMD CPUs, just as a precautionary measure
> (we're not currently aware of ways for a guest to deduce the base of a
> segment register which has a nul selector loaded).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Add DS/ES handling.
> 
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -1237,6 +1237,18 @@ arch_do_vcpu_op(
>      return rc;
>  }
>  
> +/*
> + * Loading a nul selector does not clear bases and limits on AMD CPUs. Be on
> + * the safe side and re-initialize both to flat segment values before loading
> + * a nul selector.
> + */
> +#define preload_segment(seg, value) do {              \
> +    if ( !((value) & ~3) &&                           \
> +         boot_cpu_data.x86_vendor == X86_VENDOR_AMD ) \
> +        asm volatile ( "movl %k0, %%" #seg            \

Shouldn't this be a movw? Segment selectors are 16b, not 32b, but I
might be missing something here.

I see loadsegment is also using movl, so yes, I guess I'm missing
something.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel