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

Re: [Xen-devel] [PATCH 1/2] adjust special domain creation (and call it earlier on x86)



>>> On 31.05.19 at 17:32, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 31/05/2019 02:35, Jan Beulich wrote:
>> @@ -516,6 +521,36 @@ struct domain *domain_create(domid_t dom
>>      return ERR_PTR(err);
>>  }
>>  
>> +void __init setup_special_domains(void)
>> +{
>> +    /*
>> +     * Initialise our DOMID_XEN domain.
>> +     * Any Xen-heap pages that we will allow to be mapped will have
>> +     * their domain field set to dom_xen.
>> +     * Hidden PCI devices will also be associated with this domain
>> +     * (but be [partly] controlled by Dom0 nevertheless).
>> +     */
>> +    dom_xen = domain_create(DOMID_XEN, NULL, false);
>> +    BUG_ON(IS_ERR(dom_xen));
> 
> I know this is copying code like-for-like, but this error handling is
> terrible in practice.
> 
> Even just:
> 
> if ( IS_ERR(dom_xen) )
>     panic("Failed to create dom_xen: %d\n", PTR_ERR(dom_xen));
> 
> would be an improvement.

I'll be happy to do this; I didn't just because it doesn't really belong
here.

>> +#ifdef CONFIG_HAS_PCI
>> +    INIT_LIST_HEAD(&dom_xen->arch.pdev_list);
>> +#endif
> 
> The position of this identifies that we've got obviously got bugs
> (perhaps latent) elsewhere, seeing as other special domains don't get
> working constructs such as list_empty().
> 
> In the code which currently exists, I can't spot it ever being touched
> for ARM, but it is constructed for all non-special x86 guests at the top
> of arch_domain_create().
> 
> I think the best option, given the #ifdef here, is to reposition the
> pdev fields into struct domain, rather than arch_domain, and have this
> code fragment near the top of domain_create() where special domains will
> all be covered.

Except that if I do this, then not by special casing special domains.
"Normal" domains want this too - the initialization could then be
dropped (moved there) from x86-specific code. But this will want to
be a separate patch then.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

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