Skip to content

Commit cbf7827

Browse files
niklas88joergroedel
authored andcommitted
iommu/s390: Fix potential s390_domain aperture shrinking
The s390 IOMMU driver currently sets the IOMMU domain's aperture to match the device specific DMA address range of the device that is first attached. This is not ideal. For one if the domain has no device attached in the meantime the aperture could be shrunk allowing translations outside the aperture to exist in the translation tables. Also this is a bit of a misuse of the aperture which really should describe what addresses can be translated and not some device specific limitations. Instead of misusing the aperture like this we can instead create reserved ranges for the ranges inaccessible to the attached devices allowing devices with overlapping ranges to still share an IOMMU domain. This also significantly simplifies s390_iommu_attach_device() allowing us to move the aperture check to the beginning of the function and removing the need to hold the device list's lock to check the aperture. As we then use the same aperture for all domains and it only depends on the table properties we can already check zdev->start_dma/end_dma at probe time and turn the check on attach into a WARN_ON(). Suggested-by: Jason Gunthorpe <[email protected]> Reviewed-by: Matthew Rosato <[email protected]> Reviewed-by: Jason Gunthorpe <[email protected]> Signed-off-by: Niklas Schnelle <[email protected]> Link: https://lore.kernel.org/r/[email protected] Signed-off-by: Joerg Roedel <[email protected]>
1 parent 1a3a7d6 commit cbf7827

File tree

1 file changed

+43
-20
lines changed

1 file changed

+43
-20
lines changed

drivers/iommu/s390-iommu.c

Lines changed: 43 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,9 @@ static struct iommu_domain *s390_domain_alloc(unsigned domain_type)
6262
kfree(s390_domain);
6363
return NULL;
6464
}
65+
s390_domain->domain.geometry.force_aperture = true;
66+
s390_domain->domain.geometry.aperture_start = 0;
67+
s390_domain->domain.geometry.aperture_end = ZPCI_TABLE_SIZE_RT - 1;
6568

6669
spin_lock_init(&s390_domain->dma_table_lock);
6770
spin_lock_init(&s390_domain->list_lock);
@@ -102,11 +105,15 @@ static int s390_iommu_attach_device(struct iommu_domain *domain,
102105
struct s390_domain *s390_domain = to_s390_domain(domain);
103106
struct zpci_dev *zdev = to_zpci_dev(dev);
104107
unsigned long flags;
105-
int cc, rc = 0;
108+
int cc;
106109

107110
if (!zdev)
108111
return -ENODEV;
109112

113+
if (WARN_ON(domain->geometry.aperture_start > zdev->end_dma ||
114+
domain->geometry.aperture_end < zdev->start_dma))
115+
return -EINVAL;
116+
110117
if (zdev->s390_domain)
111118
__s390_iommu_detach_device(zdev);
112119
else if (zdev->dma_table)
@@ -118,30 +125,14 @@ static int s390_iommu_attach_device(struct iommu_domain *domain,
118125
return -EIO;
119126
zdev->dma_table = s390_domain->dma_table;
120127

121-
spin_lock_irqsave(&s390_domain->list_lock, flags);
122-
/* First device defines the DMA range limits */
123-
if (list_empty(&s390_domain->devices)) {
124-
domain->geometry.aperture_start = zdev->start_dma;
125-
domain->geometry.aperture_end = zdev->end_dma;
126-
domain->geometry.force_aperture = true;
127-
/* Allow only devices with identical DMA range limits */
128-
} else if (domain->geometry.aperture_start != zdev->start_dma ||
129-
domain->geometry.aperture_end != zdev->end_dma) {
130-
spin_unlock_irqrestore(&s390_domain->list_lock, flags);
131-
rc = -EINVAL;
132-
goto out_unregister;
133-
}
128+
zdev->dma_table = s390_domain->dma_table;
134129
zdev->s390_domain = s390_domain;
130+
131+
spin_lock_irqsave(&s390_domain->list_lock, flags);
135132
list_add(&zdev->iommu_list, &s390_domain->devices);
136133
spin_unlock_irqrestore(&s390_domain->list_lock, flags);
137134

138135
return 0;
139-
140-
out_unregister:
141-
zpci_unregister_ioat(zdev, 0);
142-
zdev->dma_table = NULL;
143-
144-
return rc;
145136
}
146137

147138
static void s390_iommu_detach_device(struct iommu_domain *domain,
@@ -155,6 +146,30 @@ static void s390_iommu_detach_device(struct iommu_domain *domain,
155146
zpci_dma_init_device(zdev);
156147
}
157148

149+
static void s390_iommu_get_resv_regions(struct device *dev,
150+
struct list_head *list)
151+
{
152+
struct zpci_dev *zdev = to_zpci_dev(dev);
153+
struct iommu_resv_region *region;
154+
155+
if (zdev->start_dma) {
156+
region = iommu_alloc_resv_region(0, zdev->start_dma, 0,
157+
IOMMU_RESV_RESERVED, GFP_KERNEL);
158+
if (!region)
159+
return;
160+
list_add_tail(&region->list, list);
161+
}
162+
163+
if (zdev->end_dma < ZPCI_TABLE_SIZE_RT - 1) {
164+
region = iommu_alloc_resv_region(zdev->end_dma + 1,
165+
ZPCI_TABLE_SIZE_RT - zdev->end_dma - 1,
166+
0, IOMMU_RESV_RESERVED, GFP_KERNEL);
167+
if (!region)
168+
return;
169+
list_add_tail(&region->list, list);
170+
}
171+
}
172+
158173
static struct iommu_device *s390_iommu_probe_device(struct device *dev)
159174
{
160175
struct zpci_dev *zdev;
@@ -164,6 +179,13 @@ static struct iommu_device *s390_iommu_probe_device(struct device *dev)
164179

165180
zdev = to_zpci_dev(dev);
166181

182+
if (zdev->start_dma > zdev->end_dma ||
183+
zdev->start_dma > ZPCI_TABLE_SIZE_RT - 1)
184+
return ERR_PTR(-EINVAL);
185+
186+
if (zdev->end_dma > ZPCI_TABLE_SIZE_RT - 1)
187+
zdev->end_dma = ZPCI_TABLE_SIZE_RT - 1;
188+
167189
return &zdev->iommu_dev;
168190
}
169191

@@ -342,6 +364,7 @@ static const struct iommu_ops s390_iommu_ops = {
342364
.release_device = s390_iommu_release_device,
343365
.device_group = generic_device_group,
344366
.pgsize_bitmap = S390_IOMMU_PGSIZES,
367+
.get_resv_regions = s390_iommu_get_resv_regions,
345368
.default_domain_ops = &(const struct iommu_domain_ops) {
346369
.attach_dev = s390_iommu_attach_device,
347370
.detach_dev = s390_iommu_detach_device,

0 commit comments

Comments
 (0)