Re: [Xen-devel] [PATCH V4] tools/libxc, xen/x86: Added xc_set_mem_access_multi()

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 09/15/2016 04:55 PM, Wei Liu wrote:
> On Thu, Sep 15, 2016 at 04:52:32PM +0300, Razvan Cojocaru wrote:
>> On 09/15/2016 04:49 PM, Wei Liu wrote:
>>> On Thu, Sep 15, 2016 at 04:39:47PM +0300, Razvan Cojocaru wrote:
>>>> On 09/07/2016 07:01 PM, Jan Beulich wrote:
>>>>>>>> On 07.09.16 at 11:12, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>>>>> Currently it is only possible to set mem_access restrictions only for
>>>>>> a contiguous range of GFNs (or, as a particular case, for a single GFN).
>>>>>> This patch introduces a new libxc function taking an array of GFNs.
>>>>>> The alternative would be to set each page in turn, using a userspace-HV
>>>>>> roundtrip for each call, and triggering a TLB flush per page set.
>>>>>>
>>>>>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>>>>>> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>>
>>>>> Hypervisor parts (without ARM and x86/mm)
>>>>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>>>
>>>>> albeit I spotted one more cosmetic issue (which I guess could be
>>>>> fixed up during commit, if no other reason for a v5 arises):
>>>>>
>>>>>> @@ -321,9 +322,22 @@ int compat_memory_op(unsigned int cmd, 
>>>>>> XEN_GUEST_HANDLE_PARAM(void) compat)
>>>>>>          }
>>>>>>  
>>>>>>          case XENMEM_access_op:
>>>>>> -            return mem_access_memop(cmd,
>>>>>> -                                    guest_handle_cast(compat,
>>>>>> -                                                      
>>>>>> xen_mem_access_op_t));
>>>>>> +        {
>>>>>> +            if ( copy_from_guest(&cmp.mao, compat, 1) )
>>>>>> +                return -EFAULT;
>>>>>> +
>>>>>> +#define XLAT_mem_access_op_HNDL_pfn_list(_d_, _s_) \
>>>>>> +            guest_from_compat_handle((_d_)->pfn_list, (_s_)->pfn_list)
>>>>>> +#define XLAT_mem_access_op_HNDL_access_list(_d_, _s_) \
>>>>>> +            guest_from_compat_handle((_d_)->access_list, 
>>>>>> (_s_)->access_list)
>>>>>> +
>>>>>> +            XLAT_mem_access_op(nat.mao, &cmp.mao);
>>>>>> +
>>>>>> +#undef XLAT_mem_access_op_HNDL_pfn_list
>>>>>> +#undef XLAT_mem_access_op_HNDL_access_list
>>>>>> +
>>>>>> +            break;
>>>>>> +        }
>>>>>
>>>>> There are no local variables declared here, so I don't see the need
>>>>> for the braces.
>>>>
>>>> There have only been Acked-by replies so far, but if you'd prefer I have
>>>> no problem sending a V5 removing the braces.
>>>>
>>>
>>> Does this patch have all the necessary acks? If so I don't mind fixing
>>> it up and committing it.
>>
>> In addition to your ack, it's:
>>
>> Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
>> Acked-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
>> Acked-by: Julien Grall <julien.grall@xxxxxxx>
>>
>> for the hypervisor parts (without ARM and x86/mm), vm_event and ARM
>> respectively.
>>
> 
> I think it still needs an ack from George for x86/mm changes.

Fair enough.


Thanks,
Razvan

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