> Hi,
>
> Below is my very first attempt to write inline assembler, for atomic
> updates of PAE pagetable entries using cmpxchg8b. It builds, but is
> completely untested otherwise as I don't have a PAE-enabled guest kernel
> yet ...
>
> Can someone with more experience than me please have a look at it?
Thanks - I've checked in an 8-byte extension to cmpxchg_user() for
32-bit builds. Differences from yours include:
1. The "A" register specifier is used to match a 64-bit C variable
with EDX:EAX. This is neater than manually splitting into "a"
and "d" constraints, then manually recombining at the end.
Unfortunately there is no such shortcut for ECX:EBX.
2. n_hi, n_lo and ptr are not output constraints, so they belong in
the second constraint list, with no "=" in the specifier string.
3. I don't like the non-positional argument specifiers (e.g., [rc],
[ptr]) very much. Maybe worthwhile for longer code fragments but
for very short ones I'd rather have less clutter in the constraint
lists.
4. No need to return non-zero if the cmpxchg fails. The return code
indicates only whether the cmpxchg access faulted or not: it is up
to the caller to compare the value seen with what they expected.
This gets rid of a few lines in your cmpxchg8b_user.
I think that covers it. :-)
-- Keir
>
> Thanks,
>
> Gerd
>
> --- xen.orig/arch/x86/mm.c 2005-04-19 11:56:20.000000000 +0200
> +++ xen/arch/x86/mm.c 2005-04-19 15:29:01.000000000 +0200
> @@ -834,12 +846,55 @@ static void free_l4_table(struct pfn_inf
>
> #endif /* __x86_64__ */
>
> +static inline int cmpxchg8b_user(u64 *ptr, u64 oval, u64 nval)
> +{
> + u32 o_hi = oval >> 32;
> + u32 o_lo = oval & 0xffffffff;
> + u32 n_hi = nval >> 32;
> + u32 n_lo = nval & 0xffffffff;
> + u32 rc = 0;
> +
> + __asm__ __volatile__ (
> + "1: " LOCK_PREFIX "cmpxchg8b %[ptr]\n"
> + "2:\n"
> + ".section .fixup,\"ax\"\n"
> + "3: movl $1, %[rc]\n"
> + " jmp 2b\n"
> + ".previous\n"
> + ".section __ex_table,\"a\"\n"
> + " .align 4\n"
> + " .long 1b,3b\n"
> + ".previous"
> + : "=d" (o_hi), "=a" (o_lo), "=c" (n_hi), "=b" (n_lo),
> + [rc] "=r" (rc), [ptr] "m" (*__xg((volatile void *)ptr))
> + : "0" (o_hi), "1" (o_lo)
> + : "memory");
> + if ((o_hi != (oval >> 32)) ||
> + (o_lo != (oval & 0xffffffff)))
> + rc = 1;
> + return rc;
> +}
>
> static inline int update_l1e(l1_pgentry_t *pl1e,
> l1_pgentry_t ol1e,
> l1_pgentry_t nl1e)
> {
> - /* FIXME: breaks with PAE */
> +#if defined(__i386__) && defined(CONFIG_X86_PAE)
> +
> + int rc;
> +
> + rc = cmpxchg8b_user(pl1e, l1e_get_value(ol1e),
> + l1e_get_value(nl1e));
> + if (unlikely(rc))
> + {
> + MEM_LOG("Failed to update %llx -> %llx [pae]\n",
> + l1e_get_value(ol1e), l1e_get_value(nl1e));
> + return 0;
> + }
> + return 1;
> +
> +#else
> +
> unsigned long o = l1e_get_value(ol1e);
> unsigned long n = l1e_get_value(nl1e);
>
> @@ -850,8 +905,9 @@ static inline int update_l1e(l1_pgentry_
> l1e_get_value(ol1e), l1e_get_value(nl1e), o);
> return 0;
> }
> -
> return 1;
> +
> +#endif
> }
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home