Xen devel - Mar 2010 - [PATCH][v6] PV extension of HVM(hybrid) support in Xen

Hi Keir
 
The latest (hybrid) patchset again...
 
Change from v5:
Address the comments from Tim.

Change from v4:
1. Add support for PV clocksource on HVM. (Replace evtchn enabling with pv 
clock enabling).
2. Update the patch following Tim''s comments.
 
Change from v3:
 
1. Minor polish the patchset. Replace several specific 
is_hvm_pv_evtchn_domain() judgement.
2. Name changed...
 
Change from v2:
 
1. Change the name "hybrid" to "PV featured HVM", as well as
flag and macro
names.
 
-- 
regards
Yang, Sheng


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Fri, 2010-03-05 at 06:12 +0000, Sheng Yang wrote:
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -2240,6 +2240,13 @@
>      {
>      case VCPUOP_register_runstate_memory_area:
>      case VCPUOP_get_runstate_info:
> +    /* For evtchn on HVM */
> +    case VCPUOP_initialise:
> +    case VCPUOP_up:
> +    case VCPUOP_set_periodic_timer:
> +    case VCPUOP_stop_periodic_timer:
> +    case VCPUOP_set_singleshot_timer:
> +    case VCPUOP_stop_singleshot_timer:
Can we drop that comment? It doesn''t really say anything and
I''m not
100% sure it is accurate...
> +    case HVMOP_enable_pv: {
> [...]
> +       /* This would be called by BSP, other vcpus are adjusted
> during the
> +        * start-up */
> +       if (a.flags & HVM_PV_CLOCK) {
> +               d->hvm_pv_enabled |= XEN_HVM_PV_CLOCK_ENABLED;
> +               update_domain_wallclock_time(d);
> +               hvm_funcs.set_tsc_offset(current, 0);
> +       }
I think you''ve been asked to get rid of this construct in every posting
so far. That code belongs in the hypercall which the guest uses to
actually attach to the pv clock. I think Stefano''s patchset shows that
this hypercall isn''t necessary.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Friday 05 March 2010 18:39:34 Ian Campbell wrote:
> On Fri, 2010-03-05 at 06:12 +0000, Sheng Yang wrote:
> > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -2240,6 +2240,13 @@
> >      {
> >      case VCPUOP_register_runstate_memory_area:
> >      case VCPUOP_get_runstate_info:
> > +    /* For evtchn on HVM */
> > +    case VCPUOP_initialise:
> > +    case VCPUOP_up:
> > +    case VCPUOP_set_periodic_timer:
> > +    case VCPUOP_stop_periodic_timer:
> > +    case VCPUOP_set_singleshot_timer:
> > +    case VCPUOP_stop_singleshot_timer:
> 
> Can we drop that comment? It doesn''t really say anything and
I''m not
> 100% sure it is accurate...
OK
> 
> > +    case HVMOP_enable_pv: {
> > [...]
> > +       /* This would be called by BSP, other vcpus are adjusted
> > during the
> > +        * start-up */
> > +       if (a.flags & HVM_PV_CLOCK) {
> > +               d->hvm_pv_enabled |= XEN_HVM_PV_CLOCK_ENABLED;
> > +               update_domain_wallclock_time(d);
> > +               hvm_funcs.set_tsc_offset(current, 0);
> > +       }
> 
> I think you''ve been asked to get rid of this construct in every
posting
> so far. That code belongs in the hypercall which the guest uses to
> actually attach to the pv clock. I think Stefano''s patchset shows
that
> this hypercall isn''t necessary.
If you check the code of my patches, you would find pv clock source can work 
without pv clock event now. And if you check Stefano''s patch, you would
find
it solved the issue when they are binding(which I neglected). But it
can''t
solve the issue now.

"Actually attach" is a good point, but I am not sure there is such
kind of
things now.

-- 
regards
Yang, Sheng


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel