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

Re: [Xen-devel] [PATCH v3 4/4] x86/PV: enable the emulated PIT

On 18/01/16 09:44, Jan Beulich wrote:
>>>> On 18.01.16 at 10:29, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 18/01/2016 07:43, Jan Beulich wrote:
>>>>>> On 15.01.16 at 18:45, <roger.pau@xxxxxxxxxx> wrote:
>>>> Changes since v2:
>>>>  - Change 'if ( (a && b) || (!a && c) )' into 'if ( a ? b : c )'.
>>> Thanks, but after some more thinking about it I'm afraid there are
>>> a few more aspects to consider here:
>>>> --- a/xen/arch/x86/domain.c
>>>> +++ b/xen/arch/x86/domain.c
>>>> @@ -542,8 +542,9 @@ int arch_domain_create(struct domain *d, unsigned int 
>> domcr_flags,
>>>>                     d->domain_id, config->emulation_flags);
>>>>              return -EINVAL;
>>>>          }
>>>> -        if ( config->emulation_flags != 0 &&
>>>> -             (!is_hvm_domain(d) || config->emulation_flags != 
>>>> XEN_X86_EMU_ALL) 
>> )
>>>> +        if ( is_hvm_domain(d) ? (config->emulation_flags != 
>>>> XEN_X86_EMU_ALL &&
>>>> +             config->emulation_flags != 0) :
>>>> +             (config->emulation_flags != XEN_X86_EMU_PIT) )
>>>>          {
>>> For one I think it would be a good idea to allow zero for PV domains,
>>> and perhaps even default new DomU-s to have the PIT flag clear.
>>> (Also - indentation.)
>>> Which gets us to the second, broader issue: These flags shouldn't
>>> be forced to a particular value during migration, but instead they
>>> should be part of the state getting migrated. Incoming domains
>>> then would - if the field is missing due to coming from an older
>>> hypervisor - have the flag default to 1.
>> There is sadly another ratsnest here.
> I've been afraid of that.
>> These values are needed for domain creation, which means that putting
>> them anywhere in the migration stream is already too late, as the domain
>> has been created before the stream header is read.
> Is that an inherent requirement, or just a result of current code
> structure?

Depends.  As far as libxc/libxl migration levels go, current code structure.

Whatever (eventually) gets used to set these values will however be
present in the xl configuration, which is at the very start of the
stream, and is what is used to create the new domain.

We really don't want the libxc migrate code to be making the
DOMCTL_createdomain hypercall itself; it opens up a whole new attack
surface via cunningly-crafted save image.  The best we can do is have a
sanity check later on.

>  I ask because migrating the emulation flags is going to
> be a requirement for relaxing the current (almost) all-or-nothing
> policy on those flags.
>> In principle, the best which could occur is that a value gets stashed in
>> the stream and used as a sanity check.  That will at least catch the
>> case when they are different.
> That'd be a minimal first step.

This is a substantial quantity of work to do properly.  As the emulation
flags are just one in a very long list of fields handed like this, I
don't think this issue should block the series.


Xen-devel mailing list



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