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

[Mark Williamson <mark.williamson@xxxxxxxxxxxx>]

> Yes, this is a bug.  How about moving the last
>
>     spin_unlock_irqrestore(&iommu->lock, flags);
>
> outside of the if-statement?

That looks good to me :-)

Cheers,
Mark

> 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!



-- 
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