From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [RFC v1 02/15] vt-d: VT-d Posted-Interrupts feature detection Date: Thu, 26 Mar 2015 18:12:36 +0000 Message-ID: <55144C14.3030002@citrix.com> References: <1427286717-4093-1-git-send-email-feng.wu@intel.com> <1427286717-4093-3-git-send-email-feng.wu@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1427286717-4093-3-git-send-email-feng.wu@intel.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Feng Wu , xen-devel@lists.xen.org Cc: yang.z.zhang@intel.com, kevin.tian@intel.com, keir@xen.org, JBeulich@suse.com List-Id: xen-devel@lists.xenproject.org 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 feature detection logic for VT-d posted-interrupt. > > Signed-off-by: Feng Wu > --- > xen/drivers/passthrough/vtd/iommu.c | 15 +++++++++++++-- > xen/drivers/passthrough/vtd/iommu.h | 1 + > 2 files changed, 14 insertions(+), 2 deletions(-) > > diff --git a/xen/drivers/passthrough/vtd/iommu.c b/xen/drivers/passthrough/vtd/iommu.c > index 891b9e3..86798a3 100644 > --- a/xen/drivers/passthrough/vtd/iommu.c > +++ b/xen/drivers/passthrough/vtd/iommu.c > @@ -2030,6 +2030,7 @@ static int init_vtd_hw(void) > if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL ) > { > iommu_intremap = 0; > + iommu_intpost = 0; > dprintk(XENLOG_ERR VTDPREFIX, > "ioapic_to_iommu: ioapic %#x (id: %#x) is NULL! " > "Will not try to enable Interrupt Remapping.\n", > @@ -2046,6 +2047,7 @@ static int init_vtd_hw(void) > if ( enable_intremap(iommu, 0) != 0 ) > { > iommu_intremap = 0; > + iommu_intpost = 0; > dprintk(XENLOG_WARNING VTDPREFIX, > "Interrupt Remapping not enabled\n"); > > @@ -2119,8 +2121,8 @@ int __init intel_vtd_setup(void) > } > > /* We enable the following features only if they are supported by all VT-d > - * engines: Snoop Control, DMA passthrough, Queued Invalidation and > - * Interrupt Remapping. > + * engines: Snoop Control, DMA passthrough, Queued Invalidation, Interrupt > + * Remapping, and Posted Interrupt > */ > for_each_drhd_unit ( drhd ) > { > @@ -2146,7 +2148,13 @@ int __init intel_vtd_setup(void) > iommu_qinval = 0; > > if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) ) > + { > iommu_intremap = 0; > + iommu_intpost = 0; > + } > + > + if ( iommu_intpost && !cap_intr_post(iommu->cap)) Missing space inside the outer bracket. I am wondering whether it might be easier, instead of having "iommu_intremap = 0; iommu_intpost = 0" all over the place, to instead insist that one must check "iommu_intremap && iommu_intpost". Out of interest, which platforms have intpost capabilities? ~Andrew > + iommu_intpost = 0; > > if ( !vtd_ept_page_compatible(iommu) ) > iommu_hap_pt_share = 0; > @@ -2164,6 +2172,7 @@ int __init intel_vtd_setup(void) > if ( !iommu_qinval && iommu_intremap ) > { > iommu_intremap = 0; > + iommu_intpost = 0; > dprintk(XENLOG_WARNING VTDPREFIX, "Interrupt Remapping disabled " > "since Queued Invalidation isn't supported or enabled.\n"); > } > @@ -2173,6 +2182,7 @@ int __init intel_vtd_setup(void) > P(iommu_passthrough, "Dom0 DMA Passthrough"); > P(iommu_qinval, "Queued Invalidation"); > P(iommu_intremap, "Interrupt Remapping"); > + P(iommu_intpost, "Posted Interrupt"); > P(iommu_hap_pt_share, "Shared EPT tables"); > #undef P > > @@ -2192,6 +2202,7 @@ int __init intel_vtd_setup(void) > iommu_passthrough = 0; > iommu_qinval = 0; > iommu_intremap = 0; > + iommu_intpost = 0; > return ret; > } > > diff --git a/xen/drivers/passthrough/vtd/iommu.h b/xen/drivers/passthrough/vtd/iommu.h > index d6e6520..42047e0 100644 > --- a/xen/drivers/passthrough/vtd/iommu.h > +++ b/xen/drivers/passthrough/vtd/iommu.h > @@ -69,6 +69,7 @@ > /* > * Decoding Capability Register > */ > +#define cap_intr_post(c) (((c) >> 59) & 1) > #define cap_read_drain(c) (((c) >> 55) & 1) > #define cap_write_drain(c) (((c) >> 54) & 1) > #define cap_max_amask_val(c) (((c) >> 48) & 0x3f)