2021-12-21 09:04:54

by Vincent Whitchurch

[permalink] [raw]
Subject: [PATCH] um: virtio_uml: allow probing from devicetree

Allow the virtio_uml device to be probed from the devicetree so that
sub-devices can be specified using the standard virtio bindings, for
example:

virtio@1 {
compatible = "virtio,uml";
socket-path = "i2c.sock";
virtio-device-id = <0x22>;

i2c-controller {
compatible = "virtio,device22";
#address-cells = <0x01>;
#size-cells = <0x00>;

light-sensor@01 {
compatible = "ti,opt3001";
reg = <0x01>;
};
};
};

Signed-off-by: Vincent Whitchurch <[email protected]>
---

Notes:
Requires the UML devicetree support I posted a couple of weeks ago:
https://lore.kernel.org/all/[email protected]/

arch/um/drivers/virtio_uml.c | 50 +++++++++++++++++++++++++++++++++---
1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/arch/um/drivers/virtio_uml.c b/arch/um/drivers/virtio_uml.c
index d51e445df797..3e4fa0f262d3 100644
--- a/arch/um/drivers/virtio_uml.c
+++ b/arch/um/drivers/virtio_uml.c
@@ -21,6 +21,7 @@
* Based on Virtio MMIO driver by Pawel Moll, copyright 2011-2014, ARM Ltd.
*/
#include <linux/module.h>
+#include <linux/of.h>
#include <linux/platform_device.h>
#include <linux/slab.h>
#include <linux/virtio.h>
@@ -49,6 +50,7 @@ struct virtio_uml_platform_data {
struct virtio_uml_device {
struct virtio_device vdev;
struct platform_device *pdev;
+ struct virtio_uml_platform_data *pdata;

spinlock_t sock_lock;
int sock, req_fd, irq;
@@ -149,7 +151,7 @@ static int vhost_user_recv(struct virtio_uml_device *vu_dev,
if (rc == -ECONNRESET && vu_dev->registered) {
struct virtio_uml_platform_data *pdata;

- pdata = vu_dev->pdev->dev.platform_data;
+ pdata = vu_dev->pdata;

virtio_break_device(&vu_dev->vdev);
schedule_work(&pdata->conn_broken_wk);
@@ -1113,21 +1115,63 @@ void virtio_uml_set_no_vq_suspend(struct virtio_device *vdev,
no_vq_suspend ? "dis" : "en");
}

+static void vu_of_conn_broken(struct work_struct *wk)
+{
+ /*
+ * We can't remove the device from the devicetree so the only thing we
+ * can do is warn.
+ */
+ WARN_ON(1);
+}
+
/* Platform device */

+static struct virtio_uml_platform_data *
+virtio_uml_create_pdata(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct virtio_uml_platform_data *pdata;
+ int ret;
+
+ if (!np)
+ return ERR_PTR(-EINVAL);
+
+ pdata = devm_kzalloc(&pdev->dev, sizeof(*pdata), GFP_KERNEL);
+ if (!pdata)
+ return ERR_PTR(-ENOMEM);
+
+ INIT_WORK(&pdata->conn_broken_wk, vu_of_conn_broken);
+ pdata->pdev = pdev;
+
+ ret = of_property_read_string(np, "socket-path", &pdata->socket_path);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = of_property_read_u32(np, "virtio-device-id",
+ &pdata->virtio_device_id);
+ if (ret)
+ return ERR_PTR(ret);
+
+ return pdata;
+}
+
static int virtio_uml_probe(struct platform_device *pdev)
{
struct virtio_uml_platform_data *pdata = pdev->dev.platform_data;
struct virtio_uml_device *vu_dev;
int rc;

- if (!pdata)
- return -EINVAL;
+ if (!pdata) {
+ pdata = virtio_uml_create_pdata(pdev);
+ if (IS_ERR(pdata))
+ return PTR_ERR(pdata);
+ }

vu_dev = kzalloc(sizeof(*vu_dev), GFP_KERNEL);
if (!vu_dev)
return -ENOMEM;

+ vu_dev->pdata = pdata;
vu_dev->vdev.dev.parent = &pdev->dev;
vu_dev->vdev.dev.release = virtio_uml_release_dev;
vu_dev->vdev.config = &virtio_uml_config_ops;
--
2.33.1



2021-12-21 20:48:53

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] um: virtio_uml: allow probing from devicetree

On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote:
> Allow the virtio_uml device to be probed from the devicetree so that
> sub-devices can be specified using the standard virtio bindings, for
> example:
>
> virtio@1 {
> compatible = "virtio,uml";
> socket-path = "i2c.sock";
> virtio-device-id = <0x22>;
>

Given this, maybe it should modify
Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually
add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess?

+Rob, because I'm not really into any of this.

Also, I'm not even sure we should/need to document the DT bits that are
basically only used for testing in the first place?

Code looks good to me.

johannes

2021-12-22 10:34:21

by Vincent Whitchurch

[permalink] [raw]
Subject: Re: [PATCH] um: virtio_uml: allow probing from devicetree

On Tue, Dec 21, 2021 at 09:48:26PM +0100, Johannes Berg wrote:
> On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote:
> > Allow the virtio_uml device to be probed from the devicetree so that
> > sub-devices can be specified using the standard virtio bindings, for
> > example:
> >
> > virtio@1 {
> > compatible = "virtio,uml";
> > socket-path = "i2c.sock";
> > virtio-device-id = <0x22>;
> >
>
> Given this, maybe it should modify
> Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually
> add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess?
>
> +Rob, because I'm not really into any of this.
>
> Also, I'm not even sure we should/need to document the DT bits that are
> basically only used for testing in the first place?

I wasn't sure either, but Rob was OK with not documenting some other
bindings which are only used for testing[0], so I assumed that that
applied here too:

[0] https://lore.kernel.org/all/[email protected]/

Also, DT bindings are supposed to be generic and based on what the
hardware has, but here we have no hardware and something very Linux and
UML-specific.

> Code looks good to me.

Thanks!

2021-12-22 11:11:46

by Anton Ivanov

[permalink] [raw]
Subject: Re: [PATCH] um: virtio_uml: allow probing from devicetree

On 22/12/2021 10:34, Vincent Whitchurch wrote:
> On Tue, Dec 21, 2021 at 09:48:26PM +0100, Johannes Berg wrote:
>> On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote:
>>> Allow the virtio_uml device to be probed from the devicetree so that
>>> sub-devices can be specified using the standard virtio bindings, for
>>> example:
>>>
>>> virtio@1 {
>>> compatible = "virtio,uml";
>>> socket-path = "i2c.sock";
>>> virtio-device-id = <0x22>;
>>>
>>
>> Given this, maybe it should modify
>> Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually
>> add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess?
>>
>> +Rob, because I'm not really into any of this.
>>
>> Also, I'm not even sure we should/need to document the DT bits that are
>> basically only used for testing in the first place?
>
> I wasn't sure either, but Rob was OK with not documenting some other
> bindings which are only used for testing[0], so I assumed that that
> applied here too:
>
> [0] https://lore.kernel.org/all/[email protected]/
>
> Also, DT bindings are supposed to be generic and based on what the
> hardware has, but here we have no hardware and something very Linux and
> UML-specific.

This will probably need to be added to the general UML documentation. I
will take care of that once the patches are in the tree.

Brgds,

>
>> Code looks good to me.
>
> Thanks!
>


--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

2021-12-22 19:41:20

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH] um: virtio_uml: allow probing from devicetree

----- Ursprüngliche Mail -----
> Von: "anton ivanov" <[email protected]>
>> Also, DT bindings are supposed to be generic and based on what the
>> hardware has, but here we have no hardware and something very Linux and
>> UML-specific.
>
> This will probably need to be added to the general UML documentation. I
> will take care of that once the patches are in the tree.

That would be great! Patch applied.

Thanks,
//richard

2022-01-04 10:45:51

by Anton Ivanov

[permalink] [raw]
Subject: [PATCH] um: Document dtb command line option

From: Anton Ivanov <[email protected]>

Add documentation for the dtb command line option and the
ability to load/parse device trees.

Signed-off-by: Anton Ivanov <[email protected]>
---
.../virt/uml/user_mode_linux_howto_v2.rst | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/Documentation/virt/uml/user_mode_linux_howto_v2.rst b/Documentation/virt/uml/user_mode_linux_howto_v2.rst
index 2cafd3c3c6cb..81986fe014d4 100644
--- a/Documentation/virt/uml/user_mode_linux_howto_v2.rst
+++ b/Documentation/virt/uml/user_mode_linux_howto_v2.rst
@@ -1189,6 +1189,26 @@ E.g. ``os_close_file()`` is just a wrapper around ``close()``
which ensures that the userspace function close does not clash
with similarly named function(s) in the kernel part.

+Using UML as a Test Platform
+============================
+
+UML is an excellent test platform for device driver development. As
+with most things UML, "some user assembly may be required". It is
+up to the user to build their emulation environment. UML at present
+provides only the kernel infrastructure.
+
+Part of this infrastructure is the ability to load and parse fdt
+device tree blobs as used in Arm or Open Firmware platforms. These
+are supplied as an optional extra argument to the kernel command
+line::
+
+ dtb=filename
+
+The device tree is loaded and parsed at boottime and is accessible by
+drivers which query it. At this moment in time this facility is
+intended solely for development purposes. UML's own devices do not
+query the device tree.
+
Security Considerations
-----------------------

--
2.30.2


2022-01-04 14:13:44

by Anton Ivanov

[permalink] [raw]
Subject: Re: [PATCH] um: virtio_uml: allow probing from devicetree



On 22/12/2021 10:34, Vincent Whitchurch wrote:
> On Tue, Dec 21, 2021 at 09:48:26PM +0100, Johannes Berg wrote:
>> On Tue, 2021-12-21 at 10:04 +0100, Vincent Whitchurch wrote:
>>> Allow the virtio_uml device to be probed from the devicetree so that
>>> sub-devices can be specified using the standard virtio bindings, for
>>> example:
>>>
>>> virtio@1 {
>>> compatible = "virtio,uml";
>>> socket-path = "i2c.sock";
>>> virtio-device-id = <0x22>;
>>>
>>
>> Given this, maybe it should modify
>> Documentation/devicetree/bindings/virtio/virtio-device.yaml? Or actually
>> add a new Documentation/devicetree/bindings/virtio/uml.yaml I guess?
>>
>> +Rob, because I'm not really into any of this.
>>
>> Also, I'm not even sure we should/need to document the DT bits that are
>> basically only used for testing in the first place?

If we start adding the UML devices themselves to the DT, we might as well add all of them.

In the doc patch have described the DT support as mostly for development at this point.

It can be a good alternative to the endless command line (especially for complex devices like f.e. l2tpv3).


>
> I wasn't sure either, but Rob was OK with not documenting some other
> bindings which are only used for testing[0], so I assumed that that
> applied here too:
>
> [0] https://lore.kernel.org/all/[email protected]/
>
> Also, DT bindings are supposed to be generic and based on what the
> hardware has, but here we have no hardware and something very Linux and
> UML-specific.
>
>> Code looks good to me.
>
> Thanks!
>

Brgds,

--
Anton R. Ivanov
Cambridgegreys Limited. Registered in England. Company Number 10273661
https://www.cambridgegreys.com/

2022-01-13 15:09:43

by Vincent Whitchurch

[permalink] [raw]
Subject: Re: [PATCH] um: Document dtb command line option

On Tue, Jan 04, 2022 at 11:44:57AM +0100, [email protected] wrote:
> From: Anton Ivanov <[email protected]>
>
> Add documentation for the dtb command line option and the
> ability to load/parse device trees.
>
> Signed-off-by: Anton Ivanov <[email protected]>

LGTM, thanks.

Reviewed-by: Vincent Whitchurch <[email protected]>