Re: [PATCH V2 1/4] misc/pvpanic: move pvpanic to misc as common driver

Re: [PATCH V2 1/4] misc/pvpanic: move pvpanic to misc as common driver

[Andy Shevchenko]

On Wed, Oct 24, 2018 at 12:02 PM Peng Hao <peng.hao2@zte.com.cn> wrote:
>
> move pvpanic.c from drivers/platform/x86 to drivers/misc.
> following patches will use pvpanic device in arm64.
>

-C -M usually helps in such patches.

The question is perhaps this should go under drivers/acpi?

Rafael, what do you think?

> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
>  drivers/misc/Kconfig           |   7 +++
>  drivers/misc/Makefile          |   1 +
>  drivers/misc/pvpanic.c         | 124 +++++++++++++++++++++++++++++++++++++++++
>  drivers/platform/Kconfig       |   3 +
>  drivers/platform/Makefile      |   1 +
>  drivers/platform/x86/Kconfig   |   8 ---
>  drivers/platform/x86/Makefile  |   1 -
>  drivers/platform/x86/pvpanic.c | 124 -----------------------------------------
>  8 files changed, 136 insertions(+), 133 deletions(-)
>  create mode 100644 drivers/misc/pvpanic.c
>  delete mode 100644 drivers/platform/x86/pvpanic.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 5d71300..ca55c94 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -499,6 +499,13 @@ config MISC_RTSX
>         tristate
>         default MISC_RTSX_PCI || MISC_RTSX_USB
>
> +config PVPANIC
> +       tristate "pvpanic device support"
> +       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.
> +
>  source "drivers/misc/c2port/Kconfig"
>  source "drivers/misc/eeprom/Kconfig"
>  source "drivers/misc/cb710/Kconfig"
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 20be70c..39dc005 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -57,3 +57,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP)        += aspeed-lpc-snoop.o
>  obj-$(CONFIG_PCI_ENDPOINT_TEST)        += pci_endpoint_test.o
>  obj-$(CONFIG_OCXL)             += ocxl/
>  obj-$(CONFIG_MISC_RTSX)                += cardreader/
> +obj-$(CONFIG_PVPANIC)                  += pvpanic.o
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> new file mode 100644
> index 0000000..fd86dab
> --- /dev/null
> +++ b/drivers/misc/pvpanic.c
> @@ -0,0 +1,124 @@
> +/*
> + *  pvpanic.c - pvpanic Device Support
> + *
> + *  Copyright (C) 2013 Fujitsu.
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or
> + *  (at your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with this program; if not, write to the Free Software
> + *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/acpi.h>
> +
> +MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
> +MODULE_DESCRIPTION("pvpanic device driver");
> +MODULE_LICENSE("GPL");
> +
> +static int pvpanic_add(struct acpi_device *device);
> +static int pvpanic_remove(struct acpi_device *device);
> +
> +static const struct acpi_device_id pvpanic_device_ids[] = {
> +       { "QEMU0001", 0 },
> +       { "", 0 },
> +};
> +MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
> +
> +#define PVPANIC_PANICKED       (1 << 0)
> +
> +static u16 port;
> +
> +static struct acpi_driver pvpanic_driver = {
> +       .name =         "pvpanic",
> +       .class =        "QEMU",
> +       .ids =          pvpanic_device_ids,
> +       .ops =          {
> +                               .add =          pvpanic_add,
> +                               .remove =       pvpanic_remove,
> +                       },
> +       .owner =        THIS_MODULE,
> +};
> +
> +static void
> +pvpanic_send_event(unsigned int event)
> +{
> +       outb(event, port);
> +}
> +
> +static int
> +pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
> +                    void *unused)
> +{
> +       pvpanic_send_event(PVPANIC_PANICKED);
> +       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 acpi_status
> +pvpanic_walk_resources(struct acpi_resource *res, void *context)
> +{
> +       switch (res->type) {
> +       case ACPI_RESOURCE_TYPE_END_TAG:
> +               return AE_OK;
> +
> +       case ACPI_RESOURCE_TYPE_IO:
> +               port = res->data.io.minimum;
> +               return AE_OK;
> +
> +       default:
> +               return AE_ERROR;
> +       }
> +}
> +
> +static int pvpanic_add(struct acpi_device *device)
> +{
> +       int ret;
> +
> +       ret = acpi_bus_get_status(device);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (!device->status.enabled || !device->status.functional)
> +               return -ENODEV;
> +
> +       acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> +                           pvpanic_walk_resources, NULL);
> +
> +       if (!port)
> +               return -ENODEV;
> +
> +       atomic_notifier_chain_register(&panic_notifier_list,
> +                                      &pvpanic_panic_nb);
> +
> +       return 0;
> +}
> +
> +static int pvpanic_remove(struct acpi_device *device)
> +{
> +
> +       atomic_notifier_chain_unregister(&panic_notifier_list,
> +                                        &pvpanic_panic_nb);
> +       return 0;
> +}
> +
> +module_acpi_driver(pvpanic_driver);
> diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
> index d4c2e42..8b64b78 100644
> --- a/drivers/platform/Kconfig
> +++ b/drivers/platform/Kconfig
> @@ -1,3 +1,6 @@
> +if ARM64
> +source "drivers/platform/arm/Kconfig"
> +endif
>  if X86
>  source "drivers/platform/x86/Kconfig"
>  endif
> diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
> index 4b2ce58..232231c 100644
> --- a/drivers/platform/Makefile
> +++ b/drivers/platform/Makefile
> @@ -3,6 +3,7 @@
>  # Makefile for linux/drivers/platform
>  #
>
> +obj-$(CONFIG_ARM64)               += arm/
>  obj-$(CONFIG_X86)              += x86/
>  obj-$(CONFIG_MELLANOX_PLATFORM)        += mellanox/
>  obj-$(CONFIG_MIPS)             += mips/
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index ef016e4..a6b412e 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -1119,14 +1119,6 @@ config INTEL_SMARTCONNECT
>           This driver checks to determine whether the device has Intel Smart
>           Connect enabled, and if so disables it.
>
> -config PVPANIC
> -       tristate "pvpanic device support"
> -       depends on ACPI
> -       ---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 INTEL_PMC_IPC
>         tristate "Intel PMC IPC Driver"
>         depends on ACPI
> diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
> index 2ba6cb7..a4ec299 100644
> --- a/drivers/platform/x86/Makefile
> +++ b/drivers/platform/x86/Makefile
> @@ -75,7 +75,6 @@ obj-$(CONFIG_APPLE_GMUX)      += apple-gmux.o
>  obj-$(CONFIG_INTEL_RST)                += intel-rst.o
>  obj-$(CONFIG_INTEL_SMARTCONNECT)       += intel-smartconnect.o
>
> -obj-$(CONFIG_PVPANIC)           += pvpanic.o
>  obj-$(CONFIG_ALIENWARE_WMI)    += alienware-wmi.o
>  obj-$(CONFIG_INTEL_PMC_IPC)    += intel_pmc_ipc.o
>  obj-$(CONFIG_SILEAD_DMI)       += silead_dmi.o
> diff --git a/drivers/platform/x86/pvpanic.c b/drivers/platform/x86/pvpanic.c
> deleted file mode 100644
> index fd86dab..0000000
> --- a/drivers/platform/x86/pvpanic.c
> +++ /dev/null
> @@ -1,124 +0,0 @@
> -/*
> - *  pvpanic.c - pvpanic Device Support
> - *
> - *  Copyright (C) 2013 Fujitsu.
> - *
> - *  This program is free software; you can redistribute it and/or modify
> - *  it under the terms of the GNU General Public License as published by
> - *  the Free Software Foundation; either version 2 of the License, or
> - *  (at your option) any later version.
> - *
> - *  This program is distributed in the hope that it will be useful,
> - *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> - *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> - *  GNU General Public License for more details.
> - *
> - *  You should have received a copy of the GNU General Public License
> - *  along with this program; if not, write to the Free Software
> - *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
> - */
> -
> -#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> -
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/types.h>
> -#include <linux/acpi.h>
> -
> -MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
> -MODULE_DESCRIPTION("pvpanic device driver");
> -MODULE_LICENSE("GPL");
> -
> -static int pvpanic_add(struct acpi_device *device);
> -static int pvpanic_remove(struct acpi_device *device);
> -
> -static const struct acpi_device_id pvpanic_device_ids[] = {
> -       { "QEMU0001", 0 },
> -       { "", 0 },
> -};
> -MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
> -
> -#define PVPANIC_PANICKED       (1 << 0)
> -
> -static u16 port;
> -
> -static struct acpi_driver pvpanic_driver = {
> -       .name =         "pvpanic",
> -       .class =        "QEMU",
> -       .ids =          pvpanic_device_ids,
> -       .ops =          {
> -                               .add =          pvpanic_add,
> -                               .remove =       pvpanic_remove,
> -                       },
> -       .owner =        THIS_MODULE,
> -};
> -
> -static void
> -pvpanic_send_event(unsigned int event)
> -{
> -       outb(event, port);
> -}
> -
> -static int
> -pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
> -                    void *unused)
> -{
> -       pvpanic_send_event(PVPANIC_PANICKED);
> -       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 acpi_status
> -pvpanic_walk_resources(struct acpi_resource *res, void *context)
> -{
> -       switch (res->type) {
> -       case ACPI_RESOURCE_TYPE_END_TAG:
> -               return AE_OK;
> -
> -       case ACPI_RESOURCE_TYPE_IO:
> -               port = res->data.io.minimum;
> -               return AE_OK;
> -
> -       default:
> -               return AE_ERROR;
> -       }
> -}
> -
> -static int pvpanic_add(struct acpi_device *device)
> -{
> -       int ret;
> -
> -       ret = acpi_bus_get_status(device);
> -       if (ret < 0)
> -               return ret;
> -
> -       if (!device->status.enabled || !device->status.functional)
> -               return -ENODEV;
> -
> -       acpi_walk_resources(device->handle, METHOD_NAME__CRS,
> -                           pvpanic_walk_resources, NULL);
> -
> -       if (!port)
> -               return -ENODEV;
> -
> -       atomic_notifier_chain_register(&panic_notifier_list,
> -                                      &pvpanic_panic_nb);
> -
> -       return 0;
> -}
> -
> -static int pvpanic_remove(struct acpi_device *device)
> -{
> -
> -       atomic_notifier_chain_unregister(&panic_notifier_list,
> -                                        &pvpanic_panic_nb);
> -       return 0;
> -}
> -
> -module_acpi_driver(pvpanic_driver);
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2 2/4] misc/pvpanic: add MMIO support

