Re: [Xen-devel] [RFC v1 02/15] vt-d: VT-d Posted-Interrupts feature detection

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Andrew Cooper [mailto:andrew.cooper3@xxxxxxxxxx]
> Sent: Friday, March 27, 2015 2:13 AM
> To: Wu, Feng; xen-devel@xxxxxxxxxxxxx
> Cc: Zhang, Yang Z; Tian, Kevin; keir@xxxxxxx; JBeulich@xxxxxxxx
> Subject: Re: [Xen-devel] [RFC v1 02/15] vt-d: VT-d Posted-Interrupts feature
> detection
> 
> 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 <feng.wu@xxxxxxxxx>
> > ---
> >   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".

It that case, user must check "iommu_intremap && iommu_intpost" together,
my idea in this patchset is the when iommu_intpost == 1 guarantees 
"iommu_intremap == 1",
so we only need to check iommu_intpost later.

> 
> Out of interest, which platforms have intpost capabilities?

Another Broadwell platform, not launched yet.

Thanks,
Feng

> 
> ~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)


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