In commit 90629587e16e ("x86/shadow: replace stale literal numbers in
hash_{vcpu,domain}_foreach()") I had to work around a clang shortcoming
(if you like), leveraging that gcc tolerates static const variables in
otherwise integer constant expressions. Roberto suggests that we'd
better not rely on such behavior. Drop the involved static const-s,
using their "expansions" in both of the prior use sites each. This then
allows dropping the short-circuiting of the check for clang.
Requested-by: Roberto Bagnara <roberto.bagnara@bugseng.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -1626,12 +1626,8 @@ void shadow_hash_delete(struct domain *d
typedef int (*hash_vcpu_callback_t)(struct vcpu *v, mfn_t smfn, mfn_t other_mfn);
typedef int (*hash_domain_callback_t)(struct domain *d, mfn_t smfn, mfn_t other_mfn);
-#ifndef __clang__ /* At least some versions dislike some of the uses. */
#define HASH_CALLBACKS_CHECK(mask) \
BUILD_BUG_ON((mask) > (1U << ARRAY_SIZE(callbacks)) - 1)
-#else
-#define HASH_CALLBACKS_CHECK(mask) ((void)(mask))
-#endif
static void hash_vcpu_foreach(struct vcpu *v, unsigned int callback_mask,
const hash_vcpu_callback_t callbacks[],
@@ -1829,7 +1825,6 @@ int sh_remove_write_access(struct domain
[SH_type_l1_64_shadow] = SHADOW_INTERNAL_NAME(sh_rm_write_access_from_l1, 4),
[SH_type_fl1_64_shadow] = SHADOW_INTERNAL_NAME(sh_rm_write_access_from_l1, 4),
};
- static const unsigned int callback_mask = SHF_L1_ANY | SHF_FL1_ANY;
struct page_info *pg = mfn_to_page(gmfn);
#if SHADOW_OPTIMIZATIONS & SHOPT_WRITABLE_HEURISTIC
struct vcpu *curr = current;
@@ -2004,8 +1999,8 @@ int sh_remove_write_access(struct domain
perfc_incr(shadow_writeable_bf_1);
else
perfc_incr(shadow_writeable_bf);
- HASH_CALLBACKS_CHECK(callback_mask);
- hash_domain_foreach(d, callback_mask, callbacks, gmfn);
+ HASH_CALLBACKS_CHECK(SHF_L1_ANY | SHF_FL1_ANY);
+ hash_domain_foreach(d, SHF_L1_ANY | SHF_FL1_ANY, callbacks, gmfn);
/* If that didn't catch the mapping, then there's some non-pagetable
* mapping -- ioreq page, grant mapping, &c. */
@@ -2044,7 +2039,6 @@ int sh_remove_all_mappings(struct domain
[SH_type_l1_64_shadow] = SHADOW_INTERNAL_NAME(sh_rm_mappings_from_l1, 4),
[SH_type_fl1_64_shadow] = SHADOW_INTERNAL_NAME(sh_rm_mappings_from_l1, 4),
};
- static const unsigned int callback_mask = SHF_L1_ANY | SHF_FL1_ANY;
perfc_incr(shadow_mappings);
if ( sh_check_page_has_no_refs(page) )
@@ -2060,8 +2054,8 @@ int sh_remove_all_mappings(struct domain
/* Brute-force search of all the shadows, by walking the hash */
perfc_incr(shadow_mappings_bf);
- HASH_CALLBACKS_CHECK(callback_mask);
- hash_domain_foreach(d, callback_mask, callbacks, gmfn);
+ HASH_CALLBACKS_CHECK(SHF_L1_ANY | SHF_FL1_ANY);
+ hash_domain_foreach(d, SHF_L1_ANY | SHF_FL1_ANY, callbacks, gmfn);
/* If that didn't catch the mapping, something is very wrong */
if ( !sh_check_page_has_no_refs(page) )
@@ -2307,10 +2301,9 @@ void sh_reset_l3_up_pointers(struct vcpu
static const hash_vcpu_callback_t callbacks[SH_type_unused] = {
[SH_type_l3_64_shadow] = sh_clear_up_pointer,
};
- static const unsigned int callback_mask = SHF_L3_64;
- HASH_CALLBACKS_CHECK(callback_mask);
- hash_vcpu_foreach(v, callback_mask, callbacks, INVALID_MFN);
+ HASH_CALLBACKS_CHECK(SHF_L3_64);
+ hash_vcpu_foreach(v, SHF_L3_64, callbacks, INVALID_MFN);
}
At 08:42 +0200 on 30 Jun (1625042541), Jan Beulich wrote: > In commit 90629587e16e ("x86/shadow: replace stale literal numbers in > hash_{vcpu,domain}_foreach()") I had to work around a clang shortcoming > (if you like), leveraging that gcc tolerates static const variables in > otherwise integer constant expressions. Roberto suggests that we'd > better not rely on such behavior. Drop the involved static const-s, > using their "expansions" in both of the prior use sites each. This then > allows dropping the short-circuiting of the check for clang. > > Requested-by: Roberto Bagnara <roberto.bagnara@bugseng.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Tim Deegan <tim@xen.org>
On 30/06/2021 07:42, Jan Beulich wrote: > In commit 90629587e16e ("x86/shadow: replace stale literal numbers in > hash_{vcpu,domain}_foreach()") I had to work around a clang shortcoming > (if you like), leveraging that gcc tolerates static const variables in > otherwise integer constant expressions. Roberto suggests that we'd > better not rely on such behavior. Drop the involved static const-s, > using their "expansions" in both of the prior use sites each. This then > allows dropping the short-circuiting of the check for clang. > > Requested-by: Roberto Bagnara <roberto.bagnara@bugseng.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> It is not fair to categorise this as a shortcoming in clang. C mandates an ICE in _Static_assert(). You tried to used a GCC extension/implementation detail which Clang doesn't implement. With a suitable change to the commit message, Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
© 2016 - 2024 Red Hat, Inc.