Re: [Xen-devel] [PATCH V2 1/2] x86/altp2m: Add hypercall to set a range of sve bits

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Wed, Nov 13, 2019 at 7:51 AM Tamas K Lengyel <tamas@xxxxxxxxxxxxx> wrote:
>
> On Tue, Nov 12, 2019 at 7:31 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >
> > On 12.11.2019 15:05, Tamas K Lengyel wrote:
> > > On Tue, Nov 12, 2019 at 4:54 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > >> On 06.11.2019 16:35, Alexandru Stefan ISAILA wrote:
> > >>> +        else
> > >>> +        {
> > >>> +            rc = p2m_set_suppress_ve_multi(d, &a.u.suppress_ve);
> > >>> +
> > >>> +            if ( rc == -ERESTART )
> > >>> +                if ( __copy_field_to_guest(guest_handle_cast(arg,
> > >>> +                                           xen_hvm_altp2m_op_t),
> > >>> +                                           &a, u.suppress_ve.opaque) )
> > >>> +                    rc = -EFAULT;
> > >>
> > >> If the operation is best effort, _some_ indication of failure should
> > >> still be handed back to the caller. Whether that's through the opaque
> > >> field or by some other means is secondary. If not via that field
> > >> (which would make the outer of the two if()-s disappear), please fold
> > >> the if()-s.
> > >
> > > At least for mem_sharing_range_op we also do a best-effort and don't
> > > return an error for pages where it wasn't possible to share. So I
> > > don't think it's absolutely necessary to do that, especially if the
> > > caller can't do anything about those errors anyway.
> >
> > mem-sharing is a little different in nature, isn't it? If you
> > can't share a page, both involved guests will continue to run
> > with their own instances. If you want to suppress #VE delivery
> > and it fails, behavior won't be transparently correct, as
> > there'll potentially be #VE when there should be none. Whether
> > that's benign to the guest very much depends on its handler.
>
> Makes me wonder whether it would make more sense to flip this thing on
> its head and have supress_ve be set by default (since its ignored by
> default) and then have pages for which the EPT violation should be
> convertible to #VE be specifically enabled by turning suppress_ve off.
> That would eliminate the possibility of having the in-guest handler
> getting #VE for pages it is not ready to handle. The hypervisor (and
> the external VMI toolstack) OTOH should always be in a position to
> handle EPT violations it itself causes by changing the page
> permissions.

Actually, now that I looked at it, that's _exactly_ what we do
already. The suppress_ve bit is always set for all EPT pages. So this
operation here is going to be used to enable #VE for pages, not the
other way around. So there wouldn't be a case of "potentially be #VE
when there should be none".

Tamas

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