LinuxLists.cc - [PATCH v2] add support for pci in the pvpanic driver

[permalink] [raw]

Subject: [PATCH v2 1/3] misc/pvpanic: split-up generic and platform dependent code

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 patch.

The code from pvpanic.c was split in two new files:
- pvpanic-common.c: generic code that handles pvpanic events
- mmio.c: platform/bus dependent code

Signed-off-by: Mihai Carabas <[email protected]>
---
drivers/misc/Kconfig | 9 +--
drivers/misc/Makefile | 2 +-
drivers/misc/pvpanic.c | 111 ----------------------------------
drivers/misc/pvpanic/Kconfig | 12 ++++
drivers/misc/pvpanic/Makefile | 2 +
drivers/misc/pvpanic/mmio.c | 83 +++++++++++++++++++++++++
drivers/misc/pvpanic/pvpanic-common.c | 60 ++++++++++++++++++
drivers/misc/pvpanic/pvpanic.h | 10 +++
8 files changed, 169 insertions(+), 120 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/mmio.c
create mode 100644 drivers/misc/pvpanic/pvpanic-common.c
create mode 100644 drivers/misc/pvpanic/pvpanic.h

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index fafa8b0..0273ecb 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -448,14 +448,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
@@ -481,4 +473,5 @@ source "drivers/misc/ocxl/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 d23231e..9f411b8 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -52,7 +52,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
obj-$(CONFIG_OCXL) += ocxl/
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 41cab29..00000000
--- a/drivers/misc/pvpanic.c
+++ /dev/null
@@ -1,111 +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;
-
-MODULE_AUTHOR("Hu Tao <[email protected]>");
-MODULE_DESCRIPTION("pvpanic device driver");
-MODULE_LICENSE("GPL");
-
-static void
-pvpanic_send_event(unsigned int event)
-{
- 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;
- }
-
- atomic_notifier_chain_register(&panic_notifier_list,
- &pvpanic_panic_nb);
-
- return 0;
-}
-
-static int pvpanic_mmio_remove(struct platform_device *pdev)
-{
-
- 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,
- },
- .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..0dce6ef
--- /dev/null
+++ b/drivers/misc/pvpanic/Kconfig
@@ -0,0 +1,12 @@
+config PVPANIC
+ bool "pvpanic device support"
+ help
+ This option enables pvpanic device driver.
+
+config PVPANIC_MMIO
+ tristate "pvpanic MMIO device support"
+ depends on HAS_IOMEM && (ACPI || OF) && PVPANIC
+ 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.
diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
new file mode 100644
index 00000000..9ea3355
--- /dev/null
+++ b/drivers/misc/pvpanic/Makefile
@@ -0,0 +1,2 @@
+obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
+pvpanic-mmio-objs := pvpanic-common.o mmio.o
diff --git a/drivers/misc/pvpanic/mmio.c b/drivers/misc/pvpanic/mmio.c
new file mode 100644
index 00000000..7454eeb
--- /dev/null
+++ b/drivers/misc/pvpanic/mmio.c
@@ -0,0 +1,83 @@
+// 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 <[email protected]>");
+MODULE_DESCRIPTION("pvpanic device driver");
+MODULE_LICENSE("GPL");
+
+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;
+ }
+
+ pvpanic_probe(base);
+
+ 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,
+ },
+ .probe = pvpanic_mmio_probe,
+ .remove = pvpanic_mmio_remove,
+};
+module_platform_driver(pvpanic_mmio_driver);
diff --git a/drivers/misc/pvpanic/pvpanic-common.c b/drivers/misc/pvpanic/pvpanic-common.c
new file mode 100644
index 00000000..395ccae
--- /dev/null
+++ b/drivers/misc/pvpanic/pvpanic-common.c
@@ -0,0 +1,60 @@
+// 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>
+
+static void __iomem *base;
+
+static void
+pvpanic_send_event(unsigned int event)
+{
+ 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)
+{
+ base = pbase;
+ atomic_notifier_chain_register(&panic_notifier_list,
+ &pvpanic_panic_nb);
+}
+
+void pvpanic_remove(void)
+{
+
+ atomic_notifier_chain_unregister(&panic_notifier_list,
+ &pvpanic_panic_nb);
+ base = NULL;
+}
diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h
new file mode 100644
index 00000000..4d6c221
--- /dev/null
+++ b/drivers/misc/pvpanic/pvpanic.h
@@ -0,0 +1,10 @@
+#ifndef PVPANIC_H_
+#define PVPANIC_H_
+
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+void pvpanic_probe(void __iomem *base);
+void pvpanic_remove(void);
+
+#endif /* PVPANIC_H_ */
--
1.8.3.1

2021-02-12 10:09:08

[permalink] [raw]

