Re: [Xen-devel] [PATCH v6 5/9] x86/mem_sharing: use default_access in add_to_physmap

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.01.2020 18:02, Tamas K Lengyel wrote:
> On Tue, Jan 28, 2020 at 9:56 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>
>> On 27.01.2020 19:06, Tamas K Lengyel wrote:
>>> When plugging a hole in the target physmap don't use the access permission
>>> returned by __get_gfn_type_access as it can be non-sensical, leading to
>>> spurious vm_events being sent out for access violations at unexpected
>>> locations. Make use of p2m->default_access instead.
>>
>> As before, to me "can be non-sensical" is insufficient as a reason
>> here. If it can also be a "good" value, it still remains unclear
>> why in that case p2m->default_access is nevertheless the right
>> value to use.
> 
> I have already explained in the previous version of the patch why I
> said "can be". Forgot to change the commit message from "can be" to
> "is".

Changing just the commit message would be easy while committing.
But even with the change I would ask why this is. Looking at
ept_get_entry() (and assuming p2m_pt_get_entry() will work
similarly, minus the p2m_access_t which can't come out of the
PTE just yet), I see

    if ( is_epte_valid(ept_entry) )
    {
        *t = p2m_recalc_type(recalc || ept_entry->recalc,
                             ept_entry->sa_p2mt, p2m, gfn);
        *a = ept_entry->access;

near its end. Which means even a hole can have its access field
set. So it's still not clear to me from the description why
p2m->default_access is uniformly the value to use. Wouldn't you
rather want to override the original value only if it's
p2m_access_n together with p2m_invalid or p2m_mmio_dm (but not
paged-out pages)? Of course then the question is whether there
wouldn't be an ambiguity with p2m_access_n having got set
explicitly on the page. But maybe this is impossible for
p2m_invalid / p2m_mmio_dm?

Jan

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