On Wed, Apr 23, 2025 at 09:15:36AM +0000, CLEMENT MATHIEU--DRIF wrote: > > > On 23/04/2025 8:00 am, Michael S. Tsirkin wrote: > > Caution: External email. Do not open attachments or click links, unless > > this email comes from a known sender and you know the content is safe. > > > > > > On Wed, Apr 23, 2025 at 05:38:20AM +0000, CLEMENT MATHIEU--DRIF wrote: > >> Address space creation might end up being called without holding the > >> bql as it is exposed through the IOMMU ops. > >> > >> Signed-off-by: Clement Mathieu--Drif <clement.mathieu--d...@eviden.com> > >> --- > >> hw/i386/intel_iommu.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c > >> index dffd7ee885..cc8c9857e1 100644 > >> --- a/hw/i386/intel_iommu.c > >> +++ b/hw/i386/intel_iommu.c > >> @@ -4238,6 +4238,12 @@ VTDAddressSpace *vtd_find_add_as(IntelIOMMUState > >> *s, PCIBus *bus, > >> vtd_dev_as->context_cache_entry.context_cache_gen = 0; > >> vtd_dev_as->iova_tree = iova_tree_new(); > >> > >> + /* > >> + * memory_region_add_subregion_overlap requires the bql, > >> + * make sure we own it. > >> + */ > >> + BQL_LOCK_GUARD(); > >> + > >> memory_region_init(&vtd_dev_as->root, OBJECT(s), name, > >> UINT64_MAX); > >> address_space_init(&vtd_dev_as->as, &vtd_dev_as->root, > >> "vtd-root"); > > > > Does not look like this addresses all races here: > > https://lore.kernel.org/all/8062d868-469f-4c1d-a071-099b8e188...@redhat.com > > > > > > while this can be a separate patch on top, I'd rather we just > > address everything in a single patchset. > > Hi Michael, > > We only aim to fix the potential crash here. > I saw Paolo's response and I know the race exists. I will send a patch > set to fix it soon but are you sure both fixes must be in the same > series? I think the nature is different. > > cmd
If you have two races in the same function, fixing one can easily make another one occur more. Let's just fix it all please, I don't see any rush to apply a partial fix. -- MST
