[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



On 04/11/2014 11:20 AM, Jan Beulich wrote:
On 11.04.14 at 17:07, <dgdegra@xxxxxxxxxxxxx> wrote:
On 04/11/2014 05:13 AM, Jan Beulich wrote:
On 27.03.14 at 12:52, <dgdegra@xxxxxxxxxxxxx> wrote:
@@ -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.

But that still wouldn't necessarily do the correct thing for any use of
the macro before that new special case code in domain_create() got
run. Maybe my thinking of this is wrong, but as I tried to state above,
I would expect Dom0 to be the hardware domain up to the point
where the intended hardware domain gets created, at which point all
state Dom0 obtained because of having been the de-facto hardware
domain get transferred to hardware_domain.

I agree with this in most cases, and I think the few places where that
is not true should be changed to make them more explicit.  This must
include all checks in domain_create and those in functions called from
domain_create, because (d == hardware_domain) is always false inside
domain_create.  An initial version of this patch is below, but unless
there are objections I plan to integrate it into patch 1 to avoid doing
(d->domain_id == 0) => is_hardware_domain => is_hardware_domain_by_id
for the ARM code.

------------------------------>8------------------------

Subject: [PATCH RFC 6/8] xen: introduce is_hardware_domain_by_id

In order to better handle the period when domain 0 is running but has
not yet built a late hardware domain, change is_hardware_domain to do
what its name implies: check that the passed domain is the hardware
domain.  To address some cases where this variable is not set (in
particular, during the creation of domain 0 this variable will still be
NULL), the old check is retained with the name is_hardware_domain_by_id.

Signed-off-by: Daniel De Graaf <dgdegra@xxxxxxxxxxxxx>
---
 xen/arch/arm/domain.c   | 2 +-
 xen/arch/arm/gic.c      | 2 +-
 xen/arch/arm/vgic.c     | 2 +-
 xen/arch/arm/vtimer.c   | 2 +-
 xen/arch/arm/vuart.c    | 2 +-
 xen/common/domain.c     | 4 ++--
 xen/include/xen/sched.h | 9 ++++++++-
 7 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index ccccb77..4f235e4 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -528,7 +528,7 @@ int arch_domain_create(struct domain *d, unsigned int 
domcr_flags)
      * Only use it for the hardware domain because the linux kernel may not
      * support multi-platform.
      */
-    if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )
+    if ( is_hardware_domain_by_id(d) && (rc = domain_vuart_init(d)) )
         goto fail;
return 0;
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 8168b7b..5c16aba 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -868,7 +868,7 @@ int gicv_setup(struct domain *d)
      * The hardware domain gets the hardware address.
      * Guests get the virtual platform layout.
      */
-    if ( is_hardware_domain(d) )
+    if ( is_hardware_domain_by_id(d) )
     {
         d->arch.vgic.dbase = gic.dbase;
         d->arch.vgic.cbase = gic.cbase;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 4a7f8c0..cc795e0 100644
--- a/xen/arch/arm/vgic.c
+++ b/xen/arch/arm/vgic.c
@@ -82,7 +82,7 @@ int domain_vgic_init(struct domain *d)
     /* Currently nr_lines in vgic and gic doesn't have the same meanings
      * Here nr_lines = number of SPIs
      */
-    if ( is_hardware_domain(d) )
+    if ( is_hardware_domain_by_id(d) )
         d->arch.vgic.nr_lines = gic_number_lines() - 32;
     else
         d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */
diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
index cb690bb..4dcb80d 100644
--- a/xen/arch/arm/vtimer.c
+++ b/xen/arch/arm/vtimer.c
@@ -54,7 +54,7 @@ int vcpu_domain_init(struct domain *d)
 int vcpu_vtimer_init(struct vcpu *v)
 {
     struct vtimer *t = &v->arch.phys_timer;
-    bool_t d0 = is_hardware_domain(v->domain);
+    bool_t d0 = is_hardware_domain_by_id(v->domain);
/*
      * Hardware domain uses the hardware interrupts, guests get the virtual
diff --git a/xen/arch/arm/vuart.c b/xen/arch/arm/vuart.c
index c02a8a9..cf9745c 100644
--- a/xen/arch/arm/vuart.c
+++ b/xen/arch/arm/vuart.c
@@ -46,7 +46,7 @@
int domain_vuart_init(struct domain *d)
 {
-    ASSERT( is_hardware_domain(d) );
+    ASSERT( is_hardware_domain_by_id(d) );
d->arch.vuart.info = serial_vuart_info(SERHND_DTUART);
     if ( !d->arch.vuart.info )
diff --git a/xen/common/domain.c b/xen/common/domain.c
index c4720a9..4036e69 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -237,7 +237,7 @@ struct domain *domain_create(
     else if ( domcr_flags & DOMCRF_pvh )
         d->guest_type = guest_type_pvh;
- if ( is_hardware_domain(d) )
+    if ( is_hardware_domain_by_id(d) )
     {
         d->is_pinned = opt_dom0_vcpus_pin;
         d->disable_migrate = 1;
@@ -262,7 +262,7 @@ struct domain *domain_create(
         d->is_paused_by_controller = 1;
         atomic_inc(&d->pause_count);
- if ( !is_hardware_domain(d) )
+        if ( !is_hardware_domain_by_id(d) )
             d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
         else
             d->nr_pirqs = nr_static_irqs + extra_dom0_irqs;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index cbbe8a4..34447f1 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -797,7 +797,14 @@ 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) == hardware_domain)
+
+/*
+ * Check for the hardware domain based on domain ID, not using the global
+ * hardware_domain variable.  This is necessary inside domain_create where
+ * hardware_domain is NULL and for some checks implementing CONFIG_LATE_HWDOM.
+ */
+#define is_hardware_domain_by_id(_d) ((_d)->domain_id == 0)
/* This check is for functionality specific to a control domain */
 #define is_control_domain(_d) ((_d)->is_privileged)
--
1.9.0


Attachment: 0006-xen-introduce-is_hardware_domain_by_id.patch
Description: Text Data

Attachment: 0007-xen-Allow-hardware-domain-dom0.patch
Description: Text Data

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.