[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3] displif: add ABI for para-virtual display

On 02/17/2017 06:33 PM, Konrad Rzeszutek Wilk wrote:
On Thu, Feb 16, 2017 at 10:36:01AM +0200, Oleksandr Andrushchenko wrote:
On 02/15/2017 11:33 PM, Konrad Rzeszutek Wilk wrote:
I will define 2 sections:
     *------------------ Connector Request Transport Parameters
     * ctrl-event-channel
     * ctrl-ring-ref
     *------------------- Connector Event Transport Parameters
     * event-channel
     * event-ring-ref

Or is the other ring buffer the one that is created via 'gref_directory' ?
At the bottom:
     * In order to deliver asynchronous events from back to front a shared page
     * allocated by front and its gref propagated to back via XenStore entries
     * (event-XXX).
AAnd you may want to say this is guarded by REQ_ALLOC feature right?
Not sure I understood you. Event path is totally independent
from any feature, e.g. REQ_ALLOC.
It just provides means to send async events
from back to front, "page flip done" in my case.
<scratche his head> Why do you need a seperate ring to send
responses back? Why not use the same ring on which requests
were sent
Ok, it seems we are not on the same page for rings/channels usage.
Let me describe how those are used:

1. Command/control event channel and its corresponding ring are used
to pass requests from front to back (XENDISPL_OP_XXX) and get responses
from the back. These are synchronous, use macros from ring.h:
ctrl-event-channel + ctrl-ring-ref
I call them "ctrl-" because this way front controls back, or sends commands
if you will. Maybe "cmd-" would fit better here?

2. Event channel - asynchronous path for the backend to signal activity
to the frontend, currently used for "page flip done" event which is sent
at some point of time after back has actually completed the page flip
(so, before that the corresponding request was sent and response received,
operation didn't complete yet, instead it was scheduled)
No macros exist for this use-case in ring.h (kbdif+fbif implement
this on their own, so do I)
These are:  event-channel + event-ring-ref
Probably here is the point from where confusion comes, naming.
We can have something like "be-to-fe-event-channel" or anything else
more cute and descriptive.

Hope this explains the need for 2 paths

So this is like the network where there is an 'rx' and 'tx'!
kind of
Now I get it.
sorry, I was probably not clear
In that case why not just prefix it with 'in' and 'out'? Such as:

'out-ring-ref' and 'out-event-channel' and 'in-ring-ref' along
with 'in-event-channel'.
hmmmm, it may confuse, because you must know "out"
from which POV, e.g. frontend's or backend's.
What is more, these "out-" and "in-" are... nameless?
Yes :-)

Can we still have something like "ctrl-"/"cmd-"/"req-"
for the req/resp path and probably "evt-" for
events from back to front?
I like the 'req' and 'evt-' part. That makes it more
clear I think.  But see below.

Or perhaps better - borrow the same idea that Stefano came up for
9pfs and PV calls - where his ring does both.

Then you just need 'ring-ref', 'event-channel', 'max-page-ring-order'
(which must be 1 or larger).

And you split the ring-ref in two - one for 'in' events and the other
part for 'out' events?
yes, I saw current implementations (kbdif, fbif) and
what Stefano did, but would rather stick to what is currently
defined (I believe it is optimal as is)
Right, but with two protocols with that way ... perhaps you
could re-use some of what Stefano wrote? See below.

And hope, that maybe someone will put new functionality into ring.h
to serve async events one day :)
That would be fantastic. And knowing Stefano I would think
he has alreadhy done so? (Or at least have an prototype patch
for this?)

Re-using his mechanism in your driver means easier maintaince
as things are kind of using the same thing.

Stefano, you wouldn't have an patch for the ring.h somewhere
would you?
yes, if Stefano already has it, then I can re-use it
Actually, going kbdif/fbif way was just because nothing is in ring.h for that
.. snip..
I am attaching the diff between v3 and v4 for your convenience
I am specifically interested in the recovery section - does it look ok?
So, I can put the same into sndif
Thank you,

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.