RE: [Xen-devel] [VTD][patch 1/5] HVM device assignment using vt-d

["Kay, Allen M" <allen.m.kay@xxxxxxxxx>]

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