[PATCH v5 0/2] misc: pvpanic: introduce capability & event attribute

[PATCH v5 0/2] misc: pvpanic: introduce capability & event attribute

[zhenwei pi]

v4 -> v5:
Use sysfs_emit to export attribute value.
Only allow subset of capability to set on events.
Add more detailed infomation in document.

v3 -> v4:
Use event sysfs attribute instead of module parameter.
Use driver dev_groups instead of creating files by sysfs_* API.

v2 -> v3:
Seperate the function to 2 parts:
    1, use sysfs to expose device capability.
    2, add a module parameter to set limitation by user.

v1 -> v2:
Remove device info log, use module parameter to expose capability.

v1:
The guest sides determines pvpanic capability by RDPT, before kicking
host side, check the event is supported or not.

zhenwei pi (2):
  misc: pvpanic: introduce device capability
  misc: pvpanic: introduce module parameter 'events'

 .../ABI/testing/sysfs-bus-pci-devices-pvpanic | 14 +++++
 drivers/misc/pvpanic.c                        | 58 +++++++++++++++++--
 2 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic

-- 
2.25.1

[PATCH v5 1/2] misc: pvpanic: introduce device capability

[zhenwei pi]

According to pvpanic spec:
https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/specs/pvpanic.txt

The guest should determine pvpanic capability by RDPT, so initialize
capability during device probing. There is no need to register panic
notifier callback function if no events supported.

Before sending event to host side, check capability firstly.

Suggested by Greg KH, use sysfs to expose capability to user space,
also add new sysfs attribute in document.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 .../ABI/testing/sysfs-bus-pci-devices-pvpanic | 12 +++++++
 drivers/misc/pvpanic.c                        | 31 ++++++++++++++++---
 2 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
new file mode 100644
index 000000000000..79b7dc31cd55
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
@@ -0,0 +1,12 @@
+What:		/sys/devices/pci0000:00/*/QEMU0001:00/capability
+Date:		Jan 2021
+Contact:	zhenwei pi <pizhenwei@bytedance.com>
+Description:
+		Read-only attribute. Capabilities of pvpanic device which
+		are supported by QEMU.
+
+		Format: %x.
+
+		Detailed bit definition refers to section <Bit Definition>
+		from pvpanic device specification:
+		https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/specs/pvpanic.txt
diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 951b37da5e3c..7e5254e8af1f 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -19,6 +19,20 @@
 #include <uapi/misc/pvpanic.h>
 
 static void __iomem *base;
+static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED;
+
+static ssize_t capability_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%x", capability);
+}
+static DEVICE_ATTR_RO(capability);
+
+static struct attribute *pvpanic_dev_attrs[] = {
+	&dev_attr_capability.attr,
+	NULL
+};
+ATTRIBUTE_GROUPS(pvpanic_dev);
 
 MODULE_AUTHOR("Hu Tao <hutao@cn.fujitsu.com>");
 MODULE_DESCRIPTION("pvpanic device driver");
@@ -27,7 +41,8 @@ MODULE_LICENSE("GPL");
 static void
 pvpanic_send_event(unsigned int event)
 {
-	iowrite8(event, base);
+	if (event & capability)
+		iowrite8(event, base);
 }
 
 static int
@@ -62,8 +77,12 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
 	if (IS_ERR(base))
 		return PTR_ERR(base);
 
-	atomic_notifier_chain_register(&panic_notifier_list,
-				       &pvpanic_panic_nb);
+	/* initlize capability by RDPT */
+	capability &= ioread8(base);
+
+	if (capability)
+		atomic_notifier_chain_register(&panic_notifier_list,
+					       &pvpanic_panic_nb);
 
 	return 0;
 }
@@ -71,8 +90,9 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
 static int pvpanic_mmio_remove(struct platform_device *pdev)
 {
 
-	atomic_notifier_chain_unregister(&panic_notifier_list,
-					 &pvpanic_panic_nb);
+	if (capability)
+		atomic_notifier_chain_unregister(&panic_notifier_list,
+						 &pvpanic_panic_nb);
 
 	return 0;
 }
@@ -93,6 +113,7 @@ static struct platform_driver pvpanic_mmio_driver = {
 		.name = "pvpanic-mmio",
 		.of_match_table = pvpanic_mmio_match,
 		.acpi_match_table = pvpanic_device_ids,
+		.dev_groups = pvpanic_dev_groups,
 	},
 	.probe = pvpanic_mmio_probe,
 	.remove = pvpanic_mmio_remove,
-- 
2.25.1

[PATCH v5 2/2] misc: pvpanic: introduce events device attribue

[zhenwei pi]

Suggested by Paolo & Greg, add 'events' device attribute that can be
used to limit which capabilities the driver uses.

Finally, the pvpanic guest driver works by the limitation of both
device capability and user setting.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
---
 .../ABI/testing/sysfs-bus-pci-devices-pvpanic | 12 ++++++++
 drivers/misc/pvpanic.c                        | 30 ++++++++++++++++++-
 2 files changed, 41 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
index 79b7dc31cd55..1936f7324155 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
@@ -10,3 +10,15 @@ Description:
 		Detailed bit definition refers to section <Bit Definition>
 		from pvpanic device specification:
 		https://git.qemu.org/?p=qemu.git;a=blob_plain;f=docs/specs/pvpanic.txt
