Re: [Xen-devel] [PATCH V2 2/2] x86/mm: Make use of the default access param from xc_altp2m_create_view

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.11.2019 09:38, Alexandru Stefan ISAILA wrote:
> On 12.11.2019 14:02, Jan Beulich wrote:
>> On 06.11.2019 16:35, Alexandru Stefan ISAILA wrote:
>>> @@ -2572,17 +2574,36 @@ 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)
>>>   {
>>>       int rc = -EINVAL;
>>>       unsigned int i;
>>>   
>>> +    static const p2m_access_t memaccess[] = {
>>> +#define ACCESS(ac) [XENMEM_access_##ac] = p2m_access_##ac
>>> +        ACCESS(n),
>>> +        ACCESS(r),
>>> +        ACCESS(w),
>>> +        ACCESS(rw),
>>> +        ACCESS(x),
>>> +        ACCESS(rx),
>>> +        ACCESS(wx),
>>> +        ACCESS(rwx),
>>> +        ACCESS(rx2rw),
>>> +        ACCESS(n2rwx),
>>> +#undef ACCESS
>>> +    };
>>> +
>>> +    if ( hvmmem_default_access > XENMEM_access_default )
>>> +        return rc;
>>> +
>>>       altp2m_list_lock(d);
>>>   
>>>       for ( i = 0; i < MAX_ALTP2M; i++ )
>>> @@ -2590,7 +2611,7 @@ 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);
>>> +        rc = p2m_activate_altp2m(d, i, memaccess[hvmmem_default_access]);
>>
>> Aren't you open-coding xenmem_access_to_p2m_access() here? In
>> no event should there be two instances of the same static array.
> 
> I did this because xenmem_access_to_p2m_access() is defined static in 
> x86/mm/mem_access.c. If it's ok to have it defined in mem_access.h then 
> I can go with that and drop this part of the code.

I see no reason why this wouldn't be a reasonable step, allowing to
avoid code duplication. Looks like the function is even suitably
named already for making non-static.

Jan

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