* [PATCH] gnttab: miscellaneous fixes
@ 2012-02-09 11:15 Jan Beulich
2012-02-09 12:37 ` Keir Fraser
0 siblings, 1 reply; 2+ messages in thread
From: Jan Beulich @ 2012-02-09 11:15 UTC (permalink / raw)
To: xen-devel@lists.xensource.com
[-- Attachment #1: Type: text/plain, Size: 3200 bytes --]
- _GTF_* constants name bit positions, so binary arithmetic on them is
wrong
- gnttab_clear_flag() cannot (on x86 and ia64 at least) simply use
clear_bit(), as that may access more than the two bytes that are
intended to be accessed
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -397,7 +397,8 @@ static int _set_status_v2(domid_t domid
(id != domid) ||
(!readonly && (flags & GTF_readonly)) )
{
- gnttab_clear_flag(_GTF_reading | _GTF_writing, status);
+ gnttab_clear_flag(_GTF_writing, status);
+ gnttab_clear_flag(_GTF_reading, status);
PIN_FAIL(done, GNTST_general_error,
"Unstable flags (%x) or dom (%d). (expected dom %d) "
"(r/w: %d)\n",
@@ -1716,14 +1717,14 @@ __release_grant_for_copy(
under the domain's grant table lock. */
/* Only safe on transitive grants. Even then, note that we don't
attempt to drop any pin on the referent grant. */
-static void __fixup_status_for_pin(struct active_grant_entry *act,
+static void __fixup_status_for_pin(const struct active_grant_entry *act,
uint16_t *status)
{
if ( !(act->pin & GNTPIN_hstw_mask) )
- *status &= ~_GTF_writing;
+ *status &= ~GTF_writing;
if ( !(act->pin & GNTPIN_hstr_mask) )
- *status &= ~_GTF_reading;
+ *status &= ~GTF_reading;
}
/* Grab a frame number from a grant entry and update the flags and pin
--- a/xen/include/asm-ia64/grant_table.h
+++ b/xen/include/asm-ia64/grant_table.h
@@ -5,6 +5,8 @@
#ifndef __ASM_GRANT_TABLE_H__
#define __ASM_GRANT_TABLE_H__
+#include <asm/intrinsics.h>
+
#define INITIAL_NR_GRANT_FRAMES 1
// for grant map/unmap
@@ -82,9 +84,19 @@ int guest_physmap_add_page(struct domain
#define gnttab_mark_dirty(d, f) ((void)f)
-static inline void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
+static inline void gnttab_clear_flag(unsigned int nr, volatile uint16_t *st)
{
- clear_bit(nr, addr);
+ /*
+ * Note that this cannot be clear_bit(), as the access must be
+ * confined to the specified 2 bytes.
+ */
+ uint16_t mask = ~(1 << nr), old;
+ CMPXCHG_BUGCHECK_DECL
+
+ do {
+ CMPXCHG_BUGCHECK(st);
+ old = *st;
+ } while (cmpxchg_rel(st, old, old & mask) != old);
}
#define gnttab_host_mapping_get_page_type(op, ld, rd) \
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -48,9 +48,13 @@ int replace_grant_host_mapping(
#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), (f))
-static inline void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
+static inline void gnttab_clear_flag(unsigned int nr, uint16_t *st)
{
- clear_bit(nr, (unsigned long *)addr);
+ /*
+ * Note that this cannot be clear_bit(), as the access must be
+ * confined to the specified 2 bytes.
+ */
+ asm volatile ("lock btrw %1,%0" : "=m" (*st) : "Ir" (nr), "m" (*st));
}
/* Foreign mappings of HHVM-guest pages do not modify the type count. */
[-- Attachment #2: gnttab-misc.patch --]
[-- Type: text/plain, Size: 3225 bytes --]
gnttab: miscellaneous fixes
- _GTF_* constants name bit positions, so binary arithmetic on them is
wrong
- gnttab_clear_flag() cannot (on x86 and ia64 at least) simply use
clear_bit(), as that may access more than the two bytes that are
intended to be accessed
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -397,7 +397,8 @@ static int _set_status_v2(domid_t domid
(id != domid) ||
(!readonly && (flags & GTF_readonly)) )
{
- gnttab_clear_flag(_GTF_reading | _GTF_writing, status);
+ gnttab_clear_flag(_GTF_writing, status);
+ gnttab_clear_flag(_GTF_reading, status);
PIN_FAIL(done, GNTST_general_error,
"Unstable flags (%x) or dom (%d). (expected dom %d) "
"(r/w: %d)\n",
@@ -1716,14 +1717,14 @@ __release_grant_for_copy(
under the domain's grant table lock. */
/* Only safe on transitive grants. Even then, note that we don't
attempt to drop any pin on the referent grant. */
-static void __fixup_status_for_pin(struct active_grant_entry *act,
+static void __fixup_status_for_pin(const struct active_grant_entry *act,
uint16_t *status)
{
if ( !(act->pin & GNTPIN_hstw_mask) )
- *status &= ~_GTF_writing;
+ *status &= ~GTF_writing;
if ( !(act->pin & GNTPIN_hstr_mask) )
- *status &= ~_GTF_reading;
+ *status &= ~GTF_reading;
}
/* Grab a frame number from a grant entry and update the flags and pin
--- a/xen/include/asm-ia64/grant_table.h
+++ b/xen/include/asm-ia64/grant_table.h
@@ -5,6 +5,8 @@
#ifndef __ASM_GRANT_TABLE_H__
#define __ASM_GRANT_TABLE_H__
+#include <asm/intrinsics.h>
+
#define INITIAL_NR_GRANT_FRAMES 1
// for grant map/unmap
@@ -82,9 +84,19 @@ int guest_physmap_add_page(struct domain
#define gnttab_mark_dirty(d, f) ((void)f)
-static inline void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
+static inline void gnttab_clear_flag(unsigned int nr, volatile uint16_t *st)
{
- clear_bit(nr, addr);
+ /*
+ * Note that this cannot be clear_bit(), as the access must be
+ * confined to the specified 2 bytes.
+ */
+ uint16_t mask = ~(1 << nr), old;
+ CMPXCHG_BUGCHECK_DECL
+
+ do {
+ CMPXCHG_BUGCHECK(st);
+ old = *st;
+ } while (cmpxchg_rel(st, old, old & mask) != old);
}
#define gnttab_host_mapping_get_page_type(op, ld, rd) \
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -48,9 +48,13 @@ int replace_grant_host_mapping(
#define gnttab_mark_dirty(d, f) paging_mark_dirty((d), (f))
-static inline void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
+static inline void gnttab_clear_flag(unsigned int nr, uint16_t *st)
{
- clear_bit(nr, (unsigned long *)addr);
+ /*
+ * Note that this cannot be clear_bit(), as the access must be
+ * confined to the specified 2 bytes.
+ */
+ asm volatile ("lock btrw %1,%0" : "=m" (*st) : "Ir" (nr), "m" (*st));
}
/* Foreign mappings of HHVM-guest pages do not modify the type count. */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] gnttab: miscellaneous fixes
2012-02-09 11:15 [PATCH] gnttab: miscellaneous fixes Jan Beulich
@ 2012-02-09 12:37 ` Keir Fraser
0 siblings, 0 replies; 2+ messages in thread
From: Keir Fraser @ 2012-02-09 12:37 UTC (permalink / raw)
To: Jan Beulich, xen-devel@lists.xensource.com
On 09/02/2012 03:15, "Jan Beulich" <JBeulich@suse.com> wrote:
> - _GTF_* constants name bit positions, so binary arithmetic on them is
> wrong
> - gnttab_clear_flag() cannot (on x86 and ia64 at least) simply use
> clear_bit(), as that may access more than the two bytes that are
> intended to be accessed
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Keir Fraser <keir@xen.org>
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -397,7 +397,8 @@ static int _set_status_v2(domid_t domid
> (id != domid) ||
> (!readonly && (flags & GTF_readonly)) )
> {
> - gnttab_clear_flag(_GTF_reading | _GTF_writing, status);
> + gnttab_clear_flag(_GTF_writing, status);
> + gnttab_clear_flag(_GTF_reading, status);
> PIN_FAIL(done, GNTST_general_error,
> "Unstable flags (%x) or dom (%d). (expected dom %d) "
> "(r/w: %d)\n",
> @@ -1716,14 +1717,14 @@ __release_grant_for_copy(
> under the domain's grant table lock. */
> /* Only safe on transitive grants. Even then, note that we don't
> attempt to drop any pin on the referent grant. */
> -static void __fixup_status_for_pin(struct active_grant_entry *act,
> +static void __fixup_status_for_pin(const struct active_grant_entry *act,
> uint16_t *status)
> {
> if ( !(act->pin & GNTPIN_hstw_mask) )
> - *status &= ~_GTF_writing;
> + *status &= ~GTF_writing;
>
> if ( !(act->pin & GNTPIN_hstr_mask) )
> - *status &= ~_GTF_reading;
> + *status &= ~GTF_reading;
> }
>
> /* Grab a frame number from a grant entry and update the flags and pin
> --- a/xen/include/asm-ia64/grant_table.h
> +++ b/xen/include/asm-ia64/grant_table.h
> @@ -5,6 +5,8 @@
> #ifndef __ASM_GRANT_TABLE_H__
> #define __ASM_GRANT_TABLE_H__
>
> +#include <asm/intrinsics.h>
> +
> #define INITIAL_NR_GRANT_FRAMES 1
>
> // for grant map/unmap
> @@ -82,9 +84,19 @@ int guest_physmap_add_page(struct domain
>
> #define gnttab_mark_dirty(d, f) ((void)f)
>
> -static inline void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
> +static inline void gnttab_clear_flag(unsigned int nr, volatile uint16_t *st)
> {
> - clear_bit(nr, addr);
> + /*
> + * Note that this cannot be clear_bit(), as the access must be
> + * confined to the specified 2 bytes.
> + */
> + uint16_t mask = ~(1 << nr), old;
> + CMPXCHG_BUGCHECK_DECL
> +
> + do {
> + CMPXCHG_BUGCHECK(st);
> + old = *st;
> + } while (cmpxchg_rel(st, old, old & mask) != old);
> }
>
> #define gnttab_host_mapping_get_page_type(op, ld, rd) \
> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -48,9 +48,13 @@ int replace_grant_host_mapping(
>
> #define gnttab_mark_dirty(d, f) paging_mark_dirty((d), (f))
>
> -static inline void gnttab_clear_flag(unsigned long nr, uint16_t *addr)
> +static inline void gnttab_clear_flag(unsigned int nr, uint16_t *st)
> {
> - clear_bit(nr, (unsigned long *)addr);
> + /*
> + * Note that this cannot be clear_bit(), as the access must be
> + * confined to the specified 2 bytes.
> + */
> + asm volatile ("lock btrw %1,%0" : "=m" (*st) : "Ir" (nr), "m" (*st));
> }
>
> /* Foreign mappings of HHVM-guest pages do not modify the type count. */
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2012-02-09 12:37 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-09 11:15 [PATCH] gnttab: miscellaneous fixes Jan Beulich
2012-02-09 12:37 ` Keir Fraser
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).