Re: [PATCH 2/3] x86/altp2m: Add a hvmop for setting the suppress #VE bit

[Adrian Pop <apop@bitdefender.com>]

On Thu, Jun 08, 2017 at 08:08:56AM -0600, Jan Beulich wrote:
> >>> On 08.06.17 at 15:49, <apop@bitdefender.com> wrote:
> > On Tue, Jun 06, 2017 at 07:08:43AM -0600, Jan Beulich wrote:
> >> >>> On 06.06.17 at 15:00, <apop@bitdefender.com> wrote:
> >> > On Mon, May 29, 2017 at 08:38:33AM -0600, Jan Beulich wrote:
> >> >> >>> On 18.05.17 at 17:07, <apop@bitdefender.com> wrote:
> >> >> > +
> >> >> > +    if ( !cpu_has_vmx )
> >> >> > +        return -EOPNOTSUPP;
> >> >> 
> >> >> Is this enough? Wouldn't it be better to signal the caller whenever
> >> >> hardware (or even software) isn't going to honor the request?
> >> > 
> >> > Well, the caller checks the return value.  The libxc function
> >> > xc_altp2m_set_suppress_ve for instance will return a negative in this
> >> > case.
> >> 
> >> The question wasn't what the caller does but whether checking just
> >> cpu_has_vmx is enough. What if you're running on a machine with
> >> VMX but no #VE support?
> > 
> > Oh, all right.  I misinterpreted it.  Yes, at least using something like
> > cpu_has_vmx_virt_exceptions instead of cpu_has_vmx would definitely be
> > more appropriate in this case.  Do you think there should be a more
> > thorough check?
> 
> Depends on what "more thorough" means: You'll want to check all
> features the code requires; I'm not certain if virt_exceptions is all
> it needs.
 
The checks so far would be:
- is the domain invoking this hvmop privileged?
- does the cpu have the #VE feature?
- is #VE enabled on this vcpu?

> >> >> And then there are two general questions: Without a libxc layer
> >> >> function, how is one supposed to use this new sub-op? Is it
> >> >> really intended to permit a guest to call this for itself?
> >> >  
> >> > Well, the sub-op could be used from a Linux kernel module if libxc is
> >> > not available if struct xen_hvm_altp2m_op and struct
> >> > xen_hvm_altp2m_set_suppress_ve are defined.
> >> > 
> >> > Our use case, though, involves either Dom0 or a "privileged" DomU
> >> > altering the suppress #VE bit for the target guest.
> >> 
> >> This doesn't really answer the question: What are the security
> >> implications if a guest can invoke this on itself?
> > 
> > Indeed it would be desirable that the guest doesn't use this hvmop on
> > itself.  It's also less than ideal that a DomU can call this on other
> > DomUs.
> 
> The latter is an absolute no-go.

Indeed.

> > After some talks it turns out that restricting this hvmop to a
> > privileged domain solves this issue and still works for our use case.
> > What do you think?
> 
> Restrictions should generally be put in place because of
> abstract considerations, not because of them not harming
> one's particular use case.

Of course.

> >> (FTR I think my first question was kind of pointless, as patch 3
> >> looks like it does introduce a libxc function; I simply didn't realize
> >> that back then, because I'd generally have expected the
> >> consumer of the hypercall to be introduce together with the
> >> producer.)
> > 
> > I can merge these two patches for v2 if that's what you want.
> 
> I'd prefer that, but others may have differing opinions. And
> there are certainly benefits in keeping hypervisor and tools
> changes separate.
 
Ok then.

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