From: Subbaraya Sundeep <[email protected]>
As per the spec - ECN_Enhanced_Allocation_23_Oct_2014_Final
and section 6.9.1.2, EA capability contains fixed secondary
and subordinate bus numbers for type 1 functions.
This patch adds support to read the fixed bus numbers
from EA capability for bridge.
Signed-off-by: Subbaraya Sundeep <[email protected]>
---
v3:
As per Bjorn's suggestion placed EA stuff in pci_ea_init and
captured bus numbers in pci_dev
v2:
None just added Sean
drivers/pci/pci.c | 10 ++++++++--
include/linux/pci.h | 4 ++++
include/uapi/linux/pci_regs.h | 4 ++++
3 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c25acac..484b63e 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -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;
+ }
/* parse each EA entry */
for (i = 0; i < num_ent; ++i)
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 65f1d8c..3e9a3ae 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -459,6 +459,10 @@ struct pci_dev {
char *driver_override; /* Driver name to force a match */
unsigned long priv_flags; /* Private flags for the PCI driver */
+
+ /* bus numbers from EA capability if this device is a bridge */
+ u8 fixed_sec_busnr; /* Fixed Secondary Bus number */
+ u8 fixed_sub_busnr; /* Fixed Subordinate Bus number */
};
static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
index e1e9888..51e9ac0 100644
--- a/include/uapi/linux/pci_regs.h
+++ b/include/uapi/linux/pci_regs.h
@@ -372,6 +372,10 @@
#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 */
+/* Fixed Secondary and Subordinate bus numbers in EA for Bridge */
+#define PCI_EA_FIXED_SEC_BUS 0xff
+#define PCI_EA_FIXED_SUB_BUS 0xff00
+#define PCI_EA_FIXED_SUB_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
From: Subbaraya Sundeep <[email protected]>
As per the spec - ECN_Enhanced_Allocation_23_Oct_2014_Final
and section 6.9.1.2, bridges with EA capability work with fixed
secondary and subordinate bus numbers. Hence consider assigning
bus numbers to bridges from EA if the capability exists during
the scan.
Signed-off-by: Subbaraya Sundeep <[email protected]>
---
v3:
removed function for reading fixed bus numbers
instead those were captured in pci_ea_init
v2:
None just added Sean
drivers/pci/probe.c | 19 +++++++++++++++----
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 257b9f6..9215e2e 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1064,6 +1064,7 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
u16 bctl;
u8 primary, secondary, subordinate;
int broken = 0;
+ int next_busnr;
/*
* Make sure the bridge is powered on to be able to access config
@@ -1163,17 +1164,21 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, struct pci_dev *dev,
/* Clear errors */
pci_write_config_word(dev, PCI_STATUS, 0xffff);
+ next_busnr = max + 1;
+ /* Fixed secondary bus number from EA capability */
+ if (dev->fixed_sec_busnr)
+ next_busnr = dev->fixed_sec_busnr;
/*
* 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 +1239,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 (dev->fixed_sub_busnr)
+ max = dev->fixed_sub_busnr;
pci_bus_update_busn_res_end(child, max);
pci_write_config_byte(dev, PCI_SUBORDINATE_BUS, max);
}
--
1.8.3.1
Hi Bjorn,
Any comments?
Thanks,
Sundeep
On Wed, Jan 23, 2019 at 6:48 PM <[email protected]> wrote:
>
> From: Subbaraya Sundeep <[email protected]>
>
> As per the spec - ECN_Enhanced_Allocation_23_Oct_2014_Final
> and section 6.9.1.2, EA capability contains fixed secondary
> and subordinate bus numbers for type 1 functions.
> This patch adds support to read the fixed bus numbers
> from EA capability for bridge.
>
> Signed-off-by: Subbaraya Sundeep <[email protected]>
> ---
> v3:
> As per Bjorn's suggestion placed EA stuff in pci_ea_init and
> captured bus numbers in pci_dev
> v2:
> None just added Sean
>
> drivers/pci/pci.c | 10 ++++++++--
> include/linux/pci.h | 4 ++++
> include/uapi/linux/pci_regs.h | 4 ++++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index c25acac..484b63e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -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;
> + }
>
> /* parse each EA entry */
> for (i = 0; i < num_ent; ++i)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 65f1d8c..3e9a3ae 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -459,6 +459,10 @@ struct pci_dev {
> char *driver_override; /* Driver name to force a match */
>
> unsigned long priv_flags; /* Private flags for the PCI driver */
> +
> + /* bus numbers from EA capability if this device is a bridge */
> + u8 fixed_sec_busnr; /* Fixed Secondary Bus number */
> + u8 fixed_sub_busnr; /* Fixed Subordinate Bus number */
> };
>
> static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e1e9888..51e9ac0 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,10 @@
> #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 */
> +/* Fixed Secondary and Subordinate bus numbers in EA for Bridge */
> +#define PCI_EA_FIXED_SEC_BUS 0xff
> +#define PCI_EA_FIXED_SUB_BUS 0xff00
> +#define PCI_EA_FIXED_SUB_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
>
Ping.
Thanks,
Sundeep
On Tue, Feb 5, 2019 at 9:30 AM sundeep subbaraya <[email protected]> wrote:
>
> Hi Bjorn,
>
> Any comments?
>
> Thanks,
> Sundeep
>
> On Wed, Jan 23, 2019 at 6:48 PM <[email protected]> wrote:
> >
> > From: Subbaraya Sundeep <[email protected]>
> >
> > As per the spec - ECN_Enhanced_Allocation_23_Oct_2014_Final
> > and section 6.9.1.2, EA capability contains fixed secondary
> > and subordinate bus numbers for type 1 functions.
> > This patch adds support to read the fixed bus numbers
> > from EA capability for bridge.
> >
> > Signed-off-by: Subbaraya Sundeep <[email protected]>
> > ---
> > v3:
> > As per Bjorn's suggestion placed EA stuff in pci_ea_init and
> > captured bus numbers in pci_dev
> > v2:
> > None just added Sean
> >
> > drivers/pci/pci.c | 10 ++++++++--
> > include/linux/pci.h | 4 ++++
> > include/uapi/linux/pci_regs.h | 4 ++++
> > 3 files changed, 16 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index c25acac..484b63e 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -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;
> > + }
> >
> > /* parse each EA entry */
> > for (i = 0; i < num_ent; ++i)
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 65f1d8c..3e9a3ae 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -459,6 +459,10 @@ struct pci_dev {
> > char *driver_override; /* Driver name to force a match */
> >
> > unsigned long priv_flags; /* Private flags for the PCI driver */
> > +
> > + /* bus numbers from EA capability if this device is a bridge */
> > + u8 fixed_sec_busnr; /* Fixed Secondary Bus number */
> > + u8 fixed_sub_busnr; /* Fixed Subordinate Bus number */
> > };
> >
> > static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> > diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> > index e1e9888..51e9ac0 100644
> > --- a/include/uapi/linux/pci_regs.h
> > +++ b/include/uapi/linux/pci_regs.h
> > @@ -372,6 +372,10 @@
> > #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 */
> > +/* Fixed Secondary and Subordinate bus numbers in EA for Bridge */
> > +#define PCI_EA_FIXED_SEC_BUS 0xff
> > +#define PCI_EA_FIXED_SUB_BUS 0xff00
> > +#define PCI_EA_FIXED_SUB_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
> >
On Wed, Jan 23, 2019 at 06:48:00PM +0530, [email protected] wrote:
> From: Subbaraya Sundeep <[email protected]>
>
> As per the spec - ECN_Enhanced_Allocation_23_Oct_2014_Final
> and section 6.9.1.2, EA capability contains fixed secondary
> and subordinate bus numbers for type 1 functions.
> This patch adds support to read the fixed bus numbers
> from EA capability for bridge.
Section 6.9.1.2 "Enhanced Allocation Capability Second DW [Type 1
Functions Only]" is in the ECN (approved 23 Oct 2014), but is not in
the PCIe r4.0 spec, and is *STILL* not in the draft PCIe r5.0 spec, as
far as I can tell.
I thought we had a plan to get the spec updated? Maybe that was
wishful thinking on my part.
> Signed-off-by: Subbaraya Sundeep <[email protected]>
> ---
> v3:
> As per Bjorn's suggestion placed EA stuff in pci_ea_init and
> captured bus numbers in pci_dev
> v2:
> None just added Sean
>
> drivers/pci/pci.c | 10 ++++++++--
> include/linux/pci.h | 4 ++++
> include/uapi/linux/pci_regs.h | 4 ++++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index c25acac..484b63e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -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;
> + }
>
> /* parse each EA entry */
> for (i = 0; i < num_ent; ++i)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 65f1d8c..3e9a3ae 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -459,6 +459,10 @@ struct pci_dev {
> char *driver_override; /* Driver name to force a match */
>
> unsigned long priv_flags; /* Private flags for the PCI driver */
> +
> + /* bus numbers from EA capability if this device is a bridge */
> + u8 fixed_sec_busnr; /* Fixed Secondary Bus number */
> + u8 fixed_sub_busnr; /* Fixed Subordinate Bus number */
> };
>
> static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e1e9888..51e9ac0 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,10 @@
> #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 */
> +/* Fixed Secondary and Subordinate bus numbers in EA for Bridge */
> +#define PCI_EA_FIXED_SEC_BUS 0xff
> +#define PCI_EA_FIXED_SUB_BUS 0xff00
> +#define PCI_EA_FIXED_SUB_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
>
On Wed, Jan 23, 2019 at 06:48:00PM +0530, [email protected] wrote:
> From: Subbaraya Sundeep <[email protected]>
>
> As per the spec - ECN_Enhanced_Allocation_23_Oct_2014_Final
> and section 6.9.1.2, EA capability contains fixed secondary
> and subordinate bus numbers for type 1 functions.
> This patch adds support to read the fixed bus numbers
> from EA capability for bridge.
>
> Signed-off-by: Subbaraya Sundeep <[email protected]>
> ---
> v3:
> As per Bjorn's suggestion placed EA stuff in pci_ea_init and
> captured bus numbers in pci_dev
You were right the first time, and my idea of putting this stuff in
pci_ea_init() was well-intentioned but wrong. It doesn't seem
worthwhile to add those two fields to pci_dev when they will almost
always be unused.
I applied your v2 patch and revised it a little; I'll respond to that
patch so the comments make sense there.
> v2:
> None just added Sean
>
> drivers/pci/pci.c | 10 ++++++++--
> include/linux/pci.h | 4 ++++
> include/uapi/linux/pci_regs.h | 4 ++++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index c25acac..484b63e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -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;
> + }
>
> /* parse each EA entry */
> for (i = 0; i < num_ent; ++i)
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 65f1d8c..3e9a3ae 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -459,6 +459,10 @@ struct pci_dev {
> char *driver_override; /* Driver name to force a match */
>
> unsigned long priv_flags; /* Private flags for the PCI driver */
> +
> + /* bus numbers from EA capability if this device is a bridge */
> + u8 fixed_sec_busnr; /* Fixed Secondary Bus number */
> + u8 fixed_sub_busnr; /* Fixed Subordinate Bus number */
> };
>
> static inline struct pci_dev *pci_physfn(struct pci_dev *dev)
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index e1e9888..51e9ac0 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -372,6 +372,10 @@
> #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 */
> +/* Fixed Secondary and Subordinate bus numbers in EA for Bridge */
> +#define PCI_EA_FIXED_SEC_BUS 0xff
> +#define PCI_EA_FIXED_SUB_BUS 0xff00
> +#define PCI_EA_FIXED_SUB_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
>