drivers/xen/events/events_base.c | 7 +++++-- drivers/xen/evtchn.c | 30 ++++++++++++++++++++++-------- include/uapi/xen/evtchn.h | 9 +++++++++ include/xen/events.h | 2 +- 4 files changed, 37 insertions(+), 11 deletions(-)
Xen 4.17 supports the creation of static evtchns. To allow user space
application to bind static evtchns introduce new ioctl
"IOCTL_EVTCHN_BIND_STATIC". Existing IOCTL doing more than binding
that’s why we need to introduce the new IOCTL to only bind the static
event channels.
Also, static evtchns to be available for use during the lifetime of the
guest. When the application exits, __unbind_from_irq() ends up being
called from release() file operations because of that static evtchns
are getting closed. To avoid closing the static event channel, add the
new bool variable "is_static" in "struct irq_info" to mark the event
channel static when creating the event channel to avoid closing the
static evtchn.
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
---
v2:
* Use bool in place u8 to define is_static variable.
* Avoid closing the static evtchns in error path.
---
drivers/xen/events/events_base.c | 7 +++++--
drivers/xen/evtchn.c | 30 ++++++++++++++++++++++--------
include/uapi/xen/evtchn.h | 9 +++++++++
include/xen/events.h | 2 +-
4 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index c7715f8bd452..5d3b5c7cfe64 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -112,6 +112,7 @@ struct irq_info {
unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
u64 eoi_time; /* Time in jiffies when to EOI. */
raw_spinlock_t lock;
+ bool is_static; /* Is event channel static */
union {
unsigned short virq;
@@ -982,7 +983,8 @@ static void __unbind_from_irq(unsigned int irq)
unsigned int cpu = cpu_from_irq(irq);
struct xenbus_device *dev;
- xen_evtchn_close(evtchn);
+ if (!info->is_static)
+ xen_evtchn_close(evtchn);
switch (type_from_irq(irq)) {
case IRQT_VIRQ:
@@ -1574,7 +1576,7 @@ int xen_set_irq_priority(unsigned irq, unsigned priority)
}
EXPORT_SYMBOL_GPL(xen_set_irq_priority);
-int evtchn_make_refcounted(evtchn_port_t evtchn)
+int evtchn_make_refcounted(evtchn_port_t evtchn, bool is_static)
{
int irq = get_evtchn_to_irq(evtchn);
struct irq_info *info;
@@ -1590,6 +1592,7 @@ int evtchn_make_refcounted(evtchn_port_t evtchn)
WARN_ON(info->refcnt != -1);
info->refcnt = 1;
+ info->is_static = is_static;
return 0;
}
diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
index c99415a70051..e6d2303478b2 100644
--- a/drivers/xen/evtchn.c
+++ b/drivers/xen/evtchn.c
@@ -366,7 +366,8 @@ static int evtchn_resize_ring(struct per_user_data *u)
return 0;
}
-static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
+static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port,
+ bool is_static)
{
struct user_evtchn *evtchn;
struct evtchn_close close;
@@ -402,14 +403,16 @@ static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
if (rc < 0)
goto err;
- rc = evtchn_make_refcounted(port);
+ rc = evtchn_make_refcounted(port, is_static);
return rc;
err:
/* bind failed, should close the port now */
- close.port = port;
- if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
- BUG();
+ if (!is_static) {
+ close.port = port;
+ if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
+ BUG();
+ }
del_evtchn(u, evtchn);
return rc;
}
@@ -456,7 +459,7 @@ static long evtchn_ioctl(struct file *file,
if (rc != 0)
break;
- rc = evtchn_bind_to_user(u, bind_virq.port);
+ rc = evtchn_bind_to_user(u, bind_virq.port, false);
if (rc == 0)
rc = bind_virq.port;
break;
@@ -482,7 +485,7 @@ static long evtchn_ioctl(struct file *file,
if (rc != 0)
break;
- rc = evtchn_bind_to_user(u, bind_interdomain.local_port);
+ rc = evtchn_bind_to_user(u, bind_interdomain.local_port, false);
if (rc == 0)
rc = bind_interdomain.local_port;
break;
@@ -507,7 +510,7 @@ static long evtchn_ioctl(struct file *file,
if (rc != 0)
break;
- rc = evtchn_bind_to_user(u, alloc_unbound.port);
+ rc = evtchn_bind_to_user(u, alloc_unbound.port, false);
if (rc == 0)
rc = alloc_unbound.port;
break;
@@ -536,6 +539,17 @@ static long evtchn_ioctl(struct file *file,
break;
}
+ case IOCTL_EVTCHN_BIND_STATIC: {
+ struct ioctl_evtchn_bind bind;
+
+ rc = -EFAULT;
+ if (copy_from_user(&bind, uarg, sizeof(bind)))
+ break;
+
+ rc = evtchn_bind_to_user(u, bind.port, true);
+ break;
+ }
+
case IOCTL_EVTCHN_NOTIFY: {
struct ioctl_evtchn_notify notify;
struct user_evtchn *evtchn;
diff --git a/include/uapi/xen/evtchn.h b/include/uapi/xen/evtchn.h
index 7fbf732f168f..aef2b75f3413 100644
--- a/include/uapi/xen/evtchn.h
+++ b/include/uapi/xen/evtchn.h
@@ -101,4 +101,13 @@ struct ioctl_evtchn_restrict_domid {
domid_t domid;
};
+/*
+ * Bind statically allocated @port.
+ */
+#define IOCTL_EVTCHN_BIND_STATIC \
+ _IOC(_IOC_NONE, 'E', 7, sizeof(struct ioctl_evtchn_bind))
+struct ioctl_evtchn_bind {
+ unsigned int port;
+};
+
#endif /* __LINUX_PUBLIC_EVTCHN_H__ */
diff --git a/include/xen/events.h b/include/xen/events.h
index ac1281c5ead6..377ad7e391e8 100644
--- a/include/xen/events.h
+++ b/include/xen/events.h
@@ -69,7 +69,7 @@ int xen_set_irq_priority(unsigned irq, unsigned priority);
/*
* Allow extra references to event channels exposed to userspace by evtchn
*/
-int evtchn_make_refcounted(evtchn_port_t evtchn);
+int evtchn_make_refcounted(evtchn_port_t evtchn, bool is_static);
int evtchn_get(evtchn_port_t evtchn);
void evtchn_put(evtchn_port_t evtchn);
base-commit: 3a8a670eeeaa40d87bd38a587438952741980c18
--
2.25.1
On 29.06.23 18:46, Rahul Singh wrote:
Hello Rahul
> Xen 4.17 supports the creation of static evtchns. To allow user space
> application to bind static evtchns introduce new ioctl
> "IOCTL_EVTCHN_BIND_STATIC". Existing IOCTL doing more than binding
> that’s why we need to introduce the new IOCTL to only bind the static
> event channels.
>
> Also, static evtchns to be available for use during the lifetime of the
> guest. When the application exits, __unbind_from_irq() ends up being
> called from release() file operations because of that static evtchns
> are getting closed. To avoid closing the static event channel, add the
> new bool variable "is_static" in "struct irq_info" to mark the event
> channel static when creating the event channel to avoid closing the
> static evtchn.
>
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> ---
> v2:
> * Use bool in place u8 to define is_static variable.
> * Avoid closing the static evtchns in error path.
Patch looks good to me, just a nit (question) below.
> ---
> drivers/xen/events/events_base.c | 7 +++++--
> drivers/xen/evtchn.c | 30 ++++++++++++++++++++++--------
> include/uapi/xen/evtchn.h | 9 +++++++++
> include/xen/events.h | 2 +-
> 4 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index c7715f8bd452..5d3b5c7cfe64 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -112,6 +112,7 @@ struct irq_info {
> unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
> u64 eoi_time; /* Time in jiffies when to EOI. */
> raw_spinlock_t lock;
> + bool is_static; /* Is event channel static */
>
> union {
> unsigned short virq;
> @@ -982,7 +983,8 @@ static void __unbind_from_irq(unsigned int irq)
> unsigned int cpu = cpu_from_irq(irq);
> struct xenbus_device *dev;
>
> - xen_evtchn_close(evtchn);
> + if (!info->is_static)
> + xen_evtchn_close(evtchn);
>
> switch (type_from_irq(irq)) {
> case IRQT_VIRQ:
> @@ -1574,7 +1576,7 @@ int xen_set_irq_priority(unsigned irq, unsigned priority)
> }
> EXPORT_SYMBOL_GPL(xen_set_irq_priority);
>
> -int evtchn_make_refcounted(evtchn_port_t evtchn)
> +int evtchn_make_refcounted(evtchn_port_t evtchn, bool is_static)
> {
> int irq = get_evtchn_to_irq(evtchn);
> struct irq_info *info;
> @@ -1590,6 +1592,7 @@ int evtchn_make_refcounted(evtchn_port_t evtchn)
> WARN_ON(info->refcnt != -1);
>
> info->refcnt = 1;
> + info->is_static = is_static;
>
> return 0;
> }
> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> index c99415a70051..e6d2303478b2 100644
> --- a/drivers/xen/evtchn.c
> +++ b/drivers/xen/evtchn.c
> @@ -366,7 +366,8 @@ static int evtchn_resize_ring(struct per_user_data *u)
> return 0;
> }
>
> -static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
> +static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port,
> + bool is_static)
> {
> struct user_evtchn *evtchn;
> struct evtchn_close close;
> @@ -402,14 +403,16 @@ static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
> if (rc < 0)
> goto err;
>
> - rc = evtchn_make_refcounted(port);
> + rc = evtchn_make_refcounted(port, is_static);
> return rc;
>
> err:
> /* bind failed, should close the port now */
> - close.port = port;
> - if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> - BUG();
> + if (!is_static) {
I think now "struct evtchn_close close;" can be placed here as it is not
used outside of this block.
Also this block looks like an open-coded version of xen_evtchn_close()
defined at events_base.c, so maybe it is worth making xen_evtchn_close()
static inline and placing it into events.h, then calling helper here?
Please note, I will be ok either way.
> + close.port = port;
> + if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> + BUG();
> + }
> del_evtchn(u, evtchn);
> return rc;
> }
[snip]
Hi Oleksandr,
Thanks for reviewing the code.
> On 29 Jun 2023, at 7:06 pm, Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> wrote:
>
>
>
> On 29.06.23 18:46, Rahul Singh wrote:
>
> Hello Rahul
>
>
>> Xen 4.17 supports the creation of static evtchns. To allow user space
>> application to bind static evtchns introduce new ioctl
>> "IOCTL_EVTCHN_BIND_STATIC". Existing IOCTL doing more than binding
>> that’s why we need to introduce the new IOCTL to only bind the static
>> event channels.
>>
>> Also, static evtchns to be available for use during the lifetime of the
>> guest. When the application exits, __unbind_from_irq() ends up being
>> called from release() file operations because of that static evtchns
>> are getting closed. To avoid closing the static event channel, add the
>> new bool variable "is_static" in "struct irq_info" to mark the event
>> channel static when creating the event channel to avoid closing the
>> static evtchn.
>>
>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>> ---
>> v2:
>> * Use bool in place u8 to define is_static variable.
>> * Avoid closing the static evtchns in error path.
>
>
> Patch looks good to me, just a nit (question) below.
>
>
>> ---
>> drivers/xen/events/events_base.c | 7 +++++--
>> drivers/xen/evtchn.c | 30 ++++++++++++++++++++++--------
>> include/uapi/xen/evtchn.h | 9 +++++++++
>> include/xen/events.h | 2 +-
>> 4 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
>> index c7715f8bd452..5d3b5c7cfe64 100644
>> --- a/drivers/xen/events/events_base.c
>> +++ b/drivers/xen/events/events_base.c
>> @@ -112,6 +112,7 @@ struct irq_info {
>> unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
>> u64 eoi_time; /* Time in jiffies when to EOI. */
>> raw_spinlock_t lock;
>> + bool is_static; /* Is event channel static */
>>
>> union {
>> unsigned short virq;
>> @@ -982,7 +983,8 @@ static void __unbind_from_irq(unsigned int irq)
>> unsigned int cpu = cpu_from_irq(irq);
>> struct xenbus_device *dev;
>>
>> - xen_evtchn_close(evtchn);
>> + if (!info->is_static)
>> + xen_evtchn_close(evtchn);
>>
>> switch (type_from_irq(irq)) {
>> case IRQT_VIRQ:
>> @@ -1574,7 +1576,7 @@ int xen_set_irq_priority(unsigned irq, unsigned priority)
>> }
>> EXPORT_SYMBOL_GPL(xen_set_irq_priority);
>>
>> -int evtchn_make_refcounted(evtchn_port_t evtchn)
>> +int evtchn_make_refcounted(evtchn_port_t evtchn, bool is_static)
>> {
>> int irq = get_evtchn_to_irq(evtchn);
>> struct irq_info *info;
>> @@ -1590,6 +1592,7 @@ int evtchn_make_refcounted(evtchn_port_t evtchn)
>> WARN_ON(info->refcnt != -1);
>>
>> info->refcnt = 1;
>> + info->is_static = is_static;
>>
>> return 0;
>> }
>> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
>> index c99415a70051..e6d2303478b2 100644
>> --- a/drivers/xen/evtchn.c
>> +++ b/drivers/xen/evtchn.c
>> @@ -366,7 +366,8 @@ static int evtchn_resize_ring(struct per_user_data *u)
>> return 0;
>> }
>>
>> -static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
>> +static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port,
>> + bool is_static)
>> {
>> struct user_evtchn *evtchn;
>> struct evtchn_close close;
>> @@ -402,14 +403,16 @@ static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
>> if (rc < 0)
>> goto err;
>>
>> - rc = evtchn_make_refcounted(port);
>> + rc = evtchn_make_refcounted(port, is_static);
>> return rc;
>>
>> err:
>> /* bind failed, should close the port now */
>> - close.port = port;
>> - if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
>> - BUG();
>> + if (!is_static) {
>
>
> I think now "struct evtchn_close close;" can be placed here as it is not
> used outside of this block.
>
> Also this block looks like an open-coded version of xen_evtchn_close()
> defined at events_base.c, so maybe it is worth making xen_evtchn_close()
> static inline and placing it into events.h, then calling helper here?
> Please note, I will be ok either way.
Make sense. I will modify the patch as per your request in the next version.
I will wait for other maintainers to review the patch before sending the
next version.
Regards,
Rahul
On Fri, 30 Jun 2023, Rahul Singh wrote:
> Hi Oleksandr,
>
> Thanks for reviewing the code.
>
> > On 29 Jun 2023, at 7:06 pm, Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> wrote:
> >
> >
> >
> > On 29.06.23 18:46, Rahul Singh wrote:
> >
> > Hello Rahul
> >
> >
> >> Xen 4.17 supports the creation of static evtchns. To allow user space
> >> application to bind static evtchns introduce new ioctl
> >> "IOCTL_EVTCHN_BIND_STATIC". Existing IOCTL doing more than binding
> >> that’s why we need to introduce the new IOCTL to only bind the static
> >> event channels.
> >>
> >> Also, static evtchns to be available for use during the lifetime of the
> >> guest. When the application exits, __unbind_from_irq() ends up being
> >> called from release() file operations because of that static evtchns
> >> are getting closed. To avoid closing the static event channel, add the
> >> new bool variable "is_static" in "struct irq_info" to mark the event
> >> channel static when creating the event channel to avoid closing the
> >> static evtchn.
> >>
> >> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> >> ---
> >> v2:
> >> * Use bool in place u8 to define is_static variable.
> >> * Avoid closing the static evtchns in error path.
> >
> >
> > Patch looks good to me, just a nit (question) below.
> >
> >
> >> ---
> >> drivers/xen/events/events_base.c | 7 +++++--
> >> drivers/xen/evtchn.c | 30 ++++++++++++++++++++++--------
> >> include/uapi/xen/evtchn.h | 9 +++++++++
> >> include/xen/events.h | 2 +-
> >> 4 files changed, 37 insertions(+), 11 deletions(-)
> >>
> >> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> >> index c7715f8bd452..5d3b5c7cfe64 100644
> >> --- a/drivers/xen/events/events_base.c
> >> +++ b/drivers/xen/events/events_base.c
> >> @@ -112,6 +112,7 @@ struct irq_info {
> >> unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
> >> u64 eoi_time; /* Time in jiffies when to EOI. */
> >> raw_spinlock_t lock;
> >> + bool is_static; /* Is event channel static */
> >>
> >> union {
> >> unsigned short virq;
> >> @@ -982,7 +983,8 @@ static void __unbind_from_irq(unsigned int irq)
> >> unsigned int cpu = cpu_from_irq(irq);
> >> struct xenbus_device *dev;
> >>
> >> - xen_evtchn_close(evtchn);
> >> + if (!info->is_static)
> >> + xen_evtchn_close(evtchn);
> >>
> >> switch (type_from_irq(irq)) {
> >> case IRQT_VIRQ:
> >> @@ -1574,7 +1576,7 @@ int xen_set_irq_priority(unsigned irq, unsigned priority)
> >> }
> >> EXPORT_SYMBOL_GPL(xen_set_irq_priority);
> >>
> >> -int evtchn_make_refcounted(evtchn_port_t evtchn)
> >> +int evtchn_make_refcounted(evtchn_port_t evtchn, bool is_static)
> >> {
> >> int irq = get_evtchn_to_irq(evtchn);
> >> struct irq_info *info;
> >> @@ -1590,6 +1592,7 @@ int evtchn_make_refcounted(evtchn_port_t evtchn)
> >> WARN_ON(info->refcnt != -1);
> >>
> >> info->refcnt = 1;
> >> + info->is_static = is_static;
> >>
> >> return 0;
> >> }
> >> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
> >> index c99415a70051..e6d2303478b2 100644
> >> --- a/drivers/xen/evtchn.c
> >> +++ b/drivers/xen/evtchn.c
> >> @@ -366,7 +366,8 @@ static int evtchn_resize_ring(struct per_user_data *u)
> >> return 0;
> >> }
> >>
> >> -static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
> >> +static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port,
> >> + bool is_static)
> >> {
> >> struct user_evtchn *evtchn;
> >> struct evtchn_close close;
> >> @@ -402,14 +403,16 @@ static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
> >> if (rc < 0)
> >> goto err;
> >>
> >> - rc = evtchn_make_refcounted(port);
> >> + rc = evtchn_make_refcounted(port, is_static);
> >> return rc;
> >>
> >> err:
> >> /* bind failed, should close the port now */
> >> - close.port = port;
> >> - if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
> >> - BUG();
> >> + if (!is_static) {
> >
> >
> > I think now "struct evtchn_close close;" can be placed here as it is not
> > used outside of this block.
> >
> > Also this block looks like an open-coded version of xen_evtchn_close()
> > defined at events_base.c, so maybe it is worth making xen_evtchn_close()
> > static inline and placing it into events.h, then calling helper here?
> > Please note, I will be ok either way.
>
> Make sense. I will modify the patch as per your request in the next version.
> I will wait for other maintainers to review the patch before sending the
> next version.
I don't have any further comments.
Hi ,
> On 9 Jul 2023, at 1:10 am, Stefano Stabellini <sstabellini@kernel.org> wrote:
>
> On Fri, 30 Jun 2023, Rahul Singh wrote:
>> Hi Oleksandr,
>>
>> Thanks for reviewing the code.
>>
>>> On 29 Jun 2023, at 7:06 pm, Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com> wrote:
>>>
>>>
>>>
>>> On 29.06.23 18:46, Rahul Singh wrote:
>>>
>>> Hello Rahul
>>>
>>>
>>>> Xen 4.17 supports the creation of static evtchns. To allow user space
>>>> application to bind static evtchns introduce new ioctl
>>>> "IOCTL_EVTCHN_BIND_STATIC". Existing IOCTL doing more than binding
>>>> that’s why we need to introduce the new IOCTL to only bind the static
>>>> event channels.
>>>>
>>>> Also, static evtchns to be available for use during the lifetime of the
>>>> guest. When the application exits, __unbind_from_irq() ends up being
>>>> called from release() file operations because of that static evtchns
>>>> are getting closed. To avoid closing the static event channel, add the
>>>> new bool variable "is_static" in "struct irq_info" to mark the event
>>>> channel static when creating the event channel to avoid closing the
>>>> static evtchn.
>>>>
>>>> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
>>>> ---
>>>> v2:
>>>> * Use bool in place u8 to define is_static variable.
>>>> * Avoid closing the static evtchns in error path.
>>>
>>>
>>> Patch looks good to me, just a nit (question) below.
>>>
>>>
>>>> ---
>>>> drivers/xen/events/events_base.c | 7 +++++--
>>>> drivers/xen/evtchn.c | 30 ++++++++++++++++++++++--------
>>>> include/uapi/xen/evtchn.h | 9 +++++++++
>>>> include/xen/events.h | 2 +-
>>>> 4 files changed, 37 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
>>>> index c7715f8bd452..5d3b5c7cfe64 100644
>>>> --- a/drivers/xen/events/events_base.c
>>>> +++ b/drivers/xen/events/events_base.c
>>>> @@ -112,6 +112,7 @@ struct irq_info {
>>>> unsigned int irq_epoch; /* If eoi_cpu valid: irq_epoch of event */
>>>> u64 eoi_time; /* Time in jiffies when to EOI. */
>>>> raw_spinlock_t lock;
>>>> + bool is_static; /* Is event channel static */
>>>>
>>>> union {
>>>> unsigned short virq;
>>>> @@ -982,7 +983,8 @@ static void __unbind_from_irq(unsigned int irq)
>>>> unsigned int cpu = cpu_from_irq(irq);
>>>> struct xenbus_device *dev;
>>>>
>>>> - xen_evtchn_close(evtchn);
>>>> + if (!info->is_static)
>>>> + xen_evtchn_close(evtchn);
>>>>
>>>> switch (type_from_irq(irq)) {
>>>> case IRQT_VIRQ:
>>>> @@ -1574,7 +1576,7 @@ int xen_set_irq_priority(unsigned irq, unsigned priority)
>>>> }
>>>> EXPORT_SYMBOL_GPL(xen_set_irq_priority);
>>>>
>>>> -int evtchn_make_refcounted(evtchn_port_t evtchn)
>>>> +int evtchn_make_refcounted(evtchn_port_t evtchn, bool is_static)
>>>> {
>>>> int irq = get_evtchn_to_irq(evtchn);
>>>> struct irq_info *info;
>>>> @@ -1590,6 +1592,7 @@ int evtchn_make_refcounted(evtchn_port_t evtchn)
>>>> WARN_ON(info->refcnt != -1);
>>>>
>>>> info->refcnt = 1;
>>>> + info->is_static = is_static;
>>>>
>>>> return 0;
>>>> }
>>>> diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c
>>>> index c99415a70051..e6d2303478b2 100644
>>>> --- a/drivers/xen/evtchn.c
>>>> +++ b/drivers/xen/evtchn.c
>>>> @@ -366,7 +366,8 @@ static int evtchn_resize_ring(struct per_user_data *u)
>>>> return 0;
>>>> }
>>>>
>>>> -static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
>>>> +static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port,
>>>> + bool is_static)
>>>> {
>>>> struct user_evtchn *evtchn;
>>>> struct evtchn_close close;
>>>> @@ -402,14 +403,16 @@ static int evtchn_bind_to_user(struct per_user_data *u, evtchn_port_t port)
>>>> if (rc < 0)
>>>> goto err;
>>>>
>>>> - rc = evtchn_make_refcounted(port);
>>>> + rc = evtchn_make_refcounted(port, is_static);
>>>> return rc;
>>>>
>>>> err:
>>>> /* bind failed, should close the port now */
>>>> - close.port = port;
>>>> - if (HYPERVISOR_event_channel_op(EVTCHNOP_close, &close) != 0)
>>>> - BUG();
>>>> + if (!is_static) {
>>>
>>>
>>> I think now "struct evtchn_close close;" can be placed here as it is not
>>> used outside of this block.
>>>
>>> Also this block looks like an open-coded version of xen_evtchn_close()
>>> defined at events_base.c, so maybe it is worth making xen_evtchn_close()
>>> static inline and placing it into events.h, then calling helper here?
>>> Please note, I will be ok either way.
>>
>> Make sense. I will modify the patch as per your request in the next version.
>> I will wait for other maintainers to review the patch before sending the
>> next version.
>
> I don't have any further comments.
Thanks for the update. I will send the next version.
Regards,
Rahul
© 2016 - 2024 Red Hat, Inc.