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

[Tim Deegan <Tim.Deegan@xxxxxxxxxx>]

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?

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

> +        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?

> +            d->hvm_pv_enabled |= XEN_HVM_PV_EVTCHN_ENABLED;
> +            printk("HVM: PV featured evtchn enabled\n");

Please remove your debugging printks.

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

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, XenServer Engineering
Citrix Systems UK Ltd.  (Company #02937203, SL9 0BG)

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