Subject: [PATCH v2 3/3] misc/pvpanic: add license

Add license to the newly created files in adding support for pvpanic PCI device
driver.

Signed-off-by: Mihai Carabas <[email protected]>
---
drivers/misc/pvpanic/Kconfig | 7 +++++++
drivers/misc/pvpanic/Makefile | 7 +++++++
drivers/misc/pvpanic/pci.c | 7 +++++++
drivers/misc/pvpanic/pvpanic.h | 7 +++++++
4 files changed, 28 insertions(+)

diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
index ce8b93e..004c2b7 100644
--- a/drivers/misc/pvpanic/Kconfig
+++ b/drivers/misc/pvpanic/Kconfig
@@ -1,3 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Pvpanic Kconfig
+#
+# Copyright (C) 2021 Oracle.
+#
+
config PVPANIC
bool "pvpanic device support"
help
diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
index 1763450..898a731 100644
--- a/drivers/misc/pvpanic/Makefile
+++ b/drivers/misc/pvpanic/Makefile
@@ -1,3 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Pvpanic Makefile
+#
+# Copyright (C) 2021 Oracle.
+#
+
obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
pvpanic-mmio-objs := pvpanic-common.o mmio.o
obj-$(CONFIG_PVPANIC_PCI) += pvpanic-pci.o
diff --git a/drivers/misc/pvpanic/pci.c b/drivers/misc/pvpanic/pci.c
index b672727..d1c1ed9 100644
--- a/drivers/misc/pvpanic/pci.c
+++ b/drivers/misc/pvpanic/pci.c
@@ -1,3 +1,10 @@
+// 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>
diff --git a/drivers/misc/pvpanic/pvpanic.h b/drivers/misc/pvpanic/pvpanic.h
index 4d6c221..7e75dce 100644
--- a/drivers/misc/pvpanic/pvpanic.h
+++ b/drivers/misc/pvpanic/pvpanic.h
@@ -1,3 +1,10 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Pvpanic Device Support
+ *
+ * Copyright (C) 2021 Oracle.
+*/
+
#ifndef PVPANIC_H_
#define PVPANIC_H_

--
1.8.3.1

2021-02-12 10:09:09

[permalink] [raw]

Subject: [PATCH v2 2/3] misc/pvpanic: add PCI driver

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 <[email protected]>
---
drivers/misc/pvpanic/Kconfig | 8 +++++++
drivers/misc/pvpanic/Makefile | 2 ++
drivers/misc/pvpanic/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+)
create mode 100644 drivers/misc/pvpanic/pci.c

diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
index 0dce6ef..ce8b93e 100644
--- a/drivers/misc/pvpanic/Kconfig
+++ b/drivers/misc/pvpanic/Kconfig
@@ -10,3 +10,11 @@ config PVPANIC_MMIO
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 PVPANIC_PCI
+ tristate "pvpanic PCI device support"
+ depends on PCI && PVPANIC
+ 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.
diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
index 9ea3355..1763450 100644
--- a/drivers/misc/pvpanic/Makefile
+++ b/drivers/misc/pvpanic/Makefile
@@ -1,2 +1,4 @@
obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
pvpanic-mmio-objs := pvpanic-common.o mmio.o
+obj-$(CONFIG_PVPANIC_PCI) += pvpanic-pci.o
+pvpanic-pci-objs := pvpanic-common.o pci.o
diff --git a/drivers/misc/pvpanic/pci.c b/drivers/misc/pvpanic/pci.c
new file mode 100644
index 00000000..b672727
--- /dev/null
+++ b/drivers/misc/pvpanic/pci.c
@@ -0,0 +1,49 @@
+#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 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);
+
+ pvpanic_probe(base);
+
+ return 0;
+}
+
+static void pvpanic_pci_remove(struct pci_dev *pdev)
+{
+ pvpanic_remove();
+ iounmap(base);
+ pci_disable_device(pdev);
+}
+
+static struct pci_driver pvpanic_pci_driver = {
+ .name = "pvpanic-pci",
+ .id_table = pvpanic_pci_id_tbl,
+ .probe = pvpanic_pci_probe,
+ .remove = pvpanic_pci_remove,
+};
+
+module_pci_driver(pvpanic_pci_driver);
--
1.8.3.1

2021-02-12 10:16:40

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] misc/pvpanic: split-up generic and platform dependent code

On Fri, Feb 12, 2021 at 10:17 AM Mihai Carabas <[email protected]> wrote:
>
> 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 patch.
>
> The code from pvpanic.c was split in two new files:
> - pvpanic-common.c: generic code that handles pvpanic events
> - mmio.c: platform/bus dependent code
>
> Signed-off-by: Mihai Carabas <[email protected]>

