On 18.12.2019 12:45, Jan Beulich wrote:
> On 17.12.2019 16:12, Alexandru Stefan ISAILA wrote:
>> --- a/xen/arch/x86/mm/mem_access.c
>> +++ b/xen/arch/x86/mm/mem_access.c
>> @@ -314,9 +314,9 @@ static int set_mem_access(struct domain *d, struct
>> p2m_domain *p2m,
>> return rc;
>> }
>>
>> -static bool xenmem_access_to_p2m_access(struct p2m_domain *p2m,
>> - xenmem_access_t xaccess,
>> - p2m_access_t *paccess)
>> +bool xenmem_access_to_p2m_access(struct p2m_domain *p2m,
>> + xenmem_access_t xaccess,
>> + p2m_access_t *paccess)
>
> Would you mind taking the opportunity and add const to the first
> parameter?
Sure, given that there will be a new version, it will add it.
>
>> @@ -2601,7 +2610,15 @@ int p2m_init_next_altp2m(struct domain *d, uint16_t
>> *idx)
>> if ( d->arch.altp2m_eptp[i] != mfn_x(INVALID_MFN) )
>> continue;
>>
>> - rc = p2m_activate_altp2m(d, i);
>> + p2m = d->arch.altp2m_p2m[i];
>> +
>> + if ( !xenmem_access_to_p2m_access(p2m, hvmmem_default_access, &a) )
>> + {
>> + altp2m_list_unlock(d);
>> + return -EINVAL;
>> + }
>
> Can this be pulled out of the locked region, ahead of the loop?
> The p2m getting passed in here (which is why it's in the loop)
> shouldn't have been in use yet, i.e. its ->default_access should
> have a known value. Hence this case could be taken care of
> independently, e.g. by adjusting xenmem_access_to_p2m_access()
> to cope with a NULL p2m coming in (producing whatever the default
> of the default is).
>
Yes this sounds good. In xenmem_access_to_p2m_access() there can be a
check like:
if ( !p2m )
*paccess = p2m_access_rwx;
else
*paccess = p2m->default_access;
But before I change this maybe Tamas or George have something to add?
And can this stay in the same patch or should it have a prereq one?
Alex
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
