Re: [Xen-devel] [PATCH V2 6/6] iommu/arm: Add Renesas IPMMU-VMSA support

[Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>]

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