All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/2] misc: pvpanic: introduce capability & event attribute
@ 2021-01-10  5:37 zhenwei pi
  2021-01-10  5:37 ` [PATCH v4 1/2] misc: pvpanic: introduce device capability zhenwei pi
  2021-01-10  5:37 ` [PATCH v4 2/2] misc: pvpanic: introduce events device attribue zhenwei pi
  0 siblings, 2 replies; 5+ messages in thread
From: zhenwei pi @ 2021-01-10  5:37 UTC (permalink / raw)
  To: arnd, gregkh; +Cc: pbonzini, linux-kernel, pizhenwei

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


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v4 1/2] misc: pvpanic: introduce device capability
  2021-01-10  5:37 [PATCH v4 0/2] misc: pvpanic: introduce capability & event attribute zhenwei pi
@ 2021-01-10  5:37 ` zhenwei pi
  2021-01-10  8:39   ` Greg KH
  2021-01-10  5:37 ` [PATCH v4 2/2] misc: pvpanic: introduce events device attribue zhenwei pi
  1 sibling, 1 reply; 5+ messages in thread
From: zhenwei pi @ 2021-01-10  5:37 UTC (permalink / raw)
  To: arnd, gregkh; +Cc: pbonzini, linux-kernel, pizhenwei

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 |  7 ++++
 drivers/misc/pvpanic.c                        | 33 ++++++++++++++++---
 2 files changed, 35 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..57d014a2c339
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
@@ -0,0 +1,7 @@
+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: %s.
diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index 951b37da5e3c..c2f6a9e866b3 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -19,6 +19,22 @@
 #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 sprintf(buf, "%s%s",
+		capability & PVPANIC_PANICKED ? "PANICKED[BIT 0]\n" : "",
+		capability & PVPANIC_CRASH_LOADED ? "CRASH_LOADED[BIT 1]\n" : "");
+}
+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 +43,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 +79,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 +92,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 +115,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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v4 2/2] misc: pvpanic: introduce events device attribue
  2021-01-10  5:37 [PATCH v4 0/2] misc: pvpanic: introduce capability & event attribute zhenwei pi
  2021-01-10  5:37 ` [PATCH v4 1/2] misc: pvpanic: introduce device capability zhenwei pi
@ 2021-01-10  5:37 ` zhenwei pi
  2021-01-10  8:41   ` Greg KH
  1 sibling, 1 reply; 5+ messages in thread
From: zhenwei pi @ 2021-01-10  5:37 UTC (permalink / raw)
  To: arnd, gregkh; +Cc: pbonzini, linux-kernel, pizhenwei

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 |  7 +++++
 drivers/misc/pvpanic.c                        | 26 ++++++++++++++++++-
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
index 57d014a2c339..4750cfa0af2b 100644
--- a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
+++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
@@ -5,3 +5,10 @@ Description:
 		Read-only attribute. Capabilities of pvpanic device
 		which are supported by QEMU.
 		Format: %s.
+
+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.
+               Format: %x.
diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
index c2f6a9e866b3..07a008e15bd2 100644
--- a/drivers/misc/pvpanic.c
+++ b/drivers/misc/pvpanic.c
@@ -19,8 +19,31 @@
 #include <uapi/misc/pvpanic.h>
 
 static void __iomem *base;
+static unsigned int events = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED;
 static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED;
 
+static ssize_t events_show(struct device *dev,  struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "%x\n", 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;
+
+	events = tmp;
+
+	return count;
+
+}
+static DEVICE_ATTR_RW(events);
+
 static ssize_t capability_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
