The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP
way of finding") adds support for each physical function to have its
own MSI and MSI-X capability structures by introducing struct
dw_pcie_ep_func. However, BAR configuration and iATU mappings are still
being managed globally in struct dw_pcie_ep, meaning all PFs shared the
same BAR-to-iATU mapping table.
This creates conflicts when multiple physical functions attempts to
configure BARs independently, as they would overwrite each other's
iATU mappings and BAR configurations.
Move bar_to_atu and epf_bar from struct dw_pcie_ep to struct
dw_pcie_ep_func to allow proper multi-function endpoint operation,
where each function can configure its BARs without interfering with
other functions.
Signed-off-by: Aksh Garg <a-garg7@ti.com>
---
.../pci/controller/dwc/pcie-designware-ep.c | 50 +++++++++++++------
drivers/pci/controller/dwc/pcie-designware.h | 4 +-
2 files changed, 37 insertions(+), 17 deletions(-)
diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f222677a7a87..769602b58bd7 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -153,11 +153,16 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
int ret;
u32 free_win;
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
+ struct dw_pcie_ep_func *ep_func;
- if (!ep->bar_to_atu[bar])
+ ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
+ if (!ep_func)
+ return -EINVAL;
+
+ if (!ep_func->bar_to_atu[bar])
free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
else
- free_win = ep->bar_to_atu[bar] - 1;
+ free_win = ep_func->bar_to_atu[bar] - 1;
if (free_win >= pci->num_ib_windows) {
dev_err(pci->dev, "No free inbound window\n");
@@ -175,7 +180,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
* Always increment free_win before assignment, since value 0 is used to identify
* unallocated mapping.
*/
- ep->bar_to_atu[bar] = free_win + 1;
+ ep_func->bar_to_atu[bar] = free_win + 1;
set_bit(free_win, ep->ib_window_map);
return 0;
@@ -211,17 +216,22 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
enum pci_barno bar = epf_bar->barno;
- u32 atu_index = ep->bar_to_atu[bar] - 1;
+ struct dw_pcie_ep_func *ep_func;
+ u32 atu_index;
+
+ ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
- if (!ep->bar_to_atu[bar])
+ if (!ep_func || !ep_func->bar_to_atu[bar])
return;
+ atu_index = ep_func->bar_to_atu[bar] - 1;
+
__dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
clear_bit(atu_index, ep->ib_window_map);
- ep->epf_bar[bar] = NULL;
- ep->bar_to_atu[bar] = 0;
+ ep_func->epf_bar[bar] = NULL;
+ ep_func->bar_to_atu[bar] = 0;
}
static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no,
@@ -349,11 +359,16 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
struct dw_pcie_ep *ep = epc_get_drvdata(epc);
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
enum pci_barno bar = epf_bar->barno;
+ struct dw_pcie_ep_func *ep_func;
size_t size = epf_bar->size;
enum pci_epc_bar_type bar_type;
int flags = epf_bar->flags;
int ret, type;
+ ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
+ if (!ep_func)
+ return -EINVAL;
+
/*
* DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs
* 1 and 2 to form a 64-bit BAR.
@@ -367,14 +382,14 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
* calling clear_bar() would clear the BAR's PCI address assigned by the
* host).
*/
- if (ep->epf_bar[bar]) {
+ if (ep_func->epf_bar[bar]) {
/*
* We can only dynamically change a BAR if the new BAR size and
* BAR flags do not differ from the existing configuration.
*/
- if (ep->epf_bar[bar]->barno != bar ||
- ep->epf_bar[bar]->size != size ||
- ep->epf_bar[bar]->flags != flags)
+ if (ep_func->epf_bar[bar]->barno != bar ||
+ ep_func->epf_bar[bar]->size != size ||
+ ep_func->epf_bar[bar]->flags != flags)
return -EINVAL;
/*
@@ -420,7 +435,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
if (ret)
return ret;
- ep->epf_bar[bar] = epf_bar;
+ ep_func->epf_bar[bar] = epf_bar;
return 0;
}
@@ -782,7 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
tbl_offset &= PCI_MSIX_TABLE_OFFSET;
- msix_tbl = ep->epf_bar[bir]->addr + tbl_offset;
+ msix_tbl = ep_func->epf_bar[bir]->addr + tbl_offset;
msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr;
msg_data = msix_tbl[(interrupt_num - 1)].msg_data;
vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl;
@@ -845,11 +860,16 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no)
{
+ struct dw_pcie_ep_func *ep_func;
unsigned int offset;
unsigned int nbars;
enum pci_barno bar;
u32 reg, i, val;
+ ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
+ if (!ep_func)
+ return;
+
offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR);
if (offset) {
@@ -876,8 +896,8 @@ static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 fun
*/
val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL);
bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val);
- if (ep->epf_bar[bar])
- pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val);
+ if (ep_func->epf_bar[bar])
+ pci_epc_bar_size_to_rebar_cap(ep_func->epf_bar[bar]->size, &val);
else
val = BIT(4);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 31685951a080..a4d1733f5c6a 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -463,6 +463,8 @@ struct dw_pcie_ep_func {
u8 func_no;
u8 msi_cap; /* MSI capability offset */
u8 msix_cap; /* MSI-X capability offset */
+ u8 bar_to_atu[PCI_STD_NUM_BARS];
+ struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
};
struct dw_pcie_ep {
@@ -472,13 +474,11 @@ struct dw_pcie_ep {
phys_addr_t phys_base;
size_t addr_size;
size_t page_size;
- u8 bar_to_atu[PCI_STD_NUM_BARS];
phys_addr_t *outbound_addr;
unsigned long *ib_window_map;
unsigned long *ob_window_map;
void __iomem *msi_mem;
phys_addr_t msi_mem_phys;
- struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
};
struct dw_pcie_ops {
--
2.34.1On Wed, Jan 21, 2026 at 11:12:14AM +0530, Aksh Garg wrote:
> The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP
> way of finding") adds support for each physical function to have its
> own MSI and MSI-X capability structures by introducing struct
> dw_pcie_ep_func. However, BAR configuration and iATU mappings are still
> being managed globally in struct dw_pcie_ep, meaning all PFs shared the
> same BAR-to-iATU mapping table.
You are mentioning commit 47a062609a30 ("PCI: designware-ep: Modify MSI and
MSIX CAP way of finding"), but you should probably also mention commit
24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
which added support for PFs in the DWC driver.
That is the commit that is incorrect IMO. You cannot add support for PFs
and then have a different EPFs over overwriting address translation for
other EPFs. The design was simply broken from the start.
>
> This creates conflicts when multiple physical functions attempts to
> configure BARs independently, as they would overwrite each other's
> iATU mappings and BAR configurations.
>
> Move bar_to_atu and epf_bar from struct dw_pcie_ep to struct
> dw_pcie_ep_func to allow proper multi-function endpoint operation,
> where each function can configure its BARs without interfering with
> other functions.
>
> Signed-off-by: Aksh Garg <a-garg7@ti.com>
Fixes: 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
> ---
> .../pci/controller/dwc/pcie-designware-ep.c | 50 +++++++++++++------
> drivers/pci/controller/dwc/pcie-designware.h | 4 +-
> 2 files changed, 37 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index f222677a7a87..769602b58bd7 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -153,11 +153,16 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> int ret;
> u32 free_win;
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct dw_pcie_ep_func *ep_func;
>
> - if (!ep->bar_to_atu[bar])
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
There is no reason why this can't be initialized on the same line as your are
declaring the ep_func variable, just like we do for other variables, e.g.
struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
so please more the initialization to same line as the declaration.
> + if (!ep_func)
> + return -EINVAL;
> +
> + if (!ep_func->bar_to_atu[bar])
> free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
> else
> - free_win = ep->bar_to_atu[bar] - 1;
> + free_win = ep_func->bar_to_atu[bar] - 1;
>
> if (free_win >= pci->num_ib_windows) {
> dev_err(pci->dev, "No free inbound window\n");
> @@ -175,7 +180,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
> * Always increment free_win before assignment, since value 0 is used to identify
> * unallocated mapping.
> */
> - ep->bar_to_atu[bar] = free_win + 1;
> + ep_func->bar_to_atu[bar] = free_win + 1;
> set_bit(free_win, ep->ib_window_map);
>
> return 0;
> @@ -211,17 +216,22 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> enum pci_barno bar = epf_bar->barno;
> - u32 atu_index = ep->bar_to_atu[bar] - 1;
> + struct dw_pcie_ep_func *ep_func;
> + u32 atu_index;
> +
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
Same comment as above.
>
> - if (!ep->bar_to_atu[bar])
> + if (!ep_func || !ep_func->bar_to_atu[bar])
> return;
>
> + atu_index = ep_func->bar_to_atu[bar] - 1;
> +
> __dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
>
> dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
> clear_bit(atu_index, ep->ib_window_map);
> - ep->epf_bar[bar] = NULL;
> - ep->bar_to_atu[bar] = 0;
> + ep_func->epf_bar[bar] = NULL;
> + ep_func->bar_to_atu[bar] = 0;
> }
>
> static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no,
> @@ -349,11 +359,16 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> enum pci_barno bar = epf_bar->barno;
> + struct dw_pcie_ep_func *ep_func;
> size_t size = epf_bar->size;
> enum pci_epc_bar_type bar_type;
> int flags = epf_bar->flags;
> int ret, type;
>
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
Same comment as above.
> + if (!ep_func)
> + return -EINVAL;
> +
> /*
> * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs
> * 1 and 2 to form a 64-bit BAR.
> @@ -367,14 +382,14 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> * calling clear_bar() would clear the BAR's PCI address assigned by the
> * host).
> */
> - if (ep->epf_bar[bar]) {
> + if (ep_func->epf_bar[bar]) {
> /*
> * We can only dynamically change a BAR if the new BAR size and
> * BAR flags do not differ from the existing configuration.
> */
> - if (ep->epf_bar[bar]->barno != bar ||
> - ep->epf_bar[bar]->size != size ||
> - ep->epf_bar[bar]->flags != flags)
> + if (ep_func->epf_bar[bar]->barno != bar ||
> + ep_func->epf_bar[bar]->size != size ||
> + ep_func->epf_bar[bar]->flags != flags)
> return -EINVAL;
>
> /*
> @@ -420,7 +435,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> if (ret)
> return ret;
>
> - ep->epf_bar[bar] = epf_bar;
> + ep_func->epf_bar[bar] = epf_bar;
>
> return 0;
> }
> @@ -782,7 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
> tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>
> - msix_tbl = ep->epf_bar[bir]->addr + tbl_offset;
> + msix_tbl = ep_func->epf_bar[bir]->addr + tbl_offset;
> msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr;
> msg_data = msix_tbl[(interrupt_num - 1)].msg_data;
> vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl;
> @@ -845,11 +860,16 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
>
> static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no)
> {
> + struct dw_pcie_ep_func *ep_func;
> unsigned int offset;
> unsigned int nbars;
> enum pci_barno bar;
> u32 reg, i, val;
>
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
Same comment as above.
> + if (!ep_func)
> + return;
> +
> offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR);
>
> if (offset) {
> @@ -876,8 +896,8 @@ static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 fun
> */
> val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL);
> bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val);
> - if (ep->epf_bar[bar])
> - pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val);
> + if (ep_func->epf_bar[bar])
> + pci_epc_bar_size_to_rebar_cap(ep_func->epf_bar[bar]->size, &val);
> else
> val = BIT(4);
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index 31685951a080..a4d1733f5c6a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -463,6 +463,8 @@ struct dw_pcie_ep_func {
> u8 func_no;
> u8 msi_cap; /* MSI capability offset */
> u8 msix_cap; /* MSI-X capability offset */
> + u8 bar_to_atu[PCI_STD_NUM_BARS];
> + struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
> };
>
> struct dw_pcie_ep {
> @@ -472,13 +474,11 @@ struct dw_pcie_ep {
> phys_addr_t phys_base;
> size_t addr_size;
> size_t page_size;
> - u8 bar_to_atu[PCI_STD_NUM_BARS];
> phys_addr_t *outbound_addr;
> unsigned long *ib_window_map;
> unsigned long *ob_window_map;
> void __iomem *msi_mem;
> phys_addr_t msi_mem_phys;
> - struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
> };
>
> struct dw_pcie_ops {
> --
> 2.34.1
>
With minor nits fixed:
Reviewed-by: Niklas Cassel <cassel@kernel.org>
> On Wed, Jan 21, 2026 at 11:12:14AM +0530, Aksh Garg wrote:
>> The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP
>> way of finding") adds support for each physical function to have its
>> own MSI and MSI-X capability structures by introducing struct
>> dw_pcie_ep_func. However, BAR configuration and iATU mappings are still
>> being managed globally in struct dw_pcie_ep, meaning all PFs shared the
>> same BAR-to-iATU mapping table.
>
> You are mentioning commit 47a062609a30 ("PCI: designware-ep: Modify MSI and
> MSIX CAP way of finding"), but you should probably also mention commit
> 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
> which added support for PFs in the DWC driver.
>
> That is the commit that is incorrect IMO. You cannot add support for PFs
> and then have a different EPFs over overwriting address translation for
> other EPFs. The design was simply broken from the start.
>
Yes, the commit 24ede430fa49 is indeed the culprit for this issue. Maybe
the way I wrote this commit message confused you. I was just referring
to the commit 47a062609a30 saying that as this commit added the feature
for each PF to have its own MSI and MSI-X capability, I wanted to move
bar_to_atu and epf_bar per-PF as well, such that each PF should have its
own epf_bar[] and bar_to_atu[]. The index passed to these fields should
be unique across PF+BAR combinations while the current implementation is
only keeping it unique across BARs but not PFs. This results in
overwriting address translation regions across the PFs.
I will rephrase the commit message and add fixes tag for 24ede430fa49.
Please provide your input whether the above explaination is sufficient.
Should I refer to commit 47a062609a30 as an example in the commit
message, or that would not be required given the above explaination is
added?
>
>>
>> This creates conflicts when multiple physical functions attempts to
>> configure BARs independently, as they would overwrite each other's
>> iATU mappings and BAR configurations.
>>
>> Move bar_to_atu and epf_bar from struct dw_pcie_ep to struct
>> dw_pcie_ep_func to allow proper multi-function endpoint operation,
>> where each function can configure its BARs without interfering with
>> other functions.
>>
>> Signed-off-by: Aksh Garg <a-garg7@ti.com>
>
> Fixes: 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
>
>
>> ---
>> .../pci/controller/dwc/pcie-designware-ep.c | 50 +++++++++++++------
>> drivers/pci/controller/dwc/pcie-designware.h | 4 +-
>> 2 files changed, 37 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> index f222677a7a87..769602b58bd7 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
>> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
>> @@ -153,11 +153,16 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
>> int ret;
>> u32 free_win;
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> + struct dw_pcie_ep_func *ep_func;
>>
>> - if (!ep->bar_to_atu[bar])
>> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>
> There is no reason why this can't be initialized on the same line as your are
> declaring the ep_func variable, just like we do for other variables, e.g.
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> so please more the initialization to same line as the declaration.
>
>
>> + if (!ep_func)
>> + return -EINVAL;
>> +
>> + if (!ep_func->bar_to_atu[bar])
>> free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
>> else
>> - free_win = ep->bar_to_atu[bar] - 1;
>> + free_win = ep_func->bar_to_atu[bar] - 1;
>>
>> if (free_win >= pci->num_ib_windows) {
>> dev_err(pci->dev, "No free inbound window\n");
>> @@ -175,7 +180,7 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no, int type,
>> * Always increment free_win before assignment, since value 0 is used to identify
>> * unallocated mapping.
>> */
>> - ep->bar_to_atu[bar] = free_win + 1;
>> + ep_func->bar_to_atu[bar] = free_win + 1;
>> set_bit(free_win, ep->ib_window_map);
>>
>> return 0;
>> @@ -211,17 +216,22 @@ static void dw_pcie_ep_clear_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> enum pci_barno bar = epf_bar->barno;
>> - u32 atu_index = ep->bar_to_atu[bar] - 1;
>> + struct dw_pcie_ep_func *ep_func;
>> + u32 atu_index;
>> +
>> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>
> Same comment as above.
>
>>
>> - if (!ep->bar_to_atu[bar])
>> + if (!ep_func || !ep_func->bar_to_atu[bar])
>> return;
>>
>> + atu_index = ep_func->bar_to_atu[bar] - 1;
>> +
>> __dw_pcie_ep_reset_bar(pci, func_no, bar, epf_bar->flags);
>>
>> dw_pcie_disable_atu(pci, PCIE_ATU_REGION_DIR_IB, atu_index);
>> clear_bit(atu_index, ep->ib_window_map);
>> - ep->epf_bar[bar] = NULL;
>> - ep->bar_to_atu[bar] = 0;
>> + ep_func->epf_bar[bar] = NULL;
>> + ep_func->bar_to_atu[bar] = 0;
>> }
>>
>> static unsigned int dw_pcie_ep_get_rebar_offset(struct dw_pcie_ep *ep, u8 func_no,
>> @@ -349,11 +359,16 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> struct dw_pcie_ep *ep = epc_get_drvdata(epc);
>> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
>> enum pci_barno bar = epf_bar->barno;
>> + struct dw_pcie_ep_func *ep_func;
>> size_t size = epf_bar->size;
>> enum pci_epc_bar_type bar_type;
>> int flags = epf_bar->flags;
>> int ret, type;
>>
>> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>
> Same comment as above.
>
>
>> + if (!ep_func)
>> + return -EINVAL;
>> +
>> /*
>> * DWC does not allow BAR pairs to overlap, e.g. you cannot combine BARs
>> * 1 and 2 to form a 64-bit BAR.
>> @@ -367,14 +382,14 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> * calling clear_bar() would clear the BAR's PCI address assigned by the
>> * host).
>> */
>> - if (ep->epf_bar[bar]) {
>> + if (ep_func->epf_bar[bar]) {
>> /*
>> * We can only dynamically change a BAR if the new BAR size and
>> * BAR flags do not differ from the existing configuration.
>> */
>> - if (ep->epf_bar[bar]->barno != bar ||
>> - ep->epf_bar[bar]->size != size ||
>> - ep->epf_bar[bar]->flags != flags)
>> + if (ep_func->epf_bar[bar]->barno != bar ||
>> + ep_func->epf_bar[bar]->size != size ||
>> + ep_func->epf_bar[bar]->flags != flags)
>> return -EINVAL;
>>
>> /*
>> @@ -420,7 +435,7 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>> if (ret)
>> return ret;
>>
>> - ep->epf_bar[bar] = epf_bar;
>> + ep_func->epf_bar[bar] = epf_bar;
>>
>> return 0;
>> }
>> @@ -782,7 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
>> bir = FIELD_GET(PCI_MSIX_TABLE_BIR, tbl_offset);
>> tbl_offset &= PCI_MSIX_TABLE_OFFSET;
>>
>> - msix_tbl = ep->epf_bar[bir]->addr + tbl_offset;
>> + msix_tbl = ep_func->epf_bar[bir]->addr + tbl_offset;
>> msg_addr = msix_tbl[(interrupt_num - 1)].msg_addr;
>> msg_data = msix_tbl[(interrupt_num - 1)].msg_data;
>> vec_ctrl = msix_tbl[(interrupt_num - 1)].vector_ctrl;
>> @@ -845,11 +860,16 @@ EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit);
>>
>> static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 func_no)
>> {
>> + struct dw_pcie_ep_func *ep_func;
>> unsigned int offset;
>> unsigned int nbars;
>> enum pci_barno bar;
>> u32 reg, i, val;
>>
>> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
>
> Same comment as above.
>
>
>> + if (!ep_func)
>> + return;
>> +
>> offset = dw_pcie_ep_find_ext_capability(ep, func_no, PCI_EXT_CAP_ID_REBAR);
>>
>> if (offset) {
>> @@ -876,8 +896,8 @@ static void __dw_pcie_ep_init_non_sticky_registers(struct dw_pcie_ep *ep, u8 fun
>> */
>> val = dw_pcie_ep_readl_dbi(ep, func_no, offset + PCI_REBAR_CTRL);
>> bar = FIELD_GET(PCI_REBAR_CTRL_BAR_IDX, val);
>> - if (ep->epf_bar[bar])
>> - pci_epc_bar_size_to_rebar_cap(ep->epf_bar[bar]->size, &val);
>> + if (ep_func->epf_bar[bar])
>> + pci_epc_bar_size_to_rebar_cap(ep_func->epf_bar[bar]->size, &val);
>> else
>> val = BIT(4);
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
>> index 31685951a080..a4d1733f5c6a 100644
>> --- a/drivers/pci/controller/dwc/pcie-designware.h
>> +++ b/drivers/pci/controller/dwc/pcie-designware.h
>> @@ -463,6 +463,8 @@ struct dw_pcie_ep_func {
>> u8 func_no;
>> u8 msi_cap; /* MSI capability offset */
>> u8 msix_cap; /* MSI-X capability offset */
>> + u8 bar_to_atu[PCI_STD_NUM_BARS];
>> + struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
>> };
>>
>> struct dw_pcie_ep {
>> @@ -472,13 +474,11 @@ struct dw_pcie_ep {
>> phys_addr_t phys_base;
>> size_t addr_size;
>> size_t page_size;
>> - u8 bar_to_atu[PCI_STD_NUM_BARS];
>> phys_addr_t *outbound_addr;
>> unsigned long *ib_window_map;
>> unsigned long *ob_window_map;
>> void __iomem *msi_mem;
>> phys_addr_t msi_mem_phys;
>> - struct pci_epf_bar *epf_bar[PCI_STD_NUM_BARS];
>> };
>>
>> struct dw_pcie_ops {
>> --
>> 2.34.1
>>
>
> With minor nits fixed:
> Reviewed-by: Niklas Cassel <cassel@kernel.org>
>
Thank you for reviewing the patch. I will fix these minor nits.
Regards,
Aksh Garg
On Wed, Jan 21, 2026 at 05:05:50PM +0530, Aksh Garg wrote:
>
>
> > On Wed, Jan 21, 2026 at 11:12:14AM +0530, Aksh Garg wrote:
> > > The commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP
> > > way of finding") adds support for each physical function to have its
> > > own MSI and MSI-X capability structures by introducing struct
> > > dw_pcie_ep_func. However, BAR configuration and iATU mappings are still
> > > being managed globally in struct dw_pcie_ep, meaning all PFs shared the
> > > same BAR-to-iATU mapping table.
> >
> > You are mentioning commit 47a062609a30 ("PCI: designware-ep: Modify MSI and
> > MSIX CAP way of finding"), but you should probably also mention commit
> > 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
> > which added support for PFs in the DWC driver.
> >
> > That is the commit that is incorrect IMO. You cannot add support for PFs
> > and then have a different EPFs over overwriting address translation for
> > other EPFs. The design was simply broken from the start.
> >
>
> Yes, the commit 24ede430fa49 is indeed the culprit for this issue. Maybe the
> way I wrote this commit message confused you. I was just referring to the
> commit 47a062609a30 saying that as this commit added the feature for each PF
> to have its own MSI and MSI-X capability, I wanted to move bar_to_atu and
> epf_bar per-PF as well, such that each PF should have its own epf_bar[] and
> bar_to_atu[]. The index passed to these fields should be unique across
> PF+BAR combinations while the current implementation is only keeping it
> unique across BARs but not PFs. This results in overwriting address
> translation regions across the PFs.
> I will rephrase the commit message and add fixes tag for 24ede430fa49.
>
> Please provide your input whether the above explaination is sufficient.
> Should I refer to commit 47a062609a30 as an example in the commit message,
> or that would not be required given the above explaination is added?
As long as you mention both commits, I think it is sufficient.
Perhaps something like:
Commit 24ede430fa49 ("PCI: designware-ep: Add multiple PFs support for DWC")
added support for multiple PFs in the DWC driver.
However, this commit was incomplete, and did not properly support MSI/MSI-X
for multiple PFs, which was fixed in commit 47a062609a30 ("PCI: designware-ep:
Modify MSI and MSIX CAP way of finding").
Even with both these commits, the multiple PF support in the DWC driver is
severely broken, because one EPF for one PF will currently overwrite the address
translation done by another EPF for a completely different PF. To fix this,
just as commit 47a062609a30 ("PCI: designware-ep: Modify MSI and MSIX CAP way of
finding") did when it moved things to a per PF struct dw_pcie_ep_func, moving the
data structures needed for address translation to struct dw_pcie_ep_func.
Kind regards,
Niklas
© 2016 - 2026 Red Hat, Inc.