linux-kernel - Re: [patch 2/2] Intel IOMMU Suspend/Resume Support for Queued Invalidation.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090325202830.GB26204@elte.hu>
Date:	Wed, 25 Mar 2009 21:28:30 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	fenghua.yu@...el.com
Cc:	dwmw2@...radead.org, amluto@...il.com, kyle@...hat.com,
	mgross@...ux.intel.com, linux-kernel@...r.kernel.org,
	iommu@...ts.linux-foundation.org
Subject: Re: [patch 2/2] Intel IOMMU Suspend/Resume Support for Queued
	Invalidation.


(only small style nits.)

* fenghua.yu@...el.com <fenghua.yu@...el.com> wrote:

>  /*
> + * Enable queued invalidation.
> + */
> +static void __dmar_enable_qi(struct intel_iommu *iommu)

nice helper function.

> +{
> +	u32 cmd, sts;
> +	unsigned long flags;
> +	struct q_inval *qi = iommu->qi;
> +
> +	qi->free_head = qi->free_tail = 0;
> +	qi->free_cnt = QI_LENGTH;
> +
> +	spin_lock_irqsave(&iommu->register_lock, flags);
> +	/* write zero to the tail reg */
> +	writel(0, iommu->reg + DMAR_IQT_REG);

( small nit: critical sections are important to see at a glance, and 
  they are easier to recognize if there's a newline before and after 
  the spin_lock_irqsave() call. That will also make the comment 
  after that stand out more. )

> +
> +	dmar_writeq(iommu->reg + DMAR_IQA_REG, virt_to_phys(qi->desc));
> +
> +	cmd = iommu->gcmd | DMA_GCMD_QIE;
> +	iommu->gcmd |= DMA_GCMD_QIE;
> +	writel(cmd, iommu->reg + DMAR_GCMD_REG);
> +
> +	/* Make sure hardware complete it */
> +	IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, readl, (sts & DMA_GSTS_QIES), sts);
> +	spin_unlock_irqrestore(&iommu->register_lock, flags);

(ditto.)

> +}
> +
> +/*
>   * Enable Queued Invalidation interface. This is a must to support
>   * interrupt-remapping. Also used by DMA-remapping, which replaces
>   * register based IOTLB invalidation.
>   */
>  int dmar_enable_qi(struct intel_iommu *iommu)
>  {
> -	u32 cmd, sts;
> -	unsigned long flags;
>  	struct q_inval *qi;
>  
>  	if (!ecap_qis(iommu->ecap))
> @@ -835,19 +860,7 @@ int dmar_enable_qi(struct intel_iommu *iommu)
>  
>  	spin_lock_init(&qi->q_lock);
>  
> -	spin_lock_irqsave(&iommu->register_lock, flags);
> -	/* write zero to the tail reg */
> -	writel(0, iommu->reg + DMAR_IQT_REG);
> -
> -	dmar_writeq(iommu->reg + DMAR_IQA_REG, virt_to_phys(qi->desc));
> -
> -	cmd = iommu->gcmd | DMA_GCMD_QIE;
> -	iommu->gcmd |= DMA_GCMD_QIE;
> -	writel(cmd, iommu->reg + DMAR_GCMD_REG);
> -
> -	/* Make sure hardware complete it */
> -	IOMMU_WAIT_OP(iommu, DMAR_GSTS_REG, readl, (sts & DMA_GSTS_QIES), sts);
> -	spin_unlock_irqrestore(&iommu->register_lock, flags);
> +	__dmar_enable_qi(iommu);
>  
>  	return 0;
>  }
> @@ -1102,3 +1115,27 @@ int __init enable_drhd_fault_handling(void)
>  
>  	return 0;
>  }
> +
> +/*
> + * Re-enable Queued Invalidation interface.
> + */
> +int dmar_reenable_qi(struct intel_iommu *iommu)
> +{
> +	if (!ecap_qis(iommu->ecap))
> +		return -ENOENT;
> +
> +	if (!iommu->qi)
> +		return -ENOENT;

> +
> +	/*
> +	 * First disable queued invalidation.
> +	 */
> +	dmar_disable_qi(iommu);
> +	/* Then enable queued invalidation again. Since there is no pending
> +	 * invalidation requests now, it's safe to re-enable queued
> +	 * invalidation.
> +	 */
> +	__dmar_enable_qi(iommu);

the comment style looks a bit inconsistent here - the second 
one should be full winged comments as well, i.e.:

> +	/*
> +	 * First disable queued invalidation.
> +	 */
> +	dmar_disable_qi(iommu);
> +
> +	/*
> +	 * Then enable queued invalidation again. Since there is no pending
> +	 * invalidation requests now, it's safe to re-enable queued
> +	 * invalidation.
> +	 */
> +	__dmar_enable_qi(iommu);

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/