Re: [PATCH v6 11/29] xen/x86: add bitmap of enabled emulated devices

["Roger Pau Monné" <roger.pau@citrix.com>]

El 23/09/15 a les 15.24, Jan Beulich ha escrit:
>>>> On 23.09.15 at 14:35, <roger.pau@citrix.com> wrote:
>> El 16/09/15 a les 11.50, Jan Beulich ha escrit:
>>>>>> On 04.09.15 at 14:08, <roger.pau@citrix.com> wrote:
>>>> +                   d->domain_id, config->emulation_flags);
>>>> +            return -EINVAL;
>>>> +        }
>>>> +        if ( config->emulation_flags != emulation_mask )
>>>> +        {
>>>> +            printk(XENLOG_G_ERR "d%d: Xen does not allow HVM creation with the "
>>>> +                   "current selection of emulators: %#x.\n", d->domain_id,
>>>> +                   config->emulation_flags);
>>>> +            return -EOPNOTSUPP;
>>>> +        }
>>>> +        d->arch.emulation_flags = config->emulation_flags;
>>>> +    }
>>>
>>> Isn't there an "else" missing here, validating that the flags are zero?
>>
>> The comment in xen/include/asm-x86/domain.h above the emulation_flags
>> field already mentions that this field is ignored for PV guests. For
>> example the x86 Dom0 building code calls arch_domain_create passing in a
>> NULL xen_arch_domainconfig, so I think it's easier to just ignore this
>> for PV guests.
> 
> Easier now, but perhaps more cumbersome if we ever want to
> assign that field some meaning for PV. It's a domctl, so not _that_
> difficult to change, but you may have noticed that I generally ask
> for unused fields/bits to be validated to be zero, to allow using
> them later on. Anyway - not a big issue, I just wanted to point it
> out (and I might stumble across the missing else again during
> review of future revisions).

OK, you convinced me. I've added a check to the start of
arch_domain_create in order to make sure config is not NULL, and fixed
the x86 callers of domain_create in order to make sure a non-null config
is always provided.

Also dropped Andrew Cooper's reviewed-by tag, since the hypervisor side
code has changed substantially.

Roger.