Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()

From: Julien Grall <julien.grall@arm.com>
To: Paul Durrant <paul.durrant@citrix.com>, xen-devel@lists.xenproject.org
Cc: Kevin Tian <kevin.tian@intel.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Wei Liu <wei.liu2@citrix.com>,
	Jun Nakajima <jun.nakajima@intel.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	George Dunlap <George.Dunlap@eu.citrix.com>,
	Andrew Cooper <andrew.cooper3@citrix.com>,
	Ian Jackson <ian.jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
	Tamas K Lengyel <tamas@tklengyel.com>,
	Jan Beulich <jbeulich@suse.com>,
	Brian Woods <brian.woods@amd.com>,
	Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Subject: Re: [PATCH v14 9/9] mm / iommu: split need_iommu() into has_iommu_pt() and need_iommu_pt_sync()
Date: Thu, 4 Oct 2018 18:15:42 +0100	[thread overview]
Message-ID: <b8cb7f69-3144-8c4c-61bc-3fda8efaf0b2@arm.com> (raw)
In-Reply-To: <20181004104547.32316-10-paul.durrant@citrix.com>

Hi Paul,

On 04/10/2018 11:45, Paul Durrant wrote:
> The name 'need_iommu()' is a little confusing as it suggests a domain needs
> to use the IOMMU but something might not be set up yet, when in fact it
> represents a tri-state value (not a boolean as might be expected) where
> -1 means 'IOMMU mappings being set up' and 1 means 'IOMMU mappings have
> been fully set up'.
> 
> Two different meanings are also inferred from the macro it in various
> places in the code:
> 
> - Some callers want to test whether a domain has IOMMU mappings at all
> - Some callers want to test whether they need to synchronize the domain's
>    P2M and IOMMU mappings
> 
> This patch replaces the 'need_iommu' tri-state value with a defined
> enumeration and adds a boolean flag 'need_sync' to separate these meanings,
> and places both of these in struct domain_iommu, rather than directly in
> struct domain.
> This patch also creates two new boolean macros:
> 
> - 'has_iommu_pt()' evaluates to true if a domain has IOMMU mappings, even
>    if they are still under construction.
> - 'need_iommu_pt_sync()' evaluates to true if a domain requires explicit
>    synchronization of the P2M and IOMMU mappings.
> 
> All callers of need_iommu() are then modified to use the macro appropriate
> to what they are trying to test, except for the instance in
> xen/drivers/passthrough/pci.c:assign_device() which has simply been
> removed since it appears to be unnecessary.
> 
> NOTE: There are some callers of need_iommu() that strictly operate on
>        the hardware domain. In some of these case a more global flag is
>        used instead.
> 
> Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

For the Arm bits:

Acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel