Re: [Xen-devel] [PATCH 2/5] x86/pv: map_ldt_shadow_page() cleanup

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 29.08.17 at 13:19, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -667,45 +667,49 @@ static int alloc_segdesc_page(struct page_info *page)
>  }
>  
>  
> -/* Map shadow page at offset @off. */
> -int map_ldt_shadow_page(unsigned int off)
> +/*
> + * Map a guests LDT page (at @offset bytes from the start of the LDT) into

The comment is not really correct: The low 12 bits of offset don't
matter for where the page gets mapped. "(covering the byte at
@offset ..." perhaps?

> + * Xen's virtual range.  Returns true if the mapping changed, false 
> otherwise.
> + */
> +bool map_ldt_shadow_page(unsigned int offset)
>  {
>      struct vcpu *v = current;
>      struct domain *d = v->domain;
> -    unsigned long gmfn;
>      struct page_info *page;
> -    l1_pgentry_t l1e, nl1e;
> -    unsigned long gva = v->arch.pv_vcpu.ldt_base + (off << PAGE_SHIFT);
> -    int okay;
> +    l1_pgentry_t gl1e, *pl1e;
> +    unsigned long linear = v->arch.pv_vcpu.ldt_base + offset;
>  
>      BUG_ON(unlikely(in_irq()));
>  
> +    /* Hardware limit checking should guarentee this property. */

guarantee?

> +    ASSERT((offset >> 3) <= v->arch.pv_vcpu.ldt_ents);

Can this validly be an ASSERT()? I.e. is there really no way for
ldt_ents for a vCPU to change between the hardware limit check
and execution making it here? MMUEXT_SET_LDT acts on current,
but vcpu_reset() clearing v->is_initialised and then
arch_set_info_guest() becoming usable on this vCPU is not that
trivial to exclude (i.e. at least the comment would probably want
extending).

Jan


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