Re: [For Linux 4/4] xen/displif: add ABI for para-virtual display

From: Juergen Gross <jgross@suse.com>
To: Oleksandr Andrushchenko <andr2000@gmail.com>,
	Julien Grall <julien.grall@arm.com>,
	xen-devel@lists.xenproject.org
Cc: joculator@gmail.com, al1img@gmail.com, vlad.babchuk@gmail.com,
	andrii.anisov@gmail.com, olekstysh@gmail.com,
	konrad.wilk@oracle.com, lars.kurth@citrix.com,
	sstabellini@kernel.org, linux-kernel@vger.kernel.org,
	Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>,
	Oleksandr Grytsov <oleksandr_grytsov@epam.com>
Subject: Re: [For Linux 4/4] xen/displif: add ABI for para-virtual display
Date: Mon, 10 Apr 2017 08:07:39 +0200	[thread overview]
Message-ID: <b301470c-46cf-b51e-1427-365fd5ee7cee@suse.com> (raw)
In-Reply-To: <6f8fae9b-82fa-ec98-e381-25e60733fe22@gmail.com>

On 10/04/17 08:06, Oleksandr Andrushchenko wrote:
> On 04/10/2017 09:03 AM, Juergen Gross wrote:
>> On 07/04/17 16:02, Oleksandr Andrushchenko wrote:
>>> Hi, Julien!
>>>
>>> On 04/07/2017 04:50 PM, Julien Grall wrote:
>>>> Hi Oleksandr,
>>>>
>>>> On 07/04/17 09:30, Oleksandr Andrushchenko wrote:
>>>>> +/*
>>>>> +
>>>>> ******************************************************************************
>>>>>
>>>>>
>>>>> + *                        Back to front events delivery
>>>>> +
>>>>> ******************************************************************************
>>>>>
>>>>>
>>>>> + * In order to deliver asynchronous events from back to front a
>>>>> shared page is
>>>>> + * allocated by front and its granted reference propagated to back
>>>>> via
>>>>> + * XenStore entries (evt-ring-ref/evt-event-channel).
>>>>> + * This page has a common header used by both front and back to
>>>>> synchronize
>>>>> + * access and control event's ring buffer, while back being a
>>>>> producer of the
>>>>> + * events and front being a consumer. The rest of the page after the
>>>>> header
>>>>> + * is used for event packets.
>>>>> + *
>>>>> + * Upon reception of an event(s) front may confirm its reception
>>>>> + * for either each event, group of events or none.
>>>>> + */
>>>>> +
>>>>> +struct xendispl_event_page {
>>>>> +    uint32_t in_cons;
>>>>> +    uint32_t in_prod;
>>>>> +    uint8_t reserved[56];
>>>>> +};
>>>>> +
>>>>> +#define XENDISPL_EVENT_PAGE_SIZE 4096
>>>> This will be always the size of a grant (e.g xen page size), right? If
>>>> so, I would prefer if you use XEN_PAGE_SIZE to so we can easily update
>>>> Linux in the case Xen is using a different page size.
>>>>
>>>> Note that the Front-end, Backend and Xen may use different page size,
>>>> so your drivers would need to cope with that. The current approach is
>>>> to always use the page granularity of Xen.
>>>>
>>> Will the following satisfy the requirement?
>>> #define XENDISPL_EVENT_PAGE_SIZE XEN_PAGE_SIZE
>>>
>>>>> +#define XENDISPL_IN_RING_OFFS (sizeof(struct xendispl_event_page))
>>>>> +#define XENDISPL_IN_RING_SIZE (XENDISPL_EVENT_PAGE_SIZE -
>>>>> XENDISPL_IN_RING_OFFS)
>>>>> +#define XENDISPL_IN_RING_LEN (XENDISPL_IN_RING_SIZE / sizeof(struct
>>>>> xendispl_evt))
>>>>> +#define XENDISPL_IN_RING(page) \
>>>>> +    ((struct xendispl_evt *)((char *)(page) + XENDISPL_IN_RING_OFFS))
>>>>> +#define XENDISPL_IN_RING_REF(page, idx) \
>>>>> +    (XENDISPL_IN_RING((page))[(idx) % XENDISPL_IN_RING_LEN])
>>>>> +
>>>>> +#endif /* __XEN_PUBLIC_IO_DISPLIF_H__ */
>>>>> +
>>>>> +/*
>>>>> + * Local variables:
>>>>> + * mode: C
>>>>> + * c-file-style: "BSD"
>>>>> + * c-basic-offset: 4
>>>>> + * tab-width: 4
>>>>> + * indent-tabs-mode: nil
>>>>> + * End:
>>>>> + */
>> I believe you'll send V2 due to the XEN_PAGE_SIZE usage?
> yes
>>
>> Can you then please remove the editor mode settings block in each
>> header, too? Those are disliked in the Linux tree.
> sure, I will
>>
>>
>> Juergen
> Do you mind if I keep your acks for V2 with these changes?

You can keep them, of course.

Juergen