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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.07.13 at 03:38, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> 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.

Even if it really is (which I doubt), you still would make PVH
different from both PV and HVM, which both don't populate the
selector fields of the frame (PV obviously has ->cs and ->ss
populated [by the CPU], but HVM avoids even that).

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

We'll have to see - at the first glance I don't follow...

Jan


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