Re: [PATCH 09/17] PVH xen: create PVH vmcs, and also initialization

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

On Wed, 24 Apr 2013 08:42:49 +0100
"Jan Beulich" <JBeulich@suse.com> wrote:

> >>> On 23.04.13 at 23:25, Mukesh Rathor <mukesh.rathor@oracle.com>
> >>> wrote:
> > Changes in V4:
> >   - Remove VM_ENTRY_LOAD_DEBUG_CTLS clearing.
> >   - Add 32bit kernel changes mark.
> >   - Verify pit_init call for PVH.
> 
> Verify in what way?
> 
..
> > +
> > +    if ( (rc = hvm_vcpu_cacheattr_init(v)) != 0 )
> > +    {
> > +        hvm_funcs.vcpu_destroy(v);
> > +        return rc;
> > +    }
> > +    if ( v->vcpu_id == 0 )
> > +        pit_init(v, cpu_khz);
> 
> I'm asking in particular because my understanding of "verify" would
> be checking of an eventual return value...

Function returns void. Verified that the speaker and pit IO would
be properly handled for PVH.

> > @@ -4512,6 +4582,8 @@ static int hvm_memory_event_traps(long p,
> > uint32_t reason, 
> >  void hvm_memory_event_cr0(unsigned long value, unsigned long old) 
> >  {
> > +    if ( is_pvh_vcpu(current) )
> > +        return;
> >      hvm_memory_event_traps(current->domain->arch.hvm_domain
> >                               .params[HVM_PARAM_MEMORY_EVENT_CR0],
> >                             MEM_EVENT_REASON_CR0,
> 
> So these checks are still there, with no mark of being temporary,
> despite having pointed out that they ought to be unnecessary once
> full PVH support is there. As with the 32-bit specific changes that
> the code currently lacks, such temporary adjustments should be
> marked clearly and completely, so subsequently one can locate
> them _all_. Just consider what happens if after phase I you get
> taken off this project, and someone else would have to complete
> it.

I put action item in the cover letter:

"- Add support for monitoring guest behavior. See hvm_memory_event* functions
     in hvm.c"

I can add "PVH: fixme" comment tags too.

thanks
Mukesh