Re: [RFC V2] xen: interface: introduce pvclk interface

[Peng Fan <van.freenix@gmail.com>]

Hi Jan,
On Wed, Jan 20, 2016 at 05:01:40AM -0700, Jan Beulich wrote:
>>>> On 20.01.16 at 12:40, <van.freenix@gmail.com> wrote:
>> 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 (:
>
>The layout of above structure may end up different for 32- and
>64-bit guests, depending on the alignment of uint64_t.

Thanks for teaching me. In V3, the layout will be changed to like this
struct xen_clkif_response {
	uint32_t request_id;
	int32_t status;
	uint64_t rate;
};

And more macro definitions for the status entry:
#define XEN_CLK_OP_SUCCESS 0
#define XEN_CLK_ENABLE_FALIURE -1
#define XEN_CLK_DISABLE_FAILURE -2
#define XEN_CLK_PREPARE_FAILURE -3
#define XEN_CLK_UNPREPARE_FAILURE -4
#define XEN_CLK_SET_RATE_FAILURE -5
#define XEN_CLK_GET_RATE_FALIURE -6

>
>>>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.
>
>No, sorry, that's not the point of the inquiry. It seems to me that
>there are more aspects to this interface, not directly related to
>the request/reply protocol written down here, which need to be
>spelled out event if they don't require any particular definitions
>or type declarations.

I try to follow you about clock events(interrupts?), not sure whether I understand correct or not.
clock in this file is the clk for a device. In linux side, it managed by clk subsystem, drivers/clk/xx.
This is not the clock events or clock source in drivers/clocksource/xx.
For the pvclk interface, there will be no interrupt for the guest.

I'll add more texts in the file or commit log.

Thanks,
Peng.

>
>Jan
>