All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peng Fan <van.freenix@gmail.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Ian Campbell <ian.campbell@citrix.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	George Dunlap <george.dunlap@citrix.com>,
	Julien Grall <julien.grall@citrix.com>,
	David Vrabel <david.vrabel@citrix.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [RFC V2] xen: interface: introduce pvclk interface
Date: Wed, 20 Jan 2016 19:40:56 +0800	[thread overview]
Message-ID: <20160120114054.GA9856@linux-7smt.suse> (raw)
In-Reply-To: <569F6C0C02000078000C8F2C@prv-mh.provo.novell.com>

Hi Jan,

On Wed, Jan 20, 2016 at 03:14:20AM -0700, Jan Beulich wrote:
>>>> On 20.01.16 at 09:31, <van.freenix@gmail.com> wrote:
>> +/*
>> + * Backend response
>> + *
>> + * cmd: command for operation on clk, same with the cmd in request.
>> + * id: clk id, same with the id in request.
>> + * success: indicate failure or success for the cmd.
>> + * rate: clock rate. Used for get rate.
>> + *
>> + * cmd and id are filled by backend and passed to frontend to
>> + * let frontend check whether the response is for the current
>> + * request or not.
>> + */
>> +struct xen_clkif_response {
>> +	uint32_t cmd;
>> +	uint32_t id;
>> +	uint32_t success;
>> +	uint64_t rate;
>> +};
>
>This isn't 32-/64-bit clean. Question is whether echoing cmd is really
>needed.

As Juergen suggested, use a request id. I'll fix this in V3.
32-/64-bit unclean, I can not get you here (:

>
>Also naming a field "success" is pretty odd - is this mean to be a
>boolean? Better name it e.g. status, allowing for different (error)
>indicators.

As you suggested, how about `int status`? And in this clkif.h,
define different status value, such as `#define XEN_CLK_PREPARE_OK 0,
#define XEN_CLK_PREPARE_FAILURE -1`. The frontend and backend should
be aware of the status value.

>
>And what I'm missing throughout the file is some description of how
>clock events (interrupts?) are actually meant to make it into the
>guest.

I have a simple implementation v1 patch for linux, http://lists.xen.org/archives/html/xen-devel/2016-01/msg01860.html.
You can see how it works.

Thanks,
Peng.

>
>Jan
>

  reply	other threads:[~2016-01-20 11:41 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-20  8:31 [RFC V2] xen: interface: introduce pvclk interface Peng Fan
2016-01-20  9:05 ` Juergen Gross
2016-01-20  9:25   ` Peng Fan
2016-01-20 10:16     ` Jan Beulich
2016-01-20 10:40     ` Juergen Gross
2016-01-20 11:48       ` Peng Fan
2016-01-20 12:11         ` Juergen Gross
2016-01-20 14:13           ` Peng Fan
2016-01-20 10:14 ` Jan Beulich
2016-01-20 11:40   ` Peng Fan [this message]
2016-01-20 12:01     ` Jan Beulich
2016-01-20 14:05       ` Peng Fan
2016-01-20 14:16         ` Jan Beulich
2016-01-20 14:37           ` Peng Fan
2016-01-20 14:49             ` Ian Campbell
2016-01-20 14:52             ` Jan Beulich
2016-01-21  1:29               ` Peng Fan
2016-01-21  7:53                 ` Jan Beulich
2016-01-21  8:59                   ` Peng Fan
2016-01-21 10:19                     ` Ian Campbell
2016-01-21 11:55                       ` Peng Fan
2016-01-21 12:26                         ` Ian Campbell
2016-01-21 12:35                           ` Peng Fan
2016-01-21 12:49                             ` Ian Campbell
2016-01-22  2:19                               ` Peng Fan
2016-01-23 15:26                               ` Peng Fan
2016-01-21 12:55                             ` Stefano Stabellini
2016-01-21 13:11                               ` Ian Campbell
2016-01-21 16:11                                 ` Stefano Stabellini
2016-01-22  2:51                                   ` Peng Fan
2016-01-21 10:21                     ` Jan Beulich
2016-01-21 12:06                       ` Peng Fan
2016-01-21 12:52                         ` Jan Beulich
2016-01-22  1:56                           ` Peng Fan
2016-01-22  7:36                             ` Jan Beulich
2016-01-22  9:27                               ` Peng Fan
2016-01-22 10:25                                 ` Jan Beulich
2016-01-22 12:12                                   ` Peng Fan
2016-01-22 12:33                                     ` Jan Beulich
2016-01-22 13:55                                       ` Stefano Stabellini
2016-01-20 12:06 ` Stefano Stabellini
2016-01-20 12:27   ` Ian Campbell
2016-01-20 13:52     ` Peng Fan

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=20160120114054.GA9856@linux-7smt.suse \
    --to=van.freenix@gmail.com \
    --cc=JBeulich@suse.com \
    --cc=david.vrabel@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=julien.grall@citrix.com \
    --cc=stefano.stabellini@eu.citrix.com \
    --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 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.