Skip to content

Commit ae21f83

Browse files
edumazetbjorn-helgaas
edumazet
authored and
bjorn-helgaas
committed
PCI/P2PDMA: Finish RCU conversion of pdev->p2pdma
While looking at pci_alloc_p2pmem() I found RCU protection was not properly applied there, as pdev->p2pdma was potentially read multiple times. Fix pci_alloc_p2pmem(), add __rcu qualifier to p2pdma field of struct pci_dev, and fix all other accesses to this field with proper RCU verbs. Link: https://lore.kernel.org/r/[email protected] Fixes: 1570175 ("PCI/P2PDMA: track pgmap references per resource, not globally") Signed-off-by: Eric Dumazet <[email protected]> Signed-off-by: Bjorn Helgaas <[email protected]> Reviewed-by: Christoph Hellwig <[email protected]> Reviewed-by: Logan Gunthorpe <[email protected]> Cc: Dan Williams <[email protected]> Cc: Ira Weiny <[email protected]> Cc: Greg Kroah-Hartman <[email protected]> Cc: "Jérôme Glisse" <[email protected]> Cc: "Rafael J. Wysocki" <[email protected]>
1 parent d1b8dc0 commit ae21f83

2 files changed

Lines changed: 73 additions & 26 deletions

File tree

‎drivers/pci/p2pdma.c‎

Lines changed: 72 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,14 @@ static ssize_t size_show(struct device *dev, struct device_attribute *attr,
4848
char *buf)
4949
{
5050
struct pci_dev *pdev = to_pci_dev(dev);
51+
struct pci_p2pdma *p2pdma;
5152
size_t size = 0;
5253

53-
if (pdev->p2pdma->pool)
54-
size = gen_pool_size(pdev->p2pdma->pool);
54+
rcu_read_lock();
55+
p2pdma = rcu_dereference(pdev->p2pdma);
56+
if (p2pdma && p2pdma->pool)
57+
size = gen_pool_size(p2pdma->pool);
58+
rcu_read_unlock();
5559

5660
return scnprintf(buf, PAGE_SIZE, "%zd\n", size);
5761
}
@@ -61,10 +65,14 @@ static ssize_t available_show(struct device *dev, struct device_attribute *attr,
6165
char *buf)
6266
{
6367
struct pci_dev *pdev = to_pci_dev(dev);
68+
struct pci_p2pdma *p2pdma;
6469
size_t avail = 0;
6570

66-
if (pdev->p2pdma->pool)
67-
avail = gen_pool_avail(pdev->p2pdma->pool);
71+
rcu_read_lock();
72+
p2pdma = rcu_dereference(pdev->p2pdma);
73+
if (p2pdma && p2pdma->pool)
74+
avail = gen_pool_avail(p2pdma->pool);
75+
rcu_read_unlock();
6876

6977
return scnprintf(buf, PAGE_SIZE, "%zd\n", avail);
7078
}
@@ -74,9 +82,16 @@ static ssize_t published_show(struct device *dev, struct device_attribute *attr,
7482
char *buf)
7583
{
7684
struct pci_dev *pdev = to_pci_dev(dev);
85+
struct pci_p2pdma *p2pdma;
86+
bool published = false;
87+
88+
rcu_read_lock();
89+
p2pdma = rcu_dereference(pdev->p2pdma);
90+
if (p2pdma)
91+
published = p2pdma->p2pmem_published;
92+
rcu_read_unlock();
7793

78-
return scnprintf(buf, PAGE_SIZE, "%d\n",
79-
pdev->p2pdma->p2pmem_published);
94+
return scnprintf(buf, PAGE_SIZE, "%d\n", published);
8095
}
8196
static DEVICE_ATTR_RO(published);
8297

