Re: [PATCH v3] x86/altp2m: Added xc_altp2m_set_mem_access_multi()

[Razvan Cojocaru <rcojocaru@bitdefender.com>]

On 09.10.2017 10:23, Jan Beulich wrote:
>>>> On 06.10.17 at 18:07, <rcojocaru@bitdefender.com> wrote:
>> On 10/06/2017 06:34 PM, Jan Beulich wrote:
>>>>>> On 05.10.17 at 17:42, <ppircalabu@bitdefender.com> wrote:
>>>> @@ -4451,6 +4453,7 @@ static int do_altp2m_op(
>>>>       case HVMOP_altp2m_destroy_p2m:
>>>>       case HVMOP_altp2m_switch_p2m:
>>>>       case HVMOP_altp2m_set_mem_access:
>>>> +    case HVMOP_altp2m_set_mem_access_multi:
>>>
>>> Was it agreed that this, just like others (many wrongly, I think) is
>>> supposed to be invokable by the affected domain itself? Its non-
>>> altp2m counterpart is a DM_PRIV operation. If the one here is
>>> meant to be different, I think the commit message should say why.
>>
>> In the absence of an answer from the designers of altp2m, we've chosen
>> to remain consistent with HVMOP_altp2m_set_mem_access - since that is
>> allowed to be invoked by the domain itself, this operation is also
>> allowed to do that.
>>
>> Back in March, I've sent a DOMCTL version:
>>
>> https://patchwork.kernel.org/patch/9633615/
>>
>> and a HVMOP version (minus the compat part):
>>
>> https://patchwork.kernel.org/patch/9612799/
>>
>> It has been discussed, and an authoritative answer on the design of this
>> was sought out, but despite several kind reminders during this time, it
>> never came. At this point, the least modification to the initial design
>> appears to be to keep the new operation as a HVMOP. This is an important
>> optimization, and the waiting period for objections has surely been
>> reasonable.
> 
> Okay, this is (sort of) fine, but especially when there was no
> feedback the decision taken (and its reason) should be recorded
> in the commit message. As stated above as well as earlier, I
> strongly think that the altp2m permissions are too lax right now
> (and hence the patch here widens the problem, but I can agree
> that making it match set_mem_access is not unreasonable).

Of course. We'll update the commit message. Thanks for the review!


Razvan

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