* [PATCH v5] add support for pci in the pvpanic driver
@ 2021-03-16 12:20 Mihai Carabas
2021-03-16 12:20 ` [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code Mihai Carabas
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Mihai Carabas @ 2021-03-16 12:20 UTC (permalink / raw)
To: linux-kernel; +Cc: gregkh, arnd, bobo.shaobowang, rdunlap, Mihai Carabas
This patchset adds support for PCI in the pvpanic driver. The device already
got in qemu [1].
v2:
- mmio -> MMIO, pci -> PCI suggested by Randy Dunlap.
- group pvpanic-common.c and mmio.c in the same module. The intention was to
have only one module and the common code splitted up to be re-used by the
pvpanic-pci module in the next patch.
- add a new patch where add the licenses for the new files (when I moved the
code to mmio.c I preserved the original licenses there)
- properly clean-up PCI device when driver removed.
v3:
- drop patch 3 with licenses and add them when creating files
- create a new patch (2/3) which allowes multiple pvpanic instances
v4:
- fix Makefile in patch 1/3 and 3/3 for pvpanic.o as suggested by Arnd
v5:
- rebase on 5.12
- fix a warning caused by one of the patches
How to test:
Run a VM with latest qemu which has the pvpanic-pci device:
qemu-system-aarch64 -device pvpanic-pci
Generate a panic in the guest:
echo 1 > /proc/sys/kernel/sysrq
echo c > /proc/sysrq-trigger
Observe in the QMP console the panic events received by the device:
{"timestamp": {"seconds": 1613122186, "microseconds": 141729}, "event":
"GUEST_PANICKED", "data": {"action": "pause"}}
{"timestamp": {"seconds": 1613122186, "microseconds": 141833}, "event":
"GUEST_PANICKED", "data": {"action": "poweroff"}}
{"timestamp": {"seconds": 1613122186, "microseconds": 141896}, "event":
"SHUTDOWN", "data": {"guest": true, "reason": "guest-panic"}}
[1] https://github.com/qemu/qemu/commit/9df52f58e76e904fb141b10318362d718f470db2
Mihai Carabas (3):
misc/pvpanic: split-up generic and platform dependent code
misc/pvpanic: probe multiple instances
misc/pvpanic: add PCI driver
drivers/misc/Kconfig | 9 +-
drivers/misc/Makefile | 2 +-
drivers/misc/pvpanic.c | 160 ------------------------------------
drivers/misc/pvpanic/Kconfig | 25 ++++++
drivers/misc/pvpanic/Makefile | 8 ++
drivers/misc/pvpanic/pvpanic-mmio.c | 129 +++++++++++++++++++++++++++++
drivers/misc/pvpanic/pvpanic-pci.c | 102 +++++++++++++++++++++++
drivers/misc/pvpanic/pvpanic.c | 144 ++++++++++++++++++++++++++++++++
drivers/misc/pvpanic/pvpanic.h | 21 +++++
9 files changed, 431 insertions(+), 169 deletions(-)
delete mode 100644 drivers/misc/pvpanic.c
create mode 100644 drivers/misc/pvpanic/Kconfig
create mode 100644 drivers/misc/pvpanic/Makefile
create mode 100644 drivers/misc/pvpanic/pvpanic-mmio.c
create mode 100644 drivers/misc/pvpanic/pvpanic-pci.c
create mode 100644 drivers/misc/pvpanic/pvpanic.c
create mode 100644 drivers/misc/pvpanic/pvpanic.h
--
1.8.3.1
^ permalink raw reply [flat|nested] 12+ messages in thread* [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code 2021-03-16 12:20 [PATCH v5] add support for pci in the pvpanic driver Mihai Carabas @ 2021-03-16 12:20 ` Mihai Carabas 2021-03-16 16:44 ` Greg KH 2021-03-16 12:20 ` [PATCH v5 2/3] misc/pvpanic: probe multiple instances Mihai Carabas 2021-03-16 12:20 ` [PATCH v5 3/3] misc/pvpanic: add PCI driver Mihai Carabas 2 siblings, 1 reply; 12+ messages in thread From: Mihai Carabas @ 2021-03-16 12:20 UTC (permalink / raw) To: linux-kernel; +Cc: gregkh, arnd, bobo.shaobowang, rdunlap, Mihai Carabas Split-up generic and platform dependent code in order to be able to re-use generic event handling code in pvpanic PCI device driver in the next patches. The code from pvpanic.c was split in two new files: - pvpanic.c: generic code that handles pvpanic events - pvpanic-mmio.c: platform/bus dependent code Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> --- drivers/misc/Kconfig | 9 +- drivers/misc/Makefile | 2 +- drivers/misc/pvpanic.c | 160 ------------------------------------ drivers/misc/pvpanic/Kconfig | 19 +++++ drivers/misc/pvpanic/Makefile | 7 ++ drivers/misc/pvpanic/pvpanic-mmio.c | 134 ++++++++++++++++++++++++++++++ drivers/misc/pvpanic/pvpanic.c | 77 +++++++++++++++++ drivers/misc/pvpanic/pvpanic.h | 21 +++++ 8 files changed, 260 insertions(+), 169 deletions(-) delete mode 100644 drivers/misc/pvpanic.c create mode 100644 drivers/misc/pvpanic/Kconfig create mode 100644 drivers/misc/pvpanic/Makefile create mode 100644 drivers/misc/pvpanic/pvpanic-mmio.c create mode 100644 drivers/misc/pvpanic/pvpanic.c create mode 100644 drivers/misc/pvpanic/pvpanic.h diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index f532c59..c5f20e9 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -427,14 +427,6 @@ config MISC_RTSX tristate default MISC_RTSX_PCI || MISC_RTSX_USB -config PVPANIC - tristate "pvpanic device support" - depends on HAS_IOMEM && (ACPI || OF) - help - This driver provides support for the pvpanic device. pvpanic is - a paravirtualized device provided by QEMU; it lets a virtual machine - (guest) communicate panic events to the host. - config HISI_HIKEY_USB tristate "USB GPIO Hub on HiSilicon Hikey 960/970 Platform" depends on (OF && GPIOLIB) || COMPILE_TEST @@ -461,4 +453,5 @@ source "drivers/misc/bcm-vk/Kconfig" source "drivers/misc/cardreader/Kconfig" source "drivers/misc/habanalabs/Kconfig" source "drivers/misc/uacce/Kconfig" +source "drivers/misc/pvpanic/Kconfig" endmenu diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile index 99b6f15..fd5f87e 100644 --- a/drivers/misc/Makefile +++ b/drivers/misc/Makefile @@ -51,7 +51,7 @@ obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o obj-$(CONFIG_OCXL) += ocxl/ obj-$(CONFIG_BCM_VK) += bcm-vk/ obj-y += cardreader/ -obj-$(CONFIG_PVPANIC) += pvpanic.o +obj-$(CONFIG_PVPANIC) += pvpanic/ obj-$(CONFIG_HABANA_AI) += habanalabs/ obj-$(CONFIG_UACCE) += uacce/ obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c deleted file mode 100644 index 9f350e0..00000000 --- a/drivers/misc/pvpanic.c +++ /dev/null @@ -1,160 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0+ -/* - * Pvpanic Device Support - * - * Copyright (C) 2013 Fujitsu. - * Copyright (C) 2018 ZTE. - */ - -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt - -#include <linux/io.h> -#include <linux/kernel.h> -#include <linux/kexec.h> -#include <linux/mod_devicetable.h> -#include <linux/module.h> -#include <linux/platform_device.h> -#include <linux/types.h> - -#include <uapi/misc/pvpanic.h> - -static void __iomem *base; -static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; -static unsigned int events; - -static ssize_t capability_show(struct device *dev, - struct device_attribute *attr, char *buf) -{ - return sysfs_emit(buf, "%x\n", capability); -} -static DEVICE_ATTR_RO(capability); - -static ssize_t events_show(struct device *dev, struct device_attribute *attr, char *buf) -{ - return sysfs_emit(buf, "%x\n", events); -} - -static ssize_t events_store(struct device *dev, struct device_attribute *attr, - const char *buf, size_t count) -{ - unsigned int tmp; - int err; - - err = kstrtouint(buf, 16, &tmp); - if (err) - return err; - - if ((tmp & capability) != tmp) - return -EINVAL; - - events = tmp; - - return count; - -} -static DEVICE_ATTR_RW(events); - -static struct attribute *pvpanic_dev_attrs[] = { - &dev_attr_capability.attr, - &dev_attr_events.attr, - NULL -}; -ATTRIBUTE_GROUPS(pvpanic_dev); - -MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>"); -MODULE_DESCRIPTION("pvpanic device driver"); -MODULE_LICENSE("GPL"); - -static void -pvpanic_send_event(unsigned int event) -{ - if (event & capability & events) - iowrite8(event, base); -} - -static int -pvpanic_panic_notify(struct notifier_block *nb, unsigned long code, - void *unused) -{ - unsigned int event = PVPANIC_PANICKED; - - if (kexec_crash_loaded()) - event = PVPANIC_CRASH_LOADED; - - pvpanic_send_event(event); - - return NOTIFY_DONE; -} - -static struct notifier_block pvpanic_panic_nb = { - .notifier_call = pvpanic_panic_notify, - .priority = 1, /* let this called before broken drm_fb_helper */ -}; - -static int pvpanic_mmio_probe(struct platform_device *pdev) -{ - struct device *dev = &pdev->dev; - struct resource *res; - - res = platform_get_mem_or_io(pdev, 0); - if (!res) - return -EINVAL; - - switch (resource_type(res)) { - case IORESOURCE_IO: - base = devm_ioport_map(dev, res->start, resource_size(res)); - if (!base) - return -ENOMEM; - break; - case IORESOURCE_MEM: - base = devm_ioremap_resource(dev, res); - if (IS_ERR(base)) - return PTR_ERR(base); - break; - default: - return -EINVAL; - } - - /* initlize capability by RDPT */ - capability &= ioread8(base); - events = capability; - - if (capability) - atomic_notifier_chain_register(&panic_notifier_list, - &pvpanic_panic_nb); - - return 0; -} - -static int pvpanic_mmio_remove(struct platform_device *pdev) -{ - - if (capability) - atomic_notifier_chain_unregister(&panic_notifier_list, - &pvpanic_panic_nb); - - return 0; -} - -static const struct of_device_id pvpanic_mmio_match[] = { - { .compatible = "qemu,pvpanic-mmio", }, - {} -}; - -static const struct acpi_device_id pvpanic_device_ids[] = { - { "QEMU0001", 0 }, - { "", 0 } -}; -MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids); - -static struct platform_driver pvpanic_mmio_driver = { - .driver = { - .name = "pvpanic-mmio", - .of_match_table = pvpanic_mmio_match, - .acpi_match_table = pvpanic_device_ids, - .dev_groups = pvpanic_dev_groups, - }, - .probe = pvpanic_mmio_probe, - .remove = pvpanic_mmio_remove, -}; -module_platform_driver(pvpanic_mmio_driver); diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig new file mode 100644 index 00000000..454f1ee --- /dev/null +++ b/drivers/misc/pvpanic/Kconfig @@ -0,0 +1,19 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Pvpanic Kconfig +# +# Copyright (C) 2021 Oracle. +# + +config PVPANIC + bool "pvpanic device support" + help + This option allows to select a specific pvpanic device driver. + pvpanic is a paravirtualized device provided by QEMU; it lets + a virtual machine (guest) communicate panic events to the host. + +config PVPANIC_MMIO + tristate "pvpanic MMIO device support" + depends on HAS_IOMEM && (ACPI || OF) && PVPANIC + help + This driver provides support for the MMIO pvpanic device. diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile new file mode 100644 index 00000000..e12a2dc --- /dev/null +++ b/drivers/misc/pvpanic/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Pvpanic Makefile +# +# Copyright (C) 2021 Oracle. +# +obj-$(CONFIG_PVPANIC_MMIO) += pvpanic.o pvpanic-mmio.o diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c new file mode 100644 index 00000000..801d892 --- /dev/null +++ b/drivers/misc/pvpanic/pvpanic-mmio.c @@ -0,0 +1,134 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Pvpanic MMIO Device Support + * + * Copyright (C) 2013 Fujitsu. + * Copyright (C) 2018 ZTE. + * Copyright (C) 2021 Oracle. + */ + +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/kexec.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/types.h> + +#include <uapi/misc/pvpanic.h> + +#include "pvpanic.h" + +MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>"); +MODULE_DESCRIPTION("pvpanic device driver"); +MODULE_LICENSE("GPL"); + +static void __iomem *base; +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; +static unsigned int events; + +static ssize_t capability_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%x\n", capability); +} +static DEVICE_ATTR_RO(capability); + +static ssize_t events_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%x\n", events); +} + +static ssize_t events_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + unsigned int tmp; + int err; + + err = kstrtouint(buf, 16, &tmp); + if (err) + return err; + + if ((tmp & capability) != tmp) + return -EINVAL; + + events = tmp; + + pvpanic_set_events(events); + + return count; + +} +static DEVICE_ATTR_RW(events); + +static struct attribute *pvpanic_mmio_dev_attrs[] = { + &dev_attr_capability.attr, + &dev_attr_events.attr, + NULL +}; +ATTRIBUTE_GROUPS(pvpanic_mmio_dev); + +static int pvpanic_mmio_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + struct resource *res; + void __iomem *base; + + res = platform_get_mem_or_io(pdev, 0); + if (!res) + return -EINVAL; + + switch (resource_type(res)) { + case IORESOURCE_IO: + base = devm_ioport_map(dev, res->start, resource_size(res)); + if (!base) + return -ENOMEM; + break; + case IORESOURCE_MEM: + base = devm_ioremap_resource(dev, res); + if (IS_ERR(base)) + return PTR_ERR(base); + break; + default: + return -EINVAL; + } + + /* initlize capability by RDPT */ + capability &= ioread8(base); + events = capability; + + pvpanic_probe(base, capability); + + return 0; +} + +static int pvpanic_mmio_remove(struct platform_device *pdev) +{ + + pvpanic_remove(); + + return 0; +} + +static const struct of_device_id pvpanic_mmio_match[] = { + { .compatible = "qemu,pvpanic-mmio", }, + {} +}; + +static const struct acpi_device_id pvpanic_device_ids[] = { + { "QEMU0001", 0 }, + { "", 0 } +}; +MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids); + +static struct platform_driver pvpanic_mmio_driver = { + .driver = { + .name = "pvpanic-mmio", + .of_match_table = pvpanic_mmio_match, + .acpi_match_table = pvpanic_device_ids, + .dev_groups = pvpanic_mmio_dev_groups, + }, + .probe = pvpanic_mmio_probe, + .remove = pvpanic_mmio_remove, +}; +module_platform_driver(pvpanic_mmio_driver); diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c new file mode 100644 index 00000000..11d029c --- /dev/null +++ b/drivers/misc/pvpanic/pvpanic.c @@ -0,0 +1,77 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Pvpanic Device Support + * + * Copyright (C) 2013 Fujitsu. + * Copyright (C) 2018 ZTE. + * Copyright (C) 2021 Oracle. + */ + +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/kexec.h> +#include <linux/mod_devicetable.h> +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/types.h> + +#include <uapi/misc/pvpanic.h> + +#include "pvpanic.h" + +static void __iomem *base; +static unsigned int capability; +static unsigned int events; + +static void +pvpanic_send_event(unsigned int event) +{ + if (event & capability & events) + iowrite8(event, base); +} + +static int +pvpanic_panic_notify(struct notifier_block *nb, unsigned long code, + void *unused) +{ + unsigned int event = PVPANIC_PANICKED; + + if (kexec_crash_loaded()) + event = PVPANIC_CRASH_LOADED; + + pvpanic_send_event(event); + + return NOTIFY_DONE; +} + +static struct notifier_block pvpanic_panic_nb = { + .notifier_call = pvpanic_panic_notify, + .priority = 1, /* let this called before broken drm_fb_helper */ +}; + +void pvpanic_probe(void __iomem *pbase, unsigned int dev_cap) +{ + base = pbase; + capability = dev_cap; + events = capability; + + if (capability) + atomic_notifier_chain_register(&panic_notifier_list, + &pvpanic_panic_nb); +} +EXPORT_SYMBOL_GPL(pvpanic_probe); + +void pvpanic_remove(void) +{ + if (capability) + atomic_notifier_chain_unregister(&panic_notifier_list, + &pvpanic_panic_nb); + base = NULL; +} +EXPORT_SYMBOL_GPL(pvpanic_remove); + +void pvpanic_set_events(unsigned int dev_events) +{ + events = dev_events; +} +EXPORT_SYMBOL_GPL(pvpanic_set_events); diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h new file mode 100644 index 00000000..0e90f4a --- /dev/null +++ b/drivers/misc/pvpanic/pvpanic.h @@ -0,0 +1,21 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Pvpanic Device Support + * + * Copyright (C) 2021 Oracle. + */ + +#ifndef PVPANIC_H_ +#define PVPANIC_H_ + +#ifdef pr_fmt +#undef pr_fmt +#endif + +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +void pvpanic_probe(void __iomem *base, unsigned int dev_cap); +void pvpanic_remove(void); +void pvpanic_set_events(unsigned int dev_events); + +#endif /* PVPANIC_H_ */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code 2021-03-16 12:20 ` [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code Mihai Carabas @ 2021-03-16 16:44 ` Greg KH 0 siblings, 0 replies; 12+ messages in thread From: Greg KH @ 2021-03-16 16:44 UTC (permalink / raw) To: Mihai Carabas; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap On Tue, Mar 16, 2021 at 02:20:27PM +0200, Mihai Carabas wrote: > +#ifdef pr_fmt > +#undef pr_fmt > +#endif > + > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt This isn't even needed given you do not have any pr_* calls in this code. As you shouldn't this is driver stuff, you should always use the dev_* calls instead. But you can delete that later, this is fine for now. However generally you do not set this value in a .h file, but rather in the individual .c files if you really need it. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 2/3] misc/pvpanic: probe multiple instances 2021-03-16 12:20 [PATCH v5] add support for pci in the pvpanic driver Mihai Carabas 2021-03-16 12:20 ` [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code Mihai Carabas @ 2021-03-16 12:20 ` Mihai Carabas 2021-03-16 16:46 ` Greg KH 2021-03-16 12:20 ` [PATCH v5 3/3] misc/pvpanic: add PCI driver Mihai Carabas 2 siblings, 1 reply; 12+ messages in thread From: Mihai Carabas @ 2021-03-16 12:20 UTC (permalink / raw) To: linux-kernel; +Cc: gregkh, arnd, bobo.shaobowang, rdunlap, Mihai Carabas Create the mecahism that allows multiple pvpanic instances to call pvpanic_probe and receive panic events. A global list will retain all the mapped addresses where to write panic events. Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> --- drivers/misc/pvpanic/pvpanic-mmio.c | 11 ++-- drivers/misc/pvpanic/pvpanic.c | 105 +++++++++++++++++++++++++++++------- drivers/misc/pvpanic/pvpanic.h | 6 +-- 3 files changed, 92 insertions(+), 30 deletions(-) diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c index 801d892..62ab8d7 100644 --- a/drivers/misc/pvpanic/pvpanic-mmio.c +++ b/drivers/misc/pvpanic/pvpanic-mmio.c @@ -54,7 +54,7 @@ static ssize_t events_store(struct device *dev, struct device_attribute *attr, events = tmp; - pvpanic_set_events(events); + pvpanic_set_events(base, events); return count; @@ -72,7 +72,6 @@ static int pvpanic_mmio_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; struct resource *res; - void __iomem *base; res = platform_get_mem_or_io(pdev, 0); if (!res) @@ -97,17 +96,13 @@ static int pvpanic_mmio_probe(struct platform_device *pdev) capability &= ioread8(base); events = capability; - pvpanic_probe(base, capability); - - return 0; + return pvpanic_probe(base, capability); } static int pvpanic_mmio_remove(struct platform_device *pdev) { - pvpanic_remove(); - - return 0; + return pvpanic_remove(base); } static const struct of_device_id pvpanic_mmio_match[] = { diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c index 11d029c..73a82d9 100644 --- a/drivers/misc/pvpanic/pvpanic.c +++ b/drivers/misc/pvpanic/pvpanic.c @@ -14,20 +14,35 @@ #include <linux/module.h> #include <linux/platform_device.h> #include <linux/types.h> +#include <linux/cdev.h> +#include <linux/list.h> +#include <linux/slab.h> #include <uapi/misc/pvpanic.h> #include "pvpanic.h" -static void __iomem *base; -static unsigned int capability; -static unsigned int events; +struct pvpanic_instance { + void __iomem *base; + unsigned int capability; + unsigned int events; + struct list_head list; +}; + +struct list_head pvpanic_list; +spinlock_t pvpanic_lock; static void pvpanic_send_event(unsigned int event) { - if (event & capability & events) - iowrite8(event, base); + struct pvpanic_instance *pi_cur; + + spin_lock(&pvpanic_lock); + list_for_each_entry(pi_cur, &pvpanic_list, list) { + if (event & pi_cur->capability & pi_cur->events) + iowrite8(event, pi_cur->base); + } + spin_unlock(&pvpanic_lock); } static int @@ -49,29 +64,81 @@ .priority = 1, /* let this called before broken drm_fb_helper */ }; -void pvpanic_probe(void __iomem *pbase, unsigned int dev_cap) +int pvpanic_probe(void __iomem *pbase, unsigned int dev_cap) { - base = pbase; - capability = dev_cap; - events = capability; + struct pvpanic_instance *pi; + + if (!pbase) + return -EINVAL; + + pi = kmalloc(sizeof(*pi), GFP_ATOMIC); + if (!pi) + return -ENOMEM; + + pi->base = pbase; + pi->capability = dev_cap; + pi->events = dev_cap; + spin_lock(&pvpanic_lock); + list_add(&pi->list, &pvpanic_list); + spin_unlock(&pvpanic_lock); - if (capability) - atomic_notifier_chain_register(&panic_notifier_list, - &pvpanic_panic_nb); + return 0; } EXPORT_SYMBOL_GPL(pvpanic_probe); -void pvpanic_remove(void) +int pvpanic_remove(void __iomem *pbase) { - if (capability) - atomic_notifier_chain_unregister(&panic_notifier_list, - &pvpanic_panic_nb); - base = NULL; + struct pvpanic_instance *pi_cur, *pi_next; + + if (!pbase) + return -EINVAL; + + spin_lock(&pvpanic_lock); + list_for_each_entry_safe(pi_cur, pi_next, &pvpanic_list, list) { + if (pi_cur->base == pbase) { + list_del(&pi_cur->list); + kfree(pi_cur); + break; + } + } + spin_unlock(&pvpanic_lock); + + return 0; } EXPORT_SYMBOL_GPL(pvpanic_remove); -void pvpanic_set_events(unsigned int dev_events) + +void pvpanic_set_events(void __iomem *pbase, unsigned int dev_events) { - events = dev_events; + struct pvpanic_instance *pi_cur; + + spin_lock(&pvpanic_lock); + list_for_each_entry(pi_cur, &pvpanic_list, list) { + if (pi_cur->base == pbase) + pi_cur->events = dev_events; + } + spin_unlock(&pvpanic_lock); + } EXPORT_SYMBOL_GPL(pvpanic_set_events); + +static int pvpanic_init(void) +{ + INIT_LIST_HEAD(&pvpanic_list); + spin_lock_init(&pvpanic_lock); + + atomic_notifier_chain_register(&panic_notifier_list, + &pvpanic_panic_nb); + + return 0; +} + +static void pvpanic_exit(void) +{ + atomic_notifier_chain_unregister(&panic_notifier_list, + &pvpanic_panic_nb); + +} + +module_init(pvpanic_init); +module_exit(pvpanic_exit); diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h index 0e90f4a..e1d15fd 100644 --- a/drivers/misc/pvpanic/pvpanic.h +++ b/drivers/misc/pvpanic/pvpanic.h @@ -14,8 +14,8 @@ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt -void pvpanic_probe(void __iomem *base, unsigned int dev_cap); -void pvpanic_remove(void); -void pvpanic_set_events(unsigned int dev_events); +int pvpanic_probe(void __iomem *base, unsigned int dev_cap); +int pvpanic_remove(void __iomem *base); +void pvpanic_set_events(void __iomem *base, unsigned int dev_events); #endif /* PVPANIC_H_ */ -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/3] misc/pvpanic: probe multiple instances 2021-03-16 12:20 ` [PATCH v5 2/3] misc/pvpanic: probe multiple instances Mihai Carabas @ 2021-03-16 16:46 ` Greg KH 0 siblings, 0 replies; 12+ messages in thread From: Greg KH @ 2021-03-16 16:46 UTC (permalink / raw) To: Mihai Carabas; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap On Tue, Mar 16, 2021 at 02:20:28PM +0200, Mihai Carabas wrote: > Create the mecahism that allows multiple pvpanic instances to call > pvpanic_probe and receive panic events. A global list will retain all the > mapped addresses where to write panic events. > > Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> > --- > drivers/misc/pvpanic/pvpanic-mmio.c | 11 ++-- > drivers/misc/pvpanic/pvpanic.c | 105 +++++++++++++++++++++++++++++------- > drivers/misc/pvpanic/pvpanic.h | 6 +-- > 3 files changed, 92 insertions(+), 30 deletions(-) > > diff --git a/drivers/misc/pvpanic/pvpanic-mmio.c b/drivers/misc/pvpanic/pvpanic-mmio.c > index 801d892..62ab8d7 100644 > --- a/drivers/misc/pvpanic/pvpanic-mmio.c > +++ b/drivers/misc/pvpanic/pvpanic-mmio.c > @@ -54,7 +54,7 @@ static ssize_t events_store(struct device *dev, struct device_attribute *attr, > > events = tmp; > > - pvpanic_set_events(events); > + pvpanic_set_events(base, events); > > return count; > > @@ -72,7 +72,6 @@ static int pvpanic_mmio_probe(struct platform_device *pdev) > { > struct device *dev = &pdev->dev; > struct resource *res; > - void __iomem *base; > > res = platform_get_mem_or_io(pdev, 0); > if (!res) > @@ -97,17 +96,13 @@ static int pvpanic_mmio_probe(struct platform_device *pdev) > capability &= ioread8(base); > events = capability; > > - pvpanic_probe(base, capability); > - > - return 0; > + return pvpanic_probe(base, capability); > } > > static int pvpanic_mmio_remove(struct platform_device *pdev) > { > > - pvpanic_remove(); > - > - return 0; > + return pvpanic_remove(base); > } > > static const struct of_device_id pvpanic_mmio_match[] = { > diff --git a/drivers/misc/pvpanic/pvpanic.c b/drivers/misc/pvpanic/pvpanic.c > index 11d029c..73a82d9 100644 > --- a/drivers/misc/pvpanic/pvpanic.c > +++ b/drivers/misc/pvpanic/pvpanic.c > @@ -14,20 +14,35 @@ > #include <linux/module.h> > #include <linux/platform_device.h> > #include <linux/types.h> > +#include <linux/cdev.h> > +#include <linux/list.h> > +#include <linux/slab.h> > > #include <uapi/misc/pvpanic.h> > > #include "pvpanic.h" > > -static void __iomem *base; > -static unsigned int capability; > -static unsigned int events; > +struct pvpanic_instance { > + void __iomem *base; > + unsigned int capability; > + unsigned int events; > + struct list_head list; > +}; > + > +struct list_head pvpanic_list; > +spinlock_t pvpanic_lock; > > static void > pvpanic_send_event(unsigned int event) > { > - if (event & capability & events) > - iowrite8(event, base); > + struct pvpanic_instance *pi_cur; > + > + spin_lock(&pvpanic_lock); > + list_for_each_entry(pi_cur, &pvpanic_list, list) { > + if (event & pi_cur->capability & pi_cur->events) > + iowrite8(event, pi_cur->base); > + } > + spin_unlock(&pvpanic_lock); > } > > static int > @@ -49,29 +64,81 @@ > .priority = 1, /* let this called before broken drm_fb_helper */ > }; > > -void pvpanic_probe(void __iomem *pbase, unsigned int dev_cap) > +int pvpanic_probe(void __iomem *pbase, unsigned int dev_cap) > { > - base = pbase; > - capability = dev_cap; > - events = capability; > + struct pvpanic_instance *pi; > + > + if (!pbase) > + return -EINVAL; > + > + pi = kmalloc(sizeof(*pi), GFP_ATOMIC); > + if (!pi) > + return -ENOMEM; > + > + pi->base = pbase; > + pi->capability = dev_cap; > + pi->events = dev_cap; > + spin_lock(&pvpanic_lock); > + list_add(&pi->list, &pvpanic_list); > + spin_unlock(&pvpanic_lock); > > - if (capability) > - atomic_notifier_chain_register(&panic_notifier_list, > - &pvpanic_panic_nb); > + return 0; > } > EXPORT_SYMBOL_GPL(pvpanic_probe); > > -void pvpanic_remove(void) > +int pvpanic_remove(void __iomem *pbase) > { > - if (capability) > - atomic_notifier_chain_unregister(&panic_notifier_list, > - &pvpanic_panic_nb); > - base = NULL; > + struct pvpanic_instance *pi_cur, *pi_next; > + > + if (!pbase) > + return -EINVAL; > + > + spin_lock(&pvpanic_lock); > + list_for_each_entry_safe(pi_cur, pi_next, &pvpanic_list, list) { > + if (pi_cur->base == pbase) { > + list_del(&pi_cur->list); > + kfree(pi_cur); > + break; > + } > + } > + spin_unlock(&pvpanic_lock); > + > + return 0; > } As "remove" can not fail, this should just stay a void function. People are working to remove the int return value from remove calls in the kernel right now, please do not add new ones if at all possible. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 12:20 [PATCH v5] add support for pci in the pvpanic driver Mihai Carabas 2021-03-16 12:20 ` [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code Mihai Carabas 2021-03-16 12:20 ` [PATCH v5 2/3] misc/pvpanic: probe multiple instances Mihai Carabas @ 2021-03-16 12:20 ` Mihai Carabas 2021-03-16 16:48 ` Greg KH 2021-03-17 0:02 ` kernel test robot 2 siblings, 2 replies; 12+ messages in thread From: Mihai Carabas @ 2021-03-16 12:20 UTC (permalink / raw) To: linux-kernel; +Cc: gregkh, arnd, bobo.shaobowang, rdunlap, Mihai Carabas Add support for pvpanic PCI device added in qemu [1]. At probe time, obtain the address where to read/write pvpanic events and pass it to the generic handling code. Will follow the same logic as pvpanic MMIO device driver. At remove time, unmap base address and disable PCI device. [1] https://github.com/qemu/qemu/commit/9df52f58e76e904fb141b10318362d718f470db2 Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> --- drivers/misc/pvpanic/Kconfig | 6 +++ drivers/misc/pvpanic/Makefile | 1 + drivers/misc/pvpanic/pvpanic-pci.c | 102 +++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 drivers/misc/pvpanic/pvpanic-pci.c diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig index 454f1ee..9a081ed 100644 --- a/drivers/misc/pvpanic/Kconfig +++ b/drivers/misc/pvpanic/Kconfig @@ -17,3 +17,9 @@ config PVPANIC_MMIO depends on HAS_IOMEM && (ACPI || OF) && PVPANIC help This driver provides support for the MMIO pvpanic device. + +config PVPANIC_PCI + tristate "pvpanic PCI device support" + depends on PCI && PVPANIC + help + This driver provides support for the PCI pvpanic device. diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile index e12a2dc..9471df7 100644 --- a/drivers/misc/pvpanic/Makefile +++ b/drivers/misc/pvpanic/Makefile @@ -5,3 +5,4 @@ # Copyright (C) 2021 Oracle. # obj-$(CONFIG_PVPANIC_MMIO) += pvpanic.o pvpanic-mmio.o +obj-$(CONFIG_PVPANIC_PCI) += pvpanic.o pvpanic-pci.o diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c new file mode 100644 index 00000000..27526d3 --- /dev/null +++ b/drivers/misc/pvpanic/pvpanic-pci.c @@ -0,0 +1,102 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Pvpanic PCI Device Support + * + * Copyright (C) 2021 Oracle. + */ + +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/pci.h> +#include <linux/types.h> +#include "pvpanic.h" + +#define PCI_VENDOR_ID_REDHAT 0x1b36 +#define PCI_DEVICE_ID_REDHAT_PVPANIC 0x0011 + +static void __iomem *base; +static const struct pci_device_id pvpanic_pci_id_tbl[] = { + { PCI_DEVICE(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_PVPANIC),}, + {} +}; +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; +static unsigned int events; + +static ssize_t capability_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%x\n", capability); +} +static DEVICE_ATTR_RO(capability); + +static ssize_t events_show(struct device *dev, struct device_attribute *attr, char *buf) +{ + return sysfs_emit(buf, "%x\n", events); +} + +static ssize_t events_store(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + unsigned int tmp; + int err; + + err = kstrtouint(buf, 16, &tmp); + if (err) + return err; + + if ((tmp & capability) != tmp) + return -EINVAL; + + events = tmp; + + pvpanic_set_events(base, events); + + return count; + +} +static DEVICE_ATTR_RW(events); + +static struct attribute *pvpanic_pci_dev_attrs[] = { + &dev_attr_capability.attr, + &dev_attr_events.attr, + NULL +}; +ATTRIBUTE_GROUPS(pvpanici_pci_dev); + +static int pvpanic_pci_probe(struct pci_dev *pdev, + const struct pci_device_id *ent) +{ + int ret; + struct resource res; + + ret = pci_enable_device(pdev); + if (ret < 0) + return ret; + + base = pci_iomap(pdev, 0, 0); + if (IS_ERR(base)) + return PTR_ERR(base); + + /* initlize capability by RDPT */ + capability &= ioread8(base); + events = capability; + + return pvpanic_probe(base, capability); +} + +static void pvpanic_pci_remove(struct pci_dev *pdev) +{ + pvpanic_remove(base); + iounmap(base); + pci_disable_device(pdev); +} + +static struct pci_driver pvpanic_pci_driver = { + .name = "pvpanic-pci", + .id_table = pvpanic_pci_id_tbl, + .groups = pvpanic_pci_dev_groups, + .probe = pvpanic_pci_probe, + .remove = pvpanic_pci_remove, +}; + +module_pci_driver(pvpanic_pci_driver); -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 12:20 ` [PATCH v5 3/3] misc/pvpanic: add PCI driver Mihai Carabas @ 2021-03-16 16:48 ` Greg KH 2021-03-16 17:08 ` Mihai Carabas 2021-03-17 0:02 ` kernel test robot 1 sibling, 1 reply; 12+ messages in thread From: Greg KH @ 2021-03-16 16:48 UTC (permalink / raw) To: Mihai Carabas; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap On Tue, Mar 16, 2021 at 02:20:29PM +0200, Mihai Carabas wrote: > Add support for pvpanic PCI device added in qemu [1]. At probe time, obtain the > address where to read/write pvpanic events and pass it to the generic handling > code. Will follow the same logic as pvpanic MMIO device driver. At remove time, > unmap base address and disable PCI device. > > [1] https://github.com/qemu/qemu/commit/9df52f58e76e904fb141b10318362d718f470db2 > > Signed-off-by: Mihai Carabas <mihai.carabas@oracle.com> > --- > drivers/misc/pvpanic/Kconfig | 6 +++ > drivers/misc/pvpanic/Makefile | 1 + > drivers/misc/pvpanic/pvpanic-pci.c | 102 +++++++++++++++++++++++++++++++++++++ > 3 files changed, 109 insertions(+) > create mode 100644 drivers/misc/pvpanic/pvpanic-pci.c > > diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig > index 454f1ee..9a081ed 100644 > --- a/drivers/misc/pvpanic/Kconfig > +++ b/drivers/misc/pvpanic/Kconfig > @@ -17,3 +17,9 @@ config PVPANIC_MMIO > depends on HAS_IOMEM && (ACPI || OF) && PVPANIC > help > This driver provides support for the MMIO pvpanic device. > + > +config PVPANIC_PCI > + tristate "pvpanic PCI device support" > + depends on PCI && PVPANIC > + help > + This driver provides support for the PCI pvpanic device. Please provide a bit more information here. > diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile > index e12a2dc..9471df7 100644 > --- a/drivers/misc/pvpanic/Makefile > +++ b/drivers/misc/pvpanic/Makefile > @@ -5,3 +5,4 @@ > # Copyright (C) 2021 Oracle. > # > obj-$(CONFIG_PVPANIC_MMIO) += pvpanic.o pvpanic-mmio.o > +obj-$(CONFIG_PVPANIC_PCI) += pvpanic.o pvpanic-pci.o > diff --git a/drivers/misc/pvpanic/pvpanic-pci.c b/drivers/misc/pvpanic/pvpanic-pci.c > new file mode 100644 > index 00000000..27526d3 > --- /dev/null > +++ b/drivers/misc/pvpanic/pvpanic-pci.c > @@ -0,0 +1,102 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +/* > + * Pvpanic PCI Device Support > + * > + * Copyright (C) 2021 Oracle. > + */ > + > +#include <linux/kernel.h> > +#include <linux/module.h> > +#include <linux/pci.h> > +#include <linux/types.h> > +#include "pvpanic.h" > + > +#define PCI_VENDOR_ID_REDHAT 0x1b36 > +#define PCI_DEVICE_ID_REDHAT_PVPANIC 0x0011 > + > +static void __iomem *base; > +static const struct pci_device_id pvpanic_pci_id_tbl[] = { > + { PCI_DEVICE(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_PVPANIC),}, Why the ")}"? Shouldn't it just be "}"? > + {} > +}; > +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; > +static unsigned int events; > + > +static ssize_t capability_show(struct device *dev, > + struct device_attribute *attr, char *buf) > +{ > + return sysfs_emit(buf, "%x\n", capability); A global capability for all devices? No, this needs to be a per-device attttribute as you are showing it to userspace as such. > +} > +static DEVICE_ATTR_RO(capability); > + > +static ssize_t events_show(struct device *dev, struct device_attribute *attr, char *buf) > +{ > + return sysfs_emit(buf, "%x\n", events); Same here, this is not global for the whole module. > +} > + > +static ssize_t events_store(struct device *dev, struct device_attribute *attr, > + const char *buf, size_t count) > +{ > + unsigned int tmp; > + int err; > + > + err = kstrtouint(buf, 16, &tmp); > + if (err) > + return err; > + > + if ((tmp & capability) != tmp) > + return -EINVAL; > + > + events = tmp; > + > + pvpanic_set_events(base, events); > + > + return count; > + > +} > +static DEVICE_ATTR_RW(events); > + > +static struct attribute *pvpanic_pci_dev_attrs[] = { > + &dev_attr_capability.attr, > + &dev_attr_events.attr, > + NULL > +}; > +ATTRIBUTE_GROUPS(pvpanici_pci_dev); You did not document these new sysfs files in Documentation/ABI/ so it's hard to judge what they do. Please do so next version. thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 16:48 ` Greg KH @ 2021-03-16 17:08 ` Mihai Carabas 2021-03-16 17:29 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: Mihai Carabas @ 2021-03-16 17:08 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap ..snip.. >> +}; >> +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; >> +static unsigned int events; >> + >> +static ssize_t capability_show(struct device *dev, >> + struct device_attribute *attr, char *buf) >> +{ >> + return sysfs_emit(buf, "%x\n", capability); > A global capability for all devices? No, this needs to be a per-device > attttribute as you are showing it to userspace as such. We can only have a pvpanic-pci device. This is necessary in that case? ...snip.. >> +static struct attribute *pvpanic_pci_dev_attrs[] = { >> + &dev_attr_capability.attr, >> + &dev_attr_events.attr, >> + NULL >> +}; >> +ATTRIBUTE_GROUPS(pvpanici_pci_dev); > You did not document these new sysfs files in Documentation/ABI/ so it's > hard to judge what they do. Please do so next version. This appeared after 5.12 rebase on pvpanic-mmio. I did this changes to pvpanic-pci to be on the same page as pvpanic-mmio which is described here: Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic. Thank you, Mihai ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 17:08 ` Mihai Carabas @ 2021-03-16 17:29 ` Greg KH 2021-03-16 18:02 ` Mihai Carabas 0 siblings, 1 reply; 12+ messages in thread From: Greg KH @ 2021-03-16 17:29 UTC (permalink / raw) To: Mihai Carabas; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap On Tue, Mar 16, 2021 at 07:08:38PM +0200, Mihai Carabas wrote: > ..snip.. > > > +}; > > > +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; > > > +static unsigned int events; > > > + > > > +static ssize_t capability_show(struct device *dev, > > > + struct device_attribute *attr, char *buf) > > > +{ > > > + return sysfs_emit(buf, "%x\n", capability); > > A global capability for all devices? No, this needs to be a per-device > > attttribute as you are showing it to userspace as such. > > We can only have a pvpanic-pci device. This is necessary in that case? Why would it not be? Why constrain yourself to something that someone will only have to fix sometime in the future? > ...snip.. > > > +static struct attribute *pvpanic_pci_dev_attrs[] = { > > > + &dev_attr_capability.attr, > > > + &dev_attr_events.attr, > > > + NULL > > > +}; > > > +ATTRIBUTE_GROUPS(pvpanici_pci_dev); > > You did not document these new sysfs files in Documentation/ABI/ so it's > > hard to judge what they do. Please do so next version. > > This appeared after 5.12 rebase on pvpanic-mmio. I did this changes to > pvpanic-pci to be on the same page as pvpanic-mmio which is described here: > Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic. Ah, missed that, my fault, sorry. But you also now do this for the other device type right? So does the documentation need to reflect that too? thanks, greg k-h ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 17:29 ` Greg KH @ 2021-03-16 18:02 ` Mihai Carabas 2021-03-16 18:16 ` Greg KH 0 siblings, 1 reply; 12+ messages in thread From: Mihai Carabas @ 2021-03-16 18:02 UTC (permalink / raw) To: Greg KH; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap La 16.03.2021 19:29, Greg KH a scris: > On Tue, Mar 16, 2021 at 07:08:38PM +0200, Mihai Carabas wrote: >> ..snip.. >>>> +}; >>>> +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; >>>> +static unsigned int events; >>>> + >>>> +static ssize_t capability_show(struct device *dev, >>>> + struct device_attribute *attr, char *buf) >>>> +{ >>>> + return sysfs_emit(buf, "%x\n", capability); >>> A global capability for all devices? No, this needs to be a per-device >>> attttribute as you are showing it to userspace as such. >> We can only have a pvpanic-pci device. This is necessary in that case? > Why would it not be? Why constrain yourself to something that someone > will only have to fix sometime in the future? Agree. But why this got in in Jan2021 for pvpanic-mmio.c (ex. pvpanic.c) like this :)? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 18:02 ` Mihai Carabas @ 2021-03-16 18:16 ` Greg KH 0 siblings, 0 replies; 12+ messages in thread From: Greg KH @ 2021-03-16 18:16 UTC (permalink / raw) To: Mihai Carabas; +Cc: linux-kernel, arnd, bobo.shaobowang, rdunlap On Tue, Mar 16, 2021 at 08:02:14PM +0200, Mihai Carabas wrote: > La 16.03.2021 19:29, Greg KH a scris: > > On Tue, Mar 16, 2021 at 07:08:38PM +0200, Mihai Carabas wrote: > > > ..snip.. > > > > > +}; > > > > > +static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; > > > > > +static unsigned int events; > > > > > + > > > > > +static ssize_t capability_show(struct device *dev, > > > > > + struct device_attribute *attr, char *buf) > > > > > +{ > > > > > + return sysfs_emit(buf, "%x\n", capability); > > > > A global capability for all devices? No, this needs to be a per-device > > > > attttribute as you are showing it to userspace as such. > > > We can only have a pvpanic-pci device. This is necessary in that case? > > Why would it not be? Why constrain yourself to something that someone > > will only have to fix sometime in the future? > > Agree. But why this got in in Jan2021 for pvpanic-mmio.c (ex. pvpanic.c) > like this :)? > Because I didn't object strong enough :) ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/3] misc/pvpanic: add PCI driver 2021-03-16 12:20 ` [PATCH v5 3/3] misc/pvpanic: add PCI driver Mihai Carabas 2021-03-16 16:48 ` Greg KH @ 2021-03-17 0:02 ` kernel test robot 1 sibling, 0 replies; 12+ messages in thread From: kernel test robot @ 2021-03-17 0:02 UTC (permalink / raw) To: Mihai Carabas, linux-kernel Cc: kbuild-all, gregkh, arnd, bobo.shaobowang, rdunlap, Mihai Carabas [-- Attachment #1: Type: text/plain, Size: 6885 bytes --] Hi Mihai, Thank you for the patch! Yet something to improve: [auto build test ERROR on linux/master] [also build test ERROR on linus/master v5.12-rc3 next-20210316] [cannot apply to char-misc/char-misc-testing soc/for-next] [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/Mihai-Carabas/misc-pvpanic-split-up-generic-and-platform-dependent-code/20210316-212047 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git a74e6a014c9d4d4161061f770c9b4f98372ac778 config: arc-allyesconfig (attached as .config) compiler: arceb-elf-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/6e945cc8314339e4fd5fc2076a2a2abbd32fc1da git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Mihai-Carabas/misc-pvpanic-split-up-generic-and-platform-dependent-code/20210316-212047 git checkout 6e945cc8314339e4fd5fc2076a2a2abbd32fc1da # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=arc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> drivers/misc/pvpanic/pvpanic-pci.c:22:34: error: 'PVPANIC_PANICKED' undeclared here (not in a function) 22 | static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; | ^~~~~~~~~~~~~~~~ >> drivers/misc/pvpanic/pvpanic-pci.c:22:53: error: 'PVPANIC_CRASH_LOADED' undeclared here (not in a function) 22 | static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; | ^~~~~~~~~~~~~~~~~~~~ In file included from include/linux/kobject.h:20, from include/linux/module.h:20, from drivers/misc/pvpanic/pvpanic-pci.c:9: >> drivers/misc/pvpanic/pvpanic-pci.c:64:18: error: 'pvpanici_pci_dev_attrs' undeclared here (not in a function); did you mean 'pvpanic_pci_dev_attrs'? 64 | ATTRIBUTE_GROUPS(pvpanici_pci_dev); | ^~~~~~~~~~~~~~~~ include/linux/sysfs.h:161:11: note: in definition of macro 'ATTRIBUTE_GROUPS' 161 | .attrs = _name##_attrs, \ | ^~~~~ drivers/misc/pvpanic/pvpanic-pci.c: In function 'pvpanic_pci_probe': drivers/misc/pvpanic/pvpanic-pci.c:70:18: warning: unused variable 'res' [-Wunused-variable] 70 | struct resource res; | ^~~ drivers/misc/pvpanic/pvpanic-pci.c: At top level: >> drivers/misc/pvpanic/pvpanic-pci.c:97:18: error: 'pvpanic_pci_dev_groups' undeclared here (not in a function); did you mean 'pvpanici_pci_dev_groups'? 97 | .groups = pvpanic_pci_dev_groups, | ^~~~~~~~~~~~~~~~~~~~~~ | pvpanici_pci_dev_groups In file included from include/linux/kobject.h:20, from include/linux/module.h:20, from drivers/misc/pvpanic/pvpanic-pci.c:9: drivers/misc/pvpanic/pvpanic-pci.c:64:18: warning: 'pvpanici_pci_dev_groups' defined but not used [-Wunused-variable] 64 | ATTRIBUTE_GROUPS(pvpanici_pci_dev); | ^~~~~~~~~~~~~~~~ include/linux/sysfs.h:154:38: note: in definition of macro '__ATTRIBUTE_GROUPS' 154 | static const struct attribute_group *_name##_groups[] = { \ | ^~~~~ drivers/misc/pvpanic/pvpanic-pci.c:64:1: note: in expansion of macro 'ATTRIBUTE_GROUPS' 64 | ATTRIBUTE_GROUPS(pvpanici_pci_dev); | ^~~~~~~~~~~~~~~~ drivers/misc/pvpanic/pvpanic-pci.c:59:26: warning: 'pvpanic_pci_dev_attrs' defined but not used [-Wunused-variable] 59 | static struct attribute *pvpanic_pci_dev_attrs[] = { | ^~~~~~~~~~~~~~~~~~~~~ vim +/PVPANIC_PANICKED +22 drivers/misc/pvpanic/pvpanic-pci.c 16 17 static void __iomem *base; 18 static const struct pci_device_id pvpanic_pci_id_tbl[] = { 19 { PCI_DEVICE(PCI_VENDOR_ID_REDHAT, PCI_DEVICE_ID_REDHAT_PVPANIC),}, 20 {} 21 }; > 22 static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED; 23 static unsigned int events; 24 25 static ssize_t capability_show(struct device *dev, 26 struct device_attribute *attr, char *buf) 27 { 28 return sysfs_emit(buf, "%x\n", capability); 29 } 30 static DEVICE_ATTR_RO(capability); 31 32 static ssize_t events_show(struct device *dev, struct device_attribute *attr, char *buf) 33 { 34 return sysfs_emit(buf, "%x\n", events); 35 } 36 37 static ssize_t events_store(struct device *dev, struct device_attribute *attr, 38 const char *buf, size_t count) 39 { 40 unsigned int tmp; 41 int err; 42 43 err = kstrtouint(buf, 16, &tmp); 44 if (err) 45 return err; 46 47 if ((tmp & capability) != tmp) 48 return -EINVAL; 49 50 events = tmp; 51 52 pvpanic_set_events(base, events); 53 54 return count; 55 56 } 57 static DEVICE_ATTR_RW(events); 58 59 static struct attribute *pvpanic_pci_dev_attrs[] = { 60 &dev_attr_capability.attr, 61 &dev_attr_events.attr, 62 NULL 63 }; > 64 ATTRIBUTE_GROUPS(pvpanici_pci_dev); 65 66 static int pvpanic_pci_probe(struct pci_dev *pdev, 67 const struct pci_device_id *ent) 68 { 69 int ret; 70 struct resource res; 71 72 ret = pci_enable_device(pdev); 73 if (ret < 0) 74 return ret; 75 76 base = pci_iomap(pdev, 0, 0); 77 if (IS_ERR(base)) 78 return PTR_ERR(base); 79 80 /* initlize capability by RDPT */ 81 capability &= ioread8(base); 82 events = capability; 83 84 return pvpanic_probe(base, capability); 85 } 86 87 static void pvpanic_pci_remove(struct pci_dev *pdev) 88 { 89 pvpanic_remove(base); 90 iounmap(base); 91 pci_disable_device(pdev); 92 } 93 94 static struct pci_driver pvpanic_pci_driver = { 95 .name = "pvpanic-pci", 96 .id_table = pvpanic_pci_id_tbl, > 97 .groups = pvpanic_pci_dev_groups, 98 .probe = pvpanic_pci_probe, 99 .remove = pvpanic_pci_remove, 100 }; 101 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 67479 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-03-17 0:03 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-03-16 12:20 [PATCH v5] add support for pci in the pvpanic driver Mihai Carabas 2021-03-16 12:20 ` [PATCH v5 1/3] misc/pvpanic: split-up generic and platform dependent code Mihai Carabas 2021-03-16 16:44 ` Greg KH 2021-03-16 12:20 ` [PATCH v5 2/3] misc/pvpanic: probe multiple instances Mihai Carabas 2021-03-16 16:46 ` Greg KH 2021-03-16 12:20 ` [PATCH v5 3/3] misc/pvpanic: add PCI driver Mihai Carabas 2021-03-16 16:48 ` Greg KH 2021-03-16 17:08 ` Mihai Carabas 2021-03-16 17:29 ` Greg KH 2021-03-16 18:02 ` Mihai Carabas 2021-03-16 18:16 ` Greg KH 2021-03-17 0:02 ` kernel test robot
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).