Re: [Xen-devel] [PATCH] xen/pvshim: fix GNTTABOP_query_size hypercall forwarding with SMAP

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/01/18 16:34, Roger Pau Monné wrote:
> On Fri, Jan 26, 2018 at 04:28:27PM +0000, Wei Liu wrote:
>> On Fri, Jan 26, 2018 at 04:22:41PM +0000, Roger Pau Monné wrote:
>>> On Fri, Jan 26, 2018 at 03:52:38PM +0000, Wei Liu wrote:
>>>> On Fri, Jan 26, 2018 at 03:29:10PM +0000, Roger Pau Monne wrote:
>>>>> Disable SMAP in the shim before bouncing the hypercall, or else L0
>>>>> will fail to get the hypercall buffer.
>>>>>
>>>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>>> Reported-by: Fatih Acar <fatih.acar@xxxxxxxxx>
>>>>> ---
>>>>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>>>>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>> Cc: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
>>>>> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>> ---
>>>>> Needs to be backported to the 4.10.0-shim-comet branch.
>>>>> ---
>>>>>  xen/arch/x86/pv/shim.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>>
>>>>> diff --git a/xen/arch/x86/pv/shim.c b/xen/arch/x86/pv/shim.c
>>>>> index d5383dcfc7..eb1ee7d3c4 100644
>>>>> --- a/xen/arch/x86/pv/shim.c
>>>>> +++ b/xen/arch/x86/pv/shim.c
>>>>> @@ -748,7 +748,10 @@ static long pv_shim_grant_table_op(unsigned int cmd,
>>>>>      }
>>>>>  
>>>>>      case GNTTABOP_query_size:
>>>>> +        /* Disable SMAP so L0 can access the buffer. */
>>>> Interesting.
>>>>
>>>>> +        stac();
>>>>>          rc = xen_hypercall_grant_table_op(GNTTABOP_query_size, uop.p, 
>>>>> count);
>>>>> +        clac();
>>>> There is so far only one instance of this, so doing this ad-hoc stac /
>>>> clac is OK.
>>>>
>>>> Probably another (more uniformed) way of fixing it is to have our own
>>>> buffer (forbid passing through uop directly) and the copy back the
>>>> result. Or we should invent a set of macros to deal with uop.
>>> I guess we should figure out what's faster, disabling SMAP or copying
>>> to a buffer?
>>>
>> It depends. If there are going to be instances that L0 needs to copy to
>> a large buffer in L2 userspace, disabling SMAP is definitely better then
>> bouncing.
>>
>> In this particular case, it doesn't matter.
>>
>> You can have my Rb on this patch if you think disabling SMAP is better.
> It's less cumbersome than copying, and certainly faster if there's no
> SMAP support, since that's just a nop in that case.

This hypercall is used once on boot.  Its not a perf-critical situation.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel