From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [v3 04/15] iommu: Add iommu_intpost to control VT-d Posted-Interrupts feature Date: Thu, 25 Jun 2015 11:16:06 +0100 Message-ID: <558BD4E6.5040104@citrix.com> References: <1435123109-10481-1-git-send-email-feng.wu@intel.com> <1435123109-10481-5-git-send-email-feng.wu@intel.com> <558BC493.60201@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: "Wu, Feng" , "xen-devel@lists.xen.org" Cc: "george.dunlap@eu.citrix.com" , "Zhang, Yang Z" , "Tian, Kevin" , "keir@xen.org" , "jbeulich@suse.com" List-Id: xen-devel@lists.xenproject.org On 25/06/15 10:47, Wu, Feng wrote: > >> -----Original Message----- >> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com] >> Sent: Thursday, June 25, 2015 5:06 PM >> To: Wu, Feng; xen-devel@lists.xen.org >> Cc: Tian, Kevin; keir@xen.org; george.dunlap@eu.citrix.com; >> jbeulich@suse.com; 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 >>> --- >>> 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? How would you expect it to? I do not see any strcmp( , "intpost") in this patch. ~Andrew