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


[Xen-devel] Re: [patch 00/26] Xen-paravirt_ops: Xen guest implementation

To: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Subject: [Xen-devel] Re: [patch 00/26] Xen-paravirt_ops: Xen guest implementation for paravirt_ops interface
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Fri, 16 Mar 2007 18:59:23 +0000
Cc: Zachary Amsden <zach@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxx, Andi Kleen <ak@xxxxxx>, Rusty Russell <rusty@xxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, Chris Wright <chrisw@xxxxxxxxxxxx>, virtualization@xxxxxxxxxxxxxx, Ingo Molnar <mingo@xxxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 19 Mar 2007 03:46:48 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <45FAD35F.8040404@xxxxxxxx>
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>
Mail-followup-to: Christoph Hellwig <hch@xxxxxxxxxxxxx>, Jeremy Fitzhardinge <jeremy@xxxxxxxx>, Ingo Molnar <mingo@xxxxxxx>, Andi Kleen <ak@xxxxxx>, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>, linux-kernel@xxxxxxxxxxxxxxx, virtualization@xxxxxxxxxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxx, Chris Wright <chrisw@xxxxxxxxxxxx>, Zachary Amsden <zach@xxxxxxxxxx>, Rusty Russell <rusty@xxxxxxxxxxxxxxx>
References: <20070301232443.195603797@xxxxxxxx> <20070316092137.GL23174@xxxxxxx> <45FAD35F.8040404@xxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mutt/
On Fri, Mar 16, 2007 at 10:26:55AM -0700, Jeremy Fitzhardinge wrote:
> +     .kmap_atomic_pte = native_kmap_atomic_pte,
> +#else
> +     .kmap_atomic_pte = paravirt_nop,
> +#endif

This is ifdefing is quite ugly.  Shouldn't native_kmap_atomic_pte
just be a noop in the !CONFIG_HIGHPTE case?

> -void *kmap_atomic(struct page *page, enum km_type type)
> +void *_kmap_atomic(struct page *page, enum km_type type, pgprot_t prot)

We normally call our "secial" function __foo, not _foo.  But in this
case it really should have a more meaningfull name like
kmap_atomic_prot anyway.

> +void *kmap_atomic(struct page *page, enum km_type type)
> +{
> +     return _kmap_atomic(page, type, kmap_prot);

And this one should probably be an inline.

> +static inline void *native_kmap_atomic_pte(struct page *page, enum km_type 
> type)
> +{
> +     return kmap_atomic(page, type);
> +}
> +
> +#define kmap_atomic_pte(page, type)  kmap_atomic(page, type)
> +#endif

This is all getting rather ugly just for your pagetable hackery.

Xen-devel mailing list