[PATCH 0/4] Clarify explicit domain ID checks

[PATCH 0/4] Clarify explicit domain ID checks

[Daniel De Graaf]

The first three patches were posted for 4.3 but were deferred because
they were just cleanup; reposting for 4.4 along with similar changes in
the ARM code. This should remove all direct checks on a domain's ID in
the hypervisor, although many assumptions about dom0 remain (including
the global struct domain* dom0).

[PATCH 1/4] xen: use domid check in is_hardware_domain
[PATCH 2/4] xen/arch/x86: clarify domid == 0 checks
[PATCH 3/4] IOMMU: use is_hardware_domain instead of domid == 0
[PATCH 4/4] xen/arch/arm: clarify domid == 0 checks

[PATCH 1/4] xen: use domid check in is_hardware_domain

[Daniel De Graaf]

Instead of checking is_privileged to determine if a domain should
control the hardware, check that the domain_id is equal to zero (which
is currently the only domain for which is_privileged is true).  This
allows other places where domain_id is checked for zero to be replaced
with is_hardware_domain.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Keir Fraser <keir@xen.org>
---
 xen/common/domain.c     | 10 +++++-----
 xen/include/xen/sched.h |  4 ++--
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 6c264a5..692372a 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -238,7 +238,7 @@ struct domain *domain_create(
     if ( domcr_flags & DOMCRF_hvm )
         d->is_hvm = 1;
 
-    if ( domid == 0 )
+    if ( is_hardware_domain(d) )
     {
         d->is_pinned = opt_dom0_vcpus_pin;
         d->disable_migrate = 1;
@@ -263,10 +263,10 @@ struct domain *domain_create(
         d->is_paused_by_controller = 1;
         atomic_inc(&d->pause_count);
 
-        if ( domid )
-            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
-        else
+        if ( is_hardware_domain(d) )
             d->nr_pirqs = nr_static_irqs + extra_dom0_irqs;
+        else
+            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
         if ( d->nr_pirqs > nr_irqs )
             d->nr_pirqs = nr_irqs;
 
@@ -600,7 +600,7 @@ void domain_shutdown(struct domain *d, u8 reason)
         d->shutdown_code = reason;
     reason = d->shutdown_code;
 
-    if ( d->domain_id == 0 )
+    if ( is_hardware_domain(d) )
         dom0_shutdown(reason);
 
     if ( d->is_shutting_down )
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index ae6a3b8..4e6edf5 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -722,10 +722,10 @@ void watchdog_domain_destroy(struct domain *d);
 /* 
  * Use this check when the following are both true:
  *  - Using this feature or interface requires full access to the hardware
- *    (that is, this is would not be suitable for a driver domain)
+ *    (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)->is_privileged)
+#define is_hardware_domain(_d) ((_d)->domain_id == 0)
 
 /* This check is for functionality specific to a control domain */
 #define is_control_domain(_d) ((_d)->is_privileged)
-- 
1.8.1.4

Re: [PATCH 1/4] xen: use domid check in is_hardware_domain

[Jan Beulich]

>>> On 08.07.13 at 16:46, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -238,7 +238,7 @@ struct domain *domain_create(
>      if ( domcr_flags & DOMCRF_hvm )
>          d->is_hvm = 1;
>  
> -    if ( domid == 0 )
> +    if ( is_hardware_domain(d) )
>      {
>          d->is_pinned = opt_dom0_vcpus_pin;
>          d->disable_migrate = 1;
> @@ -263,10 +263,10 @@ struct domain *domain_create(
>          d->is_paused_by_controller = 1;
>          atomic_inc(&d->pause_count);
>  
> -        if ( domid )
> -            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
> -        else
> +        if ( is_hardware_domain(d) )
>              d->nr_pirqs = nr_static_irqs + extra_dom0_irqs;
> +        else
> +            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
>          if ( d->nr_pirqs > nr_irqs )
>              d->nr_pirqs = nr_irqs;
>  
> @@ -600,7 +600,7 @@ void domain_shutdown(struct domain *d, u8 reason)
>          d->shutdown_code = reason;
>      reason = d->shutdown_code;
>  
> -    if ( d->domain_id == 0 )
> +    if ( is_hardware_domain(d) )
>          dom0_shutdown(reason);

All earlier changes can be explained in one way or another to also
apply to other than Dom0. This one, however, can't: There can
only ever be one domain controling when to shut down the system,
and hence I think it is misleading to use is_hardware_domain()
here. Or is your targeted abstract model aiming at a single such
domain, just perhaps with a domain ID other than zero?

Jan

Re: [PATCH 1/4] xen: use domid check in is_hardware_domain

[Daniel De Graaf]

On 07/08/2013 10:58 AM, Jan Beulich wrote:
>>>> On 08.07.13 at 16:46, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -238,7 +238,7 @@ struct domain *domain_create(
>>       if ( domcr_flags & DOMCRF_hvm )
>>           d->is_hvm = 1;
>>
>> -    if ( domid == 0 )
>> +    if ( is_hardware_domain(d) )
>>       {
>>           d->is_pinned = opt_dom0_vcpus_pin;
>>           d->disable_migrate = 1;
>> @@ -263,10 +263,10 @@ struct domain *domain_create(
>>           d->is_paused_by_controller = 1;
>>           atomic_inc(&d->pause_count);
>>
>> -        if ( domid )
>> -            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
>> -        else
>> +        if ( is_hardware_domain(d) )
>>               d->nr_pirqs = nr_static_irqs + extra_dom0_irqs;
>> +        else
>> +            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
>>           if ( d->nr_pirqs > nr_irqs )
>>               d->nr_pirqs = nr_irqs;
>>
>> @@ -600,7 +600,7 @@ void domain_shutdown(struct domain *d, u8 reason)
>>           d->shutdown_code = reason;
>>       reason = d->shutdown_code;
>>
>> -    if ( d->domain_id == 0 )
>> +    if ( is_hardware_domain(d) )
>>           dom0_shutdown(reason);
>
> All earlier changes can be explained in one way or another to also
> apply to other than Dom0. This one, however, can't: There can
> only ever be one domain controling when to shut down the system,
> and hence I think it is misleading to use is_hardware_domain()
> here. Or is your targeted abstract model aiming at a single such
> domain, just perhaps with a domain ID other than zero?
>
> Jan

Yes, that is the model I am using. It splits dom0 into three domains:

0. Domain builder: bootstraps the system. May remain to perform requested
    builds of domains that need a minimal trust chain (i.e. vTPM domains).
    Other than being built by the hypervisor, nothing is special about this
    domain - although it may be useful to have is_control_domain be true.
1. Hardware domain: manages devices for PCI pass-through to driver domains
    or can act as a driver domain itself, depending on the desired degree
    of disaggregation. This is the only domain where is_hardware_domain()
    is true. The return of is_control_domain() is false for this domain.
2. Control domain: manages other domains, controls guest launch/shutdown,
    manages resource constraints, etc; is_control_domain() returns true.

This model has a working implementation derived from the work in the XOAR
paper. It requires a patch on top of these to change the IOMMU setup to
happen after dom0 starts, and a dedicated domain builder domain.

-- 
Daniel De Graaf
National Security Agency

Re: [PATCH 1/4] xen: use domid check in is_hardware_domain

[Jan Beulich]

>>> On 08.07.13 at 17:58, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> On 07/08/2013 10:58 AM, Jan Beulich wrote:
>> All earlier changes can be explained in one way or another to also
>> apply to other than Dom0. This one, however, can't: There can
>> only ever be one domain controling when to shut down the system,
>> and hence I think it is misleading to use is_hardware_domain()
>> here. Or is your targeted abstract model aiming at a single such
>> domain, just perhaps with a domain ID other than zero?
> 
> Yes, that is the model I am using. It splits dom0 into three domains:
> 
> 0. Domain builder: bootstraps the system. May remain to perform requested
>     builds of domains that need a minimal trust chain (i.e. vTPM domains).
>     Other than being built by the hypervisor, nothing is special about this
>     domain - although it may be useful to have is_control_domain be true.
> 1. Hardware domain: manages devices for PCI pass-through to driver domains
>     or can act as a driver domain itself, depending on the desired degree
>     of disaggregation. This is the only domain where is_hardware_domain()
>     is true. The return of is_control_domain() is false for this domain.
> 2. Control domain: manages other domains, controls guest launch/shutdown,
>     manages resource constraints, etc; is_control_domain() returns true.
> 
> This model has a working implementation derived from the work in the XOAR
> paper. It requires a patch on top of these to change the IOMMU setup to
> happen after dom0 starts, and a dedicated domain builder domain.

Okay, in that case

Reviewed-by: Jan Beulich <jbeulich@suse.com>

for the full series, and for patches 2 and 3 alternatively/additionally

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan

Re: [PATCH 1/4] xen: use domid check in is_hardware_domain

[George Dunlap]

On Mon, Jul 8, 2013 at 4:58 PM, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
> On 07/08/2013 10:58 AM, Jan Beulich wrote:
>>>>>
>>>>> On 08.07.13 at 16:46, Daniel De Graaf <dgdegra@tycho.nsa.gov> wrote:
>>>
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -238,7 +238,7 @@ struct domain *domain_create(
>>>       if ( domcr_flags & DOMCRF_hvm )
>>>           d->is_hvm = 1;
>>>
>>> -    if ( domid == 0 )
>>> +    if ( is_hardware_domain(d) )
>>>       {
>>>           d->is_pinned = opt_dom0_vcpus_pin;
>>>           d->disable_migrate = 1;
>>> @@ -263,10 +263,10 @@ struct domain *domain_create(
>>>           d->is_paused_by_controller = 1;
>>>           atomic_inc(&d->pause_count);
>>>
>>> -        if ( domid )
>>> -            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
>>> -        else
>>> +        if ( is_hardware_domain(d) )
>>>               d->nr_pirqs = nr_static_irqs + extra_dom0_irqs;
>>> +        else
>>> +            d->nr_pirqs = nr_static_irqs + extra_domU_irqs;
>>>           if ( d->nr_pirqs > nr_irqs )
>>>               d->nr_pirqs = nr_irqs;
>>>
>>> @@ -600,7 +600,7 @@ void domain_shutdown(struct domain *d, u8 reason)
>>>           d->shutdown_code = reason;
>>>       reason = d->shutdown_code;
>>>
>>> -    if ( d->domain_id == 0 )
>>> +    if ( is_hardware_domain(d) )
>>>           dom0_shutdown(reason);
>>
>>
>> All earlier changes can be explained in one way or another to also
>> apply to other than Dom0. This one, however, can't: There can
>> only ever be one domain controling when to shut down the system,
>> and hence I think it is misleading to use is_hardware_domain()
>> here. Or is your targeted abstract model aiming at a single such
>> domain, just perhaps with a domain ID other than zero?
>>
>> Jan
>
>
> Yes, that is the model I am using. It splits dom0 into three domains:
>
> 0. Domain builder: bootstraps the system. May remain to perform requested
>    builds of domains that need a minimal trust chain (i.e. vTPM domains).
>    Other than being built by the hypervisor, nothing is special about this
>    domain - although it may be useful to have is_control_domain be true.
> 1. Hardware domain: manages devices for PCI pass-through to driver domains
>    or can act as a driver domain itself, depending on the desired degree
>    of disaggregation. This is the only domain where is_hardware_domain()
>    is true. The return of is_control_domain() is false for this domain.
> 2. Control domain: manages other domains, controls guest launch/shutdown,
>    manages resource constraints, etc; is_control_domain() returns true.
>
> This model has a working implementation derived from the work in the XOAR
> paper. It requires a patch on top of these to change the IOMMU setup to
> happen after dom0 starts, and a dedicated domain builder domain.

Is the reason you have this broken down into four patches in order to
be able to CC different sets of people?

I think that will make it really hard for archaeologists to go back
and figure out what on earth these changes were for.  Since all the
patches basically do the same thing, I think it would be better to
have one big patch.

In any case, we need a description in either a comment or at least in
a changeset what your target model is (which you describe above).

 -George

[PATCH 2/4] xen/arch/x86: clarify domid == 0 checks

[Daniel De Graaf]

This makes checks for dom0 more explicit than checking the domain ID,
using is_hardware_domain for checks relating to hardware access and
is_control_domain for things that a control domain may need.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Keir Fraser <keir@xen.org>
Cc: Jan Beulich <jbeulich@suse.com>
---
 xen/arch/x86/domain.c    | 2 +-
 xen/arch/x86/hvm/i8254.c | 2 +-
 xen/arch/x86/time.c      | 4 ++--
 xen/arch/x86/traps.c     | 4 ++--
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 874742c..51d0ea6 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1459,7 +1459,7 @@ void context_switch(struct vcpu *prev, struct vcpu *next)
         }
 
         set_cpuid_faulting(!is_hvm_vcpu(next) &&
-                           (next->domain->domain_id != 0));
+                           !is_control_domain(next->domain));
     }
 
     if (is_hvm_vcpu(next) && (prev != next) )
diff --git a/xen/arch/x86/hvm/i8254.c b/xen/arch/x86/hvm/i8254.c
index c0d6bc2..add61e3 100644
--- a/xen/arch/x86/hvm/i8254.c
+++ b/xen/arch/x86/hvm/i8254.c
@@ -541,7 +541,7 @@ int pv_pit_handler(int port, int data, int write)
         .data = data
     };
 
-    if ( (current->domain->domain_id == 0) && dom0_pit_access(&ioreq) )
+    if ( is_hardware_domain(current->domain) && dom0_pit_access(&ioreq) )
     {
         /* nothing to do */;
     }
diff --git a/xen/arch/x86/time.c b/xen/arch/x86/time.c
index cf8bc78..bacc8ef 100644
--- a/xen/arch/x86/time.c
+++ b/xen/arch/x86/time.c
@@ -1885,7 +1885,7 @@ void tsc_set_info(struct domain *d,
                   uint32_t tsc_mode, uint64_t elapsed_nsec,
                   uint32_t gtsc_khz, uint32_t incarnation)
 {
-    if ( is_idle_domain(d) || (d->domain_id == 0) )
+    if ( is_idle_domain(d) || is_hardware_domain(d) )
     {
         d->arch.vtsc = 0;
         return;
@@ -1962,7 +1962,7 @@ static void dump_softtsc(unsigned char key)
                "warp=%lu (count=%lu)\n", tsc_max_warp, tsc_check_count);
     for_each_domain ( d )
     {
-        if ( d->domain_id == 0 && d->arch.tsc_mode == TSC_MODE_DEFAULT )
+        if ( is_hardware_domain(d) && d->arch.tsc_mode == TSC_MODE_DEFAULT )
             continue;
         printk("dom%u%s: mode=%d",d->domain_id,
                 is_hvm_domain(d) ? "(hvm)" : "", d->arch.tsc_mode);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 57dbd0c..8e8d3d1 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -745,7 +745,7 @@ static void pv_cpuid(struct cpu_user_regs *regs)
     c = regs->ecx;
     d = regs->edx;
 
-    if ( current->domain->domain_id != 0 )
+    if ( !is_control_domain(current->domain) )
     {
         unsigned int cpuid_leaf = a, sub_leaf = c;
 
@@ -1836,7 +1836,7 @@ static inline uint64_t guest_misc_enable(uint64_t val)
 static int is_cpufreq_controller(struct domain *d)
 {
     return ((cpufreq_controller == FREQCTL_dom0_kernel) &&
-            (d->domain_id == 0));
+            is_hardware_domain(d));
 }
 
 #include "x86_64/mmconfig.h"
-- 
1.8.1.4

[PATCH 3/4] IOMMU: use is_hardware_domain instead of domid == 0

[Daniel De Graaf]

This makes checks for dom0 more explicit than checking the domain ID.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Jacob Shin <jacob.shin@amd.com>
Cc: Xiantao Zhang <xiantao.zhang@intel.com>
---
 xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
 xen/drivers/passthrough/vtd/iommu.c         | 8 ++++----
 xen/drivers/passthrough/vtd/x86/vtd.c       | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
index 5ea1a1d..e8ec695 100644
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -122,7 +122,7 @@ static void amd_iommu_setup_domain_device(
 
     BUG_ON( !hd->root_table || !hd->paging_mode || !iommu->dev_table.buffer );
 
-    if ( iommu_passthrough && (domain->domain_id == 0) )
+    if ( iommu_passthrough && is_hardware_domain(domain) )
         valid = 0;
 
     if ( ats_enabled )
diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
index 0fc10de..8d5c43d 100644
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -1336,7 +1336,7 @@ int domain_context_mapping_one(
         return res;
     }
 
-    if ( iommu_passthrough && (domain->domain_id == 0) )
+    if ( iommu_passthrough && is_hardware_domain(domain) )
     {
         context_set_translation_type(*context, CONTEXT_TT_PASS_THRU);
         agaw = level_to_agaw(iommu->nr_pt_levels);
@@ -1710,7 +1710,7 @@ static int intel_iommu_map_page(
         return 0;
 
     /* do nothing if dom0 and iommu supports pass thru */
-    if ( iommu_passthrough && (d->domain_id == 0) )
+    if ( iommu_passthrough && is_hardware_domain(d) )
         return 0;
 
     spin_lock(&hd->mapping_lock);
@@ -1754,7 +1754,7 @@ static int intel_iommu_map_page(
 static int intel_iommu_unmap_page(struct domain *d, unsigned long gfn)
 {
     /* Do nothing if dom0 and iommu supports pass thru. */
-    if ( iommu_passthrough && (d->domain_id == 0) )
+    if ( iommu_passthrough && is_hardware_domain(d) )
         return 0;
 
     dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
@@ -1927,7 +1927,7 @@ static int intel_iommu_remove_device(u8 devfn, struct pci_dev *pdev)
     /* If the device belongs to dom0, and it has RMRR, don't remove it
      * from dom0, because BIOS may use RMRR at booting time.
      */
-    if ( pdev->domain->domain_id == 0 )
+    if ( is_hardware_domain(pdev->domain) )
     {
         for_each_rmrr_device ( rmrr, bdf, i )
         {
diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
index 875b033..eb9e52a 100644
--- a/xen/drivers/passthrough/vtd/x86/vtd.c
+++ b/xen/drivers/passthrough/vtd/x86/vtd.c
@@ -117,7 +117,7 @@ void __init iommu_set_dom0_mapping(struct domain *d)
 {
     unsigned long i, j, tmp, top;
 
-    BUG_ON(d->domain_id != 0);
+    BUG_ON(!is_hardware_domain(d));
 
     top = max(max_pdx, pfn_to_pdx(0xffffffffUL >> PAGE_SHIFT) + 1);
 
-- 
1.8.1.4

Re: [PATCH 3/4] IOMMU: use is_hardware_domain instead of domid == 0

[Suravee Suthikulanit]

On 7/8/2013 9:46 AM, Daniel De Graaf wrote:
> This makes checks for dom0 more explicit than checking the domain ID.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Acked-by: Suravee Suthikulpanit <suravee.suthiuklpanit@amd.com>

> Cc: Jacob Shin <jacob.shin@amd.com>
> Cc: Xiantao Zhang <xiantao.zhang@intel.com>
> ---
>   xen/drivers/passthrough/amd/pci_amd_iommu.c | 2 +-
>   xen/drivers/passthrough/vtd/iommu.c         | 8 ++++----
>   xen/drivers/passthrough/vtd/x86/vtd.c       | 2 +-
>   3 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/xen/drivers/passthrough/amd/pci_amd_iommu.c b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> index 5ea1a1d..e8ec695 100644
> --- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
> +++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
> @@ -122,7 +122,7 @@ static void amd_iommu_setup_domain_device(
>   
>       BUG_ON( !hd->root_table || !hd->paging_mode || !iommu->dev_table.buffer );
>   
> -    if ( iommu_passthrough && (domain->domain_id == 0) )
> +    if ( iommu_passthrough && is_hardware_domain(domain) )
>           valid = 0;
>   
>       if ( ats_enabled )
> diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c
> index 0fc10de..8d5c43d 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1336,7 +1336,7 @@ int domain_context_mapping_one(
>           return res;
>       }
>   
> -    if ( iommu_passthrough && (domain->domain_id == 0) )
> +    if ( iommu_passthrough && is_hardware_domain(domain) )
>       {
>           context_set_translation_type(*context, CONTEXT_TT_PASS_THRU);
>           agaw = level_to_agaw(iommu->nr_pt_levels);
> @@ -1710,7 +1710,7 @@ static int intel_iommu_map_page(
>           return 0;
>   
>       /* do nothing if dom0 and iommu supports pass thru */
> -    if ( iommu_passthrough && (d->domain_id == 0) )
> +    if ( iommu_passthrough && is_hardware_domain(d) )
>           return 0;
>   
>       spin_lock(&hd->mapping_lock);
> @@ -1754,7 +1754,7 @@ static int intel_iommu_map_page(
>   static int intel_iommu_unmap_page(struct domain *d, unsigned long gfn)
>   {
>       /* Do nothing if dom0 and iommu supports pass thru. */
> -    if ( iommu_passthrough && (d->domain_id == 0) )
> +    if ( iommu_passthrough && is_hardware_domain(d) )
>           return 0;
>   
>       dma_pte_clear_one(d, (paddr_t)gfn << PAGE_SHIFT_4K);
> @@ -1927,7 +1927,7 @@ static int intel_iommu_remove_device(u8 devfn, struct pci_dev *pdev)
>       /* If the device belongs to dom0, and it has RMRR, don't remove it
>        * from dom0, because BIOS may use RMRR at booting time.
>        */
> -    if ( pdev->domain->domain_id == 0 )
> +    if ( is_hardware_domain(pdev->domain) )
>       {
>           for_each_rmrr_device ( rmrr, bdf, i )
>           {
> diff --git a/xen/drivers/passthrough/vtd/x86/vtd.c b/xen/drivers/passthrough/vtd/x86/vtd.c
> index 875b033..eb9e52a 100644
> --- a/xen/drivers/passthrough/vtd/x86/vtd.c
> +++ b/xen/drivers/passthrough/vtd/x86/vtd.c
> @@ -117,7 +117,7 @@ void __init iommu_set_dom0_mapping(struct domain *d)
>   {
>       unsigned long i, j, tmp, top;
>   
> -    BUG_ON(d->domain_id != 0);
> +    BUG_ON(!is_hardware_domain(d));
>   
>       top = max(max_pdx, pfn_to_pdx(0xffffffffUL >> PAGE_SHIFT) + 1);
>   

[PATCH 4/4] xen/arch/arm: clarify domid == 0 checks

[Daniel De Graaf]

This makes checks for dom0 more explicit than checking the domain ID,
using is_hardware_domain for checks relating to hardware access.

Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
Cc: Tim Deegan <tim@xen.org>
---
 xen/arch/arm/domain.c | 2 +-
 xen/arch/arm/vgic.c   | 2 +-
 xen/arch/arm/vpl011.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index f465ab7..c7dc69a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -504,7 +504,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
         goto fail;
 
     /* Domain 0 gets a real UART not an emulated one */
-    if ( d->domain_id && (rc = domain_uart0_init(d)) != 0 )
+    if ( !is_hardware_domain(d) && (rc = domain_uart0_init(d)) != 0 )
         goto fail;
 
     return 0;
diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c
index 2e4b11f..d9c73be 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 ( d->domain_id == 0 )
+    if ( is_hardware_domain(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/vpl011.c b/xen/arch/arm/vpl011.c
index 13ba623..0e9454f 100644
--- a/xen/arch/arm/vpl011.c
+++ b/xen/arch/arm/vpl011.c
@@ -43,7 +43,7 @@
 
 int domain_uart0_init(struct domain *d)
 {
-    ASSERT( d->domain_id );
+    ASSERT( !is_hardware_domain(d) );
 
     spin_lock_init(&d->arch.uart0.lock);
     d->arch.uart0.idx = 0;
@@ -87,7 +87,7 @@ static int uart0_mmio_check(struct vcpu *v, paddr_t addr)
 {
     struct domain *d = v->domain;
 
-    return d->domain_id != 0 && addr >= UART0_START && addr < UART0_END;
+    return !is_hardware_domain(d) && addr >= UART0_START && addr < UART0_END;
 }
 
 static int uart0_mmio_read(struct vcpu *v, mmio_info_t *info)
-- 
1.8.1.4