[V10,0/3] toolstack support for generic virtio devices on Arm

Message

Viresh Kumar Dec. 14, 2022, 5:19 a.m. UTC

Hello,

This patchset adds toolstack support for I2C, GPIO and generic virtio devices.
This is inspired from the work done by Oleksandr for the Disk device.

This is developed as part of Linaro's Project Stratos, where we are working
towards Hypervisor agnostic Rust based backends [1].

This is based of Xen's master branch.

V9->V10:
- Swap I2C/GPIO compatible strings, they were wrongly added earlier.
- Arrange tags in timely order and add new ones.

V8->V9:
- Drop changes to tools/ocaml/libs/xl/genwrap.py file.
- Replace GPIO with I2C in a comment.
- Add Reviewed-by tags.

V7->V8:
- Use macros for compatible string names.
- Use strcmp() instead of strncmp() at several places.
- Rename "virtio,devices" to "virtio,device" in commit log.
- Remove extra call to fdt_end_node().
- Disallow "unknown" in xenstore transport.
- Use libxl__strdup().
- Update documentation.
- Remove DEVICE_ADDREMOVE(virtio).

V6->V7:
- Support generic virtio devices too. They are passed with type=virtio,device,
  and we only create the MMIO DT node for them.
- Add links to DT bindings of I2C and GPIO, in code and documentation.
- Call libxl__device_add() for all hypervisor types.
- Add (0, "UNKNOWN") for libxl_virtio_transport.
- Removed libxl_virtio_backend and libxl_virtioinfo, as they were unused.
- Remove unnecessary stuff from libxl__virtio_from_xenstore() and add support
  for type and transport.
- Add backend=domid in documentation and replace compatible with type.
- Improved commit logs.

V5->V6:
- The cleanup patches are sent separately [2].
- We don't add I2C or GPIO specific device changes anymore, rather we create
  generic "virtio" devices. The "type" of a virtio devices helps us identify the
  right device, and create an entry in the DT node. The same can be used for all
  Virtio devices now.
- Update man page xl.cfg.

V4->V5:
- Fixed indentation at few places.
- Removed/added blank lines.
- Added few comments.
- Added review tags from Oleksandr.
- Rebased over latest staging branch.

V3->V4:
- Update virtio_enabled independently of all devices, so we don't miss setting
  it to true.

- Add iommu handling for i2c/gpio and move it as part of
  make_virtio_mmio_node_common(), which gets backend_domid parameter as a
  result.

V2->V3:
- Rebased over latest tree and made changes according to changes in Oleksandr's
  patches from sometime back.
- Minor cleanups.

V1->V2:
- Patches 3/6 and 4/6 are new.
- Patches 5/6 and 6/6 updated based on the above two patches.
- Added link to the bindings for I2C and GPIO.
- Rebased over latest master branch.


Thanks.

--
Viresh

[1] https://lore.kernel.org/xen-devel/20220414092358.kepxbmnrtycz7mhe@vireshk-i7/

Viresh Kumar (3):
  libxl: Add support for generic virtio device
  xl: Add support to parse generic virtio device
  docs: Add documentation for generic virtio devices

 docs/man/xl.cfg.5.pod.in                  |  33 +++++
 tools/libs/light/Makefile                 |   1 +
 tools/libs/light/libxl_arm.c              | 100 +++++++++++++++
 tools/libs/light/libxl_create.c           |   4 +
 tools/libs/light/libxl_internal.h         |   6 +
 tools/libs/light/libxl_types.idl          |  18 +++
 tools/libs/light/libxl_types_internal.idl |   1 +
 tools/libs/light/libxl_virtio.c           | 144 ++++++++++++++++++++++
 tools/xl/xl_parse.c                       |  81 ++++++++++++
 9 files changed, 388 insertions(+)
 create mode 100644 tools/libs/light/libxl_virtio.c

Comments

Jan Beulich Dec. 14, 2022, 8:21 a.m. UTC | #1

On 14.12.2022 06:19, Viresh Kumar wrote:
> This patchset adds toolstack support for I2C, GPIO and generic virtio devices.
> This is inspired from the work done by Oleksandr for the Disk device.
> 
> This is developed as part of Linaro's Project Stratos, where we are working
> towards Hypervisor agnostic Rust based backends [1].
> 
> This is based of Xen's master branch.
> 
> V9->V10:
> - Swap I2C/GPIO compatible strings, they were wrongly added earlier.
> - Arrange tags in timely order and add new ones.

The affected patch surely needed all tags to be dropped which weren't
explicitly indicated to remain valid across the bug fix.

Jan

Viresh Kumar Dec. 14, 2022, 8:34 a.m. UTC | #2

On 14-12-22, 09:21, Jan Beulich wrote:
> On 14.12.2022 06:19, Viresh Kumar wrote:
> > This patchset adds toolstack support for I2C, GPIO and generic virtio devices.
> > This is inspired from the work done by Oleksandr for the Disk device.
> > 
> > This is developed as part of Linaro's Project Stratos, where we are working
> > towards Hypervisor agnostic Rust based backends [1].
> > 
> > This is based of Xen's master branch.
> > 
> > V9->V10:
> > - Swap I2C/GPIO compatible strings, they were wrongly added earlier.
> > - Arrange tags in timely order and add new ones.
> 
> The affected patch surely needed all tags to be dropped which weren't
> explicitly indicated to remain valid across the bug fix.

