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

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

>
>> --- a/xen/arch/x86/x86_64/mm.c
>> +++ b/xen/arch/x86/x86_64/mm.c
>> @@ -1031,6 +1031,9 @@ long do_set_segment_base(unsigned int which, unsigned 
>> long base)
>>      struct vcpu *v = current;
>>      long ret = 0;
>>  
>> +    if ( is_pv_32bit_vcpu(v) )
>> +        return -ENOSYS; /* x86/64 only. */
> And this addition could as well happen when that later change
> does the re-org.

Again, that would further complicate an already-complicated patch.

~Andrew

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