This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] [PATCH 1/2] Virtual frame buffer: frontend

To: "Markus Armbruster" <armbru@xxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH 1/2] Virtual frame buffer: frontend
From: "Christian Limpach" <christian.limpach@xxxxxxxxx>
Date: Fri, 7 Jul 2006 17:45:30 +0100
Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Fri, 07 Jul 2006 09:45:58 -0700
Domainkey-signature: a=rsa-sha1; q=dns; c=nofws; s=beta; d=gmail.com; h=received:message-id:date:from:reply-to:to:subject:cc:in-reply-to:mime-version:content-type:content-transfer-encoding:content-disposition:references; b=NoOoR6khX5xRYDoMJQVnqrhxFzR/kdDQuMqzUqQYFamyu8WFC9pklg+2UIvuDEcQXO1lKR6D6mK9jFVoudAwjW5vrqyJUYR3qrTkolVFe3fr7q1oHEPyIJUeVX3FOA03ISdqpvYUD/FfOjPfb4iHDJZlc8Y60gk2FR6fCarLiXI=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <8764ioghnn.fsf@xxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <87ac80ghox.fsf@xxxxxxxxxxxxxxxxx> <8764ioghnn.fsf@xxxxxxxxxxxxxxxxx>
Reply-to: Christian.Limpach@xxxxxxxxxxxx
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
On 6/26/06, Markus Armbruster <armbru@xxxxxxxxxx> wrote:
Derived from

Converted to Xenstore and ported to current kernels.

This looks good.

Could you address the following issues:

- the patch which defined struct xenfb_page seems to be missing

- handling of the ring indexes:
 + we generally prefer to store ring indexes in their unmasked form,
it makes debugging easier and it allows the ring to be used to its
full capacity instead of requiring leaving one slot empty to be able
distinguish empty/full
 + unless I misread the code, the producer (for frontend to backend
messages) seems to write to out_prod + 1 > out_cons, then increment
out_prob (xenfb_do_update) and then the consumer processes entries
from out_cons upto but not include the updated out_prod
(xenfb_on_fb_event) -- the producer for the other way around (backend
to frontend, xenfb_fb_event) seems to be correcter.
 + the lack of barriers -- /* FIXME barriers */ doesn't really cut it ;-)
==> I would suggest looking at the block or net frontend/backend
drivers and copy/pasting some code from there...

- xenbus transactions can fail and there's no code to retry failed transactions

Additionally, I think that the dirty region protocol doesn't really
perform too well in quite simple usage cases like having video play in
one corner of the screen and the mouse being moved in the opposite
corner.  It's probably good enough for this version and supporting
this protocol in the future isn't too bad.

I'm looking forward to seeing a new version of this patch, thanks!


Xen-devel mailing list