Re: [Xen-devel] [PATCH 3/3] x86/mm: Consolidate all Xen L4 slot writing into init_xen_l4_slots()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.10.17 at 17:24, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 12/10/17 16:08, Jan Beulich wrote:
>>>>> On 12.10.17 at 15:54, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/hap/hap.c
>>> +++ b/xen/arch/x86/mm/hap/hap.c
>>> @@ -391,41 +391,24 @@ int hap_set_allocation(struct domain *d, unsigned int 
> pages, bool *preempted)
>>>      return 0;
>>>  }
>>>  
>>> -static void hap_install_xen_entries_in_l4(struct vcpu *v, mfn_t l4mfn)
>>> -{
>>> -    struct domain *d = v->domain;
>>> -    l4_pgentry_t *l4e;
>>> -
>>> -    l4e = map_domain_page(l4mfn);
>>> -
>>> -    /* Copy the common Xen mappings from the idle domain */
>>> -    memcpy(&l4e[ROOT_PAGETABLE_FIRST_XEN_SLOT],
>>> -           &idle_pg_table[ROOT_PAGETABLE_FIRST_XEN_SLOT],
>>> -           ROOT_PAGETABLE_XEN_SLOTS * sizeof(l4_pgentry_t));
>>> -
>>> -    /* Install the per-domain mappings for this domain */
>>> -    l4e[l4_table_offset(PERDOMAIN_VIRT_START)] =
>>> -        l4e_from_page(d->arch.perdomain_l3_pg, __PAGE_HYPERVISOR_RW);
>>> -
>>> -    /* Install a linear mapping */
>>> -    l4e[l4_table_offset(LINEAR_PT_VIRT_START)] =
>>> -        l4e_from_mfn(l4mfn, __PAGE_HYPERVISOR_RW);
>>> -
>>> -    unmap_domain_page(l4e);
>>> -}
>>> -
>>>  static mfn_t hap_make_monitor_table(struct vcpu *v)
>>>  {
>>>      struct domain *d = v->domain;
>>>      struct page_info *pg;
>>> +    l4_pgentry_t *l4e;
>>>      mfn_t m4mfn;
>>>  
>>>      ASSERT(pagetable_get_pfn(v->arch.monitor_table) == 0);
>>>  
>>>      if ( (pg = hap_alloc(d)) == NULL )
>>>          goto oom;
>>> +
>>>      m4mfn = page_to_mfn(pg);
>>> -    hap_install_xen_entries_in_l4(v, m4mfn);
>>> +    l4e = __map_domain_page(pg);
>> If you obtain the MFN anyway, map_domain_page() is cheaper
>> generated code wise.
> 
> Ah yes.  Given that __map_domain_page() is a define, I'd hope the
> compiler can spot and optimise away the double page_to_mfn().

Considering this is a non-trivial operation, I'm afraid many (if not
all) compiler versions won't be smart enough.

>>> --- a/xen/arch/x86/pv/domain.c
>>> +++ b/xen/arch/x86/pv/domain.c
>>> @@ -35,7 +35,7 @@ static int setup_compat_l4(struct vcpu *v)
>>>  
>>>      l4tab = __map_domain_page(pg);
>>>      clear_page(l4tab);
>>> -    init_guest_l4_table(l4tab, v->domain, 1);
>>> +    init_xen_l4_slots(l4tab, page_to_mfn(pg), v->domain, INVALID_MFN, 
> false);
>> Perhaps worth avoiding the double translation here too.
>>
>> In any event
>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> Just to confirm, the additional delta is:

Yes, looks fine, thanks.

Jan


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