linux-kernel - Re: [PATCH RFC] paravirt: cleanup lazy mode handling

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]

Date:	Tue, 02 Oct 2007 17:53:04 +1000
From:	Rusty Russell <rusty@...tcorp.com.au>
To:	Jeremy Fitzhardinge <jeremy@...p.org>
Cc:	Virtualization Mailing List <virtualization@...ts.osdl.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andi Kleen <ak@...e.de>, Zachary Amsden <zach@...are.com>,
	Avi Kivity <avi@...ranet.com>,
	Anthony Liguori <anthony@...emonkey.ws>,
	Glauber de Oliveira Costa <glommer@...il.com>,
	"Nakajima, Jun" <jun.nakajima@...el.com>
Subject: Re: [PATCH RFC] paravirt: cleanup lazy mode handling

On Mon, 2007-10-01 at 23:29 -0700, Jeremy Fitzhardinge wrote: 
> Rusty Russell wrote:
> > That's good, but this code does lose on native because we no longer
> > simply replace the entire thing with noops.
> >
> > Perhaps inverting this and having (inline) helpers is the way to go?
> > 
> > static inline void paravirt_enter_lazy(enum paravirt_lazy_mode mode)
> > {
> > 	BUG_ON(x86_read_percpu(paravirt_lazy_mode) != PARAVIRT_LAZY_NONE);
> > 	BUG_ON(preemptible());
> >
> > 	x86_write_percpu(paravirt_lazy_mode, mode);
> > }
> >
> > static inline void paravirt_exit_lazy(enum paravirt_lazy_mode mode)
> > {
> > 	BUG_ON(x86_read_percpu(paravirt_lazy_mode) != mode);
> > 	BUG_ON(preemptible());
> >
> > 	x86_write_percpu(paravirt_lazy_mode, PARAVIRT_LAZY_NONE);
> > }
> >   
> 
> Er, they should probably call something to make the switch actually
> happen, no?

No, they're helpers.  eg:

static void lguest_exit_lazy(enum paravirt_lazy_mode mode)
{
paravirt_exit_lazy(mode);
lguest_flush_hcalls();
}

> > The only trick would be that the flushes are so rarely required it's
> > probably worth putting the unlikely() in the top level:
> 
> Sure, I guess.  Would it make any difference?  (I've never personally
> noticed likely/unlikely change the generated code in any seriously
> positive way.)

Probably overkill (I was trying to avoid the branch for the case where
we don't need to flush, as that's always what happens).

So just expose a flush hook:

static inline void arch_flush_lazy_cpu_mode(void)
{
	PVOP_VCALL1(flush_lazy_mode, PARAVIRT_LAZY_CPU);
}

....

static void lguest_flush_lazy_mode(enum paravirt_lazy_mode mode)
{
	if (unlikely(x86_read_percpu(paravirt_lazy_mode) == mode)) {
		lguest_lazy_cpu_leave();
		lguest_lazy_cpu_enter();
	}
}

Cheers,
Rusty.

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