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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.02.1401051757260.8667@kaball.uk.xensource.com>
Date:	Sun, 5 Jan 2014 18:11:39 +0000
From:	Stefano Stabellini <stefano.stabellini@...citrix.com>
To:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
CC:	<xen-devel@...ts.xenproject.org>, <linux-kernel@...r.kernel.org>,
	<boris.ostrovsky@...cle.com>, <stefano.stabellini@...citrix.com>,
	<david.vrabel@...rix.com>, <hpa@...or.com>,
	Mukesh Rathor <mukesh.rathor@...cle.com>
Subject: Re: [PATCH v13 08/19] xen/pvh/mmu: Use PV TLB instead of native.

On Fri, 3 Jan 2014, Konrad Rzeszutek Wilk wrote:
> From: Mukesh Rathor <mukesh.rathor@...cle.com>
> 
> 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@...cle.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
> ---
>  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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/