Re: [Xen-devel] [RFC v1 01/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: Friday, March 27, 2015 1:40 AM
> To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> Cc: Zhang, Yang Z; Tian, Kevin; keir@xxxxxxx; JBeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [RFC v1 01/15] iommu: Add iommu_intpost to control
> VT-d Posted-Interrupts feature
> 
> On 25/03/15 12:31, 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>
> > ---
> >   xen/drivers/passthrough/iommu.c | 11 ++++++++++-
> >   xen/include/xen/iommu.h         |  2 +-
> >   2 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/xen/drivers/passthrough/iommu.c
> b/xen/drivers/passthrough/iommu.c
> > index 92ea26f..302e3e4 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 = 0;
> >   bool_t __read_mostly iommu_hap_pt_share = 1;
> >   bool_t __read_mostly iommu_debug;
> >   bool_t __read_mostly amd_iommu_perdev_intremap = 1;
> > @@ -94,7 +96,11 @@ static void __init parse_iommu_param(char *s)
> >           else if ( !strcmp(s, "qinval") )
> >               iommu_qinval = val;
> >           else if ( !strcmp(s, "intremap") )
> > +        {
> >               iommu_intremap = val;
> > +            if ( iommu_intremap == 0 )
> > +                iommu_intpost = 0;
> > +        }
> >           else if ( !strcmp(s, "debug") )
> >           {
> >               iommu_debug = val;
> 
> At no point here do you add an strcmp(s, "intpost"), which means that
> you do not alter the allowable command line syntax.
> 
> intpost must be able to be controlled independently of intremap, so I
> suggest
> 
> else if ( !strcmp(s, "intpost") )
>      iommu_intpost = val;

In this patch, I only define variable 'iommu_qinval' and implement some
logics related to it, while command line 'intpost' is added and checked in
patch [15/15]. So before patch [15/15] these patches don't take effect.

Thanks,
Feng

> 
> and after the while loop,
> 
> if ( !iommu_intremap )
>      iommu_intpost = 0;
> 
> To ensure that intpost is never 1 if intremap is 0.
> 
> Also, you must adjust the documentation in
> docs/misc/xen-command-line.markdown
> 
> ~Andrew
> 
> > @@ -272,7 +278,10 @@ int __init iommu_setup(void)
> >           iommu_enabled = (rc == 0);
> >       }
> >       if ( !iommu_enabled )
> > +    {
> >           iommu_intremap = 0;
> > +        iommu_intpost = 0;
> > +    }
> >
> >       if ( (force_iommu && !iommu_enabled) ||
> >            (force_intremap && !iommu_intremap) )
> > @@ -341,7 +350,7 @@ void iommu_crash_shutdown(void)
> >       const struct iommu_ops *ops = iommu_get_ops();
> >       if ( iommu_enabled )
> >           ops->crash_shutdown();
> > -    iommu_enabled = iommu_intremap = 0;
> > +    iommu_enabled = iommu_intremap = iommu_intpost = 0;
> >   }
> >
> >   bool_t iommu_has_feature(struct domain *d, enum iommu_feature
> feature)
> > diff --git a/xen/include/xen/iommu.h b/xen/include/xen/iommu.h
> > index bf4aff0..91063bb 100644
> > --- a/xen/include/xen/iommu.h
> > +++ b/xen/include/xen/iommu.h
> > @@ -31,7 +31,7 @@
> >   extern bool_t iommu_enable, iommu_enabled;
> >   extern bool_t force_iommu, iommu_verbose;
> >   extern bool_t iommu_workaround_bios_bug, iommu_passthrough;
> > -extern bool_t iommu_snoop, iommu_qinval, iommu_intremap;
> > +extern bool_t iommu_snoop, iommu_qinval, iommu_intremap,
> iommu_intpost;
> >   extern bool_t iommu_hap_pt_share;
> >   extern bool_t iommu_debug;
> >   extern bool_t amd_iommu_perdev_intremap;


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