[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 10/44] x86/boot: introduce boot module flags


  • To: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Date: Thu, 17 Oct 2024 20:54:03 -0400
  • Arc-authentication-results: i=1; mx.zohomail.com; dkim=pass header.i=apertussolutions.com; spf=pass smtp.mailfrom=dpsmith@xxxxxxxxxxxxxxxxxxxx; dmarc=pass header.from=<dpsmith@xxxxxxxxxxxxxxxxxxxx>
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1729212846; h=Content-Type:Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:MIME-Version:Message-ID:References:Subject:Subject:To:To:Message-Id:Reply-To; bh=KLF4GOTGKi39kyoT2GIHZ0az9UBYS+/DtaPYr71MTOA=; b=RpkFJIuS+l+/bFpI2QrOV1FjbDV0bUu+765dCRitGoVvZs3RhpXuk8oaKoANqlC9FdP4OpcHA4RiQaMtyHGq7IN5HYLR9yuUDEcqB7arn8NEfvAzmoUuRN3rivxNjTn09HJ70R9qUPJTGq0NvewGEybDdMWl92ZuSsThv5AbEbE=
  • Arc-seal: i=1; a=rsa-sha256; t=1729212846; cv=none; d=zohomail.com; s=zohoarc; b=YgvKeqDMw28AQLtdR+bF2r0Aa+Q86pmrZPMu9Nk0hZkb2sAW7scaeccJ+p7e6PyKxKyamZ+NjNMgyJ7ykvdUbtTZSWNkW310jBOFgbwMbjfS/mBeDSFJzetlHdpSGWllVKaB7m7VCp7nPxd24mPaHgY7wlnXPZzXkmscln+9bvQ=
  • Cc: jason.andryuk@xxxxxxx, christopher.w.clark@xxxxxxxxx, stefano.stabellini@xxxxxxx, Jan Beulich <jbeulich@xxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Fri, 18 Oct 2024 00:54:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 10/17/24 19:58, Andrew Cooper wrote:
On 17/10/2024 6:02 pm, Daniel P. Smith wrote:
diff --git a/xen/arch/x86/include/asm/bootinfo.h 
b/xen/arch/x86/include/asm/bootinfo.h
index 18281d80fa97..e8ba9390a51f 100644
--- a/xen/arch/x86/include/asm/bootinfo.h
+++ b/xen/arch/x86/include/asm/bootinfo.h
@@ -39,6 +39,9 @@ struct boot_module {
       */
      unsigned long headroom;
      enum bootmod_type type;
+
+    uint32_t flags;
+#define BOOTMOD_FLAG_X86_RELOCATED     (1U << 0)

There are two parts to this request.  First, there's nothing X86
specific about any of the flags added (in this series at least), and the
FLAG infix doesn't feel as if it adds much.

But, looking over the code, I get the feeling it would be better as simply:

     /*
      * Misc flags  XXX docs
      */
     bool relocated:1;
     bool consumed:1;

(Possibly with an enclosing struct { ... } flags; but I don't think
that's necessary either.)

I see no reason why not a bitfield, and as you state, will make code simpler (and possibly shorter) elsewhere.

because flags is never operated on as a unit of multiple things, or
passed around separately from a bm-> pointer.  For misc independent
flags like this, bitfields lead to far more legible code because you're
not having to express everything as binary operators in the logic.

I know this will be a moderately invasive change, but I suspect the
improvement will speak for itself.  (Assuming there isn't a need to keep
it as a plain flags field later.)

Yah it will be a bit painful, but I would rather have cleaner and easier to read code.

v/r,
dps




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.