Date:	Thu, 15 Jan 2015 11:28:58 +0800
From:	Baoquan He <bhe@...hat.com>
To:	"Li, Zhen-Hua" <zhen-hual@...com>
Cc:	dwmw2@...radead.org, indou.takao@...fujitsu.com, joro@...tes.org,
	vgoyal@...hat.com, dyoung@...hat.com, jerry.hoemann@...com,
	tom.vaden@...com, rwright@...com, linux-pci@...r.kernel.org,
	kexec@...ts.infradead.org, iommu@...ts.linux-foundation.org,
	lisa.mitchell@...com, linux-kernel@...r.kernel.org,
	alex.williamson@...hat.com, ddutile@...hat.com, doug.hatch@...com,
	ishii.hironobu@...fujitsu.com, bhelgaas@...gle.com,
	billsumnerlinux@...il.com, li.zhang6@...com
Subject: Re: [PATCH v8 06/10] iommu/vt-d: datatypes and functions used for
 kdump

On 01/12/15 at 03:06pm, Li, Zhen-Hua wrote:
> +/*
> + * Interface to the "copy translation tables" set of functions
> + * from mainline code.
> + */
> +static int intel_iommu_load_translation_tables(struct dmar_drhd_unit *drhd,
> +		int g_num_of_iommus)

The argument g_num_of_iommus is the same as the global variable, it's better
to rename it as num_of_iommus. And even it can be removed since you can
just use the global variable g_num_of_iommus in this function.

Argument drhd can be intel_iommu because no other member variable in
drhd is needed.

> +{
> +	struct intel_iommu *iommu;	/* Virt(iommu hardware registers) */
> +	unsigned long long q;		/* quadword scratch */
> +	int ret = 0;			/* Integer return code */
> +	int i = 0;			/* Loop index */
> +	unsigned long flags;
> +
> +	/* Structure so copy_page_addr() can accumulate things
> +	 * over multiple calls and returns
> +	 */
> +	struct copy_page_addr_parms ppa_parms = copy_page_addr_parms_init;
> +	struct copy_page_addr_parms *ppap = &ppa_parms;
> +
> +
> +	iommu = drhd->iommu;
> +	q = dmar_readq(iommu->reg + DMAR_RTADDR_REG);
> +	if (!q)
> +		return -1;
> +
> +	/* If (list needs initializing) do it here */

This initializing should not be here, because it's not only for this
drhd. It should be done in init_dmars().

> +	if (!domain_values_list) {
> +		domain_values_list =
> +			 kcalloc(g_num_of_iommus, sizeof(struct list_head),
> +					GFP_KERNEL);
> +
> +		if (!domain_values_list) {
> +			pr_err("Allocation failed for domain_values_list array\n");
> +			return -ENOMEM;
> +		}
> +		for (i = 0; i < g_num_of_iommus; i++)
> +			INIT_LIST_HEAD(&domain_values_list[i]);
> +	}
> +
> +	spin_lock_irqsave(&iommu->lock, flags);
> +
> +	/* Load the root-entry table from the old kernel
> +	 * foreach context_entry_table in root_entry
> +	 *    foreach context_entry in context_entry_table
> +	 *       foreach level-1 page_table_entry in context_entry
> +	 *          foreach level-2 page_table_entry in level 1 page_table_entry
> +	 *             Above pattern continues up to 6 levels of page tables
> +	 *                Sanity-check the entry
> +	 *                Process the bus, devfn, page_address, page_size
> +	 */
> +	if (!iommu->root_entry) {
> +		iommu->root_entry =
> +			(struct root_entry *)alloc_pgtable_page(iommu->node);
> +		if (!iommu->root_entry) {
> +			spin_unlock_irqrestore(&iommu->lock, flags);
> +			return -ENOMEM;
> +		}
> +	}
> +
> +	iommu->root_entry_old_phys = q & VTD_PAGE_MASK;
> +	if (!iommu->root_entry_old_phys) {
> +		pr_err("Could not read old root entry address.");
> +		return -1;
> +	}
> +
> +	iommu->root_entry_old_virt = ioremap_cache(iommu->root_entry_old_phys,
> +						VTD_PAGE_SIZE);
> +	if (!iommu->root_entry_old_virt) {
> +		pr_err("Could not map the old root entry.");
> +		return -ENOMEM;
> +	}
> +
> +	__iommu_load_old_root_entry(iommu);
> +	ret = copy_root_entry_table(iommu, ppap);
> +	__iommu_flush_cache(iommu, iommu->root_entry, PAGE_SIZE);
> +	__iommu_update_old_root_entry(iommu, -1);
> +
> +	spin_unlock_irqrestore(&iommu->lock, flags);
> +
> +	__iommu_free_mapped_mem();
> +
> +	if (ret)
> +		return ret;
> +
> +	ppa_parms.last = 1;
> +	copy_page_addr(0, 0, 0, 0, 0, NULL, ppap);
> +
> +	return 0;
> +}
> +
>  #endif /* CONFIG_CRASH_DUMP */
> -- 
> 2.0.0-rc0
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
--
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/

Powered by blists - more mailing lists