Re: [Xen-devel] [PATCH v13 08/19] xen/pvh/mmu: Use PV TLB instead of native.

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 3 Jan 2014, Konrad Rzeszutek Wilk wrote:
> From: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> 
> We also optimize one - the TLB flush. The native operation would
> needlessly IPI offline VCPUs causing extra wakeups. Using the
> Xen one avoids that and lets the hypervisor determine which
> VCPU needs the TLB flush.
> 
> Signed-off-by: Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>  arch/x86/xen/mmu.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
> index 490ddb3..c1d406f 100644
> --- a/arch/x86/xen/mmu.c
> +++ b/arch/x86/xen/mmu.c
> @@ -2222,6 +2222,15 @@ static const struct pv_mmu_ops xen_mmu_ops __initconst 
> = {
>  void __init xen_init_mmu_ops(void)
>  {
>       x86_init.paging.pagetable_init = xen_pagetable_init;
> +
> +     /* Optimization - we can use the HVM one but it has no idea which
> +      * VCPUs are descheduled - which means that it will needlessly IPI
> +      * them. Xen knows so let it do the job.
> +      */
> +     if (xen_feature(XENFEAT_auto_translated_physmap)) {
> +             pv_mmu_ops.flush_tlb_others = xen_flush_tlb_others;
> +             return;
> +     }
>       pv_mmu_ops = xen_mmu_ops;
>  
>       memset(dummy_mapping, 0xff, PAGE_SIZE);

Regarding this patch, the next one and the other changes to
xen_setup_shared_info, xen_setup_mfn_list_list,
xen_setup_vcpu_info_placement, etc: considering that the mmu related
stuff is very different between PV and PVH guests, I wonder if it makes
any sense to keep calling xen_init_mmu_ops on PVH.

I would introduce a new function, xen_init_pvh_mmu_ops, that sets
pv_mmu_ops.flush_tlb_others and only calls whatever is needed for PVH
under a new xen_pvh_pagetable_init.
Just to give you an idea, not even compiled tested:



diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 23ead29..4e53fa3 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1117,15 +1117,12 @@ static int xen_write_msr_safe(unsigned int msr, 
unsigned low, unsigned high)
 
 void xen_setup_shared_info(void)
 {
-       if (!xen_feature(XENFEAT_auto_translated_physmap)) {
-               set_fixmap(FIX_PARAVIRT_BOOTMAP,
-                          xen_start_info->shared_info);
+       BUG_ON(xen_feature(XENFEAT_auto_translated_physmap));
+       set_fixmap(FIX_PARAVIRT_BOOTMAP,
+                       xen_start_info->shared_info);
 
-               HYPERVISOR_shared_info =
-                       (struct shared_info *)fix_to_virt(FIX_PARAVIRT_BOOTMAP);
-       } else
-               HYPERVISOR_shared_info =
-                       (struct shared_info *)__va(xen_start_info->shared_info);
+       HYPERVISOR_shared_info =
+               (struct shared_info *)fix_to_virt(FIX_PARAVIRT_BOOTMAP);
 
 #ifndef CONFIG_SMP
        /* In UP this is as good a place as any to set up shared info */
@@ -1467,7 +1464,10 @@ asmlinkage void __init xen_start_kernel(void)
         * Set up some pagetable state before starting to set any ptes.
         */
 
-       xen_init_mmu_ops();
+       if (xen_pvh_domain())
+               xen_init_pvh_mmu_ops();
+       else
+               xen_init_mmu_ops();
 
        /* Prevent unwanted bits from being set in PTEs. */
        __supported_pte_mask &= ~_PAGE_GLOBAL;
diff --git a/arch/x86/xen/mmu.c b/arch/x86/xen/mmu.c
index 490ddb3..04405bc 100644
--- a/arch/x86/xen/mmu.c
+++ b/arch/x86/xen/mmu.c
@@ -1254,6 +1254,15 @@ static void __init xen_pagetable_p2m_copy(void)
 }
 #endif
 
+static void __init xen_pvh_pagetable_init(void)
+{
+       paging_init();
+       HYPERVISOR_shared_info =
+               (struct shared_info *)__va(xen_start_info->shared_info);
+       for_each_possible_cpu(cpu)
+               xen_vcpu_setup(cpu);
+}
+
 static void __init xen_pagetable_init(void)
 {
        paging_init();
@@ -2219,6 +2228,20 @@ static const struct pv_mmu_ops xen_mmu_ops __initconst = 
{
        .set_fixmap = xen_set_fixmap,
 };
 
+void __init xen_init_pvh_mmu_ops(void)
+{
+       x86_init.paging.pagetable_init = xen_pvh_pagetable_init;
+
+       /* Optimization - we can use the HVM one but it has no idea which
+        * VCPUs are descheduled - which means that it will needlessly IPI
+        * them. Xen knows so let it do the job.
+        */
+       if (xen_feature(XENFEAT_auto_translated_physmap)) {
+               pv_mmu_ops.flush_tlb_others = xen_flush_tlb_others;
+               return;
+       }
+}
+
 void __init xen_init_mmu_ops(void)
 {
        x86_init.paging.pagetable_init = xen_pagetable_init;

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