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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 6 Jul 2017, Jan Beulich wrote:
> >>> On 05.07.17 at 22:39, <julien.grall@xxxxxxx> wrote:
> > On 05/07/2017 19:35, Stefano Stabellini wrote:
> >> On Tue, 4 Jul 2017, Jan Beulich wrote:
> >>> 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.
> 
> There's nothing memaccess-specific in the handler of the operation.
> Where faults go merely depends on whether a monitor has been
> registered afaict. Hence ...
> 
> > 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.
> 
> ... no matter which way we allow setting the permissions for the
> purpose here, we'll have to deal with what you describe, except
> that as per above the question of setting up a ring looks orthogonal
> to the apparent conflicts here.
> 
> Considering the intended purpose here (as far as I recall it), was it
> already taken into consideration to request suitable attributes right
> at the time the page gets installed into the physmap? Iirc there's no
> need to actually "play" with the attributes at random times.

This operation would be done before the guest starts.


Let's give a look at the list the changes that would be required to make
these hypercalls suitable for this task:

1) remove the dependency on CONFIG_HAS_MEM_ACCESS
2) remove the p2m_mem_access_sanity_check check for these two hypercalls
3) remove the (!d->vm_event->monitor.ring_page) check for these two hypercalls
4) prevent p2m->mem_access_enabled from being set for these two hypercalls

Am I missing anything? After we do this, would they still be useful for
their original mem_access related purpose?

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