| Message ID | 20190916204158.6889-7-efremov@linux.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [RESEND,v3,01/26] PCI: Add define for the number of standard PCI BARs | expand |
On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote: > Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use > global one PCI_STD_NUM_BARS instead. > > Acked-by: Sebastian Ott <sebott@linux.ibm.com> > Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com> > Signed-off-by: Denis Efremov <efremov@linux.com> > --- > arch/s390/include/asm/pci.h | 5 +---- > arch/s390/include/asm/pci_clp.h | 6 +++--- > arch/s390/pci/pci.c | 16 ++++++++-------- > arch/s390/pci/pci_clp.c | 6 +++--- > 4 files changed, 15 insertions(+), 18 deletions(-) > > diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h > index a2399eff84ca..3a06c264ea53 100644 > --- a/arch/s390/include/asm/pci.h > +++ b/arch/s390/include/asm/pci.h > @@ -2,9 +2,6 @@ > #ifndef __ASM_S390_PCI_H > #define __ASM_S390_PCI_H > > -/* must be set before including pci_clp.h */ > -#define PCI_BAR_COUNT 6 > - > #include <linux/pci.h> > #include <linux/mutex.h> > #include <linux/iommu.h> > @@ -138,7 +135,7 @@ struct zpci_dev { > > char res_name[16]; > bool mio_capable; > - struct zpci_bar_struct bars[PCI_BAR_COUNT]; > + struct zpci_bar_struct bars[PCI_STD_NUM_BARS]; > > u64 start_dma; /* Start of available DMA addresses */ > u64 end_dma; /* End of available DMA addresses */ > diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h > index 50359172cc48..bd2cb4ea7d93 100644 > --- a/arch/s390/include/asm/pci_clp.h > +++ b/arch/s390/include/asm/pci_clp.h > @@ -77,7 +77,7 @@ struct mio_info { > struct { > u64 wb; > u64 wt; > - } addr[PCI_BAR_COUNT]; > + } addr[PCI_STD_NUM_BARS]; > u32 reserved[6]; > } __packed; > > @@ -98,9 +98,9 @@ struct clp_rsp_query_pci { > u16 util_str_avail : 1; /* utility string available? */ > u16 pfgid : 8; /* pci function group id */ > u32 fid; /* pci function id */ > - u8 bar_size[PCI_BAR_COUNT]; > + u8 bar_size[PCI_STD_NUM_BARS]; > u16 pchid; > - __le32 bar[PCI_BAR_COUNT]; > + __le32 bar[PCI_STD_NUM_BARS]; > u8 pfip[CLP_PFIP_NR_SEGMENTS]; /* pci function internal path */ > u32 : 16; > u8 fmb_len; > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c > index b0e3b9a0e488..aca372c8e34f 100644 > --- a/arch/s390/pci/pci.c > +++ b/arch/s390/pci/pci.c > @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES); > static DEFINE_SPINLOCK(zpci_domain_lock); > > #define ZPCI_IOMAP_ENTRIES \ > - min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2), \ > + min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2), \ > ZPCI_IOMAP_MAX_ENTRIES) > > static DEFINE_SPINLOCK(zpci_iomap_lock); > @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar, > void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar, > unsigned long offset, unsigned long max) > { > - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) > + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) > return NULL; > > if (static_branch_likely(&have_mio)) > @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar, > void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar, > unsigned long offset, unsigned long max) > { > - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) > + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) > return NULL; This looks like a latent bug fix here. If 'bar' is out of range we return NULL instead accessing an invalid item of an array. Should this not be a separate patch and tagged as stable? Thanks, Andrew Murray > > if (static_branch_likely(&have_mio)) > @@ -416,7 +416,7 @@ static void zpci_map_resources(struct pci_dev *pdev) > resource_size_t len; > int i; > > - for (i = 0; i < PCI_BAR_COUNT; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > len = pci_resource_len(pdev, i); > if (!len) > continue; > @@ -451,7 +451,7 @@ static void zpci_unmap_resources(struct pci_dev *pdev) > if (zpci_use_mio(zdev)) > return; > > - for (i = 0; i < PCI_BAR_COUNT; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > len = pci_resource_len(pdev, i); > if (!len) > continue; > @@ -514,7 +514,7 @@ static int zpci_setup_bus_resources(struct zpci_dev *zdev, > snprintf(zdev->res_name, sizeof(zdev->res_name), > "PCI Bus %04x:%02x", zdev->domain, ZPCI_BUS_NR); > > - for (i = 0; i < PCI_BAR_COUNT; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > if (!zdev->bars[i].size) > continue; > entry = zpci_alloc_iomap(zdev); > @@ -551,7 +551,7 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev) > { > int i; > > - for (i = 0; i < PCI_BAR_COUNT; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > if (!zdev->bars[i].size || !zdev->bars[i].res) > continue; > > @@ -573,7 +573,7 @@ int pcibios_add_device(struct pci_dev *pdev) > pdev->dev.dma_ops = &s390_pci_dma_ops; > zpci_map_resources(pdev); > > - for (i = 0; i < PCI_BAR_COUNT; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > res = &pdev->resource[i]; > if (res->parent || !res->flags) > continue; > diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c > index 9bdff4defef1..8b729b5f2972 100644 > --- a/arch/s390/pci/pci_clp.c > +++ b/arch/s390/pci/pci_clp.c > @@ -145,7 +145,7 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, > { > int i; > > - for (i = 0; i < PCI_BAR_COUNT; i++) { > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > zdev->bars[i].val = le32_to_cpu(response->bar[i]); > zdev->bars[i].size = response->bar_size[i]; > } > @@ -164,8 +164,8 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, > sizeof(zdev->util_str)); > } > zdev->mio_capable = response->mio_addr_avail; > - for (i = 0; i < PCI_BAR_COUNT; i++) { > - if (!(response->mio.valid & (1 << (PCI_BAR_COUNT - i - 1)))) > + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > + if (!(response->mio.valid & (1 << (PCI_STD_NUM_BARS - i - 1)))) > continue; > > zdev->bars[i].mio_wb = (void __iomem *) response->mio.addr[i].wb; > -- > 2.21.0 >
On 9/18/19 11:58 AM, Andrew Murray wrote: > On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote: >> Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use >> global one PCI_STD_NUM_BARS instead. >> >> Acked-by: Sebastian Ott <sebott@linux.ibm.com> >> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com> >> Signed-off-by: Denis Efremov <efremov@linux.com> >> --- >> arch/s390/include/asm/pci.h | 5 +---- >> arch/s390/include/asm/pci_clp.h | 6 +++--- >> arch/s390/pci/pci.c | 16 ++++++++-------- >> arch/s390/pci/pci_clp.c | 6 +++--- >> 4 files changed, 15 insertions(+), 18 deletions(-) >> >> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h >> index a2399eff84ca..3a06c264ea53 100644 >> --- a/arch/s390/include/asm/pci.h >> +++ b/arch/s390/include/asm/pci.h >> @@ -2,9 +2,6 @@ >> #ifndef __ASM_S390_PCI_H >> #define __ASM_S390_PCI_H >> >> -/* must be set before including pci_clp.h */ >> -#define PCI_BAR_COUNT 6 >> - >> #include <linux/pci.h> >> #include <linux/mutex.h> >> #include <linux/iommu.h> >> @@ -138,7 +135,7 @@ struct zpci_dev { >> >> char res_name[16]; >> bool mio_capable; >> - struct zpci_bar_struct bars[PCI_BAR_COUNT]; >> + struct zpci_bar_struct bars[PCI_STD_NUM_BARS]; >> >> u64 start_dma; /* Start of available DMA addresses */ >> u64 end_dma; /* End of available DMA addresses */ >> diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h >> index 50359172cc48..bd2cb4ea7d93 100644 >> --- a/arch/s390/include/asm/pci_clp.h >> +++ b/arch/s390/include/asm/pci_clp.h >> @@ -77,7 +77,7 @@ struct mio_info { >> struct { >> u64 wb; >> u64 wt; >> - } addr[PCI_BAR_COUNT]; >> + } addr[PCI_STD_NUM_BARS]; >> u32 reserved[6]; >> } __packed; >> >> @@ -98,9 +98,9 @@ struct clp_rsp_query_pci { >> u16 util_str_avail : 1; /* utility string available? */ >> u16 pfgid : 8; /* pci function group id */ >> u32 fid; /* pci function id */ >> - u8 bar_size[PCI_BAR_COUNT]; >> + u8 bar_size[PCI_STD_NUM_BARS]; >> u16 pchid; >> - __le32 bar[PCI_BAR_COUNT]; >> + __le32 bar[PCI_STD_NUM_BARS]; >> u8 pfip[CLP_PFIP_NR_SEGMENTS]; /* pci function internal path */ >> u32 : 16; >> u8 fmb_len; >> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c >> index b0e3b9a0e488..aca372c8e34f 100644 >> --- a/arch/s390/pci/pci.c >> +++ b/arch/s390/pci/pci.c >> @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES); >> static DEFINE_SPINLOCK(zpci_domain_lock); >> >> #define ZPCI_IOMAP_ENTRIES \ >> - min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2), \ >> + min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2), \ >> ZPCI_IOMAP_MAX_ENTRIES) >> >> static DEFINE_SPINLOCK(zpci_iomap_lock); >> @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar, >> void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar, >> unsigned long offset, unsigned long max) >> { >> - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) >> + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) >> return NULL; >> >> if (static_branch_likely(&have_mio)) >> @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar, >> void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar, >> unsigned long offset, unsigned long max) >> { >> - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) >> + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) >> return NULL; > > This looks like a latent bug fix here. If 'bar' is out of range we return > NULL instead accessing an invalid item of an array. Should this not be > a separate patch and tagged as stable? > This fix was suggested by Bjorn in v1 review: https://lkml.org/lkml/2019/8/12/997 > Thanks, > > Andrew Murray > >> >> if (static_branch_likely(&have_mio)) >> @@ -416,7 +416,7 @@ static void zpci_map_resources(struct pci_dev *pdev) >> resource_size_t len; >> int i; >> >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> len = pci_resource_len(pdev, i); >> if (!len) >> continue; >> @@ -451,7 +451,7 @@ static void zpci_unmap_resources(struct pci_dev *pdev) >> if (zpci_use_mio(zdev)) >> return; >> >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> len = pci_resource_len(pdev, i); >> if (!len) >> continue; >> @@ -514,7 +514,7 @@ static int zpci_setup_bus_resources(struct zpci_dev *zdev, >> snprintf(zdev->res_name, sizeof(zdev->res_name), >> "PCI Bus %04x:%02x", zdev->domain, ZPCI_BUS_NR); >> >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> if (!zdev->bars[i].size) >> continue; >> entry = zpci_alloc_iomap(zdev); >> @@ -551,7 +551,7 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev) >> { >> int i; >> >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> if (!zdev->bars[i].size || !zdev->bars[i].res) >> continue; >> >> @@ -573,7 +573,7 @@ int pcibios_add_device(struct pci_dev *pdev) >> pdev->dev.dma_ops = &s390_pci_dma_ops; >> zpci_map_resources(pdev); >> >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> res = &pdev->resource[i]; >> if (res->parent || !res->flags) >> continue; >> diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c >> index 9bdff4defef1..8b729b5f2972 100644 >> --- a/arch/s390/pci/pci_clp.c >> +++ b/arch/s390/pci/pci_clp.c >> @@ -145,7 +145,7 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, >> { >> int i; >> >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> zdev->bars[i].val = le32_to_cpu(response->bar[i]); >> zdev->bars[i].size = response->bar_size[i]; >> } >> @@ -164,8 +164,8 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, >> sizeof(zdev->util_str)); >> } >> zdev->mio_capable = response->mio_addr_avail; >> - for (i = 0; i < PCI_BAR_COUNT; i++) { >> - if (!(response->mio.valid & (1 << (PCI_BAR_COUNT - i - 1)))) >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { >> + if (!(response->mio.valid & (1 << (PCI_STD_NUM_BARS - i - 1)))) >> continue; >> >> zdev->bars[i].mio_wb = (void __iomem *) response->mio.addr[i].wb; >> -- >> 2.21.0 >>
On Wed, Sep 18, 2019 at 05:26:59PM +0300, Denis Efremov wrote: > On 9/18/19 11:58 AM, Andrew Murray wrote: > > On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote: > >> Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use > >> global one PCI_STD_NUM_BARS instead. > >> > >> Acked-by: Sebastian Ott <sebott@linux.ibm.com> > >> Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com> > >> Signed-off-by: Denis Efremov <efremov@linux.com> > >> --- > >> arch/s390/include/asm/pci.h | 5 +---- > >> arch/s390/include/asm/pci_clp.h | 6 +++--- > >> arch/s390/pci/pci.c | 16 ++++++++-------- > >> arch/s390/pci/pci_clp.c | 6 +++--- > >> 4 files changed, 15 insertions(+), 18 deletions(-) > >> > >> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h > >> index a2399eff84ca..3a06c264ea53 100644 > >> --- a/arch/s390/include/asm/pci.h > >> +++ b/arch/s390/include/asm/pci.h > >> @@ -2,9 +2,6 @@ > >> #ifndef __ASM_S390_PCI_H > >> #define __ASM_S390_PCI_H > >> > >> -/* must be set before including pci_clp.h */ > >> -#define PCI_BAR_COUNT 6 > >> - > >> #include <linux/pci.h> > >> #include <linux/mutex.h> > >> #include <linux/iommu.h> > >> @@ -138,7 +135,7 @@ struct zpci_dev { > >> > >> char res_name[16]; > >> bool mio_capable; > >> - struct zpci_bar_struct bars[PCI_BAR_COUNT]; > >> + struct zpci_bar_struct bars[PCI_STD_NUM_BARS]; > >> > >> u64 start_dma; /* Start of available DMA addresses */ > >> u64 end_dma; /* End of available DMA addresses */ > >> diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h > >> index 50359172cc48..bd2cb4ea7d93 100644 > >> --- a/arch/s390/include/asm/pci_clp.h > >> +++ b/arch/s390/include/asm/pci_clp.h > >> @@ -77,7 +77,7 @@ struct mio_info { > >> struct { > >> u64 wb; > >> u64 wt; > >> - } addr[PCI_BAR_COUNT]; > >> + } addr[PCI_STD_NUM_BARS]; > >> u32 reserved[6]; > >> } __packed; > >> > >> @@ -98,9 +98,9 @@ struct clp_rsp_query_pci { > >> u16 util_str_avail : 1; /* utility string available? */ > >> u16 pfgid : 8; /* pci function group id */ > >> u32 fid; /* pci function id */ > >> - u8 bar_size[PCI_BAR_COUNT]; > >> + u8 bar_size[PCI_STD_NUM_BARS]; > >> u16 pchid; > >> - __le32 bar[PCI_BAR_COUNT]; > >> + __le32 bar[PCI_STD_NUM_BARS]; > >> u8 pfip[CLP_PFIP_NR_SEGMENTS]; /* pci function internal path */ > >> u32 : 16; > >> u8 fmb_len; > >> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c > >> index b0e3b9a0e488..aca372c8e34f 100644 > >> --- a/arch/s390/pci/pci.c > >> +++ b/arch/s390/pci/pci.c > >> @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES); > >> static DEFINE_SPINLOCK(zpci_domain_lock); > >> > >> #define ZPCI_IOMAP_ENTRIES \ > >> - min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2), \ > >> + min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2), \ > >> ZPCI_IOMAP_MAX_ENTRIES) > >> > >> static DEFINE_SPINLOCK(zpci_iomap_lock); > >> @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar, > >> void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar, > >> unsigned long offset, unsigned long max) > >> { > >> - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) > >> + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) > >> return NULL; > >> > >> if (static_branch_likely(&have_mio)) > >> @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar, > >> void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar, > >> unsigned long offset, unsigned long max) > >> { > >> - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) > >> + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) > >> return NULL; > > > > This looks like a latent bug fix here. If 'bar' is out of range we return > > NULL instead accessing an invalid item of an array. Should this not be > > a separate patch and tagged as stable? > > > > This fix was suggested by Bjorn in v1 review: > https://lkml.org/lkml/2019/8/12/997 > Ah yes, apologies - I'll re-read the previous threads next time. Thanks, Andrew Murray > > > Thanks, > > > > Andrew Murray > > > >> > >> if (static_branch_likely(&have_mio)) > >> @@ -416,7 +416,7 @@ static void zpci_map_resources(struct pci_dev *pdev) > >> resource_size_t len; > >> int i; > >> > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> len = pci_resource_len(pdev, i); > >> if (!len) > >> continue; > >> @@ -451,7 +451,7 @@ static void zpci_unmap_resources(struct pci_dev *pdev) > >> if (zpci_use_mio(zdev)) > >> return; > >> > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> len = pci_resource_len(pdev, i); > >> if (!len) > >> continue; > >> @@ -514,7 +514,7 @@ static int zpci_setup_bus_resources(struct zpci_dev *zdev, > >> snprintf(zdev->res_name, sizeof(zdev->res_name), > >> "PCI Bus %04x:%02x", zdev->domain, ZPCI_BUS_NR); > >> > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> if (!zdev->bars[i].size) > >> continue; > >> entry = zpci_alloc_iomap(zdev); > >> @@ -551,7 +551,7 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev) > >> { > >> int i; > >> > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> if (!zdev->bars[i].size || !zdev->bars[i].res) > >> continue; > >> > >> @@ -573,7 +573,7 @@ int pcibios_add_device(struct pci_dev *pdev) > >> pdev->dev.dma_ops = &s390_pci_dma_ops; > >> zpci_map_resources(pdev); > >> > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> res = &pdev->resource[i]; > >> if (res->parent || !res->flags) > >> continue; > >> diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c > >> index 9bdff4defef1..8b729b5f2972 100644 > >> --- a/arch/s390/pci/pci_clp.c > >> +++ b/arch/s390/pci/pci_clp.c > >> @@ -145,7 +145,7 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, > >> { > >> int i; > >> > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> zdev->bars[i].val = le32_to_cpu(response->bar[i]); > >> zdev->bars[i].size = response->bar_size[i]; > >> } > >> @@ -164,8 +164,8 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, > >> sizeof(zdev->util_str)); > >> } > >> zdev->mio_capable = response->mio_addr_avail; > >> - for (i = 0; i < PCI_BAR_COUNT; i++) { > >> - if (!(response->mio.valid & (1 << (PCI_BAR_COUNT - i - 1)))) > >> + for (i = 0; i < PCI_STD_NUM_BARS; i++) { > >> + if (!(response->mio.valid & (1 << (PCI_STD_NUM_BARS - i - 1)))) > >> continue; > >> > >> zdev->bars[i].mio_wb = (void __iomem *) response->mio.addr[i].wb; > >> -- > >> 2.21.0 > >> >
On Wed, Sep 18, 2019 at 09:58:06AM +0100, Andrew Murray wrote: > On Mon, Sep 16, 2019 at 11:41:38PM +0300, Denis Efremov wrote: > > Remove local definition PCI_BAR_COUNT for the number of PCI BARs and use > > global one PCI_STD_NUM_BARS instead. > > > > Acked-by: Sebastian Ott <sebott@linux.ibm.com> > > Cc: Gerald Schaefer <gerald.schaefer@de.ibm.com> > > Signed-off-by: Denis Efremov <efremov@linux.com> > > --- > > arch/s390/include/asm/pci.h | 5 +---- > > arch/s390/include/asm/pci_clp.h | 6 +++--- > > arch/s390/pci/pci.c | 16 ++++++++-------- > > arch/s390/pci/pci_clp.c | 6 +++--- > > 4 files changed, 15 insertions(+), 18 deletions(-) > > > > diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h > > index a2399eff84ca..3a06c264ea53 100644 > > --- a/arch/s390/include/asm/pci.h > > +++ b/arch/s390/include/asm/pci.h > > @@ -2,9 +2,6 @@ > > #ifndef __ASM_S390_PCI_H > > #define __ASM_S390_PCI_H > > > > -/* must be set before including pci_clp.h */ > > -#define PCI_BAR_COUNT 6 > > - > > #include <linux/pci.h> > > #include <linux/mutex.h> > > #include <linux/iommu.h> > > @@ -138,7 +135,7 @@ struct zpci_dev { > > > > char res_name[16]; > > bool mio_capable; > > - struct zpci_bar_struct bars[PCI_BAR_COUNT]; > > + struct zpci_bar_struct bars[PCI_STD_NUM_BARS]; > > > > u64 start_dma; /* Start of available DMA addresses */ > > u64 end_dma; /* End of available DMA addresses */ > > diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h > > index 50359172cc48..bd2cb4ea7d93 100644 > > --- a/arch/s390/include/asm/pci_clp.h > > +++ b/arch/s390/include/asm/pci_clp.h > > @@ -77,7 +77,7 @@ struct mio_info { > > struct { > > u64 wb; > > u64 wt; > > - } addr[PCI_BAR_COUNT]; > > + } addr[PCI_STD_NUM_BARS]; > > u32 reserved[6]; > > } __packed; > > > > @@ -98,9 +98,9 @@ struct clp_rsp_query_pci { > > u16 util_str_avail : 1; /* utility string available? */ > > u16 pfgid : 8; /* pci function group id */ > > u32 fid; /* pci function id */ > > - u8 bar_size[PCI_BAR_COUNT]; > > + u8 bar_size[PCI_STD_NUM_BARS]; > > u16 pchid; > > - __le32 bar[PCI_BAR_COUNT]; > > + __le32 bar[PCI_STD_NUM_BARS]; > > u8 pfip[CLP_PFIP_NR_SEGMENTS]; /* pci function internal path */ > > u32 : 16; > > u8 fmb_len; > > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c > > index b0e3b9a0e488..aca372c8e34f 100644 > > --- a/arch/s390/pci/pci.c > > +++ b/arch/s390/pci/pci.c > > @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES); > > static DEFINE_SPINLOCK(zpci_domain_lock); > > > > #define ZPCI_IOMAP_ENTRIES \ > > - min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2), \ > > + min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2), \ > > ZPCI_IOMAP_MAX_ENTRIES) > > > > static DEFINE_SPINLOCK(zpci_iomap_lock); > > @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar, > > void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar, > > unsigned long offset, unsigned long max) > > { > > - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) > > + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) > > return NULL; > > > > if (static_branch_likely(&have_mio)) > > @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar, > > void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar, > > unsigned long offset, unsigned long max) > > { > > - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) > > + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) > > return NULL; > > This looks like a latent bug fix here. If 'bar' is out of range we return > NULL instead accessing an invalid item of an array. Should this not be > a separate patch and tagged as stable? Sharp eyes! I didn't think of this as accessing an invalid item, but indeed it does (if 'bar' is out of range). But I doubt it's worth the hassle of a separate patch, since we return failure anyway.
diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h index a2399eff84ca..3a06c264ea53 100644 --- a/arch/s390/include/asm/pci.h +++ b/arch/s390/include/asm/pci.h @@ -2,9 +2,6 @@ #ifndef __ASM_S390_PCI_H #define __ASM_S390_PCI_H -/* must be set before including pci_clp.h */ -#define PCI_BAR_COUNT 6 - #include <linux/pci.h> #include <linux/mutex.h> #include <linux/iommu.h> @@ -138,7 +135,7 @@ struct zpci_dev { char res_name[16]; bool mio_capable; - struct zpci_bar_struct bars[PCI_BAR_COUNT]; + struct zpci_bar_struct bars[PCI_STD_NUM_BARS]; u64 start_dma; /* Start of available DMA addresses */ u64 end_dma; /* End of available DMA addresses */ diff --git a/arch/s390/include/asm/pci_clp.h b/arch/s390/include/asm/pci_clp.h index 50359172cc48..bd2cb4ea7d93 100644 --- a/arch/s390/include/asm/pci_clp.h +++ b/arch/s390/include/asm/pci_clp.h @@ -77,7 +77,7 @@ struct mio_info { struct { u64 wb; u64 wt; - } addr[PCI_BAR_COUNT]; + } addr[PCI_STD_NUM_BARS]; u32 reserved[6]; } __packed; @@ -98,9 +98,9 @@ struct clp_rsp_query_pci { u16 util_str_avail : 1; /* utility string available? */ u16 pfgid : 8; /* pci function group id */ u32 fid; /* pci function id */ - u8 bar_size[PCI_BAR_COUNT]; + u8 bar_size[PCI_STD_NUM_BARS]; u16 pchid; - __le32 bar[PCI_BAR_COUNT]; + __le32 bar[PCI_STD_NUM_BARS]; u8 pfip[CLP_PFIP_NR_SEGMENTS]; /* pci function internal path */ u32 : 16; u8 fmb_len; diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c index b0e3b9a0e488..aca372c8e34f 100644 --- a/arch/s390/pci/pci.c +++ b/arch/s390/pci/pci.c @@ -43,7 +43,7 @@ static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES); static DEFINE_SPINLOCK(zpci_domain_lock); #define ZPCI_IOMAP_ENTRIES \ - min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2), \ + min(((unsigned long) ZPCI_NR_DEVICES * PCI_STD_NUM_BARS / 2), \ ZPCI_IOMAP_MAX_ENTRIES) static DEFINE_SPINLOCK(zpci_iomap_lock); @@ -294,7 +294,7 @@ static void __iomem *pci_iomap_range_mio(struct pci_dev *pdev, int bar, void __iomem *pci_iomap_range(struct pci_dev *pdev, int bar, unsigned long offset, unsigned long max) { - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) return NULL; if (static_branch_likely(&have_mio)) @@ -324,7 +324,7 @@ static void __iomem *pci_iomap_wc_range_mio(struct pci_dev *pdev, int bar, void __iomem *pci_iomap_wc_range(struct pci_dev *pdev, int bar, unsigned long offset, unsigned long max) { - if (!pci_resource_len(pdev, bar) || bar >= PCI_BAR_COUNT) + if (bar >= PCI_STD_NUM_BARS || !pci_resource_len(pdev, bar)) return NULL; if (static_branch_likely(&have_mio)) @@ -416,7 +416,7 @@ static void zpci_map_resources(struct pci_dev *pdev) resource_size_t len; int i; - for (i = 0; i < PCI_BAR_COUNT; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { len = pci_resource_len(pdev, i); if (!len) continue; @@ -451,7 +451,7 @@ static void zpci_unmap_resources(struct pci_dev *pdev) if (zpci_use_mio(zdev)) return; - for (i = 0; i < PCI_BAR_COUNT; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { len = pci_resource_len(pdev, i); if (!len) continue; @@ -514,7 +514,7 @@ static int zpci_setup_bus_resources(struct zpci_dev *zdev, snprintf(zdev->res_name, sizeof(zdev->res_name), "PCI Bus %04x:%02x", zdev->domain, ZPCI_BUS_NR); - for (i = 0; i < PCI_BAR_COUNT; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { if (!zdev->bars[i].size) continue; entry = zpci_alloc_iomap(zdev); @@ -551,7 +551,7 @@ static void zpci_cleanup_bus_resources(struct zpci_dev *zdev) { int i; - for (i = 0; i < PCI_BAR_COUNT; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { if (!zdev->bars[i].size || !zdev->bars[i].res) continue; @@ -573,7 +573,7 @@ int pcibios_add_device(struct pci_dev *pdev) pdev->dev.dma_ops = &s390_pci_dma_ops; zpci_map_resources(pdev); - for (i = 0; i < PCI_BAR_COUNT; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { res = &pdev->resource[i]; if (res->parent || !res->flags) continue; diff --git a/arch/s390/pci/pci_clp.c b/arch/s390/pci/pci_clp.c index 9bdff4defef1..8b729b5f2972 100644 --- a/arch/s390/pci/pci_clp.c +++ b/arch/s390/pci/pci_clp.c @@ -145,7 +145,7 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, { int i; - for (i = 0; i < PCI_BAR_COUNT; i++) { + for (i = 0; i < PCI_STD_NUM_BARS; i++) { zdev->bars[i].val = le32_to_cpu(response->bar[i]); zdev->bars[i].size = response->bar_size[i]; } @@ -164,8 +164,8 @@ static int clp_store_query_pci_fn(struct zpci_dev *zdev, sizeof(zdev->util_str)); } zdev->mio_capable = response->mio_addr_avail; - for (i = 0; i < PCI_BAR_COUNT; i++) { - if (!(response->mio.valid & (1 << (PCI_BAR_COUNT - i - 1)))) + for (i = 0; i < PCI_STD_NUM_BARS; i++) { + if (!(response->mio.valid & (1 << (PCI_STD_NUM_BARS - i - 1)))) continue; zdev->bars[i].mio_wb = (void __iomem *) response->mio.addr[i].wb;