The patch looks fine, but for review purposes it's better to generate
the email with 'git format-patch -M'.

Reviewed-by: Arnd Bergmann <[email protected]>

2021-02-12 10:17:25

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] misc/pvpanic: add license

On Fri, Feb 12, 2021 at 11:17:06AM +0200, Mihai Carabas wrote:
> Add license to the newly created files in adding support for pvpanic PCI device
> driver.
>
> Signed-off-by: Mihai Carabas <[email protected]>
> ---
> drivers/misc/pvpanic/Kconfig | 7 +++++++
> drivers/misc/pvpanic/Makefile | 7 +++++++
> drivers/misc/pvpanic/pci.c | 7 +++++++
> drivers/misc/pvpanic/pvpanic.h | 7 +++++++

Do not add this after-the-fact, add them when you create these files.

thanks,

greg k-h

2021-02-12 10:17:29

[permalink] [raw]

Subject: Re: [PATCH v2 3/3] misc/pvpanic: add license

On Fri, Feb 12, 2021 at 10:17 AM Mihai Carabas <[email protected]> wrote:
>
> Add license to the newly created files in adding support for pvpanic PCI device
> driver.
>
> Signed-off-by: Mihai Carabas <[email protected]>

I don't see why this is a separate patch, rather than part of the patch
that creates these files.

Arnd

2021-02-12 10:17:52

[permalink] [raw]

Subject: Re: [PATCH v2 2/3] misc/pvpanic: add PCI driver

On Fri, Feb 12, 2021 at 10:17 AM Mihai Carabas <[email protected]> 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 <[email protected]>

Reviewed-by: Arnd Bergmann <[email protected]>

2021-02-12 10:19:58

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] misc/pvpanic: split-up generic and platform dependent code

On Fri, Feb 12, 2021 at 11:17:04AM +0200, Mihai Carabas wrote:
> 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 patch.
>
> The code from pvpanic.c was split in two new files:
> - pvpanic-common.c: generic code that handles pvpanic events
> - mmio.c: platform/bus dependent code
>
> Signed-off-by: Mihai Carabas <[email protected]>
> ---
> drivers/misc/Kconfig | 9 +--
> drivers/misc/Makefile | 2 +-
> drivers/misc/pvpanic.c | 111 ----------------------------------
> drivers/misc/pvpanic/Kconfig | 12 ++++
> drivers/misc/pvpanic/Makefile | 2 +
> drivers/misc/pvpanic/mmio.c | 83 +++++++++++++++++++++++++
> drivers/misc/pvpanic/pvpanic-common.c | 60 ++++++++++++++++++
> drivers/misc/pvpanic/pvpanic.h | 10 +++
> 8 files changed, 169 insertions(+), 120 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/mmio.c
> create mode 100644 drivers/misc/pvpanic/pvpanic-common.c
> create mode 100644 drivers/misc/pvpanic/pvpanic.h
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index fafa8b0..0273ecb 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -448,14 +448,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
> @@ -481,4 +473,5 @@ source "drivers/misc/ocxl/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 d23231e..9f411b8 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -52,7 +52,7 @@ obj-$(CONFIG_CXL_BASE) += cxl/
> obj-$(CONFIG_PCI_ENDPOINT_TEST) += pci_endpoint_test.o
> obj-$(CONFIG_OCXL) += ocxl/
> 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 41cab29..00000000
> --- a/drivers/misc/pvpanic.c
> +++ /dev/null
> @@ -1,111 +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;
> -
> -MODULE_AUTHOR("Hu Tao <[email protected]>");
> -MODULE_DESCRIPTION("pvpanic device driver");
> -MODULE_LICENSE("GPL");
> -
> -static void
> -pvpanic_send_event(unsigned int event)
> -{
> - 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;
> - }
> -
> - atomic_notifier_chain_register(&panic_notifier_list,
> - &pvpanic_panic_nb);
> -
> - return 0;
> -}
> -
> -static int pvpanic_mmio_remove(struct platform_device *pdev)
> -{
> -
> - 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,
> - },
> - .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..0dce6ef
> --- /dev/null
> +++ b/drivers/misc/pvpanic/Kconfig
> @@ -0,0 +1,12 @@
> +config PVPANIC
> + bool "pvpanic device support"
> + help
> + This option enables pvpanic device driver.

No it doesn't, it allows you to _select_ a specific pvpanic driver, on
its own, it is not a driver, right?

> +
> +config PVPANIC_MMIO
> + tristate "pvpanic MMIO device support"
> + depends on HAS_IOMEM && (ACPI || OF) && PVPANIC
> + 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.
> diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
> new file mode 100644
> index 00000000..9ea3355
> --- /dev/null
> +++ b/drivers/misc/pvpanic/Makefile
> @@ -0,0 +1,2 @@
> +obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
> +pvpanic-mmio-objs := pvpanic-common.o mmio.o

