From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andi Kleen Subject: Re: [PATCH 1/4] x86 paravirt_ops: create no_paravirt.h for native ops Date: Tue, 8 Aug 2006 03:59:12 +0200 Message-ID: <200608080359.12958.ak@muc.de> References: <1154925835.21647.29.camel@localhost.localdomain> <1154930669.7642.12.camel@localhost.localdomain> <44D7A7E6.2060401@vmware.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <44D7A7E6.2060401@vmware.com> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: virtualization-bounces@lists.osdl.org Errors-To: virtualization-bounces@lists.osdl.org To: virtualization@lists.osdl.org Cc: Andrew Morton , Linux Kernel Mailing List , Chris Wright List-Id: virtualization@lists.linuxfoundation.org On Monday 07 August 2006 22:51, Zachary Amsden wrote: > Rusty Russell wrote: > >>> + > >>> +/* > >>> + * Set IOPL bits in EFLAGS from given mask > >>> + */ > >>> +static inline void set_iopl_mask(unsigned mask) > >>> = > >> This function can be completely written in C using local_save_flags()/= local_restore_flags() > >> Please do that. I guess it's still a good idea to keep it separated > >> though because it might allow other optimizations. > >> > >> e.g. i've been thinking about special casing IF changes in save/restor= e flags = > >> to optimize CPUs which have slow pushf/popf. If you already make sure > >> all non IF manipulations of flags are separated that would help. > >> = > = > = > Actually, that is not quite true. Local_save_flags / = > raw_local_irq_restore today is used only for operating on IF flag, and = > raw_local_restore_flags does not exist. = Yes, sorry for the typo. > Our implementation of these in = > VMI assumes that only the IF flag is being changed, and this is the = > default assumption under which Xen runs as well. Using local_restore to = > switch IOPL as well causes the extremely performance critical common = > case of pure IRQ restore to do potentially a lot more work in a hyperviso= r. > = > So if you do want us to go with the C approach, I would propose using = > raw_local_iopl_restore, which can make a different hypercall (actually, = > in our case, this is not even a hypercall, merely a VMI call). I meant Rusty can use local restore in his native implementation. The higher level interface can be different. -Andi