From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mukesh Rathor Subject: Re: [PATCH 09/18] PVH xen: Support privileged op emulation for PVH Date: Tue, 2 Jul 2013 18:38:15 -0700 Message-ID: <20130702183815.621e95f8@mantra.us.oracle.com> References: <1372118507-16864-1-git-send-email-mukesh.rathor@oracle.com> <1372118507-16864-10-git-send-email-mukesh.rathor@oracle.com> <51C980C902000078000E0445@nat28.tlf.novell.com> <20130626154130.6c50b347@mantra.us.oracle.com> <51CC046202000078000E106E@nat28.tlf.novell.com> <20130627164339.476d2ef3@mantra.us.oracle.com> <51CD718F02000078000E17B3@nat28.tlf.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <51CD718F02000078000E17B3@nat28.tlf.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: xen-devel List-Id: xen-devel@lists.xenproject.org On Fri, 28 Jun 2013 10:20:47 +0100 "Jan Beulich" wrote: > >>> On 28.06.13 at 01:43, Mukesh Rathor > >>> wrote: > > On Thu, 27 Jun 2013 08:22:42 +0100 > > "Jan Beulich" wrote: > > > >> >>> On 27.06.13 at 00:41, Mukesh Rathor > >> >>> wrote: > >> > On Tue, 25 Jun 2013 10:36:41 +0100 > >> > "Jan Beulich" wrote: > >> > > values from struct cpu_user_regs in the PVH case is wrong. It was > you continuing to point to the context switch path, making me > believe that so far you don't properly suppress the uses of > {save,load}_segments() for PVH. > > >> Furthermore - the reading from struct cpu_user_regs continues > >> to be bogus (read: at least a latent bug) as long as you don't > >> always save the selector registers, which you now validly don't > >> do anymore. > > > > Right, because you made me move it to the path that calls the > > macro. So, for the path that the macro is called, the selectors > > would have been read. So, whats the latent bug? > > The problem is that you think that now and forever this macro > will only be used from the MMIO emulation path (or some such, in > any case - just from one very specific path). This is an assumption > you may make while in an early development phase, but not in > patches that you intend to be committed: Someone adding another > use of the macro is _very_ unlikely to go and check what contraints > apply to that use. The macro has to work in the general case. Hmm.. Ok, I still fail to see the difference, caching upfront always is such a low overhead. Anyways, I can make the change, but do realize that the name parameter will need to change to 'enum x86_segment', and so all callers will need to change too. The macro will now need to have a switch statement inside for non-pvh case... I may as well change it from macro to an inlined function. hope all that sounds ok. mukesh