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

[Greg KH <gregkh@linuxfoundation.org>]

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