+
+What:		/sys/devices/pci0000:00/*/QEMU0001:00/events
+Date:		Jan 2021
+Contact:	zhenwei pi <pizhenwei@bytedance.com>
+Description:
+		RW attribute. Set/get which features in-use. This attribute
+		is used to enable/disable feature(s) of pvpanic device.
+		Notice that this value should be a subset of capability.
+
+		Format: %x.
+
+		Also refer to pvpanic device specification.
diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 7e5254e8af1f..a9ac26c7b11f 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -20,6 +20,7 @@
 
 static void __iomem *base;
 static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED;
+static unsigned int events;
 
 static ssize_t capability_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
@@ -28,8 +29,34 @@ static ssize_t capability_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(capability);
 
+static ssize_t events_show(struct device *dev,  struct device_attribute *attr, char *buf)
+{
+	return sysfs_emit(buf, "%x", events);
+}
+
+static ssize_t events_store(struct device *dev,  struct device_attribute *attr,
+			    const char *buf, size_t count)
+{
+	unsigned int tmp;
+	int err;
+
+	err = kstrtouint(buf, 16, &tmp);
+	if (err)
+		return err;
+
+	if ((tmp & capability) != tmp)
+		return -EINVAL;
+
+	events = tmp;
+
+	return count;
+
+}
+static DEVICE_ATTR_RW(events);
+
 static struct attribute *pvpanic_dev_attrs[] = {
 	&dev_attr_capability.attr,
+	&dev_attr_events.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(pvpanic_dev);
@@ -41,7 +68,7 @@ MODULE_LICENSE("GPL");
 static void
 pvpanic_send_event(unsigned int event)
 {
-	if (event & capability)
+	if (event & capability & events)
 		iowrite8(event, base);
 }
 
@@ -79,6 +106,7 @@ static int pvpanic_mmio_probe(struct platform_device *pdev)
 
 	/* initlize capability by RDPT */
 	capability &= ioread8(base);
+	events = capability;
 
 	if (capability)
 		atomic_notifier_chain_register(&panic_notifier_list,
-- 
2.25.1

PING: [PATCH v5 0/2] misc: pvpanic: introduce capability & event attribute

[zhenwei pi]

Hi, Greg

What's the next step I should take?

On 1/10/21 7:53 PM, zhenwei pi wrote:
> v4 -> v5:
> Use sysfs_emit to export attribute value.
> Only allow subset of capability to set on events.
> Add more detailed infomation in document.
> 
> v3 -> v4:
> Use event sysfs attribute instead of module parameter.
> Use driver dev_groups instead of creating files by sysfs_* API.
> 
> v2 -> v3:
> Seperate the function to 2 parts:
>      1, use sysfs to expose device capability.
>      2, add a module parameter to set limitation by user.
> 
> v1 -> v2:
> Remove device info log, use module parameter to expose capability.
> 
> v1:
> The guest sides determines pvpanic capability by RDPT, before kicking
> host side, check the event is supported or not.
> 
> zhenwei pi (2):
>    misc: pvpanic: introduce device capability
>    misc: pvpanic: introduce module parameter 'events'
> 
>   .../ABI/testing/sysfs-bus-pci-devices-pvpanic | 14 +++++
>   drivers/misc/pvpanic.c                        | 58 +++++++++++++++++--
>   2 files changed, 67 insertions(+), 5 deletions(-)
>   create mode 100644 Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
> 

-- 
zhenwei pi

Re: PING: [PATCH v5 0/2] misc: pvpanic: introduce capability & event attribute

[Greg KH]

On Mon, Jan 18, 2021 at 09:42:57AM +0800, zhenwei pi wrote:
> Hi, Greg
> 
> What's the next step I should take?

I need some reviews by the people who will be doing the qemu portion of
this, or someone else, please.

thanks,

greg k-h

Re: Re: PING: [PATCH v5 0/2] misc: pvpanic: introduce capability & event attribute

[zhenwei pi]

On 1/21/21 1:53 AM, Greg KH wrote:
> X-Gm-Spam: 0
> X-Gm-Phishy: 0
> 
> On Mon, Jan 18, 2021 at 09:42:57AM +0800, zhenwei pi wrote:
>> Hi, Greg
>>
>> What's the next step I should take?
> 
> I need some reviews by the people who will be doing the qemu portion of
> this, or someone else, please.
> 
> thanks,
> 
> greg k-h
> 

Paolo has already fixed QEMU side compat stuff.
https://github.com/qemu/qemu/commit/b1b0393c3c58c0e96c7c44e2e98baa252d6c6813

And I realized that guest side also needs to test capability, so I 
always CC Paolo in this patch series.

-- 
zhenwei pi

Re: PING: [PATCH v5 0/2] misc: pvpanic: introduce capability & event attribute

[Paolo Bonzini]

On 20/01/21 18:53, Greg KH wrote:
> On Mon, Jan 18, 2021 at 09:42:57AM +0800, zhenwei pi wrote:
>> Hi, Greg
>>
>> What's the next step I should take?
> 
> I need some reviews by the people who will be doing the qemu portion of
> this, or someone else, please.

QEMU bits are already part of the last release, so all good from our 
side.  As far as the "hardware" spec is concerned,

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo