All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>,
	"George Dunlap" <george.dunlap@citrix.com>,
	"Tim Deegan" <tim@xen.org>
Subject: [PATCH 01/16] x86/shadow: fix and improve sh_page_has_multiple_shadows()
Date: Wed, 22 Mar 2023 10:29:49 +0100	[thread overview]
Message-ID: <290c9058-9907-4c6d-3fe8-987868a3a843@suse.com> (raw)
In-Reply-To: <dd9205b8-63f0-b1bc-f2b8-50d5da2bf2a7@suse.com>

While no caller currently invokes the function without first making sure
there is at least one shadow [1], we'd better eliminate UB here:
find_first_set_bit() requires input to be non-zero to return a well-
defined result.

Further, using find_first_set_bit() isn't very efficient in the first
place for the intended purpose.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

[1] The function has exactly two uses, and both are from OOS code, which
    is HVM-only. For HVM (but not for PV) sh_mfn_is_a_page_table(),
    guarding the call to sh_unsync(), guarantees at least one shadow.
    Hence even if sh_page_has_multiple_shadows() returned a bogus value
    when invoked for a PV domain, the subsequent is_hvm_vcpu() and
    oos_active checks (the former being redundant with the latter) will
    compensate. (Arguably that oos_active check should come first, for
    both clarity and efficiency reasons.)
---
Considering present uses, ASSERT(shadows) might be an option as well,
instead of making the null check part of the return value expression.

--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -332,7 +332,7 @@ static inline int sh_page_has_multiple_s
         return 0;
     shadows = pg->shadow_flags & SHF_page_type_mask;
     /* More than one type bit set in shadow-flags? */
-    return ( (shadows & ~(1UL << find_first_set_bit(shadows))) != 0 );
+    return shadows && (shadows & (shadows - 1));
 }
 
 #if (SHADOW_OPTIMIZATIONS & SHOPT_OUT_OF_SYNC)



  reply	other threads:[~2023-03-22  9:30 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-22  9:28 [PATCH 00/16] x86: assorted (mostly) shadow mode adjustments Jan Beulich
2023-03-22  9:29 ` Jan Beulich [this message]
2023-03-23 11:55   ` [PATCH 01/16] x86/shadow: fix and improve sh_page_has_multiple_shadows() Andrew Cooper
2023-03-22  9:30 ` [PATCH 02/16] x86/shadow: fold/rename sh_unhook_*_mappings() Jan Beulich
2023-03-22  9:31 ` [PATCH 03/16] x86/shadow: drop redundant present bit checks from SHADOW_FOREACH_L<N>E() "bodys" Jan Beulich
2023-03-23 12:14   ` Andrew Cooper
2023-03-22  9:31 ` [PATCH 04/16] x86/shadow: replace memcmp() in sh_resync_l1() Jan Beulich
2023-03-23 12:55   ` Andrew Cooper
2023-03-22  9:31 ` [PATCH 05/16] x86/shadow: reduce explicit log-dirty recording for HVM Jan Beulich
2023-03-22  9:32 ` [PATCH 06/16] x86/shadow: purge {write,cmpxchg}_guest_entry() hooks Jan Beulich
2023-03-23 13:13   ` Andrew Cooper
2023-03-22  9:33 ` [PATCH 07/16] x86/shadow: call sh_update_cr3() directly from sh_page_fault() Jan Beulich
2023-03-23 13:34   ` Andrew Cooper
2023-03-27 15:39   ` Tim Deegan
2023-03-28 10:37     ` Jan Beulich
2023-03-29  6:49       ` Tim Deegan
2023-03-22  9:34 ` [PATCH 08/16] x86/shadow: use lighter weight mode checks Jan Beulich
2023-03-22  9:34 ` [PATCH 09/16] x86/shadow: OOS mode is HVM-only Jan Beulich
2023-03-23 13:42   ` Andrew Cooper
2023-03-22  9:35 ` [PATCH 10/16] x86/shadow: move OOS functions to their own file Jan Beulich
2023-03-23 14:30   ` Andrew Cooper
2023-03-23 15:40     ` Jan Beulich
2023-03-23 17:44       ` Andrew Cooper
2023-03-22  9:35 ` [PATCH 11/16] x86/shadow: drop is_hvm_...() where easily possible Jan Beulich
2023-03-23 18:18   ` Andrew Cooper
2023-03-24  7:38     ` Jan Beulich
2023-03-28 13:57       ` Andrew Cooper
2023-03-28 14:41         ` Jan Beulich
2023-03-22  9:35 ` [PATCH 12/16] x86/shadow: make monitor table create/destroy more consistent Jan Beulich
2023-03-23 18:28   ` Andrew Cooper
2023-03-24  7:52     ` Jan Beulich
2023-03-22  9:36 ` [PATCH 13/16] x86/shadow: vCPU-s never have "no mode" Jan Beulich
2023-03-23 18:29   ` Andrew Cooper
2023-03-22  9:37 ` [PATCH 14/16] x86/shadow: "monitor table" is a HVM-only concept Jan Beulich
2023-03-23 18:30   ` Andrew Cooper
2023-03-22  9:37 ` [PATCH 15/16] x86/shadow: adjust monitor table prealloc amount Jan Beulich
2023-03-22  9:38 ` [PATCH 16/16] x86/PV: conditionalize arch_set_info_guest()'s call to update_cr3() Jan Beulich
2023-03-23 18:34   ` Andrew Cooper
2023-03-23 11:40 ` [PATCH 00/16] x86: assorted (mostly) shadow mode adjustments Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=290c9058-9907-4c6d-3fe8-987868a3a843@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=george.dunlap@citrix.com \
    --cc=roger.pau@citrix.com \
    --cc=tim@xen.org \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.