xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86/shadow: fix DO_UNSHADOW()
@ 2021-05-19 12:36 Jan Beulich
  2021-05-19 13:12 ` Luca Fancellu
  2021-05-24 16:28 ` Tim Deegan
  0 siblings, 2 replies; 3+ messages in thread
From: Jan Beulich @ 2021-05-19 12:36 UTC (permalink / raw)
  To: xen-devel
  Cc: Tim Deegan, George Dunlap, Andrew Cooper, Wei Liu, Roger Pau Monné

When adding the HASH_CALLBACKS_CHECK() I failed to properly recognize
the (somewhat unusually formatted) if() around the call to
hash_domain_foreach()). Gcc 11 is absolutely right in pointing out the
apparently misleading indentation. Besides adding the missing braces,
also adjust the two oddly formatted if()-s in the macro.

Fixes: 90629587e16e ("x86/shadow: replace stale literal numbers in hash_{vcpu,domain}_foreach()")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
I'm puzzled as to why this bug didn't cause any fallout.

--- a/xen/arch/x86/mm/shadow/common.c
+++ b/xen/arch/x86/mm/shadow/common.c
@@ -2220,8 +2220,8 @@ void sh_remove_shadows(struct domain *d,
      */
 #define DO_UNSHADOW(_type) do {                                         \
     t = (_type);                                                        \
-    if( !(pg->count_info & PGC_page_table)                              \
-        || !(pg->shadow_flags & (1 << t)) )                             \
+    if ( !(pg->count_info & PGC_page_table) ||                          \
+         !(pg->shadow_flags & (1 << t)) )                               \
         break;                                                          \
     smfn = shadow_hash_lookup(d, mfn_x(gmfn), t);                       \
     if ( unlikely(!mfn_valid(smfn)) )                                   \
@@ -2235,11 +2235,13 @@ void sh_remove_shadows(struct domain *d,
         sh_unpin(d, smfn);                                              \
     else if ( sh_type_has_up_pointer(d, t) )                            \
         sh_remove_shadow_via_pointer(d, smfn);                          \
-    if( !fast                                                           \
-        && (pg->count_info & PGC_page_table)                            \
-        && (pg->shadow_flags & (1 << t)) )                              \
+    if ( !fast &&                                                       \
+         (pg->count_info & PGC_page_table) &&                           \
+         (pg->shadow_flags & (1 << t)) )                                \
+    {                                                                   \
         HASH_CALLBACKS_CHECK(SHF_page_type_mask);                       \
         hash_domain_foreach(d, masks[t], callbacks, smfn);              \
+    }                                                                   \
 } while (0)
 
     DO_UNSHADOW(SH_type_l2_32_shadow);


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/shadow: fix DO_UNSHADOW()
  2021-05-19 12:36 [PATCH] x86/shadow: fix DO_UNSHADOW() Jan Beulich
@ 2021-05-19 13:12 ` Luca Fancellu
  2021-05-24 16:28 ` Tim Deegan
  1 sibling, 0 replies; 3+ messages in thread
From: Luca Fancellu @ 2021-05-19 13:12 UTC (permalink / raw)
  To: Jan Beulich
  Cc: xen-devel, Tim Deegan, George Dunlap, Andrew Cooper, Wei Liu,
	Roger Pau Monné



> On 19 May 2021, at 13:36, Jan Beulich <jbeulich@suse.com> wrote:
> 
> When adding the HASH_CALLBACKS_CHECK() I failed to properly recognize
> the (somewhat unusually formatted) if() around the call to
> hash_domain_foreach()). Gcc 11 is absolutely right in pointing out the
> apparently misleading indentation. Besides adding the missing braces,
> also adjust the two oddly formatted if()-s in the macro.
> 
> Fixes: 90629587e16e ("x86/shadow: replace stale literal numbers in hash_{vcpu,domain}_foreach()")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Luca Fancellu <luca.fancellu@arm.com>

Cheers,
Luca

> ---
> I'm puzzled as to why this bug didn't cause any fallout.
> 
> --- a/xen/arch/x86/mm/shadow/common.c
> +++ b/xen/arch/x86/mm/shadow/common.c
> @@ -2220,8 +2220,8 @@ void sh_remove_shadows(struct domain *d,
>      */
> #define DO_UNSHADOW(_type) do {                                         \
>     t = (_type);                                                        \
> -    if( !(pg->count_info & PGC_page_table)                              \
> -        || !(pg->shadow_flags & (1 << t)) )                             \
> +    if ( !(pg->count_info & PGC_page_table) ||                          \
> +         !(pg->shadow_flags & (1 << t)) )                               \
>         break;                                                          \
>     smfn = shadow_hash_lookup(d, mfn_x(gmfn), t);                       \
>     if ( unlikely(!mfn_valid(smfn)) )                                   \
> @@ -2235,11 +2235,13 @@ void sh_remove_shadows(struct domain *d,
>         sh_unpin(d, smfn);                                              \
>     else if ( sh_type_has_up_pointer(d, t) )                            \
>         sh_remove_shadow_via_pointer(d, smfn);                          \
> -    if( !fast                                                           \
> -        && (pg->count_info & PGC_page_table)                            \
> -        && (pg->shadow_flags & (1 << t)) )                              \
> +    if ( !fast &&                                                       \
> +         (pg->count_info & PGC_page_table) &&                           \
> +         (pg->shadow_flags & (1 << t)) )                                \
> +    {                                                                   \
>         HASH_CALLBACKS_CHECK(SHF_page_type_mask);                       \
>         hash_domain_foreach(d, masks[t], callbacks, smfn);              \
> +    }                                                                   \
> } while (0)
> 
>     DO_UNSHADOW(SH_type_l2_32_shadow);
> 



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/shadow: fix DO_UNSHADOW()
  2021-05-19 12:36 [PATCH] x86/shadow: fix DO_UNSHADOW() Jan Beulich
  2021-05-19 13:12 ` Luca Fancellu
@ 2021-05-24 16:28 ` Tim Deegan
  1 sibling, 0 replies; 3+ messages in thread
From: Tim Deegan @ 2021-05-24 16:28 UTC (permalink / raw)
  To: Jan Beulich
  Cc: xen-devel, George Dunlap, Andrew Cooper, Wei Liu, Roger Pau Monné

At 14:36 +0200 on 19 May (1621434982), Jan Beulich wrote:
> When adding the HASH_CALLBACKS_CHECK() I failed to properly recognize
> the (somewhat unusually formatted) if() around the call to
> hash_domain_foreach()). Gcc 11 is absolutely right in pointing out the
> apparently misleading indentation. Besides adding the missing braces,
> also adjust the two oddly formatted if()-s in the macro.
> 
> Fixes: 90629587e16e ("x86/shadow: replace stale literal numbers in hash_{vcpu,domain}_foreach()")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Tim Deegan <tim@xen.org>


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-24 16:29 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-19 12:36 [PATCH] x86/shadow: fix DO_UNSHADOW() Jan Beulich
2021-05-19 13:12 ` Luca Fancellu
2021-05-24 16:28 ` Tim Deegan

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).