Re: [PATCH v5 11/15] x86/altp2m: define and implement alternate p2m HVMOP types.

["Jan Beulich" <JBeulich@suse.com>]

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