Re: [PATCH 10/18 V2]: PVH xen: introduce vmx_pvh.c and pvh.c

[Mukesh Rathor <mukesh.rathor@oracle.com>]

On Tue, 2 Apr 2013 10:10:12 -0400
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:

> On Mon, Apr 01, 2013 at 06:26:45PM -0700, Mukesh Rathor wrote:
> > On Mon, 18 Mar 2013 12:32:06 -0400
> > Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > 
> > No, hide the X96EMUL_* from the caller of this function which deals
> > with rc or non-zero.
> 
> In that case please make that part of the comment at the start. It
> says: 0 success. But nothing about the failure path.

it's better to have it say some thing than nothing! failure paths are
many and change over time, but when debugging its good to know right
away if a function succeeded.

> > >  - HYPERVISOR_set_segment_base
> > >  - HYPERVISOR_set_gdt
> > >  - HYPERVISOR_tmem
> > >  .. and host of other.
> > > 
> > > This should be documented somewhere in docs?
> > > Perhaps in docs/misc/pvh.txt and just outline which ones are not
> > > to be used anymore?
> > 
> > I am keeping track of all doc stuff, lets document in the end when
> > I enable PVH. We'll be changing stuff for a short while.
> 
> Why not make this 'doc stuff' part of the patches? That way when you
> are done with one item you can have a patch to remove it out the TODO
> list. Also this way other folks can look at it and if they have time
> help you on some of the TODOs.

Lets keep things manageable. I have fixme's in the code, and also documentd
in the cover what needs to be done. When we enable PVH, we'll know what
got done and what didn't, and we can document it properly.

> > 
> > > Could you use 'cpuid' macro defined in processor.h?
> > 
> > since we know in this fucntion we are on intel, we can just do
> > cpuid. the macro has somw quirks for cyrix cpus, and clears rcx.
> > Besides this is user mode cpuid that will exactly be like this on a
> > pure PV guest.
> 
> I am not following you. Is the reason you are doing this b/c the
> macro clears rcx?

Correct.