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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.05.15 at 11:07, <feng.wu@xxxxxxxxx> wrote:
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2044,6 +2044,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",
> @@ -2060,6 +2061,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");
>  

Instead of sprinkling these around, wouldn't it be possible to clear
the variable more centrally upon !iommu_intremap (assuming the
clearing in generic IOMMU code doesn't already suffice)?

> @@ -2160,7 +2162,13 @@ int __init intel_vtd_setup(void)
>              iommu_qinval = 0;
>  
>          if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
> +        {
>              iommu_intremap = 0;
> +            iommu_intpost = 0;
> +        }

Just like in the previous patch, I think this would better be folded
into the next if():

> +        if ( iommu_intpost && !cap_intr_post(iommu->cap) )
> +            iommu_intpost = 0;

Please try to avoid copying odd code: There's no point (afaics) for
the if to check iommu_intpost. Together with the above I think what
you want is

        if ( !iommu_intremap || !cap_intr_post(iommu->cap) )
            iommu_intpost = 0;

Jan


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