Re: [Xen-devel] [PATCH] RFC x86/hvm: Don't truncate the hvm hypercall index before range checking it

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 24/10/16 10:33, Jan Beulich wrote:
>>>> On 14.10.16 at 17:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>> When the compat hypercall ABI was added for HVM guests (i.e. supporting 32bit
>> operating systems making hypercalls against a 64bit Xen), an ABI breakage was
>> introduced for non-compat guests, as the 64bit hypercall index became
>> truncated to 32 bits.
>>
>> This has been the case for a very long time, but is not very obvious from the
>> code, and definitely counterintuitive, seeing as all other 64bit parameters
>> are passed without truncation.
>>
>> However, the only supported method of making hypercalls is to call into the
>> hypercall page, which in practice means that only hypercall index up to 63 
>> are
>> supported.
>>
>> Therefore, take the opportunity to fix the ABI before it becomes impossible 
>> to
>> fix.
> Considering
>
>     if ( (eax & 0x80000000) && is_viridian_domain(currd) )
>         return viridian_hypercall(regs);

Why is this a problem?  Truncation or not has no effect of this
calculation, and the exact value comes from the Xen-provided Viridian
hypercall page (see enable_hypercall_page() in viridian.c); we are at
liberty to change the semantics if we wish.

>
> I'm not convinced we should change current behavior, the more that
> the change has at least theoretical potential of breaking existing guests.

My argument is that no existing guests can be relying on this behaviour.

>
>> @@ -4283,6 +4283,8 @@ int hvm_do_hypercall(struct cpu_user_regs *regs)
>>          break;
>>      }
>>  
>> +    eax = (mode == 8) ? regs->eax : regs->_eax;
> But if we indeed want to make the adjustment, please use regs->rax
> here (slightly helping the register field renaming series I have pending
> for 4.9).

Can do.

~Andrew

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