Hi Clement, >-----Original Message----- >From: Duan, Zhenzhong >Subject: RE: [PATCH v1 11/13] intel_iommu_accel: drop _lock suffix in >vtd_flush_host_piotlb_all_locked() > >Hi Clement, > >>-----Original Message----- >>From: CLEMENT MATHIEU--DRIF <[email protected]> >>Subject: Re: [PATCH v1 11/13] intel_iommu_accel: drop _lock suffix in >>vtd_flush_host_piotlb_all_locked() >> >>vtd_piotlb_page_invalidate can be called from >>vtd_pri_perform_implicit_invalidation without any lock. Thus we can call >>vtd_flush_host_piotlb_all_accel unlocked. Is there a risk in case of >>concurrent hot >>plugging of another device? > >Good catch, hotplug is protected by BQL, but if an iothread could send PRI >request, >we suffer from the race. > >I think we can bypass vtd_pri_perform_implicit_invalidation() for passthrough >device >which doesn't cache iotlb entry in QEMU but in host, like: > >@@ -5401,7 +5401,8 @@ static int vtd_pri_request_page(PCIBus *bus, void >*opaque, int devfn, > return -ENOSPC; > } > >- if (vtd_pri_perform_implicit_invalidation(vtd_as, addr)) { >+ if (!vtd_find_hiod_iommufd(vtd_as) && >+ vtd_pri_perform_implicit_invalidation(vtd_as, addr)) { > return -EINVAL; > }
After further thinking, I found above change doesn’t resolve the race you mentioned. But I think the iothread should take BQL before calling ::pri_request_page(), because it is a function changing vtd device context, e.g., PQT and PRS register, page invalidation queue and iotlb entries. We should always take BQL before changing context of any device. Let me know if you have different opinions. Thanks Zhenzhong
