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

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Mon, Nov 18, 2019 at 2:53 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> 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.

Sounds fine to me too.

Thanks,
Tamas

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