All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: Feng Wu <feng.wu@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>, "keir@xen.org" <keir@xen.org>,
	"george.dunlap@eu.citrix.com" <george.dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	"xen-devel@lists.xen.org" <xen-devel@lists.xen.org>,
	Yang Z Zhang <yang.z.zhang@intel.com>
Subject: Re: [v3 12/15] vmx: posted-interrupt handling when vCPU is blocked
Date: Fri, 10 Jul 2015 07:32:28 +0100	[thread overview]
Message-ID: <559F831C020000780008F360@mail.emea.novell.com> (raw)
In-Reply-To: <E959C4978C3B6342920538CF579893F0026021F7@SHSMSX104.ccr.corp.intel.com>

>>> On 10.07.15 at 08:21, <feng.wu@intel.com> wrote:

> 
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Thursday, July 09, 2015 3:26 PM
>> To: Wu, Feng; Tian, Kevin
>> Cc: Andrew Cooper; george.dunlap@eu.citrix.com; Zhang, Yang Z;
>> xen-devel@lists.xen.org; keir@xen.org 
>> Subject: RE: [Xen-devel] [v3 12/15] vmx: posted-interrupt handling when vCPU
>> is blocked
>> 
>> >>> On 09.07.15 at 00:49, <kevin.tian@intel.com> wrote:
>> >>  From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
>> >> Sent: Wednesday, July 08, 2015 9:09 PM
>> >> On 08/07/2015 13:46, Jan Beulich wrote:
>> >> >>>> On 08.07.15 at 13:00, <kevin.tian@intel.com> wrote:
>> >> >>> @@ -1848,6 +1869,33 @@ static struct hvm_function_table __initdata
>> >> >>> vmx_function_table = {
>> >> >>>      .enable_msr_exit_interception =
>> vmx_enable_msr_exit_interception,
>> >> >>>  };
>> >> >>>
>> >> >>> +/*
>> >> >>> + * Handle VT-d posted-interrupt when VCPU is blocked.
>> >> >>> + */
>> >> >>> +static void pi_wakeup_interrupt(struct cpu_user_regs *regs)
>> >> >>> +{
>> >> >>> +    struct arch_vmx_struct *vmx;
>> >> >>> +    unsigned int cpu = smp_processor_id();
>> >> >>> +
>> >> >>> +    spin_lock(&per_cpu(pi_blocked_vcpu_lock, cpu));
>> >> >>> +
>> >> >>> +    /*
>> >> >>> +     * FIXME: The length of the list depends on how many
>> >> >>> +     * vCPU is current blocked on this specific pCPU.
>> >> >>> +     * This may hurt the interrupt latency if the list
>> >> >>> +     * grows to too many entries.
>> >> >>> +     */
>> >> >> let's go with this linked list first until a real issue is identified.
>> >> > This is exactly the way of thinking I dislike when it comes to code
>> >> > that isn't intended to be experimental only: We shouldn't wait
>> >> > for problems to surface when we already can see them. I.e. if
>> >> > there are no plans to deal with this, I'd ask for the feature to be
>> >> > off by default and be properly marked experimental in the
>> >> > command line option documentation (so people know to stay
>> >> > away from it).
>> >>
>> >> And in this specific case, there is no balancing of vcpus across the
>> >> pcpus lists.
>> >>
>> >> One can construct a pathological case using pinning and pausing to get
>> >> almost every vcpu on a single pcpu list, and vcpus recieving fewer
>> >> interrupts will exasperate the problem by staying on the list for longer
>> >> periods of time.
>> >
>> > In that extreme case I believe many contentions in other code paths will
>> > be much larger than overhead caused by this structure limitation.
>> 
>> Examples?
>> 
>> >> IMO, the PI feature cannot be declared as done/supported with this bug
>> >> remaining.  OTOH, it is fine to be experimental, and disabled by default
>> >> for people who wish to experiment.
>> >>
>> >
>> > Again, I don't expect to see it disabled as experimental. For good
>> > production
>> > environment where vcpus are well balanced and interrupt latency is
>> > sensitive,
>> > linked list should be efficient here. For bad environment like extreme case
>> > you raised, I don't know whether it really matters to just tune interrupt
>> > path.
>> 
>> Can you _guarantee_ that everything potentially leading to such a
>> pathological situation is covered by XSA-77? And even if it is now,
>> removing elements from the waiver list would become significantly
>> more difficult if disconnected behavior like this one would need to
>> be taken into account.
>> 
>> Please understand that history has told us to be rather more careful
>> than might seem necessary with this: ATS originally having been
>> enabled by default is one bold example, and the recent flood of MSI
>> related XSAs is another; I suppose I could find more. All affecting
>> code originating from Intel, apparently written with only functionality
>> in mind, while having left out (other than basic) security considerations.
>> 
>> IOW, with my committer role hat on, the feature is going to be
>> experimental (and hence default off) unless the issue here gets
>> addressed. And no, I cannot immediately suggest a good approach,
>> and with all of the rush before the feature freeze I also can't justify
>> taking a lot of time to think of options.
> 
> Is it acceptable to you if I only add the blocked vcpus that has
> assigned devices to the list? I think that should shorten the
> length of the list.

I actually implied this to be the case already, i.e.
- if it's not, this needs to be fixed anyway,
- it's not going to eliminate the concern (just think of a couple of
  many-vCPU guests all having devices assigned).

