Re: [Xen-devel] [PATCH v4 2/2] allow hardware domain != dom0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 14.04.14 at 23:23, <dgdegra@xxxxxxxxxxxxx> wrote:

Much better!

> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -590,6 +590,16 @@ Paging (HAP).
>  Flag to enable 2 MB host page table support for Hardware Assisted
>  Paging (HAP).
>  
> +### hardware\_dom
> +> `= <domid>`
> +
> +> Default: `0`
> +
> +Enable late hardware domain creation using the specified domain ID.  This is
> +intended to be used when domain 0 is a stub domain which builds a 
> disaggregated
> +system including a hardware domain with the specified domain ID.  This 
> option is
> +supported only when compiled with CONFIG\_XSM on x86.

I think we ought to name the build option (XSM_ENABLE=y) here
rather than the internally used manifest constant to propagate the
requested setting.

> +static int late_hwdom_init(struct domain *d)
> +{
> +#ifdef CONFIG_LATE_HWDOM
> +    struct domain *dom0;
> +    int rv;
> +
> +    if ( d != hardware_domain || d->domain_id == 0 )
> +        return 0;
> +
> +    rv = xsm_init_hardware_domain(XSM_HOOK, d);
> +    if ( rv )
> +        return rv;
> +
> +    printk("Initialising hardware domain %d\n", hardware_domid);
> +
> +    dom0 = rcu_lock_domain_by_id(0);
> +    ASSERT(dom0 != NULL);
> +    /*
> +     * Hardware resource ranges for domain 0 have been set up from
> +     * various sources intended to restrict the hardware domain's
> +     * access.  Apply these ranges to the actual hardware domain.
> +     *
> +     * Because the lists are being swapped, a side effect of this
> +     * operation is that Domain 0's rangesets are cleared.  Since
> +     * domain 0 should not be accessing the hardware when it constructs
> +     * a hardware domain, this should not be a problem.  Both lists
> +     * may be modified after this hypercall returns if a more complex
> +     * device model is desired.
> +     *
> +     * Since late hardware domain initialization is only supported on
> +     * x86, the reference to arch.ioport_caps does not need its own
> +     * preprocessor conditional.

This is only "for now" - please either say so, or even better drop this
part of the comment and add the conditional regardless of it being
redundant right now.

> +void rangeset_swap(struct rangeset *a, struct rangeset *b)
> +{
> +    struct list_head tmp;
> +    if (&a < &b)

Coding style. Also I pretty certain you don't want to compare the
addresses of the pointers here, but the pointer themselves.

> @@ -73,4 +73,7 @@ void rangeset_printk(
>  void rangeset_domain_printk(
>      struct domain *d);
>  
> +/* swap contents */
> +void rangeset_swap(struct rangeset *a, struct rangeset *b);
> +

Please put this above the printing ones.

Jan


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