Re: [PATCH v4] x86/mm: Add mem access rights to NPT

[Isaila Alexandru <aisaila@bitdefender.com>]

On Mi, 2018-07-25 at 03:14 -0600, Jan Beulich wrote:
> > 
> > > 
> > > > 
> > > > On 25.07.18 at 10:29, <aisaila@bitdefender.com> wrote:
> > > > 
> > > > +static void p2m_set_access(struct p2m_domain *p2m, unsigned
> > > > long
> > > > gfn,
> > > > +                                      p2m_access_t a)
> > > > +{
> > > > +    int rc;
> > > > +
> > > > +    if ( !p2m->mem_access_settings )
> > > > +        return;
> > > No error indication?
> > I would say ASSERT is a better choice if the code got this far and
> > it
> > could not allocate memory
> For one ASSERT() is a no-op in release builds. And then it is
> extremely bad practices to bring down the host when an operation
> targeting just a single guest has failed. You either return an error
> indicator here (and pass it up the call tree), or if that's really
> unfeasible then you crash the affected domain (we do so in quite
> a few other situations). But you'd need to make clear (if it's not
> obvious) why passing up an error is unacceptable here.
> 
By this time in the code the radix tree should be in place. If it is
not then the domain should crash because something is wrong and the mem
access feature will not function so passing the error up will have a
result of crashing the domain later after checking. 

I will add a domain crash here and a comment regarding it.

Thanks,
Alex

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