Minor changes from v10 with some cosmetic fixes and DT values
validation.
This adds a mechanism to detect stalls on the guest vCPUS by creating a
per CPU hrtimer which periodically 'pets' the host backend driver.
On a conventional watchdog-core driver, the userspace is responsible for
delivering the 'pet' events by writing to the particular /dev/watchdogN node.
In this case we require a strong thread affinity to be able to
account for lost time on a per vCPU basis.
This device driver acts as a soft lockup detector by relying on the host
backend driver to measure the elapesed time between subsequent 'pet' events.
If the elapsed time doesn't match an expected value, the backend driver
decides that the guest vCPU is locked and resets the guest. The host
backend driver takes into account the time that the guest is not
running. The communication with the backend driver is done through MMIO
and the register layout of the virtual watchdog is described as part of
the backend driver changes.
The host backend driver is implemented as part of:
https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3548817
Changelog v11:
- verify the values from DT if they are in an expected range and
fallback to default values in case they are not.
- added Will's review-by tag
Changelog v10:
- keep only the hrtimer and a flag in the per_cpu structure and move
the other fields in a separate config structure
- fix a potential race condition as pointed out by Will: the
driver remove(..) can race with the hotplug cpu notifiers
- replace alloc_percpu with devm_alloc_percpu and remove the free_percpu
- unregister the hotplug notifiers
- improve the Kconfig description and fix the license in the header
file
- add the review-by tag from Rob as the DT has not changed since v9
Changelog v9:
- make the driver depend on CONFIG_OF
- remove the platform_(set|get)_drvdata calls and keep a per-cpu static
variable `vm_stall_detect` as suggested by Guenter on the (v8) series
- improve commit description and fix styling
Sebastian Ene (2):
dt-bindings: vcpu_stall_detector: Add qemu,vcpu-stall-detector
compatible
misc: Add a mechanism to detect stalls on guest vCPUs
.../misc/qemu,vcpu-stall-detector.yaml | 51 ++++
drivers/misc/Kconfig | 14 ++
drivers/misc/Makefile | 1 +
drivers/misc/vcpu_stall_detector.c | 223 ++++++++++++++++++
4 files changed, 289 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/qemu,vcpu-stall-detector.yaml
create mode 100644 drivers/misc/vcpu_stall_detector.c
--
2.37.0.rc0.161.g10f37bed90-goog
The VCPU stall detection mechanism allows to configure the expiration
duration and the internal counter clock frequency measured in Hz.
Add these properties in the schema.
While this is a memory mapped virtual device, it is expected to be loaded
when the DT contains the compatible: "qemu,vcpu-stall-detector" node.
In a protected VM we trust the generated DT nodes and we don't rely on
the host to present the hardware peripherals.
Reviewed-by: Rob Herring <[email protected]>
Signed-off-by: Sebastian Ene <[email protected]>
---
.../misc/qemu,vcpu-stall-detector.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
create mode 100644 Documentation/devicetree/bindings/misc/qemu,vcpu-stall-detector.yaml
diff --git a/Documentation/devicetree/bindings/misc/qemu,vcpu-stall-detector.yaml b/Documentation/devicetree/bindings/misc/qemu,vcpu-stall-detector.yaml
new file mode 100644
index 000000000000..1aebeb696ee0
--- /dev/null
+++ b/Documentation/devicetree/bindings/misc/qemu,vcpu-stall-detector.yaml
@@ -0,0 +1,51 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/misc/qemu,vcpu-stall-detector.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: VCPU stall detector
+
+description:
+ This binding describes a CPU stall detector mechanism for virtual CPUs
+ which is accessed through MMIO.
+
+maintainers:
+ - Sebastian Ene <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - qemu,vcpu-stall-detector
+
+ reg:
+ maxItems: 1
+
+ clock-frequency:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ The internal clock of the stall detector peripheral measure in Hz used
+ to decrement its internal counter register on each tick.
+ Defaults to 10 if unset.
+ default: 10
+
+ timeout-sec:
+ description: |
+ The stall detector expiration timeout measured in seconds.
+ Defaults to 8 if unset. Please note that it also takes into account the
+ time spent while the VCPU is not running.
+ default: 8
+
+required:
+ - compatible
+
+additionalProperties: false
+
+examples:
+ - |
+ vmwdt@9030000 {
+ compatible = "qemu,vcpu-stall-detector";
+ reg = <0x9030000 0x10000>;
+ clock-frequency = <10>;
+ timeout-sec = <8>;
+ };
--
2.37.0.rc0.161.g10f37bed90-goog
This driver creates per-cpu hrtimers which are required to do the
periodic 'pet' operation. On a conventional watchdog-core driver, the
userspace is responsible for delivering the 'pet' events by writing to
the particular /dev/watchdogN node. In this case we require a strong
thread affinity to be able to account for lost time on a per vCPU.
This part of the driver is the 'frontend' which is reponsible for
delivering the periodic 'pet' events, configuring the virtual peripheral
and listening for cpu hotplug events. The other part of the driver is
an emulated MMIO device which is part of the KVM virtual machine
monitor and this part accounts for lost time by looking at the
/proc/{}/task/{}/stat entries.
Reviewed-by: Will Deacon <[email protected]>
Signed-off-by: Sebastian Ene <[email protected]>
---
drivers/misc/Kconfig | 14 ++
drivers/misc/Makefile | 1 +
drivers/misc/vcpu_stall_detector.c | 223 +++++++++++++++++++++++++++++
3 files changed, 238 insertions(+)
create mode 100644 drivers/misc/vcpu_stall_detector.c
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index 41d2bb0ae23a..d5b7610459f7 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -483,6 +483,20 @@ config OPEN_DICE
If unsure, say N.
+config VCPU_STALL_DETECTOR
+ tristate "Guest vCPU stall detector"
+ select LOCKUP_DETECTOR
+ depends on OF && HAS_IOMEM
+ help
+ When this driver is bound inside a KVM guest, it will
+ periodically "pet" an MMIO stall detector device from each vCPU
+ and allow the host to detect vCPU stalls.
+
+ To compile this driver as a module, choose M here: the module
+ will be called vcpu_stall_detector.
+
+ If you do not intend to run this kernel as a guest, say N.
+
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 70e800e9127f..2be8542616dd 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -60,3 +60,4 @@ obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
obj-$(CONFIG_OPEN_DICE) += open-dice.o
+obj-$(CONFIG_VCPU_STALL_DETECTOR) += vcpu_stall_detector.o
\ No newline at end of file
diff --git a/drivers/misc/vcpu_stall_detector.c b/drivers/misc/vcpu_stall_detector.c
new file mode 100644
index 000000000000..53b5506080e1
--- /dev/null
+++ b/drivers/misc/vcpu_stall_detector.c
@@ -0,0 +1,223 @@
+// SPDX-License-Identifier: GPL-2.0-only
+//
+// VCPU stall detector.
+// Copyright (C) Google, 2022
+
+#include <linux/cpu.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/kernel.h>
+
+#include <linux/device.h>
+#include <linux/interrupt.h>
+#include <linux/module.h>
+#include <linux/nmi.h>
+#include <linux/of.h>
+#include <linux/of_device.h>
+#include <linux/param.h>
+#include <linux/percpu.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+#define VCPU_STALL_REG_STATUS (0x00)
+#define VCPU_STALL_REG_LOAD_CNT (0x04)
+#define VCPU_STALL_REG_CURRENT_CNT (0x08)
+#define VCPU_STALL_REG_CLOCK_FREQ_HZ (0x0C)
+#define VCPU_STALL_REG_LEN (0x10)
+
+#define VCPU_STALL_DEFAULT_CLOCK_HZ (10)
+#define VCPU_STALL_MAX_CLOCK_HZ (100)
+#define VCPU_STALL_DEFAULT_TIMEOUT_SEC (8)
+#define VCPU_STALL_MAX_TIMEOUT_SEC (600)
+
+struct vcpu_stall_detect_config {
+ u32 clock_freq_hz;
+ u32 stall_timeout_sec;
+
+ void __iomem *membase;
+ struct platform_device *dev;
+ enum cpuhp_state hp_online;
+};
+
+struct vcpu_stall_priv {
+ struct hrtimer vcpu_hrtimer;
+ bool is_initialized;
+};
+
+/* The vcpu stall configuration structure which applies to all the CPUs */
+static struct vcpu_stall_detect_config vcpu_stall_config;
+
+#define vcpu_stall_reg_write(vcpu, reg, value) \
+ writel_relaxed((value), \
+ (void __iomem *)(vcpu_stall_config.membase + \
+ (vcpu) * VCPU_STALL_REG_LEN + (reg)))
+
+
+static struct vcpu_stall_priv __percpu *vcpu_stall_detectors;
+
+static enum hrtimer_restart
+vcpu_stall_detect_timer_fn(struct hrtimer *hrtimer)
+{
+ u32 ticks, ping_timeout_ms;
+
+ /* Reload the stall detector counter register every
+ * `ping_timeout_ms` to prevent the virtual device
+ * from decrementing it to 0. The virtual device decrements this
+ * register at 'clock_freq_hz' frequency.
+ */
+ ticks = vcpu_stall_config.clock_freq_hz *
+ vcpu_stall_config.stall_timeout_sec;
+ vcpu_stall_reg_write(smp_processor_id(),
+ VCPU_STALL_REG_LOAD_CNT, ticks);
+
+ ping_timeout_ms = vcpu_stall_config.stall_timeout_sec *
+ MSEC_PER_SEC / 2;
+ hrtimer_forward_now(hrtimer,
+ ms_to_ktime(ping_timeout_ms));
+
+ return HRTIMER_RESTART;
+}
+
+static int start_stall_detector_cpu(unsigned int cpu)
+{
+ u32 ticks, ping_timeout_ms;
+ struct vcpu_stall_priv *vcpu_stall_detector =
+ this_cpu_ptr(vcpu_stall_detectors);
+ struct hrtimer *vcpu_hrtimer = &vcpu_stall_detector->vcpu_hrtimer;
+
+ vcpu_stall_reg_write(cpu, VCPU_STALL_REG_CLOCK_FREQ_HZ,
+ vcpu_stall_config.clock_freq_hz);
+
+ /* Compute the number of ticks required for the stall detector
+ * counter register based on the internal clock frequency and the
+ * timeout value given from the device tree.
+ */
+ ticks = vcpu_stall_config.clock_freq_hz *
+ vcpu_stall_config.stall_timeout_sec;
+ vcpu_stall_reg_write(cpu, VCPU_STALL_REG_LOAD_CNT, ticks);
+
+ /* Enable the internal clock and start the stall detector */
+ vcpu_stall_reg_write(cpu, VCPU_STALL_REG_STATUS, 1);
+
+ /* Pet the stall detector at half of its expiration timeout
+ * to prevent spurious resets.
+ */
+ ping_timeout_ms = vcpu_stall_config.stall_timeout_sec *
+ MSEC_PER_SEC / 2;
+
+ hrtimer_init(vcpu_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
+ vcpu_hrtimer->function = vcpu_stall_detect_timer_fn;
+ vcpu_stall_detector->is_initialized = true;
+
+ hrtimer_start(vcpu_hrtimer, ms_to_ktime(ping_timeout_ms),
+ HRTIMER_MODE_REL_PINNED);
+
+ return 0;
+}
+
+static int stop_stall_detector_cpu(unsigned int cpu)
+{
+ struct vcpu_stall_priv *vcpu_stall_detector =
+ per_cpu_ptr(vcpu_stall_detectors, cpu);
+
+ if (!vcpu_stall_detector->is_initialized)
+ return 0;
+
+ /* Disable the stall detector for the current CPU */
+ hrtimer_cancel(&vcpu_stall_detector->vcpu_hrtimer);
+ vcpu_stall_reg_write(cpu, VCPU_STALL_REG_STATUS, 0);
+ vcpu_stall_detector->is_initialized = false;
+
+ return 0;
+}
+
+static int vcpu_stall_detect_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct resource *r;
+ void __iomem *membase;
+ u32 clock_freq_hz = VCPU_STALL_DEFAULT_CLOCK_HZ;
+ u32 stall_timeout_sec = VCPU_STALL_DEFAULT_TIMEOUT_SEC;
+ struct device_node *np = pdev->dev.of_node;
+
+ vcpu_stall_detectors = devm_alloc_percpu(&pdev->dev,
+ typeof(struct vcpu_stall_priv));
+ if (!vcpu_stall_detectors)
+ return -ENOMEM;
+
+ membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
+ if (IS_ERR(membase)) {
+ dev_err(&pdev->dev, "Failed to get memory resource\n");
+ return PTR_ERR(membase);
+ }
+
+ if (!of_property_read_u32(np, "clock-frequency", &clock_freq_hz)) {
+ if (!(clock_freq_hz > 0 &&
+ clock_freq_hz < VCPU_STALL_MAX_CLOCK_HZ)) {
+ dev_warn(&pdev->dev, "clk out of range\n");
+ clock_freq_hz = VCPU_STALL_DEFAULT_CLOCK_HZ;
+ }
+ }
+
+ if (!of_property_read_u32(np, "timeout-sec", &stall_timeout_sec)) {
+ if (!(stall_timeout_sec > 0 &&
+ stall_timeout_sec < VCPU_STALL_MAX_TIMEOUT_SEC)) {
+ dev_warn(&pdev->dev, "stall timeout out of range\n");
+ stall_timeout_sec = VCPU_STALL_DEFAULT_TIMEOUT_SEC;
+ }
+ }
+
+ vcpu_stall_config = (struct vcpu_stall_detect_config) {
+ .membase = membase,
+ .clock_freq_hz = clock_freq_hz,
+ .stall_timeout_sec = stall_timeout_sec
+ };
+
+ ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
+ "virt/vcpu_stall_detector:online",
+ start_stall_detector_cpu,
+ stop_stall_detector_cpu);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to install cpu hotplug");
+ goto err;
+ }
+
+ vcpu_stall_config.hp_online = ret;
+ return 0;
+err:
+ return ret;
+}
+
+static int vcpu_stall_detect_remove(struct platform_device *pdev)
+{
+ int cpu;
+
+ cpuhp_remove_state(vcpu_stall_config.hp_online);
+
+ for_each_possible_cpu(cpu)
+ stop_stall_detector_cpu(cpu);
+
+ return 0;
+}
+
+static const struct of_device_id vcpu_stall_detect_of_match[] = {
+ { .compatible = "qemu,vcpu-stall-detector", },
+ {}
+};
+
+MODULE_DEVICE_TABLE(of, vcpu_stall_detect_of_match);
+
+static struct platform_driver vcpu_stall_detect_driver = {
+ .probe = vcpu_stall_detect_probe,
+ .remove = vcpu_stall_detect_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = vcpu_stall_detect_of_match,
+ },
+};
+
+module_platform_driver(vcpu_stall_detect_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Sebastian Ene <[email protected]>");
+MODULE_DESCRIPTION("VCPU stall detector");
--
2.37.0.rc0.161.g10f37bed90-goog
On Fri, Jul 08, 2022 at 11:23:45AM +0000, Sebastian Ene wrote:
> This driver creates per-cpu hrtimers which are required to do the
> periodic 'pet' operation. On a conventional watchdog-core driver, the
> userspace is responsible for delivering the 'pet' events by writing to
> the particular /dev/watchdogN node. In this case we require a strong
> thread affinity to be able to account for lost time on a per vCPU.
>
> This part of the driver is the 'frontend' which is reponsible for
> delivering the periodic 'pet' events, configuring the virtual peripheral
> and listening for cpu hotplug events. The other part of the driver is
> an emulated MMIO device which is part of the KVM virtual machine
> monitor and this part accounts for lost time by looking at the
> /proc/{}/task/{}/stat entries.
>
> Reviewed-by: Will Deacon <[email protected]>
> Signed-off-by: Sebastian Ene <[email protected]>
Reviewed-by: Guenter Roeck <[email protected]>
> ---
> drivers/misc/Kconfig | 14 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/vcpu_stall_detector.c | 223 +++++++++++++++++++++++++++++
> 3 files changed, 238 insertions(+)
> create mode 100644 drivers/misc/vcpu_stall_detector.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 41d2bb0ae23a..d5b7610459f7 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -483,6 +483,20 @@ config OPEN_DICE
>
> If unsure, say N.
>
> +config VCPU_STALL_DETECTOR
> + tristate "Guest vCPU stall detector"
> + select LOCKUP_DETECTOR
> + depends on OF && HAS_IOMEM
> + help
> + When this driver is bound inside a KVM guest, it will
> + periodically "pet" an MMIO stall detector device from each vCPU
> + and allow the host to detect vCPU stalls.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called vcpu_stall_detector.
> +
> + If you do not intend to run this kernel as a guest, say N.
> +
> 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 70e800e9127f..2be8542616dd 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -60,3 +60,4 @@ obj-$(CONFIG_XILINX_SDFEC) += xilinx_sdfec.o
> obj-$(CONFIG_HISI_HIKEY_USB) += hisi_hikey_usb.o
> obj-$(CONFIG_HI6421V600_IRQ) += hi6421v600-irq.o
> obj-$(CONFIG_OPEN_DICE) += open-dice.o
> +obj-$(CONFIG_VCPU_STALL_DETECTOR) += vcpu_stall_detector.o
> \ No newline at end of file
> diff --git a/drivers/misc/vcpu_stall_detector.c b/drivers/misc/vcpu_stall_detector.c
> new file mode 100644
> index 000000000000..53b5506080e1
> --- /dev/null
> +++ b/drivers/misc/vcpu_stall_detector.c
> @@ -0,0 +1,223 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// VCPU stall detector.
> +// Copyright (C) Google, 2022
> +
> +#include <linux/cpu.h>
> +#include <linux/init.h>
> +#include <linux/io.h>
> +#include <linux/kernel.h>
> +
> +#include <linux/device.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/nmi.h>
> +#include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <linux/param.h>
> +#include <linux/percpu.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +
> +#define VCPU_STALL_REG_STATUS (0x00)
> +#define VCPU_STALL_REG_LOAD_CNT (0x04)
> +#define VCPU_STALL_REG_CURRENT_CNT (0x08)
> +#define VCPU_STALL_REG_CLOCK_FREQ_HZ (0x0C)
> +#define VCPU_STALL_REG_LEN (0x10)
> +
> +#define VCPU_STALL_DEFAULT_CLOCK_HZ (10)
> +#define VCPU_STALL_MAX_CLOCK_HZ (100)
> +#define VCPU_STALL_DEFAULT_TIMEOUT_SEC (8)
> +#define VCPU_STALL_MAX_TIMEOUT_SEC (600)
> +
> +struct vcpu_stall_detect_config {
> + u32 clock_freq_hz;
> + u32 stall_timeout_sec;
> +
> + void __iomem *membase;
> + struct platform_device *dev;
> + enum cpuhp_state hp_online;
> +};
> +
> +struct vcpu_stall_priv {
> + struct hrtimer vcpu_hrtimer;
> + bool is_initialized;
> +};
> +
> +/* The vcpu stall configuration structure which applies to all the CPUs */
> +static struct vcpu_stall_detect_config vcpu_stall_config;
> +
> +#define vcpu_stall_reg_write(vcpu, reg, value) \
> + writel_relaxed((value), \
> + (void __iomem *)(vcpu_stall_config.membase + \
> + (vcpu) * VCPU_STALL_REG_LEN + (reg)))
> +
> +
> +static struct vcpu_stall_priv __percpu *vcpu_stall_detectors;
> +
> +static enum hrtimer_restart
> +vcpu_stall_detect_timer_fn(struct hrtimer *hrtimer)
> +{
> + u32 ticks, ping_timeout_ms;
> +
> + /* Reload the stall detector counter register every
> + * `ping_timeout_ms` to prevent the virtual device
> + * from decrementing it to 0. The virtual device decrements this
> + * register at 'clock_freq_hz' frequency.
> + */
> + ticks = vcpu_stall_config.clock_freq_hz *
> + vcpu_stall_config.stall_timeout_sec;
> + vcpu_stall_reg_write(smp_processor_id(),
> + VCPU_STALL_REG_LOAD_CNT, ticks);
> +
> + ping_timeout_ms = vcpu_stall_config.stall_timeout_sec *
> + MSEC_PER_SEC / 2;
> + hrtimer_forward_now(hrtimer,
> + ms_to_ktime(ping_timeout_ms));
> +
> + return HRTIMER_RESTART;
> +}
> +
> +static int start_stall_detector_cpu(unsigned int cpu)
> +{
> + u32 ticks, ping_timeout_ms;
> + struct vcpu_stall_priv *vcpu_stall_detector =
> + this_cpu_ptr(vcpu_stall_detectors);
> + struct hrtimer *vcpu_hrtimer = &vcpu_stall_detector->vcpu_hrtimer;
> +
> + vcpu_stall_reg_write(cpu, VCPU_STALL_REG_CLOCK_FREQ_HZ,
> + vcpu_stall_config.clock_freq_hz);
> +
> + /* Compute the number of ticks required for the stall detector
> + * counter register based on the internal clock frequency and the
> + * timeout value given from the device tree.
> + */
> + ticks = vcpu_stall_config.clock_freq_hz *
> + vcpu_stall_config.stall_timeout_sec;
> + vcpu_stall_reg_write(cpu, VCPU_STALL_REG_LOAD_CNT, ticks);
> +
> + /* Enable the internal clock and start the stall detector */
> + vcpu_stall_reg_write(cpu, VCPU_STALL_REG_STATUS, 1);
> +
> + /* Pet the stall detector at half of its expiration timeout
> + * to prevent spurious resets.
> + */
> + ping_timeout_ms = vcpu_stall_config.stall_timeout_sec *
> + MSEC_PER_SEC / 2;
> +
> + hrtimer_init(vcpu_hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> + vcpu_hrtimer->function = vcpu_stall_detect_timer_fn;
> + vcpu_stall_detector->is_initialized = true;
> +
> + hrtimer_start(vcpu_hrtimer, ms_to_ktime(ping_timeout_ms),
> + HRTIMER_MODE_REL_PINNED);
> +
> + return 0;
> +}
> +
> +static int stop_stall_detector_cpu(unsigned int cpu)
> +{
> + struct vcpu_stall_priv *vcpu_stall_detector =
> + per_cpu_ptr(vcpu_stall_detectors, cpu);
> +
> + if (!vcpu_stall_detector->is_initialized)
> + return 0;
> +
> + /* Disable the stall detector for the current CPU */
> + hrtimer_cancel(&vcpu_stall_detector->vcpu_hrtimer);
> + vcpu_stall_reg_write(cpu, VCPU_STALL_REG_STATUS, 0);
> + vcpu_stall_detector->is_initialized = false;
> +
> + return 0;
> +}
> +
> +static int vcpu_stall_detect_probe(struct platform_device *pdev)
> +{
> + int ret;
> + struct resource *r;
> + void __iomem *membase;
> + u32 clock_freq_hz = VCPU_STALL_DEFAULT_CLOCK_HZ;
> + u32 stall_timeout_sec = VCPU_STALL_DEFAULT_TIMEOUT_SEC;
> + struct device_node *np = pdev->dev.of_node;
> +
> + vcpu_stall_detectors = devm_alloc_percpu(&pdev->dev,
> + typeof(struct vcpu_stall_priv));
> + if (!vcpu_stall_detectors)
> + return -ENOMEM;
> +
> + membase = devm_platform_get_and_ioremap_resource(pdev, 0, &r);
> + if (IS_ERR(membase)) {
> + dev_err(&pdev->dev, "Failed to get memory resource\n");
> + return PTR_ERR(membase);
> + }
> +
> + if (!of_property_read_u32(np, "clock-frequency", &clock_freq_hz)) {
> + if (!(clock_freq_hz > 0 &&
> + clock_freq_hz < VCPU_STALL_MAX_CLOCK_HZ)) {
> + dev_warn(&pdev->dev, "clk out of range\n");
> + clock_freq_hz = VCPU_STALL_DEFAULT_CLOCK_HZ;
> + }
> + }
> +
> + if (!of_property_read_u32(np, "timeout-sec", &stall_timeout_sec)) {
> + if (!(stall_timeout_sec > 0 &&
> + stall_timeout_sec < VCPU_STALL_MAX_TIMEOUT_SEC)) {
> + dev_warn(&pdev->dev, "stall timeout out of range\n");
> + stall_timeout_sec = VCPU_STALL_DEFAULT_TIMEOUT_SEC;
> + }
> + }
> +
> + vcpu_stall_config = (struct vcpu_stall_detect_config) {
> + .membase = membase,
> + .clock_freq_hz = clock_freq_hz,
> + .stall_timeout_sec = stall_timeout_sec
> + };
> +
> + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> + "virt/vcpu_stall_detector:online",
> + start_stall_detector_cpu,
> + stop_stall_detector_cpu);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "failed to install cpu hotplug");
> + goto err;
> + }
> +
> + vcpu_stall_config.hp_online = ret;
> + return 0;
> +err:
> + return ret;
> +}
> +
> +static int vcpu_stall_detect_remove(struct platform_device *pdev)
> +{
> + int cpu;
> +
> + cpuhp_remove_state(vcpu_stall_config.hp_online);
> +
> + for_each_possible_cpu(cpu)
> + stop_stall_detector_cpu(cpu);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id vcpu_stall_detect_of_match[] = {
> + { .compatible = "qemu,vcpu-stall-detector", },
> + {}
> +};
> +
> +MODULE_DEVICE_TABLE(of, vcpu_stall_detect_of_match);
> +
> +static struct platform_driver vcpu_stall_detect_driver = {
> + .probe = vcpu_stall_detect_probe,
> + .remove = vcpu_stall_detect_remove,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = vcpu_stall_detect_of_match,
> + },
> +};
> +
> +module_platform_driver(vcpu_stall_detect_driver);
> +
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Sebastian Ene <[email protected]>");
> +MODULE_DESCRIPTION("VCPU stall detector");
> --
> 2.37.0.rc0.161.g10f37bed90-goog
>
On Fri, Jul 08, 2022 at 11:23:45AM +0000, Sebastian Ene wrote:
> This driver creates per-cpu hrtimers which are required to do the
> periodic 'pet' operation. On a conventional watchdog-core driver, the
> userspace is responsible for delivering the 'pet' events by writing to
> the particular /dev/watchdogN node. In this case we require a strong
> thread affinity to be able to account for lost time on a per vCPU.
>
> This part of the driver is the 'frontend' which is reponsible for
> delivering the periodic 'pet' events, configuring the virtual peripheral
> and listening for cpu hotplug events. The other part of the driver is
> an emulated MMIO device which is part of the KVM virtual machine
> monitor and this part accounts for lost time by looking at the
> /proc/{}/task/{}/stat entries.
>
> Reviewed-by: Will Deacon <[email protected]>
> Signed-off-by: Sebastian Ene <[email protected]>
> ---
> drivers/misc/Kconfig | 14 ++
> drivers/misc/Makefile | 1 +
> drivers/misc/vcpu_stall_detector.c | 223 +++++++++++++++++++++++++++++
> 3 files changed, 238 insertions(+)
> create mode 100644 drivers/misc/vcpu_stall_detector.c
>
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index 41d2bb0ae23a..d5b7610459f7 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -483,6 +483,20 @@ config OPEN_DICE
>
> If unsure, say N.
>
> +config VCPU_STALL_DETECTOR
> + tristate "Guest vCPU stall detector"
> + select LOCKUP_DETECTOR
This should be a "depends on", not a select, right? This got enabled on
my build when I didn't want it to, and trying to track down why it was
enabled would be a pain for people.
thanks,
greg k-h
On Fri, Jul 08, 2022 at 03:47:23PM +0200, Greg Kroah-Hartman wrote:
> On Fri, Jul 08, 2022 at 11:23:45AM +0000, Sebastian Ene wrote:
> > This driver creates per-cpu hrtimers which are required to do the
> > periodic 'pet' operation. On a conventional watchdog-core driver, the
> > userspace is responsible for delivering the 'pet' events by writing to
> > the particular /dev/watchdogN node. In this case we require a strong
> > thread affinity to be able to account for lost time on a per vCPU.
> >
> > This part of the driver is the 'frontend' which is reponsible for
> > delivering the periodic 'pet' events, configuring the virtual peripheral
> > and listening for cpu hotplug events. The other part of the driver is
> > an emulated MMIO device which is part of the KVM virtual machine
> > monitor and this part accounts for lost time by looking at the
> > /proc/{}/task/{}/stat entries.
> >
> > Reviewed-by: Will Deacon <[email protected]>
> > Signed-off-by: Sebastian Ene <[email protected]>
> > ---
> > drivers/misc/Kconfig | 14 ++
> > drivers/misc/Makefile | 1 +
> > drivers/misc/vcpu_stall_detector.c | 223 +++++++++++++++++++++++++++++
> > 3 files changed, 238 insertions(+)
> > create mode 100644 drivers/misc/vcpu_stall_detector.c
> >
> > diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> > index 41d2bb0ae23a..d5b7610459f7 100644
> > --- a/drivers/misc/Kconfig
> > +++ b/drivers/misc/Kconfig
> > @@ -483,6 +483,20 @@ config OPEN_DICE
> >
> > If unsure, say N.
> >
> > +config VCPU_STALL_DETECTOR
> > + tristate "Guest vCPU stall detector"
> > + select LOCKUP_DETECTOR
Hi Greh,
>
> This should be a "depends on", not a select, right? This got enabled on
> my build when I didn't want it to, and trying to track down why it was
> enabled would be a pain for people.
Thanks for noticing it ! I think we can completely remove this
because it was needed in (v9) for the `watchdog_cpumask` and currently
we are not using it anymore.
>
> thanks,
>
> greg k-h
Thanks,
Seb