Re: [Xen-devel] [PATCH V4] x86/altp2m: Hypercall to set altp2m view visibility

[Alexandru Stefan ISAILA <aisaila@xxxxxxxxxxxxxxx>]

On 24.02.2020 11:06, Alexandru Stefan ISAILA wrote:
> 
> 
> On 21.02.2020 18:39, Jan Beulich wrote:
>> On 21.02.2020 09:30, Alexandru Stefan ISAILA wrote:
>>> @@ -4835,6 +4836,26 @@ static int do_altp2m_op(
>>>            break;
>>>        }
>>>    
>>> +    case HVMOP_altp2m_set_visibility:
>>> +    {
>>> +        uint16_t idx = a.u.set_visibility.altp2m_idx;
>>> +
>>> +        if ( a.u.set_visibility.pad ||
>>> +             idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
>>
>> Why min() here? You only access MAX_EPTP-dimensioned arrays afaics. If
>> this is intentional, I think it deserves a comment.
> 
> I have min() here because the altp2m index should not be grater then
> MAX_ALTP2M. I know this is used as altp2m_eptp() index but the two are
> coupled.
> 
>>
>>> +             d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)] ==
>>> +             mfn_x(INVALID_MFN) )
>>> +            rc = -EINVAL;
>>> +        else if ( !altp2m_active(d) )
>>> +            rc = -EOPNOTSUPP;
>>> +        else if ( a.u.set_visibility.visible )
>>> +            d->arch.altp2m_working_eptp[array_index_nospec(idx, MAX_EPTP)] 
>>> =
>>> +                d->arch.altp2m_eptp[array_index_nospec(idx, MAX_EPTP)];
>>> +        else
>>> +            d->arch.altp2m_working_eptp[array_index_nospec(idx, MAX_EPTP)] 
>>> =
>>> +                mfn_x(INVALID_MFN);
>>> +        break;
>>
>> You don't seem to be holding any locks. At the very least this means
>> the in-range-index-is-valid check further up will have become stale
>> by the time you actually consume the slot.
>>
> 
> Good thing to point this out here. I will add altp2m_list_lock/unlock(d)
> to guard this check and operation.

I think it's better to move all the altp2m things in a new function, 
something like p2m_set_altp2m_view_visibility(), in p2m.c. This way 
there will be no additional include needed.

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