Re: [v4 04/17] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature

["Wu, Feng" <feng.wu@intel.com>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Thursday, July 23, 2015 10:05 PM
> To: Wu, Feng; xen-devel@lists.xen.org
> Cc: Tian, Kevin; Jan Beulich
> Subject: Re: [Xen-devel] [v4 04/17] iommu: Add iommu_intpost to control VT-d
> Posted-Interrupts feature
> 
> On 23/07/15 15:01, Andrew Cooper wrote:
> > On 23/07/15 12:35, Feng Wu wrote:
> >> VT-d Posted-Interrupts is an enhancement to CPU side Posted-Interrupt.
> >> With VT-d Posted-Interrupts enabled, external interrupts from
> >> direct-assigned devices can be delivered to guests without VMM
> >> intervention when guest is running in non-root mode.
> >>
> >> This patch adds variable 'iommu_intpost' to control whether enable VT-d
> >> posted-interrupt or not in the generic IOMMU code.
> >>
> >> CC: Jan Beulich <jbeulich@suse.com>
> >> CC: Kevin Tian <kevin.tian@intel.com>
> >> Signed-off-by: Feng Wu <feng.wu@intel.com>
> >> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
> > Please patch docs/misc/xen-command-line.markdown as you are
> >
> >> ---
> >> v4:
> >> - No changes
> >>
> >> v3:
> >> - Remove pointless initializer for 'iommu_intpost'.
> >> - Some adjustment for "if no intremap then no intpost" logic.
> >>     * For parse_iommu_param(), move it to the end of the function,
> >>       so we don't need to add the some logic when introduing the
> >>       new kernel parameter 'intpost' in later patch.
> >>     * Add this logic in iommu_setup() after iommu_hardware_setup()
> >>       is called.
> >>
> >>  xen/drivers/passthrough/iommu.c | 17 ++++++++++++++++-
> >>  xen/include/xen/iommu.h         |  2 +-
> >>  2 files changed, 17 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> >> index 06cb38f..536b69f 100644
> >> --- a/xen/drivers/passthrough/iommu.c
> >> +++ b/xen/drivers/passthrough/iommu.c
> >> @@ -39,6 +39,7 @@ static void iommu_dump_p2m_table(unsigned char
> key);
> >>   *   no-snoop                   Disable VT-d Snoop Control
> >>   *   no-qinval                  Disable VT-d Queued Invalidation
> >>   *   no-intremap                Disable VT-d Interrupt Remapping
> >> + *   no-intpost                 Disable VT-d Interrupt posting
> >>   */
> >>  custom_param("iommu", parse_iommu_param);
> >>  bool_t __initdata iommu_enable = 1;
> >> @@ -51,6 +52,14 @@ bool_t __read_mostly iommu_passthrough;
> >>  bool_t __read_mostly iommu_snoop = 1;
> >>  bool_t __read_mostly iommu_qinval = 1;
> >>  bool_t __read_mostly iommu_intremap = 1;
> >> +
> >> +/*
> >> + * In the current implementation of VT-d posted interrupts, in some
> extreme
> >> + * cases, the per cpu list which saves the blocked vCPU will be very long,
> >> + * and this will affect the interrupt latency, so let this feature off by
> >> + * default until we find a good solution to resolve it.
> >> + */
> >> +bool_t __read_mostly iommu_intpost;
> >>  bool_t __read_mostly iommu_hap_pt_share = 1;
> >>  bool_t __read_mostly iommu_debug;
> >>  bool_t __read_mostly amd_iommu_perdev_intremap = 1;
> >> @@ -112,6 +121,9 @@ static void __init parse_iommu_param(char *s)
> >>
> >>          s = ss + 1;
> >>      } while ( ss );
> >> +
> >> +    if ( !iommu_intremap )
> >> +        iommu_intpost = 0;
> > This hunk is still wrong, as I said in v3.
> 
> Furthermore, you appear to have no way in v4 to enable intpost on the
> Xen command line.

Oh, sorry, since this feature is required default off, I forget to send out the last
patch, which add the Xen command line. I will re-add it in the next version.

Thanks,
Feng

> 
> ~Andrew