From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@suse.com>
Subject: Re: [PATCH v5 11/15] x86/altp2m: define and implement
 alternate p2m HVMOP types.
Date: Thu, 16 Jul 2015 10:09:35 +0100
Message-ID: <55A790EF0200007800091BFC@mail.emea.novell.com>
References: <1436832903-12639-1-git-send-email-edmund.h.white@intel.com>
	<1436832903-12639-12-git-send-email-edmund.h.white@intel.com>
	<55A53AA00200007800090CE9@mail.emea.novell.com>
	<DBC12B0F5509554280826E40BCDEE8BE54FD773C@ORSMSX104.amr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <DBC12B0F5509554280826E40BCDEE8BE54FD773C@ORSMSX104.amr.corp.intel.com>
Content-Disposition: inline
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: Ravi Sahita <ravi.sahita@intel.com>
Cc: Tim Deegan <tim@xen.org>, Wei Liu <wei.liu2@citrix.com>, George Dunlap <george.dunlap@eu.citrix.com>, Andrew Cooper <andrew.cooper3@citrix.com>, Ian Jackson <ian.jackson@eu.citrix.com>, Edmund H White <edmund.h.white@intel.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>, "tlengyel@novetta.com" <tlengyel@novetta.com>, Daniel De Graaf <dgdegra@tycho.nsa.gov>
List-Id: xen-devel@lists.xenproject.org

>>> On 16.07.15 at 11:02, <ravi.sahita@intel.com> wrote:
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>>Sent: Tuesday, July 14, 2015 7:37 AM
>>
>>>>> On 14.07.15 at 02:14, <edmund.h.white@intel.com> wrote:
>>> Signed-off-by: Ed White <edmund.h.white@intel.com>
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -6443,6 +6443,148 @@ long do_hvm_op(unsigned long op,
>>XEN_GUEST_HANDLE_PARAM(void) arg)
>>>          break;
>>>      }
>>>
>>> +    case HVMOP_altp2m:
>>> +    {
>>> +        struct xen_hvm_altp2m_op a;
>>> +        struct domain *d = NULL;
>>> +
>>> +        if ( copy_from_guest(&a, arg, 1) )
>>> +            return -EFAULT;
>>> +
>>> +        if ( a.pad[0] || a.pad[1] )
>>> +            return -EINVAL;
>>
>>Why can't the field be uint16_t, making this a single check?
>>
> 
> Could be of course;  we had asked for an example and we found domctl, which 
> pads with uint8_t[] and followed the same approach.

domctl is a particularly bad example, as there checking of padding
space to be zero is pointless as there is no ABI guarantee anyway
(same for sysctl). I.e. I'd be surprised if you found such a check in
any domctl handling...

> Change required?

Yes please - let's avoid ugly code like the above.

Jan