[PATCH v6 0/7] ptp: vmclock: Add VM generation counter and ACPI notification

[PATCH v6 0/7] ptp: vmclock: Add VM generation counter and ACPI notification

[Takahiro Itazuri]

Similarly to live migration, starting a VM from some serialized state
(aka snapshot) is an event which calls for adjusting guest clocks, hence
a hypervisor should increase the disruption_marker before resuming the
VM vCPUs, letting the guest know.

However, loading a snapshot, is slightly different than live migration,
especially since we can start multiple VMs from the same serialized
state. Apart from adjusting clocks, the guest needs to take additional
action during such events, e.g. recreate UUIDs, reset network
adapters/connections, reseed entropy pools, etc. These actions are not
necessary during live migration. This calls for a differentiation
between the two triggering events.

We differentiate between the two events via an extra field in the
vmclock_abi, called vm_generation_counter. Whereas hypervisors should
increase the disruption marker in both cases, they should only increase
vm_generation_counter when a snapshot is loaded in a VM (not during live
migration).

Additionally, we attach an ACPI notification to VMClock. Implementing
the notification is optional for the device. VMClock device will declare
that it implements the notification by setting
VMCLOCK_FLAG_NOTIFICATION_PRESENT bit in vmclock_abi flags. Hypervisors
that implement the notification must send an ACPI notification every
time seq_count changes to an even number. The driver will propagate
these notifications to userspace via the poll() interface.

Changes:

* RFC -> v1:
  - Made the notification support optional. Hypervisor needs to
    advertise support for the notification via a flag in vmclock_abi.
    Subsequently, poll() will return POLLHUP when the feature is not
    supported, to avoid having userspace blocking indefinitely waiting
    for events that won't arrive
  - Reworded the comment around vm_generation_counter field to avoid
    speaking about "jumping forward in time".
* v1 -> v2:
  - Correctly handle failures when calling vmclock_setup_notification to
    setup notifications.
  - Use atomic_t for fst->seq and handle the case of concurrent
    read()/poll() accesses.
  - Initialize fst->seq to 0 rather than what is currently stored in the
    shared page. This is to avoid reading odd numbers.
  - Add DT bindings similar to existing VMGenID ones.
* v2 -> v3:
  - Include missing header file and drop unused variables in PATH 2/4.
  - Include missing Reviewed-by in PATCH 1/4.
  - Fix DT node name to be generic (s/vmclock/ptp).
  - Include missing maintainers.
* v3 -> v4:
  - Added bindings file as maintained by David as part for PTP VMCLOCK
    SUPPORT.
  - Use le64_to_cpu() to access clk->flags.
  - Reference public specification.
  - Pass struct vmclock_state * in the of IRQ handler
  - Included three new patches from David:
    * Making ACPI optional for the driver.
    * Adding "VMCLOCK" to ACPI match.
    * Return TAI instead of UTC.
* v4 -> v5:
  - Use IF_ENABLED instead of #ifdef
  - Use reverse christmas tree order in variable declaration
  - Fix empty changelog
* v5 -> v6:
  - Fix blank line CHECKs reported by checkpatch.pl

Chalios, Babis (2):
  ptp: vmclock: add vm generation counter
  ptp: vmclock: support device notifications

David Woodhouse (5):
  dt-bindings: ptp: Add amazon,vmclock
  ptp: ptp_vmclock: Add device tree support
  ptp: ptp_vmclock: add 'VMCLOCK' to ACPI device match
  ptp: ptp_vmclock: remove dependency on CONFIG_ACPI
  ptp: ptp_vmclock: return TAI not UTC

 .../bindings/ptp/amazon,vmclock.yaml          |  46 ++++
 MAINTAINERS                                   |   1 +
 drivers/ptp/Kconfig                           |   2 +-
 drivers/ptp/ptp_vmclock.c                     | 196 ++++++++++++++++--
 include/uapi/linux/vmclock-abi.h              |  20 ++
 5 files changed, 251 insertions(+), 14 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/ptp/amazon,vmclock.yaml

-- 
2.47.3

[PATCH v6 1/7] ptp: vmclock: add vm generation counter

[Takahiro Itazuri]

From: "Babis Chalios" <bchalios@amazon.es>

Similar to live migration, loading a VM from some saved state (aka
snapshot) is also an event that calls for clock adjustments in the
guest. However, guests might want to take more actions as a response to
such events, e.g. as discarding UUIDs, resetting network connections,
reseeding entropy pools, etc. These are actions that guests don't
typically take during live migration, so add a new field in the
vmclock_abi called vm_generation_counter which informs the guest about
such events.

