| Message ID | 1452879951-76391-1-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. And then - is all this working as intended for the hwdom != Dom0 case? Jan
On 18/01/2016 07:43, Jan Beulich wrote: >>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. 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. 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. I was planning to do exactly the same for the vcpu count etc. as part of my further cpuid work. However, there is a substantial quantity of development work required to make this function in a rational way. For now, I wouldn't worry too much. It is just one of a very large number of things which should be moved on migrate, but isn't. (Most notably, the cpuid policy.) ~Andrew
>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: > On 18/01/2016 07:43, Jan Beulich wrote: >>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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? 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. Jan
El 18/01/16 a les 8.43, Jan Beulich ha escrit: >>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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.) This sounds fine to me, but IMHO, it should be done in a separate patch. This patch just restores previous behaviour for PV guests, then we can move on from there. > And then - is all this working as intended for the hwdom != Dom0 > case? I have to admit I have not tried it, but AFAICT in the hwdom != Dom0 case the set of enabled emulated devices should be the same as a normal guest, the hardware domain doesn't get any more or less emulated devices than any other guest ATM. Roger.
>>> On 18.01.16 at 10:50, <roger.pau@citrix.com> wrote: > El 18/01/16 a les 8.43, Jan Beulich ha escrit: >>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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.) > > This sounds fine to me, but IMHO, it should be done in a separate patch. > This patch just restores previous behaviour for PV guests, then we can > move on from there. Well, maybe. To me it would seem cleaner if both options got permitted by the code right away. >> And then - is all this working as intended for the hwdom != Dom0 >> case? > > I have to admit I have not tried it, but AFAICT in the hwdom != Dom0 > case the set of enabled emulated devices should be the same as a normal > guest, the hardware domain doesn't get any more or less emulated devices > than any other guest ATM. No - see the use of is_hardware_domain() in pv_pit_handler(). IMO the flag should be forced on by the hypervisor for the hardware domain, i.e. things be made independent of whatever tooling gets used to create that domain. Jan
On 18/01/16 09:44, Jan Beulich wrote: >>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: >> On 18/01/2016 07:43, Jan Beulich wrote: >>>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. ~Andrew
>>> On 18.01.16 at 11:41, <andrew.cooper3@citrix.com> wrote: > On 18/01/16 09:44, Jan Beulich wrote: >>>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: >>> On 18/01/2016 07:43, Jan Beulich wrote: >>>>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. I.e. fixable. > 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. Which makes me repeat the question: Is this an inherent property or just "that's the way it is right now"? And then of course the question arises whether setting those flags at domain creation time is the right model. I.e. ... > 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. ... what about deriving the emulation flags from the various pieces of state getting loaded, at least when there are matching pairs (which namely is the case for PIT)? >>> 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. Ugly, but the way things seem to stand it may indeed be unavoidable to ignore the issue for now. Jan
On 18/01/16 11:06, Jan Beulich wrote: >> 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. > I.e. fixable. > >> 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. > Which makes me repeat the question: Is this an inherent property > or just "that's the way it is right now"? And then of course the > question arises whether setting those flags at domain creation time > is the right model. I.e. ... > >> 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. > ... what about deriving the emulation flags from the various > pieces of state getting loaded, at least when there are matching > pairs (which namely is the case for PIT)? How would you suggest setting theses flags up in the plain domain build case then? ~Andrew
>>> On 18.01.16 at 17:10, <andrew.cooper3@citrix.com> wrote: > On 18/01/16 11:06, Jan Beulich wrote: >>> 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. >> Which makes me repeat the question: Is this an inherent property >> or just "that's the way it is right now"? And then of course the >> question arises whether setting those flags at domain creation time >> is the right model. I.e. ... >> >>> 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. >> ... what about deriving the emulation flags from the various >> pieces of state getting loaded, at least when there are matching >> pairs (which namely is the case for PIT)? > > How would you suggest setting theses flags up in the plain domain build > case then? Via a specific (new) hypercall, along the lines of what XEN_DOMCTL_arm_configure_domain was? Jan
On 18/01/16 16:27, Jan Beulich wrote: >>>> On 18.01.16 at 17:10, <andrew.cooper3@citrix.com> wrote: >> On 18/01/16 11:06, Jan Beulich wrote: >>>> 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. >>> Which makes me repeat the question: Is this an inherent property >>> or just "that's the way it is right now"? And then of course the >>> question arises whether setting those flags at domain creation time >>> is the right model. I.e. ... >>> >>>> 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. >>> ... what about deriving the emulation flags from the various >>> pieces of state getting loaded, at least when there are matching >>> pairs (which namely is the case for PIT)? >> How would you suggest setting theses flags up in the plain domain build >> case then? > Via a specific (new) hypercall, along the lines of what > XEN_DOMCTL_arm_configure_domain was? This adds the existing problems we have between the createdomain and max_cpus hypercalls. We need to either specify all information in a single hypercall, or have a dedicated construction phase, during which most hypercalls are invalid to use. (IMO - All domain construction is a rats nest in need of redesigning from scratch.) ~Andrew
>>> On 18.01.16 at 17:33, <andrew.cooper3@citrix.com> wrote: > On 18/01/16 16:27, Jan Beulich wrote: >>>>> On 18.01.16 at 17:10, <andrew.cooper3@citrix.com> wrote: >>> On 18/01/16 11:06, Jan Beulich wrote: >>>>> 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. >>>> Which makes me repeat the question: Is this an inherent property >>>> or just "that's the way it is right now"? And then of course the >>>> question arises whether setting those flags at domain creation time >>>> is the right model. I.e. ... >>>> >>>>> 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. >>>> ... what about deriving the emulation flags from the various >>>> pieces of state getting loaded, at least when there are matching >>>> pairs (which namely is the case for PIT)? >>> How would you suggest setting theses flags up in the plain domain build >>> case then? >> Via a specific (new) hypercall, along the lines of what >> XEN_DOMCTL_arm_configure_domain was? > > This adds the existing problems we have between the createdomain and > max_cpus hypercalls. > > We need to either specify all information in a single hypercall, or have > a dedicated construction phase, during which most hypercalls are invalid > to use. Which is a reasonable requirement to enforce imo. Jan
El 18/01/16 a les 11.41, Andrew Cooper ha escrit: > On 18/01/16 09:44, Jan Beulich wrote: >>>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: >>> On 18/01/2016 07:43, Jan Beulich wrote: >>>>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. You certainly are more familiar with the migration code than me, but wouldn't it be enough to add a new field to libxl_domain_build_info (uint32_t emulation_flags), and teach libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_json how to properly parse it? This however raises the question about how to signal that the field is not initialised, because 0 is a valid value (maybe ~0)? Roger.
On 18/01/16 17:58, Roger Pau Monné wrote: > El 18/01/16 a les 11.41, Andrew Cooper ha escrit: >> On 18/01/16 09:44, Jan Beulich wrote: >>>>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: >>>> On 18/01/2016 07:43, Jan Beulich wrote: >>>>>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. > You certainly are more familiar with the migration code than me, but > wouldn't it be enough to add a new field to libxl_domain_build_info > (uint32_t emulation_flags), and teach > libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_json > how to properly parse it? That would let it be configured from an xl.cfg file, and would normally be moved in the migration stream. However, there is a specific option in xl to restore but using a brand new configuration file. What it doesn't do it check that the settings for the domain in the stream match the settings of the domid being restored into. ~Andrew > > This however raises the question about how to signal that the field is > not initialised, because 0 is a valid value (maybe ~0)? > > Roger. >
On Mon, 2016-01-18 at 18:03 +0000, Andrew Cooper wrote: > On 18/01/16 17:58, Roger Pau Monné wrote: > > El 18/01/16 a les 11.41, Andrew Cooper ha escrit: > > > On 18/01/16 09:44, Jan Beulich wrote: > > > > > > > On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: > > > > > On 18/01/2016 07:43, Jan Beulich wrote: > > > > > > > > > On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. > > You certainly are more familiar with the migration code than me, but > > wouldn't it be enough to add a new field to libxl_domain_build_info > > (uint32_t emulation_flags), and teach > > libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_json > > how to properly parse it? > > That would let it be configured from an xl.cfg file, and would normally > be moved in the migration stream. However, there is a specific option > in xl to restore but using a brand new configuration file. > > What it doesn't do it check that the settings for the domain in the > stream match the settings of the domid being restored into. That would be the responsibility of the user who has chosen to override the configuration in this way. Ian.
On 19/01/16 09:24, Ian Campbell wrote: > On Mon, 2016-01-18 at 18:03 +0000, Andrew Cooper wrote: >> On 18/01/16 17:58, Roger Pau Monné wrote: >>> El 18/01/16 a les 11.41, Andrew Cooper ha escrit: >>>> On 18/01/16 09:44, Jan Beulich wrote: >>>>>>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: >>>>>> On 18/01/2016 07:43, Jan Beulich wrote: >>>>>>>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. >>> You certainly are more familiar with the migration code than me, but >>> wouldn't it be enough to add a new field to libxl_domain_build_info >>> (uint32_t emulation_flags), and teach >>> libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_json >>> how to properly parse it? >> That would let it be configured from an xl.cfg file, and would normally >> be moved in the migration stream. However, there is a specific option >> in xl to restore but using a brand new configuration file. >> >> What it doesn't do it check that the settings for the domain in the >> stream match the settings of the domid being restored into. > That would be the responsibility of the user who has chosen to override the > configuration in this way. It is the responsibility of Xen to ensure there are no exploitable holes due to partial or misconfiguration. In particular, this PIT emulation patch fixes an accidental NULL pointer dereference in Xen, due to the accidental disabling of the PIT in PV guests. ~Andrew
On Tue, 2016-01-19 at 10:09 +0000, Andrew Cooper wrote: > On 19/01/16 09:24, Ian Campbell wrote: > > On Mon, 2016-01-18 at 18:03 +0000, Andrew Cooper wrote: > > > On 18/01/16 17:58, Roger Pau Monné wrote: > > > > El 18/01/16 a les 11.41, Andrew Cooper ha escrit: > > > > > On 18/01/16 09:44, Jan Beulich wrote: > > > > > > > > > On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: > > > > > > > On 18/01/2016 07:43, Jan Beulich wrote: > > > > > > > > > > > On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. > > > > You certainly are more familiar with the migration code than me, > > > > but > > > > wouldn't it be enough to add a new field to libxl_domain_build_info > > > > (uint32_t emulation_flags), and teach > > > > libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_jso > > > > n > > > > how to properly parse it? > > > That would let it be configured from an xl.cfg file, and would > > > normally > > > be moved in the migration stream. However, there is a specific > > > option > > > in xl to restore but using a brand new configuration file. > > > > > > What it doesn't do it check that the settings for the domain in the > > > stream match the settings of the domid being restored into. > > That would be the responsibility of the user who has chosen to override > > the > > configuration in this way. > > It is the responsibility of Xen to ensure there are no exploitable holes > due to partial or misconfiguration. Indeed, but it only needs to check things and fail, not work in the face of a bogus save file + cfg file configuration. Perhaps I misunderstood what was being contended here. Ian. > In particular, this PIT emulation patch fixes an accidental NULL pointer > dereference in Xen, due to the accidental disabling of the PIT in PV > guests. > > ~Andrew
On 19/01/16 10:28, Ian Campbell wrote: > On Tue, 2016-01-19 at 10:09 +0000, Andrew Cooper wrote: >> On 19/01/16 09:24, Ian Campbell wrote: >>> On Mon, 2016-01-18 at 18:03 +0000, Andrew Cooper wrote: >>>> On 18/01/16 17:58, Roger Pau Monné wrote: >>>>> El 18/01/16 a les 11.41, Andrew Cooper ha escrit: >>>>>> On 18/01/16 09:44, Jan Beulich wrote: >>>>>>>>>> On 18.01.16 at 10:29, <andrew.cooper3@citrix.com> wrote: >>>>>>>> On 18/01/2016 07:43, Jan Beulich wrote: >>>>>>>>>>>> On 15.01.16 at 18:45, <roger.pau@citrix.com> 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. >>>>> You certainly are more familiar with the migration code than me, >>>>> but >>>>> wouldn't it be enough to add a new field to libxl_domain_build_info >>>>> (uint32_t emulation_flags), and teach >>>>> libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_jso >>>>> n >>>>> how to properly parse it? >>>> That would let it be configured from an xl.cfg file, and would >>>> normally >>>> be moved in the migration stream. However, there is a specific >>>> option >>>> in xl to restore but using a brand new configuration file. >>>> >>>> What it doesn't do it check that the settings for the domain in the >>>> stream match the settings of the domid being restored into. >>> That would be the responsibility of the user who has chosen to override >>> the >>> configuration in this way. >> It is the responsibility of Xen to ensure there are no exploitable holes >> due to partial or misconfiguration. > Indeed, but it only needs to check things and fail, not work in the face of > a bogus save file + cfg file configuration. Perhaps I misunderstood what > was being contended here. It would appear that the choices are: 1) Rearchitect all domain building/restore from scratch 2) Implement a check & fail properly (Still a large quantity of work, but less than 1) 3) Hack up a check & fail quickly There are a very large number of areas which should be checked on migrate which currently are not. I already have plans to address 2) for the cpuid work. ~Andrew
On Mon, 2016-01-18 at 18:58 +0100, Roger Pau Monné wrote: > You certainly are more familiar with the migration code than me, but > wouldn't it be enough to add a new field to libxl_domain_build_info > (uint32_t emulation_flags), and teach > libxl_domain_build_info_gen_json/libxl__domain_build_info_parse_json > how to properly parse it? Is libxl not already aware whether a domain is to be dmlite vs pv or hvm based on the domain config? As such can't it figure out the hardcoded set to use already without actually needing to expose this in libxl API (until we actually have a desire to expose such nobs to users)? Can't this work on resume just like it does on create? > This however raises the question about how to signal that the field is > not initialised, because 0 is a valid value (maybe ~0)? In libxl's API we tend to try and avoid opaque hex numbers, so the natural result would be a struct full of libxl_defbool options, which IIRC already get properly propagated and have handling for defaulting already in place. Ian. >
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c index 46cfafb..7f47cc5 100644 --- a/tools/libxl/libxl_x86.c +++ b/tools/libxl/libxl_x86.c @@ -13,8 +13,14 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc, LIBXL_DEVICE_MODEL_VERSION_NONE) { /* HVM domains with a device model. */ xc_config->emulation_flags = XEN_X86_EMU_ALL; + } else if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_PV) { + /* PV domains. */ + xc_config->emulation_flags = XEN_X86_EMU_PIT; } else { - /* PV or HVM domains without a device model. */ + assert(d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM); + assert(d_config->b_info.device_model_version == + LIBXL_DEVICE_MODEL_VERSION_NONE); + /* HVMlite domains. */ xc_config->emulation_flags = 0; } diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 159d960..ae85e45 100644 --- 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) ) { printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation " "with the current selection of emulators: %#x\n", diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 76c7b0f..08bd3fb 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -582,7 +582,9 @@ void __init noreturn __start_xen(unsigned long mbi_p) .parity = 'n', .stop_bits = 1 }; - struct xen_arch_domainconfig config = { .emulation_flags = 0 }; + struct xen_arch_domainconfig config = { + .emulation_flags = XEN_X86_EMU_PIT, + }; /* Critical region without IDT or TSS. Any fault is deadly! */