[RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[sjur.brandeland]

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Add information so rproc-serial can be easily recogniced
from user space. Add the following information to uevent:
DRIVER=virtio_console|virtio_rproc_serial
INTERFACE=grand-parent/parent/name

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
Hi,

I need some way to identify the major/minor number for
the rproc-serial device, given the udev event. 
Review comments are welcomed.

Thanks,
Sjur

 drivers/char/virtio_console.c |   22 ++++++++++++++++++++++
 1 files changed, 22 insertions(+), 0 deletions(-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index 031be0b..96c5ed9 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2190,6 +2190,27 @@ static struct virtio_driver virtio_rproc_serial = {
 	.remove =	virtcons_remove,
 };
 
+int class_virtio_ports_uevent(struct device *_dev, struct kobj_uevent_env *env)
+{
+	struct port *port = dev_get_drvdata(_dev);
+	struct device *dev;
+	int err;
+
+	if (!port || !port->portdev || !port->portdev->vdev)
+		return 0;
+
+	dev = &port->portdev->vdev->dev;
+	err = add_uevent_var(env, "DRIVER=%s", dev->driver->name);
+	if (err)
+		return err;
+
+	return add_uevent_var(env, "INTERFACE=%s/%s/%s",
+			      dev->parent->parent ?
+					dev_name(dev->parent->parent) : "",
+			      dev->parent ? dev_name(dev->parent) : "",
+			      dev_name(dev));
+}
+
 static int __init init(void)
 {
 	int err;
@@ -2201,6 +2222,7 @@ static int __init init(void)
 		return err;
 	}
 
+	pdrvdata.class->dev_uevent = class_virtio_ports_uevent;
 	pdrvdata.debugfs_dir = debugfs_create_dir("virtio-ports", NULL);
 	if (!pdrvdata.debugfs_dir) {
 		pr_warning("Error %ld creating debugfs dir for virtio-ports\n",
-- 
1.7.5.4

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[Rusty Russell]

sjur.brandeland@stericsson.com writes:

> From: Sjur Brændeland <sjur.brandeland@stericsson.com>
>
> Add information so rproc-serial can be easily recogniced
> from user space. Add the following information to uevent:
> DRIVER=virtio_console|virtio_rproc_serial
> INTERFACE=grand-parent/parent/name
>
> Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
> ---
> Hi,
>
> I need some way to identify the major/minor number for
> the rproc-serial device, given the udev event. 
> Review comments are welcomed.
>
> Thanks,
> Sjur

Hmm, I send all uevent questions to Greg KH (CC'd).

Thanks,
Rusty.

>  drivers/char/virtio_console.c |   22 ++++++++++++++++++++++
>  1 files changed, 22 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> index 031be0b..96c5ed9 100644
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -2190,6 +2190,27 @@ static struct virtio_driver virtio_rproc_serial = {
>  	.remove =	virtcons_remove,
>  };
>  
> +int class_virtio_ports_uevent(struct device *_dev, struct kobj_uevent_env *env)
> +{
> +	struct port *port = dev_get_drvdata(_dev);
> +	struct device *dev;
> +	int err;
> +
> +	if (!port || !port->portdev || !port->portdev->vdev)
> +		return 0;
> +
> +	dev = &port->portdev->vdev->dev;
> +	err = add_uevent_var(env, "DRIVER=%s", dev->driver->name);
> +	if (err)
> +		return err;
> +
> +	return add_uevent_var(env, "INTERFACE=%s/%s/%s",
> +			      dev->parent->parent ?
> +					dev_name(dev->parent->parent) : "",
> +			      dev->parent ? dev_name(dev->parent) : "",
> +			      dev_name(dev));
> +}
> +
>  static int __init init(void)
>  {
>  	int err;
> @@ -2201,6 +2222,7 @@ static int __init init(void)
>  		return err;
>  	}
>  
> +	pdrvdata.class->dev_uevent = class_virtio_ports_uevent;
>  	pdrvdata.debugfs_dir = debugfs_create_dir("virtio-ports", NULL);
>  	if (!pdrvdata.debugfs_dir) {
>  		pr_warning("Error %ld creating debugfs dir for virtio-ports\n",
> -- 
> 1.7.5.4
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[Greg KH]

On Tue, Jan 22, 2013 at 09:55:20AM +1030, Rusty Russell wrote:
> sjur.brandeland@stericsson.com writes:
> 
> > From: Sjur Brændeland <sjur.brandeland@stericsson.com>
> >
> > Add information so rproc-serial can be easily recogniced
> > from user space. Add the following information to uevent:
> > DRIVER=virtio_console|virtio_rproc_serial
> > INTERFACE=grand-parent/parent/name
> >
> > Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
> > ---
> > Hi,
> >
> > I need some way to identify the major/minor number for
> > the rproc-serial device, given the udev event. 
> > Review comments are welcomed.
> >
> > Thanks,
> > Sjur
> 
> Hmm, I send all uevent questions to Greg KH (CC'd).

The "INTERFACE" uevent variable is only for USB devices, so for anything
else to be sending userspace that variable, would confuse things
greatly.

Sjur, what exactly are you trying to do here?  What do you want
userspace to know, and what should it do with that information?

thanks,

greg k-h

Re: [RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[Sjur Brændeland]

Hi Greg,

On Tue, Jan 22, 2013 at 5:16 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Tue, Jan 22, 2013 at 09:55:20AM +1030, Rusty Russell wrote:
>> sjur.brandeland@stericsson.com writes:
>>
>> > From: Sjur Brændeland <sjur.brandeland@stericsson.com>
>> >
>> > Add information so rproc-serial can be easily recogniced
>> > from user space. Add the following information to uevent:
>> > DRIVER=virtio_console|virtio_rproc_serial
>> > INTERFACE=grand-parent/parent/name
>> >
>> > Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
>> > ---
>> > Hi,
>> >
>> > I need some way to identify the major/minor number for
>> > the rproc-serial device, given the udev event.
>> > Review comments are welcomed.
>> >
>> > Thanks,
>> > Sjur
>>
>> Hmm, I send all uevent questions to Greg KH (CC'd).
>
> The "INTERFACE" uevent variable is only for USB devices, so for anything
> else to be sending userspace that variable, would confuse things
> greatly.
>
> Sjur, what exactly are you trying to do here?  What do you want
> userspace to know, and what should it do with that information?

I'm trying to help user-space make sense of the port-name generated
by virtio_console. I'm using remoteproc and virtio_console for talking
to a remote device (modem). At startup the port-device will be named
vport<X>p0. If we get a reboot of the remote device, the device name
will change to vport<Y>p0. It is difficult for applications to guess
the X or Y values in order to get hold of the right file after a
reboot.

If we at some point get more than one remote processor using
virtio_console the port name may not even be deterministic.
(X is currently just a monotonic counter).

The current device path is:
/sys/devices/platform/ste-modem/remoteproc0/virtio0/virtio-ports/vport0p0

$cat uevent
MAJOR=254
MINOR=0
DEVNAME=vport0p0

So what I'm trying to do is to add information to udev events in order
to be able create good device names and be able identify the remoteproc
device. I should probably also mention that we're currently running this on
Android.

The initial stab at this was to add information to the udev event.

Another approach could perhaps be to change the way port-names are
generated by virtio_console...


Thanks,
Sjur
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[Greg KH]

On Thu, Jan 24, 2013 at 02:30:59PM +0100, Sjur Brændeland wrote:
> Hi Greg,
> 
> On Tue, Jan 22, 2013 at 5:16 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> > On Tue, Jan 22, 2013 at 09:55:20AM +1030, Rusty Russell wrote:
> >> sjur.brandeland@stericsson.com writes:
> >>
> >> > From: Sjur Brændeland <sjur.brandeland@stericsson.com>
> >> >
> >> > Add information so rproc-serial can be easily recogniced
> >> > from user space. Add the following information to uevent:
> >> > DRIVER=virtio_console|virtio_rproc_serial
> >> > INTERFACE=grand-parent/parent/name
> >> >
> >> > Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
> >> > ---
> >> > Hi,
> >> >
> >> > I need some way to identify the major/minor number for
> >> > the rproc-serial device, given the udev event.
> >> > Review comments are welcomed.
> >> >
> >> > Thanks,
> >> > Sjur
> >>
> >> Hmm, I send all uevent questions to Greg KH (CC'd).
> >
> > The "INTERFACE" uevent variable is only for USB devices, so for anything
> > else to be sending userspace that variable, would confuse things
> > greatly.
> >
> > Sjur, what exactly are you trying to do here?  What do you want
> > userspace to know, and what should it do with that information?
> 
> I'm trying to help user-space make sense of the port-name generated
> by virtio_console. I'm using remoteproc and virtio_console for talking
> to a remote device (modem). At startup the port-device will be named
> vport<X>p0. If we get a reboot of the remote device, the device name
> will change to vport<Y>p0. It is difficult for applications to guess
> the X or Y values in order to get hold of the right file after a
> reboot.
> 
> If we at some point get more than one remote processor using
> virtio_console the port name may not even be deterministic.
> (X is currently just a monotonic counter).
> 
> The current device path is:
> /sys/devices/platform/ste-modem/remoteproc0/virtio0/virtio-ports/vport0p0
> 
> $cat uevent
> MAJOR=254
> MINOR=0
> DEVNAME=vport0p0
> 
> So what I'm trying to do is to add information to udev events in order
> to be able create good device names and be able identify the remoteproc
> device. I should probably also mention that we're currently running this on
> Android.

Ok, but why would you use the "INTERFACE" uevent field for something
like this?  All that looked to do was to export the path to your device,
which userspace already gets, right?

What were you thinking that you could send to userspace through a uevent
that would allow it to uniquely identify your device, that it already
doesn't have by just looking in the sysfs files for your device?

> The initial stab at this was to add information to the udev event.

Android doesn't use udev :)

> Another approach could perhaps be to change the way port-names are
> generated by virtio_console...

How would that help here?

The way to solve this is to provide userspace with some "unique"
identifier of your device.  Traditionally that is done with a number of
different methods:
	- serial numbers and other unique strings in sysfs files
	- device paths (plugged into the 3rd port on the 4th USB device)
	- a "label" in the device (think about filesystems)

All of these are either determined by sysfs files owned by the device
(and hence, no uevent changes are needed), or userspace can query the
device (filesystem labels).

Do you have anything that can help uniquely identify this device?  If
not, can you make something?

Hope this helps,

greg k-h

Re: [RFC] virtio_console: Add DRIVER and INTERFACE to uevent.

[Amit Shah]

On (Thu) 24 Jan 2013 [08:51:39], Greg KH wrote:
> On Thu, Jan 24, 2013 at 02:30:59PM +0100, Sjur Brændeland wrote:

> > Another approach could perhaps be to change the way port-names are
> > generated by virtio_console...
> 
> How would that help here?
> 
> The way to solve this is to provide userspace with some "unique"
> identifier of your device.  Traditionally that is done with a number of
> different methods:
> 	- serial numbers and other unique strings in sysfs files
> 	- device paths (plugged into the 3rd port on the 4th USB device)
> 	- a "label" in the device (think about filesystems)

virtio-console does have a concept of port 'names'.  For example,
the QEMU guest agent knows it just has to open ports available with
the /dev/virtio-ports/org.qemu.guest-agent* names.  Sjur, if you add
minimal support for multiport, and use the VIRTIO_CONSOLE_PORT_NAME
feature, this should be already supported by udev.

		Amit