* [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
@ 2018-11-19 13:14 sundeep.lkml
2018-11-26 6:40 ` sundeep subbaraya
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: sundeep.lkml @ 2018-11-19 13:14 UTC (permalink / raw)
To: bhelgaas, linux-pci, linux-kernel, sean.stalley
Cc: sgoutham, Subbaraya Sundeep
From: Subbaraya Sundeep <sbhatta@marvell.com>
As per the spec, bridges with EA capability work
with fixed secondary and subordinate bus numbers.
Hence assign bus numbers to bridges from EA if the
capability exists.
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
---
Changes for v2:
No changes just added Sean Stalley who did EA support for BARs.
drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
include/uapi/linux/pci_regs.h | 6 +++++
2 files changed, 60 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index b1c05b5..f41d2e6 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
unsigned int available_buses);
+/*
+ * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
+ * numbers from EA capability.
+ * @dev: Bridge with EA
+ * @secondary: updated with secondary bus number in EA
+ * @subordinate: updated with subordinate bus number in EA
+ *
+ * If it is a bridge with EA capability then fixed bus numbers are
+ * read from EA capability list and secondary, subordinate reference
+ * variables will be updated. Otherwise secondary and subordinate reference
+ * variables will be zeroed.
+ */
+static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
+ u8 *subordinate)
+{
+ int ea;
+ int offset;
+ u32 dw;
+
+ *secondary = *subordinate = 0;
+
+ if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
+ return;
+
+ /* find PCI EA capability in list */
+ ea = pci_find_capability(dev, PCI_CAP_ID_EA);
+ if (!ea)
+ return;
+
+ offset = ea + PCI_EA_FIRST_ENT;
+ pci_read_config_dword(dev, offset, &dw);
+ *secondary = dw & PCI_EA_SEC_BUS_MASK;
+ *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
+}
/*
* pci_scan_bridge_extend() - Scan buses behind a bridge
@@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
u16 bctl;
u8 primary, secondary, subordinate;
int broken = 0;
+ u8 fixed_sec, fixed_sub;
+ int next_busnr;
/*
* Make sure the bridge is powered on to be able to access config
@@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
/* Clear errors */
pci_write_config_word(dev, PCI_STATUS, 0xffff);
+ /* read bus numbers from EA */
+ pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
+
+ next_busnr = max + 1;
+ /* Use secondary bus number in EA */
+ if (fixed_sec)
+ next_busnr = fixed_sec;
+
/*
* Prevent assigning a bus number that already exists.
* This can happen when a bridge is hot-plugged, so in this
* case we only re-scan this bus.
*/
- child = pci_find_bus(pci_domain_nr(bus), max+1);
+ child = pci_find_bus(pci_domain_nr(bus), next_busnr);
if (!child) {
- child = pci_add_new_bus(bus, dev, max+1);
+ child = pci_add_new_bus(bus, dev, next_busnr);
if (!child)
goto out;
- pci_bus_insert_busn_res(child, max+1,
+ pci_bus_insert_busn_res(child, next_busnr,
bus->busn_res.end);
}
max++;
@@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
max += i;
}
- /* Set subordinate bus number to its real value */
+ /*
+ * Set subordinate bus number to its real value.
+ * If fixed subordinate bus number exists from EA
+ * capability then use it.
+ */
+ if (fixed_sub)
+ max = fixed_sub;
pci_bus_update_busn_res_end(child, max);
pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
}
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e1e9888..c3d0904 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -372,6 +372,12 @@
#define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
#define PCI_EA_ES 0x00000007 /* Entry Size */
#define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
+
+/* EA fixed Secondary and Subordinate bus numbers for Bridge */
+#define PCI_EA_SEC_BUS_MASK 0xff
+#define PCI_EA_SUB_BUS_MASK 0xff00
+#define PCI_EA_SUB_BUS_SHIFT 8
+
/* 0-5 map to BARs 0-5 respectively */
#define PCI_EA_BEI_BAR0 0
#define PCI_EA_BEI_BAR5 5
--
1.8.3.1
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2018-11-19 13:14 [PATCH v2] PCI: assign bus numbers present in EA capability for bridges sundeep.lkml
@ 2018-11-26 6:40 ` sundeep subbaraya
2018-11-28 21:55 ` Bjorn Helgaas
2019-04-17 20:16 ` Bjorn Helgaas
2 siblings, 0 replies; 8+ messages in thread
From: sundeep subbaraya @ 2018-11-26 6:40 UTC (permalink / raw)
To: Bjorn Helgaas, linux-pci, linux-kernel, sean.stalley, helgaas
Cc: sgoutham, Subbaraya Sundeep
Hi Bjorn / Sean,
Any comments?
Thanks,
Sundeep
On Mon, Nov 19, 2018 at 6:44 PM <sundeep.lkml@gmail.com> wrote:
>
> From: Subbaraya Sundeep <sbhatta@marvell.com>
>
> As per the spec, bridges with EA capability work
> with fixed secondary and subordinate bus numbers.
> Hence assign bus numbers to bridges from EA if the
> capability exists.
>
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> ---
> Changes for v2:
> No changes just added Sean Stalley who did EA support for BARs.
>
> drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> include/uapi/linux/pci_regs.h | 6 +++++
> 2 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index b1c05b5..f41d2e6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
>
> static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> unsigned int available_buses);
> +/*
> + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> + * numbers from EA capability.
> + * @dev: Bridge with EA
> + * @secondary: updated with secondary bus number in EA
> + * @subordinate: updated with subordinate bus number in EA
> + *
> + * If it is a bridge with EA capability then fixed bus numbers are
> + * read from EA capability list and secondary, subordinate reference
> + * variables will be updated. Otherwise secondary and subordinate reference
> + * variables will be zeroed.
> + */
> +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> + u8 *subordinate)
> +{
> + int ea;
> + int offset;
> + u32 dw;
> +
> + *secondary = *subordinate = 0;
> +
> + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> + return;
> +
> + /* find PCI EA capability in list */
> + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> + if (!ea)
> + return;
> +
> + offset = ea + PCI_EA_FIRST_ENT;
> + pci_read_config_dword(dev, offset, &dw);
> + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> +}
>
> /*
> * pci_scan_bridge_extend() - Scan buses behind a bridge
> @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> u16 bctl;
> u8 primary, secondary, subordinate;
> int broken = 0;
> + u8 fixed_sec, fixed_sub;
> + int next_busnr;
>
> /*
> * Make sure the bridge is powered on to be able to access config
> @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> /* Clear errors */
> pci_write_config_word(dev, PCI_STATUS, 0xffff);
>
> + /* read bus numbers from EA */
> + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> +
> + next_busnr = max + 1;
> + /* Use secondary bus number in EA */
> + if (fixed_sec)
> + next_busnr = fixed_sec;
> +
> /*
> * Prevent assigning a bus number that already exists.
> * This can happen when a bridge is hot-plugged, so in this
> * case we only re-scan this bus.
> */
> - child = pci_find_bus(pci_domain_nr(bus), max+1);
> + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> if (!child) {
> - child = pci_add_new_bus(bus, dev, max+1);
> + child = pci_add_new_bus(bus, dev, next_busnr);
> if (!child)
> goto out;
> - pci_bus_insert_busn_res(child, max+1,
> + pci_bus_insert_busn_res(child, next_busnr,
> bus->busn_res.end);
> }
> max++;
> @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> max += i;
> }
>
> - /* Set subordinate bus number to its real value */
> + /*
> + * Set subordinate bus number to its real value.
> + * If fixed subordinate bus number exists from EA
> + * capability then use it.
> + */
> + if (fixed_sub)
> + max = fixed_sub;
> pci_bus_update_busn_res_end(child, max);
> pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e1e9888..c3d0904 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,12 @@
> #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> #define PCI_EA_ES 0x00000007 /* Entry Size */
> #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> +
> +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> +#define PCI_EA_SEC_BUS_MASK 0xff
> +#define PCI_EA_SUB_BUS_MASK 0xff00
> +#define PCI_EA_SUB_BUS_SHIFT 8
> +
> /* 0-5 map to BARs 0-5 respectively */
> #define PCI_EA_BEI_BAR0 0
> #define PCI_EA_BEI_BAR5 5
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2018-11-19 13:14 [PATCH v2] PCI: assign bus numbers present in EA capability for bridges sundeep.lkml
2018-11-26 6:40 ` sundeep subbaraya
@ 2018-11-28 21:55 ` Bjorn Helgaas
2018-11-29 13:30 ` sundeep subbaraya
2019-04-17 20:16 ` Bjorn Helgaas
2 siblings, 1 reply; 8+ messages in thread
From: Bjorn Helgaas @ 2018-11-28 21:55 UTC (permalink / raw)
To: sundeep.lkml
Cc: linux-pci, linux-kernel, sean.stalley, sgoutham, Subbaraya Sundeep
On Mon, Nov 19, 2018 at 06:44:32PM +0530, sundeep.lkml@gmail.com wrote:
> From: Subbaraya Sundeep <sbhatta@marvell.com>
>
> As per the spec, bridges with EA capability work
> with fixed secondary and subordinate bus numbers.
> Hence assign bus numbers to bridges from EA if the
> capability exists.
A reference to the spec section would be good, i.e., PCIe r4.0, sec xxx.
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> ---
> Changes for v2:
> No changes just added Sean Stalley who did EA support for BARs.
>
> drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> include/uapi/linux/pci_regs.h | 6 +++++
> 2 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index b1c05b5..f41d2e6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
>
> static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> unsigned int available_buses);
> +/*
> + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> + * numbers from EA capability.
> + * @dev: Bridge with EA
> + * @secondary: updated with secondary bus number in EA
> + * @subordinate: updated with subordinate bus number in EA
> + *
> + * If it is a bridge with EA capability then fixed bus numbers are
> + * read from EA capability list and secondary, subordinate reference
> + * variables will be updated. Otherwise secondary and subordinate reference
> + * variables will be zeroed.
> + */
> +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> + u8 *subordinate)
> +{
> + int ea;
> + int offset;
> + u32 dw;
> +
> + *secondary = *subordinate = 0;
> +
> + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> + return;
> +
> + /* find PCI EA capability in list */
> + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> + if (!ea)
> + return;
> +
> + offset = ea + PCI_EA_FIRST_ENT;
> + pci_read_config_dword(dev, offset, &dw);
"Num Entries" in the first DW of the capability is allowed to be zero,
in which case this word (the second DW) is invalid. [See comments
below; this code would be valid based on the 2014 ECN, but not per the
2017 PCIe r4.0 spec]
It would be much better if this function could be somehow incorporated
into pci_ea_init(), which already knows how to parse much of the EA
capability.
> + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> +}
>
> /*
> * pci_scan_bridge_extend() - Scan buses behind a bridge
> @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> u16 bctl;
> u8 primary, secondary, subordinate;
> int broken = 0;
> + u8 fixed_sec, fixed_sub;
> + int next_busnr;
>
> /*
> * Make sure the bridge is powered on to be able to access config
> @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> /* Clear errors */
> pci_write_config_word(dev, PCI_STATUS, 0xffff);
>
> + /* read bus numbers from EA */
> + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> +
> + next_busnr = max + 1;
> + /* Use secondary bus number in EA */
> + if (fixed_sec)
> + next_busnr = fixed_sec;
> +
> /*
> * Prevent assigning a bus number that already exists.
> * This can happen when a bridge is hot-plugged, so in this
> * case we only re-scan this bus.
> */
> - child = pci_find_bus(pci_domain_nr(bus), max+1);
> + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> if (!child) {
> - child = pci_add_new_bus(bus, dev, max+1);
> + child = pci_add_new_bus(bus, dev, next_busnr);
> if (!child)
> goto out;
> - pci_bus_insert_busn_res(child, max+1,
> + pci_bus_insert_busn_res(child, next_busnr,
> bus->busn_res.end);
> }
> max++;
> @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> max += i;
> }
>
> - /* Set subordinate bus number to its real value */
> + /*
> + * Set subordinate bus number to its real value.
> + * If fixed subordinate bus number exists from EA
> + * capability then use it.
> + */
> + if (fixed_sub)
> + max = fixed_sub;
> pci_bus_update_busn_res_end(child, max);
> pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e1e9888..c3d0904 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,12 @@
> #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
You didn't add PCI_EA_FIRST_ENT_BRIDGE (Sean did with f80b0ba95964
("PCI: Add Enhanced Allocation register entries")), but it is unused
and I can't match it with anything in the spec (PCIe r4.0, sec 7.8.5).
It might be related to the code in pci_ea_init() that skips DWORD 2
for type 1 functions. But I still don't see that in the spec.
If it's obsolete, we should remove it (with a separate patch).
Hmm, I do see this in the ECN dated 23 Oct 2014, sec 6.9.1.2. But
presumably that would be incorporated in PCIe r4.0, which is dated 27
Sep 2017.
I have no idea what to do with this. I can't merge this without some
clarification here,
> #define PCI_EA_ES 0x00000007 /* Entry Size */
> #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> +
> +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> +#define PCI_EA_SEC_BUS_MASK 0xff
> +#define PCI_EA_SUB_BUS_MASK 0xff00
> +#define PCI_EA_SUB_BUS_SHIFT 8
> +
> /* 0-5 map to BARs 0-5 respectively */
> #define PCI_EA_BEI_BAR0 0
> #define PCI_EA_BEI_BAR5 5
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2018-11-28 21:55 ` Bjorn Helgaas
@ 2018-11-29 13:30 ` sundeep subbaraya
2018-11-29 15:03 ` Bjorn Helgaas
0 siblings, 1 reply; 8+ messages in thread
From: sundeep subbaraya @ 2018-11-29 13:30 UTC (permalink / raw)
To: helgaas
Cc: linux-pci, linux-kernel, sean.stalley, sgoutham, Subbaraya Sundeep
Hi Bjorn,
On Thu, Nov 29, 2018 at 3:25 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Nov 19, 2018 at 06:44:32PM +0530, sundeep.lkml@gmail.com wrote:
> > From: Subbaraya Sundeep <sbhatta@marvell.com>
> >
> > As per the spec, bridges with EA capability work
> > with fixed secondary and subordinate bus numbers.
> > Hence assign bus numbers to bridges from EA if the
> > capability exists.
>
> A reference to the spec section would be good, i.e., PCIe r4.0, sec xxx.
>
Ok. I referred ECN 2014 section 6.9.1.2.
> > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> > ---
> > Changes for v2:
> > No changes just added Sean Stalley who did EA support for BARs.
> >
> > drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> > include/uapi/linux/pci_regs.h | 6 +++++
> > 2 files changed, 60 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index b1c05b5..f41d2e6 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
> >
> > static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> > unsigned int available_buses);
> > +/*
> > + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> > + * numbers from EA capability.
> > + * @dev: Bridge with EA
> > + * @secondary: updated with secondary bus number in EA
> > + * @subordinate: updated with subordinate bus number in EA
> > + *
> > + * If it is a bridge with EA capability then fixed bus numbers are
> > + * read from EA capability list and secondary, subordinate reference
> > + * variables will be updated. Otherwise secondary and subordinate reference
> > + * variables will be zeroed.
> > + */
> > +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> > + u8 *subordinate)
> > +{
> > + int ea;
> > + int offset;
> > + u32 dw;
> > +
> > + *secondary = *subordinate = 0;
> > +
> > + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> > + return;
> > +
> > + /* find PCI EA capability in list */
> > + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> > + if (!ea)
> > + return;
> > +
> > + offset = ea + PCI_EA_FIRST_ENT;
> > + pci_read_config_dword(dev, offset, &dw);
>
> "Num Entries" in the first DW of the capability is allowed to be zero,
> in which case this word (the second DW) is invalid. [See comments
> below; this code would be valid based on the 2014 ECN, but not per the
> 2017 PCIe r4.0 spec]
>
Yes but Entries follow after first DW of EA capability for devices and
after second
DW for bridges.
2014 ECN says for bridges DW2 of EA must be present:
"For Type 1 functions only, there is a second DW in the capability,
preceding the first entry.
This second DW must be included in the Enhanced Allocation Capability
whenever this
capability is implemented in a Type 1 Function"
So for normal device EA DW1, Entries(if any)
for bridges EA DW1,EA DW2, Entries(if any)
> It would be much better if this function could be somehow incorporated
> into pci_ea_init(), which already knows how to parse much of the EA
> capability.
>
I initially thought of this but didn't do it to avoid new members in pci_dev.
> > + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> > + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> > +}
> >
> > /*
> > * pci_scan_bridge_extend() - Scan buses behind a bridge
> > @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > u16 bctl;
> > u8 primary, secondary, subordinate;
> > int broken = 0;
> > + u8 fixed_sec, fixed_sub;
> > + int next_busnr;
> >
> > /*
> > * Make sure the bridge is powered on to be able to access config
> > @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > /* Clear errors */
> > pci_write_config_word(dev, PCI_STATUS, 0xffff);
> >
> > + /* read bus numbers from EA */
> > + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> > +
> > + next_busnr = max + 1;
> > + /* Use secondary bus number in EA */
> > + if (fixed_sec)
> > + next_busnr = fixed_sec;
> > +
> > /*
> > * Prevent assigning a bus number that already exists.
> > * This can happen when a bridge is hot-plugged, so in this
> > * case we only re-scan this bus.
> > */
> > - child = pci_find_bus(pci_domain_nr(bus), max+1);
> > + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> > if (!child) {
> > - child = pci_add_new_bus(bus, dev, max+1);
> > + child = pci_add_new_bus(bus, dev, next_busnr);
> > if (!child)
> > goto out;
> > - pci_bus_insert_busn_res(child, max+1,
> > + pci_bus_insert_busn_res(child, next_busnr,
> > bus->busn_res.end);
> > }
> > max++;
> > @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > max += i;
> > }
> >
> > - /* Set subordinate bus number to its real value */
> > + /*
> > + * Set subordinate bus number to its real value.
> > + * If fixed subordinate bus number exists from EA
> > + * capability then use it.
> > + */
> > + if (fixed_sub)
> > + max = fixed_sub;
> > pci_bus_update_busn_res_end(child, max);
> > pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> > }
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index e1e9888..c3d0904 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -372,6 +372,12 @@
> > #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
>
> You didn't add PCI_EA_FIRST_ENT_BRIDGE (Sean did with f80b0ba95964
> ("PCI: Add Enhanced Allocation register entries")), but it is unused
> and I can't match it with anything in the spec (PCIe r4.0, sec 7.8.5).
>
> It might be related to the code in pci_ea_init() that skips DWORD 2
> for type 1 functions. But I still don't see that in the spec.
>
> If it's obsolete, we should remove it (with a separate patch).
>
PCI_EA_FIRST_ENT_BRIDGE is not needed we can remove.
I will send a patch to remove it.
I will add new members for fixed secondary and subordinate bus numbers in
pci_dev and make changes as below please let me know it is okay for you:
@@ -2909,6 +2909,7 @@ void pci_ea_init(struct pci_dev *dev)
u8 num_ent;
int offset;
int i;
+ u32 dw;
/* find PCI EA capability in list */
ea = pci_find_capability(dev, PCI_CAP_ID_EA); @@ -2922,9
+2923,14 @@ void pci_ea_init(struct pci_dev *dev)
offset = ea + PCI_EA_FIRST_ENT;
- /* Skip DWORD 2 for type 1 functions */
- if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
+ /* Note fixed bus numbers for type 1 functions */
+ if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
+ pci_read_config_dword(dev, offset, &dw);
+ dev->fixed_sec_busnr = dw & PCI_EA_FIXED_SEC_BUS;
+ dev->fixed_sub_busnr = (dw & PCI_EA_FIXED_SUB_BUS) >>
+ PCI_EA_FIXED_SUB_SHIFT;
offset += 4;
+ }
> Hmm, I do see this in the ECN dated 23 Oct 2014, sec 6.9.1.2. But
> presumably that would be incorporated in PCIe r4.0, which is dated 27
> Sep 2017.
>
> I have no idea what to do with this. I can't merge this without some
> clarification here,
>
Yeah I see there is no mention of EA for bridges in r4.0.
Since we check whether the device is bridge or not before reading DW2 of EA
I guess we are okay.
Thanks,
Sundeep
> > #define PCI_EA_ES 0x00000007 /* Entry Size */
> > #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> > +
> > +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> > +#define PCI_EA_SEC_BUS_MASK 0xff
> > +#define PCI_EA_SUB_BUS_MASK 0xff00
> > +#define PCI_EA_SUB_BUS_SHIFT 8
> > +
> > /* 0-5 map to BARs 0-5 respectively */
> > #define PCI_EA_BEI_BAR0 0
> > #define PCI_EA_BEI_BAR5 5
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2018-11-29 13:30 ` sundeep subbaraya
@ 2018-11-29 15:03 ` Bjorn Helgaas
0 siblings, 0 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2018-11-29 15:03 UTC (permalink / raw)
To: sundeep subbaraya
Cc: linux-pci, linux-kernel, sean.stalley, sgoutham, Subbaraya Sundeep
On Thu, Nov 29, 2018 at 07:00:14PM +0530, sundeep subbaraya wrote:
> On Thu, Nov 29, 2018 at 3:25 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Nov 19, 2018 at 06:44:32PM +0530, sundeep.lkml@gmail.com wrote:
> > > From: Subbaraya Sundeep <sbhatta@marvell.com>
> > >
> > > As per the spec, bridges with EA capability work
> > > with fixed secondary and subordinate bus numbers.
> > > Hence assign bus numbers to bridges from EA if the
> > > capability exists.
> >
> > A reference to the spec section would be good, i.e., PCIe r4.0, sec xxx.
> >
> Ok. I referred ECN 2014 section 6.9.1.2.
>
> > > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> > > ---
> > > Changes for v2:
> > > No changes just added Sean Stalley who did EA support for BARs.
> > >
> > > drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> > > include/uapi/linux/pci_regs.h | 6 +++++
> > > 2 files changed, 60 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > > index b1c05b5..f41d2e6 100644
> > > --- a/drivers/pci/probe.c
> > > +++ b/drivers/pci/probe.c
> > > @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
> > >
> > > static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> > > unsigned int available_buses);
> > > +/*
> > > + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> > > + * numbers from EA capability.
> > > + * @dev: Bridge with EA
> > > + * @secondary: updated with secondary bus number in EA
> > > + * @subordinate: updated with subordinate bus number in EA
> > > + *
> > > + * If it is a bridge with EA capability then fixed bus numbers are
> > > + * read from EA capability list and secondary, subordinate reference
> > > + * variables will be updated. Otherwise secondary and subordinate reference
> > > + * variables will be zeroed.
> > > + */
> > > +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> > > + u8 *subordinate)
> > > +{
> > > + int ea;
> > > + int offset;
> > > + u32 dw;
> > > +
> > > + *secondary = *subordinate = 0;
> > > +
> > > + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> > > + return;
> > > +
> > > + /* find PCI EA capability in list */
> > > + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> > > + if (!ea)
> > > + return;
> > > +
> > > + offset = ea + PCI_EA_FIRST_ENT;
> > > + pci_read_config_dword(dev, offset, &dw);
> >
> > "Num Entries" in the first DW of the capability is allowed to be zero,
> > in which case this word (the second DW) is invalid. [See comments
> > below; this code would be valid based on the 2014 ECN, but not per the
> > 2017 PCIe r4.0 spec]
> >
> Yes but Entries follow after first DW of EA capability for devices and
> after second
> DW for bridges.
> 2014 ECN says for bridges DW2 of EA must be present:
> "For Type 1 functions only, there is a second DW in the capability,
> preceding the first entry.
> This second DW must be included in the Enhanced Allocation Capability
> whenever this
> capability is implemented in a Type 1 Function"
> So for normal device EA DW1, Entries(if any)
> for bridges EA DW1,EA DW2, Entries(if any)
>
> > It would be much better if this function could be somehow incorporated
> > into pci_ea_init(), which already knows how to parse much of the EA
> > capability.
> >
> I initially thought of this but didn't do it to avoid new members in pci_dev.
>
> > > + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> > > + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> > > +}
> > >
> > > /*
> > > * pci_scan_bridge_extend() - Scan buses behind a bridge
> > > @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > > u16 bctl;
> > > u8 primary, secondary, subordinate;
> > > int broken = 0;
> > > + u8 fixed_sec, fixed_sub;
> > > + int next_busnr;
> > >
> > > /*
> > > * Make sure the bridge is powered on to be able to access config
> > > @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > > /* Clear errors */
> > > pci_write_config_word(dev, PCI_STATUS, 0xffff);
> > >
> > > + /* read bus numbers from EA */
> > > + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> > > +
> > > + next_busnr = max + 1;
> > > + /* Use secondary bus number in EA */
> > > + if (fixed_sec)
> > > + next_busnr = fixed_sec;
> > > +
> > > /*
> > > * Prevent assigning a bus number that already exists.
> > > * This can happen when a bridge is hot-plugged, so in this
> > > * case we only re-scan this bus.
> > > */
> > > - child = pci_find_bus(pci_domain_nr(bus), max+1);
> > > + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> > > if (!child) {
> > > - child = pci_add_new_bus(bus, dev, max+1);
> > > + child = pci_add_new_bus(bus, dev, next_busnr);
> > > if (!child)
> > > goto out;
> > > - pci_bus_insert_busn_res(child, max+1,
> > > + pci_bus_insert_busn_res(child, next_busnr,
> > > bus->busn_res.end);
> > > }
> > > max++;
> > > @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > > max += i;
> > > }
> > >
> > > - /* Set subordinate bus number to its real value */
> > > + /*
> > > + * Set subordinate bus number to its real value.
> > > + * If fixed subordinate bus number exists from EA
> > > + * capability then use it.
> > > + */
> > > + if (fixed_sub)
> > > + max = fixed_sub;
> > > pci_bus_update_busn_res_end(child, max);
> > > pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> > > }
> > > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > > index e1e9888..c3d0904 100644
> > > --- a/include/uapi/linux/pci_regs.h
> > > +++ b/include/uapi/linux/pci_regs.h
> > > @@ -372,6 +372,12 @@
> > > #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> >
> > You didn't add PCI_EA_FIRST_ENT_BRIDGE (Sean did with f80b0ba95964
> > ("PCI: Add Enhanced Allocation register entries")), but it is unused
> > and I can't match it with anything in the spec (PCIe r4.0, sec 7.8.5).
> >
> > It might be related to the code in pci_ea_init() that skips DWORD 2
> > for type 1 functions. But I still don't see that in the spec.
> >
> > If it's obsolete, we should remove it (with a separate patch).
> >
> PCI_EA_FIRST_ENT_BRIDGE is not needed we can remove.
> I will send a patch to remove it.
> I will add new members for fixed secondary and subordinate bus numbers in
> pci_dev and make changes as below please let me know it is okay for you:
> @@ -2909,6 +2909,7 @@ void pci_ea_init(struct pci_dev *dev)
> u8 num_ent;
> int offset;
> int i;
> + u32 dw;
>
> /* find PCI EA capability in list */
> ea = pci_find_capability(dev, PCI_CAP_ID_EA); @@ -2922,9
> +2923,14 @@ void pci_ea_init(struct pci_dev *dev)
> offset = ea + PCI_EA_FIRST_ENT;
> - /* Skip DWORD 2 for type 1 functions */
> - if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE)
> + /* Note fixed bus numbers for type 1 functions */
> + if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) {
> + pci_read_config_dword(dev, offset, &dw);
> + dev->fixed_sec_busnr = dw & PCI_EA_FIXED_SEC_BUS;
> + dev->fixed_sub_busnr = (dw & PCI_EA_FIXED_SUB_BUS) >>
> + PCI_EA_FIXED_SUB_SHIFT;
> offset += 4;
> + }
>
> > Hmm, I do see this in the ECN dated 23 Oct 2014, sec 6.9.1.2. But
> > presumably that would be incorporated in PCIe r4.0, which is dated 27
> > Sep 2017.
> >
> > I have no idea what to do with this. I can't merge this without some
> > clarification here,
> >
> Yeah I see there is no mention of EA for bridges in r4.0.
> Since we check whether the device is bridge or not before reading DW2 of EA
> I guess we are okay.
I don't think so. We need at least some plan to clarify this in the
spec. I think Sean is plugged into the PCI-SIG, so maybe he can help
with that?
Bjorn
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2018-11-19 13:14 [PATCH v2] PCI: assign bus numbers present in EA capability for bridges sundeep.lkml
2018-11-26 6:40 ` sundeep subbaraya
2018-11-28 21:55 ` Bjorn Helgaas
@ 2019-04-17 20:16 ` Bjorn Helgaas
2019-04-18 6:48 ` sundeep subbaraya
2019-06-27 15:05 ` sundeep subbaraya
2 siblings, 2 replies; 8+ messages in thread
From: Bjorn Helgaas @ 2019-04-17 20:16 UTC (permalink / raw)
To: sundeep.lkml
Cc: linux-pci, linux-kernel, sean.stalley, sgoutham, Subbaraya Sundeep
On Mon, Nov 19, 2018 at 06:44:32PM +0530, sundeep.lkml@gmail.com wrote:
> From: Subbaraya Sundeep <sbhatta@marvell.com>
>
> As per the spec, bridges with EA capability work
> with fixed secondary and subordinate bus numbers.
> Hence assign bus numbers to bridges from EA if the
> capability exists.
>
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
I applied this with minor revisions to pci/enumeration for v5.2,
thanks!
> ---
> Changes for v2:
> No changes just added Sean Stalley who did EA support for BARs.
>
> drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> include/uapi/linux/pci_regs.h | 6 +++++
> 2 files changed, 60 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index b1c05b5..f41d2e6 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
>
> static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> unsigned int available_buses);
> +/*
> + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> + * numbers from EA capability.
> + * @dev: Bridge with EA
> + * @secondary: updated with secondary bus number in EA
> + * @subordinate: updated with subordinate bus number in EA
> + *
> + * If it is a bridge with EA capability then fixed bus numbers are
> + * read from EA capability list and secondary, subordinate reference
> + * variables will be updated. Otherwise secondary and subordinate reference
> + * variables will be zeroed.
> + */
> +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> + u8 *subordinate)
> +{
> + int ea;
> + int offset;
> + u32 dw;
> +
> + *secondary = *subordinate = 0;
> +
> + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> + return;
> +
> + /* find PCI EA capability in list */
> + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> + if (!ea)
> + return;
> +
> + offset = ea + PCI_EA_FIRST_ENT;
> + pci_read_config_dword(dev, offset, &dw);
> + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> +}
>
> /*
> * pci_scan_bridge_extend() - Scan buses behind a bridge
> @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> u16 bctl;
> u8 primary, secondary, subordinate;
> int broken = 0;
> + u8 fixed_sec, fixed_sub;
> + int next_busnr;
>
> /*
> * Make sure the bridge is powered on to be able to access config
> @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> /* Clear errors */
> pci_write_config_word(dev, PCI_STATUS, 0xffff);
>
> + /* read bus numbers from EA */
> + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> +
> + next_busnr = max + 1;
> + /* Use secondary bus number in EA */
> + if (fixed_sec)
> + next_busnr = fixed_sec;
I initially thought this was not quite safe because EA could supply a
secondary bus number of 0, in which case we would erroneously ignore
it. But it's actually not possible for a PCI-to-PCI bridge to have a
secondary bus number of 0, so it *is* safe.
But it's still a little subtle and since I've already done the work to
add a bool return value from pci_ea_fixed_busnrs(), maybe it will be
OK to keep that. The patch as applied is below, let me know if you
have any comments.
> +
> /*
> * Prevent assigning a bus number that already exists.
> * This can happen when a bridge is hot-plugged, so in this
> * case we only re-scan this bus.
> */
> - child = pci_find_bus(pci_domain_nr(bus), max+1);
> + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> if (!child) {
> - child = pci_add_new_bus(bus, dev, max+1);
> + child = pci_add_new_bus(bus, dev, next_busnr);
> if (!child)
> goto out;
> - pci_bus_insert_busn_res(child, max+1,
> + pci_bus_insert_busn_res(child, next_busnr,
> bus->busn_res.end);
> }
> max++;
> @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> max += i;
> }
>
> - /* Set subordinate bus number to its real value */
> + /*
> + * Set subordinate bus number to its real value.
> + * If fixed subordinate bus number exists from EA
> + * capability then use it.
> + */
> + if (fixed_sub)
> + max = fixed_sub;
> pci_bus_update_busn_res_end(child, max);
> pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e1e9888..c3d0904 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,12 @@
> #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> #define PCI_EA_ES 0x00000007 /* Entry Size */
> #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> +
> +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> +#define PCI_EA_SEC_BUS_MASK 0xff
> +#define PCI_EA_SUB_BUS_MASK 0xff00
> +#define PCI_EA_SUB_BUS_SHIFT 8
> +
> /* 0-5 map to BARs 0-5 respectively */
> #define PCI_EA_BEI_BAR0 0
> #define PCI_EA_BEI_BAR5 5
> --
> 1.8.3.1
>
commit 2dbce5901179
Author: Subbaraya Sundeep <sbhatta@marvell.com>
Date: Mon Nov 19 18:44:32 2018 +0530
PCI: Assign bus numbers present in EA capability for bridges
The "Enhanced Allocation (EA) for Memory and I/O Resources" ECN, approved
23 October 2014, sec 6.9.1.2, specifies a second DW in the capability for
type 1 (bridge) functions to describe fixed secondary and subordinate bus
numbers. This ECN was included in the PCIe r4.0 spec, but sec 6.9.1.2 was
omitted, presumably by mistake.
Read fixed bus numbers from the EA capability for bridges.
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
[bhelgaas: add pci_ea_fixed_busnrs() return value]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 012250a78da7..a6874c306908 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1086,6 +1086,36 @@ static void pci_enable_crs(struct pci_dev *pdev)
static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
unsigned int available_buses);
+/**
+ * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
+ * numbers from EA capability.
+ * @dev: Bridge
+ * @sec: updated with secondary bus number from EA
+ * @sub: updated with subordinate bus number from EA
+ *
+ * If @dev is a bridge with EA capability, update @sec and @sub with
+ * fixed bus numbers from the capability and return true. Otherwise,
+ * return false.
+ */
+static bool pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *sec, u8 *sub)
+{
+ int ea, offset;
+ u32 dw;
+
+ if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
+ return false;
+
+ /* find PCI EA capability in list */
+ ea = pci_find_capability(dev, PCI_CAP_ID_EA);
+ if (!ea)
+ return false;
+
+ offset = ea + PCI_EA_FIRST_ENT;
+ pci_read_config_dword(dev, offset, &dw);
+ *sec = dw & PCI_EA_SEC_BUS_MASK;
+ *sub = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
+ return true;
+}
/*
* pci_scan_bridge_extend() - Scan buses behind a bridge
@@ -1120,6 +1150,9 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
u16 bctl;
u8 primary, secondary, subordinate;
int broken = 0;
+ bool fixed_buses;
+ u8 fixed_sec, fixed_sub;
+ int next_busnr;
/*
* Make sure the bridge is powered on to be able to access config
@@ -1219,17 +1252,24 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
/* Clear errors */
pci_write_config_word(dev, PCI_STATUS, 0xffff);
+ /* Read bus numbers from EA Capability (if present) */
+ fixed_buses = pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
+ if (fixed_buses)
+ next_busnr = fixed_sec;
+ else
+ next_busnr = max + 1;
+
/*
* Prevent assigning a bus number that already exists.
* This can happen when a bridge is hot-plugged, so in this
* case we only re-scan this bus.
*/
- child = pci_find_bus(pci_domain_nr(bus), max+1);
+ child = pci_find_bus(pci_domain_nr(bus), next_busnr);
if (!child) {
- child = pci_add_new_bus(bus, dev, max+1);
+ child = pci_add_new_bus(bus, dev, next_busnr);
if (!child)
goto out;
- pci_bus_insert_busn_res(child, max+1,
+ pci_bus_insert_busn_res(child, next_busnr,
bus->busn_res.end);
}
max++;
@@ -1290,7 +1330,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
max += i;
}
- /* Set subordinate bus number to its real value */
+ /*
+ * Set subordinate bus number to its real value.
+ * If fixed subordinate bus number exists from EA
+ * capability then use it.
+ */
+ if (fixed_buses)
+ max = fixed_sub;
pci_bus_update_busn_res_end(child, max);
pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
}
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index 5c98133f2c94..c51e0066de8b 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -372,6 +372,12 @@
#define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
#define PCI_EA_ES 0x00000007 /* Entry Size */
#define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
+
+/* EA fixed Secondary and Subordinate bus numbers for Bridge */
+#define PCI_EA_SEC_BUS_MASK 0xff
+#define PCI_EA_SUB_BUS_MASK 0xff00
+#define PCI_EA_SUB_BUS_SHIFT 8
+
/* 0-5 map to BARs 0-5 respectively */
#define PCI_EA_BEI_BAR0 0
#define PCI_EA_BEI_BAR5 5
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2019-04-17 20:16 ` Bjorn Helgaas
@ 2019-04-18 6:48 ` sundeep subbaraya
2019-06-27 15:05 ` sundeep subbaraya
1 sibling, 0 replies; 8+ messages in thread
From: sundeep subbaraya @ 2019-04-18 6:48 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, Stalley, Sean, sgoutham, Subbaraya Sundeep
Hi Bjorn,
On Thu, Apr 18, 2019 at 1:46 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Nov 19, 2018 at 06:44:32PM +0530, sundeep.lkml@gmail.com wrote:
> > From: Subbaraya Sundeep <sbhatta@marvell.com>
> >
> > As per the spec, bridges with EA capability work
> > with fixed secondary and subordinate bus numbers.
> > Hence assign bus numbers to bridges from EA if the
> > capability exists.
> >
> > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
>
> I applied this with minor revisions to pci/enumeration for v5.2,
> thanks!
>
> > ---
> > Changes for v2:
> > No changes just added Sean Stalley who did EA support for BARs.
> >
> > drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> > include/uapi/linux/pci_regs.h | 6 +++++
> > 2 files changed, 60 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index b1c05b5..f41d2e6 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
> >
> > static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> > unsigned int available_buses);
> > +/*
> > + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> > + * numbers from EA capability.
> > + * @dev: Bridge with EA
> > + * @secondary: updated with secondary bus number in EA
> > + * @subordinate: updated with subordinate bus number in EA
> > + *
> > + * If it is a bridge with EA capability then fixed bus numbers are
> > + * read from EA capability list and secondary, subordinate reference
> > + * variables will be updated. Otherwise secondary and subordinate reference
> > + * variables will be zeroed.
> > + */
> > +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> > + u8 *subordinate)
> > +{
> > + int ea;
> > + int offset;
> > + u32 dw;
> > +
> > + *secondary = *subordinate = 0;
> > +
> > + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> > + return;
> > +
> > + /* find PCI EA capability in list */
> > + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> > + if (!ea)
> > + return;
> > +
> > + offset = ea + PCI_EA_FIRST_ENT;
> > + pci_read_config_dword(dev, offset, &dw);
> > + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> > + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> > +}
> >
> > /*
> > * pci_scan_bridge_extend() - Scan buses behind a bridge
> > @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > u16 bctl;
> > u8 primary, secondary, subordinate;
> > int broken = 0;
> > + u8 fixed_sec, fixed_sub;
> > + int next_busnr;
> >
> > /*
> > * Make sure the bridge is powered on to be able to access config
> > @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > /* Clear errors */
> > pci_write_config_word(dev, PCI_STATUS, 0xffff);
> >
> > + /* read bus numbers from EA */
> > + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> > +
> > + next_busnr = max + 1;
> > + /* Use secondary bus number in EA */
> > + if (fixed_sec)
> > + next_busnr = fixed_sec;
>
> I initially thought this was not quite safe because EA could supply a
> secondary bus number of 0, in which case we would erroneously ignore
> it. But it's actually not possible for a PCI-to-PCI bridge to have a
> secondary bus number of 0, so it *is* safe.
>
> But it's still a little subtle and since I've already done the work to
> add a bool return value from pci_ea_fixed_busnrs(), maybe it will be
> OK to keep that. The patch as applied is below, let me know if you
> have any comments.
>
Your changes looks good. Thanks for applying the patch.
Sundeep
> > +
> > /*
> > * Prevent assigning a bus number that already exists.
> > * This can happen when a bridge is hot-plugged, so in this
> > * case we only re-scan this bus.
> > */
> > - child = pci_find_bus(pci_domain_nr(bus), max+1);
> > + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> > if (!child) {
> > - child = pci_add_new_bus(bus, dev, max+1);
> > + child = pci_add_new_bus(bus, dev, next_busnr);
> > if (!child)
> > goto out;
> > - pci_bus_insert_busn_res(child, max+1,
> > + pci_bus_insert_busn_res(child, next_busnr,
> > bus->busn_res.end);
> > }
> > max++;
> > @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > max += i;
> > }
> >
> > - /* Set subordinate bus number to its real value */
> > + /*
> > + * Set subordinate bus number to its real value.
> > + * If fixed subordinate bus number exists from EA
> > + * capability then use it.
> > + */
> > + if (fixed_sub)
> > + max = fixed_sub;
> > pci_bus_update_busn_res_end(child, max);
> > pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> > }
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index e1e9888..c3d0904 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -372,6 +372,12 @@
> > #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> > #define PCI_EA_ES 0x00000007 /* Entry Size */
> > #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> > +
> > +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> > +#define PCI_EA_SEC_BUS_MASK 0xff
> > +#define PCI_EA_SUB_BUS_MASK 0xff00
> > +#define PCI_EA_SUB_BUS_SHIFT 8
> > +
> > /* 0-5 map to BARs 0-5 respectively */
> > #define PCI_EA_BEI_BAR0 0
> > #define PCI_EA_BEI_BAR5 5
> > --
> > 1.8.3.1
> >
>
> commit 2dbce5901179
> Author: Subbaraya Sundeep <sbhatta@marvell.com>
> Date: Mon Nov 19 18:44:32 2018 +0530
>
> PCI: Assign bus numbers present in EA capability for bridges
>
> The "Enhanced Allocation (EA) for Memory and I/O Resources" ECN, approved
> 23 October 2014, sec 6.9.1.2, specifies a second DW in the capability for
> type 1 (bridge) functions to describe fixed secondary and subordinate bus
> numbers. This ECN was included in the PCIe r4.0 spec, but sec 6.9.1.2 was
> omitted, presumably by mistake.
>
> Read fixed bus numbers from the EA capability for bridges.
>
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> [bhelgaas: add pci_ea_fixed_busnrs() return value]
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 012250a78da7..a6874c306908 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1086,6 +1086,36 @@ static void pci_enable_crs(struct pci_dev *pdev)
>
> static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> unsigned int available_buses);
> +/**
> + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> + * numbers from EA capability.
> + * @dev: Bridge
> + * @sec: updated with secondary bus number from EA
> + * @sub: updated with subordinate bus number from EA
> + *
> + * If @dev is a bridge with EA capability, update @sec and @sub with
> + * fixed bus numbers from the capability and return true. Otherwise,
> + * return false.
> + */
> +static bool pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *sec, u8 *sub)
> +{
> + int ea, offset;
> + u32 dw;
> +
> + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> + return false;
> +
> + /* find PCI EA capability in list */
> + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> + if (!ea)
> + return false;
> +
> + offset = ea + PCI_EA_FIRST_ENT;
> + pci_read_config_dword(dev, offset, &dw);
> + *sec = dw & PCI_EA_SEC_BUS_MASK;
> + *sub = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> + return true;
> +}
>
> /*
> * pci_scan_bridge_extend() - Scan buses behind a bridge
> @@ -1120,6 +1150,9 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> u16 bctl;
> u8 primary, secondary, subordinate;
> int broken = 0;
> + bool fixed_buses;
> + u8 fixed_sec, fixed_sub;
> + int next_busnr;
>
> /*
> * Make sure the bridge is powered on to be able to access config
> @@ -1219,17 +1252,24 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> /* Clear errors */
> pci_write_config_word(dev, PCI_STATUS, 0xffff);
>
> + /* Read bus numbers from EA Capability (if present) */
> + fixed_buses = pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> + if (fixed_buses)
> + next_busnr = fixed_sec;
> + else
> + next_busnr = max + 1;
> +
> /*
> * Prevent assigning a bus number that already exists.
> * This can happen when a bridge is hot-plugged, so in this
> * case we only re-scan this bus.
> */
> - child = pci_find_bus(pci_domain_nr(bus), max+1);
> + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> if (!child) {
> - child = pci_add_new_bus(bus, dev, max+1);
> + child = pci_add_new_bus(bus, dev, next_busnr);
> if (!child)
> goto out;
> - pci_bus_insert_busn_res(child, max+1,
> + pci_bus_insert_busn_res(child, next_busnr,
> bus->busn_res.end);
> }
> max++;
> @@ -1290,7 +1330,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> max += i;
> }
>
> - /* Set subordinate bus number to its real value */
> + /*
> + * Set subordinate bus number to its real value.
> + * If fixed subordinate bus number exists from EA
> + * capability then use it.
> + */
> + if (fixed_buses)
> + max = fixed_sub;
> pci_bus_update_busn_res_end(child, max);
> pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 5c98133f2c94..c51e0066de8b 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,12 @@
> #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> #define PCI_EA_ES 0x00000007 /* Entry Size */
> #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> +
> +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> +#define PCI_EA_SEC_BUS_MASK 0xff
> +#define PCI_EA_SUB_BUS_MASK 0xff00
> +#define PCI_EA_SUB_BUS_SHIFT 8
> +
> /* 0-5 map to BARs 0-5 respectively */
> #define PCI_EA_BEI_BAR0 0
> #define PCI_EA_BEI_BAR5 5
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] PCI: assign bus numbers present in EA capability for bridges
2019-04-17 20:16 ` Bjorn Helgaas
2019-04-18 6:48 ` sundeep subbaraya
@ 2019-06-27 15:05 ` sundeep subbaraya
1 sibling, 0 replies; 8+ messages in thread
From: sundeep subbaraya @ 2019-06-27 15:05 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci, linux-kernel, Stalley, Sean, sgoutham, Subbaraya Sundeep
Hi Bjorn,
On Thu, Apr 18, 2019 at 1:46 AM Bjorn Helgaas <helgaas@kernel.org> wrote:
>
> On Mon, Nov 19, 2018 at 06:44:32PM +0530, sundeep.lkml@gmail.com wrote:
> > From: Subbaraya Sundeep <sbhatta@marvell.com>
> >
> > As per the spec, bridges with EA capability work
> > with fixed secondary and subordinate bus numbers.
> > Hence assign bus numbers to bridges from EA if the
> > capability exists.
> >
> > Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
>
> I applied this with minor revisions to pci/enumeration for v5.2,
> thanks!
>
> > ---
> > Changes for v2:
> > No changes just added Sean Stalley who did EA support for BARs.
> >
> > drivers/pci/probe.c | 58 ++++++++++++++++++++++++++++++++++++++++---
> > include/uapi/linux/pci_regs.h | 6 +++++
> > 2 files changed, 60 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> > index b1c05b5..f41d2e6 100644
> > --- a/drivers/pci/probe.c
> > +++ b/drivers/pci/probe.c
> > @@ -1030,6 +1030,40 @@ static void pci_enable_crs(struct pci_dev *pdev)
> >
> > static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> > unsigned int available_buses);
> > +/*
> > + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> > + * numbers from EA capability.
> > + * @dev: Bridge with EA
> > + * @secondary: updated with secondary bus number in EA
> > + * @subordinate: updated with subordinate bus number in EA
> > + *
> > + * If it is a bridge with EA capability then fixed bus numbers are
> > + * read from EA capability list and secondary, subordinate reference
> > + * variables will be updated. Otherwise secondary and subordinate reference
> > + * variables will be zeroed.
> > + */
> > +static void pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *secondary,
> > + u8 *subordinate)
> > +{
> > + int ea;
> > + int offset;
> > + u32 dw;
> > +
> > + *secondary = *subordinate = 0;
> > +
> > + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> > + return;
> > +
> > + /* find PCI EA capability in list */
> > + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> > + if (!ea)
> > + return;
> > +
> > + offset = ea + PCI_EA_FIRST_ENT;
> > + pci_read_config_dword(dev, offset, &dw);
> > + *secondary = dw & PCI_EA_SEC_BUS_MASK;
> > + *subordinate = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> > +}
> >
> > /*
> > * pci_scan_bridge_extend() - Scan buses behind a bridge
> > @@ -1064,6 +1098,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > u16 bctl;
> > u8 primary, secondary, subordinate;
> > int broken = 0;
> > + u8 fixed_sec, fixed_sub;
> > + int next_busnr;
> >
> > /*
> > * Make sure the bridge is powered on to be able to access config
> > @@ -1163,17 +1199,25 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > /* Clear errors */
> > pci_write_config_word(dev, PCI_STATUS, 0xffff);
> >
> > + /* read bus numbers from EA */
> > + pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> > +
> > + next_busnr = max + 1;
> > + /* Use secondary bus number in EA */
> > + if (fixed_sec)
> > + next_busnr = fixed_sec;
>
> I initially thought this was not quite safe because EA could supply a
> secondary bus number of 0, in which case we would erroneously ignore
> it. But it's actually not possible for a PCI-to-PCI bridge to have a
When EA supplies secondary bus number as 0 then we
should proceed with normal sequence where we scan with max + 1
as bus number for next round. Otherwise subordinate bus number
will always be returning zero.
As per the spec, regd. Fixed Secondary Bus Number and
Fixed Subordinate Bus Number:
"If at least one Function that uses EA is located behind this function,
then this field must be set to indicate the bus number of the PCI bus segment
to which the secondary interface of this Function is connected.
If no Function that uses EA is located behind this function,
then this field must be set to all 0’s."
which implies that normal enumeration should take place behind this
last EA bridge.
Shall I revert your changes which returns bool ?
Or simply add a check if (fixed_buses && fixed_sec) ?
Sorry for the trouble.
Thanks,
Sundeep
> secondary bus number of 0, so it *is* safe.
>
> But it's still a little subtle and since I've already done the work to
> add a bool return value from pci_ea_fixed_busnrs(), maybe it will be
> OK to keep that. The patch as applied is below, let me know if you
> have any comments.
>
> > +
> > /*
> > * Prevent assigning a bus number that already exists.
> > * This can happen when a bridge is hot-plugged, so in this
> > * case we only re-scan this bus.
> > */
> > - child = pci_find_bus(pci_domain_nr(bus), max+1);
> > + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> > if (!child) {
> > - child = pci_add_new_bus(bus, dev, max+1);
> > + child = pci_add_new_bus(bus, dev, next_busnr);
> > if (!child)
> > goto out;
> > - pci_bus_insert_busn_res(child, max+1,
> > + pci_bus_insert_busn_res(child, next_busnr,
> > bus->busn_res.end);
> > }
> > max++;
> > @@ -1234,7 +1278,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> > max += i;
> > }
> >
> > - /* Set subordinate bus number to its real value */
> > + /*
> > + * Set subordinate bus number to its real value.
> > + * If fixed subordinate bus number exists from EA
> > + * capability then use it.
> > + */
> > + if (fixed_sub)
> > + max = fixed_sub;
> > pci_bus_update_busn_res_end(child, max);
> > pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> > }
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index e1e9888..c3d0904 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -372,6 +372,12 @@
> > #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> > #define PCI_EA_ES 0x00000007 /* Entry Size */
> > #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> > +
> > +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> > +#define PCI_EA_SEC_BUS_MASK 0xff
> > +#define PCI_EA_SUB_BUS_MASK 0xff00
> > +#define PCI_EA_SUB_BUS_SHIFT 8
> > +
> > /* 0-5 map to BARs 0-5 respectively */
> > #define PCI_EA_BEI_BAR0 0
> > #define PCI_EA_BEI_BAR5 5
> > --
> > 1.8.3.1
> >
>
> commit 2dbce5901179
> Author: Subbaraya Sundeep <sbhatta@marvell.com>
> Date: Mon Nov 19 18:44:32 2018 +0530
>
> PCI: Assign bus numbers present in EA capability for bridges
>
> The "Enhanced Allocation (EA) for Memory and I/O Resources" ECN, approved
> 23 October 2014, sec 6.9.1.2, specifies a second DW in the capability for
> type 1 (bridge) functions to describe fixed secondary and subordinate bus
> numbers. This ECN was included in the PCIe r4.0 spec, but sec 6.9.1.2 was
> omitted, presumably by mistake.
>
> Read fixed bus numbers from the EA capability for bridges.
>
> Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
> [bhelgaas: add pci_ea_fixed_busnrs() return value]
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 012250a78da7..a6874c306908 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1086,6 +1086,36 @@ static void pci_enable_crs(struct pci_dev *pdev)
>
> static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
> unsigned int available_buses);
> +/**
> + * pci_ea_fixed_busnrs() - Read fixed Secondary and Subordinate bus
> + * numbers from EA capability.
> + * @dev: Bridge
> + * @sec: updated with secondary bus number from EA
> + * @sub: updated with subordinate bus number from EA
> + *
> + * If @dev is a bridge with EA capability, update @sec and @sub with
> + * fixed bus numbers from the capability and return true. Otherwise,
> + * return false.
> + */
> +static bool pci_ea_fixed_busnrs(struct pci_dev *dev, u8 *sec, u8 *sub)
> +{
> + int ea, offset;
> + u32 dw;
> +
> + if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE)
> + return false;
> +
> + /* find PCI EA capability in list */
> + ea = pci_find_capability(dev, PCI_CAP_ID_EA);
> + if (!ea)
> + return false;
> +
> + offset = ea + PCI_EA_FIRST_ENT;
> + pci_read_config_dword(dev, offset, &dw);
> + *sec = dw & PCI_EA_SEC_BUS_MASK;
> + *sub = (dw & PCI_EA_SUB_BUS_MASK) >> PCI_EA_SUB_BUS_SHIFT;
> + return true;
> +}
>
> /*
> * pci_scan_bridge_extend() - Scan buses behind a bridge
> @@ -1120,6 +1150,9 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> u16 bctl;
> u8 primary, secondary, subordinate;
> int broken = 0;
> + bool fixed_buses;
> + u8 fixed_sec, fixed_sub;
> + int next_busnr;
>
> /*
> * Make sure the bridge is powered on to be able to access config
> @@ -1219,17 +1252,24 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> /* Clear errors */
> pci_write_config_word(dev, PCI_STATUS, 0xffff);
>
> + /* Read bus numbers from EA Capability (if present) */
> + fixed_buses = pci_ea_fixed_busnrs(dev, &fixed_sec, &fixed_sub);
> + if (fixed_buses)
> + next_busnr = fixed_sec;
> + else
> + next_busnr = max + 1;
> +
> /*
> * Prevent assigning a bus number that already exists.
> * This can happen when a bridge is hot-plugged, so in this
> * case we only re-scan this bus.
> */
> - child = pci_find_bus(pci_domain_nr(bus), max+1);
> + child = pci_find_bus(pci_domain_nr(bus), next_busnr);
> if (!child) {
> - child = pci_add_new_bus(bus, dev, max+1);
> + child = pci_add_new_bus(bus, dev, next_busnr);
> if (!child)
> goto out;
> - pci_bus_insert_busn_res(child, max+1,
> + pci_bus_insert_busn_res(child, next_busnr,
> bus->busn_res.end);
> }
> max++;
> @@ -1290,7 +1330,13 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
> max += i;
> }
>
> - /* Set subordinate bus number to its real value */
> + /*
> + * Set subordinate bus number to its real value.
> + * If fixed subordinate bus number exists from EA
> + * capability then use it.
> + */
> + if (fixed_buses)
> + max = fixed_sub;
> pci_bus_update_busn_res_end(child, max);
> pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
> }
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 5c98133f2c94..c51e0066de8b 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,12 @@
> #define PCI_EA_FIRST_ENT_BRIDGE 8 /* First EA Entry for Bridges */
> #define PCI_EA_ES 0x00000007 /* Entry Size */
> #define PCI_EA_BEI 0x000000f0 /* BAR Equivalent Indicator */
> +
> +/* EA fixed Secondary and Subordinate bus numbers for Bridge */
> +#define PCI_EA_SEC_BUS_MASK 0xff
> +#define PCI_EA_SUB_BUS_MASK 0xff00
> +#define PCI_EA_SUB_BUS_SHIFT 8
> +
> /* 0-5 map to BARs 0-5 respectively */
> #define PCI_EA_BEI_BAR0 0
> #define PCI_EA_BEI_BAR5 5
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-06-27 15:05 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-19 13:14 [PATCH v2] PCI: assign bus numbers present in EA capability for bridges sundeep.lkml
2018-11-26 6:40 ` sundeep subbaraya
2018-11-28 21:55 ` Bjorn Helgaas
2018-11-29 13:30 ` sundeep subbaraya
2018-11-29 15:03 ` Bjorn Helgaas
2019-04-17 20:16 ` Bjorn Helgaas
2019-04-18 6:48 ` sundeep subbaraya
2019-06-27 15:05 ` sundeep subbaraya
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).