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

[Roger Pau Monnà <roger.pau@xxxxxxxxxx>]

El 05/10/15 a les 11.34, Andrew Cooper ha escrit:
> 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;

But then the above chunk would become:

#ifdef CONFIG_X86
    struct xen_arch_domainconfig x86_config = { .emulation_flags = 0 };
    void *config = &x86_config;
#else
    void *config = NULL;
#endif

and the call to domain_create would be:

idle_domain = domain_create(DOMID_IDLE, 0, 0, config);

TBH I don't mind either way.

> 
> In the previous hunk.
> 
>>      BUG_ON(IS_ERR(idle_domain));
>>      idle_domain->vcpu = idle_vcpu;
>>      idle_domain->max_vcpus = nr_cpu_ids;
>> diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
>> index f1d7ed6..84ae4c1 100644
>> --- a/xen/include/asm-x86/domain.h
>> +++ b/xen/include/asm-x86/domain.h
>> @@ -387,8 +387,20 @@ struct arch_domain
>>      /* Mem_access emulation control */
>>      bool_t mem_access_emulate_enabled;
>>      bool_t mem_access_emulate_each_rep;
>> +
>> +    /* Emulated devices enabled bitmap. */
>> +    uint32_t emulation_flags;
>>  } __cacheline_aligned;
>>  
>> +#define has_vlapic(d)       ((d)->arch.emulation_flags & XEN_X86_EMU_LAPIC)
>> +#define has_vhpet(d)        ((d)->arch.emulation_flags & XEN_X86_EMU_HPET)
>> +#define has_vpmtimer(d)     ((d)->arch.emulation_flags & 
>> XEN_X86_EMU_PMTIMER)
>> +#define has_vrtc(d)         ((d)->arch.emulation_flags & XEN_X86_EMU_RTC)
>> +#define has_vioapic(d)      ((d)->arch.emulation_flags & XEN_X86_EMU_IOAPIC)
>> +#define has_vpic(d)         ((d)->arch.emulation_flags & XEN_X86_EMU_PIC)
>> +#define has_vvga(d)         ((d)->arch.emulation_flags & XEN_X86_EMU_VGA)
>> +#define has_viommu(d)       ((d)->arch.emulation_flags & XEN_X86_EMU_IOMMU)
> 
> It might be an idea to wrap these with (!! ...) to turn them into proper
> boolean values.

Thanks for catching this, will fix.

Roger.


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