| Message ID | 2f9544433fd9bb5c4b7ccccbacc27bc928f57dfb.1714148012.git.matthew.barnes@cloud.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enumerate all allocated evtchns in lsevtchn | expand |
On 29.04.2024 15:42, Matthew Barnes wrote: > When the evtchn_status hypercall fails, it is not possible to determine > the cause of the error, since the library call returns -1 to the tool > and not the errno. That's normal behavior for such library functions. If you want to know the specific error number, you ought to look at the errno variable. Or are you saying that errno isn't set correctly in this case (I can't spot such an issue when looking at do_evtchn_op(), backing xc_evtchn_status())? > --- a/xen/common/event_channel.c > +++ b/xen/common/event_channel.c > @@ -1030,7 +1030,17 @@ int evtchn_status(evtchn_status_t *status) > > d = rcu_lock_domain_by_any_id(dom); > if ( d == NULL ) > - return -ESRCH; > + { > + status->status = EVTCHNSTAT_dom_invalid; > + return 0; This surely ought to remain -ESRCH. You may not break existing callers. > + } > + > + if ( !port_is_valid(d, port) ) > + { > + status->status = EVTCHNSTAT_port_invalid; > + rcu_unlock_domain(d); > + return 0; > + } I can see that for the purpose of patch 2 this wants distinguishing from > chn = _evtchn_from_port(d, port); > if ( !chn ) ... the -EINVAL returned here. Yet "success" doesn't look correct there either. -ENOENT, -EBADF, -ENFILE, or -EDOM maybe? > --- a/xen/include/public/event_channel.h > +++ b/xen/include/public/event_channel.h > @@ -200,6 +200,8 @@ struct evtchn_status { > #define EVTCHNSTAT_pirq 3 /* Channel is bound to a phys IRQ line. */ > #define EVTCHNSTAT_virq 4 /* Channel is bound to a virtual IRQ line */ > #define EVTCHNSTAT_ipi 5 /* Channel is bound to a virtual IPI line */ > +#define EVTCHNSTAT_dom_invalid 6 /* Given domain ID is not a valid domain */ > +#define EVTCHNSTAT_port_invalid 7 /* Given port is not within valid range */ > uint32_t status; > uint32_t vcpu; /* VCPU to which this channel is bound. */ > union { If such indicators are to be added, I'm pretty sure they want to be discontiguous from the presently used range. Sadly, with status having unsigned type, using negative values wouldn't feel quite right. Jan
diff --git a/xen/common/event_channel.c b/xen/common/event_channel.c index aceee0695f9f..0f11e71c3e6f 100644 --- a/xen/common/event_channel.c +++ b/xen/common/event_channel.c @@ -1030,7 +1030,17 @@ int evtchn_status(evtchn_status_t *status) d = rcu_lock_domain_by_any_id(dom); if ( d == NULL ) - return -ESRCH; + { + status->status = EVTCHNSTAT_dom_invalid; + return 0; + } + + if ( !port_is_valid(d, port) ) + { + status->status = EVTCHNSTAT_port_invalid; + rcu_unlock_domain(d); + return 0; + } chn = _evtchn_from_port(d, port); if ( !chn ) diff --git a/xen/include/public/event_channel.h b/xen/include/public/event_channel.h index 0d91a1c4afab..29cbf43945b3 100644 --- a/xen/include/public/event_channel.h +++ b/xen/include/public/event_channel.h @@ -200,6 +200,8 @@ struct evtchn_status { #define EVTCHNSTAT_pirq 3 /* Channel is bound to a phys IRQ line. */ #define EVTCHNSTAT_virq 4 /* Channel is bound to a virtual IRQ line */ #define EVTCHNSTAT_ipi 5 /* Channel is bound to a virtual IPI line */ +#define EVTCHNSTAT_dom_invalid 6 /* Given domain ID is not a valid domain */ +#define EVTCHNSTAT_port_invalid 7 /* Given port is not within valid range */ uint32_t status; uint32_t vcpu; /* VCPU to which this channel is bound. */ union {
When the evtchn_status hypercall fails, it is not possible to determine the cause of the error, since the library call returns -1 to the tool and not the errno. Because of this, lsevtchn is unable to determine whether to continue event channel enumeration upon an evtchn_status hypercall error. Add error status indicators for the eventchn_status hypercall for lsevtchn to terminate its loop on, and keep other errors as failed hypercalls. Signed-off-by: Matthew Barnes <matthew.barnes@cloud.com> --- xen/common/event_channel.c | 12 +++++++++++- xen/include/public/event_channel.h | 2 ++ 2 files changed, 13 insertions(+), 1 deletion(-)