Re: [Xen-devel] [PATCH 4/5] xen/gnttab: Refactor gnttab_clear_flag() to be gnttab_clear_flags()

[Jan Beulich <JBeulich@xxxxxxxx>]

>>> On 21.06.19 at 11:36, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -60,14 +60,11 @@ static inline int replace_grant_host_mapping(uint64_t 
> addr, mfn_t frame,
>  
>  #define gnttab_mark_dirty(d, f) paging_mark_dirty(d, f)
>  
> -static inline void gnttab_clear_flag(struct domain *d, unsigned int nr,
> -                                     uint16_t *st)
> +static inline void gnttab_clear_flags(struct domain *d,
> +                                      uint16_t mask, uint16_t *addr)
>  {
> -    /*
> -     * Note that this cannot be clear_bit(), as the access must be
> -     * confined to the specified 2 bytes.
> -     */
> -    asm volatile ("lock btrw %w1,%0" : "+m" (*st) : "Ir" (nr));
> +    /* Access must be confined to the specified 2 bytes. */
> +    asm volatile ("lock andw %w1,%0" : "+m" (*addr) : "Ir" (~mask));

The I constraint limits literal numbers to 5 bits, which is not what
you want here (albeit it's fine for the two bits we currently want
this for). "N" would widen the range to 8 bits, while "e" and "i"
would go beyond the 16 we want. Therefore I think you want
to cast ~mask to uint16_t and use "e" or "i". (This would then
also allow dropping the w modifier.)

Speaking of uint16_t - the "mask" parameter could easily be
unsigned int, possibly allowing the compiler to avoid a 66 prefix
here and there.

Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel