From: Jan Beulich <jbeulich@suse.com>
To: Matthew Barnes <matthew.barnes@cloud.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
George Dunlap <george.dunlap@citrix.com>,
Julien Grall <julien@xen.org>,
Stefano Stabellini <sstabellini@kernel.org>,
xen-devel@lists.xenproject.org
Subject: Re: [XEN PATCH v2 1/2] evtchn: Add error status indicators for evtchn_status hypercall
Date: Tue, 30 Apr 2024 14:19:38 +0200 [thread overview]
Message-ID: <249214ee-9c2d-4f12-8af5-0779493323b1@suse.com> (raw)
In-Reply-To: <2f9544433fd9bb5c4b7ccccbacc27bc928f57dfb.1714148012.git.matthew.barnes@cloud.com>
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
next prev parent reply other threads:[~2024-04-30 12:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 13:42 [XEN PATCH v2 0/2] Enumerate all allocated evtchns in lsevtchn Matthew Barnes
2024-04-29 13:42 ` [XEN PATCH v2 1/2] evtchn: Add error status indicators for evtchn_status hypercall Matthew Barnes
2024-04-30 12:19 ` Jan Beulich [this message]
2024-04-29 13:42 ` [XEN PATCH v2 2/2] tools/lsevtchn: Use new status identifiers in loop Matthew Barnes
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=249214ee-9c2d-4f12-8af5-0779493323b1@suse.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=george.dunlap@citrix.com \
--cc=julien@xen.org \
--cc=matthew.barnes@cloud.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).