On 29.11.2019 13:41, Jan Beulich wrote:
> On 21.11.2019 16:02, Alexandru Stefan ISAILA wrote:
>> Changes since V2:
>> - Drop static from xenmem_access_to_p2m_access() and declare it
>> in mem_access.h
>> - Use xenmem_access_to_p2m_access() in p2m_init_next_altp2m()
>> - Pull out the p2m specifics from p2m_init_altp2m_ept().
>
> I guess this last point would better have been a prereq patch,
> but anyway.
Should I have a prereq patch for this in the next version?
>
>> @@ -2577,16 +2586,23 @@ int p2m_init_altp2m_by_id(struct domain *d, unsigned
>> int idx)
>> altp2m_list_lock(d);
>>
>> if ( d->arch.altp2m_eptp[idx] == mfn_x(INVALID_MFN) )
>> - rc = p2m_activate_altp2m(d, idx);
>> + rc = p2m_activate_altp2m(d, idx, hostp2m->default_access);
>>
>> altp2m_list_unlock(d);
>> return rc;
>> }
>>
>> -int p2m_init_next_altp2m(struct domain *d, uint16_t *idx)
>> +int p2m_init_next_altp2m(struct domain *d, uint16_t *idx,
>> + uint16_t hvmmem_default_access)
>
> Does this new parameter really need to be a fixed width type,
> rather than simply unsigned int (or even a suitable enum
> type if there [hopefully] is one)?
I think xenmem_access_t would be a good fit here.
>
>> {
>> int rc = -EINVAL;
>> unsigned int i;
>> + p2m_access_t a;
>> + struct p2m_domain *p2m;
>> +
>> +
>
> Two successive blank lines again.
I will fix that.
>
>> @@ -2595,7 +2611,12 @@ 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) )
>> + return -EINVAL;
>
> Returning with a lock still held?
Thanks for spotting this, it definitely needs a free.
Alex
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
