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

[Julien Grall <julien.grall@linaro.org>]

Hello Daniel,

On 05/03/14 06:51, Daniel De Graaf wrote:
> 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.
>
> The distinction between is_hardware_domain, is_control_domain, and
> domain 0 is based on the following disaggregation model:
>
> Domain 0 bootstraps the system.  It 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() return
> true depending on the toolstack it uses to build other domains.
>
> The 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.  It is also the domain managing devices that
> do not support pass-through: PCI configuration space access, parsing the
> hardware ACPI tables and system power or machine check events.  This is
> the only domain where is_hardware_domain() is true.  The return of
> is_control_domain() is false for this domain.
>
> The control domain manages other domains, controls guest launch and
> shutdown, and manages resource constraints; is_control_domain() returns
> true.  The functionality guarded by is_control_domain may in the future
> be adapted to use explicit hypercalls, eliminating the special treatment
> of this domain.  It may be reasonable to have multiple control domains
> on a multi-tenant system.
>
> Guest domains and other service or driver domains are all treated
> identically by the hypervisor; the security policy may further constrain
> administrative actions on or communication between these domains.
>
> Signed-off-by: Daniel De Graaf <dgdegra@tycho.nsa.gov>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Keir Fraser <keir@xen.org>
> Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Xiantao Zhang <xiantao.zhang@intel.com>
> ---
>   xen/arch/arm/domain.c                       |  2 +-
>   xen/arch/arm/gic.c                          |  2 +-
>   xen/arch/arm/vgic.c                         |  2 +-
>   xen/arch/arm/vuart.c                        |  2 +-
>   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 ++--
>   xen/common/domain.c                         | 10 +++++-----
>   xen/common/xenoprof.c                       |  2 +-
>   xen/drivers/passthrough/amd/pci_amd_iommu.c |  2 +-
>   xen/drivers/passthrough/iommu.c             |  2 +-
>   xen/drivers/passthrough/vtd/iommu.c         |  8 ++++----
>   xen/drivers/passthrough/vtd/x86/vtd.c       |  2 +-
>   xen/include/xen/sched.h                     |  4 ++--
>   15 files changed, 25 insertions(+), 25 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index 8f20fdf..4b9afb2 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -547,7 +547,7 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags)
>        * Only use it for dom0 because the linux kernel may not support
>        * multi-platform.
>        */
> -    if ( (d->domain_id == 0) && (rc = domain_vuart_init(d)) )
> +    if ( is_hardware_domain(d) && (rc = domain_vuart_init(d)) )

Can you update the comment above the check?

>           goto fail;
>
>       return 0;
> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
> index 074624e..5d7ae3d 100644
> --- a/xen/arch/arm/gic.c
> +++ b/xen/arch/arm/gic.c
> @@ -862,7 +862,7 @@ int gicv_setup(struct domain *d)
>        * Domain 0 gets the hardware address.
>        * Guests get the virtual platform layout.
>        */
> -    if ( d->domain_id == 0 )
> +    if ( is_hardware_domain(d) )

Same here.

Regards,

-- 
Julien Grall