Re: [Xen-devel] [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature

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

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: Thursday, June 25, 2015 5:06 PM
> To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> Cc: Tian, Kevin; keir@xxxxxxx; george.dunlap@xxxxxxxxxxxxx;
> jbeulich@xxxxxxxx; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v3 04/15] iommu: Add iommu_intpost to control VT-d
> Posted-Interrupts feature
> 
> On 24/06/15 06:18, 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.
> >
> > Signed-off-by: Feng Wu <feng.wu@xxxxxxxxx>
> > ---
> > 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 | 10 +++++++++-
> >  xen/include/xen/iommu.h         |  2 +-
> >  2 files changed, 10 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> > index 06cb38f..597f676 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,7 @@ 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;
> > +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 +114,9 @@ static void __init parse_iommu_param(char *s)
> >
> >          s = ss + 1;
> >      } while ( ss );
> > +
> > +    if ( !iommu_intremap )
> > +        iommu_intpost = 0;
> 
> This check is redundant - It will unconditionally be performed by
> iommu_setup().  I would just drop the hunk.
> 
> However, what you are missing is parsing logic to catch a command line
> configuration such as "iommu=intremap,no-intpost"

Doesn't the above code cover this command line?

Thanks,
Feng

> 
> ~Andrew

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