WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] Re: [PATCH][v4] PV extension of HVM(hybrid) support in Xen

To: Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Subject: [Xen-devel] Re: [PATCH][v4] PV extension of HVM(hybrid) support in Xen
From: Sheng Yang <sheng@xxxxxxxxxxxxxxx>
Date: Mon, 1 Mar 2010 19:40:50 +0800
Cc: Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>, Ian Pratt <Ian.Pratt@xxxxxxxxxxxxx>, xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>, Keir Fraser <Keir.Fraser@xxxxxxxxxxxxx>
Delivery-date: Mon, 01 Mar 2010 03:40:34 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <20100301102127.GA17243@xxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: Intel Opensource Technology Center
References: <201003011743.41341.sheng@xxxxxxxxxxxxxxx> <20100301102127.GA17243@xxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: KMail/1.12.2 (Linux/2.6.31-19-generic; KDE/4.3.2; x86_64; ; )
On Monday 01 March 2010 18:21:27 Tim Deegan wrote:
> At 09:43 +0000 on 01 Mar (1267436621), Sheng Yang wrote:
> > @@ -3109,6 +3117,36 @@
> >          break;
> >      }
> >
> > +    case HVMOP_enable_pv: {
> 
> Why does this have to be explicitly enabled?  Can't you just notice that
> a domain is using the evtchnop hypercalls?

The issue is pv timer. It assumed the tsc start from 0, which is different 
from HVM. So I'd like to give it a explicit call here. Otherwise it can be 
hooked in evtchn binding, but I don't think that's clear...
 
> > +        struct xen_hvm_pv_type a;
> > +        struct domain *d;
> > +
> > +        if ( copy_from_guest(&a, arg, 1) )
> > +            return -EFAULT;
> > +
> > +        rc = rcu_lock_target_domain_by_id(a.domid, &d);
> 
> Domains do this to each other?  It looks like it has surprising
>  side-effects.

Should not allowed... I think a.domid should always be the current domain. 
Replace it with DOMID_SELF?
 
> > +        if ( rc != 0 )
> > +            return rc;
> > +
> > +        rc = -EINVAL;
> > +        if ( !is_hvm_domain(d) )
> > +            goto param_fail5;
> > +
> > +        rc = xsm_hvm_param(d, op);
> > +        if ( rc )
> > +            goto param_fail5;
> > +
> > +        if (a.flags & HVM_PV_EVTCHN) {
> > +            update_domain_wallclock_time(d);
> > +            hvm_funcs.set_tsc_offset(d->vcpu[0], 0);
> 
> Only vcpu 0?  Doesn't this do horrible things to timekeeping in the guest?

The other vcpus are initialized when it is brought up. TSC started from 0 is a 
fundamental assumption for pv clock in Linux... 
> 
> > +            d->hvm_pv_enabled |= XEN_HVM_PV_EVTCHN_ENABLED;
> > +            printk("HVM: PV featured evtchn enabled\n");
> 
> Please remove your debugging printks.

OK...

-- 
regards
Yang, Sheng

> 
> > +        }
> > +param_fail5:
> > +        rcu_unlock_domain(d);
> > +        break;
> > +    }
> > +
> >      default:
> >      {
> >          gdprintk(XENLOG_WARNING, "Bad HVM op %ld.\n", op);
> 
> Tim.
> 

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