Redundant code in fn identity_mapping of intel-iommu.c (intel IOMMU driver)?

Rawcoder rawcoder at openmailbox.org
Sun Sep 18 08:57:44 EDT 2016


Hi,

I hope this is the right place for driver code related queries.

I came across some redundant checks in function identity_mapping of file
intel-iommu.c
(https://github.com/torvalds/linux/blob/master/drivers/iommu/intel-iommu.c#L2681).
This function is called by function iommu_no_mapping of same file
(https://github.com/torvalds/linux/blob/master/drivers/iommu/intel-iommu.c#L3452):

	static int iommu_no_mapping(struct device *dev)
	{
		int found;

		if (iommu_dummy(dev))
		/* dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO */
			return 1;

		if (!iommu_identity_mapping)
			return 0;

		found = identity_mapping(dev);	/* this function */
		if (found) {
	// ...

The function iommu_dummy is checking whether the device belongs to the
dummy domain, DUMMY_DEVICE_DOMAIN_INFO.  Followed by that we're checking 
whether identity mapping is set.
After that we come to the identity_mapping function (comments are mine):

	static int identity_mapping(struct device *dev)
	{
		struct device_domain_info *info;

		if (likely(!iommu_identity_mapping))
		/* we just checked this expression and we are here because it
		   was false */
			return 0;

		info = dev->archdata.iommu;
		if (info && info != DUMMY_DEVICE_DOMAIN_INFO)
		/* this is just !iommu_dummy(dev); which we know is the case */
			return (info->domain == si_domain);

		return 0;
	}

If we inline that code we get:

	int found;
	struct device_domain_info *info;

	/* iommu_dummy */
	if (dev->archdata.iommu == DUMMY_DEVICE_DOMAIN_INFO)
		return 1;

	if (!iommu_identity_mapping)
		return 0;

	/* identity_mapping */
	if (likely(!iommu_identity_mapping)) /* redundant? */
		found = 0;

	found = 0;
	info = dev->archdata.iommu;
	if (info && info != DUMMY_DEVICE_DOMAIN_INFO) /* redundant? */
		found = (info->domain == si_domain);


No other function is calling identity_mapping so it couldn't be the case
that there can some other flows which require these checks.

I'm not sure whether this is intentional (doesn't seem to me).  Can anyone
confirm this?

I'm a newbie so I posted here first to check whether I'm not making that
"newbie" mistake.


Thanks and regards,
Abhishek



More information about the Kernelnewbies mailing list