You put the "common" logic in the mmio driver? How is that going to
work for the PCI driver?

Why is there not a pvpanic.ko that contains the "core" code only?

thanks,

greg k-h

2021-02-12 10:21:05

[permalink] [raw]

Subject: Re: [PATCH v2 2/3] misc/pvpanic: add PCI driver

On Fri, Feb 12, 2021 at 11:17:05AM +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 <[email protected]>
> ---
> drivers/misc/pvpanic/Kconfig | 8 +++++++
> drivers/misc/pvpanic/Makefile | 2 ++
> drivers/misc/pvpanic/pci.c | 49 +++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 59 insertions(+)
> create mode 100644 drivers/misc/pvpanic/pci.c
>
> diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
> index 0dce6ef..ce8b93e 100644
> --- a/drivers/misc/pvpanic/Kconfig
> +++ b/drivers/misc/pvpanic/Kconfig
> @@ -10,3 +10,11 @@ config PVPANIC_MMIO
> 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 PVPANIC_PCI
> + tristate "pvpanic PCI device support"
> + depends on PCI && PVPANIC
> + 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.
> diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
> index 9ea3355..1763450 100644
> --- a/drivers/misc/pvpanic/Makefile
> +++ b/drivers/misc/pvpanic/Makefile
> @@ -1,2 +1,4 @@
> obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
> pvpanic-mmio-objs := pvpanic-common.o mmio.o
> +obj-$(CONFIG_PVPANIC_PCI) += pvpanic-pci.o
> +pvpanic-pci-objs := pvpanic-common.o pci.o

So you now have the pvpanic-common.o file linked into both modules at
the same time? What happens if they are both loaded?

This feels really broken...

greg k-h

2021-02-12 10:27:09

[permalink] [raw]

Subject: Re: [PATCH v2 2/3] misc/pvpanic: add PCI driver

On Fri, Feb 12, 2021 at 11:18 AM Greg KH <[email protected]> wrote:
le
> > @@ -1,2 +1,4 @@
> > obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
> > pvpanic-mmio-objs := pvpanic-common.o mmio.o
> > +obj-$(CONFIG_PVPANIC_PCI) += pvpanic-pci.o
> > +pvpanic-pci-objs := pvpanic-common.o pci.o
>
> So you now have the pvpanic-common.o file linked into both modules at
> the same time? What happens if they are both loaded?
>
> This feels really broken...

Loading them as two modules should work, but this will probably lead to
a link failure if one of the two drivers is built-in and the other one is
a loadable module.

The usual way to fix this is to make the common part a separate module
with exported symbols for the probe/release functions and built
based on CONFIG_PVPANIC, which then needs to be a tristate
symbol again.

Arnd

2021-02-12 10:32:46

[permalink] [raw]

Subject: Re: [PATCH v2 1/3] misc/pvpanic: split-up generic and platform dependent code

..snip
>> -};
>> -module_platform_driver(pvpanic_mmio_driver);
>> diff --git a/drivers/misc/pvpanic/Kconfig b/drivers/misc/pvpanic/Kconfig
>> new file mode 100644
>> index 00000000..0dce6ef
>> --- /dev/null
>> +++ b/drivers/misc/pvpanic/Kconfig
>> @@ -0,0 +1,12 @@
>> +config PVPANIC
>> + bool "pvpanic device support"
>> + help
>> + This option enables pvpanic device driver.
> No it doesn't, it allows you to _select_ a specific pvpanic driver, on
> its own, it is not a driver, right?

Yes. I will update the comment.

>> +
>> +config PVPANIC_MMIO
>> + tristate "pvpanic MMIO device support"
>> + depends on HAS_IOMEM && (ACPI || OF) && PVPANIC
>> + 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.
>> diff --git a/drivers/misc/pvpanic/Makefile b/drivers/misc/pvpanic/Makefile
>> new file mode 100644
>> index 00000000..9ea3355
>> --- /dev/null
>> +++ b/drivers/misc/pvpanic/Makefile
>> @@ -0,0 +1,2 @@
>> +obj-$(CONFIG_PVPANIC_MMIO) += pvpanic-mmio.o
>> +pvpanic-mmio-objs := pvpanic-common.o mmio.o
> You put the "common" logic in the mmio driver? How is that going to
> work for the PCI driver?
>
> Why is there not a pvpanic.ko that contains the "core" code only?

My intention was to put the code in both drivers. There is no generic
module as it will complicate things: the generic module would have to
have its own state (e.g. pvpanic_probe would have to create a queue of
base addresses).

Do you want to see a generic module pvpanic.ko with its own state?

Thank you,
Mihai

2021-02-12 11:05:19