@@ -32,6 +55,7 @@ static DEVICE_ATTR_RO(capability);
 
 static struct attribute *pvpanic_dev_attrs[] = {
 	&dev_attr_capability.attr,
+	&dev_attr_events.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(pvpanic_dev);
@@ -43,7 +67,7 @@ MODULE_LICENSE("GPL");
 static void
 pvpanic_send_event(unsigned int event)
 {
-	if (event & capability)
+	if (event & capability & events)
 		iowrite8(event, base);
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 1/2] misc: pvpanic: introduce device capability
  2021-01-10  5:37 ` [PATCH v4 1/2] misc: pvpanic: introduce device capability zhenwei pi
@ 2021-01-10  8:39   ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-01-10  8:39 UTC (permalink / raw)
  To: zhenwei pi; +Cc: arnd, pbonzini, linux-kernel

On Sun, Jan 10, 2021 at 01:37:18PM +0800, zhenwei pi wrote:
> 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 |  7 ++++
>  drivers/misc/pvpanic.c                        | 33 ++++++++++++++++---
>  2 files changed, 35 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..57d014a2c339
> --- /dev/null
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
> @@ -0,0 +1,7 @@
> +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: %s.

Again, you are not saying exactly what the %s is, shouldn't you?  And
this does NOT match with the code below :(

> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index 951b37da5e3c..c2f6a9e866b3 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -19,6 +19,22 @@
>  #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 sprintf(buf, "%s%s",
> +		capability & PVPANIC_PANICKED ? "PANICKED[BIT 0]\n" : "",
> +		capability & PVPANIC_CRASH_LOADED ? "CRASH_LOADED[BIT 1]\n" : "");

Why do you have "BIT X" in here?  Why would userspace care?

The rule for sysfs is "one value per file".  You just printed out
multiple lines.  Not good, and totally not allowed.

Also please use sysfs_emit().

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v4 2/2] misc: pvpanic: introduce events device attribue
  2021-01-10  5:37 ` [PATCH v4 2/2] misc: pvpanic: introduce events device attribue zhenwei pi
@ 2021-01-10  8:41   ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2021-01-10  8:41 UTC (permalink / raw)
  To: zhenwei pi; +Cc: arnd, pbonzini, linux-kernel

On Sun, Jan 10, 2021 at 01:37:19PM +0800, zhenwei pi wrote:
> 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 |  7 +++++
>  drivers/misc/pvpanic.c                        | 26 ++++++++++++++++++-
>  2 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
> index 57d014a2c339..4750cfa0af2b 100644
> --- a/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
> +++ b/Documentation/ABI/testing/sysfs-bus-pci-devices-pvpanic
> @@ -5,3 +5,10 @@ Description:
>  		Read-only attribute. Capabilities of pvpanic device
>  		which are supported by QEMU.
>  		Format: %s.
> +
> +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.
> +               Format: %x.

Please describe the allowed values.

> diff --git a/drivers/misc/pvpanic.c b/drivers/misc/pvpanic.c
> index c2f6a9e866b3..07a008e15bd2 100644
> --- a/drivers/misc/pvpanic.c
> +++ b/drivers/misc/pvpanic.c
> @@ -19,8 +19,31 @@
>  #include <uapi/misc/pvpanic.h>
>  
>  static void __iomem *base;
> +static unsigned int events = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED;
>  static unsigned int capability = PVPANIC_PANICKED | PVPANIC_CRASH_LOADED;
>  
> +static ssize_t events_show(struct device *dev,  struct device_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "%x\n", 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;
> +
> +	events = tmp;
> +
> +	return count;
> +
> +}
> +static DEVICE_ATTR_RW(events);
> +
>  static ssize_t capability_show(struct device *dev,
>  			       struct device_attribute *attr, char *buf)
>  {
> @@ -32,6 +55,7 @@ static DEVICE_ATTR_RO(capability);
>  
>  static struct attribute *pvpanic_dev_attrs[] = {
>  	&dev_attr_capability.attr,
> +	&dev_attr_events.attr,
>  	NULL
>  };
>  ATTRIBUTE_GROUPS(pvpanic_dev);
> @@ -43,7 +67,7 @@ MODULE_LICENSE("GPL");
>  static void
>  pvpanic_send_event(unsigned int event)
>  {
> -	if (event & capability)
> +	if (event & capability & events)

That's just going to be so crazy to try to figure out, I'm glad I'm not
a user trying to configure this.

User apis are hard.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2021-01-10  8:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-10  5:37 [PATCH v4 0/2] misc: pvpanic: introduce capability & event attribute zhenwei pi
2021-01-10  5:37 ` [PATCH v4 1/2] misc: pvpanic: introduce device capability zhenwei pi
2021-01-10  8:39   ` Greg KH
2021-01-10  5:37 ` [PATCH v4 2/2] misc: pvpanic: introduce events device attribue zhenwei pi
2021-01-10  8:41   ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.