2023-10-16 07:12:48

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V4 0/4] xen: privcmd: Add ioeventfd and fix irqfd support

Hello,

Now that irqfd support (backend to guest interrupt) is already merged, this
series solves the other part of the problem, i.e. ioeventfd (guest to
backend interrupt).

More details inside the commits.

Arnd reported few issues with the ioctl macro usage and argument's layout, fixed
them for irqfd too, which was added recently.

--
Viresh

V3->V4:
- Use __u64 for indirect pointers in an ioctl command's arguments.
- Use u64_to_user_ptr() in kernel driver to access the same.
- Use _IOW() macro instead of the internal one: _IOC().

V2->V3:
- Remove explicit barriers and depend on spin lock instead to take care of it.
- Move check for empty ioeventfds list to privcmd_ioeventfd_deassign(), which
could earlier call ioreq_free() even when the list wasn't empty and so we
returned without printing a warning in v1 earlier. V2 implemented it
incorrectly.

V1->V2:
- Increment irq_info refcnt only for valid info.
- Use u64 type for addr.
- Add comments for use of barriers.
- Use spin lock instead of mutex as we need to use them in irq handler.
- Add a warning when kioreq is getting freed and ioeventfds list isn't empty.
- Use struct_size().
- Validate number of vcpus as well.

Viresh Kumar (4):
xen: Make struct privcmd_irqfd's layout architecture independent
xen: irqfd: Use _IOW instead of the internal _IOC() macro
xen: evtchn: Allow shared registration of IRQ handers
xen: privcmd: Add support for ioeventfd

drivers/xen/Kconfig | 8 +-
drivers/xen/events/events_base.c | 3 +-
drivers/xen/evtchn.c | 2 +-
drivers/xen/privcmd.c | 407 +++++++++++++++++++++++++++++-
include/uapi/xen/privcmd.h | 22 +-
include/xen/interface/hvm/ioreq.h | 51 ++++
6 files changed, 482 insertions(+), 11 deletions(-)
create mode 100644 include/xen/interface/hvm/ioreq.h

--
2.31.1.272.g89b43f80a514


2023-10-16 07:12:51

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V4 2/4] xen: irqfd: Use _IOW instead of the internal _IOC() macro

_IOC() an internal helper that we should not use in driver code. In
particular, we got the data direction wrong here, which breaks a number
of tools, as having "_IOC_NONE" should never be paired with a nonzero
size.

Use _IOW() instead.

Fixes: f8941e6c4c71 ("xen: privcmd: Add support for irqfd")
Reported-by: Arnd Bergmann <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Signed-off-by: Viresh Kumar <[email protected]>
---
include/uapi/xen/privcmd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/uapi/xen/privcmd.h b/include/uapi/xen/privcmd.h
index b143fafce84d..e145bca5105c 100644
--- a/include/uapi/xen/privcmd.h
+++ b/include/uapi/xen/privcmd.h
@@ -138,6 +138,6 @@ struct privcmd_irqfd {
#define IOCTL_PRIVCMD_MMAP_RESOURCE \
_IOC(_IOC_NONE, 'P', 7, sizeof(struct privcmd_mmap_resource))
#define IOCTL_PRIVCMD_IRQFD \
- _IOC(_IOC_NONE, 'P', 8, sizeof(struct privcmd_irqfd))
+ _IOW('P', 8, struct privcmd_irqfd)

#endif /* __LINUX_PUBLIC_PRIVCMD_H__ */
--
2.31.1.272.g89b43f80a514

2023-10-16 07:12:52

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH V4 3/4] xen: evtchn: Allow shared registration of IRQ handers

Currently the handling of events is supported either in the kernel or
userspace, but not both.

In order to support fast delivery of interrupts from the guest to the
backend, we need to handle the Queue notify part of Virtio protocol in
kernel and the rest in userspace.

Update the interrupt handler registration flag to IRQF_SHARED for event
channels, which would allow multiple entities to bind their interrupt
handler for the same event channel port.

Also increment the reference count of irq_info when multiple entities
try to bind event channel to irqchip, so the unbinding happens only
after all the users are gone.

Signed-off-by: Viresh Kumar <[email protected]>
---
drivers/xen/events/events_base.c | 3 ++-
drivers/xen/evtchn.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index c7715f8bd452..d72fb26cc051 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1238,7 +1238,8 @@ static int bind_evtchn_to_irq_chip(evtchn_port_t evtchn, struct irq_chip *chip,
bind_evtchn_to_cpu(evtchn, 0, false);
} else {
struct irq_info *info = info_for_irq(irq);
- WARN_ON(info == NULL || info->type != IRQT_EVTCHN);
+ if (!WARN_ON(!info || info->type != IRQT_EVTCHN))
+ info->refcnt++;
}

out:
diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index c99415a70051..43f77915feb5 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -397,7 +397,7 @@ static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
if (rc < 0)
goto err;

- rc = bind_evtchn_to_irqhandler_lateeoi(port, evtchn_interrupt, 0,
+ rc = bind_evtchn_to_irqhandler_lateeoi(port, evtchn_interrupt, IRQF_SHARED,
u->name, evtchn);
if (rc < 0)
goto err;
--
2.31.1.272.g89b43f80a514

2023-10-16 07:16:01

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH V4 2/4] xen: irqfd: Use _IOW instead of the internal _IOC() macro

On 16.10.23 09:11, Viresh Kumar wrote:
> _IOC() an internal helper that we should not use in driver code. In
> particular, we got the data direction wrong here, which breaks a number
> of tools, as having "_IOC_NONE" should never be paired with a nonzero
> size.
>
> Use _IOW() instead.
>
> Fixes: f8941e6c4c71 ("xen: privcmd: Add support for irqfd")
> Reported-by: Arnd Bergmann <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Viresh Kumar <[email protected]>

Reviewed-by: Juergen Gross <[email protected]>


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.08 kB)
OpenPGP public key
OpenPGP_signature.asc (505.00 B)
OpenPGP digital signature
Download all attachments

2023-10-16 07:18:07

by Juergen Gross

[permalink] [raw]
Subject: Re: [PATCH V4 3/4] xen: evtchn: Allow shared registration of IRQ handers

On 16.10.23 09:11, Viresh Kumar wrote:
> Currently the handling of events is supported either in the kernel or
> userspace, but not both.
>
> In order to support fast delivery of interrupts from the guest to the
> backend, we need to handle the Queue notify part of Virtio protocol in
> kernel and the rest in userspace.
>
> Update the interrupt handler registration flag to IRQF_SHARED for event
> channels, which would allow multiple entities to bind their interrupt
> handler for the same event channel port.
>
> Also increment the reference count of irq_info when multiple entities
> try to bind event channel to irqchip, so the unbinding happens only
> after all the users are gone.
>
> Signed-off-by: Viresh Kumar <[email protected]>

Reviewed-by: Juergen Gross <[email protected]>


Juergen


Attachments:
OpenPGP_0xB0DE9DD628BF132F.asc (3.08 kB)
OpenPGP public key
OpenPGP_signature.asc (505.00 B)
OpenPGP digital signature
Download all attachments