Re: [Xen-devel] [PATCH 2/9] x86/pv: Support do_set_segment_base() for compat guests

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/08/16 14:31, Jan Beulich wrote:
>>>> On 02.08.16 at 15:25, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 02/08/16 13:52, Jan Beulich wrote:
>>>>>> On 18.07.16 at 11:51, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> set_segment_base is the only hypercall exists in only one of the two modes
>>>> guests might run in; all other hypercalls are either implemented, or
>>>> unimplemented in both modes.
>>>>
>>>> Remove this split, by allowing do_set_segment_base() to be called in the
>>>> compat hypercall path.  This change will simplify the verification logic 
>>>> in a
>>>> later change.
>>>>
>>>> No behavioural change from a guests point of view.
>>> Nevertheless I don't view this as a reasonable change on its own:
>> Why not?  It is far better to call it out in isolation, than to mix it
>> in with an already-existing complicated change.
> If the changes here were anywhere near being themselves
> complicated, I'd agree.
>
>>>> --- a/xen/arch/x86/x86_64/compat/entry.S
>>>> +++ b/xen/arch/x86/x86_64/compat/entry.S
>>>> @@ -453,7 +453,7 @@ ENTRY(compat_hypercall_table)
>>>>          .quad compat_update_va_mapping_otherdomain
>>>>          .quad compat_iret
>>>>          .quad compat_vcpu_op
>>>> -        .quad compat_ni_hypercall       /* 25 */
>>>> +        .quad do_set_segment_base       /* 25 */
>>> This part will (I suppose) be deleted by a later change.
>> When moved into C, it will be hidden behind the macros used to construct
>> both table values at once.
> And compat_set_segment_base could then just be #define-d to NULL.

The entire purpose of making this change is so we don't end up with NULL
in one half a pair of function pointers, and have to extend the runtime
logic to check different pointers.

This is the only hypercall with this problem, so I have opted to make it
not a problem, rather than causing an extra runtime hit on all hypercalls.

A 32bit PV guest doesn't have any reason to make this hypercall.  It it
does, I really don't care that it takes a few instructions extra to
return -ENOSYS.

~Andrew

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