| Message ID | 1487588434-4359-11-git-send-email-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: > The domain builder in libxc no longer depends on leaked CPUID information to > properly construct HVM domains. Remove the control domain exclusion. Am I missing some intermediate step? As long as there's a raw CPUID invocation in xc_cpuid_x86.c (which is still there in staging and I don't recall this series removing it) it at least _feels_ unsafe. Also the change here then results in Dom0 observing different behavior between faulting-capable and faulting-incapable hosts. I'm not convinced this is desirable. Jan
On 22/02/17 09:23, Jan Beulich wrote: >>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >> The domain builder in libxc no longer depends on leaked CPUID information to >> properly construct HVM domains. Remove the control domain exclusion. > Am I missing some intermediate step? As long as there's a raw > CPUID invocation in xc_cpuid_x86.c (which is still there in staging > and I don't recall this series removing it) it at least _feels_ unsafe. Strictly speaking, the domain builder part of this was completed after my xsave adjustments. All the guest-type-dependent information now comes from non-cpuid sources in libxc, or Xen ignores the toolstack values and recalculates information itself. However, until the Intel leaves were complete, dom0 had a hard time booting with this change as there were no toolstack-provided policy and no leakage from hardware. > > Also the change here then results in Dom0 observing different > behavior between faulting-capable and faulting-incapable hosts. > I'm not convinced this is desirable. I disagree. Avoiding the leakage is very desirable moving forwards. Other side effects are that it makes PV and PVH dom0 functionally identical WRT CPUID, and PV userspace (which, unlikely the kernel, tends not to be Xen-aware) sees sensible information. ~Andrew
>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: > On 22/02/17 09:23, Jan Beulich wrote: >>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >>> The domain builder in libxc no longer depends on leaked CPUID information to >>> properly construct HVM domains. Remove the control domain exclusion. >> Am I missing some intermediate step? As long as there's a raw >> CPUID invocation in xc_cpuid_x86.c (which is still there in staging >> and I don't recall this series removing it) it at least _feels_ unsafe. > > Strictly speaking, the domain builder part of this was completed after > my xsave adjustments. All the guest-type-dependent information now > comes from non-cpuid sources in libxc, or Xen ignores the toolstack > values and recalculates information itself. > > However, until the Intel leaves were complete, dom0 had a hard time > booting with this change as there were no toolstack-provided policy and > no leakage from hardware. So what are the CPUID uses in libxc then needed for at this point? Could they be removed in a prereq patch to make clear all needed information is now being obtained via hypercalls? >> Also the change here then results in Dom0 observing different >> behavior between faulting-capable and faulting-incapable hosts. >> I'm not convinced this is desirable. > > I disagree. Avoiding the leakage is very desirable moving forwards. > > Other side effects are that it makes PV and PVH dom0 functionally > identical WRT CPUID, and PV userspace (which, unlikely the kernel, tends > not to be Xen-aware) sees sensible information. I can see the upsides too, hence the "I'm not convinced" ... Jan
On 22/02/17 10:10, Jan Beulich wrote: >>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: >> On 22/02/17 09:23, Jan Beulich wrote: >>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >>>> The domain builder in libxc no longer depends on leaked CPUID information to >>>> properly construct HVM domains. Remove the control domain exclusion. >>> Am I missing some intermediate step? As long as there's a raw >>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging >>> and I don't recall this series removing it) it at least _feels_ unsafe. >> Strictly speaking, the domain builder part of this was completed after >> my xsave adjustments. All the guest-type-dependent information now >> comes from non-cpuid sources in libxc, or Xen ignores the toolstack >> values and recalculates information itself. >> >> However, until the Intel leaves were complete, dom0 had a hard time >> booting with this change as there were no toolstack-provided policy and >> no leakage from hardware. > So what are the CPUID uses in libxc then needed for at this point? > Could they be removed in a prereq patch to make clear all needed > information is now being obtained via hypercalls? I'd prefer to defer that work. The next chunk of CPUID work is going to be redesigning and reimplementing the hypervisor/libxc interface, and all cpuid() calls in libxc will fall out there, but its not a trivial set of changes to make. > >>> Also the change here then results in Dom0 observing different >>> behavior between faulting-capable and faulting-incapable hosts. >>> I'm not convinced this is desirable. >> I disagree. Avoiding the leakage is very desirable moving forwards. >> >> Other side effects are that it makes PV and PVH dom0 functionally >> identical WRT CPUID, and PV userspace (which, unlikely the kernel, tends >> not to be Xen-aware) sees sensible information. > I can see the upsides too, hence the "I'm not convinced" ... So is that an ack or a nack? I am afraid that this isn't very helpful. ~Andrew
>>> On 27.02.17 at 16:10, <andrew.cooper3@citrix.com> wrote: > On 22/02/17 10:10, Jan Beulich wrote: >>>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: >>> On 22/02/17 09:23, Jan Beulich wrote: >>>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >>>>> The domain builder in libxc no longer depends on leaked CPUID information to >>>>> properly construct HVM domains. Remove the control domain exclusion. >>>> Am I missing some intermediate step? As long as there's a raw >>>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging >>>> and I don't recall this series removing it) it at least _feels_ unsafe. >>> Strictly speaking, the domain builder part of this was completed after >>> my xsave adjustments. All the guest-type-dependent information now >>> comes from non-cpuid sources in libxc, or Xen ignores the toolstack >>> values and recalculates information itself. >>> >>> However, until the Intel leaves were complete, dom0 had a hard time >>> booting with this change as there were no toolstack-provided policy and >>> no leakage from hardware. >> So what are the CPUID uses in libxc then needed for at this point? >> Could they be removed in a prereq patch to make clear all needed >> information is now being obtained via hypercalls? > > I'd prefer to defer that work. The next chunk of CPUID work is going to > be redesigning and reimplementing the hypervisor/libxc interface, and > all cpuid() calls in libxc will fall out there, but its not a trivial > set of changes to make. With that, could you live with deferring the patch here until then? I ask because ... >>>> Also the change here then results in Dom0 observing different >>>> behavior between faulting-capable and faulting-incapable hosts. >>>> I'm not convinced this is desirable. >>> I disagree. Avoiding the leakage is very desirable moving forwards. >>> >>> Other side effects are that it makes PV and PVH dom0 functionally >>> identical WRT CPUID, and PV userspace (which, unlikely the kernel, tends >>> not to be Xen-aware) sees sensible information. >> I can see the upsides too, hence the "I'm not convinced" ... > > So is that an ack or a nack? I am afraid that this isn't very helpful. ... I understand this isn't helpful, yet no, at this point its neither an ack nor a nak. Jan
On 28/02/17 09:31, Jan Beulich wrote: >>>> On 27.02.17 at 16:10, <andrew.cooper3@citrix.com> wrote: >> On 22/02/17 10:10, Jan Beulich wrote: >>>>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: >>>> On 22/02/17 09:23, Jan Beulich wrote: >>>>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >>>>>> The domain builder in libxc no longer depends on leaked CPUID information to >>>>>> properly construct HVM domains. Remove the control domain exclusion. >>>>> Am I missing some intermediate step? As long as there's a raw >>>>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging >>>>> and I don't recall this series removing it) it at least _feels_ unsafe. >>>> Strictly speaking, the domain builder part of this was completed after >>>> my xsave adjustments. All the guest-type-dependent information now >>>> comes from non-cpuid sources in libxc, or Xen ignores the toolstack >>>> values and recalculates information itself. >>>> >>>> However, until the Intel leaves were complete, dom0 had a hard time >>>> booting with this change as there were no toolstack-provided policy and >>>> no leakage from hardware. >>> So what are the CPUID uses in libxc then needed for at this point? >>> Could they be removed in a prereq patch to make clear all needed >>> information is now being obtained via hypercalls? >> I'd prefer to defer that work. The next chunk of CPUID work is going to >> be redesigning and reimplementing the hypervisor/libxc interface, and >> all cpuid() calls in libxc will fall out there, but its not a trivial >> set of changes to make. > With that, could you live with deferring the patch here until then? We currently have a lot of dom0 implicit dependencies on leaked CPUID state into PV dom0. With this series, I believe I have identified all leaked dependencies, and I really want to prevent is introducing any new implicit dependences accidentally. ~Andrew
>>> On 10.03.17 at 18:10, <andrew.cooper3@citrix.com> wrote: > On 28/02/17 09:31, Jan Beulich wrote: >>>>> On 27.02.17 at 16:10, <andrew.cooper3@citrix.com> wrote: >>> On 22/02/17 10:10, Jan Beulich wrote: >>>>>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: >>>>> On 22/02/17 09:23, Jan Beulich wrote: >>>>>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >>>>>>> The domain builder in libxc no longer depends on leaked CPUID information to >>>>>>> properly construct HVM domains. Remove the control domain exclusion. >>>>>> Am I missing some intermediate step? As long as there's a raw >>>>>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging >>>>>> and I don't recall this series removing it) it at least _feels_ unsafe. >>>>> Strictly speaking, the domain builder part of this was completed after >>>>> my xsave adjustments. All the guest-type-dependent information now >>>>> comes from non-cpuid sources in libxc, or Xen ignores the toolstack >>>>> values and recalculates information itself. >>>>> >>>>> However, until the Intel leaves were complete, dom0 had a hard time >>>>> booting with this change as there were no toolstack-provided policy and >>>>> no leakage from hardware. >>>> So what are the CPUID uses in libxc then needed for at this point? >>>> Could they be removed in a prereq patch to make clear all needed >>>> information is now being obtained via hypercalls? >>> I'd prefer to defer that work. The next chunk of CPUID work is going to >>> be redesigning and reimplementing the hypervisor/libxc interface, and >>> all cpuid() calls in libxc will fall out there, but its not a trivial >>> set of changes to make. >> With that, could you live with deferring the patch here until then? > > We currently have a lot of dom0 implicit dependencies on leaked CPUID > state into PV dom0. > > With this series, I believe I have identified all leaked dependencies, > and I really want to prevent is introducing any new implicit dependences > accidentally. I can certainly understand this, but the state libxc code is in then makes this a rather implicit thing, instead of being fully explicit. I think I'd like to have another (tools or REST) maintainer voice a 3rd opinion. Extending Cc list ... Jan
On Mon, Mar 13, 2017 at 05:48:44AM -0600, Jan Beulich wrote: > >>> On 10.03.17 at 18:10, <andrew.cooper3@citrix.com> wrote: > > On 28/02/17 09:31, Jan Beulich wrote: > >>>>> On 27.02.17 at 16:10, <andrew.cooper3@citrix.com> wrote: > >>> On 22/02/17 10:10, Jan Beulich wrote: > >>>>>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: > >>>>> On 22/02/17 09:23, Jan Beulich wrote: > >>>>>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: > >>>>>>> The domain builder in libxc no longer depends on leaked CPUID information to > >>>>>>> properly construct HVM domains. Remove the control domain exclusion. > >>>>>> Am I missing some intermediate step? As long as there's a raw > >>>>>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging > >>>>>> and I don't recall this series removing it) it at least _feels_ unsafe. > >>>>> Strictly speaking, the domain builder part of this was completed after > >>>>> my xsave adjustments. All the guest-type-dependent information now > >>>>> comes from non-cpuid sources in libxc, or Xen ignores the toolstack > >>>>> values and recalculates information itself. > >>>>> > >>>>> However, until the Intel leaves were complete, dom0 had a hard time > >>>>> booting with this change as there were no toolstack-provided policy and > >>>>> no leakage from hardware. > >>>> So what are the CPUID uses in libxc then needed for at this point? > >>>> Could they be removed in a prereq patch to make clear all needed > >>>> information is now being obtained via hypercalls? > >>> I'd prefer to defer that work. The next chunk of CPUID work is going to > >>> be redesigning and reimplementing the hypervisor/libxc interface, and > >>> all cpuid() calls in libxc will fall out there, but its not a trivial > >>> set of changes to make. > >> With that, could you live with deferring the patch here until then? > > > > We currently have a lot of dom0 implicit dependencies on leaked CPUID > > state into PV dom0. > > > > With this series, I believe I have identified all leaked dependencies, > > and I really want to prevent is introducing any new implicit dependences > > accidentally. > > I can certainly understand this, but the state libxc code is in then > makes this a rather implicit thing, instead of being fully explicit. I > think I'd like to have another (tools or REST) maintainer voice a 3rd > opinion. Extending Cc list ... > I'm not sure I follow the implicit vs explicit bit. Wei. > Jan >
>>> On 14.03.17 at 16:06, <wei.liu2@citrix.com> wrote: > On Mon, Mar 13, 2017 at 05:48:44AM -0600, Jan Beulich wrote: >> >>> On 10.03.17 at 18:10, <andrew.cooper3@citrix.com> wrote: >> > On 28/02/17 09:31, Jan Beulich wrote: >> >>>>> On 27.02.17 at 16:10, <andrew.cooper3@citrix.com> wrote: >> >>> On 22/02/17 10:10, Jan Beulich wrote: >> >>>>>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: >> >>>>> On 22/02/17 09:23, Jan Beulich wrote: >> >>>>>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: >> >>>>>>> The domain builder in libxc no longer depends on leaked CPUID information > to >> >>>>>>> properly construct HVM domains. Remove the control domain exclusion. >> >>>>>> Am I missing some intermediate step? As long as there's a raw >> >>>>>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging >> >>>>>> and I don't recall this series removing it) it at least _feels_ unsafe. >> >>>>> Strictly speaking, the domain builder part of this was completed after >> >>>>> my xsave adjustments. All the guest-type-dependent information now >> >>>>> comes from non-cpuid sources in libxc, or Xen ignores the toolstack >> >>>>> values and recalculates information itself. >> >>>>> >> >>>>> However, until the Intel leaves were complete, dom0 had a hard time >> >>>>> booting with this change as there were no toolstack-provided policy and >> >>>>> no leakage from hardware. >> >>>> So what are the CPUID uses in libxc then needed for at this point? >> >>>> Could they be removed in a prereq patch to make clear all needed >> >>>> information is now being obtained via hypercalls? >> >>> I'd prefer to defer that work. The next chunk of CPUID work is going to >> >>> be redesigning and reimplementing the hypervisor/libxc interface, and >> >>> all cpuid() calls in libxc will fall out there, but its not a trivial >> >>> set of changes to make. >> >> With that, could you live with deferring the patch here until then? >> > >> > We currently have a lot of dom0 implicit dependencies on leaked CPUID >> > state into PV dom0. >> > >> > With this series, I believe I have identified all leaked dependencies, >> > and I really want to prevent is introducing any new implicit dependences >> > accidentally. >> >> I can certainly understand this, but the state libxc code is in then >> makes this a rather implicit thing, instead of being fully explicit. I >> think I'd like to have another (tools or REST) maintainer voice a 3rd >> opinion. Extending Cc list ... > > I'm not sure I follow the implicit vs explicit bit. Right now libxc still uses the CPUID instruction, thus implicitly (via CPUID faulting / masking) obtaining the intended (filtered) state of individual feature flags. The intended explicit way would be for it to instead use hypercalls to retrieve the information. Jan
On Tue, Mar 14, 2017 at 09:13:08AM -0600, Jan Beulich wrote: > >>> On 14.03.17 at 16:06, <wei.liu2@citrix.com> wrote: > > On Mon, Mar 13, 2017 at 05:48:44AM -0600, Jan Beulich wrote: > >> >>> On 10.03.17 at 18:10, <andrew.cooper3@citrix.com> wrote: > >> > On 28/02/17 09:31, Jan Beulich wrote: > >> >>>>> On 27.02.17 at 16:10, <andrew.cooper3@citrix.com> wrote: > >> >>> On 22/02/17 10:10, Jan Beulich wrote: > >> >>>>>>> On 22.02.17 at 11:00, <andrew.cooper3@citrix.com> wrote: > >> >>>>> On 22/02/17 09:23, Jan Beulich wrote: > >> >>>>>>>>> On 20.02.17 at 12:00, <andrew.cooper3@citrix.com> wrote: > >> >>>>>>> The domain builder in libxc no longer depends on leaked CPUID information > > to > >> >>>>>>> properly construct HVM domains. Remove the control domain exclusion. > >> >>>>>> Am I missing some intermediate step? As long as there's a raw > >> >>>>>> CPUID invocation in xc_cpuid_x86.c (which is still there in staging > >> >>>>>> and I don't recall this series removing it) it at least _feels_ unsafe. > >> >>>>> Strictly speaking, the domain builder part of this was completed after > >> >>>>> my xsave adjustments. All the guest-type-dependent information now > >> >>>>> comes from non-cpuid sources in libxc, or Xen ignores the toolstack > >> >>>>> values and recalculates information itself. > >> >>>>> > >> >>>>> However, until the Intel leaves were complete, dom0 had a hard time > >> >>>>> booting with this change as there were no toolstack-provided policy and > >> >>>>> no leakage from hardware. > >> >>>> So what are the CPUID uses in libxc then needed for at this point? > >> >>>> Could they be removed in a prereq patch to make clear all needed > >> >>>> information is now being obtained via hypercalls? > >> >>> I'd prefer to defer that work. The next chunk of CPUID work is going to > >> >>> be redesigning and reimplementing the hypervisor/libxc interface, and > >> >>> all cpuid() calls in libxc will fall out there, but its not a trivial > >> >>> set of changes to make. > >> >> With that, could you live with deferring the patch here until then? > >> > > >> > We currently have a lot of dom0 implicit dependencies on leaked CPUID > >> > state into PV dom0. > >> > > >> > With this series, I believe I have identified all leaked dependencies, > >> > and I really want to prevent is introducing any new implicit dependences > >> > accidentally. > >> > >> I can certainly understand this, but the state libxc code is in then > >> makes this a rather implicit thing, instead of being fully explicit. I > >> think I'd like to have another (tools or REST) maintainer voice a 3rd > >> opinion. Extending Cc list ... > > > > I'm not sure I follow the implicit vs explicit bit. > > Right now libxc still uses the CPUID instruction, thus implicitly (via > CPUID faulting / masking) obtaining the intended (filtered) state of > individual feature flags. The intended explicit way would be for it > to instead use hypercalls to retrieve the information. > AFAICT having this patch won't make things worse than before, with the added benefit of avoiding dependencies of leaked information. If my understanding is correct, I'm slightly in favour of committing it now. Wei. > Jan >
diff --git a/xen/arch/x86/cpu/intel.c b/xen/arch/x86/cpu/intel.c index 2e20327..238c47d 100644 --- a/xen/arch/x86/cpu/intel.c +++ b/xen/arch/x86/cpu/intel.c @@ -159,22 +159,10 @@ static void intel_ctxt_switch_levelling(const struct vcpu *next) if (cpu_has_cpuid_faulting) { /* - * We *should* be enabling faulting for the control domain. - * - * Unfortunately, the domain builder (having only ever been a - * PV guest) expects to be able to see host cpuid state in a - * native CPUID instruction, to correctly build a CPUID policy - * for HVM guests (notably the xstate leaves). - * - * This logic is fundimentally broken for HVM toolstack - * domains, and faulting causes PV guests to behave like HVM - * guests from their point of view. - * - * Future development plans will move responsibility for - * generating the maximum full cpuid policy into Xen, at which - * this problem will disappear. + * Always enable faulting for all PV domains. Enable faulting + * for HVM guests if they have configured it. */ - set_cpuid_faulting(nextd && !is_control_domain(nextd) && + set_cpuid_faulting(nextd && (is_pv_domain(nextd) || next->arch.cpuid_faulting)); return;
The domain builder in libxc no longer depends on leaked CPUID information to properly construct HVM domains. Remove the control domain exclusion. On capable hardware, this prevents all unintended leakage of hardware CPUID values into the control domain, and brings the hypervisor leaves into view. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> --- xen/arch/x86/cpu/intel.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-)