[PATCHv2 4/6] Crashdump-Accepting-Active-IOMMU-Copy-Translations

[Baoquan He]

On 12/20/13 at 02:04pm, Baoquan He wrote:
> On 12/19/13 at 07:49pm, Bill Sumner wrote:
> 
> > +static int copy_page_addr(u64 page_addr, u32 shift, u32 bus, u32 devfn,
> > +				u64 pte, struct dmar_domain *domain,
> > +				void *parms)
> > +{
> > +	struct copy_page_addr_parms *ppap = parms;
> > +
> > +	u64 page_size = ((u64)1 << shift);	/* page_size */
> > +	u64 pfn_lo;				/* For reserving IOVA range */
> > +	u64 pfn_hi;				/* For reserving IOVA range */
> > +	struct iova *iova_p;			/* For reserving IOVA range */
> > +
> > +	if (!ppap) {
> > +		pr_err("ERROR: ppap is NULL: 0x%3.3x(%3.3d) DevFn: 0x%3.3x(%3.3d) Page: 0x%16.16llx Size: 0x%16.16llx(%lld)\n",
> > +			bus, bus, devfn, devfn,  page_addr,
> > +			page_size, page_size);
> > +		return 0;
> > +	}
> > +
> 
> > +	/* Prepare for a new page */
> > +	ppap->first     = 0;		/* Not first-time anymore */
> > +	ppap->bus       = bus;
> > +	ppap->devfn     = devfn;
> > +	ppap->shift     = shift;
> > +	ppap->pte       = pte;
> > +	ppap->next_addr = page_addr + page_size; /* Next-expected page_addr */
> > +
> > +	ppap->page_addr = page_addr;	/* Addr(new page) */
> > +	ppap->page_size = page_size;	/* Size(new page) */
> > +
> > +	ppap->domain    = domain;	/* adr(domain for the new range) */
> 
> Here I am confused, ppap is used to collect the copied ranges and
> necessary information. To my understanding, this domain is the
> dmar_domain which the 1st device is on. When ppat->last is set to 1,
> among the whole collected range, there may be many domains. I just feel
> not good for this. Is it OK to define a specific lock for ppap
> structure, possibly? Please correct me if I am wrong.

Well, check it again. It's not about the lock. Just all address is
recorded in one dmar_domain.