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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.02.2020 14:18, Alexandru Stefan ISAILA wrote:
> @@ -4840,6 +4841,19 @@ static int do_altp2m_op(
>          break;
>      }
>  
> +    case HVMOP_altp2m_set_visibility:
> +    {
> +        uint16_t idx = a.u.set_visibility.altp2m_idx;

Why a fixed width type (and even one inefficient to deal with)?
(One might even ask - why a local variable in the first place,
when it's used ...

> +        if ( a.u.set_visibility.pad )
> +            rc = -EINVAL;
> +        else if ( !altp2m_active(d) )
> +            rc = -EOPNOTSUPP;
> +        else
> +            rc = p2m_set_altp2m_view_visibility(d, idx,
> +                                                a.u.set_visibility.visible);

... just once here.) The function takes "unsigned int" in any
event.

> @@ -3145,6 +3148,35 @@ int p2m_get_suppress_ve(struct domain *d, gfn_t gfn, 
> bool *suppress_ve,
>  
>      return rc;
>  }
> +
> +int p2m_set_altp2m_view_visibility(struct domain *d, unsigned int altp2m_idx,
> +                                   uint8_t visible)
> +{
> +    altp2m_list_lock(d);
> +
> +    /*
> +     * Eptp index is correlated with altp2m index and should not exceed
> +     * min(MAX_ALTP2M, MAX_EPTP).
> +     */
> +    if ( altp2m_idx >= min(ARRAY_SIZE(d->arch.altp2m_p2m), MAX_EPTP) ||
> +         d->arch.altp2m_eptp[array_index_nospec(altp2m_idx, MAX_EPTP)] ==
> +         mfn_x(INVALID_MFN) )
> +    {
> +        altp2m_list_unlock(d);

I think it would be nice if this went the normal function exit path.
Would be pretty simple to arrange for by introducing a local variable
holding the function return value.

Jan

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