[image] Generalise concept of selected image

Most image flags are independent values: any combination of flags may
be set for any image, and the flags for one image are independent of
the flags for any other image.  The "selected" flag does not follow
this pattern: at most one image may be marked as selected at any time.

When invoking a kernel via the UEFI shim, there will be multiple
"special" images: the selected kernel itself, the shim image, and
potentially a shim-signed GRUB binary to be used as a crutch to assist
shim in loading the kernel (since current versions of the UEFI shim
are not capable of directly loading a Linux kernel).

Remove the "selected" image flag and replace it with a general concept
of an image tag with the same semantics: a given tag may be assigned
to at most one image, an image may be found by its tag only while the
image is currently registered, and a tag will survive unregistration
and reregistration of an image (if it has not already been assigned to
a new image).  For visual consistency, also replace the current image
pointer with a current image tag.

The image pointer stored within the image tag holds only a weak
reference to the image, since the selection of an image should not
prevent that image from being freed.  (The strong reference to the
currently executing image is held locally within the execution scope
of image_exec(), and is logically separate from the current image
pointer.)

Signed-off-by: Michael Brown <mcb30@ipxe.org>

diff --git a/src/core/image.c b/src/core/image.c
index b280eb4d3bebc730cee2219e763050b3e2edb4c5..3e65b5edf1a085f989d9ef90dfec4f1bbe145997 100644 (file)
--- a/src/core/image.c
+++ b/src/core/image.c
@@ -56,8 +56,15 @@ FILE_LICENCE ( GPL2_OR_LATER_OR_UBDL );
 /** List of registered images */
 struct list_head images = LIST_HEAD_INIT ( images );
 
+/** Image selected for execution */
+struct image_tag selected_image __image_tag = {
+       .name = "SELECTED",
+};
+
 /** Currently-executing image */
-struct image *current_image;
+struct image_tag current_image __image_tag = {
+       .name = "CURRENT",
+};
 
 /** Current image trust requirement */
 static int require_trusted_images = 0;
