From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?UTF-8?Q?Roger_Pau_Monn=c3=a9?= <roger.pau@citrix.com>
Subject: Re: [PATCH v6 11/29] xen/x86: add bitmap of enabled
	emulated devices
Date: Wed, 23 Sep 2015 17:02:46 +0200
Message-ID: <5602BF16.1010508@citrix.com>
References: <1441368548-43465-1-git-send-email-roger.pau@citrix.com>
	<1441368548-43465-12-git-send-email-roger.pau@citrix.com>
	<55F9577502000078000A3372@prv-mh.provo.novell.com>
	<56029C79.8040505@citrix.com>
	<5602C41D02000078000A4E1C@prv-mh.provo.novell.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
Received: from mail6.bemta3.messagelabs.com ([195.245.230.39])
	by lists.xen.org with esmtp (Exim 4.72)
	(envelope-from <prvs=701ebc18d=roger.pau@citrix.com>)
	id 1ZelZw-0007KR-Hz
	for xen-devel@lists.xenproject.org; Wed, 23 Sep 2015 15:03:28 +0000
In-Reply-To: <5602C41D02000078000A4E1C@prv-mh.provo.novell.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Jan Beulich <JBeulich@suse.com>
Cc: Wei Liu <wei.liu2@citrix.com>, Ian Campbell <ian.campbell@citrix.com>, Stefano Stabellini <stefano.stabellini@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, xen-devel@lists.xenproject.org
List-Id: xen-devel@lists.xenproject.org

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.