Hi Clement,
>-----Original Message-----
>From: CLEMENT MATHIEU--DRIF <clement.mathieu--d...@eviden.com>
>Subject: [PATCH v4 3/3] intel_iommu: Take the VTD lock when looking for and
>creating address spaces
>
>vtd_find_add_as can be called by multiple threads which leads to a race
>condition on address space creation. The IOMMU lock must be taken to
>avoid such a race.
>
>Signed-off-by: Clement Mathieu--Drif <clement.mathieu--d...@eviden.com>
>---
> hw/i386/intel_iommu.c | 28 ++++++++++++++++++++++++++--
> 1 file changed, 26 insertions(+), 2 deletions(-)
>
>diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>index b7855f4b87..931ac01ef0 100644
>--- a/hw/i386/intel_iommu.c
>+++ b/hw/i386/intel_iommu.c
>@@ -4203,11 +4203,15 @@ VTDAddressSpace
>*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
> .pasid = pasid,
> };
> VTDAddressSpace *vtd_dev_as;
>+ struct vtd_as_key *new_key = NULL;
> char name[128];
>
>+ vtd_iommu_lock(s);
> vtd_dev_as = g_hash_table_lookup(s->vtd_address_spaces, &key);
>+ vtd_iommu_unlock(s);
>+
> if (!vtd_dev_as) {
>- struct vtd_as_key *new_key = g_malloc(sizeof(*new_key));
>+ new_key = g_malloc(sizeof(*new_key));
>
> new_key->bus = bus;
> new_key->devfn = devfn;
>@@ -4302,9 +4306,29 @@ VTDAddressSpace
>*vtd_find_add_as(IntelIOMMUState *s, PCIBus *bus,
> &vtd_dev_as->nodmar, 0);
>
> vtd_switch_address_space(vtd_dev_as);
>+ }
>
>- g_hash_table_insert(s->vtd_address_spaces, new_key, vtd_dev_as);
>+ if (new_key != NULL) {
>+ VTDAddressSpace *second_vtd_dev_as;
>+
>+ /*
>+ * Take the lock again and recheck as the AS might have
>+ * been created in the meantime.
>+ */
>+ vtd_iommu_lock(s);
>+
>+ second_vtd_dev_as = g_hash_table_lookup(s->vtd_address_spaces, &key);
>+ if (!second_vtd_dev_as) {
>+ g_hash_table_insert(s->vtd_address_spaces, new_key, vtd_dev_as);
>+ } else {
>+ vtd_dev_as = second_vtd_dev_as;
>+ g_free(vtd_dev_as);
>+ g_free(new_key);
We need to release memory regions under this vtd_dev_as to avoid leak.
Thanks
Zhenzhong
