Re: [PATCH 4/6] xen: Allow hardare domain != dom0

[Daniel De Graaf <dgdegra@tycho.nsa.gov>]

On 03/04/2014 10:50 PM, Julien Grall wrote:
> Hello Daniel,
>
> On 05/03/14 06:51, Daniel De Graaf wrote:
>> This adds a hypervisor command line option "hardware_dom=" which takes a
>> domain ID.  When the domain with this ID is created, it will be used as
>> the hardware domain.
>>
>> This is intended to be used when dom0 is a dedicated stub domain for
>> domain building, allowing the hardware domain to be de-privileged and
>> act only as a driver domain.
>>
>> 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_build.c |  4 +++-
>>   xen/arch/x86/setup.c        |  3 +++
>>   xen/common/domctl.c         |  8 ++++++++
>>   xen/common/rangeset.c       | 26 ++++++++++++++++++++++++++
>>   xen/include/xen/rangeset.h  |  3 +++
>>   xen/include/xen/sched.h     |  3 ++-
>>   6 files changed, 45 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/domain_build.c b/xen/arch/x86/domain_build.c
>> index 84ce392..e9de496 100644
>> --- a/xen/arch/x86/domain_build.c
>> +++ b/xen/arch/x86/domain_build.c
>> @@ -1149,7 +1149,9 @@ int __init construct_dom0(
>>           printk(" Xen warning: dom0 kernel broken ELF: %s\n",
>>                  elf_check_broken(&elf));
>>
>> -    iommu_dom0_init(dom0);
>> +    if( is_hardware_domain(dom0) )
>> +        iommu_dom0_init(dom0);
>> +
>>       return 0;
>>
>>   out:
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index 3a4f69c..3480854 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -88,6 +88,9 @@ unsigned long __initdata highmem_start;
>>   size_param("highmem-start", highmem_start);
>>   #endif
>>
>> +unsigned int __read_mostly hardware_dom;
>> +integer_param("hardware_dom", hardware_dom);
>> +
>>   cpumask_t __read_mostly cpu_present_map;
>>
>>   unsigned long __read_mostly xen_phys_start;
>> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
>> index eebeee7..11e6b94 100644
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -472,6 +472,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
>>               break;
>>           }
>>
>> +        if (d->domain_id == hardware_dom) {
>> +            printk("Initialising hardware domain %d\n", hardware_dom);
>> +            rangeset_swap(d->irq_caps, dom0->irq_caps);
>> +
>> +            dom0 = d;
>> +            iommu_dom0_init(dom0);
>> +        }
>> +
>
> This patch will break compilation on ARM. You are using hardware_dom which is defined in xen/arch/x86/setup.c.
>
> I'm not sure if the best solution is to move the defined for hardware_dom in common code ... because settings this variable to a value other than 0 will break ARM boot with your changes in patch #1.
>
> Supporting hardware domain on ARM will need rework on dom0 builder.

For the moment, I think adding hardware_dom as a #define to 0 for ARM
may be the best solution.  Some of the arguments for needing a
separate hardware domain and domain builder are less relevant on an
ARM system (at least based on my limited understanding of how ARM
hardware usually speaks with other devices), and the domain builder
that I posted currently will not compile for ARM.

-- 
Daniel De Graaf
National Security Agency