Re: [Xen-devel] [PATCH v10] x86/emulate: Send vm_event from emulate

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxxxxxx>]

On 9/17/19 5:11 PM, Alexandru Stefan ISAILA wrote:
>>>>> +bool hvm_monitor_check_p2m(unsigned long gla, gfn_t gfn, uint32_t pfec,
>>>>> +                           uint16_t kind)
>>>>> +{
>>>>> +    xenmem_access_t access;
>>>>> +    vm_event_request_t req = {};
>>>>> +    paddr_t gpa = (gfn_to_gaddr(gfn) | (gla & ~PAGE_MASK));
>>>>> +
>>>>> +    ASSERT(current->arch.vm_event->send_event);
>>>>> +
>>>>> +    current->arch.vm_event->send_event = false;
>>>>> +
>>>>> +    if ( p2m_get_mem_access(current->domain, gfn, &access,
>>>>> +                            altp2m_vcpu_idx(current)) != 0 )
>>>>> +        return false;
>>>> ... next to the call here (but the maintainers of the file would
>>>> have to judge in the end). That said, I continue to not understand
>>>> why a not found entry means unrestricted access. Isn't it
>>>> ->default_access which controls what such a "virtual" entry would
>>>> permit?
>>> I'm sorry for this misleading comment. The code states that if entry was
>>> not found the access will be default_access and return 0. So in this
>>> case the default_access will be checked.
>>>
>>> /* If request to get default access. */
>>> if ( gfn_eq(gfn, INVALID_GFN) )
>>> {
>>>        *access = memaccess[p2m->default_access];
>>>        return 0;
>>> }
>>>
>>> If this clears thing up I can remove the "NOTE" part if the comment.
>> I'm afraid it doesn't clear things up: I'm still lost as to why
>> "entry not found" implies "full access". And I'm further lost as
>> to what the code fragment above (dealing with INVALID_GFN, but
>> not really the "entry not found" case, which would be INVALID_MFN
>> coming back from a translation) is supposed to tell me.
>>
> It is safe enough to consider a invalid mfn from hostp2 to be a
> violation. There is still a small problem with having the altp2m view
> not having the page propagated from hostp2m. In this case we have to use
> altp2m_get_effective_entry().

In the absence of clear guidance from the Intel SDM on what the hardware 
default is for a page not present in the p2m, we should probably follow 
Jan's advice and check violations against default_access for such pages.

There are indeed - as discussed privately - two cases for an altp2m though:

1. Page not found in the altp2m, but set in the hostp2m - in which case 
I suggest that we take the hostp2m value into account (with or without 
propagation to the altp2m; I see no harm in propagating the entry but 
other may see something I'm missing).

2. Page not found in both altp2m and hostp2m - in which case we should 
probably check against default_access.


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