| Message ID | 1469549597-19253-4-git-send-email-rui.y.wang@intel.com |
|---|---|
| State | Not Applicable |
| Headers | show |
Hi,
[auto build test WARNING on pm/linux-next]
[also build test WARNING on v4.7 next-20160726]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Rui-Wang/Fixing-a-set-of-bugs-for-ioapic-hotplug/20160727-003628
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-x015-201630 (attached as .config)
compiler: gcc-6 (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
Note: it may well be a FALSE warning. FWIW you are at least aware of it now.
http://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
All warnings (new ones prefixed by >>):
drivers/acpi/ioapic.c: In function 'handle_ioapic_add':
>> drivers/acpi/ioapic.c:159:18: warning: 'pci_res' may be used uninitialized in this function [-Wmaybe-uninitialized]
if (!res || !res->flags)
~~~^~~~~~~
vim +/pci_res +159 drivers/acpi/ioapic.c
143 pci_dev_put(dev);
144 dev = NULL;
145 }
146
147 crs_res = &ioapic->res;
148 acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res);
149 if (crs_res->flags == 0) {
150 acpi_handle_warn(handle, "failed to get resource\n");
151 goto exit_release;
152 } else if (request_resource(&iomem_resource, crs_res)) {
153 acpi_handle_warn(handle, "failed to insert resource\n");
154 goto exit_release;
155 }
156
157 /* try pci resource first, then "_CRS" resource */
158 res = pci_res;
> 159 if (!res || !res->flags)
160 res = crs_res;
161
162 if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) {
163 acpi_handle_warn(handle, "failed to register IOAPIC\n");
164 goto exit_release;
165 }
166 done:
167 list_add(&ioapic->list, &ioapic_list);
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Wed, Jul 27, 2016 at 12:13:16AM +0800, Rui Wang wrote: > ioapic resource at 0xfecxxxxx gets lost from /proc/iomem after > hot-removing and then hot-adding the ioapic devices. > > After system boot, in /proc/iomem: > fec00000-fecfffff : PNP0003:00 > fec00000-fec003ff : IOAPIC 0 > fec01000-fec013ff : IOAPIC 1 > fec40000-fec403ff : IOAPIC 2 > fec80000-fec803ff : IOAPIC 3 > fecc0000-fecc03ff : IOAPIC 4 > > Then hot-remove IOAPIC 2 and hot-add it again: > fec00000-fecfffff : PNP0003:00 > fec00000-fec003ff : IOAPIC 0 > fec01000-fec013ff : IOAPIC 1 > fec80000-fec803ff : IOAPIC 3 > fecc0000-fecc03ff : IOAPIC 4 > > The range at 0xfec40000 is lost from /proc/iomem. It is because > handle_ioapic_add() requests resource from either PCI config BAR or > acpi _CRS, not both. But Intel platforms map the IOxAPIC registers s/acpi/ACPI/ s/ioapic/IOAPIC/ throughout > at both the PCI config BAR (called MBAR) and the 0xfecX_YZ00 to > 0xfecX_Y2FF range (called ABAR). Both of the ranges should be claimed I guess you mean the 0xfecX_YZ00-0xfecX_Y2FF range appears in _CRS? > from /proc/iomem for exclusive use. > > Signed-off-by: Rui Wang <rui.y.wang@intel.com> > --- > drivers/acpi/ioapic.c | 36 ++++++++++++++++++++---------------- > 1 file changed, 20 insertions(+), 16 deletions(-) > > diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c > index daf4a40..80b0b1a 100644 > --- a/drivers/acpi/ioapic.c > +++ b/drivers/acpi/ioapic.c > @@ -97,7 +97,7 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl, > unsigned long long gsi_base; > struct acpi_pci_ioapic *ioapic; > struct pci_dev *dev = NULL; > - struct resource *res = NULL; > + struct resource *res = NULL, *pci_res, *crs_res; > char *type = NULL; > > if (!acpi_is_ioapic(handle, &type)) > @@ -137,23 +137,28 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl, > pci_set_master(dev); > if (pci_request_region(dev, 0, type)) > goto exit_disable; > - res = &dev->resource[0]; > + pci_res = &dev->resource[0]; > ioapic->pdev = dev; > } else { > pci_dev_put(dev); > dev = NULL; > + } > > - res = &ioapic->res; > - acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res); > - if (res->flags == 0) { > - acpi_handle_warn(handle, "failed to get resource\n"); > - goto exit_free; > - } else if (request_resource(&iomem_resource, res)) { > - acpi_handle_warn(handle, "failed to insert resource\n"); > - goto exit_free; > - } > + crs_res = &ioapic->res; > + acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res); > + if (crs_res->flags == 0) { > + acpi_handle_warn(handle, "failed to get resource\n"); > + goto exit_release; > + } else if (request_resource(&iomem_resource, crs_res)) { > + acpi_handle_warn(handle, "failed to insert resource\n"); > + goto exit_release; > } > > + /* try pci resource first, then "_CRS" resource */ > + res = pci_res; > + if (!res || !res->flags) > + res = crs_res; > + > if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) { > acpi_handle_warn(handle, "failed to register IOAPIC\n"); > goto exit_release; > @@ -174,14 +179,13 @@ done: > exit_release: > if (dev) > pci_release_region(dev, 0); > - else > - release_resource(res); > + if (ioapic->res.flags && ioapic->res.parent) > + release_resource(&ioapic->res); > exit_disable: > if (dev) > pci_disable_device(dev); > exit_put: > pci_dev_put(dev); > -exit_free: > kfree(ioapic); > exit: > mutex_unlock(&ioapic_list_lock); > @@ -218,9 +222,9 @@ int acpi_ioapic_remove(struct acpi_pci_root *root) > pci_release_region(ioapic->pdev, 0); > pci_disable_device(ioapic->pdev); > pci_dev_put(ioapic->pdev); > - } else if (ioapic->res.flags && ioapic->res.parent) { > - release_resource(&ioapic->res); > } > + if (ioapic->res.flags && ioapic->res.parent) > + release_resource(&ioapic->res); > list_del(&ioapic->list); > kfree(ioapic); > } > -- > 1.8.3.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, July 27, 2016 4:24 AM, Bjorn Helgaas wrote: > On Wed, Jul 27, 2016 at 12:13:16AM +0800, Rui Wang wrote: > > ioapic resource at 0xfecxxxxx gets lost from /proc/iomem after > > hot-removing and then hot-adding the ioapic devices. > > > > After system boot, in /proc/iomem: > > fec00000-fecfffff : PNP0003:00 > > fec00000-fec003ff : IOAPIC 0 > > fec01000-fec013ff : IOAPIC 1 > > fec40000-fec403ff : IOAPIC 2 > > fec80000-fec803ff : IOAPIC 3 > > fecc0000-fecc03ff : IOAPIC 4 > > > > Then hot-remove IOAPIC 2 and hot-add it again: > > fec00000-fecfffff : PNP0003:00 > > fec00000-fec003ff : IOAPIC 0 > > fec01000-fec013ff : IOAPIC 1 > > fec80000-fec803ff : IOAPIC 3 > > fecc0000-fecc03ff : IOAPIC 4 > > > > The range at 0xfec40000 is lost from /proc/iomem. It is because > > handle_ioapic_add() requests resource from either PCI config BAR or > > acpi _CRS, not both. But Intel platforms map the IOxAPIC registers > > s/acpi/ACPI/ > s/ioapic/IOAPIC/ throughout > > > at both the PCI config BAR (called MBAR) and the 0xfecX_YZ00 to > > 0xfecX_Y2FF range (called ABAR). Both of the ranges should be claimed > > I guess you mean the 0xfecX_YZ00-0xfecX_Y2FF range appears in _CRS? Yes. That range appears in _CRS for each IOAPIC. I'll make it cleaner in the commit message. Thanks Rui -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/ioapic.c b/drivers/acpi/ioapic.c index daf4a40..80b0b1a 100644 --- a/drivers/acpi/ioapic.c +++ b/drivers/acpi/ioapic.c @@ -97,7 +97,7 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl, unsigned long long gsi_base; struct acpi_pci_ioapic *ioapic; struct pci_dev *dev = NULL; - struct resource *res = NULL; + struct resource *res = NULL, *pci_res, *crs_res; char *type = NULL; if (!acpi_is_ioapic(handle, &type)) @@ -137,23 +137,28 @@ static acpi_status handle_ioapic_add(acpi_handle handle, u32 lvl, pci_set_master(dev); if (pci_request_region(dev, 0, type)) goto exit_disable; - res = &dev->resource[0]; + pci_res = &dev->resource[0]; ioapic->pdev = dev; } else { pci_dev_put(dev); dev = NULL; + } - res = &ioapic->res; - acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res); - if (res->flags == 0) { - acpi_handle_warn(handle, "failed to get resource\n"); - goto exit_free; - } else if (request_resource(&iomem_resource, res)) { - acpi_handle_warn(handle, "failed to insert resource\n"); - goto exit_free; - } + crs_res = &ioapic->res; + acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, crs_res); + if (crs_res->flags == 0) { + acpi_handle_warn(handle, "failed to get resource\n"); + goto exit_release; + } else if (request_resource(&iomem_resource, crs_res)) { + acpi_handle_warn(handle, "failed to insert resource\n"); + goto exit_release; } + /* try pci resource first, then "_CRS" resource */ + res = pci_res; + if (!res || !res->flags) + res = crs_res; + if (acpi_register_ioapic(handle, res->start, (u32)gsi_base)) { acpi_handle_warn(handle, "failed to register IOAPIC\n"); goto exit_release; @@ -174,14 +179,13 @@ done: exit_release: if (dev) pci_release_region(dev, 0); - else - release_resource(res); + if (ioapic->res.flags && ioapic->res.parent) + release_resource(&ioapic->res); exit_disable: if (dev) pci_disable_device(dev); exit_put: pci_dev_put(dev); -exit_free: kfree(ioapic); exit: mutex_unlock(&ioapic_list_lock); @@ -218,9 +222,9 @@ int acpi_ioapic_remove(struct acpi_pci_root *root) pci_release_region(ioapic->pdev, 0); pci_disable_device(ioapic->pdev); pci_dev_put(ioapic->pdev); - } else if (ioapic->res.flags && ioapic->res.parent) { - release_resource(&ioapic->res); } + if (ioapic->res.flags && ioapic->res.parent) + release_resource(&ioapic->res); list_del(&ioapic->list); kfree(ioapic); }
ioapic resource at 0xfecxxxxx gets lost from /proc/iomem after hot-removing and then hot-adding the ioapic devices. After system boot, in /proc/iomem: fec00000-fecfffff : PNP0003:00 fec00000-fec003ff : IOAPIC 0 fec01000-fec013ff : IOAPIC 1 fec40000-fec403ff : IOAPIC 2 fec80000-fec803ff : IOAPIC 3 fecc0000-fecc03ff : IOAPIC 4 Then hot-remove IOAPIC 2 and hot-add it again: fec00000-fecfffff : PNP0003:00 fec00000-fec003ff : IOAPIC 0 fec01000-fec013ff : IOAPIC 1 fec80000-fec803ff : IOAPIC 3 fecc0000-fecc03ff : IOAPIC 4 The range at 0xfec40000 is lost from /proc/iomem. It is because handle_ioapic_add() requests resource from either PCI config BAR or acpi _CRS, not both. But Intel platforms map the IOxAPIC registers at both the PCI config BAR (called MBAR) and the 0xfecX_YZ00 to 0xfecX_Y2FF range (called ABAR). Both of the ranges should be claimed from /proc/iomem for exclusive use. Signed-off-by: Rui Wang <rui.y.wang@intel.com> --- drivers/acpi/ioapic.c | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-)