Re: [PATCH (V9) 2/2] xen: Add V4V implementation

[Ross Philipson <ross.philipson@citrix.com>]

On 05/28/2013 08:43 PM, Matt Wilson wrote:
> On Tue, May 28, 2013 at 03:43:31PM -0400, Ross Philipson wrote:
>> Setup of v4v domains a domain gets created and cleanup
>> when a domain die. Wire up the v4v hypercall.
>>
>> Include v4v internal and public headers.
>>
>> Signed-off-by: Ross Philipson<ross.philipson@citrix.com>
>
> Would you consider making a boot-time option to disable v4v entirely?

I think that sounds reasonable since if you are not using it, it is 
consuming resources needlessly. I actually was thinking about this 
yesterday shortly after submitting the last patch set.

Thanks
Ross

>
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -199,7 +199,8 @@ struct domain *domain_create(
>>   {
>>       struct domain *d, **pd;
>>       enum { INIT_xsm = 1u<<0, INIT_watchdog = 1u<<1, INIT_rangeset = 1u<<2,
>> -           INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5 };
>> +           INIT_evtchn = 1u<<3, INIT_gnttab = 1u<<4, INIT_arch = 1u<<5,
>> +           INIT_v4v = 1u<<6 };
>>       int err, init_status = 0;
>>       int poolid = CPUPOOLID_NONE;
>>
>> @@ -312,6 +313,13 @@ struct domain *domain_create(
>>           spin_unlock(&domlist_update_lock);
>>       }
>
> Perhaps an opt_v4v check here?
>
>> +    if ( !is_idle_domain(d) )
>> +    {
>> +        if ( (err = v4v_init(d)) != 0 )
>> +            goto fail;
>> +        init_status |= INIT_v4v;
>> +    }
>> +
>>       return d;
>>
>>    fail:
>
> [...]
>
>> new file mode 100644
>> index 0000000..701877d
>> --- /dev/null
>> +++ b/xen/common/v4v.c
>
> [...]
>
>> +/*
>> + * hypercall glue
>> + */
>> +long
>> +do_v4v_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg1,
>> +          XEN_GUEST_HANDLE_PARAM(void) arg2,
>> +          uint32_t arg3, uint32_t arg4)
>> +{
>> +    struct domain *d = current->domain;
>> +    long rc = -EFAULT;
>> +
>> +    v4v_dprintk("->do_v4v_op(%d,%p,%p,%d,%d)\n", cmd,
>> +                (void *)arg1.p, (void *)arg2.p, (int) arg3, (int) arg4);
>> +
>
> and check for d->v4v != NULL here, bailing with -ENOSYS?
>
>> +    domain_lock(d);
>> +    switch (cmd)
>> +    {
>> +    case V4VOP_register_ring:
>
> [...]
>
> --msw