All of lore.kernel.org
 help / color / mirror / Atom feed
From: Wei Liu <wei.liu2@citrix.com>
To: David Vrabel <david.vrabel@citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	wei.liu2@citrix.com
Subject: Re: FIFO-based event channel ABI design (draft B)
Date: Fri, 15 Feb 2013 18:19:38 +0000	[thread overview]
Message-ID: <1360952378.16636.195.camel@zion.uk.xensource.com> (raw)
In-Reply-To: <511E46FD.3010605@citrix.com>

Apart from the TAIL field, I have some comments on the hypercall
interface. Please see inline comments.

On Fri, 2013-02-15 at 14:32 +0000, David Vrabel wrote:
> 
> Hypercalls
> ----------
> 
> Four new EVTCHNOP hypercall sub-operations are added:
> 
> * `EVTCHNOP_init_control`
> 
> * `EVTCHNOP_expand_array`
> 
> * `EVTCHNOP_set_priority`
> 
> * `EVTCHNOP_set_limit`
> 
> ### `EVTCHNOP_init_control`
> 
> This call initializes a single VCPU's control block.
> 
> A guest should call this during initial VCPU bring up.  The guest must
> have already successfully registered a vcpu_info structure and the
> control block must be in the same page.
> 
> If this call fails on the boot VCPU, the guest should continue to use
> the 2-level event channel ABI for all VCPUs.  If this call fails on
> any non-boot VCPU then the VCPU will be unable to receive events and
> the guest should offline the VCPU.
> 

Why offline this CPU? This call only registers control block, we can
switch back to use 2-level for all CPUs.

This is not a right or wrong question, but consider, how many vcpus will
a guest have? If the number is small, user would probably choose to have
all their cpus online over the new event interface.

> > Note: This only initializes the control block. At least one page
> > needs to be added to the event arrary with `EVTCHNOP_expand_array`.
> 
>     struct evtchnop_init_control {
>         uint64_t control_pfn;
>         uint32_t offset;
>         uint32_t vcpu;
>     };
> 
> Field          Purpose
> -----          -------
> `control_pfn`  [in] The PFN or GMFN of the page containing the control
> block.
> `offset`       [in] Offset in bytes from the start of the page to the
> beginning of the control block.
> `vcpu`         [in] The VCPU number.
> 
> Error code  Reason
> ----------  ------
> EINVAL      `vcpu` is invalid or already initialized.
> EINVAL      `control_pfn` is not a valid frame for the domain.
> EINVAL      `control_pfn` is not the same frame as the vcpu_info structure.

Hmm... then you can omit control_pfn entirely?

> EINVAL      `offset` is not a multiple of 8 or the control block would
> cross a page boundary.
> ENOMEM      Insufficient memory to allocate internal structures.
> 
> ### `EVTCHNOP_expand_array`
> 
> This call expands the event array by appending an additional page.
> 

A bit implementation detail:

What state should guest / hypervisor be in when issuing this call?

> A guest should call this when a new event channel is required and
> there is insufficient space in the current event array.
> 
> It is not possible to shrink the event array once it has been
> expanded.
> 
> If this call fails, then subsequent attempts to bind event channels
> may fail with -ENOSPC.  If the first page cannot be added then the
> guest cannot receive any events and it should panic.
> 
>     struct evtchnop_expand_array {
>         uint64_t array_pfn;
>     };
> 
> Field          Purpose
> -----          -------
> `array_pfn`    [in] The PFN or GMFN of a page to be used for the next page
>                of the event array.
> 
> Error code  Reason
> ----------  ------
> EINVAL      `array_pfn` is not a valid frame for the domain.
> EINVAL      The event array already has the maximum number of pages.
> ENOMEM      Insufficient memory to allocate internal structures.
[snip]
> 
> ### `EVTCHNOP_set_limit`
> 
> This privileged call sets the highest port number a domain can bind an
> event channel to.  The default for dom0 is the maximum supported
> ($2^{17}-1$).  Other domains default to 1023 (requiring only a single
> page for their event array).
> 
> The limit only affects future attempts to bind event channels.  Event
> channels that are already bound are not affected.
> 
> It is recommended that the toolstack only calls this during domain
> creation before the guest is started.
> 
>     struct evtchnop_set_limit {
>         uint32_t domid;
>         uint32_t max_port;
>     };
> 
> Field       Purpose
> -----       -------
> `domid`     [in] The domain ID.
> `max_port`  [in] The highest port number that the domain may bound an
> event channel to.
> 
> Error code  Reason
> ----------  ------
> EINVAL      `domid` is invalid.
> EPERM       The calling domain has insufficient privileges.
> 

Is it OK to shrink port limit?

> 
> Upcall
> ------
> 
> When Xen places an event on an empty queue it sets the queue as ready
> in the control block.  If the ready bit transitions from 0 to 1, a new
> event is signalled to the guest.
> 
> The guest uses the control block's ready field to find the highest
> priority queue with pending events.  The bit in the ready word is
> cleared when the queue is emptied.
> 
> Higher priority events do not need to preempt lower priority event
> handlers so the guest can handle events by taking one event off the
> currently ready queue with highest priority.
> 
>     r = C.ready
>     while r
>         q = find_first_set_bit(r)
>         consume_one_event_from_queue(q)
>         if C.queue[q].H == 0
>             C.ready[q] = 0
>             r[b] = 0

What is b? Could be clear_bit(r,q)?


Wei.

  parent reply	other threads:[~2013-02-15 18:19 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-15 14:32 FIFO-based event channel ABI design (draft B) David Vrabel
2013-02-15 15:05 ` Jan Beulich
2013-02-15 15:17   ` David Vrabel
2013-02-15 15:36     ` Jan Beulich
2013-02-15 18:19 ` Wei Liu [this message]
2013-02-15 18:46   ` David Vrabel
2013-02-15 19:04     ` Wei Liu
2013-02-18  8:10     ` Jan Beulich
2013-02-18 15:19 ` Ian Campbell
2013-02-18 20:16   ` David Vrabel
2013-02-18 20:50     ` Wei Liu
2013-02-18 20:21   ` Keir Fraser
2013-02-19 18:39 ` Ian Jackson

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=1360952378.16636.195.camel@zion.uk.xensource.com \
    --to=wei.liu2@citrix.com \
    --cc=david.vrabel@citrix.com \
    --cc=xen-devel@lists.xensource.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.