Jan

  reply	other threads:[~2015-07-10  6:32 UTC|newest]

Thread overview: 155+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-24  5:18 [v3 00/15] Add VT-d Posted-Interrupts support Feng Wu
2015-06-24  5:18 ` [v3 01/15] Vt-d Posted-intterrupt (PI) design Feng Wu
2015-06-24  6:15   ` Meng Xu
2015-06-24  6:19     ` Wu, Feng
2015-07-08  7:21   ` Tian, Kevin
2015-07-08  7:29     ` Wu, Feng
2015-06-24  5:18 ` [v3 02/15] Add helper macro for X86_FEATURE_CX16 feature detection Feng Wu
2015-06-24 17:31   ` Andrew Cooper
2015-07-08  7:23   ` Tian, Kevin
2015-06-24  5:18 ` [v3 03/15] Add cmpxchg16b support for x86-64 Feng Wu
2015-06-24 18:35   ` Andrew Cooper
2015-07-08  7:06     ` Wu, Feng
2015-07-08  8:12       ` Jan Beulich
2015-07-08  8:33         ` Wu, Feng
2015-07-08  8:43           ` Jan Beulich
2015-07-08  8:50             ` Wu, Feng
2015-07-08  8:50         ` Andrew Cooper
2015-07-10 12:57   ` Jan Beulich
2015-06-24  5:18 ` [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature Feng Wu
2015-06-25  9:06   ` Andrew Cooper
2015-06-25  9:47     ` Wu, Feng
2015-06-25 10:16       ` Andrew Cooper
2015-06-25 12:47         ` Wu, Feng
2015-07-08  7:30   ` Tian, Kevin
2015-06-24  5:18 ` [v3 05/15] vt-d: VT-d Posted-Interrupts feature detection Feng Wu
2015-06-25 10:21   ` Andrew Cooper
2015-06-25 13:02     ` Wu, Feng
2015-07-08  7:32   ` Tian, Kevin
2015-07-08  8:00     ` Wu, Feng
2015-06-24  5:18 ` [v3 06/15] vmx: Extend struct pi_desc to support VT-d Posted-Interrupts Feng Wu
2015-06-29 15:04   ` Andrew Cooper
2015-07-08  7:48   ` Tian, Kevin
2015-07-10 13:08   ` Jan Beulich
2015-07-15  2:40     ` Wu, Feng
2015-07-15  8:20       ` Jan Beulich
2015-07-15  8:26         ` Wu, Feng
2015-07-15  8:36           ` Jan Beulich
2015-07-15  8:43             ` Wu, Feng
2015-07-15  9:28               ` Jan Beulich
2015-07-15  9:30                 ` Wu, Feng
2015-07-15  3:13     ` Wu, Feng
2015-06-24  5:18 ` [v3 07/15] vmx: Initialize VT-d Posted-Interrupts Descriptor Feng Wu
2015-06-29 15:32   ` Andrew Cooper
2015-06-30  1:46     ` Wu, Feng
2015-06-30  2:32     ` Dario Faggioli
2015-07-08  7:53   ` Tian, Kevin
2015-06-24  5:18 ` [v3 08/15] Suppress posting interrupts when 'SN' is set Feng Wu
2015-06-29 15:41   ` Andrew Cooper
2015-06-30  1:48     ` Wu, Feng
2015-07-08  9:06   ` Tian, Kevin
2015-07-08 10:11     ` Wu, Feng
2015-07-08 11:31       ` Tian, Kevin
2015-07-08 11:58         ` Wu, Feng
2015-07-10 13:20   ` Jan Beulich
2015-06-24  5:18 ` [v3 09/15] vt-d: Extend struct iremap_entry to support VT-d Posted-Interrupts Feng Wu
2015-06-29 16:04   ` Andrew Cooper
2015-06-30  1:52     ` Wu, Feng
2015-07-08  9:10   ` Tian, Kevin
2015-07-10 13:27   ` Jan Beulich
2015-06-24  5:18 ` [v3 10/15] vt-d: Add API to update IRTE when VT-d PI is used Feng Wu
2015-06-29 16:22   ` Andrew Cooper
2015-07-08  9:59   ` Tian, Kevin
2015-07-08 10:12     ` Wu, Feng
2015-07-10 14:01   ` Jan Beulich
2015-07-15  6:04     ` Wu, Feng
2015-07-15  8:24       ` Jan Beulich
2015-07-15  8:38         ` Wu, Feng
2015-07-15  8:46           ` Jan Beulich
2015-07-15  8:55             ` Wu, Feng
2015-07-15  9:32               ` Jan Beulich
2015-06-24  5:18 ` [v3 11/15] Update IRTE according to guest interrupt config changes Feng Wu
2015-06-29 16:46   ` Andrew Cooper
2015-07-08 10:22   ` Tian, Kevin
2015-07-08 10:31     ` Wu, Feng
2015-07-08 11:46       ` Tian, Kevin
2015-07-08 11:52         ` Wu, Feng
2015-07-08 11:54           ` Tian, Kevin
2015-07-10 14:23   ` Jan Beulich
2015-06-24  5:18 ` [v3 12/15] vmx: posted-interrupt handling when vCPU is blocked Feng Wu
2015-06-29 17:07   ` Andrew Cooper
2015-07-08 10:36     ` Wu, Feng
2015-07-08 10:48       ` Jan Beulich
     [not found]   ` <559181F9.6020106@citrix.com>
2015-06-30  2:51     ` Fwd: " Dario Faggioli
2015-06-30  2:59       ` Wu, Feng
2015-06-30  9:46         ` Dario Faggioli
2015-06-30 10:11   ` Andrew Cooper
2015-07-01 13:26     ` Dario Faggioli
2015-07-02  4:27       ` Wu, Feng
2015-07-02  8:30         ` Dario Faggioli
2015-07-02  8:58           ` Wu, Feng
2015-07-02 10:09             ` Dario Faggioli
2015-07-02 10:41               ` Wu, Feng
2015-07-02 10:30           ` Andrew Cooper
2015-07-02 10:56             ` Wu, Feng
2015-07-02 12:04             ` Dario Faggioli
2015-07-02 12:10               ` Wu, Feng
2015-07-02 12:16               ` Andrew Cooper
2015-07-02 12:38                 ` Dario Faggioli
2015-07-02 12:59                   ` Andrew Cooper
2015-07-03  1:33                     ` Wu, Feng
2015-07-02  4:25     ` Wu, Feng
2015-07-08 11:00   ` Tian, Kevin
2015-07-08 11:02     ` Wu, Feng
2015-07-08 12:46     ` Jan Beulich
2015-07-08 13:09       ` Andrew Cooper
2015-07-08 22:49         ` Tian, Kevin
2015-07-09  7:25           ` Jan Beulich
2015-07-10  6:21             ` Wu, Feng
2015-07-10  6:32               ` Jan Beulich [this message]
2015-07-10  7:29                 ` Wu, Feng
2015-07-10  8:49                   ` Jan Beulich
2015-07-10  8:57                     ` Wu, Feng
2015-07-08 22:31       ` Tian, Kevin
2015-06-24  5:18 ` [v3 13/15] vmx: Properly handle notification event when vCPU is running Feng Wu
2015-07-08 11:03   ` Tian, Kevin
2015-07-10 14:40   ` Jan Beulich
2015-06-24  5:18 ` [v3 14/15] Update Posted-Interrupts Descriptor during vCPU scheduling Feng Wu
     [not found]   ` <55918214.4030102@citrix.com>
2015-06-30  2:58     ` Fwd: " Dario Faggioli
2015-07-02  4:32       ` Wu, Feng
2015-07-02  4:34         ` Wu, Feng
2015-07-02  8:20         ` Dario Faggioli
2015-07-09  3:09           ` Wu, Feng
2015-07-09  8:18             ` Dario Faggioli
2015-07-09 11:19             ` George Dunlap
2015-07-09 11:29               ` George Dunlap
2015-07-09 11:38               ` Wu, Feng
2015-07-09 12:42                 ` Dario Faggioli
2015-07-10  0:07                   ` Wu, Feng
2015-07-10 12:40                     ` Dario Faggioli
2015-07-10 13:47                       ` Konrad Rzeszutek Wilk
2015-07-10 13:59                         ` Dario Faggioli
2015-07-09 12:53                 ` George Dunlap
2015-07-09 13:44                   ` Jan Beulich
2015-07-09 14:18                     ` Dario Faggioli
2015-07-09 14:27                       ` George Dunlap
2015-07-09 14:47                         ` Dario Faggioli
2015-07-10  5:59                         ` Wu, Feng
2015-07-10  6:22                           ` Jan Beulich
2015-07-10 11:05                             ` Dario Faggioli
2015-07-14  5:44                               ` Wu, Feng
2015-07-14 14:08                               ` Wu, Feng
2015-07-14 14:54                                 ` Jan Beulich
2015-07-14 15:20                                   ` Dario Faggioli
2015-07-14 16:41                                     ` George Dunlap
2015-07-14 16:02                                 ` Dario Faggioli
2015-07-15  0:54                                   ` Wu, Feng
2015-07-17  7:46                                   ` Wu, Feng
2015-07-17 10:13                                     ` Dario Faggioli
2015-07-17 22:57                                       ` Wu, Feng
2015-07-18 13:43                                         ` Dario Faggioli
2015-07-10  0:15                   ` Wu, Feng
2015-07-08 11:24   ` Tian, Kevin
2015-07-10 14:48   ` Jan Beulich
2015-06-24  5:18 ` [v3 15/15] Add a command line parameter for VT-d posted-interrupts Feng Wu
2015-07-08 11:25   ` Tian, Kevin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=559F831C020000780008F360@mail.emea.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=feng.wu@intel.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=keir@xen.org \
    --cc=kevin.tian@intel.com \
    --cc=xen-devel@lists.xen.org \
    --cc=yang.z.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.