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

[Jan Beulich <jbeulich@xxxxxxxx>]

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?

> @@ -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).

Jan

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