[Andy Shevchenko]

On Wed, Oct 24, 2018 at 12:02 PM Peng Hao <peng.hao2@zte.com.cn> wrote:
>
> On some architectures (e.g. arm64), it's preferable to use MMIO, since
> this can be used standalone. Add MMIO support to the pvpanic driver.
>

No global variables, please.

If your switch to ioreadxx()iowritexx() it would really help you here.

> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
>  drivers/misc/pvpanic.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index fd86dab..bcfefdd 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -43,6 +43,8 @@
>
>  static u16 port;
>
> +static void __iomem *base;
> +
>  static struct acpi_driver pvpanic_driver = {
>         .name =         "pvpanic",
>         .class =        "QEMU",
> @@ -57,7 +59,10 @@
>  static void
>  pvpanic_send_event(unsigned int event)
>  {
> -       outb(event, port);
> +       if (port)
> +               outb(event, port);
> +       else if (base)
> +               writeb(event, base);
>  }
>
>  static int
> @@ -77,6 +82,8 @@
>  static acpi_status
>  pvpanic_walk_resources(struct acpi_resource *res, void *context)
>  {
> +       struct acpi_resource_fixed_memory32 *fixmem32;
> +
>         switch (res->type) {
>         case ACPI_RESOURCE_TYPE_END_TAG:
>                 return AE_OK;
> @@ -85,6 +92,11 @@
>                 port = res->data.io.minimum;
>                 return AE_OK;
>
> +       case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
> +               fixmem32 = &res->data.fixed_memory32;
> +               base = ioremap(fixmem32->address, fixmem32->address_length);
> +               return AE_OK;
> +
>         default:
>                 return AE_ERROR;
>         }
> @@ -104,7 +116,7 @@ static int pvpanic_add(struct acpi_device *device)
>         acpi_walk_resources(device->handle, METHOD_NAME__CRS,
>                             pvpanic_walk_resources, NULL);
>
> -       if (!port)
> +       if (!port && !base)
>                 return -ENODEV;
>
>         atomic_notifier_chain_register(&panic_notifier_list,
> @@ -118,6 +130,8 @@ static int pvpanic_remove(struct acpi_device *device)
>
>         atomic_notifier_chain_unregister(&panic_notifier_list,
>                                          &pvpanic_panic_nb);
> +       if (base)
> +               iounmap(base);
>         return 0;
>  }
>
> --
> 1.8.3.1
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2 3/4] misc/pvpanic: add support to pvpanic device information by using FDT

[Andy Shevchenko]

On Wed, Oct 24, 2018 at 12:02 PM Peng Hao <peng.hao2@zte.com.cn> wrote:
>
> By default, when ACPI tables and FDT coexist for ARM64,
> current kernel takes precedence over FDT to get device information.
> This patch increases the way to get information through FDT.

>   *  pvpanic.c - pvpanic Device Support
>   *
>   *  Copyright (C) 2013 Fujitsu.
> + *  Copyright (C) 2018 ZTE.
>   *
>   *  This program is free software; you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by

This hunk would be a separate patch which includes switch to SPDX and
removing the file name from the file itself.

> @@ -25,6 +26,10 @@
>  #include <linux/init.h>
>  #include <linux/types.h>
>  #include <linux/acpi.h>
> +#include <linux/of.h>
> +#include <linux/of_address.h>

> +#include <asm/virt.h>

asm/* goes after linux/*

> +#include <linux/platform_device.h>

Better to keep sorted.

> +       if (!mem)
> +               return -EINVAL;
> +
> +       if (!devm_request_mem_region(&pdev->dev, mem->start,
> +                                    resource_size(mem), pdev->name))
> +               return -EBUSY;
> +
> +       base = devm_ioremap(&pdev->dev, mem->start,
> +                           resource_size(mem));
> +       if (base == NULL)
> +               return -EFAULT;

devm_ioremap_resource()

> +static const struct of_device_id pvpanic_mmio_match[] = {
> +       { .compatible = "qemu,pvpanic-mmio", },
> +       {},

terminators better w/o comma.

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH V2 4/4] misc/pvpanic : pvpanic: add document for pvpanic-mmio DT

[Rob Herring]

Please Cc DT list for bindings.

On Wed, Oct 24, 2018 at 4:01 AM Peng Hao <peng.hao2@zte.com.cn> wrote:
>

Commit message?

"dt-bindings: misc: ..." for the subject.

> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
>  .../devicetree/bindings/arm/pvpanic-mmio.txt       | 29 ++++++++++++++++++++++

As Mark said, not ARM specific. So please move to bindings/misc/ and
use the compatible string for the name (qemu,pvpanic-mmio.txt).

>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/arm/pvpanic-mmio.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/pvpanic-mmio.txt b/Documentation/devicetree/bindings/arm/pvpanic-mmio.txt
> new file mode 100644
> index 0000000..a6bdacd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/pvpanic-mmio.txt
> @@ -0,0 +1,29 @@
> +* QEMU PVPANIC MMIO Configuration bindings for ARM

Drop the "for ARM"

> +
> +QEMU's emulation / virtualization targets provide the following PVPANIC
> +MMIO Configuration interface on the "virt" machine.
> +type:
> +
> +- a read-write, 16-bit wide data register.
> +
> +QEMU exposes the data register to guests as memory mapped registers.

I have to wonder why we need a QEMU specific way to signal a panic.
What about every real platform that panics? What are they supposed to
do? Shouldn't this be a PSCI call so we can have something not per
platform?

> +
> +Required properties:
> +
> +- compatible: "qemu,pvpanic-mmio".
> +- reg: the MMIO region used by the device.
> +  * Bytes 0x0  Write panic event to the reg when guest OS panics.
> +  * Bytes 0x1  Reserved.
> +
> +Example:
> +
> +/ {
> +        #size-cells = <0x2>;
> +        #address-cells = <0x2>;
> +
> +        pvpanic-mmio@9060000 {
> +                compatible = "qemu,pvpanic-mmio";
> +                reg = <0x0 0x9060000 0x0 0x2>;
> +        };
> +};
> +
> --
> 1.8.3.1
>

[PATCH V2 1/4] misc/pvpanic: move pvpanic to misc as common driver

[Peng Hao]

move pvpanic.c from drivers/platform/x86 to drivers/misc.
following patches will use pvpanic device in arm64.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 drivers/misc/Kconfig           |   7 +++
 drivers/misc/Makefile          |   1 +
 drivers/misc/pvpanic.c         | 124 +++++++++++++++++++++++++++++++++++++++++
 drivers/platform/Kconfig       |   3 +
 drivers/platform/Makefile      |   1 +
 drivers/platform/x86/Kconfig   |   8 ---
 drivers/platform/x86/Makefile  |   1 -
 drivers/platform/x86/pvpanic.c | 124 -----------------------------------------
 8 files changed, 136 insertions(+), 133 deletions(-)
 create mode 100644 drivers/misc/pvpanic.c
 delete mode 100644 drivers/platform/x86/pvpanic.c

diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 5d71300..ca55c94 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -499,6 +499,13 @@ config MISC_RTSX
 	tristate
 	default MISC_RTSX_PCI || MISC_RTSX_USB
 
+config PVPANIC
+	tristate "pvpanic device support"
+	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.
+
 source "drivers/misc/c2port/Kconfig"
 source "drivers/misc/eeprom/Kconfig"
 source "drivers/misc/cb710/Kconfig"
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 20be70c..39dc005 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -57,3 +57,4 @@ obj-$(CONFIG_ASPEED_LPC_SNOOP)	+= aspeed-lpc-snoop.o
 obj-$(CONFIG_PCI_ENDPOINT_TEST)	+= pci_endpoint_test.o
 obj-$(CONFIG_OCXL)		+= ocxl/
 obj-$(CONFIG_MISC_RTSX)		+= cardreader/
+obj-$(CONFIG_PVPANIC)  		+= pvpanic.o
diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
new file mode 100644
index 0000000..fd86dab
--- /dev/null
+++ b/drivers/misc/pvpanic.c
@@ -0,0 +1,124 @@
+/*
+ *  pvpanic.c - pvpanic Device Support
+ *
+ *  Copyright (C) 2013 Fujitsu.
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/acpi.h>
+
+MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
+MODULE_DESCRIPTION("pvpanic device driver");
+MODULE_LICENSE("GPL");
+
+static int pvpanic_add(struct acpi_device *device);
+static int pvpanic_remove(struct acpi_device *device);
+
+static const struct acpi_device_id pvpanic_device_ids[] = {
+	{ "QEMU0001", 0 },
+	{ "", 0 },
+};
+MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
+
+#define PVPANIC_PANICKED	(1 << 0)
+
+static u16 port;
+
+static struct acpi_driver pvpanic_driver = {
+	.name =		"pvpanic",
+	.class =	"QEMU",
+	.ids =		pvpanic_device_ids,
+	.ops =		{
+				.add =		pvpanic_add,
+				.remove =	pvpanic_remove,
+			},
+	.owner =	THIS_MODULE,
+};
+
+static void
+pvpanic_send_event(unsigned int event)
+{
+	outb(event, port);
+}
+
+static int
+pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
+		     void *unused)
+{
+	pvpanic_send_event(PVPANIC_PANICKED);
+	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 acpi_status
+pvpanic_walk_resources(struct acpi_resource *res, void *context)
+{
+	switch (res->type) {
+	case ACPI_RESOURCE_TYPE_END_TAG:
+		return AE_OK;
+
+	case ACPI_RESOURCE_TYPE_IO:
+		port = res->data.io.minimum;
+		return AE_OK;
+
+	default:
+		return AE_ERROR;
+	}
+}
+
+static int pvpanic_add(struct acpi_device *device)
+{
+	int ret;
+
+	ret = acpi_bus_get_status(device);
+	if (ret < 0)
+		return ret;
+
+	if (!device->status.enabled || !device->status.functional)
+		return -ENODEV;
+
+	acpi_walk_resources(device->handle, METHOD_NAME__CRS,
+			    pvpanic_walk_resources, NULL);
+
+	if (!port)
+		return -ENODEV;
+
+	atomic_notifier_chain_register(&panic_notifier_list,
+				       &pvpanic_panic_nb);
+
+	return 0;
+}
+
+static int pvpanic_remove(struct acpi_device *device)
+{
+
+	atomic_notifier_chain_unregister(&panic_notifier_list,
+					 &pvpanic_panic_nb);
+	return 0;
+}
+
+module_acpi_driver(pvpanic_driver);
diff --git a/drivers/platform/Kconfig b/drivers/platform/Kconfig
index d4c2e42..8b64b78 100644
--- a/drivers/platform/Kconfig
+++ b/drivers/platform/Kconfig
@@ -1,3 +1,6 @@
+if ARM64
+source "drivers/platform/arm/Kconfig"
+endif
 if X86
 source "drivers/platform/x86/Kconfig"
 endif
diff --git a/drivers/platform/Makefile b/drivers/platform/Makefile
index 4b2ce58..232231c 100644
--- a/drivers/platform/Makefile
+++ b/drivers/platform/Makefile
@@ -3,6 +3,7 @@
 # Makefile for linux/drivers/platform
 #
 
+obj-$(CONFIG_ARM64)               += arm/
 obj-$(CONFIG_X86)		+= x86/
 obj-$(CONFIG_MELLANOX_PLATFORM)	+= mellanox/
 obj-$(CONFIG_MIPS)		+= mips/
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index ef016e4..a6b412e 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -1119,14 +1119,6 @@ config INTEL_SMARTCONNECT
 	  This driver checks to determine whether the device has Intel Smart
 	  Connect enabled, and if so disables it.
 
-config PVPANIC
-	tristate "pvpanic device support"
-	depends on ACPI
-	---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 INTEL_PMC_IPC
 	tristate "Intel PMC IPC Driver"
 	depends on ACPI
diff --git a/drivers/platform/x86/Makefile b/drivers/platform/x86/Makefile
index 2ba6cb7..a4ec299 100644
--- a/drivers/platform/x86/Makefile
+++ b/drivers/platform/x86/Makefile
@@ -75,7 +75,6 @@ obj-$(CONFIG_APPLE_GMUX)	+= apple-gmux.o
 obj-$(CONFIG_INTEL_RST)		+= intel-rst.o
 obj-$(CONFIG_INTEL_SMARTCONNECT)	+= intel-smartconnect.o
 
-obj-$(CONFIG_PVPANIC)           += pvpanic.o
 obj-$(CONFIG_ALIENWARE_WMI)	+= alienware-wmi.o
 obj-$(CONFIG_INTEL_PMC_IPC)	+= intel_pmc_ipc.o
 obj-$(CONFIG_SILEAD_DMI)	+= silead_dmi.o
diff --git a/drivers/platform/x86/pvpanic.c b/drivers/platform/x86/pvpanic.c
deleted file mode 100644
index fd86dab..0000000
--- a/drivers/platform/x86/pvpanic.c
+++ /dev/null
@@ -1,124 +0,0 @@
-/*
- *  pvpanic.c - pvpanic Device Support
- *
- *  Copyright (C) 2013 Fujitsu.
- *
- *  This program is free software; you can redistribute it and/or modify
- *  it under the terms of the GNU General Public License as published by
- *  the Free Software Foundation; either version 2 of the License, or
- *  (at your option) any later version.
- *
- *  This program is distributed in the hope that it will be useful,
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- *  GNU General Public License for more details.
- *
- *  You should have received a copy of the GNU General Public License
- *  along with this program; if not, write to the Free Software
- *  Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
- */
-
-#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/types.h>
-#include <linux/acpi.h>
-
-MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
-MODULE_DESCRIPTION("pvpanic device driver");
-MODULE_LICENSE("GPL");
-
-static int pvpanic_add(struct acpi_device *device);
-static int pvpanic_remove(struct acpi_device *device);
-
-static const struct acpi_device_id pvpanic_device_ids[] = {
-	{ "QEMU0001", 0 },
-	{ "", 0 },
-};
-MODULE_DEVICE_TABLE(acpi, pvpanic_device_ids);
-
-#define PVPANIC_PANICKED	(1 << 0)
-
-static u16 port;
-
-static struct acpi_driver pvpanic_driver = {
-	.name =		"pvpanic",
-	.class =	"QEMU",
-	.ids =		pvpanic_device_ids,
-	.ops =		{
-				.add =		pvpanic_add,
-				.remove =	pvpanic_remove,
-			},
-	.owner =	THIS_MODULE,
-};
-
-static void
-pvpanic_send_event(unsigned int event)
-{
-	outb(event, port);
-}
-
-static int
-pvpanic_panic_notify(struct notifier_block *nb, unsigned long code,
-		     void *unused)
-{
-	pvpanic_send_event(PVPANIC_PANICKED);
-	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 acpi_status
-pvpanic_walk_resources(struct acpi_resource *res, void *context)
-{
-	switch (res->type) {
-	case ACPI_RESOURCE_TYPE_END_TAG:
-		return AE_OK;
-
-	case ACPI_RESOURCE_TYPE_IO:
-		port = res->data.io.minimum;
-		return AE_OK;
-
-	default:
-		return AE_ERROR;
-	}
-}
-
-static int pvpanic_add(struct acpi_device *device)
-{
-	int ret;
-
-	ret = acpi_bus_get_status(device);
-	if (ret < 0)
-		return ret;
-
-	if (!device->status.enabled || !device->status.functional)
-		return -ENODEV;
-
-	acpi_walk_resources(device->handle, METHOD_NAME__CRS,
-			    pvpanic_walk_resources, NULL);
-
-	if (!port)
-		return -ENODEV;
-
-	atomic_notifier_chain_register(&panic_notifier_list,
-				       &pvpanic_panic_nb);
-
-	return 0;
-}
-
-static int pvpanic_remove(struct acpi_device *device)
-{
-
-	atomic_notifier_chain_unregister(&panic_notifier_list,
-					 &pvpanic_panic_nb);
-	return 0;
-}
-
-module_acpi_driver(pvpanic_driver);
-- 
1.8.3.1

[PATCH V2 2/4] misc/pvpanic: add MMIO support

[Peng Hao]

On some architectures (e.g. arm64), it's preferable to use MMIO, since
this can be used standalone. Add MMIO support to the pvpanic driver.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 drivers/misc/pvpanic.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index fd86dab..bcfefdd 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -43,6 +43,8 @@
 
 static u16 port;
 
+static void __iomem *base;
+
 static struct acpi_driver pvpanic_driver = {
 	.name =		"pvpanic",
 	.class =	"QEMU",
@@ -57,7 +59,10 @@
 static void
 pvpanic_send_event(unsigned int event)
 {
-	outb(event, port);
+	if (port)
+		outb(event, port);
+	else if (base)
+		writeb(event, base);
 }
 
 static int
@@ -77,6 +82,8 @@
 static acpi_status
 pvpanic_walk_resources(struct acpi_resource *res, void *context)
 {
+	struct acpi_resource_fixed_memory32 *fixmem32;
+
 	switch (res->type) {
 	case ACPI_RESOURCE_TYPE_END_TAG:
 		return AE_OK;
@@ -85,6 +92,11 @@
 		port = res->data.io.minimum;
 		return AE_OK;
 
+	case ACPI_RESOURCE_TYPE_FIXED_MEMORY32:
+		fixmem32 = &res->data.fixed_memory32;
+		base = ioremap(fixmem32->address, fixmem32->address_length);
+		return AE_OK;
+
 	default:
 		return AE_ERROR;
 	}
@@ -104,7 +116,7 @@ static int pvpanic_add(struct acpi_device *device)
 	acpi_walk_resources(device->handle, METHOD_NAME__CRS,
 			    pvpanic_walk_resources, NULL);
 
-	if (!port)
+	if (!port && !base)
 		return -ENODEV;
 
 	atomic_notifier_chain_register(&panic_notifier_list,
@@ -118,6 +130,8 @@ static int pvpanic_remove(struct acpi_device *device)
 
 	atomic_notifier_chain_unregister(&panic_notifier_list,
 					 &pvpanic_panic_nb);
+	if (base)
+		iounmap(base);
 	return 0;
 }
 
-- 
1.8.3.1

[PATCH V2 3/4] misc/pvpanic: add support to pvpanic device information by using FDT

[Peng Hao]

By default, when ACPI tables and FDT coexist for ARM64,
current kernel takes precedence over FDT to get device information.
This patch increases the way to get information through FDT.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 drivers/misc/pvpanic.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index bcfefdd..e4b2b40 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -2,6 +2,7 @@
  *  pvpanic.c - pvpanic Device Support
  *
  *  Copyright (C) 2013 Fujitsu.
+ *  Copyright (C) 2018 ZTE.
  *
  *  This program is free software; you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -25,6 +26,10 @@
 #include <linux/init.h>
 #include <linux/types.h>
 #include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <asm/virt.h>
+#include <linux/platform_device.h>
 
 MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
 MODULE_DESCRIPTION("pvpanic device driver");
@@ -78,6 +83,37 @@
 	.priority = 1, /* let this called before broken drm_fb_helper */
 };
 
+static int pvpanic_mmio_probe(struct platform_device *pdev)
+{
+	struct resource *mem;
+
+	mem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!mem)
+		return -EINVAL;
+
+	if (!devm_request_mem_region(&pdev->dev, mem->start,
+				     resource_size(mem), pdev->name))
+		return -EBUSY;
+
+	base = devm_ioremap(&pdev->dev, mem->start,
+			    resource_size(mem));
+	if (base == NULL)
+		return -EFAULT;
+
+	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 acpi_status
 pvpanic_walk_resources(struct acpi_resource *res, void *context)
@@ -135,4 +171,35 @@ static int pvpanic_remove(struct acpi_device *device)
 	return 0;
 }
 
-module_acpi_driver(pvpanic_driver);
+static const struct of_device_id pvpanic_mmio_match[] = {
+	{ .compatible = "qemu,pvpanic-mmio", },
+	{},
+};
+
+static struct platform_driver pvpanic_mmio_driver = {
+	.driver = {
+		.name = "pvpanic-mmio",
+		.of_match_table = pvpanic_mmio_match,
+	},
+	.probe = pvpanic_mmio_probe,
+	.remove = pvpanic_mmio_remove,
+};
+
+static int __init pvpanic_mmio_init(void)
+{
+	if (acpi_disabled)
+		return platform_driver_register(&pvpanic_mmio_driver);
+	else
+		return acpi_bus_register_driver(&pvpanic_driver);
+}
+
+static void __exit pvpanic_mmio_exit(void)
+{
+	if (acpi_disabled)
+		platform_driver_unregister(&pvpanic_mmio_driver);
+	else
+		acpi_bus_unregister_driver(&pvpanic_driver);
+}
+
+module_init(pvpanic_mmio_init);
+module_exit(pvpanic_mmio_exit);
-- 
1.8.3.1

[PATCH V2 4/4] misc/pvpanic : pvpanic: add document for pvpanic-mmio DT

[Peng Hao]

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 .../devicetree/bindings/arm/pvpanic-mmio.txt       | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/arm/pvpanic-mmio.txt

diff --git a/Documentation/devicetree/bindings/arm/pvpanic-mmio.txt b/Documentation/devicetree/bindings/arm/pvpanic-mmio.txt
new file mode 100644
index 0000000..a6bdacd
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/pvpanic-mmio.txt
@@ -0,0 +1,29 @@
+* QEMU PVPANIC MMIO Configuration bindings for ARM
+
+QEMU's emulation / virtualization targets provide the following PVPANIC
+MMIO Configuration interface on the "virt" machine.
+type:
+
+- a read-write, 16-bit wide data register.
+
+QEMU exposes the data register to guests as memory mapped registers.
+
+Required properties:
+
+- compatible: "qemu,pvpanic-mmio".
+- reg: the MMIO region used by the device.
+  * Bytes 0x0  Write panic event to the reg when guest OS panics.
+  * Bytes 0x1  Reserved.
+
+Example:
+
+/ {
+        #size-cells = <0x2>;
+        #address-cells = <0x2>;
+
+        pvpanic-mmio@9060000 {
+                compatible = "qemu,pvpanic-mmio";
+                reg = <0x0 0x9060000 0x0 0x2>;
+        };
+};
+
-- 
1.8.3.1

Re: Re: [PATCH V2 2/4] misc/pvpanic: add MMIO support

[Arnd Bergmann]

On 10/26/18, peng.hao2@zte.com.cn <peng.hao2@zte.com.cn> wrote:
>>
>> On some architectures (e.g. arm64), it's preferable to use MMIO, since
>> this can be used standalone. Add MMIO support to the pvpanic driver.
>>
>>
>>No global variables, please.

There obviously has to be at least one global variable here, since the
caller of panic() has no context about the device that provides
the MMIO register. But I agree that using ioreadxx() here would
be nice to simplify the code, and it avoids having separate variables
for pio and mmio mode.

>>If your switch to ioreadxx()iowritexx() it would really help you here.
>>
> I can use iowrite8, but here:
>  if (base)
>       iounmap(base);
> I found the nearest API pci_iounmap, but it's a bit strange to see how this
> function is
> used here from the name of the function.
> And I do not confirm that this API is suitable for all architectures.

From reading the driver, I think you either come from a caller that
always uses PIO or from one that always uses MMIO, and they
are paired with probe/remove, so you can assign the global
__iomem pointer using either ioport_map() or ioremap(), and
undo that using ioport_unmap/iounmap respectively, while the
user just uses iowrite8() unconditionally. This is allowed by the
iowritexx() interfaces.

Alternatively, using devm_ioremap_resource() should work for
either pio or mmio, but it requires a device pointer, which in turn
means you have to slightly rewrite the ACPI portion to use a
platform_driver instead.

      Arnd