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

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

On 29/08/17 15:14, Jan Beulich wrote:
>>>> On 29.08.17 at 13:19, <andrew.cooper3@citrix.com> 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?

Ok.

>
>> + * 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?

Yes.

>
>> +    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).

vcpu_reset() calls vcpu_pause() first, which will block until the #PF
handler completes.

arch_set_info_guest() can't be called on already-initialised vcpus.

I will extend the comment to explain why the ASSERT() is safe.

~Andrew

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