Hmm, since it was a very small change (only fixed the compatible
strings) I thought it would be okay to carry the tags and rather harsh
to the reviewers to drop them, since they invested a lot of time
reviewing these. Though I do get what you are suggesting here.

Should I ask Oleksandr / Anthony to provide their tags for patch 1/3
again ?

Jan Beulich Dec. 14, 2022, 9:09 a.m. UTC | #3

On 14.12.2022 09:34, Viresh Kumar wrote:
> On 14-12-22, 09:21, Jan Beulich wrote:
>> On 14.12.2022 06:19, Viresh Kumar wrote:
>>> This patchset adds toolstack support for I2C, GPIO and generic virtio devices.
>>> This is inspired from the work done by Oleksandr for the Disk device.
>>>
>>> This is developed as part of Linaro's Project Stratos, where we are working
>>> towards Hypervisor agnostic Rust based backends [1].
>>>
>>> This is based of Xen's master branch.
>>>
>>> V9->V10:
>>> - Swap I2C/GPIO compatible strings, they were wrongly added earlier.
>>> - Arrange tags in timely order and add new ones.
>>
>> The affected patch surely needed all tags to be dropped which weren't
>> explicitly indicated to remain valid across the bug fix.
> 
> Hmm, since it was a very small change (only fixed the compatible
> strings) I thought it would be okay to carry the tags and rather harsh
> to the reviewers to drop them, since they invested a lot of time
> reviewing these. Though I do get what you are suggesting here.
> 
> Should I ask Oleksandr / Anthony to provide their tags for patch 1/3
> again ?

Let's simply wait for them to confirm their tags.

Jan

Oleksandr Tyshchenko Dec. 14, 2022, 9:16 a.m. UTC | #4

On 14.12.22 11:09, Jan Beulich wrote:

Hello all

> On 14.12.2022 09:34, Viresh Kumar wrote:
>> On 14-12-22, 09:21, Jan Beulich wrote:
>>> On 14.12.2022 06:19, Viresh Kumar wrote:
>>>> This patchset adds toolstack support for I2C, GPIO and generic virtio devices.
>>>> This is inspired from the work done by Oleksandr for the Disk device.
>>>>
>>>> This is developed as part of Linaro's Project Stratos, where we are working
>>>> towards Hypervisor agnostic Rust based backends [1].
>>>>
>>>> This is based of Xen's master branch.
>>>>
>>>> V9->V10:
>>>> - Swap I2C/GPIO compatible strings, they were wrongly added earlier.
>>>> - Arrange tags in timely order and add new ones.
>>> The affected patch surely needed all tags to be dropped which weren't
>>> explicitly indicated to remain valid across the bug fix.
>> Hmm, since it was a very small change (only fixed the compatible
>> strings) I thought it would be okay to carry the tags and rather harsh
>> to the reviewers to drop them, since they invested a lot of time
>> reviewing these. Though I do get what you are suggesting here.
>>
>> Should I ask Oleksandr / Anthony to provide their tags for patch 1/3
>> again ?
> Let's simply wait for them to confirm their tags.


Viresh, I confirm that my R-b for #1/3 is still valid (thanks for making 
a change). I probably should have mentioned that explicitly when making 
a comment.



>
> Jan

Anthony PERARD Dec. 14, 2022, 10:43 a.m. UTC | #5

On Wed, Dec 14, 2022 at 02:04:56PM +0530, Viresh Kumar wrote:
> On 14-12-22, 09:21, Jan Beulich wrote:
> > On 14.12.2022 06:19, Viresh Kumar wrote:
> > > This patchset adds toolstack support for I2C, GPIO and generic virtio devices.
> > > This is inspired from the work done by Oleksandr for the Disk device.
> > > 
> > > This is developed as part of Linaro's Project Stratos, where we are working
> > > towards Hypervisor agnostic Rust based backends [1].
> > > 
> > > This is based of Xen's master branch.
> > > 
> > > V9->V10:
> > > - Swap I2C/GPIO compatible strings, they were wrongly added earlier.
> > > - Arrange tags in timely order and add new ones.
> > 
> > The affected patch surely needed all tags to be dropped which weren't
> > explicitly indicated to remain valid across the bug fix.
> 
> Hmm, since it was a very small change (only fixed the compatible
> strings) I thought it would be okay to carry the tags and rather harsh
> to the reviewers to drop them, since they invested a lot of time
> reviewing these. Though I do get what you are suggesting here.

I would have carry over the tags as well in this case.

If the tag were dropped, it wouldn't really have been an issue for me,
it doesn't take much time to review a patch series that I've already
reviewed. I usually look at a diff with the previous version (with `git
range-diff`, patchew can help as well[1]), and my previous comment if
needed.

But dropping rvb tag of non-maintainer maybe more "harsh" as you says as
they may not have time to re-apply their tags before patch gets committed.

> Should I ask Oleksandr / Anthony to provide their tags for patch 1/3
> again ?

My tag is still valid. Thanks.

Cheers,


[1] https://patchew.org/Xen/[e-mail]/
    Then look for "diff against: v9"