Re: [Xen-devel] [PATCH xen v2] xen: arm: fully implement multicall interface.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 07.04.14 at 14:48, <ian.campbell@xxxxxxxxxx> wrote:
> I'm not sure what I was smoking at the time of 5d74ad1a082e "xen: arm:
> implement do_multicall_call for both 32 and 64-bit" but it is obviously
> insufficient since it doesn't actually wire up the hypercall.
> 
> Before doing so we need to make the usual adjustments for ARM and turn the
> unsigned longs into xen_ulong_t. There is no difference in the resulting
> structure for x86.
> 
> There are knock on changes to the trace interface, but again they are nops 
> on
> x86.
> 
> For 32-bit ARM guests we require that the arguments which they pass to a
> hypercall via a multicall do not use the upper bits of xen_ulong_t and kill
> them if they violate this. This should ensure that no ABI surprises can be
> silently lurking when running on a 32-bit hypervisor waiting to pounce when 
> the
> same kernel is run on a 64-bit hypervisor. Killing the guest is harsh but it
> will be far easier to relax the restriction if it turns out to cause 
> problems
> than to tighten it up if we were lax to begin with.
> 
> In the interests of clarity and always using explicitly sized types change 
> the
> unsigned int in the hypercall arguments to a uint32_t. There is no actual
> change here on any platform.
> 
> We should consider backporting this to 4.4.1 in case a guest decides they 
> want
> to use a multicall in common code e.g. I suggested such a thing while
> reviewing a netback change recently.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

on the non-ARM parts, with one comment on the ARM ones below.

> @@ -1159,6 +1160,21 @@ static void do_trap_hypercall(struct cpu_user_regs 
> *regs, register_t *nr,
>  #endif
>  }
>  
> +static void check_multicall_32bit_clean(struct multicall_entry *multi)
> +{
> +    int i;
> +
> +    for ( i = 0; i < arm_hypercall_table[multi->op].nr_args; i++ )
> +    {
> +        if ( unlikely(multi->args[i] & 0xffffffff00000000ULL) )
> +        {
> +            printk("%pv: multicall argument %d is not 32-bit clean 
> %"PRIx64"\n",
> +                   current, i, multi->args[i]);
> +            domain_crash_synchronous();

On x86 we actually decided quite a long while ago to try to avoid
domain_crash_synchronous() whenever possible. Would
domain_crash(current->domain) not work for you here?

Also I think you want the loop variable to be uint32_t, in line with
your changes elsewhere in this patch.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel