Re: [Xen-devel] [PATCH 25/27] xen/arm: mm: Embed permission in the flags

[Julien Grall <julien.grall@xxxxxxx>]

On 08/23/2017 03:37 PM, Andre Przywara wrote:
> Hi,
Hi Andre,

> On 23/08/17 15:26, Julien Grall wrote:
> > On 08/23/2017 03:08 PM, Andre Przywara wrote:
> > > Hi,
> > Hi,
> >
> > > On 14/08/17 15:24, Julien Grall wrote:
> > > > Currently, it is not possible to specify the permission of a new
> > > > mapping. It would be necessary to use the function modify_xen_mappings
> > > > with a different set of flags.
> Just saw that I forgot the typos here:
>
> > > > Add introduce a couple of new flags for the permissions (Non-eXecutable,
> Either "add" or "introduce", I guess.
I guess my mind disagree with my hands :). I will use "introduce" here.

> > > > Read-Only) and also provides define that combine the memory attribute
> > > > and permission for common combination.
> Somehow the plural/singular is messed up here, I needed to read that
> sentence multiple times.
>
> > > If I haven't been lost in the definitions, this now adds "not
> > > executable" to the existing definitions, which seems to make sense, but
> > > is a change that might trigger regressions (especially for
> > > PAGE_HYPERVISOR). So I wonder if that should be mentioned in the commit
> > > message then?
> > It will not trigger regression because mfn_to_xen_entry is setting xn to
> > 1 by default. So all the mapping will be execute never when using
> > PAGE_HYPERVISOR.
> Ah right, I missed that. Might still be worth to mention in the commit
> message, as this isn't obvious from just that patch.
I can do that. Below the suggested commit message:

"Currently, it is not possible to specify the permission of a new
mapping. It would be necessary to use the function modify_xen_mappings
with a different set of flags.

Introduce a couple of new flags for the permissions (Non-eXecutable,
Read-Only) and also provides define that combine the memory attribute
and permission for common combination.

PAGE_HYPERVISOR is now an alias to PAGE_HYPERVISOR_RW (read-write 
non-executable mappings). This does not affect the current mapping using 
PAGE_HYPERVISOR because this
A follow-up patch will change modify_xen_mappings to use the new flags."

Can I keep your reviewed-by?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel