Yes, this is a bug. How about moving the last
spin_unlock_irqrestore(&iommu->lock, flags);
outside of the if-statement?
Allen
>-----Original Message-----
>From: M.A. Williamson [mailto:maw48@xxxxxxxxxxxxxxxx] On
>Behalf Of Mark Williamson
>Sent: Wednesday, May 30, 2007 6:38 PM
>To: xen-devel@xxxxxxxxxxxxxxxxxxx
>Cc: Kay, Allen M
>Subject: Re: [Xen-devel] [VTD][patch 1/5] HVM device
>assignment using vt-d
>
>On Wednesday 30 May 2007, Kay, Allen M wrote:
>> vtd1.patch:
>> - vt-d specific code
>> - low risk changes in common code
>>
>> Signed-off-by: Allen Kay <allen.m.kay@xxxxxxxxx>
>> Signed-off-by: Xiaohui Xin <xiaohui.xin@xxxxxxxxx>
>
>iommu_set_root_entry can exit with locking. Is this
>unintentional behaviour?
>
>/* iommu handling */
>static int iommu_set_root_entry(struct iommu *iommu)
>{
> void *addr;
> u32 cmd, sts;
> struct root_entry *root;
> unsigned long flags;
>
> if (iommu == NULL)
> gdprintk(XENLOG_ERR VTDPREFIX,
> "iommu_set_root_entry: iommu == NULL\n");
>
> spin_lock_irqsave(&iommu->lock, flags);
>
>MAW: if iommu->root_entry is already set at this point
>
> if (!iommu->root_entry) {
> spin_unlock_irqrestore(&iommu->lock, flags);
> root = (struct root_entry *)alloc_xenheap_page();
> memset((u8*)root, 0, PAGE_SIZE);
> iommu_flush_cache_page(iommu, root);
> spin_lock_irqsave(&iommu->lock, flags);
>
> if (!root && !iommu->root_entry) {
> spin_unlock_irqrestore(&iommu->lock, flags);
> return -ENOMEM;
> }
>
> if (!iommu->root_entry)
> iommu->root_entry = root;
> else /* somebody is fast */
> free_xenheap_page((void *)root);
> spin_unlock_irqrestore(&iommu->lock, flags);
> }
>
>MAW: then we never unlock iommu->lock. In all other cases
>it's released
>before we return.
>
>Cheers,
>Mark
>
>--
>Dave: Just a question. What use is a unicyle with no seat?
>And no pedals!
>Mark: To answer a question with a question: What use is a skateboard?
>Dave: Skateboards have wheels.
>Mark: My wheel has a wheel!
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|