Re: [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.

[Julien Grall <julien.grall@arm.com>]

On 06/07/16 17:12, Tamas K Lengyel wrote:
> On Wed, Jul 6, 2016 at 8:32 AM, Julien Grall <julien.grall@arm.com> wrote:
>>
>>
>> On 05/07/16 22:55, Sergej Proskurin wrote:
>>>
>>> Hello Julien,
>>>
>>> On 07/05/2016 02:49 PM, Julien Grall wrote:
>>>>
>>>> Hello Sergej,
>>>>
>>>> On 04/07/16 12:45, Sergej Proskurin wrote:
>>>>>
>>>>> +static inline
>>>>> +int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain
>>>>> *hp2m,
>>>>> +                              struct p2m_domain *ap2m, p2m_access_t a,
>>>>> +                              gfn_t gfn)
>>>>> +{
>>>>
>>>>
>>>> [...]
>>>>
>>>>> +    /* Set mem access attributes - currently supporting only one (4K)
>>>>> page. */
>>>>> +    mask = level_masks[3];
>>>>> +    return apply_p2m_changes(d, ap2m, INSERT,
>>>>> +                             gpa & mask,
>>>>> +                             (gpa + level_sizes[level]) & mask,
>>>>> +                             maddr & mask, mattr, 0, p2mt, a);
>>>>
>>>>
>>>> The operation INSERT will remove the previous mapping and decrease page
>>>> reference for foreign mapping (see p2m_put_l3_page). So if you set the
>>>> memory access for this kind of page, the reference count will be wrong
>>>> afterward.
>>>>
>>>
>>> I see your point. As far as I understand, the purpose of the function
>>> p2m_put_l3_page to simply decrement the ref count of the particular pte
>>> and free the page if its' ref count reaches zero. Since p2m_put_l3_page
>>> is called only twice in p2m.c, we could insert a check
>>> (p2m_is_hostp2m/altp2m) making sure that the ref count is decremented
>>> only if the p2m in question is the hostp2m. This will make sure that the
>>> ref count is maintained correctly.
>>
>>
>> Why don't you use the operation MEMACCESS?
>
> I believe it would require duplicating some parts of the INSERT
> routine as MEMACCESS assumes the pte is already present in the p2m,
> which may not be the case for ap2m.

Hmmm right. The p2m code is already complex so extending MEMACCESS will 
be worst. I don't much like the idea of a check which I think is very 
fragile, although it would be my preference for now.

Regards,

-- 
Julien Grall

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