- Stratos-dev - op-lists.linaro.org

Re: [PATCH] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

by Alex Bennée

Philippe Mathieu-Daudé <philmd(a)linaro.org> writes: > On 17/4/23 08:02, Viresh Kumar wrote: >> Since the driver doesn't support interrupts, we must return early when >> index is set to VIRTIO_CONFIG_IRQ_IDX. >> Fixes: 544f0278afca ("virtio: introduce macro >> VIRTIO_CONFIG_IRQ_IDX") >> Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> >> --- >> hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++ >> 1 file changed, 20 insertions(+) >> diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c >> index 60eaf0d95be0..45100a24953c 100644 >> --- a/hw/virtio/vhost-user-i2c.c >> +++ b/hw/virtio/vhost-user-i2c.c >> @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) >> { >> VHostUserI2C *i2c = VHOST_USER_I2C(vdev); >> + /* >> + * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1 >> + * as the Marco of configure interrupt's IDX, If this driver does not > > Copy/paste of pre-existing comment, still I wonder who is "the Marco > of configure" :P > >> + * support, the function will return >> + */ First patch of my last VirtIO series fixes these all up. -- Alex Bennée Virtualisation Tech Lead @ Linaro

1 year, 8 months

1
0
0 0

Re: [PATCH] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

by Viresh Kumar

On 17-04-23, 09:36, Philippe Mathieu-Daudé wrote: > On 17/4/23 08:02, Viresh Kumar wrote: > > Since the driver doesn't support interrupts, we must return early when > > index is set to VIRTIO_CONFIG_IRQ_IDX. > > > > Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX") > > Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> > > --- > > hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c > > index 60eaf0d95be0..45100a24953c 100644 > > --- a/hw/virtio/vhost-user-i2c.c > > +++ b/hw/virtio/vhost-user-i2c.c > > @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) > > { > > VHostUserI2C *i2c = VHOST_USER_I2C(vdev); > > + /* > > + * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1 > > + * as the Marco of configure interrupt's IDX, If this driver does not > > Copy/paste of pre-existing comment, still I wonder who is "the Marco of > configure" :P > > > + * support, the function will return > > + */ Yeah the comment could be improved, I didn't touch it as it was written this way for many drivers :) Maybe a simple comment like is all we need: /* * We don't support interrupts, return early if index is set to * VIRTIO_CONFIG_IRQ_IDX. */ -- viresh

1 year, 8 months

1
0
0 0

[PATCH] virtio: i2c: Check notifier helpers for VIRTIO_CONFIG_IRQ_IDX

by Viresh Kumar

Since the driver doesn't support interrupts, we must return early when index is set to VIRTIO_CONFIG_IRQ_IDX. Fixes: 544f0278afca ("virtio: introduce macro VIRTIO_CONFIG_IRQ_IDX") Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- hw/virtio/vhost-user-i2c.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/hw/virtio/vhost-user-i2c.c b/hw/virtio/vhost-user-i2c.c index 60eaf0d95be0..45100a24953c 100644 --- a/hw/virtio/vhost-user-i2c.c +++ b/hw/virtio/vhost-user-i2c.c @@ -128,6 +128,16 @@ static void vu_i2c_guest_notifier_mask(VirtIODevice *vdev, int idx, bool mask) { VHostUserI2C *i2c = VHOST_USER_I2C(vdev); + /* + * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1 + * as the Marco of configure interrupt's IDX, If this driver does not + * support, the function will return + */ + + if (idx == VIRTIO_CONFIG_IRQ_IDX) { + return; + } + vhost_virtqueue_mask(&i2c->vhost_dev, vdev, idx, mask); } @@ -135,6 +145,16 @@ static bool vu_i2c_guest_notifier_pending(VirtIODevice *vdev, int idx) { VHostUserI2C *i2c = VHOST_USER_I2C(vdev); + /* + * Add the check for configure interrupt, Use VIRTIO_CONFIG_IRQ_IDX -1 + * as the Marco of configure interrupt's IDX, If this driver does not + * support, the function will return + */ + + if (idx == VIRTIO_CONFIG_IRQ_IDX) { + return false; + } + return vhost_virtqueue_pending(&i2c->vhost_dev, idx); } -- 2.31.1.272.g89b43f80a514

1 year, 8 months

1
0
0 0

Re: [PATCH] gpio: aggregator: Add interrupt support

by Geert Uytterhoeven

Hi Kamel, On Thu, Apr 13, 2023 at 9:48 AM <kamel.bouhara(a)bootlin.com> wrote: > Le 2021-10-04 14:44, Geert Uytterhoeven a écrit : > What is the status for this patch, is there any remaining > changes to be made ? You mean commit a00128dfc8fc0cc8 ("gpio: aggregator: Add interrupt support") in v5.17? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert(a)linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds

1 year, 8 months

1
0
0 0

[PATCH V3 0/2] qemu: vhost-user: Support Xen memory mapping quirks

by Viresh Kumar

Hello, This patchset tries to update the vhost-user protocol to make it support special memory mapping required in case of Xen hypervisor. The first patch is mostly cleanup and second one introduces a new xen specific feature. V2->V3: - Remove the extra message and instead update the memory regions to carry additional data. - Drop the one region one mmap relationship and allow back-end to map only parts of a region at once, required for Xen grant mappings. - Additional cleanup patch 1/2. V1->V2: - Make the custom mmap feature Xen specific, instead of being generic. - Clearly define which memory regions are impacted by this change. - Allow VHOST_USER_SET_XEN_MMAP to be called multiple times. - Additional Bit(2) property in flags. Viresh Kumar (2): docs: vhost-user: Define memory region separately docs: vhost-user: Add Xen specific memory mapping support docs/interop/vhost-user.rst | 60 ++++++++++++++++++++++++------------- 1 file changed, 39 insertions(+), 21 deletions(-) -- 2.31.1.272.g89b43f80a514

1 year, 8 months

4
13
0 0

[PATCH V2 1/2] docs: Allow generic virtio device types to contain device-id

by Viresh Kumar

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(a)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…> -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

1 year, 8 months

3
6
0 0

[PATCH] libxl: fix matching of generic virtio device

by Viresh Kumar

The strings won't be an exact match, and 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. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- 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

1 year, 8 months

2
1
0 0

[PATCH V2] docs: vhost-user: Add Xen specific memory mapping support

by Viresh Kumar

