Re: [Xen-devel] [RFC v1 01/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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;

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