Re: [Xen-devel] [PATCH v7 12/32] xen/x86: add bitmap of enabled emulated devices

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/10/15 14:32, George Dunlap wrote:
> On 07/10/15 12:55, Roger Pau Monnà wrote:
>> El 06/10/15 a les 13.05, George Dunlap ha escrit:
>>> On 05/10/15 10:34, Andrew Cooper wrote:
>>>> On 02/10/15 16:48, Roger Pau Monne wrote:
>>>>> Introduce a bitmap in x86 xen_arch_domainconfig that allows enabling or
>>>>> disabling specific devices emulated inside of Xen for HVM guests.
>>>>>
>>>>> Signed-off-by: Roger Pau Monnà <roger.pau@xxxxxxxxxx>
>>>>> Acked-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
>>>>> Cc: Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>
>>>>> Cc: Ian Campbell <ian.campbell@xxxxxxxxxx>
>>>>> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
>>>>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>>>>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>>>> Cc: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>>>> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, with 2 small
>>>> suggestions.
>>>>
>>>>> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
>>>>> index 5ffa1a1..aeb911e 100644
>>>>> --- a/xen/common/schedule.c
>>>>> +++ b/xen/common/schedule.c
>>>>> @@ -1428,6 +1428,9 @@ static struct notifier_block cpu_schedule_nfb = {
>>>>>  /* Initialise the data structures. */
>>>>>  void __init scheduler_init(void)
>>>>>  {
>>>>> +#ifdef CONFIG_X86
>>>>> +    struct xen_arch_domainconfig config = { .emulation_flags = 0 };
>>>>> +#endif
>>>>>      struct domain *idle_domain;
>>>>>      int i;
>>>>>  
>>>>> @@ -1474,8 +1477,11 @@ void __init scheduler_init(void)
>>>>>          sched_ratelimit_us = SCHED_DEFAULT_RATELIMIT_US;
>>>>>      }
>>>>>  
>>>>> -    /* There is no need of arch-specific configuration for an idle 
>>>>> domain */
>>>>> +#ifdef CONFIG_X86
>>>>> +    idle_domain = domain_create(DOMID_IDLE, 0, 0, &config);
>>>>> +#else
>>>>>      idle_domain = domain_create(DOMID_IDLE, 0, 0, NULL);
>>>>> +#endif
>>>> You could get away without this ifdefary if you have
>>>>
>>>> #else
>>>>     void *config = NULL;
>>>>
>>>> In the previous hunk.
>>> It would be even better if you could do what arm does, and accept NULL
>>> configurations for the idle, xen, io, and cow domains.
>> I'm not sure of that. HVM domains certainly require config to be
>> provided, while PV domains could do without it. IMHO it's better for
>> consistency of interfaces to always require a valid arch config rather
>> than make it depend on the domain type.
> ARM requires a config for all *real* domains -- including all guests and
> dom0; there's an ASSERT in xen/arm/domain.c:arch_domain_create() to that
> effect.  Nonetheless, it accepts NULL for the four "domains" I mentioned
> above, because they aren't real domains.  Consistency says it would be
> better if x86 behaved like arm (requiring config for real HVM and PV
> domains, but not for fake domains).

I have to say that I am leaning towards the same opinion.

~Andrew

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