The current model of memory mapping at the back-end works fine where a standard call to mmap() (for the respective file descriptor) is enough before the front-end can start accessing the guest memory. There are other complex cases though where the back-end needs more information and simple mmap() isn't enough. For example Xen, a type-1 hypervisor, currently supports memory mapping via two different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via /dev/gntdev). In both these cases, the back-end needs to call mmap() and ioctl(), and need to pass extra information via the ioctl(), like the Xen domain-id of the guest whose memory we are trying to map. Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_XEN_MMAP', which lets the back-end know about the additional memory mapping requirements. When this feature is negotiated, the front-end can send the 'VHOST_USER_SET_XEN_MMAP' message type to provide the additional information to the back-end. Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> --- V1->V2: - Make the custom mmap feature Xen specific, instead of being generic. - Clearly define which memory regions are impacted by this change. - Allow VHOST_USER_SET_XEN_MMAP to be called multiple times. - Additional Bit(2) property in flags. docs/interop/vhost-user.rst | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst index 3f18ab424eb0..8be5f5eae941 100644 --- a/docs/interop/vhost-user.rst +++ b/docs/interop/vhost-user.rst @@ -258,6 +258,24 @@ Inflight description :queue size: a 16-bit size of virtqueues +Xen mmap description +^^^^^^^^^^^^^^^^^^^^ + ++-------+-------+ +| flags | domid | ++-------+-------+ + +:flags: 64-bit bit field + +- Bit 0 is set for Xen foreign memory memory mapping. +- Bit 1 is set for Xen grant memory memory mapping. +- Bit 2 is set if the back-end can directly map additional memory (like + descriptor buffers or indirect descriptors, which aren't part of already + shared memory regions) without the need of front-end sending an additional + memory region first. + +:domid: a 64-bit Xen hypervisor specific domain id. + C structure ----------- @@ -867,6 +885,7 @@ Protocol features #define VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS 14 #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 #define VHOST_USER_PROTOCOL_F_STATUS 16 + #define VHOST_USER_PROTOCOL_F_XEN_MMAP 17 Front-end message types ----------------------- @@ -1422,6 +1441,23 @@ Front-end message types query the back-end for its device status as defined in the Virtio specification. +``VHOST_USER_SET_XEN_MMAP`` + :id: 41 + :equivalent ioctl: N/A + :request payload: Xen mmap description + :reply payload: N/A + + When the ``VHOST_USER_PROTOCOL_F_XEN_MMAP`` protocol feature has been + successfully negotiated, this message is submitted by the front-end to set the + Xen hypervisor specific memory mapping configurations at the back-end. These + configurations should be used to mmap memory regions, virtqueues, descriptors + and descriptor buffers. The front-end must send this message before any + memory-regions are sent to the back-end via ``VHOST_USER_SET_MEM_TABLE`` or + ``VHOST_USER_ADD_MEM_REG`` message types. The front-end can send this message + multiple times, if different mmap configurations are required for different + memory regions, where the most recent ``VHOST_USER_SET_XEN_MMAP`` must be used + by the back-end to map any newly shared memory regions. + Back-end message types ---------------------- -- 2.31.1.272.g89b43f80a514

1 year, 9 months

2
4
0 0

Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support

by Stefan Hajnoczi

Resend - for some reason my email didn't make it out. ----- Forwarded message from Stefan Hajnoczi <stefanha(a)redhat.com> ----- Date: Tue, 21 Feb 2023 10:17:01 -0500 From: Stefan Hajnoczi <stefanha(a)redhat.com> To: Viresh Kumar <viresh.kumar(a)linaro.org> Cc: qemu-devel(a)nongnu.org, virtio-dev(a)lists.oasis-open.org, "Michael S. Tsirkin" <mst(a)redhat.com>, Vincent Guittot <vincent.guittot(a)linaro.org>, Alex Bennée <alex.bennee(a)linaro.org>, stratos-dev(a)op-lists.linaro.org, Oleksandr Tyshchenko <olekstysh(a)gmail.com>, xen-devel(a)lists.xen.org, Andrew Cooper <andrew.cooper3(a)citrix.com>, Juergen Gross <jgross(a)suse.com>, Sebastien Boeuf <sebastien.boeuf(a)intel.com>, Liu Jiang <gerry(a)linux.alibaba.com>, Mathieu Poirier <mathieu.poirier(a)linaro.org> Subject: Re: [virtio-dev] [RFC QEMU] docs: vhost-user: Add custom memory mapping support Message-ID: <Y/TgbZLY894p4a1S@fedora> On Tue, Feb 21, 2023 at 03:20:41PM +0530, Viresh Kumar wrote: > The current model of memory mapping at the back-end works fine with > Qemu, where a standard call to mmap() for the respective file > descriptor, passed from front-end, is generally all we need to do before > the front-end can start accessing the guest memory. > > There are other complex cases though, where we need more information at > the backend and need to do more than just an mmap() call. For example, > Xen, a type-1 hypervisor, currently supports memory mapping via two > different methods, foreign-mapping (via /dev/privcmd) and grant-dev (via > /dev/gntdev). In both these cases, the back-end needs to call mmap() > followed by an ioctl() (or vice-versa), and need to pass extra > information via the ioctl(), like the Xen domain-id of the guest whose > memory we are trying to map. > > Add a new protocol feature, 'VHOST_USER_PROTOCOL_F_CUSTOM_MMAP', which > lets the back-end know about the additional memory mapping requirements. > When this feature is negotiated, the front-end can send the > 'VHOST_USER_CUSTOM_MMAP' message type to provide the additional > information to the back-end. > > Signed-off-by: Viresh Kumar <viresh.kumar(a)linaro.org> > --- > docs/interop/vhost-user.rst | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) The alternative to an in-band approach is to configure these details out-of-band. For example, via command-line options to the vhost-user back-end: $ my-xen-device --mapping-type=foreign-mapping --domain-id=123 I was thinking about both approaches and don't see an obvious reason to choose one or the other. What do you think? > diff --git a/docs/interop/vhost-user.rst b/docs/interop/vhost-user.rst > index 3f18ab424eb0..f2b1d705593a 100644 > --- a/docs/interop/vhost-user.rst > +++ b/docs/interop/vhost-user.rst > @@ -258,6 +258,23 @@ Inflight description > > :queue size: a 16-bit size of virtqueues > > +Custom mmap description > +^^^^^^^^^^^^^^^^^^^^^^^ > + > ++-------+-------+ > +| flags | value | > ++-------+-------+ > + > +:flags: 64-bit bit field > + > +- Bit 0 is Xen foreign memory access flag - needs Xen foreign memory mapping. > +- Bit 1 is Xen grant memory access flag - needs Xen grant memory mapping. > + > +:value: a 64-bit hypervisor specific value. > + > +- For Xen foreign or grant memory access, this is set with guest's xen domain > + id. This is highly Xen-specific. How about naming the feature XEN_MMAP instead of CUSTOM_MMAP? If someone needs to add other mmap data later, they should define their own struct instead of trying to squeeze into the same fields as Xen. There is an assumption in this design that a single VHOST_USER_CUSTOM_MMAP message provides the information necessary for all mmaps. Are you sure the limitation that every mmap belongs to the same domain will be workable in the future? > + > C structure > ----------- > > @@ -867,6 +884,7 @@ Protocol features > #define VHOST_USER_PROTOCOL_F_INBAND_NOTIFICATIONS 14 > #define VHOST_USER_PROTOCOL_F_CONFIGURE_MEM_SLOTS 15 > #define VHOST_USER_PROTOCOL_F_STATUS 16 > + #define VHOST_USER_PROTOCOL_F_CUSTOM_MMAP 17 > > Front-end message types > ----------------------- > @@ -1422,6 +1440,20 @@ Front-end message types > query the back-end for its device status as defined in the Virtio > specification. > > +``VHOST_USER_CUSTOM_MMAP`` Most vhost-user protocol messages have a verb like get/set/close/add/listen/etc. I suggest renaming this to VHOST_USER_SET_XEN_MMAP_INFO. > + :id: 41 > + :equivalent ioctl: N/A > + :request payload: Custom mmap description > + :reply payload: N/A > + > + When the ``VHOST_USER_PROTOCOL_F_CUSTOM_MMAP`` protocol feature has been > + successfully negotiated, this message is submitted by the front-end to > + notify the back-end of the special memory mapping requirements, that the > + back-end needs to take care of, while mapping any memory regions sent > + over by the front-end. The front-end must send this message before > + any memory-regions are sent to the back-end via ``VHOST_USER_SET_MEM_TABLE`` > + or ``VHOST_USER_ADD_MEM_REG`` message types. > + > > Back-end message types > ---------------------- > -- > 2.31.1.272.g89b43f80a514 > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: virtio-dev-unsubscribe(a)lists.oasis-open.org > For additional commands, e-mail: virtio-dev-help(a)lists.oasis-open.org > ----- End forwarded message -----

1 year, 9 months

3
5
0 0

Re: [Discussion] Xen grants and access permissions

by Viresh Kumar

On 20-02-23, 07:13, Juergen Gross wrote: > There are no permission flags in Xen PV device protocols either. The kind of a > mapping (RO or RW) in the backend is selected via the I/O operation: in case it > is a write type operation (guest writing data to a device), the related grants > are mapper as RO in the backend, in all other cases they are mapped as RW. > > The same applies to granted pages for virtio: the frontend side will grant the > page as RO in case the I/O operation is flagged as "DMA_TO_DEVICE", and as RW > in all other cases. The backend should always know, which direction the data is > flowing, so it should be able to do the mapping with the correct access mode. Right, so the back-end actually knows the permission details, but it is getting lost while we do some vhost-user operations. Anyway, I have taken this in a different direction now and suggested a change to vhost-user protocol itself. That lets the back-end know that it is actually running on Xen and then it can do the mapping itself instead of asking the front-end, which doesn't make us loose the permission details. This also lets us write the backends in hypervisor agnostic way, hypervisor specific stuff is handled in vhost-user protocol's implementation now. https://lists.gnu.org/archive/html/qemu-devel/2023-02/msg05946.html -- viresh

1 year, 9 months

1
0
0 0

2024

2023

2022

2021

2020

Stratos-dev