Re: [Xen-devel] [RFC] DOMCTL_memattrs_op : a new DOMCTL to play with stage-2 page attributes

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 4 Jul 2017, Jan Beulich wrote:
> >>> On 03.07.17 at 19:58, <sstabellini@xxxxxxxxxx> wrote:
> > On Mon, 3 Jul 2017, Zhongze Liu wrote:
> >> 2017-07-03 22:25 GMT+08:00 Jan Beulich <JBeulich@xxxxxxxx>:
> >> >>>> On 30.06.17 at 22:15, <blackskygg@xxxxxxxxx> wrote:
> >> >> /* flags for XEN_DOMCTL_MEMATTRS_OP_SET_PERMISSIONS */
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_N         0x00U
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_R         (0x01U<<0)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_W         (0x01U<<1)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_X         (0x01U<<2)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RW        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_W)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RX        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_R|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_WX        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_W|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> >> #define XEN_DOMCTL_MEMATTRS_ACCESS_RWX        \
> >> >> (XEN_DOMCTL_MEMATTRS_ACCESS_RW|XEN_DOMCTL_MEMATTRS_ACCESS_X)
> >> >
> >> > ... with this basically duplicating
> >> > XENMEM_access_op_{set,get}_access I now wonder whether
> >> > we don't already have all you need (apart from an ARM variant of
> >> > DOMCTL_pin_mem_cacheattr).
> >> 
> >> In fact, there isn't much description on the usage of this
> >> interface, so I turned to the implementation in
> >> xen/common/mem_access.c, where I see this
> >> interface invoking  p2m_set_mem_acess, which further invokes
> >> set_mem_acess and finally
> >> p2m->set_entry(), so I guess this might be the right interface to use.
> >> To confirm the guess, I turned to Stabellini for help, and he told me
> >> that XENMEM_access_op
> >> is "for getting very detail info on what the guest is accessing", and
> >> might not be suitable
> >> for this scenario, so I just gave up using it, and that's why I have this 
> >> RFC.
> >> I'll re-confirm this with Stabellini.
> > 
> > I thought that those two hypercalls were meant to be used for mem_access
> > and vm_event operations, as in xen/arch/arm/mem_access.c and
> > xen/arch/x86/mm/mem_access.c. The only caller is
> > tools/tests/xen-access/xen-access.c. They are enabled separatly as part
> > of the mem_access interface: their build is conditional to
> > CONFIG_HAS_MEM_ACCESS. Unless we want to move them from XENMEM_access_*
> > to DOMCTL_* operations, I don't think they could be used?
> 
> For one, we could remove the CONFIG_HAS_MEM_ACCESS around
> them if a broader use is planned. And in general we should try to
> avoid having two ways of doing the same thing, unless backwards
> compatibility makes this a requirement. Hence if a new, better way
> is to be introduced, the old one should at once go away. Finally, I'm
> still unconvinced a new DOMCTL_* is better here than a (tool stack
> only) XENMEM_*, but I agree the boundary between when to use
> what is at best fuzzy.

Do we maintain ABI compatibility for XENMEM_* hypercalls? I think we do,
don't we? Also, XENMEM_* hypercalls are usually available to both
guests and toolstack, right?

We don't want two ways of doing the same thing, but at the same time
XENMEM_ hypercalls are very different from DOMCTLs, which don't come
with any ABI compatibility guarantees and are only available to the
toolstack. And these two specific XENMEM hypercalls even depend on
CONFIG_HAS_MEM_ACCESS.

I am not completely sure about what the best way forward would be. I am
OK with anything that is clear and maintainable. I would probably still
go with updating DOMCTL_pin_mem_cacheattr into something that can handle
both ARM and permissions, but I am also OK with making changes to
XENMEM_access_op_{set,get}_access so that they become an option for this
use case.

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