@@ -72,8 +79,13 @@ static int require_trusted_images_permanent = 0;
  */
 static void free_image ( struct refcnt *refcnt ) {
        struct image *image = container_of ( refcnt, struct image, refcnt );
+       struct image_tag *tag;
 
        DBGC ( image, "IMAGE %s freed\n", image->name );
+       for_each_table_entry ( tag, IMAGE_TAGS ) {
+               if ( tag->image == image )
+                       tag->image = NULL;
+       }
        free ( image->name );
        free ( image->cmdline );
        uri_put ( image->uri );
@@ -261,12 +273,6 @@ int register_image ( struct image *image ) {
                        return rc;
        }
 
-       /* Avoid ending up with multiple "selected" images on
-        * re-registration
-        */
-       if ( image_find_selected() )
-               image->flags &= ~IMAGE_SELECTED;
-
        /* Add to image list */
        image_get ( image );
        image->flags |= IMAGE_REGISTERED;
@@ -320,6 +326,23 @@ struct image * find_image ( const char *name ) {
        return NULL;
 }
 
+/**
+ * Find image by tag
+ *
+ * @v tag              Image tag
+ * @ret image          Executable image, or NULL
+ */
+struct image * find_image_tag ( struct image_tag *tag ) {
+       struct image *image;
+
+       for_each_image ( image ) {
+               if ( tag->image == image )
+                       return image;
+       }
+
+       return NULL;
+}
+
 /**
  * Execute image
  *
@@ -346,13 +369,13 @@ int image_exec ( struct image *image ) {
        if ( image->uri )
                churi ( image->uri );
 
-       /* Preserve record of any currently-running image */
-       saved_current_image = current_image;
+       /* Set as currently running image */
+       saved_current_image = image_tag ( image, &current_image );
 
        /* Take out a temporary reference to the image, so that it
         * does not get freed when temporarily unregistered.
         */
-       current_image = image_get ( image );
+       image_get ( image );
 
        /* Check that this image can be executed */
        if ( ! ( image->type && image->type->exec ) ) {
@@ -419,7 +442,7 @@ int image_exec ( struct image *image ) {
        image_put ( image );
 
        /* Restore previous currently-running image */
-       current_image = saved_current_image;
+       image_tag ( saved_current_image, &current_image );
 
        /* Reset current working directory */
        churi ( old_cwuri );
@@ -442,7 +465,7 @@ int image_exec ( struct image *image ) {
  * registered until the currently-executing image returns.
  */
 int image_replace ( struct image *replacement ) {
-       struct image *image = current_image;
+       struct image *image = current_image.image;
        int rc;
 
        /* Sanity check */
@@ -478,37 +501,17 @@ int image_replace ( struct image *replacement ) {
  * @ret rc             Return status code
  */
 int image_select ( struct image *image ) {
-       struct image *tmp;
-
-       /* Unselect all other images */
-       for_each_image ( tmp )
-               tmp->flags &= ~IMAGE_SELECTED;
 
        /* Check that this image can be executed */
        if ( ! ( image->type && image->type->exec ) )
                return -ENOEXEC;
 
        /* Mark image as selected */
-       image->flags |= IMAGE_SELECTED;
+       image_tag ( image, &selected_image );
 
        return 0;
 }
 
-/**
- * Find selected image
- *
- * @ret image          Executable image, or NULL
- */
-struct image * image_find_selected ( void ) {
-       struct image *image;
-
-       for_each_image ( image ) {
-               if ( image->flags & IMAGE_SELECTED )
-                       return image;
-       }
-       return NULL;
-}
-
 /**
  * Change image trust requirement
  *

diff --git a/src/hci/commands/image_cmd.c b/src/hci/commands/image_cmd.c
index 4a7c500a45b2885e96cdda19a3cf54bf97fe5379..bf97b4deb95d2cd6b82d9d4d231b0250e98c7949 100644 (file)
--- a/src/hci/commands/image_cmd.c
+++ b/src/hci/commands/image_cmd.c
@@ -129,7 +129,7 @@ static int imgsingle_exec ( int argc, char **argv,
                                            &image ) ) != 0 )
                        goto err_acquire;
        } else {
-               image = image_find_selected();
+               image = find_image_tag ( &selected_image );
                if ( ! image ) {
                        printf ( "No image selected\n" );
                        goto err_acquire;

diff --git a/src/image/script.c b/src/image/script.c
index b34df1e21a52ee78525d12dfd2e9d824e59dd75b..49b356403a5eed09bed584f968eb6c9457d52407 100644 (file)
--- a/src/image/script.c
+++ b/src/image/script.c
@@ -311,6 +311,7 @@ static int terminate_on_label_found ( int rc ) {
  * @ret rc             Return status code
  */
 static int goto_exec ( int argc, char **argv ) {
+       struct image *image = current_image.image;
        struct goto_options opts;
        size_t saved_offset;
        int rc;
@@ -320,7 +321,7 @@ static int goto_exec ( int argc, char **argv ) {
                return rc;
 
        /* Sanity check */
-       if ( ! current_image ) {
+       if ( ! image ) {
                rc = -ENOTTY;
                printf ( "Not in a script: %s\n", strerror ( rc ) );
                return rc;
@@ -331,10 +332,10 @@ static int goto_exec ( int argc, char **argv ) {
 
        /* Find label */
        saved_offset = script_offset;
-       if ( ( rc = process_script ( current_image, goto_find_label,
+       if ( ( rc = process_script ( image, goto_find_label,
                                     terminate_on_label_found ) ) != 0 ) {
                script_offset = saved_offset;
-               DBGC ( current_image, "[%04zx] No such label :%s\n",
+               DBGC ( image, "[%04zx] No such label :%s\n",
                       script_offset, goto_label );
                return rc;
        }

diff --git a/src/include/ipxe/image.h b/src/include/ipxe/image.h
index e00af82ef2d9233c8a6c0a251502d18b574a2fb2..cd51188d3cd865a86adc85d65f4e0f40087cfcd1 100644 (file)
--- a/src/include/ipxe/image.h
+++ b/src/include/ipxe/image.h
@@ -61,19 +61,16 @@ struct image {
 };
 
 /** Image is registered */
-#define IMAGE_REGISTERED 0x00001
-
-/** Image is selected for execution */
-#define IMAGE_SELECTED 0x0002
+#define IMAGE_REGISTERED 0x0001
 
 /** Image is trusted */
-#define IMAGE_TRUSTED 0x0004
+#define IMAGE_TRUSTED 0x0002
 
 /** Image will be automatically unregistered after execution */
-#define IMAGE_AUTO_UNREGISTER 0x0008
+#define IMAGE_AUTO_UNREGISTER 0x0004
 
 /** Image will be hidden from enumeration */
-#define IMAGE_HIDDEN 0x0010
+#define IMAGE_HIDDEN 0x0008
 
 /** An executable image type */
 struct image_type {
@@ -153,8 +150,23 @@ struct image_type {
 /** An executable image type */
 #define __image_type( probe_order ) __table_entry ( IMAGE_TYPES, probe_order )
 
+/** An image tag */
+struct image_tag {
+       /** Name */
+       const char *name;
+       /** Image (weak reference, nullified when image is freed) */
+       struct image *image;
+};
+
+/** Image tag table */
+#define IMAGE_TAGS __table ( struct image_tag, "image_tags" )
+
+/** An image tag */
+#define __image_tag __table_entry ( IMAGE_TAGS, 01 )
+
 extern struct list_head images;
-extern struct image *current_image;
+extern struct image_tag current_image;
+extern struct image_tag selected_image;
 
 /** Iterate over all registered images */
 #define for_each_image( image ) \
@@ -181,11 +193,11 @@ extern int image_set_len ( struct image *image, size_t len );
 extern int image_set_data ( struct image *image, userptr_t data, size_t len );
 extern int register_image ( struct image *image );
 extern void unregister_image ( struct image *image );
-struct image * find_image ( const char *name );
+extern struct image * find_image ( const char *name );
+extern struct image * find_image_tag ( struct image_tag *tag );
 extern int image_exec ( struct image *image );
 extern int image_replace ( struct image *replacement );
 extern int image_select ( struct image *image );
-extern struct image * image_find_selected ( void );
 extern int image_set_trust ( int require_trusted, int permanent );
 extern struct image * image_memory ( const char *name, userptr_t data,
                                     size_t len );
@@ -244,4 +256,19 @@ static inline void image_untrust ( struct image *image ) {
        image->flags &= ~IMAGE_TRUSTED;
 }
 
+/**
+ * Tag image
+ *
+ * @v image            Image
+ * @v tag              Image tag
+ * @ret prev           Previous tagged image (if any)
+ */
+static inline struct image * image_tag ( struct image *image,
+                                        struct image_tag *tag ) {
+       struct image *prev = tag->image;
+
+       tag->image = image;
+       return prev;
+}
+
 #endif /* _IPXE_IMAGE_H */

diff --git a/src/interface/efi/efi_file.c b/src/interface/efi/efi_file.c
index 266de4a6c1db5d825b859e8e2ebd8aac8b3714ff..2ae3a0cb4cb8b70babee7995add175b8c2a19ed0 100644 (file)
--- a/src/interface/efi/efi_file.c
+++ b/src/interface/efi/efi_file.c
@@ -420,7 +420,7 @@ efi_file_open ( EFI_FILE_PROTOCOL *this, EFI_FILE_PROTOCOL **new,
        if ( ( strncasecmp ( name, "grub", 4 ) == 0 ) &&
             ( ( sep = strrchr ( name, '.' ) ) != NULL ) &&
             ( strcasecmp ( sep, ".efi" ) == 0 ) &&
-            ( ( image = image_find_selected() ) != NULL ) ) {
+            ( ( image = find_image_tag ( &selected_image ) ) != NULL ) ) {
                return efi_file_open_image ( image, wname, new );
        }
 

diff --git a/src/usr/imgmgmt.c b/src/usr/imgmgmt.c
index 94f3d2ce05ead8fa9119fb3ccd45188feb53b1bd..92bf236f9b6668efc4b115ce135ee0e63c82bc81 100644 (file)
--- a/src/usr/imgmgmt.c
+++ b/src/usr/imgmgmt.c
@@ -156,13 +156,17 @@ int imgacquire ( const char *name_uri, unsigned long timeout,
  * @v image            Executable/loadable image
  */
 void imgstat ( struct image *image ) {
+       struct image_tag *tag;
+
        printf ( "%s : %zd bytes", image->name, image->len );
        if ( image->type )
                printf ( " [%s]", image->type->name );
+       for_each_table_entry ( tag, IMAGE_TAGS ) {
+               if ( tag->image == image )
+                       printf ( " [%s]", tag->name );
+       }
        if ( image->flags & IMAGE_TRUSTED )
                printf ( " [TRUSTED]" );
-       if ( image->flags & IMAGE_SELECTED )
-               printf ( " [SELECTED]" );
        if ( image->flags & IMAGE_AUTO_UNREGISTER )
                printf ( " [AUTOFREE]" );
        if ( image->flags & IMAGE_HIDDEN )