[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Hi,

On 05/07/2017 19:35, Stefano Stabellini wrote:
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.

I am struggling to understand how you could make memaccess_op_*_access supporting 2 distinct use cases. They are currently used to instrospect memory by restricting the permission. All the faults will be forwarded to a monitor.

Here you suggest to extend them to restrict permission. But we want to be able to support introspection on that share page (I don't see why not) and we don't want to have to set-up a VM-event ring just for restrict the page.

Moreover, you would have to store the access permission for the time-being... whilst here you just modify the permission of the page for good.

Am I missing something here?

Cheers,

--
Julien Grall

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.