Hypervisor advertises support for vm_generation_counter through the
VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT flag. Users need to check the
presence of this bit in vmclock_abi flags field before using this flag.

Signed-off-by: Babis Chalios <bchalios@amazon.es>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
 include/uapi/linux/vmclock-abi.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/include/uapi/linux/vmclock-abi.h b/include/uapi/linux/vmclock-abi.h
index 2d99b29ac44a..937fe00e4f33 100644
--- a/include/uapi/linux/vmclock-abi.h
+++ b/include/uapi/linux/vmclock-abi.h
@@ -115,6 +115,12 @@ struct vmclock_abi {
 	 * bit again after the update, using the about-to-be-valid fields.
 	 */
 #define VMCLOCK_FLAG_TIME_MONOTONIC		(1 << 7)
+	/*
+	 * If the VM_GEN_COUNTER_PRESENT flag is set, the hypervisor will
+	 * bump the vm_generation_counter field every time the guest is
+	 * loaded from some save state (restored from a snapshot).
+	 */
+#define VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT     (1 << 8)
 
 	__u8 pad[2];
 	__u8 clock_status;
@@ -177,6 +183,15 @@ struct vmclock_abi {
 	__le64 time_frac_sec;		/* Units of 1/2^64 of a second */
 	__le64 time_esterror_nanosec;
 	__le64 time_maxerror_nanosec;
+
+	/*
+	 * This field changes to another non-repeating value when the guest
+	 * has been loaded from a snapshot. In addition to handling a
+	 * disruption in time (which will also be signalled through the
+	 * disruption_marker field), a guest may wish to discard UUIDs,
+	 * reset network connections, reseed entropy, etc.
+	 */
+	__le64 vm_generation_counter;
 };
 
 #endif /*  __VMCLOCK_ABI_H__ */
-- 
2.47.3

[PATCH v6 2/7] ptp: vmclock: support device notifications

[Takahiro Itazuri]

From: "Babis Chalios" <bchalios@amazon.es>

Add optional support for device notifications in VMClock. When
supported, the hypervisor will send a device notification every time it
updates the seq_count to a new even value.

Moreover, add support for poll() in VMClock as a means to propagate this
notification to user space. poll() will return a POLLIN event to
listeners every time seq_count changes to a value different than the one
last seen (since open() or last read()/pread()). This means that when
poll() returns a POLLIN event, listeners need to use read() to observe
what has changed and update the reader's view of seq_count. In other
words, after a poll() returned, all subsequent calls to poll() will
immediately return with a POLLIN event until the listener calls read().

The device advertises support for the notification mechanism by setting
flag VMCLOCK_FLAG_NOTIFICATION_PRESENT in vmclock_abi flags field. If
the flag is not present the driver won't setup the ACPI notification
handler and poll() will always immediately return POLLHUP.

Signed-off-by: Babis Chalios <bchalios@amazon.es>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
---
 drivers/ptp/ptp_vmclock.c        | 125 +++++++++++++++++++++++++++++--
 include/uapi/linux/vmclock-abi.h |   5 ++
 2 files changed, 123 insertions(+), 7 deletions(-)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index b3a83b03d9c1..38b2bacb755e 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -5,6 +5,9 @@
  * Copyright © 2024 Amazon.com, Inc. or its affiliates.
  */
 
+#include "linux/poll.h"
+#include "linux/types.h"
+#include "linux/wait.h"
 #include <linux/acpi.h>
 #include <linux/device.h>
 #include <linux/err.h>
@@ -39,6 +42,7 @@ struct vmclock_state {
 	struct resource res;
 	struct vmclock_abi *clk;
 	struct miscdevice miscdev;
+	wait_queue_head_t disrupt_wait;
 	struct ptp_clock_info ptp_clock_info;
 	struct ptp_clock *ptp_clock;
 	enum clocksource_ids cs_id, sys_cs_id;
@@ -357,10 +361,15 @@ static struct ptp_clock *vmclock_ptp_register(struct device *dev,
 	return ptp_clock_register(&st->ptp_clock_info, dev);
 }
 
+struct vmclock_file_state {
+	struct vmclock_state *st;
+	atomic_t seq;
+};
+
 static int vmclock_miscdev_mmap(struct file *fp, struct vm_area_struct *vma)
 {
-	struct vmclock_state *st = container_of(fp->private_data,
-						struct vmclock_state, miscdev);
+	struct vmclock_file_state *fst = fp->private_data;
+	struct vmclock_state *st = fst->st;
 
 	if ((vma->vm_flags & (VM_READ|VM_WRITE)) != VM_READ)
 		return -EROFS;
@@ -379,11 +388,11 @@ static int vmclock_miscdev_mmap(struct file *fp, struct vm_area_struct *vma)
 static ssize_t vmclock_miscdev_read(struct file *fp, char __user *buf,
 				    size_t count, loff_t *ppos)
 {
-	struct vmclock_state *st = container_of(fp->private_data,
-						struct vmclock_state, miscdev);
 	ktime_t deadline = ktime_add(ktime_get(), VMCLOCK_MAX_WAIT);
+	struct vmclock_file_state *fst = fp->private_data;
+	struct vmclock_state *st = fst->st;
+	uint32_t seq, old_seq;
 	size_t max_count;
-	uint32_t seq;
 
 	if (*ppos >= PAGE_SIZE)
 		return 0;
@@ -392,6 +401,7 @@ static ssize_t vmclock_miscdev_read(struct file *fp, char __user *buf,
 	if (count > max_count)
 		count = max_count;
 
+	old_seq = atomic_read(&fst->seq);
 	while (1) {
 		seq = le32_to_cpu(st->clk->seq_count) & ~1U;
 		/* Pairs with hypervisor wmb */
@@ -402,8 +412,16 @@ static ssize_t vmclock_miscdev_read(struct file *fp, char __user *buf,
 
 		/* Pairs with hypervisor wmb */
 		virt_rmb();
-		if (seq == le32_to_cpu(st->clk->seq_count))
-			break;
+		if (seq == le32_to_cpu(st->clk->seq_count)) {
+			/*
+			 * Either we updated fst->seq to seq (the latest version we observed)
+			 * or someone else did (old_seq == seq), so we can break.
+			 */
+			if (atomic_try_cmpxchg(&fst->seq, &old_seq, seq) ||
+			    old_seq == seq) {
+				break;
+			}
+		}
 
 		if (ktime_after(ktime_get(), deadline))
 			return -ETIMEDOUT;
@@ -413,10 +431,58 @@ static ssize_t vmclock_miscdev_read(struct file *fp, char __user *buf,
 	return count;
 }
 
+static __poll_t vmclock_miscdev_poll(struct file *fp, poll_table *wait)
+{
+	struct vmclock_file_state *fst = fp->private_data;
+	struct vmclock_state *st = fst->st;
+	uint32_t seq;
+
+	/*
+	 * Hypervisor will not send us any notifications, so fail immediately
+	 * to avoid having caller sleeping for ever.
+	 */
+	if (!(le64_to_cpu(st->clk->flags) & VMCLOCK_FLAG_NOTIFICATION_PRESENT))
+		return POLLHUP;
+
+	poll_wait(fp, &st->disrupt_wait, wait);
+
+	seq = le32_to_cpu(st->clk->seq_count);
+	if (atomic_read(&fst->seq) != seq)
+		return POLLIN | POLLRDNORM;
+
+	return 0;
+}
+
+static int vmclock_miscdev_open(struct inode *inode, struct file *fp)
+{
+	struct vmclock_state *st = container_of(fp->private_data,
+						struct vmclock_state, miscdev);
+	struct vmclock_file_state *fst = kzalloc(sizeof(*fst), GFP_KERNEL);
+
+	if (!fst)
+		return -ENOMEM;
+
+	fst->st = st;
+	atomic_set(&fst->seq, 0);
+
+	fp->private_data = fst;
+
+	return 0;
+}
+
+static int vmclock_miscdev_release(struct inode *inode, struct file *fp)
+{
+	kfree(fp->private_data);
+	return 0;
+}
+
 static const struct file_operations vmclock_miscdev_fops = {
 	.owner = THIS_MODULE,
+	.open = vmclock_miscdev_open,
+	.release = vmclock_miscdev_release,
 	.mmap = vmclock_miscdev_mmap,
 	.read = vmclock_miscdev_read,
+	.poll = vmclock_miscdev_poll,
 };
 
 /* module operations */
@@ -459,6 +525,44 @@ static acpi_status vmclock_acpi_resources(struct acpi_resource *ares, void *data
 	return AE_ERROR;
 }
 
+static void
+vmclock_acpi_notification_handler(acpi_handle __always_unused handle,
+				  u32 __always_unused event, void *dev)
+{
+	struct device *device = dev;
+	struct vmclock_state *st = device->driver_data;
+
+	wake_up_interruptible(&st->disrupt_wait);
+}
+
+static int vmclock_setup_notification(struct device *dev, struct vmclock_state *st)
+{
+	struct acpi_device *adev = ACPI_COMPANION(dev);
+	acpi_status status;
+
+	/*
+	 * This should never happen as this function is only called when
+	 * has_acpi_companion(dev) is true, but the logic is sufficiently
+	 * complex that Coverity can't see the tautology.
+	 */
+	if (!adev)
+		return -ENODEV;
+
+	/* The device does not support notifications. Nothing else to do */
+	if (!(le64_to_cpu(st->clk->flags) & VMCLOCK_FLAG_NOTIFICATION_PRESENT))
+		return 0;
+
+	status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
+					     vmclock_acpi_notification_handler,
+					     dev);
+	if (ACPI_FAILURE(status)) {
+		dev_err(dev, "failed to install notification handler");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+
 static int vmclock_probe_acpi(struct device *dev, struct vmclock_state *st)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
@@ -549,6 +653,11 @@ static int vmclock_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
+	init_waitqueue_head(&st->disrupt_wait);
+	ret = vmclock_setup_notification(dev, st);
+	if (ret)
+		return ret;
+
 	/*
 	 * If the structure is big enough, it can be mapped to userspace.
 	 * Theoretically a guest OS even using larger pages could still
@@ -581,6 +690,8 @@ static int vmclock_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
+	dev->driver_data = st;
+
 	dev_info(dev, "%s: registered %s%s%s\n", st->name,
 		 st->miscdev.minor ? "miscdev" : "",
 		 (st->miscdev.minor && st->ptp_clock) ? ", " : "",
diff --git a/include/uapi/linux/vmclock-abi.h b/include/uapi/linux/vmclock-abi.h
index 937fe00e4f33..d320623b0118 100644
--- a/include/uapi/linux/vmclock-abi.h
+++ b/include/uapi/linux/vmclock-abi.h
@@ -121,6 +121,11 @@ struct vmclock_abi {
 	 * loaded from some save state (restored from a snapshot).
 	 */
 #define VMCLOCK_FLAG_VM_GEN_COUNTER_PRESENT     (1 << 8)
+	/*
+	 * If the NOTIFICATION_PRESENT flag is set, the hypervisor will send
+	 * a notification every time it updates seq_count to a new even number.
+	 */
+#define VMCLOCK_FLAG_NOTIFICATION_PRESENT       (1 << 9)
 
 	__u8 pad[2];
 	__u8 clock_status;
-- 
2.47.3

[PATCH v6 3/7] dt-bindings: ptp: Add amazon,vmclock

[Takahiro Itazuri]

From: David Woodhouse <dwmw@amazon.co.uk>

The vmclock device provides a PTP clock source and precise timekeeping
across live migration and snapshot/restore operations.

The binding has a required memory region containing the vmclock_abi
structure and an optional interrupt for clock disruption notifications.

The full spec is at https://uapi-group.org/specifications/specs/vmclock/

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Babis Chalios <bchalios@amazon.es>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
---
 .../bindings/ptp/amazon,vmclock.yaml          | 46 +++++++++++++++++++
 MAINTAINERS                                   |  1 +
 2 files changed, 47 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ptp/amazon,vmclock.yaml

diff --git a/Documentation/devicetree/bindings/ptp/amazon,vmclock.yaml b/Documentation/devicetree/bindings/ptp/amazon,vmclock.yaml
new file mode 100644
index 000000000000..357790df876f
--- /dev/null
+++ b/Documentation/devicetree/bindings/ptp/amazon,vmclock.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ptp/amazon,vmclock.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Virtual Machine Clock
+
+maintainers:
+  - David Woodhouse <dwmw2@infradead.org>
+
+description:
+  The vmclock device provides a precise clock source and allows for
+  accurate timekeeping across live migration and snapshot/restore
+  operations. The full specification of the shared data structure is
+  available at https://uapi-group.org/specifications/specs/vmclock/
+
+properties:
+  compatible:
+    const: amazon,vmclock
+
+  reg:
+    description:
+      Specifies the shared memory region containing the vmclock_abi structure.
+    maxItems: 1
+
+  interrupts:
+    description:
+      Interrupt used to notify when the contents of the vmclock_abi structure
+      have been updated.
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    ptp@80000000 {
+      compatible = "amazon,vmclock";
+      reg = <0x80000000 0x1000>;
+      interrupts = <GIC_SPI 36 IRQ_TYPE_EDGE_RISING>;
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index da9dbc1a4019..0dd35070fe9b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21025,6 +21025,7 @@ PTP VMCLOCK SUPPORT
 M:	David Woodhouse <dwmw2@infradead.org>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	Documentation/devicetree/bindings/ptp/amazon,vmclock.yaml
 F:	drivers/ptp/ptp_vmclock.c
 F:	include/uapi/linux/vmclock-abi.h
 
-- 
2.47.3

[PATCH v6 4/7] ptp: ptp_vmclock: Add device tree support

[Takahiro Itazuri]

From: David Woodhouse <dwmw@amazon.co.uk>

Add device tree support to the ptp_vmclock driver, allowing it to probe
via device tree in addition to ACPI.

Handle optional interrupt for clock disruption notifications, mirroring
the ACPI notification behaviour.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Babis Chalios <bchalios@amazon.es>
---
 drivers/ptp/ptp_vmclock.c | 70 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 62 insertions(+), 8 deletions(-)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index 38b2bacb755e..8605e441fad7 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -14,10 +14,13 @@
 #include <linux/file.h>
 #include <linux/fs.h>
 #include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/miscdevice.h>
 #include <linux/mm.h>
 #include <linux/module.h>
+#include <linux/of.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 
@@ -535,7 +538,7 @@ vmclock_acpi_notification_handler(acpi_handle __always_unused handle,
 	wake_up_interruptible(&st->disrupt_wait);
 }
 
-static int vmclock_setup_notification(struct device *dev, struct vmclock_state *st)
+static int vmclock_setup_acpi_notification(struct device *dev)
 {
 	struct acpi_device *adev = ACPI_COMPANION(dev);
 	acpi_status status;
@@ -548,10 +551,6 @@ static int vmclock_setup_notification(struct device *dev, struct vmclock_state *
 	if (!adev)
 		return -ENODEV;
 
-	/* The device does not support notifications. Nothing else to do */
-	if (!(le64_to_cpu(st->clk->flags) & VMCLOCK_FLAG_NOTIFICATION_PRESENT))
-		return 0;
-
 	status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
 					     vmclock_acpi_notification_handler,
 					     dev);
@@ -586,6 +585,55 @@ static int vmclock_probe_acpi(struct device *dev, struct vmclock_state *st)
 	return 0;
 }
 
+static irqreturn_t vmclock_of_irq_handler(int __always_unused irq, void *_st)
+{
+	struct vmclock_state *st = _st;
+
+	wake_up_interruptible(&st->disrupt_wait);
+	return IRQ_HANDLED;
+}
+
+static int vmclock_probe_dt(struct device *dev, struct vmclock_state *st)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	struct resource *res;
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	if (!res)
+		return -ENODEV;
+
+	st->res = *res;
+
+	return 0;
+}
+
+static int vmclock_setup_of_notification(struct device *dev)
+{
+	struct platform_device *pdev = to_platform_device(dev);
+	int irq;
+
+	irq = platform_get_irq(pdev, 0);
+	if (irq < 0)
+		return irq;
+
+	return devm_request_irq(dev, irq, vmclock_of_irq_handler, IRQF_SHARED,
+				"vmclock", dev->driver_data);
+}
+
+static int vmclock_setup_notification(struct device *dev,
+				      struct vmclock_state *st)
+{
+	/* The device does not support notifications. Nothing else to do */
+	if (!(le64_to_cpu(st->clk->flags) & VMCLOCK_FLAG_NOTIFICATION_PRESENT))
+		return 0;
+
+	if (has_acpi_companion(dev)) {
+		return vmclock_setup_acpi_notification(dev);
+	} else {
+		return vmclock_setup_of_notification(dev);
+	}
+}
+
 static void vmclock_put_idx(void *data)
 {
 	struct vmclock_state *st = data;
@@ -606,7 +654,7 @@ static int vmclock_probe(struct platform_device *pdev)
 	if (has_acpi_companion(dev))
 		ret = vmclock_probe_acpi(dev, st);
 	else
-		ret = -EINVAL; /* Only ACPI for now */
+		ret = vmclock_probe_dt(dev, st);
 
 	if (ret) {
 		dev_info(dev, "Failed to obtain physical address: %d\n", ret);
@@ -654,6 +702,7 @@ static int vmclock_probe(struct platform_device *pdev)
 		return ret;
 
 	init_waitqueue_head(&st->disrupt_wait);
+	dev->driver_data = st;
 	ret = vmclock_setup_notification(dev, st);
 	if (ret)
 		return ret;
@@ -690,8 +739,6 @@ static int vmclock_probe(struct platform_device *pdev)
 		return -ENODEV;
 	}
 
-	dev->driver_data = st;
-
 	dev_info(dev, "%s: registered %s%s%s\n", st->name,
 		 st->miscdev.minor ? "miscdev" : "",
 		 (st->miscdev.minor && st->ptp_clock) ? ", " : "",
@@ -706,11 +753,18 @@ static const struct acpi_device_id vmclock_acpi_ids[] = {
 };
 MODULE_DEVICE_TABLE(acpi, vmclock_acpi_ids);
 
+static const struct of_device_id vmclock_of_ids[] = {
+	{ .compatible = "amazon,vmclock", },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, vmclock_of_ids);
+
 static struct platform_driver vmclock_platform_driver = {
 	.probe		= vmclock_probe,
 	.driver	= {
 		.name	= "vmclock",
 		.acpi_match_table = vmclock_acpi_ids,
+		.of_match_table = vmclock_of_ids,
 	},
 };
 
-- 
2.47.3

[PATCH v6 5/7] ptp: ptp_vmclock: add 'VMCLOCK' to ACPI device match

[Takahiro Itazuri]

From: David Woodhouse <dwmw@amazon.co.uk>

As we finalised the spec, we spotted that vmgenid actually says that the
_HID is supposed to be hypervisor-specific. Although in the 13 years
since the original vmgenid doc was published, nobody seems to have cared
about using _HID to distinguish between implementations on different
hypervisors, and we only ever use the _CID.

For consistency, match the _CID of "VMCLOCK" too.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Babis Chalios <bchalios@amazon.es>
---
 drivers/ptp/ptp_vmclock.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index 8605e441fad7..b5e264264051 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -749,6 +749,7 @@ static int vmclock_probe(struct platform_device *pdev)
 
 static const struct acpi_device_id vmclock_acpi_ids[] = {
 	{ "AMZNC10C", 0 },
+	{ "VMCLOCK", 0 },
 	{}
 };
 MODULE_DEVICE_TABLE(acpi, vmclock_acpi_ids);
-- 
2.47.3

[PATCH v6 6/7] ptp: ptp_vmclock: remove dependency on CONFIG_ACPI

[Takahiro Itazuri]

From: David Woodhouse <dwmw@amazon.co.uk>

Now that we added device tree support we can remove dependency on
CONFIG_ACPI.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Babis Chalios <bchalios@amazon.es>
---
 drivers/ptp/Kconfig       |  2 +-
 drivers/ptp/ptp_vmclock.c | 12 ++++++++----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/Kconfig b/drivers/ptp/Kconfig
index 5f8ea34d11d6..b93640ca08b7 100644
--- a/drivers/ptp/Kconfig
+++ b/drivers/ptp/Kconfig
@@ -134,7 +134,7 @@ config PTP_1588_CLOCK_KVM
 config PTP_1588_CLOCK_VMCLOCK
 	tristate "Virtual machine PTP clock"
 	depends on X86_TSC || ARM_ARCH_TIMER
-	depends on PTP_1588_CLOCK && ACPI && ARCH_SUPPORTS_INT128
+	depends on PTP_1588_CLOCK && ARCH_SUPPORTS_INT128
 	default PTP_1588_CLOCK_KVM
 	help
 	  This driver adds support for using a virtual precision clock
diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index b5e264264051..55ec3cda3995 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -501,6 +501,7 @@ static void vmclock_remove(void *data)
 		misc_deregister(&st->miscdev);
 }
 
+#if IS_ENABLED(CONFIG_ACPI)
 static acpi_status vmclock_acpi_resources(struct acpi_resource *ares, void *data)
 {
 	struct vmclock_state *st = data;
@@ -584,6 +585,7 @@ static int vmclock_probe_acpi(struct device *dev, struct vmclock_state *st)
 
 	return 0;
 }
+#endif /* CONFIG_ACPI */
 
 static irqreturn_t vmclock_of_irq_handler(int __always_unused irq, void *_st)
 {
@@ -627,11 +629,11 @@ static int vmclock_setup_notification(struct device *dev,
 	if (!(le64_to_cpu(st->clk->flags) & VMCLOCK_FLAG_NOTIFICATION_PRESENT))
 		return 0;
 
-	if (has_acpi_companion(dev)) {
+#if IS_ENABLED(CONFIG_ACPI)
+	if (has_acpi_companion(dev))
 		return vmclock_setup_acpi_notification(dev);
-	} else {
-		return vmclock_setup_of_notification(dev);
-	}
+#endif
+	return vmclock_setup_of_notification(dev);
 }
 
 static void vmclock_put_idx(void *data)
@@ -651,9 +653,11 @@ static int vmclock_probe(struct platform_device *pdev)
 	if (!st)
 		return -ENOMEM;
 
+#if IS_ENABLED(CONFIG_ACPI)
 	if (has_acpi_companion(dev))
 		ret = vmclock_probe_acpi(dev, st);
 	else
+#endif
 		ret = vmclock_probe_dt(dev, st);
 
 	if (ret) {
-- 
2.47.3

[PATCH v6 7/7] ptp: ptp_vmclock: return TAI not UTC

[Takahiro Itazuri]

From: David Woodhouse <dwmw@amazon.co.uk>

To output UTC would involve complex calculations about whether the time
elapsed since the reference time has crossed the end of the month when
a leap second takes effect. I've prototyped that, but it made me sad.

Much better to report TAI, which is what PHCs should do anyway.
And much much simpler.

Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Babis Chalios <bchalios@amazon.es>
---
 drivers/ptp/ptp_vmclock.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
index 55ec3cda3995..6cf63a86c997 100644
--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -83,13 +83,13 @@ static uint64_t mul_u64_u64_shr_add_u64(uint64_t *res_hi, uint64_t delta,
 
 static bool tai_adjust(struct vmclock_abi *clk, uint64_t *sec)
 {
-	if (likely(clk->time_type == VMCLOCK_TIME_UTC))
+	if (clk->time_type == VMCLOCK_TIME_TAI)
 		return true;
 
-	if (clk->time_type == VMCLOCK_TIME_TAI &&
+	if (clk->time_type == VMCLOCK_TIME_UTC &&
 	    (le64_to_cpu(clk->flags) & VMCLOCK_FLAG_TAI_OFFSET_VALID)) {
 		if (sec)
-			*sec += (int16_t)le16_to_cpu(clk->tai_offset_sec);
+			*sec -= (int16_t)le16_to_cpu(clk->tai_offset_sec);
 		return true;
 	}
 	return false;
@@ -350,9 +350,9 @@ static struct ptp_clock *vmclock_ptp_register(struct device *dev,
 		return NULL;
 	}
 
-	/* Only UTC, or TAI with offset */
+	/* Accept TAI directly, or UTC with valid offset for conversion to TAI */
 	if (!tai_adjust(st->clk, NULL)) {
-		dev_info(dev, "vmclock does not provide unambiguous UTC\n");
+		dev_info(dev, "vmclock does not provide unambiguous time\n");
 		return NULL;
 	}
 
-- 
2.47.3

Re: [v6,2/7] ptp: vmclock: support device notifications

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
ptp: vmclock: support device notifications

This patch adds optional device notification support to VMClock, allowing
the hypervisor to notify the driver when seq_count changes. It also adds
poll() support to propagate these notifications to userspace.

> diff --git a/drivers/ptp/ptp_vmclock.c b/drivers/ptp/ptp_vmclock.c
> index b3a83b03d9c1..38b2bacb755e 100644
> --- a/drivers/ptp/ptp_vmclock.c
> +++ b/drivers/ptp/ptp_vmclock.c

[ ... ]

> +static void
> +vmclock_acpi_notification_handler(acpi_handle __always_unused handle,
> +				  u32 __always_unused event, void *dev)
> +{
> +	struct device *device = dev;
> +	struct vmclock_state *st = device->driver_data;
> +
> +	wake_up_interruptible(&st->disrupt_wait);
> +}
> +
> +static int vmclock_setup_notification(struct device *dev, struct vmclock_state *st)
> +{

[ ... ]

> +	status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
> +					     vmclock_acpi_notification_handler,
> +					     dev);

Since acpi_install_notify_handler() is called here, should there be a
corresponding acpi_remove_notify_handler() in vmclock_remove()? Currently
the handler remains registered after device removal, which could lead to
a use-after-free if the hypervisor sends a notification after the driver
state has been freed.

[ ... ]

> @@ -549,6 +653,11 @@ static int vmclock_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>
> +	init_waitqueue_head(&st->disrupt_wait);
> +	ret = vmclock_setup_notification(dev, st);
> +	if (ret)
> +		return ret;

[ ... ]

> @@ -581,6 +690,8 @@ static int vmclock_probe(struct platform_device *pdev)
>  		return -ENODEV;
>  	}
>
> +	dev->driver_data = st;

Is there a race window here? The ACPI notification handler is installed
in vmclock_setup_notification() which runs earlier in vmclock_probe(),
but the handler accesses device->driver_data. If the hypervisor sends a
notification between vmclock_setup_notification() and this assignment,
vmclock_acpi_notification_handler() would dereference NULL or garbage.

I see this is fixed later in the series by commit 47fdd294385e which
moves dev->driver_data = st before vmclock_setup_notification().
-- 
pw-bot: cr

Re: [v6,2/7] ptp: vmclock: support device notifications

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 3813 bytes --]

On Thu, 2026-01-22 at 19:11 -0800, Jakub Kicinski wrote:
> 
> > +	status = acpi_install_notify_handler(adev->handle, ACPI_DEVICE_NOTIFY,
> > +					     vmclock_acpi_notification_handler,
> > +					     dev);
> 
> Since acpi_install_notify_handler() is called here, should there be a
> corresponding acpi_remove_notify_handler() in vmclock_remove()? Currently
> the handler remains registered after device removal, which could lead to
> a use-after-free if the hypervisor sends a notification after the driver
> state has been freed.
> 
> [ ... ]
> 
> > @@ -549,6 +653,11 @@ static int vmclock_probe(struct platform_device *pdev)
> >   	if (ret)
> >   		return ret;
> > 
> > +	init_waitqueue_head(&st->disrupt_wait);
> > +	ret = vmclock_setup_notification(dev, st);
> > +	if (ret)
> > +		return ret;
> 
> [ ... ]
> 
> > @@ -581,6 +690,8 @@ static int vmclock_probe(struct platform_device *pdev)
> >   		return -ENODEV;
> >   	}
> > 
> > +	dev->driver_data = st;
> 
> Is there a race window here? The ACPI notification handler is installed
> in vmclock_setup_notification() which runs earlier in vmclock_probe(),
> but the handler accesses device->driver_data. If the hypervisor sends a
> notification between vmclock_setup_notification() and this assignment,
> vmclock_acpi_notification_handler() would dereference NULL or garbage.
> 
> I see this is fixed later in the series by commit 47fdd294385e which
> moves dev->driver_data = st before vmclock_setup_notification().

With the incremental diff below, I've fixed both of these in
https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/vmclock

Takahiro and Babis have the test environment set up to actually
exercise the notifications, so I'll let one of them actually do the
testing and repost v7.

Takahiro, please remember to add your own 'Signed-off-by:' as you do.

Thanks!


--- a/drivers/ptp/ptp_vmclock.c
+++ b/drivers/ptp/ptp_vmclock.c
@@ -490,17 +490,6 @@ static const struct file_operations vmclock_miscdev_fops = {
 
 /* module operations */
 
-static void vmclock_remove(void *data)
-{
-	struct vmclock_state *st = data;
-
-	if (st->ptp_clock)
-		ptp_clock_unregister(st->ptp_clock);
-
-	if (st->miscdev.minor != MISC_DYNAMIC_MINOR)
-		misc_deregister(&st->miscdev);
-}
-
 #if IS_ENABLED(CONFIG_ACPI)
 static acpi_status vmclock_acpi_resources(struct acpi_resource *ares, void *data)
 {
@@ -636,6 +625,32 @@ static int vmclock_setup_notification(struct device *dev,
 	return vmclock_setup_of_notification(dev);
 }
 
+static void vmclock_remove(void *data)
+{
+	struct device *dev = data;
+	struct vmclock_state *st = dev->driver_data;
+
+	if (!st) {
+		dev_err(dev, "vmclock_remove() called with NULL driver_data");
+		return;
+	}
+
+#if IS_ENABLED(CONFIG_ACPI)
+	if (has_acpi_companion(dev))
+		acpi_remove_notify_handler(ACPI_COMPANION(dev)->handle,
+					   ACPI_DEVICE_NOTIFY,
+					   vmclock_acpi_notification_handler);
+#endif
+
+	if (st->ptp_clock)
+		ptp_clock_unregister(st->ptp_clock);
+
+	if (st->miscdev.minor != MISC_DYNAMIC_MINOR)
+		misc_deregister(&st->miscdev);
+
+	dev->driver_data = NULL;
+}
+
 static void vmclock_put_idx(void *data)
 {
 	struct vmclock_state *st = data;
@@ -701,12 +716,13 @@ static int vmclock_probe(struct platform_device *pdev)
 
 	st->miscdev.minor = MISC_DYNAMIC_MINOR;
 
-	ret = devm_add_action_or_reset(&pdev->dev, vmclock_remove, st);
+	init_waitqueue_head(&st->disrupt_wait);
+	dev->driver_data = st;
+
+	ret = devm_add_action_or_reset(&pdev->dev, vmclock_remove, dev);
 	if (ret)
 		return ret;
 
-	init_waitqueue_head(&st->disrupt_wait);
-	dev->driver_data = st;
 	ret = vmclock_setup_notification(dev, st);
 	if (ret)
 		return ret;



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]