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: <20140105194155.GC12263@phenom.dumpdata.com>
Date:	Sun, 5 Jan 2014 14:41:55 -0500
From:	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To:	Stefano Stabellini <stefano.stabellini@...citrix.com>
Cc:	xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
	boris.ostrovsky@...cle.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 Sun, Jan 05, 2014 at 06:11:39PM +0000, Stefano Stabellini wrote:
> 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:

There is something to be said about sharing the same code path
that "old-style" PV is using with the new-style - code coverage.

That is the code gets tested under both platforms and if I (or
anybody else) introduce a bug in the "common-PV-paths" it will
be immediately obvious as hopefully the regression tests
will pick it up.

It is not nice - as low-level code is sprinkled with the one-offs
for the PVH - which mostly is doing _less_.

What I was thinking is to flip this around. Make the PVH paths
the default and then have something like 'if (!xen_pvh_domain())'
... the big code.

Would you be OK with this line of thinking going forward say
after this patchset?

Thanks!
--
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/