@@ -95,8 +110,9 @@ static const struct attribute_group p2pmem_group = {
95110
static void pci_p2pdma_release(void *data)
96111
{
97112
struct pci_dev *pdev = data;
98-
struct pci_p2pdma *p2pdma = pdev->p2pdma;
113+
struct pci_p2pdma *p2pdma;
99114

115+
p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
100116
if (!p2pdma)
101117
return;
102118

@@ -128,16 +144,14 @@ static int pci_p2pdma_setup(struct pci_dev *pdev)
128144
if (error)
129145
goto out_pool_destroy;
130146

131-
pdev->p2pdma = p2p;
132-
133147
error = sysfs_create_group(&pdev->dev.kobj, &p2pmem_group);
134148
if (error)
135149
goto out_pool_destroy;
136150

151+
rcu_assign_pointer(pdev->p2pdma, p2p);
137152
return 0;
138153

139154
out_pool_destroy:
140-
pdev->p2pdma = NULL;
141155
gen_pool_destroy(p2p->pool);
142156
out:
143157
devm_kfree(&pdev->dev, p2p);
@@ -159,6 +173,7 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
159173
{
160174
struct pci_p2pdma_pagemap *p2p_pgmap;
161175
struct dev_pagemap *pgmap;
176+
struct pci_p2pdma *p2pdma;
162177
void *addr;
163178
int error;
164179

@@ -200,7 +215,8 @@ int pci_p2pdma_add_resource(struct pci_dev *pdev, int bar, size_t size,
200215
goto pgmap_free;
201216
}
202217

203-
error = gen_pool_add_owner(pdev->p2pdma->pool, (unsigned long)addr,
218+
p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
219+
error = gen_pool_add_owner(p2pdma->pool, (unsigned long)addr,
204220
pci_bus_address(pdev, bar) + offset,
205221
range_len(&pgmap->range), dev_to_node(&pdev->dev),
206222
pgmap->ref);
@@ -437,6 +453,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
437453
enum pci_p2pdma_map_type map_type = PCI_P2PDMA_MAP_THRU_HOST_BRIDGE;
438454
struct pci_dev *a = provider, *b = client, *bb;
439455
bool acs_redirects = false;
456+
struct pci_p2pdma *p2pdma;
440457
struct seq_buf acs_list;
441458
int acs_cnt = 0;
442459
int dist_a = 0;
@@ -515,9 +532,12 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
515532
map_type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
516533
}
517534
done:
518-
if (provider->p2pdma)
519-
xa_store(&provider->p2pdma->map_types, map_types_idx(client),
535+
rcu_read_lock();
536+
p2pdma = rcu_dereference(provider->p2pdma);
537+
if (p2pdma)
538+
xa_store(&p2pdma->map_types, map_types_idx(client),
520539
xa_mk_value(map_type), GFP_KERNEL);
540+
rcu_read_unlock();
521541
return map_type;
522542
}
523543

@@ -586,7 +606,15 @@ EXPORT_SYMBOL_GPL(pci_p2pdma_distance_many);
586606
*/
587607
bool pci_has_p2pmem(struct pci_dev *pdev)
588608
{
589-
return pdev->p2pdma && pdev->p2pdma->p2pmem_published;
609+
struct pci_p2pdma *p2pdma;
610+
bool res;
611+
612+
rcu_read_lock();
613+
p2pdma = rcu_dereference(pdev->p2pdma);
614+
res = p2pdma && p2pdma->p2pmem_published;
615+
rcu_read_unlock();
616+
617+
return res;
590618
}
591619
EXPORT_SYMBOL_GPL(pci_has_p2pmem);
592620

@@ -666,23 +694,24 @@ void *pci_alloc_p2pmem(struct pci_dev *pdev, size_t size)
666694
{
667695
void *ret = NULL;
668696
struct percpu_ref *ref;
697+
struct pci_p2pdma *p2pdma;
669698

670699
/*
671700
* Pairs with synchronize_rcu() in pci_p2pdma_release() to
672701
* ensure pdev->p2pdma is non-NULL for the duration of the
673702
* read-lock.
674703
*/
675704
rcu_read_lock();
676-
if (unlikely(!pdev->p2pdma))
705+
p2pdma = rcu_dereference(pdev->p2pdma);
706+
if (unlikely(!p2pdma))
677707
goto out;
678708

679-
ret = (void *)gen_pool_alloc_owner(pdev->p2pdma->pool, size,
680-
(void **) &ref);
709+
ret = (void *)gen_pool_alloc_owner(p2pdma->pool, size, (void **) &ref);
681710
if (!ret)
682711
goto out;
683712

684713
if (unlikely(!percpu_ref_tryget_live(ref))) {
685-
gen_pool_free(pdev->p2pdma->pool, (unsigned long) ret, size);
714+
gen_pool_free(p2pdma->pool, (unsigned long) ret, size);
686715
ret = NULL;
687716
goto out;
688717
}
@@ -701,8 +730,9 @@ EXPORT_SYMBOL_GPL(pci_alloc_p2pmem);
701730
void pci_free_p2pmem(struct pci_dev *pdev, void *addr, size_t size)
702731
{
703732
struct percpu_ref *ref;
733+
struct pci_p2pdma *p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
704734

705-
gen_pool_free_owner(pdev->p2pdma->pool, (uintptr_t)addr, size,
735+
gen_pool_free_owner(p2pdma->pool, (uintptr_t)addr, size,
706736
(void **) &ref);
707737
percpu_ref_put(ref);
708738
}
@@ -716,17 +746,21 @@ EXPORT_SYMBOL_GPL(pci_free_p2pmem);
716746
*/
717747
pci_bus_addr_t pci_p2pmem_virt_to_bus(struct pci_dev *pdev, void *addr)
718748
{
749+
struct pci_p2pdma *p2pdma;
750+
719751
if (!addr)
720752
return 0;
721-
if (!pdev->p2pdma)
753+
754+
p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
755+
if (!p2pdma)
722756
return 0;
723757

724758
/*
725759
* Note: when we added the memory to the pool we used the PCI
726760
* bus address as the physical address. So gen_pool_virt_to_phys()
727761
* actually returns the bus address despite the misleading name.
728762
*/
729-
return gen_pool_virt_to_phys(pdev->p2pdma->pool, (unsigned long)addr);
763+
return gen_pool_virt_to_phys(p2pdma->pool, (unsigned long)addr);
730764
}
731765
EXPORT_SYMBOL_GPL(pci_p2pmem_virt_to_bus);
732766

@@ -797,16 +831,23 @@ EXPORT_SYMBOL_GPL(pci_p2pmem_free_sgl);
797831
*/
798832
void pci_p2pmem_publish(struct pci_dev *pdev, bool publish)
799833
{
800-
if (pdev->p2pdma)
801-
pdev->p2pdma->p2pmem_published = publish;
834+
struct pci_p2pdma *p2pdma;
835+
836+
rcu_read_lock();
837+
p2pdma = rcu_dereference(pdev->p2pdma);
838+
if (p2pdma)
839+
p2pdma->p2pmem_published = publish;
840+
rcu_read_unlock();
802841
}
803842
EXPORT_SYMBOL_GPL(pci_p2pmem_publish);
804843

805844
static enum pci_p2pdma_map_type pci_p2pdma_map_type(struct dev_pagemap *pgmap,
806845
struct device *dev)
807846
{
847+
enum pci_p2pdma_map_type type = PCI_P2PDMA_MAP_NOT_SUPPORTED;
808848
struct pci_dev *provider = to_p2p_pgmap(pgmap)->provider;
809849
struct pci_dev *client;
850+
struct pci_p2pdma *p2pdma;
810851

811852
if (!provider->p2pdma)
812853
return PCI_P2PDMA_MAP_NOT_SUPPORTED;
@@ -816,8 +857,14 @@ static enum pci_p2pdma_map_type pci_p2pdma_map_type(struct dev_pagemap *pgmap,
816857

817858
client = to_pci_dev(dev);
818859

819-
return xa_to_value(xa_load(&provider->p2pdma->map_types,
820-
map_types_idx(client)));
860+
rcu_read_lock();
861+
p2pdma = rcu_dereference(provider->p2pdma);
862+
863+
if (p2pdma)
864+
type = xa_to_value(xa_load(&p2pdma->map_types,
865+
map_types_idx(client)));
866+
rcu_read_unlock();
867+
return type;
821868
}
822869

823870
static int __pci_p2pdma_map_sg(struct pci_p2pdma_pagemap *p2p_pgmap,

‎include/linux/pci.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,7 @@ struct pci_dev {
497497
u16 pasid_features;
498498
#endif
499499
#ifdef CONFIG_PCI_P2PDMA
500-
struct pci_p2pdma *p2pdma;
500+
struct pci_p2pdma __rcu *p2pdma;
501501
#endif
502502
u16 acs_cap; /* ACS Capability offset */
503503
phys_addr_t rom; /* Physical address if not from BAR */

0 commit comments

Comments
 (0)