Hi Oleksandr-san,
> From: Oleksandr, Sent: Thursday, August 8, 2019 1:01 AM
>
>
> Hi, Shimoda-san.
>
> Thank you for the review.
You're welcome.
<snip>
> > +/* Xen IOMMU ops */
> >> +static int __must_check ipmmu_iotlb_flush_all(struct domain *d)
> >> +{
> >> + struct ipmmu_vmsa_xen_domain *xen_domain = dom_iommu(d)->arch.priv;
> >> +
> >> + if ( !xen_domain || !xen_domain->root_domain )
> >> + return 0;
> >> +
> >> + spin_lock(&xen_domain->lock);
> > Is local irq is already disabled here?
> > If no, you should use spin_lock_irqsave() because the ipmmu_irq() also
> > gets the lock.
>
>
> No, it is not disabled. But, ipmmu_irq() uses another mmu->lock. So, I
> think, there won't be a deadlock.
>
> Or I really missed something?
You're correct. I didn't realized that ipmmu_irq() used another mmu->lock.
> If we worry about ipmmu_tlb_invalidate() which is called here (to
> perform a flush by request from P2M code, which manages a page table)
> and from the irq handler (to perform a flush to resume address
> translation), I could use a tasklet to schedule ipmmu_tlb_invalidate()
> from the irq handler then. This way we would get this serialized. What
> do you think?
I just concerned about a dead-lock issue by recursive spin locks.
So, calling ipmmu_tlb_invalidate() here is OK, I think.
Best regards,
Yoshihiro Shimoda
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
