Re: [Xen-devel] [PATCH 6/7] xen: Allow hardare domain != dom0

[Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>]

On 04/11/2014 05:13 AM, Jan Beulich wrote:
> > > > On 27.03.14 at 12:52, <dgdegra@xxxxxxxxxxxxx> wrote:
> I was about to commit this, but further changes are needed (along
> with fixing the typo in the title):
I noticed the typo soon after posting v3, but didn't want to post
v4 with that and some other whitespace.  It looks like v4 will have
useful changes now, however.

> > --- a/xen/arch/x86/setup.c
> > +++ b/xen/arch/x86/setup.c
> > @@ -84,6 +84,8 @@ unsigned long __initdata highmem_start;
> >   size_param("highmem-start", highmem_start);
> >   #endif
> >
> > +integer_param("hardware_dom", hardware_domid);
> This should be moved alongside the definition of the symbol in
> common code, and be enclosed in "#ifdef CONFIG_LATE_HWDOM".
>
> And docs/misc/xen-command-line.markdown wants an entry for it.
OK.

> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -45,6 +45,7 @@ DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_compat_t);
> >
> >   /* A global pointer to the hardware domain (usually DOM0). */
> >   extern struct domain *hardware_domain;
> > +extern domid_t hardware_domid;
> >
> >   #ifndef CONFIG_COMPAT
> >   #define BITS_PER_EVTCHN_WORD(d) BITS_PER_XEN_ULONG
> > @@ -794,7 +795,7 @@ void watchdog_domain_destroy(struct domain *d);
> >    *    (that is, this would not be suitable for a driver domain)
> >    *  - There is never a reason to deny dom0 access to this
> >    */
> > -#define is_hardware_domain(_d) ((_d)->domain_id == 0)
> > +#define is_hardware_domain(d)  ((d)->domain_id == hardware_domid)
> This macro should imo evaluate to true for Dom0 until the hardware
> domain go created, i.e. you should compare _d with hardware_domain
> rather than their IDs. With that the definition of hardware_domid can
> then also be moved inside the #ifdef requested above.
This isn't quite as simple as changing the function since there are
some places where is_hardware_domain needs to return false for domain 0
when a hardware domain is used.  Also, the hardware_domain variable is
not set until domain_create returns, so there are a few places where the
domain ID still needs to be checked explicitly.  It should be possible
to create an is_hardware_domid function for those cases, if comparing to
hardware_domain is preferred for most cases; I think that would belong in
a new patch 5.5/7 (i.e. 6/8 in v4).

Otherwise, I think the is_hardware_domain definition should be:

#ifdef CONFIG_LATE_HWDOM
#define is_hardware_domain(_d) ((_d)->domain_id == hardware_domid)
#else
#define is_hardware_domain(_d) ((_d)->domain_id == 0)
#endif

This also allows hardware_domid to be declared inside the #ifdef.

--
Daniel De Graaf
National Security Agency

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