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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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"

~Andrew

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