Re: [Xen-devel] [PATCH 09/18] PVH xen: Support privileged op emulation for PVH

[Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>]

On Fri, 28 Jun 2013 10:20:47 +0100
"Jan Beulich" <JBeulich@xxxxxxxx> wrote:

> >>> On 28.06.13 at 01:43, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> >>> wrote:
> > On Thu, 27 Jun 2013 08:22:42 +0100
> > "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
> > 
> >> >>> On 27.06.13 at 00:41, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx>
> >> >>> wrote:
> >> > On Tue, 25 Jun 2013 10:36:41 +0100
> >> > "Jan Beulich" <JBeulich@xxxxxxxx> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel