Requesting for freeze exception for VT-d posted-interrupts

Requesting for freeze exception for VT-d posted-interrupts

[Wu, Feng]

Hi maintainers,

We would like to request an extension for freeze exception for VT-d posted-interrupts patch-set.

1. clarify the state of patch series / feature.
[v3 01/15] Vt-d Posted-interrupt (PI) design
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

[v3 02/15] Add helper macro for X86_FEATURE_CX16 feature detection
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

[v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

[v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>

[v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
Acked-by: Kevin Tian <kevin.tian@intel.com>

[v3 09/15] vt-d: Extend struct iremap_entry to support VT-d Posted-Interrupts
Acked-by: Kevin Tian <kevin.tian@intel.com>

[v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used
Acked-by: Kevin Tian <kevin.tian@intel.com>

[v3 13/15] vmx: Properly handle notification event when vCPU is running
Acked-by: Kevin Tian <kevin.tian@intel.com>

[v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling
Acked-by: Kevin Tian <kevin.tian@intel.com>

[v3 15/15] Add a command line parameter for VT-d posted-interrupts
Reviewed-by: Kevin Tian <kevin.tian@intel.com>

2. explain why it needs to be in this release (benefits).
VT-d posted-interrupts is an important interrupt virtualization feature for
device pass-through, the running guest can handle external interrupts
in non-root mode, hence it can eliminate the VM-Exits caused by external
interrupts. Please refer to the design doc:
http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03691.html

>From our experimental environment, after using VT-d posted-interrupts, we
measured 25% improvement in transaction rate netperf TCP_RR benchmark
and 28% reduction in host CPU utilization when using assigned devices.
(10G NIC in my test).

3. explain why it doesn't break things (risks).
This feature only exists in Broadwell Server platform, it has no effect on the
current hardware.

4. CC relevant maintainers and release manager.
Done

There are two main outstanding issues so far:
1. Jan's security concern. I have proposed some solutions but Jan still has
some problems with my proposals. It would be great if Jan can give a clear
proposal so that we can discuss and keep making progress.
2. Scheduler issue: there are conflicts among maintainers Jan/George/Dario.
I would agree with Jan's suggestion below:

" Doing this in a central place is certainly the right approach, but
adding an arch hook that needs to be called everywhere
vcpu_runstate_change() wouldn't serve that purpose. Instead
we'd need to replace all current vcpu_runstate_change() calls
with calls to a new function calling both this and the to be added
arch hook."

However, if different maintainers still hold different opinions, I would appreciate
it if maintainers can reach consensus among themselves so that we can keep
making progress

Thanks,
Feng

Re: Requesting for freeze exception for VT-d posted-interrupts

[Jan Beulich]

>>> On 13.07.15 at 08:55, <feng.wu@intel.com> wrote:
> There are two main outstanding issues so far:
> 1. Jan's security concern. I have proposed some solutions but Jan still has
> some problems with my proposals. It would be great if Jan can give a clear
> proposal so that we can discuss and keep making progress.

My proposal was quite clear: The functionality remains experimental,
default off until you can come up with a satisfactory model here.
Giving the impression that I'm the one to propose a model is simply
inadequate: You want the functionality in, so it's primarily you who
should find an implementation that's free of (latent) security issues.
While in general other maintainers may help with this, implying that
if they can't suggest a suitable model code with recognized potential
for security problems can go in _and_ become supported is wrong.

Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Mon, Jul 13, 2015 at 06:55:30AM +0000, Wu, Feng wrote:
> Hi maintainers,
> 
> We would like to request an extension for freeze exception for VT-d posted-interrupts patch-set.
> 
> 1. clarify the state of patch series / feature.
> [v3 01/15] Vt-d Posted-interrupt (PI) design
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 02/15] Add helper macro for X86_FEATURE_CX16 feature detection
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 09/15] vt-d: Extend struct iremap_entry to support VT-d Posted-Interrupts
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 13/15] vmx: Properly handle notification event when vCPU is running
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling
> Acked-by: Kevin Tian <kevin.tian@intel.com>
> 
> [v3 15/15] Add a command line parameter for VT-d posted-interrupts
> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> 
> 2. explain why it needs to be in this release (benefits).
> VT-d posted-interrupts is an important interrupt virtualization feature for
> device pass-through, the running guest can handle external interrupts
> in non-root mode, hence it can eliminate the VM-Exits caused by external
> interrupts. Please refer to the design doc:
> http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03691.html
> 
> >From our experimental environment, after using VT-d posted-interrupts, we
> measured 25% improvement in transaction rate netperf TCP_RR benchmark
> and 28% reduction in host CPU utilization when using assigned devices.
> (10G NIC in my test).
> 
> 3. explain why it doesn't break things (risks).
> This feature only exists in Broadwell Server platform, it has no effect on the
> current hardware.
> 

You miss the part that how much common code it touches. There is still
risk of breaking VMX and VT-D even if PI is disabled.

> 4. CC relevant maintainers and release manager.
> Done
> 
> There are two main outstanding issues so far:
> 1. Jan's security concern. I have proposed some solutions but Jan still has
> some problems with my proposals. It would be great if Jan can give a clear
> proposal so that we can discuss and keep making progress.
> 2. Scheduler issue: there are conflicts among maintainers Jan/George/Dario.
> I would agree with Jan's suggestion below:
> 
> " Doing this in a central place is certainly the right approach, but
> adding an arch hook that needs to be called everywhere
> vcpu_runstate_change() wouldn't serve that purpose. Instead
> we'd need to replace all current vcpu_runstate_change() calls
> with calls to a new function calling both this and the to be added
> arch hook."
> 

Given the current time scale now, I think it would be very hard to get
these two concerns addressed within a week. Xen has always taken
security serious, I don't want to rush in a feature with possible flawed
design.

My answer to this request is no until these concerns are addressed.

> However, if different maintainers still hold different opinions, I would appreciate
> it if maintainers can reach consensus among themselves so that we can keep
> making progress
> 

Yes, this is fore sure. This is what we need to do to work as a
community whether this feature is aimed for 4.6 or not.

Wei.

> Thanks,
> Feng

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wu, Feng]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: Monday, July 13, 2015 7:01 PM
> To: Wu, Feng
> Cc: wei.liu2@citrix.com; xen-devel@lists.xen.org; Jan Beulich
> (JBeulich@suse.com); andrew.cooper3@citrix.com; George Dunlap; Tian, Kevin;
> Zhang, Yang Z; Wang, Yong Y
> Subject: Re: Requesting for freeze exception for VT-d posted-interrupts
> 
> On Mon, Jul 13, 2015 at 06:55:30AM +0000, Wu, Feng wrote:
> > Hi maintainers,
> >
> > We would like to request an extension for freeze exception for VT-d
> posted-interrupts patch-set.
> >
> > 1. clarify the state of patch series / feature.
> > [v3 01/15] Vt-d Posted-interrupt (PI) design
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 02/15] Add helper macro for X86_FEATURE_CX16 feature detection
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >
> > [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts
> feature
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts
> > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 09/15] vt-d: Extend struct iremap_entry to support VT-d
> Posted-Interrupts
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 13/15] vmx: Properly handle notification event when vCPU is running
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling
> > Acked-by: Kevin Tian <kevin.tian@intel.com>
> >
> > [v3 15/15] Add a command line parameter for VT-d posted-interrupts
> > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> >
> > 2. explain why it needs to be in this release (benefits).
> > VT-d posted-interrupts is an important interrupt virtualization feature for
> > device pass-through, the running guest can handle external interrupts
> > in non-root mode, hence it can eliminate the VM-Exits caused by external
> > interrupts. Please refer to the design doc:
> > http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03691.html
> >
> > >From our experimental environment, after using VT-d posted-interrupts, we
> > measured 25% improvement in transaction rate netperf TCP_RR benchmark
> > and 28% reduction in host CPU utilization when using assigned devices.
> > (10G NIC in my test).
> >
> > 3. explain why it doesn't break things (risks).
> > This feature only exists in Broadwell Server platform, it has no effect on the
> > current hardware.
> >
> 
> You miss the part that how much common code it touches. There is still
> risk of breaking VMX and VT-D even if PI is disabled.
> 
> > 4. CC relevant maintainers and release manager.
> > Done
> >
> > There are two main outstanding issues so far:
> > 1. Jan's security concern. I have proposed some solutions but Jan still has
> > some problems with my proposals. It would be great if Jan can give a clear
> > proposal so that we can discuss and keep making progress.
> > 2. Scheduler issue: there are conflicts among maintainers Jan/George/Dario.
> > I would agree with Jan's suggestion below:
> >
> > " Doing this in a central place is certainly the right approach, but
> > adding an arch hook that needs to be called everywhere
> > vcpu_runstate_change() wouldn't serve that purpose. Instead
> > we'd need to replace all current vcpu_runstate_change() calls
> > with calls to a new function calling both this and the to be added
> > arch hook."
> >
> 
> Given the current time scale now, I think it would be very hard to get
> these two concerns addressed within a week. Xen has always taken
> security serious, I don't want to rush in a feature with possible flawed
> design.
> 
> My answer to this request is no until these concerns are addressed.

Is it possible to get to 4.6 if making this feature default off?

Thanks,
Feng

> 
> > However, if different maintainers still hold different opinions, I would
> appreciate
> > it if maintainers can reach consensus among themselves so that we can keep
> > making progress
> >
> 
> Yes, this is fore sure. This is what we need to do to work as a
> community whether this feature is aimed for 4.6 or not.
> 
> Wei.
> 
> > Thanks,
> > Feng

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
> 
> 
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: Monday, July 13, 2015 7:01 PM
> > To: Wu, Feng
> > Cc: wei.liu2@citrix.com; xen-devel@lists.xen.org; Jan Beulich
> > (JBeulich@suse.com); andrew.cooper3@citrix.com; George Dunlap; Tian, Kevin;
> > Zhang, Yang Z; Wang, Yong Y
> > Subject: Re: Requesting for freeze exception for VT-d posted-interrupts
> > 
> > On Mon, Jul 13, 2015 at 06:55:30AM +0000, Wu, Feng wrote:
> > > Hi maintainers,
> > >
> > > We would like to request an extension for freeze exception for VT-d
> > posted-interrupts patch-set.
> > >
> > > 1. clarify the state of patch series / feature.
> > > [v3 01/15] Vt-d Posted-interrupt (PI) design
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 02/15] Add helper macro for X86_FEATURE_CX16 feature detection
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > >
> > > [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts
> > feature
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts
> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> > > Acked-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor
> > > Acked-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 09/15] vt-d: Extend struct iremap_entry to support VT-d
> > Posted-Interrupts
> > > Acked-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used
> > > Acked-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 13/15] vmx: Properly handle notification event when vCPU is running
> > > Acked-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling
> > > Acked-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > [v3 15/15] Add a command line parameter for VT-d posted-interrupts
> > > Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > >
> > > 2. explain why it needs to be in this release (benefits).
> > > VT-d posted-interrupts is an important interrupt virtualization feature for
> > > device pass-through, the running guest can handle external interrupts
> > > in non-root mode, hence it can eliminate the VM-Exits caused by external
> > > interrupts. Please refer to the design doc:
> > > http://lists.xenproject.org/archives/html/xen-devel/2015-06/msg03691.html
> > >
> > > >From our experimental environment, after using VT-d posted-interrupts, we
> > > measured 25% improvement in transaction rate netperf TCP_RR benchmark
> > > and 28% reduction in host CPU utilization when using assigned devices.
> > > (10G NIC in my test).
> > >
> > > 3. explain why it doesn't break things (risks).
> > > This feature only exists in Broadwell Server platform, it has no effect on the
> > > current hardware.
> > >
> > 
> > You miss the part that how much common code it touches. There is still
> > risk of breaking VMX and VT-D even if PI is disabled.
> > 
> > > 4. CC relevant maintainers and release manager.
> > > Done
> > >
> > > There are two main outstanding issues so far:
> > > 1. Jan's security concern. I have proposed some solutions but Jan still has
> > > some problems with my proposals. It would be great if Jan can give a clear
> > > proposal so that we can discuss and keep making progress.
> > > 2. Scheduler issue: there are conflicts among maintainers Jan/George/Dario.
> > > I would agree with Jan's suggestion below:
> > >
> > > " Doing this in a central place is certainly the right approach, but
> > > adding an arch hook that needs to be called everywhere
> > > vcpu_runstate_change() wouldn't serve that purpose. Instead
> > > we'd need to replace all current vcpu_runstate_change() calls
> > > with calls to a new function calling both this and the to be added
> > > arch hook."
> > >
> > 
> > Given the current time scale now, I think it would be very hard to get
> > these two concerns addressed within a week. Xen has always taken
> > security serious, I don't want to rush in a feature with possible flawed
> > design.
> > 
> > My answer to this request is no until these concerns are addressed.
> 
> Is it possible to get to 4.6 if making this feature default off?
> 

Note that I'm not the only one who makes the decision and I can't speak
for maintainers. The first thing you ought to do is to convince
maintainers, not me.

If you ask for my opinion -- I don't see a point in releasing feature
with security flaw in design, even if it is off by default. 

Wei.

Re: Requesting for freeze exception for VT-d posted-interrupts

[Jan Beulich]

>>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
> On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
>> Is it possible to get to 4.6 if making this feature default off?
> 
> Note that I'm not the only one who makes the decision and I can't speak
> for maintainers. The first thing you ought to do is to convince
> maintainers, not me.
> 
> If you ask for my opinion -- I don't see a point in releasing feature
> with security flaw in design, even if it is off by default. 

It was actually me who suggested that by flagging this experimental
and defaulting it to off, chances would increase for this to be allowed
in without said issue fixed.

Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
> >> Is it possible to get to 4.6 if making this feature default off?
> > 
> > Note that I'm not the only one who makes the decision and I can't speak
> > for maintainers. The first thing you ought to do is to convince
> > maintainers, not me.
> > 
> > If you ask for my opinion -- I don't see a point in releasing feature
> > with security flaw in design, even if it is off by default. 
> 
> It was actually me who suggested that by flagging this experimental
> and defaulting it to off, chances would increase for this to be allowed
> in without said issue fixed.
> 

Are you satisfied with that?  Currently I only know from this email
there is concern with regard to security but I don't know what it is and
how big an impact it can possibly have.

I could maybe go dig up that series and try to understand what is the
security implication, but it would take a long time and I'm not sure I
have the right technical background to make the call.

Wei.

> Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Jan Beulich]

>>> On 14.07.15 at 16:17, <wei.liu2@citrix.com> wrote:
> On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
>> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
>> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
>> >> Is it possible to get to 4.6 if making this feature default off?
>> > 
>> > Note that I'm not the only one who makes the decision and I can't speak
>> > for maintainers. The first thing you ought to do is to convince
>> > maintainers, not me.
>> > 
>> > If you ask for my opinion -- I don't see a point in releasing feature
>> > with security flaw in design, even if it is off by default. 
>> 
>> It was actually me who suggested that by flagging this experimental
>> and defaulting it to off, chances would increase for this to be allowed
>> in without said issue fixed.
> 
> Are you satisfied with that?  Currently I only know from this email
> there is concern with regard to security but I don't know what it is and
> how big an impact it can possibly have.
> 
> I could maybe go dig up that series and try to understand what is the
> security implication, but it would take a long time and I'm not sure I
> have the right technical background to make the call.

The thing is that the way vCPU-s are being put on lists attached to
pCPU-s, in a pathological case (which can be "helped" by a malicious
tool stack) all vCPU-s could pile up on one such list. List traversal (in
an interrupt handler) could then take (almost) arbitrarily long.

Otoh one wouldn't expect lists to grow too large on well behaved,
properly operating systems. And hence for the sake of experimenting
with the feature or even using it in known well behaved production
environments it would seem reasonable to allow the feature to be
there.

Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Tue, Jul 14, 2015 at 03:46:46PM +0100, Jan Beulich wrote:
> >>> On 14.07.15 at 16:17, <wei.liu2@citrix.com> wrote:
> > On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
> >> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
> >> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
> >> >> Is it possible to get to 4.6 if making this feature default off?
> >> > 
> >> > Note that I'm not the only one who makes the decision and I can't speak
> >> > for maintainers. The first thing you ought to do is to convince
> >> > maintainers, not me.
> >> > 
> >> > If you ask for my opinion -- I don't see a point in releasing feature
> >> > with security flaw in design, even if it is off by default. 
> >> 
> >> It was actually me who suggested that by flagging this experimental
> >> and defaulting it to off, chances would increase for this to be allowed
> >> in without said issue fixed.
> > 
> > Are you satisfied with that?  Currently I only know from this email
> > there is concern with regard to security but I don't know what it is and
> > how big an impact it can possibly have.
> > 
> > I could maybe go dig up that series and try to understand what is the
> > security implication, but it would take a long time and I'm not sure I
> > have the right technical background to make the call.
> 
> The thing is that the way vCPU-s are being put on lists attached to
> pCPU-s, in a pathological case (which can be "helped" by a malicious
> tool stack) all vCPU-s could pile up on one such list. List traversal (in
> an interrupt handler) could then take (almost) arbitrarily long.

You mentioned "malicious toolstack", does that mean this feature, if on,
doesn't expose new attack vector to malicious guest?

And what do you mean by "malicious toolstack"? I don't see patches
related to toolstack.

> 
> Otoh one wouldn't expect lists to grow too large on well behaved,
> properly operating systems. And hence for the sake of experimenting
> with the feature or even using it in known well behaved production
> environments it would seem reasonable to allow the feature to be
> there.
> 

Right.

Wei.

> Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Jan Beulich]

>>> On 14.07.15 at 17:02, <wei.liu2@citrix.com> wrote:
> On Tue, Jul 14, 2015 at 03:46:46PM +0100, Jan Beulich wrote:
>> >>> On 14.07.15 at 16:17, <wei.liu2@citrix.com> wrote:
>> > On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
>> >> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
>> >> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
>> >> >> Is it possible to get to 4.6 if making this feature default off?
>> >> > 
>> >> > Note that I'm not the only one who makes the decision and I can't speak
>> >> > for maintainers. The first thing you ought to do is to convince
>> >> > maintainers, not me.
>> >> > 
>> >> > If you ask for my opinion -- I don't see a point in releasing feature
>> >> > with security flaw in design, even if it is off by default. 
>> >> 
>> >> It was actually me who suggested that by flagging this experimental
>> >> and defaulting it to off, chances would increase for this to be allowed
>> >> in without said issue fixed.
>> > 
>> > Are you satisfied with that?  Currently I only know from this email
>> > there is concern with regard to security but I don't know what it is and
>> > how big an impact it can possibly have.
>> > 
>> > I could maybe go dig up that series and try to understand what is the
>> > security implication, but it would take a long time and I'm not sure I
>> > have the right technical background to make the call.
>> 
>> The thing is that the way vCPU-s are being put on lists attached to
>> pCPU-s, in a pathological case (which can be "helped" by a malicious
>> tool stack) all vCPU-s could pile up on one such list. List traversal (in
>> an interrupt handler) could then take (almost) arbitrarily long.
> 
> You mentioned "malicious toolstack", does that mean this feature, if on,
> doesn't expose new attack vector to malicious guest?

I think getting a guest to affect this would be more involved, but
I can't entirely exclude it.

> And what do you mean by "malicious toolstack"? I don't see patches
> related to toolstack.

