Re: [Xen-devel] [RFC 21/22] xen/arm: p2m: Re-implement p2m_set_mem_access using p2m_{set, get}_entry

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Aug 1, 2016 at 10:33 AM, Julien Grall <julien.grall@xxxxxxx> wrote:
>
>
> On 01/08/16 17:27, Tamas K Lengyel wrote:
>>
>> On Mon, Aug 1, 2016 at 10:15 AM, Julien Grall <julien.grall@xxxxxxx>
>> wrote:
>>>
>>>
>>>
>>> On 01/08/16 16:59, Tamas K Lengyel wrote:
>>>>
>>>>
>>>> On Mon, Aug 1, 2016 at 9:40 AM, Julien Grall <julien.grall@xxxxxxx>
>>>> wrote:
>>>> IMHO we should just pause the domain while mem_access permissions are
>>>> being changed. On x86 it is not necessary as the mem_access
>>>> bookkeeping is kept in the ept pte entries but since on ARM the two
>>>> things are disjoint it is required. Even on x86 though - while it's
>>>> not strictly necessary - I think it would be a good idea to just pause
>>>> the domain as from a user perspective it would be more intuitive then
>>>> keeping the vCPUs running.
>>>
>>>
>>>
>>> The problem is not because of the bookkeeping, but how the TLBs work. I
>>> never mentioned the radix tree in my previous mail because storing the
>>> access in the entry will not help here. The entry may have been updated
>>> but
>>> the TLBs not yet invalidated.
>>>
>>> x86 does not seem to be affected because if the mem access check fail,
>>> the
>>> instruction is replayed (see hvm_hap_nested_page_fault). This is exactly
>>> the
>>> solution I suggested, which is IHMO the best.
>>
>>
>> I see. Yes, indeed that sounds like the route to take here.
>
>
> I am looking forward to see a patch.
>
>>>
>>> I don't think pausing all the vCPUs of a domain is a good solution, the
>>> overhead will be too high for modifying only one page (see
>>> p2m_mem_access_check for instance).
>>>
>>
>> IMHO the overhead of pausing the domain for setting mem_access
>> permissions is not critical as it is done only on occasion. We can
>> certainly leave the default behavior like this but I'll probably also
>> introduce a new XENMEM_access_op_set_access_sync op that will pause
>> the domain for the duration of the op.
>
>
> Oh, you meant for a user app to set the memaccess. I guess it is fine. I
> mentioned p2m_mem_access_check because this function is supposed to be
> called often in the data/instruction abort handlers, so pausing a domain
> here would be a big overhead.

Right, that would be in most cases. We have a couple mem_access types
though that automatically change to a different one on the first
violation (rx2rw, n2rwx) which could also benefit from the domain
being paused for the duration of the change. All the other permissions
are fine as they are and should only require a domain pause when the
user is setting them.

Tamas

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