Xen-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id
@ 2023-04-05  0:12 Viresh Kumar
  2023-04-05  0:12 ` [PATCH V2 2/2] libxl: fix matching of generic virtio device Viresh Kumar
  2023-04-05  8:36 ` [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Oleksandr Tyshchenko
  0 siblings, 2 replies; 7+ messages in thread
From: Viresh Kumar @ 2023-04-05  0:12 UTC (permalink / raw
  To: xen-devel, Juergen Gross, Julien Grall, Anthony PERARD
  Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
	Mathieu Poirier, Oleksandr Tyshchenko, Erik Schilling

For generic virtio devices, where we don't need to add compatible or
other special DT properties, the type field is set to "virtio,device".

But this misses the case where the user sets the type with a valid
virtio device id as well, like "virtio,device26" for file system device.

Update documentation to support that as well.

Fixes: dd54ea500be8 ("docs: add documentation for generic virtio devices")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1->V2: New patch.

 docs/man/xl.cfg.5.pod.in | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 10f37990be57..ea20eac0ba32 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1608,8 +1608,9 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is
 
 L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml>
 
-For generic virtio devices, where we don't need to set special or compatible
-properties in the Device Tree, the type field must be set to "virtio,device".
+For other generic virtio devices, where we don't need to set special or
+compatible properties in the Device Tree, the type field must be set to
+"virtio,device" or "virtio,device<N>", where "N" is the virtio device id.
 
 =item B<transport=STRING>
 
-- 
2.31.1.272.g89b43f80a514



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

* [PATCH V2 2/2] libxl: fix matching of generic virtio device
  2023-04-05  0:12 [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Viresh Kumar
@ 2023-04-05  0:12 ` Viresh Kumar
  2023-04-05  9:11   ` Oleksandr Tyshchenko
  2023-04-05  8:36 ` [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Oleksandr Tyshchenko
  1 sibling, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2023-04-05  0:12 UTC (permalink / raw
  To: xen-devel, Juergen Gross, Julien Grall, Anthony PERARD
  Cc: Viresh Kumar, Vincent Guittot, stratos-dev, Alex Bennée,
	Mathieu Poirier, Oleksandr Tyshchenko, Erik Schilling

The strings won't be an exact match, as we are only looking to match the
prefix here, i.e. "virtio,device". This is already done properly in
libxl_virtio.c file, lets do the same here too.

Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
---
V1->V2: Add the missing fixes tag.

 tools/libs/light/libxl_arm.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
index ddc7b2a15975..97c80d7ed0fa 100644
--- a/tools/libs/light/libxl_arm.c
+++ b/tools/libs/light/libxl_arm.c
@@ -1033,10 +1033,14 @@ static int make_virtio_mmio_node_device(libxl__gc *gc, void *fdt, uint64_t base,
     } else if (!strcmp(type, VIRTIO_DEVICE_TYPE_GPIO)) {
         res = make_virtio_mmio_node_gpio(gc, fdt);
         if (res) return res;
-    } else if (strcmp(type, VIRTIO_DEVICE_TYPE_GENERIC)) {
-        /* Doesn't match generic virtio device */
-        LOG(ERROR, "Invalid type for virtio device: %s", type);
-        return -EINVAL;
+    } else {
+        int len = sizeof(VIRTIO_DEVICE_TYPE_GENERIC) - 1;
+
+        if (strncmp(type, VIRTIO_DEVICE_TYPE_GENERIC, len)) {
+            /* Doesn't match generic virtio device */
+            LOG(ERROR, "Invalid type for virtio device: %s", type);
+            return -EINVAL;
+        }
     }
 
     return fdt_end_node(fdt);
-- 
2.31.1.272.g89b43f80a514



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

* Re: [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id
  2023-04-05  0:12 [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Viresh Kumar
  2023-04-05  0:12 ` [PATCH V2 2/2] libxl: fix matching of generic virtio device Viresh Kumar
@ 2023-04-05  8:36 ` Oleksandr Tyshchenko
  2023-04-05  8:51   ` Viresh Kumar
  1 sibling, 1 reply; 7+ messages in thread
From: Oleksandr Tyshchenko @ 2023-04-05  8:36 UTC (permalink / raw
  To: Viresh Kumar
  Cc: Vincent Guittot, Juergen Gross, stratos-dev, xen-devel,
	Alex Bennée, Mathieu Poirier, Erik Schilling, Julien Grall,
	Anthony PERARD



On 05.04.23 03:12, Viresh Kumar wrote:


Hello Viresh

> For generic virtio devices, where we don't need to add compatible or
> other special DT properties, the type field is set to "virtio,device".
> 
> But this misses the case where the user sets the type with a valid
> virtio device id as well, like "virtio,device26" for file system device.


ok. For the record, a valid virtio device ids can be found at:

https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-2160005

I don't know, maybe it is worth adding that link to commit description.


Also a NIT, is this example "like "virtio,device26" for file system 
device" precise?

According to
https://www.kernel.org/doc/Documentation/devicetree/bindings/virtio/virtio-device.yaml

the virtio device id should be in hex, so for file system device it
should be "virtio,device1a", or I really missed something?

With updating description if NIT is correct (I don't know, maybe this 
could be done on commit):
Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>



> 
> Update documentation to support that as well.
> 
> Fixes: dd54ea500be8 ("docs: add documentation for generic virtio devices")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V1->V2: New patch.
> 
>   docs/man/xl.cfg.5.pod.in | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
> index 10f37990be57..ea20eac0ba32 100644
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1608,8 +1608,9 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is
>   
>   L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml>
>   
> -For generic virtio devices, where we don't need to set special or compatible
> -properties in the Device Tree, the type field must be set to "virtio,device".
> +For other generic virtio devices, where we don't need to set special or
> +compatible properties in the Device Tree, the type field must be set to
> +"virtio,device" or "virtio,device<N>", where "N" is the virtio device id.



>   
>   =item B<transport=STRING>
>   


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

* Re: [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id
  2023-04-05  8:36 ` [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Oleksandr Tyshchenko
@ 2023-04-05  8:51   ` Viresh Kumar
  2023-04-05  9:15     ` Jan Beulich
  0 siblings, 1 reply; 7+ messages in thread
From: Viresh Kumar @ 2023-04-05  8:51 UTC (permalink / raw
  To: Oleksandr Tyshchenko
  Cc: Vincent Guittot, Juergen Gross, stratos-dev, xen-devel,
	Alex Bennée, Mathieu Poirier, Erik Schilling, Julien Grall,
	Anthony PERARD

On 05-04-23, 11:36, Oleksandr Tyshchenko wrote:
> Also a NIT, is this example "like "virtio,device26" for file system device"
> precise?

No :(

I will send the patch again later, this is how it looks now. I have
also updated the documentation to contain the hexadecimal format for
N.

Author: Viresh Kumar <viresh.kumar@linaro.org>
Date:   Wed Apr 5 05:36:19 2023 +0530

    docs: Allow generic virtio device types to contain device-id

    For generic virtio devices, where we don't need to add compatible or
    other special DT properties, the type field is set to "virtio,device".

    But this misses the case where the user sets the type with a valid
    virtio device id as well, like "virtio,device1a" for file system device.
    The complete list of virtio device ids is mentioned here:

    https://docs.oasis-open.org/virtio/virtio/v1.2/cs01/virtio-v1.2-cs01.html#x1-2160005

    Update documentation to support that as well.

    Fixes: dd54ea500be8 ("docs: add documentation for generic virtio devices")
    Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
    Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
---
 docs/man/xl.cfg.5.pod.in | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 10f37990be57..938aea22c798 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1608,8 +1608,10 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is

 L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml>

-For generic virtio devices, where we don't need to set special or compatible
-properties in the Device Tree, the type field must be set to "virtio,device".
+For other generic virtio devices, where we don't need to set special or
+compatible properties in the Device Tree, the type field must be set to
+"virtio,device" or "virtio,device<N>", where "N" is the virtio device id in
+hexadecimal format.

 =item B<transport=STRING>

-- 
viresh


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

* Re: [PATCH V2 2/2] libxl: fix matching of generic virtio device
  2023-04-05  0:12 ` [PATCH V2 2/2] libxl: fix matching of generic virtio device Viresh Kumar
@ 2023-04-05  9:11   ` Oleksandr Tyshchenko
  0 siblings, 0 replies; 7+ messages in thread
From: Oleksandr Tyshchenko @ 2023-04-05  9:11 UTC (permalink / raw
  To: Viresh Kumar
  Cc: Vincent Guittot, Anthony PERARD, Julien Grall, Juergen Gross,
	xen-devel, stratos-dev, Alex Bennée, Mathieu Poirier,
	Erik Schilling



On 05.04.23 03:12, Viresh Kumar wrote:


Hello Viresh

> The strings won't be an exact match, as we are only looking to match the
> prefix here, i.e. "virtio,device". This is already done properly in
> libxl_virtio.c file, lets do the same here too.
> 
> Fixes: 43ba5202e2ee ("libxl: add support for generic virtio device")
> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> V1->V2: Add the missing fixes tag.

Reviewed-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>

> 
>   tools/libs/light/libxl_arm.c | 12 ++++++++----
>   1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/libs/light/libxl_arm.c b/tools/libs/light/libxl_arm.c
> index ddc7b2a15975..97c80d7ed0fa 100644
> --- a/tools/libs/light/libxl_arm.c
> +++ b/tools/libs/light/libxl_arm.c
> @@ -1033,10 +1033,14 @@ static int make_virtio_mmio_node_device(libxl__gc *gc, void *fdt, uint64_t base,
>       } else if (!strcmp(type, VIRTIO_DEVICE_TYPE_GPIO)) {
>           res = make_virtio_mmio_node_gpio(gc, fdt);
>           if (res) return res;
> -    } else if (strcmp(type, VIRTIO_DEVICE_TYPE_GENERIC)) {
> -        /* Doesn't match generic virtio device */
> -        LOG(ERROR, "Invalid type for virtio device: %s", type);
> -        return -EINVAL;
> +    } else {
> +        int len = sizeof(VIRTIO_DEVICE_TYPE_GENERIC) - 1;
> +
> +        if (strncmp(type, VIRTIO_DEVICE_TYPE_GENERIC, len)) {
> +            /* Doesn't match generic virtio device */
> +            LOG(ERROR, "Invalid type for virtio device: %s", type);
> +            return -EINVAL;
> +        }
>       }
>   
>       return fdt_end_node(fdt);


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

* Re: [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id
  2023-04-05  8:51   ` Viresh Kumar
@ 2023-04-05  9:15     ` Jan Beulich
  2023-04-05  9:24       ` Viresh Kumar
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2023-04-05  9:15 UTC (permalink / raw
  To: Viresh Kumar
  Cc: Vincent Guittot, Juergen Gross, stratos-dev, xen-devel,
	Alex Bennée, Mathieu Poirier, Erik Schilling, Julien Grall,
	Anthony PERARD, Oleksandr Tyshchenko

On 05.04.2023 10:51, Viresh Kumar wrote:
> --- a/docs/man/xl.cfg.5.pod.in
> +++ b/docs/man/xl.cfg.5.pod.in
> @@ -1608,8 +1608,10 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is
> 
>  L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml>
> 
> -For generic virtio devices, where we don't need to set special or compatible
> -properties in the Device Tree, the type field must be set to "virtio,device".
> +For other generic virtio devices, where we don't need to set special or
> +compatible properties in the Device Tree, the type field must be set to
> +"virtio,device" or "virtio,device<N>", where "N" is the virtio device id in
> +hexadecimal format.

Are "virtio,device0x1a" or "virtio,device1A" valid, too? If so, all is fine,
but if not, constraints on the hex representation may want mentioning.

Jan


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

* Re: [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id
  2023-04-05  9:15     ` Jan Beulich
@ 2023-04-05  9:24       ` Viresh Kumar
  0 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2023-04-05  9:24 UTC (permalink / raw
  To: Jan Beulich
  Cc: Vincent Guittot, Juergen Gross, stratos-dev, xen-devel,
	Alex Bennée, Mathieu Poirier, Erik Schilling, Julien Grall,
	Anthony PERARD, Oleksandr Tyshchenko

On 05-04-23, 11:15, Jan Beulich wrote:
> On 05.04.2023 10:51, Viresh Kumar wrote:
> > --- a/docs/man/xl.cfg.5.pod.in
> > +++ b/docs/man/xl.cfg.5.pod.in
> > @@ -1608,8 +1608,10 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is
> > 
> >  L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml>
> > 
> > -For generic virtio devices, where we don't need to set special or compatible
> > -properties in the Device Tree, the type field must be set to "virtio,device".
> > +For other generic virtio devices, where we don't need to set special or
> > +compatible properties in the Device Tree, the type field must be set to
> > +"virtio,device" or "virtio,device<N>", where "N" is the virtio device id in
> > +hexadecimal format.
> 
> Are "virtio,device0x1a" or "virtio,device1A" valid, too? If so, all is fine,
> but if not, constraints on the hex representation may want mentioning.

From [1], both above are invalid. Updated doc as:

diff --git a/docs/man/xl.cfg.5.pod.in b/docs/man/xl.cfg.5.pod.in
index 10f37990be57..24ac92718288 100644
--- a/docs/man/xl.cfg.5.pod.in
+++ b/docs/man/xl.cfg.5.pod.in
@@ -1608,8 +1608,11 @@ example, "type=virtio,device22" for the I2C device, whose device-tree binding is

 L<https://www.kernel.org/doc/Documentation/devicetree/bindings/i2c/i2c-virtio.yaml>

-For generic virtio devices, where we don't need to set special or compatible
-properties in the Device Tree, the type field must be set to "virtio,device".
+For other generic virtio devices, where we don't need to set special or
+compatible properties in the Device Tree, the type field must be set to
+"virtio,device" or "virtio,device<N>", where "N" is the virtio device id in
+hexadecimal format, without the "0x" prefix and all in lower case, like
+"virtio,device1a" for the file system device.

-- 
viresh

[1] https://www.kernel.org/doc/Documentation/devicetree/bindings/virtio/virtio-device.yaml


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

end of thread, other threads:[~2023-04-05  9:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-05  0:12 [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Viresh Kumar
2023-04-05  0:12 ` [PATCH V2 2/2] libxl: fix matching of generic virtio device Viresh Kumar
2023-04-05  9:11   ` Oleksandr Tyshchenko
2023-04-05  8:36 ` [PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id Oleksandr Tyshchenko
2023-04-05  8:51   ` Viresh Kumar
2023-04-05  9:15     ` Jan Beulich
2023-04-05  9:24       ` Viresh Kumar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).