This is because the tool stack can control placement of vCPU-s on
pCPU-s, not because new tool stack code is being added.

Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Tue, Jul 14, 2015 at 05:01:33PM +0100, Jan Beulich wrote:
> >>> On 14.07.15 at 17:02, <wei.liu2@citrix.com> wrote:
> > On Tue, Jul 14, 2015 at 03:46:46PM +0100, Jan Beulich wrote:
> >> >>> On 14.07.15 at 16:17, <wei.liu2@citrix.com> wrote:
> >> > On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
> >> >> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
> >> >> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
> >> >> >> Is it possible to get to 4.6 if making this feature default off?
> >> >> > 
> >> >> > Note that I'm not the only one who makes the decision and I can't speak
> >> >> > for maintainers. The first thing you ought to do is to convince
> >> >> > maintainers, not me.
> >> >> > 
> >> >> > If you ask for my opinion -- I don't see a point in releasing feature
> >> >> > with security flaw in design, even if it is off by default. 
> >> >> 
> >> >> It was actually me who suggested that by flagging this experimental
> >> >> and defaulting it to off, chances would increase for this to be allowed
> >> >> in without said issue fixed.
> >> > 
> >> > Are you satisfied with that?  Currently I only know from this email
> >> > there is concern with regard to security but I don't know what it is and
> >> > how big an impact it can possibly have.
> >> > 
> >> > I could maybe go dig up that series and try to understand what is the
> >> > security implication, but it would take a long time and I'm not sure I
> >> > have the right technical background to make the call.
> >> 
> >> The thing is that the way vCPU-s are being put on lists attached to
> >> pCPU-s, in a pathological case (which can be "helped" by a malicious
> >> tool stack) all vCPU-s could pile up on one such list. List traversal (in
> >> an interrupt handler) could then take (almost) arbitrarily long.
> > 
> > You mentioned "malicious toolstack", does that mean this feature, if on,
> > doesn't expose new attack vector to malicious guest?
> 
> I think getting a guest to affect this would be more involved, but
> I can't entirely exclude it.
> 
> > And what do you mean by "malicious toolstack"? I don't see patches
> > related to toolstack.
> 
> This is because the tool stack can control placement of vCPU-s on
> pCPU-s, not because new tool stack code is being added.
> 

I fished out the thread and tried my best to digest that.

It does seem that it requires tool stack help to pin too many vcpus to a
pcpu to trigger a problem.  Another possibility I can think of is that
the scheduler piling too many vcpus on one pcpu.

All in all, neither of the above two approaches can be used directly by
a malicious guest, so the concern with regard to security is not as big
as I thought.

Jan, I agree with you this feature should be marked as experimental if
we are to merge it for 4.6.

Wu, note the decision has not been made because the patch series is not
in shape yet, in theory you do have time to address all issues by
Friday if you want to try. I will revisit this on Friday.

Wei.

> Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wu, Feng]

> -----Original Message-----
> From: Wei Liu [mailto:wei.liu2@citrix.com]
> Sent: Wednesday, July 15, 2015 11:47 PM
> To: Jan Beulich
> Cc: Wei Liu; andrew.cooper3@citrix.com; George Dunlap; Wu, Feng; Tian, Kevin;
> Zhang, Yang Z; Wang, Yong Y; xen-devel@lists.xen.org
> Subject: Re: Requesting for freeze exception for VT-d posted-interrupts
> 
> On Tue, Jul 14, 2015 at 05:01:33PM +0100, Jan Beulich wrote:
> > >>> On 14.07.15 at 17:02, <wei.liu2@citrix.com> wrote:
> > > On Tue, Jul 14, 2015 at 03:46:46PM +0100, Jan Beulich wrote:
> > >> >>> On 14.07.15 at 16:17, <wei.liu2@citrix.com> wrote:
> > >> > On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
> > >> >> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
> > >> >> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
> > >> >> >> Is it possible to get to 4.6 if making this feature default off?
> > >> >> >
> > >> >> > Note that I'm not the only one who makes the decision and I can't
> speak
> > >> >> > for maintainers. The first thing you ought to do is to convince
> > >> >> > maintainers, not me.
> > >> >> >
> > >> >> > If you ask for my opinion -- I don't see a point in releasing feature
> > >> >> > with security flaw in design, even if it is off by default.
> > >> >>
> > >> >> It was actually me who suggested that by flagging this experimental
> > >> >> and defaulting it to off, chances would increase for this to be allowed
> > >> >> in without said issue fixed.
> > >> >
> > >> > Are you satisfied with that?  Currently I only know from this email
> > >> > there is concern with regard to security but I don't know what it is and
> > >> > how big an impact it can possibly have.
> > >> >
> > >> > I could maybe go dig up that series and try to understand what is the
> > >> > security implication, but it would take a long time and I'm not sure I
> > >> > have the right technical background to make the call.
> > >>
> > >> The thing is that the way vCPU-s are being put on lists attached to
> > >> pCPU-s, in a pathological case (which can be "helped" by a malicious
> > >> tool stack) all vCPU-s could pile up on one such list. List traversal (in
> > >> an interrupt handler) could then take (almost) arbitrarily long.
> > >
> > > You mentioned "malicious toolstack", does that mean this feature, if on,
> > > doesn't expose new attack vector to malicious guest?
> >
> > I think getting a guest to affect this would be more involved, but
> > I can't entirely exclude it.
> >
> > > And what do you mean by "malicious toolstack"? I don't see patches
> > > related to toolstack.
> >
> > This is because the tool stack can control placement of vCPU-s on
> > pCPU-s, not because new tool stack code is being added.
> >
> 
> I fished out the thread and tried my best to digest that.
> 
> It does seem that it requires tool stack help to pin too many vcpus to a
> pcpu to trigger a problem.  Another possibility I can think of is that
> the scheduler piling too many vcpus on one pcpu.
> 
> All in all, neither of the above two approaches can be used directly by
> a malicious guest, so the concern with regard to security is not as big
> as I thought.
> 
> Jan, I agree with you this feature should be marked as experimental if
> we are to merge it for 4.6.
> 
> Wu, note the decision has not been made because the patch series is not
> in shape yet, in theory you do have time to address all issues by
> Friday if you want to try. I will revisit this on Friday.

Wei, Thanks a lot for your analysis, I will try my best to address the issues
before Friday.

Thanks,
Feng

> 
> Wei.
> 
> > Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Wei Liu]

On Wed, Jul 15, 2015 at 10:48:31PM +0000, Wu, Feng wrote:
> 
> 
> > -----Original Message-----
> > From: Wei Liu [mailto:wei.liu2@citrix.com]
> > Sent: Wednesday, July 15, 2015 11:47 PM
> > To: Jan Beulich
> > Cc: Wei Liu; andrew.cooper3@citrix.com; George Dunlap; Wu, Feng; Tian, Kevin;
> > Zhang, Yang Z; Wang, Yong Y; xen-devel@lists.xen.org
> > Subject: Re: Requesting for freeze exception for VT-d posted-interrupts
> > 
> > On Tue, Jul 14, 2015 at 05:01:33PM +0100, Jan Beulich wrote:
> > > >>> On 14.07.15 at 17:02, <wei.liu2@citrix.com> wrote:
> > > > On Tue, Jul 14, 2015 at 03:46:46PM +0100, Jan Beulich wrote:
> > > >> >>> On 14.07.15 at 16:17, <wei.liu2@citrix.com> wrote:
> > > >> > On Tue, Jul 14, 2015 at 11:09:15AM +0100, Jan Beulich wrote:
> > > >> >> >>> On 14.07.15 at 11:21, <wei.liu2@citrix.com> wrote:
> > > >> >> > On Tue, Jul 14, 2015 at 05:51:02AM +0000, Wu, Feng wrote:
> > > >> >> >> Is it possible to get to 4.6 if making this feature default off?
> > > >> >> >
> > > >> >> > Note that I'm not the only one who makes the decision and I can't
> > speak
> > > >> >> > for maintainers. The first thing you ought to do is to convince
> > > >> >> > maintainers, not me.
> > > >> >> >
> > > >> >> > If you ask for my opinion -- I don't see a point in releasing feature
> > > >> >> > with security flaw in design, even if it is off by default.
> > > >> >>
> > > >> >> It was actually me who suggested that by flagging this experimental
> > > >> >> and defaulting it to off, chances would increase for this to be allowed
> > > >> >> in without said issue fixed.
> > > >> >
> > > >> > Are you satisfied with that?  Currently I only know from this email
> > > >> > there is concern with regard to security but I don't know what it is and
> > > >> > how big an impact it can possibly have.
> > > >> >
> > > >> > I could maybe go dig up that series and try to understand what is the
> > > >> > security implication, but it would take a long time and I'm not sure I
> > > >> > have the right technical background to make the call.
> > > >>
> > > >> The thing is that the way vCPU-s are being put on lists attached to
> > > >> pCPU-s, in a pathological case (which can be "helped" by a malicious
> > > >> tool stack) all vCPU-s could pile up on one such list. List traversal (in
> > > >> an interrupt handler) could then take (almost) arbitrarily long.
> > > >
> > > > You mentioned "malicious toolstack", does that mean this feature, if on,
> > > > doesn't expose new attack vector to malicious guest?
> > >
> > > I think getting a guest to affect this would be more involved, but
> > > I can't entirely exclude it.
> > >
> > > > And what do you mean by "malicious toolstack"? I don't see patches
> > > > related to toolstack.
> > >
> > > This is because the tool stack can control placement of vCPU-s on
> > > pCPU-s, not because new tool stack code is being added.
> > >
> > 
> > I fished out the thread and tried my best to digest that.
> > 
> > It does seem that it requires tool stack help to pin too many vcpus to a
> > pcpu to trigger a problem.  Another possibility I can think of is that
> > the scheduler piling too many vcpus on one pcpu.
> > 
> > All in all, neither of the above two approaches can be used directly by
> > a malicious guest, so the concern with regard to security is not as big
> > as I thought.
> > 
> > Jan, I agree with you this feature should be marked as experimental if
> > we are to merge it for 4.6.
> > 
> > Wu, note the decision has not been made because the patch series is not
> > in shape yet, in theory you do have time to address all issues by
> > Friday if you want to try. I will revisit this on Friday.
> 
> Wei, Thanks a lot for your analysis, I will try my best to address the issues
> before Friday.
> 

I haven't seen a new version that has all comments addressed, so I'm
sorry to say this feature is going to miss 4.6.

Do keep up with your good work and make it happen in 4.7.

Wei.

> Thanks,
> Feng
> 
> > 
> > Wei.
> > 
> > > Jan

Re: Requesting for freeze exception for VT-d posted-interrupts

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 3203 bytes --]

On Mon, 2015-07-13 at 06:55 +0000, Wu, Feng wrote:
> There are two main outstanding issues so far:
> 1. Jan's security concern. I have proposed some solutions but Jan still has
> some problems with my proposals. It would be great if Jan can give a clear
> proposal so that we can discuss and keep making progress.
> 2. Scheduler issue: there are conflicts among maintainers Jan/George/Dario.
> I would agree with Jan's suggestion below:
> 
Oh, so there are conflicts, eh? Let's have a look.

I said, basically, that you should change things so that you update your
data structure in the actual places where the events calling for such an
update happen.

I'm not maintaining any code you touch, though. I'm a quite active
developer in scheduling, and plan to continue to be, but let's look at
maintainers.

Jan is one of the maintainer of almost all the code you touch (one of
the few exceptions being scheduling), and he said this that you're
quoting:

> " Doing this in a central place is certainly the right approach, but
> adding an arch hook that needs to be called everywhere
> vcpu_runstate_change() wouldn't serve that purpose. Instead
> we'd need to replace all current vcpu_runstate_change() calls
> with calls to a new function calling both this and the to be added
> arch hook."
> 
but also this, that you may have "forgot":

"But please wait for George's / Dario's feedback, because they
seem to be even less convinced than me about your model of
tying the updates to runstate changes"

Let's look at George, then, which is the formal and official authority
here:

"The right thing to do in this situation is either to change
vcpu_runstate_change() so that it is the central place to make all (or
most) hooks happen; or to add a set of architectural hooks (similar to
the SCHED_OP() hooks) in the various places you need them.

I'm inclined to think that the second is the better option; if for no
other reason that it makes it more clear which states are handled."

For Wei's --and everyone which was not involved in the discussion--
benfit George's "second option" is one (the best I can think of,
actually) way to implement my own suggestion.

So, basically, I and George agree, and Jan says to look at out
feedback... I don't think I see much disagreement, not to mention
'conflicts'! :-O

> However, if different maintainers still hold different opinions, I would appreciate
> it if maintainers can reach consensus among themselves so that we can keep
> making progress
> 
IMO, go implement the 'architectural hooks' George suggested, and
progress is ensured.

Back to the 'freeze exception' matter, for what my opinion is worth, I'm
rather skeptical for the above, and all the changes to the series that
doing it would require, to be ready and fully acked by the time horizon
we're looking at... but it's Wei that is on call, of course. :-)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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