Re: [PATCH v1 16/20] x86: Allow LAPIC-only emulation_flags for HVM guests

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 03/08/16 17:11, Jan Beulich wrote:
>>>> On 05.07.16 at 21:05, <boris.ostrovsky@oracle.com> wrote:
>> --- a/xen/arch/x86/domain.c
>> +++ b/xen/arch/x86/domain.c
>> @@ -545,25 +545,31 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>>      }
>>      else
>>      {
>> -        if ( (config->emulation_flags & ~XEN_X86_EMU_ALL) != 0 )
>> +        uint32_t emflags;
>> +
>> +        if ( is_hardware_domain(d) )
>> +            config->emulation_flags |= XEN_X86_EMU_PIT;
>> +
>> +        emflags = config->emulation_flags;
>> +        if ( (emflags & ~XEN_X86_EMU_ALL) != 0 )
> In cases like this please consider dropping the pointless != 0.
>
>>          {
>>              printk(XENLOG_G_ERR "d%d: Invalid emulation bitmap: %#x\n",
>> -                   d->domain_id, config->emulation_flags);
>> +                   d->domain_id, emflags);
>>              return -EINVAL;
>>          }
>> -        if ( is_hardware_domain(d) )
>> -            config->emulation_flags |= XEN_X86_EMU_PIT;
>> -        if ( config->emulation_flags != 0 &&
>> -             (config->emulation_flags !=
>> -              (is_hvm_domain(d) ? XEN_X86_EMU_ALL : XEN_X86_EMU_PIT)) )
>> +
>> +        /* PVHv2 guests can request emulated APIC */
> Comment style. With at least this one fixed
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
>> +        if ( emflags &&
>> +            (is_hvm_domain(d) ? ((emflags != XEN_X86_EMU_ALL) &&
>> +                                 (emflags != XEN_X86_EMU_LAPIC)) :
>> +                                (emflags != XEN_X86_EMU_PIT)) )
> I guess sooner or later this will need re-arranging so that it's easier
> to maintain when more variations are permitted for HVM, but for
> now I think it's fine.

Roger already did this at my request on a different thread.

Message: <20160801112343.7ndtre6tfzxk3xjd@mac>, subject "[PATCH RFC
03/12] xen/x86: allow the emulated APICs to be enbled for the hardware
domain"

And on that note, Roger: you have a typo in your patch subject.

~Andrew

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