[PATCH] powerpc/rtas_flash: fix a potential buffer overflow
Since snprintf() returns the possible output size instead of the
actual output size, the available flash_msg length returned by
get_validate_flash_msg may exceed the given buffer limit when
simple_read_from_buffer calls copy_to_user
Signed-off-by: Yi Zhuang
---
arch/powerpc/kernel/rtas_flash.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index a99179d83538..4aa6bad28556 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -473,6 +473,10 @@ static int get_validate_flash_msg(struct
rtas_validate_flash_t *args_buf,
(args_buf->update_results == VALIDATE_TMP_UPDATE))
n += snprintf(msg + n, msglen - n, "%s\n",
args_buf->buf);
+ if (n >= msglen) {
+ n = msglen;
+ printk(KERN_ERR "FLASH: msg too long.\n");
+ }
} else {
n = sprintf(msg, "%d\n", args_buf->status);
}
--
2.26.0.106.g9fadedd
Re: [PATCH] powerpc/rtas_flash: fix a potential buffer overflow
Hi Yi,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on powerpc/next]
[also build test WARNING on v5.14-rc1 next-20210713]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Yi-Zhuang/powerpc-rtas_flash-fix-a-potential-buffer-overflow/20210714-090314
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allyesconfig (attached as .config)
compiler: powerpc64-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/546db7a99374dedd110a01801ad4456f56170d4d
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Yi-Zhuang/powerpc-rtas_flash-fix-a-potential-buffer-overflow/20210714-090314
git checkout 546db7a99374dedd110a01801ad4456f56170d4d
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All warnings (new ones prefixed by >>):
arch/powerpc/kernel/rtas_flash.c: In function 'get_validate_flash_msg':
>> arch/powerpc/kernel/rtas_flash.c:472:3: warning: this 'if' clause does not
>> guard... [-Wmisleading-indentation]
472 | if ((args_buf->update_results >= VALIDATE_CUR_UNKNOWN) ||
| ^~
arch/powerpc/kernel/rtas_flash.c:476:4: note: ...this statement, but the
latter is misleadingly indented as if it were guarded by the 'if'
476 |if (n >= msglen) {
|^~
vim +/if +472 arch/powerpc/kernel/rtas_flash.c
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 464
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 465
static int get_validate_flash_msg(struct rtas_validate_flash_t *args_buf,
a94a14720eaf55 arch/powerpc/kernel/rtas_flash.c Vasant Hegde 2013-05-07 466
char *msg, int msglen)
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 467
{
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 468
int n;
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 469
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 470
if (args_buf->status >= VALIDATE_TMP_UPDATE) {
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 471
n = sprintf(msg, "%d\n", args_buf->update_results);
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 @472
if ((args_buf->update_results >= VALIDATE_CUR_UNKNOWN) ||
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 473
(args_buf->update_results == VALIDATE_TMP_UPDATE))
a94a14720eaf55 arch/powerpc/kernel/rtas_flash.c Vasant Hegde 2013-05-07 474
n += snprintf(msg + n, msglen - n, "%s\n",
a94a14720eaf55 arch/powerpc/kernel/rtas_flash.c Vasant Hegde 2013-05-07 475
args_buf->buf);
546db7a99374de arch/powerpc/kernel/rtas_flash.c Yi Zhuang 2021-07-14 476
if (n >= msglen) {
546db7a99374de arch/powerpc/kernel/rtas_flash.c Yi Zhuang 2021-07-14 477
n = msglen;
546db7a99374de arch/powerpc/kernel/rtas_flash.c Yi Zhuang 2021-07-14 478
printk(KERN_ERR "FLASH: msg too long.\n");
546db7a99374de arch/powerpc/kernel/rtas_flash.c Yi Zhuang 2021-07-14 479
}
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 480
} else {
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 481
n = sprintf(msg, "%d\n", args_buf->status);
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 482
}
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 483
return n;
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 484
}
^1da177e4c3f41 arch/ppc64/kernel/rtas_flash.c Linus Torvalds 2005-04-16 485
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v15 06/12] swiotlb: Use is_swiotlb_force_bounce for swiotlb data bouncing
..snip.. > > > I think the main question I have is how would you like to see patches for > > > 5.15? i.e. as patches on top of devel/for-linus-5.14 or something else? > > > > Yes that would be perfect. If there are any dependencies on the rc1, I > > can rebase it on top of that. > > Yes, please, rebasing would be very helpful. The broader rework of > 'io_tlb_default_mem' is going to conflict quite badly otherwise. There is a devel/for-linus-5.15 (based on v5.14-rc1) now. Thank you! > > Cheers, > > Will
[PATCH v4 5/5] bus: Make remove callback return void
The driver core ignores the return value of this callback because there is only little it can do when a device disappears. This is the final bit of a long lasting cleanup quest where several buses were converted to also return void from their remove callback. Additionally some resource leaks were fixed that were caused by drivers returning an error code in the expectation that the driver won't go away. With struct bus_type::remove returning void it's prevented that newly implemented buses return an ignored error code and so don't anticipate wrong expectations for driver authors. Acked-by: Russell King (Oracle) (For ARM, Amba and related parts) Acked-by: Mark Brown Acked-by: Chen-Yu Tsai (for sunxi-rsb) Acked-by: Pali Rohár Acked-by: Mauro Carvalho Chehab (for media) Acked-by: Hans de Goede (For drivers/platform) Acked-by: Alexandre Belloni Acked-By: Vinod Koul Acked-by: Juergen Gross (For xen) Acked-by: Lee Jones (For mfd) Acked-by: Johannes Thumshirn (For mcb) Acked-by: Johan Hovold Acked-by: Srinivas Kandagatla (For slimbus) Acked-by: Kirti Wankhede (For vfio) Acked-by: Maximilian Luz Acked-by: Heikki Krogerus (For ulpi and typec) Acked-by: Samuel Iglesias Gonsálvez (For ipack) Reviewed-by: Tom Rix (For fpga) Acked-by: Geoff Levand (For ps3) Acked-by: Yehezkel Bernat (For thunderbolt) Reviewed-by: Mathieu Poirier Acked-by: Alexander Shishkin (For intel_th) Acked-by: Dominik Brodowski (For pcmcia) Reviewed-by: Cornelia Huck (For drivers/s390 and drivers/vfio) Acked-by: Rafael J. Wysocki (For ACPI) Acked-by: Bjorn Andersson (rpmsg and apr) Acked-by: Srinivas Pandruvada (For intel-ish-hid) Acked-by: Dan Williams (For CXL, DAX, and NVDIMM) Acked-by: William Breathitt Gray (For isa) Acked-by: Stefan Richter (For firewire) Acked-by: Benjamin Tissoires (For hid) Acked-by: Thorsten Scherer (For siox) Acked-by: Sven Van Asbroeck (For anybuss) Acked-by: Ulf Hansson (For MMC) Signed-off-by: Uwe Kleine-König --- arch/arm/common/locomo.c | 3 +-- arch/arm/common/sa.c | 4 +--- arch/arm/mach-rpc/ecard.c | 4 +--- arch/mips/sgi-ip22/ip22-gio.c | 3 +-- arch/parisc/kernel/drivers.c | 5 ++--- arch/powerpc/platforms/ps3/system-bus.c | 3 +-- arch/powerpc/platforms/pseries/ibmebus.c | 3 +-- arch/powerpc/platforms/pseries/vio.c | 3 +-- arch/sparc/kernel/vio.c | 4 +--- drivers/acpi/bus.c| 3 +-- drivers/amba/bus.c| 4 +--- drivers/base/auxiliary.c | 4 +--- drivers/base/isa.c| 4 +--- drivers/base/platform.c | 4 +--- drivers/bcma/main.c | 6 ++ drivers/bus/sunxi-rsb.c | 4 +--- drivers/cxl/core.c| 3 +-- drivers/dax/bus.c | 4 +--- drivers/dma/idxd/sysfs.c | 4 +--- drivers/firewire/core-device.c| 4 +--- drivers/firmware/arm_scmi/bus.c | 4 +--- drivers/firmware/google/coreboot_table.c | 4 +--- drivers/fpga/dfl.c| 4 +--- drivers/hid/hid-core.c| 4 +--- drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +--- drivers/hv/vmbus_drv.c| 5 + drivers/hwtracing/intel_th/core.c | 4 +--- drivers/i2c/i2c-core-base.c | 5 + drivers/i3c/master.c | 4 +--- drivers/input/gameport/gameport.c | 3 +-- drivers/input/serio/serio.c | 3 +-- drivers/ipack/ipack.c | 4 +--- drivers/macintosh/macio_asic.c| 4 +--- drivers/mcb/mcb-core.c| 4 +--- drivers/media/pci/bt8xx/bttv-gpio.c | 3 +-- drivers/memstick/core/memstick.c | 3 +-- drivers/mfd/mcp-core.c| 3 +-- drivers/misc/mei/bus.c| 4 +--- drivers/misc/tifm_core.c | 3 +-- drivers/mmc/core/bus.c| 4 +--- drivers/mmc/core/sdio_bus.c | 4 +--- drivers/net/netdevsim/bus.c | 3 +-- drivers/ntb/core.c| 4 +--- drivers/ntb/ntb_transport.c | 4 +--- drivers/nubus/bus.c | 6 ++ drivers/nvdimm/bus.c | 3 +-- drivers/pci/endpoint/pci-epf-core.c | 4 +--- drivers/pci/pci-driver.c | 3 +-- drivers/pcmcia/ds.c | 4 +--- drivers/platform/surface/aggregator/bus.c | 4 +--- drivers/platform/x86/wmi.c| 4 +--- drivers/pnp/driver.c | 3 +-- drivers/rapidio/rio-driver.c | 4 +--- drivers/rpmsg/rpmsg_core.c| 7 ++- drivers/s390/cio/ccwgroup.c | 4 +--- drivers/s390/cio/css.c| 4 +--- drivers/s390/cio/device.c | 4 +--- drivers/s390/cio/scm.c| 4 +--- drivers/s390
[PATCH v4 0/5] bus: Make remove callback return void
Hello, this is v4 of the final patch set for my effort to make struct bus_type::remove return void. The first four patches contain cleanups that make some of these callbacks (more obviously) always return 0. They are acked by the respective maintainers. Bjorn Helgaas explicitly asked to include the pci patch (#1) into this series, so Greg taking this is fine. I assume the s390 people are fine with Greg taking patches #2 to #4, too, they didn't explicitly said so though. The last patch actually changes the prototype and so touches quite some drivers and has the potential to conflict with future developments, so I consider it beneficial to put these patches into next soon. I expect that it will be Greg who takes the complete series, he already confirmed via irc (for v2) to look into this series. The only change compared to v3 is in the fourth patch where I modified a few more drivers to fix build failures. Some of them were found by build bots (thanks!), some of them I found myself using a regular expression search. The newly modified files are: arch/sparc/kernel/vio.c drivers/nubus/bus.c drivers/sh/superhyway/superhyway.c drivers/vlynq/vlynq.c drivers/zorro/zorro-driver.c sound/ac97/bus.c Best regards Uwe Uwe Kleine-König (5): PCI: endpoint: Make struct pci_epf_driver::remove return void s390/cio: Make struct css_driver::remove return void s390/ccwgroup: Drop if with an always false condition s390/scm: Make struct scm_driver::remove return void bus: Make remove callback return void arch/arm/common/locomo.c | 3 +-- arch/arm/common/sa.c | 4 +--- arch/arm/mach-rpc/ecard.c | 4 +--- arch/mips/sgi-ip22/ip22-gio.c | 3 +-- arch/parisc/kernel/drivers.c | 5 ++--- arch/powerpc/platforms/ps3/system-bus.c | 3 +-- arch/powerpc/platforms/pseries/ibmebus.c | 3 +-- arch/powerpc/platforms/pseries/vio.c | 3 +-- arch/s390/include/asm/eadm.h | 2 +- arch/sparc/kernel/vio.c | 4 +--- drivers/acpi/bus.c| 3 +-- drivers/amba/bus.c| 4 +--- drivers/base/auxiliary.c | 4 +--- drivers/base/isa.c| 4 +--- drivers/base/platform.c | 4 +--- drivers/bcma/main.c | 6 ++ drivers/bus/sunxi-rsb.c | 4 +--- drivers/cxl/core.c| 3 +-- drivers/dax/bus.c | 4 +--- drivers/dma/idxd/sysfs.c | 4 +--- drivers/firewire/core-device.c| 4 +--- drivers/firmware/arm_scmi/bus.c | 4 +--- drivers/firmware/google/coreboot_table.c | 4 +--- drivers/fpga/dfl.c| 4 +--- drivers/hid/hid-core.c| 4 +--- drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +--- drivers/hv/vmbus_drv.c| 5 + drivers/hwtracing/intel_th/core.c | 4 +--- drivers/i2c/i2c-core-base.c | 5 + drivers/i3c/master.c | 4 +--- drivers/input/gameport/gameport.c | 3 +-- drivers/input/serio/serio.c | 3 +-- drivers/ipack/ipack.c | 4 +--- drivers/macintosh/macio_asic.c| 4 +--- drivers/mcb/mcb-core.c| 4 +--- drivers/media/pci/bt8xx/bttv-gpio.c | 3 +-- drivers/memstick/core/memstick.c | 3 +-- drivers/mfd/mcp-core.c| 3 +-- drivers/misc/mei/bus.c| 4 +--- drivers/misc/tifm_core.c | 3 +-- drivers/mmc/core/bus.c| 4 +--- drivers/mmc/core/sdio_bus.c | 4 +--- drivers/net/netdevsim/bus.c | 3 +-- drivers/ntb/core.c| 4 +--- drivers/ntb/ntb_transport.c | 4 +--- drivers/nubus/bus.c | 6 ++ drivers/nvdimm/bus.c | 3 +-- drivers/pci/endpoint/pci-epf-core.c | 7 ++- drivers/pci/pci-driver.c | 3 +-- drivers/pcmcia/ds.c | 4 +--- drivers/platform/surface/aggregator/bus.c | 4 +--- drivers/platform/x86/wmi.c| 4 +--- drivers/pnp/driver.c | 3 +-- drivers/rapidio/rio-driver.c | 4 +--- drivers/rpmsg/rpmsg_core.c| 7 ++- drivers/s390/block/scm_drv.c | 4 +--- drivers/s390/cio/ccwgroup.c | 6 +- drivers/s390/cio/chsc_sch.c | 3 +-- drivers/s390/cio/css.c| 7 +++ drivers/s390/cio/css.h| 2 +- drivers/s390/cio/device.c | 9 +++-- drivers/s390/cio/eadm_sch.c | 4 +--- drivers/s390/cio/scm.c| 5 +++-- drivers/s390/cio/vfio_ccw_drv.c | 3 +-- drivers/s390/crypto/ap_bus.c | 4 +--- drivers/scsi/scsi_debug.c | 3 +-- drivers/sh/superhyway/superhyway.c
[PATCH v3 5/5] bus: Make remove callback return void
The driver core ignores the return value of this callback because there is only little it can do when a device disappears. This is the final bit of a long lasting cleanup quest where several buses were converted to also return void from their remove callback. Additionally some resource leaks were fixed that were caused by drivers returning an error code in the expectation that the driver won't go away. With struct bus_type::remove returning void it's prevented that newly implemented buses return an ignored error code and so don't anticipate wrong expectations for driver authors. Acked-by: Russell King (Oracle) (For ARM, Amba and related parts) Acked-by: Mark Brown Acked-by: Chen-Yu Tsai (for sunxi-rsb) Acked-by: Pali Rohár Acked-by: Mauro Carvalho Chehab (for media) Acked-by: Hans de Goede (For drivers/platform) Acked-by: Alexandre Belloni Acked-By: Vinod Koul Acked-by: Juergen Gross (For xen) Acked-by: Lee Jones (For mfd) Acked-by: Johannes Thumshirn (For mcb) Acked-by: Johan Hovold Acked-by: Srinivas Kandagatla (For slimbus) Acked-by: Kirti Wankhede (For vfio) Acked-by: Maximilian Luz Acked-by: Heikki Krogerus (For ulpi and typec) Acked-by: Samuel Iglesias Gonsálvez (For ipack) Reviewed-by: Tom Rix (For fpga) Acked-by: Geoff Levand (For ps3) Acked-by: Yehezkel Bernat (For thunderbolt) Reviewed-by: Mathieu Poirier Acked-by: Alexander Shishkin (For intel_th) Acked-by: Dominik Brodowski (For pcmcia) Reviewed-by: Cornelia Huck (For drivers/s390 and drivers/vfio) Acked-by: Rafael J. Wysocki (For ACPI) Acked-by: Bjorn Andersson (rpmsg and apr) Acked-by: Srinivas Pandruvada (For intel-ish-hid) Acked-by: Dan Williams (For CXL, DAX, and NVDIMM) Acked-by: William Breathitt Gray (For isa) Acked-by: Stefan Richter (For firewire) Acked-by: Benjamin Tissoires (For hid) Acked-by: Thorsten Scherer (For siox) Acked-by: Sven Van Asbroeck (For anybuss) Acked-by: Ulf Hansson (For MMC) Signed-off-by: Uwe Kleine-König --- arch/arm/common/locomo.c | 3 +-- arch/arm/common/sa.c | 4 +--- arch/arm/mach-rpc/ecard.c | 4 +--- arch/mips/sgi-ip22/ip22-gio.c | 3 +-- arch/parisc/kernel/drivers.c | 5 ++--- arch/powerpc/platforms/ps3/system-bus.c | 3 +-- arch/powerpc/platforms/pseries/ibmebus.c | 3 +-- arch/powerpc/platforms/pseries/vio.c | 3 +-- drivers/acpi/bus.c| 3 +-- drivers/amba/bus.c| 4 +--- drivers/base/auxiliary.c | 4 +--- drivers/base/isa.c| 4 +--- drivers/base/platform.c | 4 +--- drivers/bcma/main.c | 6 ++ drivers/bus/sunxi-rsb.c | 4 +--- drivers/cxl/core.c| 3 +-- drivers/dax/bus.c | 4 +--- drivers/dma/idxd/sysfs.c | 4 +--- drivers/firewire/core-device.c| 4 +--- drivers/firmware/arm_scmi/bus.c | 4 +--- drivers/firmware/google/coreboot_table.c | 4 +--- drivers/fpga/dfl.c| 4 +--- drivers/hid/hid-core.c| 4 +--- drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +--- drivers/hv/vmbus_drv.c| 5 + drivers/hwtracing/intel_th/core.c | 4 +--- drivers/i2c/i2c-core-base.c | 5 + drivers/i3c/master.c | 4 +--- drivers/input/gameport/gameport.c | 3 +-- drivers/input/serio/serio.c | 3 +-- drivers/ipack/ipack.c | 4 +--- drivers/macintosh/macio_asic.c| 4 +--- drivers/mcb/mcb-core.c| 4 +--- drivers/media/pci/bt8xx/bttv-gpio.c | 3 +-- drivers/memstick/core/memstick.c | 3 +-- drivers/mfd/mcp-core.c| 3 +-- drivers/misc/mei/bus.c| 4 +--- drivers/misc/tifm_core.c | 3 +-- drivers/mmc/core/bus.c| 4 +--- drivers/mmc/core/sdio_bus.c | 4 +--- drivers/net/netdevsim/bus.c | 3 +-- drivers/ntb/core.c| 4 +--- drivers/ntb/ntb_transport.c | 4 +--- drivers/nvdimm/bus.c | 3 +-- drivers/pci/endpoint/pci-epf-core.c | 4 +--- drivers/pci/pci-driver.c | 3 +-- drivers/pcmcia/ds.c | 4 +--- drivers/platform/surface/aggregator/bus.c | 4 +--- drivers/platform/x86/wmi.c| 4 +--- drivers/pnp/driver.c | 3 +-- drivers/rapidio/rio-driver.c | 4 +--- drivers/rpmsg/rpmsg_core.c| 7 ++- drivers/s390/cio/ccwgroup.c | 4 +--- drivers/s390/cio/css.c| 4 +--- drivers/s390/cio/device.c | 4 +--- drivers/s390/cio/scm.c| 4 +--- drivers/s390/crypto/ap_bus.c | 4 +--- drivers/scsi/scsi_debug.c | 3 +-- drivers/siox/si
[PATCH v3 0/5] bus: Make remove callback return void
Hello, this is the final patch set for my effort to make struct bus_type::remove return void. The first four patches contain cleanups that make some of these callbacks (more obviously) always return 0. They are acked by the respective maintainers. Bjorn Helgaas explicitly asked to include the pci patch (#1) into this series, so Greg taking this is fine. I assume the s390 people are fine with Greg taking patches #2 to #4, too, they didn't explicitly said so though. The last patch actually changes the prototype and so touches quite some drivers and has the potential to conflict with future developments, so I consider it beneficial to put these patches into next soon. I expect that it will be Greg who takes the complete series, he already confirmed via irc (for v2) to look into this series. In the last round I failed to send the cover letter to all affected people, sorry for that, this should be fixed now. Best regards Uwe Changes since v2: - Add several acks/review tags - Include patch #1 explicitly - rebase to v5.14-rc1, build test on amd64, arm64 and s390 using allmodconfig Uwe Kleine-König (5): PCI: endpoint: Make struct pci_epf_driver::remove return void s390/cio: Make struct css_driver::remove return void s390/ccwgroup: Drop if with an always false condition s390/scm: Make struct scm_driver::remove return void bus: Make remove callback return void arch/arm/common/locomo.c | 3 +-- arch/arm/common/sa.c | 4 +--- arch/arm/mach-rpc/ecard.c | 4 +--- arch/mips/sgi-ip22/ip22-gio.c | 3 +-- arch/parisc/kernel/drivers.c | 5 ++--- arch/powerpc/platforms/ps3/system-bus.c | 3 +-- arch/powerpc/platforms/pseries/ibmebus.c | 3 +-- arch/powerpc/platforms/pseries/vio.c | 3 +-- arch/s390/include/asm/eadm.h | 2 +- drivers/acpi/bus.c| 3 +-- drivers/amba/bus.c| 4 +--- drivers/base/auxiliary.c | 4 +--- drivers/base/isa.c| 4 +--- drivers/base/platform.c | 4 +--- drivers/bcma/main.c | 6 ++ drivers/bus/sunxi-rsb.c | 4 +--- drivers/cxl/core.c| 3 +-- drivers/dax/bus.c | 4 +--- drivers/dma/idxd/sysfs.c | 4 +--- drivers/firewire/core-device.c| 4 +--- drivers/firmware/arm_scmi/bus.c | 4 +--- drivers/firmware/google/coreboot_table.c | 4 +--- drivers/fpga/dfl.c| 4 +--- drivers/hid/hid-core.c| 4 +--- drivers/hid/intel-ish-hid/ishtp/bus.c | 4 +--- drivers/hv/vmbus_drv.c| 5 + drivers/hwtracing/intel_th/core.c | 4 +--- drivers/i2c/i2c-core-base.c | 5 + drivers/i3c/master.c | 4 +--- drivers/input/gameport/gameport.c | 3 +-- drivers/input/serio/serio.c | 3 +-- drivers/ipack/ipack.c | 4 +--- drivers/macintosh/macio_asic.c| 4 +--- drivers/mcb/mcb-core.c| 4 +--- drivers/media/pci/bt8xx/bttv-gpio.c | 3 +-- drivers/memstick/core/memstick.c | 3 +-- drivers/mfd/mcp-core.c| 3 +-- drivers/misc/mei/bus.c| 4 +--- drivers/misc/tifm_core.c | 3 +-- drivers/mmc/core/bus.c| 4 +--- drivers/mmc/core/sdio_bus.c | 4 +--- drivers/net/netdevsim/bus.c | 3 +-- drivers/ntb/core.c| 4 +--- drivers/ntb/ntb_transport.c | 4 +--- drivers/nvdimm/bus.c | 3 +-- drivers/pci/endpoint/pci-epf-core.c | 7 ++- drivers/pci/pci-driver.c | 3 +-- drivers/pcmcia/ds.c | 4 +--- drivers/platform/surface/aggregator/bus.c | 4 +--- drivers/platform/x86/wmi.c| 4 +--- drivers/pnp/driver.c | 3 +-- drivers/rapidio/rio-driver.c | 4 +--- drivers/rpmsg/rpmsg_core.c| 7 ++- drivers/s390/block/scm_drv.c | 4 +--- drivers/s390/cio/ccwgroup.c | 6 +- drivers/s390/cio/chsc_sch.c | 3 +-- drivers/s390/cio/css.c| 7 +++ drivers/s390/cio/css.h| 2 +- drivers/s390/cio/device.c | 9 +++-- drivers/s390/cio/eadm_sch.c | 4 +--- drivers/s390/cio/scm.c| 5 +++-- drivers/s390/cio/vfio_ccw_drv.c | 3 +-- drivers/s390/crypto/ap_bus.c | 4 +--- drivers/scsi/scsi_debug.c | 3 +-- drivers/siox/siox-core.c | 4 +--- drivers/slimbus/core.c| 4 +--- drivers/soc/qcom/apr.c| 4 +--- drivers/spi/spi.c | 4 +--- drivers/spmi/spmi.c | 3 +-- drivers/ssb/main
Re: [PATCH v2 1/2] sched/topology: Skip updating masks for non-online nodes
On 12/07/21 18:18, Srikar Dronamraju wrote:
> Hi Valentin,
>
>> On 01/07/21 09:45, Srikar Dronamraju wrote:
>> > @@ -1891,12 +1894,30 @@ void sched_init_numa(void)
>> > void sched_domains_numa_masks_set(unsigned int cpu)
>> > {
>>
>> Hmph, so we're playing games with masks of offline nodes - is that really
>> necessary? Your modification of sched_init_numa() still scans all of the
>> nodes (regardless of their online status) to build the distance map, and
>> that is never updated (sched_init_numa() is pretty much an __init
>> function).
>>
>> So AFAICT this is all to cope with topology_span_sane() not applying
>> 'cpu_map' to its masks. That seemed fine to me back when I wrote it, but in
>> light of having bogus distance values for offline nodes, not so much...
>>
>> What about the below instead?
>>
>> ---
>> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
>> index b77ad49dc14f..c2d9caad4aa6 100644
>> --- a/kernel/sched/topology.c
>> +++ b/kernel/sched/topology.c
>> @@ -2075,6 +2075,7 @@ static struct sched_domain *build_sched_domain(struct
>> sched_domain_topology_leve
>> static bool topology_span_sane(struct sched_domain_topology_level *tl,
>>const struct cpumask *cpu_map, int cpu)
>> {
>> +struct cpumask *intersect = sched_domains_tmpmask;
>> int i;
>>
>> /* NUMA levels are allowed to overlap */
>> @@ -2090,14 +2091,17 @@ static bool topology_span_sane(struct
>> sched_domain_topology_level *tl,
>> for_each_cpu(i, cpu_map) {
>> if (i == cpu)
>> continue;
>> +
>> /*
>> - * We should 'and' all those masks with 'cpu_map' to exactly
>> - * match the topology we're about to build, but that can only
>> - * remove CPUs, which only lessens our ability to detect
>> - * overlaps
>> + * We shouldn't have to bother with cpu_map here, unfortunately
>> + * some architectures (powerpc says hello) have to deal with
>> + * offline NUMA nodes reporting bogus distance values. This can
>> + * lead to funky NODE domain spans, but since those are offline
>> + * we can mask them out.
>> */
>> +cpumask_and(intersect, tl->mask(cpu), tl->mask(i));
>> if (!cpumask_equal(tl->mask(cpu), tl->mask(i)) &&
>> -cpumask_intersects(tl->mask(cpu), tl->mask(i)))
>> +cpumask_intersects(intersect, cpu_map))
>> return false;
>> }
>>
>
> Unfortunately this is not helping.
> I tried this patch alone and also with 2/2 patch of this series where
> we update/fill fake topology numbers. However both cases are still failing.
>
Thanks for testing it.
Now, let's take examples from your cover letter:
node distances:
node 0 1 2 3 4 5 6 7
0: 10 20 40 40 40 40 40 40
1: 20 10 40 40 40 40 40 40
2: 40 40 10 20 40 40 40 40
3: 40 40 20 10 40 40 40 40
4: 40 40 40 40 10 20 40 40
5: 40 40 40 40 20 10 40 40
6: 40 40 40 40 40 40 10 20
7: 40 40 40 40 40 40 20 10
But the system boots with just nodes 0 and 1, thus only this distance
matrix is valid:
node 0 1
0: 10 20
1: 20 10
topology_span_sane() is going to use tl->mask(cpu), and as you reported the
NODE topology level should cause issues. Let's assume all offline nodes say
they're 10 distance away from everyone else, and that we have one CPU per
node. This would give us:
NODE->mask(0) == 0,2-7
NODE->mask(1) == 1-7
The intersection is 2-7, we'll trigger the WARN_ON().
Now, with the above snippet, we'll check if that intersection covers any
online CPU. For sched_init_domains(), cpu_map is cpu_active_mask, so we'd
end up with an empty intersection and we shouldn't warn - that's the theory
at least.
Looking at sd_numa_mask(), I think there's a bug with topology_span_sane():
it doesn't run in the right place wrt where sched_domains_curr_level is
updated. Could you try the below on top of the previous snippet?
If that doesn't help, could you share the node distances / topology masks
that lead to the WARN_ON()? Thanks.
---
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index b77ad49dc14f..cda69dfa4065 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1516,13 +1516,6 @@ sd_init(struct sched_domain_topology_level *tl,
int sd_id, sd_weight, sd_flags = 0;
struct cpumask *sd_span;
-#ifdef CONFIG_NUMA
- /*
-* Ugly hack to pass state to sd_numa_mask()...
-*/
- sched_domains_curr_level = tl->numa_level;
-#endif
-
sd_weight = cpumask_weight(tl->mask(cpu));
if (tl->sd_flags)
@@ -2131,7 +2124,12 @@ build_sched_domains(const struct cpumask *cpu_map,
struct sched_domain_attr *att
sd = NULL;
for_each_sd_topology(tl) {
-
+#ifdef CONFIG_NUMA
+
Re: [PATCH v5 0/6] Add support for FORM2 associativity
On 7/13/21 7:57 PM, Daniel Henrique Barboza wrote:
Aneesh,
This series compiles with a configuration made with "pseries_le_defconfig"
but fails with a config based on an existing RHEL8 config.
The reason, which is hinted in the robot replies in patch 4, is that you
defined
a "__vphn_get_associativity" inside a #ifdef CONFIG_PPC_SPLPAR guard but
didn't
define how the function would behave without the config, and you ended up
using the function elsewhere.
This fixes the compilation but I'm not sure if this is what you intended
for this function:
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index c68846fc9550..6e8551d16b7a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -680,6 +680,11 @@ static int vphn_get_nid(long lcpu)
}
#else
+static int __vphn_get_associativity(long lcpu, __be32 *associativity)
+{
+ return -1;
+}
+
static int vphn_get_nid(long unused)
{
return NUMA_NO_NODE;
I'll post a new version of the QEMU FORM2 changes using these patches as
is (with
the above fixup), but I guess you'll want to post a v6.
kernel test robot did report that earlier and I have that fixed in my
local tree. I haven't posted v6 yet because I want to close the review
on the approach with v5 patchset.
-aneesh
Re: [PATCH v5 0/6] Add support for FORM2 associativity
Aneesh,
This series compiles with a configuration made with "pseries_le_defconfig"
but fails with a config based on an existing RHEL8 config.
The reason, which is hinted in the robot replies in patch 4, is that you defined
a "__vphn_get_associativity" inside a #ifdef CONFIG_PPC_SPLPAR guard but didn't
define how the function would behave without the config, and you ended up
using the function elsewhere.
This fixes the compilation but I'm not sure if this is what you intended
for this function:
diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index c68846fc9550..6e8551d16b7a 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -680,6 +680,11 @@ static int vphn_get_nid(long lcpu)
}
#else
+static int __vphn_get_associativity(long lcpu, __be32 *associativity)
+{
+ return -1;
+}
+
static int vphn_get_nid(long unused)
{
return NUMA_NO_NODE;
I'll post a new version of the QEMU FORM2 changes using these patches as is
(with
the above fixup), but I guess you'll want to post a v6.
Thanks,
Daniel
On 6/28/21 12:11 PM, Aneesh Kumar K.V wrote:
Form2 associativity adds a much more flexible NUMA topology layout
than what is provided by Form1. More details can be found in patch 7.
$ numactl -H
...
node distances:
node 0 1 2 3
0: 10 11 222 33
1: 44 10 55 66
2: 77 88 10 99
3: 101 121 132 10
$
After DAX kmem memory add
# numactl -H
available: 5 nodes (0-4)
...
node distances:
node 0 1 2 3 4
0: 10 11 222 33 240
1: 44 10 55 66 255
2: 77 88 10 99 255
3: 101 121 132 10 230
4: 255 255 255 230 10
PAPR SCM now use the numa distance details to find the numa_node and target_node
for the device.
kvaneesh@ubuntu-guest:~$ ndctl list -N -v
[
{
"dev":"namespace0.0",
"mode":"devdax",
"map":"dev",
"size":1071644672,
"uuid":"d333d867-3f57-44c8-b386-d4d3abdc2bf2",
"raw_uuid":"915361ad-fe6a-42dd-848f-d6dc9f5af362",
"daxregion":{
"id":0,
"size":1071644672,
"devices":[
{
"chardev":"dax0.0",
"size":1071644672,
"target_node":4,
"mode":"devdax"
}
]
},
"align":2097152,
"numa_node":3
}
]
kvaneesh@ubuntu-guest:~$
The above output is with a Qemu command line
-numa node,nodeid=4 \
-numa dist,src=0,dst=1,val=11 -numa dist,src=0,dst=2,val=222 -numa
dist,src=0,dst=3,val=33 -numa dist,src=0,dst=4,val=240 \
-numa dist,src=1,dst=0,val=44 -numa dist,src=1,dst=2,val=55 -numa
dist,src=1,dst=3,val=66 -numa dist,src=1,dst=4,val=255 \
-numa dist,src=2,dst=0,val=77 -numa dist,src=2,dst=1,val=88 -numa
dist,src=2,dst=3,val=99 -numa dist,src=2,dst=4,val=255 \
-numa dist,src=3,dst=0,val=101 -numa dist,src=3,dst=1,val=121 -numa
dist,src=3,dst=2,val=132 -numa dist,src=3,dst=4,val=230 \
-numa dist,src=4,dst=0,val=255 -numa dist,src=4,dst=1,val=255 -numa
dist,src=4,dst=2,val=255 -numa dist,src=4,dst=3,val=230 \
-object
memory-backend-file,id=memnvdimm1,prealloc=yes,mem-path=$PMEM_DISK,share=yes,size=${PMEM_SIZE}
\
-device
nvdimm,label-size=128K,memdev=memnvdimm1,id=nvdimm1,slot=4,uuid=72511b67-0b3b-42fd-8d1d-5be3cae8bcaa,node=4
Qemu changes can be found at
https://lore.kernel.org/qemu-devel/20210616011944.2996399-1-danielhb...@gmail.com/
Changes from v4:
* Drop DLPAR related device tree property for now because both Qemu nor PowerVM
will provide the distance details of all possible NUMA nodes during boot.
* Rework numa distance code based on review feedback.
Changes from v3:
* Drop PAPR SCM specific changes and depend completely on NUMA distance
information.
Changes from v2:
* Add nvdimm list to Cc:
* update PATCH 8 commit message.
Changes from v1:
* Update FORM2 documentation.
* rename max_domain_index to max_associativity_domain_index
Aneesh Kumar K.V (6):
powerpc/pseries: rename min_common_depth to primary_domain_index
powerpc/pseries: rename distance_ref_points_depth to
max_associativity_domain_index
powerpc/pseries: Rename TYPE1_AFFINITY to FORM1_AFFINITY
powerpc/pseries: Consolidate different NUMA distance update code paths
powerpc/pseries: Add a helper for form1 cpu distance
powerpc/pseries: Add support for FORM2 associativity
Documentation/powerpc/associativity.rst | 103 +
arch/powerpc/include/asm/firmware.h | 7 +-
arch/powerpc/include/asm/prom.h | 3 +-
arch/powerpc/include/asm/topology.h | 4 +-
arch/powerpc/kernel/prom_init.c | 3 +-
arch/powerpc/mm/numa.c| 415 +-
arch/powerpc/platforms/pseries/firmware.c | 3 +-
arch/powerpc/platforms/pseries/hotplug-cpu.c | 2 +
.../platforms/pseries/hotplug-memory.c| 2 +
arch/powerpc/platforms/pseries/lpar.c | 4 +-
arch/powerpc/platforms/pseries/pseries.h | 1 +
11 files changed, 432 insertions(+), 115 deletions(-)
create
[PATCH v3] fpga: dfl: fme: Fix cpu hotplug issue in performance reporting
The performance reporting driver added cpu hotplug
feature but it didn't add pmu migration call in cpu
offline function.
This can create an issue incase the current designated
cpu being used to collect fme pmu data got offline,
as based on current code we are not migrating fme pmu to
new target cpu. Because of that perf will still try to
fetch data from that offline cpu and hence we will not
get counter data.
Patch fixed this issue by adding pmu_migrate_context call
in fme_perf_offline_cpu function.
Fixes: 724142f8c42a ("fpga: dfl: fme: add performance reporting support")
Tested-by: Xu Yilun
Acked-by: Wu Hao
Signed-off-by: Kajol Jain
Cc: sta...@vger.kernel.org
---
drivers/fpga/dfl-fme-perf.c | 2 ++
1 file changed, 2 insertions(+)
---
Changelog:
v2 -> v3:
- Added Acked-by tag
- Removed comment as suggested by Wu Hao
- Link to patch v2: https://lkml.org/lkml/2021/7/9/143
v1 -> v2:
- Add sta...@vger.kernel.org in cc list
- Link to patch v1: https://lkml.org/lkml/2021/6/28/275
RFC -> PATCH v1
- Remove RFC tag
- Did nits changes on subject and commit message as suggested by Xu Yilun
- Added Tested-by tag
- Link to rfc patch: https://lkml.org/lkml/2021/6/28/112
---
diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
index 4299145ef347..587c82be12f7 100644
--- a/drivers/fpga/dfl-fme-perf.c
+++ b/drivers/fpga/dfl-fme-perf.c
@@ -953,6 +953,8 @@ static int fme_perf_offline_cpu(unsigned int cpu, struct
hlist_node *node)
return 0;
priv->cpu = target;
+ perf_pmu_migrate_context(&priv->pmu, cpu, target);
+
return 0;
}
--
2.31.1
Re: [PATCH v2] fpga: dfl: fme: Fix cpu hotplug issue in performance reporting
On 7/12/21 7:57 AM, Wu, Hao wrote:
>> Subject: [PATCH v2] fpga: dfl: fme: Fix cpu hotplug issue in performance
>> reporting
>>
>> The performance reporting driver added cpu hotplug
>> feature but it didn't add pmu migration call in cpu
>> offline function.
>> This can create an issue incase the current designated
>> cpu being used to collect fme pmu data got offline,
>> as based on current code we are not migrating fme pmu to
>> new target cpu. Because of that perf will still try to
>> fetch data from that offline cpu and hence we will not
>> get counter data.
>>
>> Patch fixed this issue by adding pmu_migrate_context call
>> in fme_perf_offline_cpu function.
>>
>> Fixes: 724142f8c42a ("fpga: dfl: fme: add performance reporting support")
>> Tested-by: Xu Yilun
>> Signed-off-by: Kajol Jain
>
> Thanks for this fixing! And thanks Yilun for the verification too. : )
>
>> Cc: sta...@vger.kernel.org
>> ---
>> drivers/fpga/dfl-fme-perf.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> ---
>> Changelog:
>> v1 -> v2:
>> - Add sta...@vger.kernel.org in cc list
>>
>> RFC -> PATCH v1
>> - Remove RFC tag
>> - Did nits changes on subject and commit message as suggested by Xu Yilun
>> - Added Tested-by tag
>> - Link to rfc patch: https://lkml.org/lkml/2021/6/28/112
>> ---
>> diff --git a/drivers/fpga/dfl-fme-perf.c b/drivers/fpga/dfl-fme-perf.c
>> index 4299145ef347..b9a54583e505 100644
>> --- a/drivers/fpga/dfl-fme-perf.c
>> +++ b/drivers/fpga/dfl-fme-perf.c
>> @@ -953,6 +953,10 @@ static int fme_perf_offline_cpu(unsigned int cpu, struct
>> hlist_node *node)
>> return 0;
>>
>> priv->cpu = target;
>> +
>> +/* Migrate fme_perf pmu events to the new target cpu */
>
> Only one minor item, this line is not needed. : )
>
> After remove it,
> Acked-by: Wu Hao
Hi Wu Hao,
Sure I will remove this comment and send new patch.
Thanks,
Kajol Jain
>
> Thanks
> Hao
>
>> +perf_pmu_migrate_context(&priv->pmu, cpu, target);
>> +
>> return 0;
>> }
>>
>> --
>> 2.31.1
>
