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

[Roger Pau Monnà <royger@xxxxxxxxxxx>]

El 07/10/15 a les 16.01, Andrew Cooper ha escrit:
> 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.

Ack, I also prefer this way if ARM already does it.

Roger.


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