diff mbox series

[U-Boot,v3,3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

Message ID 20191004030523.21579-4-takahiro.akashi@linaro.org
State Superseded
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk | expand

Commit Message

AKASHI Takahiro Oct. 4, 2019, 3:05 a.m. UTC
In the current implementation, EFI_SIMPLEFILE_SYSTEM_PROTOCOL is always
installed to all the partitions even if some of them may house no file
system.

With this patch, that protocol will be installed only if any file system
exists.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 lib/efi_loader/efi_disk.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Comments

Heinrich Schuchardt Oct. 4, 2019, 7:13 p.m. UTC | #1
On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> In the current implementation, EFI_SIMPLEFILE_SYSTEM_PROTOCOL is always
> installed to all the partitions even if some of them may house no file
> system.
>
> With this patch, that protocol will be installed only if any file system
> exists.

I generally prefer if each individual patch also has a version history.

>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_loader/efi_disk.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 9007a5f77f3d..27329cadb6f1 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -9,6 +9,7 @@
>  #include <blk.h>
>  #include <dm.h>
>  #include <efi_loader.h>
> +#include <fs.h>
>  #include <part.h>
>  #include <malloc.h>
>
> @@ -262,6 +263,19 @@ efi_fs_from_path(struct efi_device_path *full_path)
>  	return handler->protocol_interface;
>  }
>

Please, add:

/**
 * efi_fs_exists() - check if a partition bears a file system
 *
 * @desc:	block device descriptor
 * @part:	partition number
 * Return:	1 if a file system exists on the partition
 *		0 otherwise
 */

Otherwise

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> +static int efi_fs_exists(struct blk_desc *desc, int part)
> +{
> +	if (fs_set_blk_dev_with_part(desc, part))
> +		return 0;
> +
> +	if (fs_get_type() == FS_TYPE_ANY)
> +		return 0;
> +
> +	fs_close();
> +
> +	return 1;
> +}
> +
>  /*
>   * Create a handle for a partition or disk
>   *
> @@ -315,7 +329,7 @@ static efi_status_t efi_disk_add_dev(
>  			       diskobj->dp);
>  	if (ret != EFI_SUCCESS)
>  		return ret;
> -	if (part >= 1) {
> +	if (part >= 1 && efi_fs_exists(desc, part)) {
>  		diskobj->volume = efi_simple_file_system(desc, part,
>  							 diskobj->dp);
>  		ret = efi_add_protocol(&diskobj->header,
>
AKASHI Takahiro Oct. 7, 2019, 2:15 a.m. UTC | #2
On Fri, Oct 04, 2019 at 09:13:08PM +0200, Heinrich Schuchardt wrote:
> On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> > In the current implementation, EFI_SIMPLEFILE_SYSTEM_PROTOCOL is always
> > installed to all the partitions even if some of them may house no file
> > system.
> >
> > With this patch, that protocol will be installed only if any file system
> > exists.
> 
> I generally prefer if each individual patch also has a version history.

It's not my style.

> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  lib/efi_loader/efi_disk.c | 16 +++++++++++++++-
> >  1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 9007a5f77f3d..27329cadb6f1 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -9,6 +9,7 @@
> >  #include <blk.h>
> >  #include <dm.h>
> >  #include <efi_loader.h>
> > +#include <fs.h>
> >  #include <part.h>
> >  #include <malloc.h>
> >
> > @@ -262,6 +263,19 @@ efi_fs_from_path(struct efi_device_path *full_path)
> >  	return handler->protocol_interface;
> >  }
> >
> 
> Please, add:
> 
> /**
>  * efi_fs_exists() - check if a partition bears a file system
>  *
>  * @desc:	block device descriptor
>  * @part:	partition number
>  * Return:	1 if a file system exists on the partition
>  *		0 otherwise
>  */
> 
> Otherwise
> 
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

Okay, thanks
-Takahiro Akashi


> > +static int efi_fs_exists(struct blk_desc *desc, int part)
> > +{
> > +	if (fs_set_blk_dev_with_part(desc, part))
> > +		return 0;
> > +
> > +	if (fs_get_type() == FS_TYPE_ANY)
> > +		return 0;
> > +
> > +	fs_close();
> > +
> > +	return 1;
> > +}
> > +
> >  /*
> >   * Create a handle for a partition or disk
> >   *
> > @@ -315,7 +329,7 @@ static efi_status_t efi_disk_add_dev(
> >  			       diskobj->dp);
> >  	if (ret != EFI_SUCCESS)
> >  		return ret;
> > -	if (part >= 1) {
> > +	if (part >= 1 && efi_fs_exists(desc, part)) {
> >  		diskobj->volume = efi_simple_file_system(desc, part,
> >  							 diskobj->dp);
> >  		ret = efi_add_protocol(&diskobj->header,
> >
>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 9007a5f77f3d..27329cadb6f1 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -9,6 +9,7 @@ 
 #include <blk.h>
 #include <dm.h>
 #include <efi_loader.h>
+#include <fs.h>
 #include <part.h>
 #include <malloc.h>
 
@@ -262,6 +263,19 @@  efi_fs_from_path(struct efi_device_path *full_path)
 	return handler->protocol_interface;
 }
 
+static int efi_fs_exists(struct blk_desc *desc, int part)
+{
+	if (fs_set_blk_dev_with_part(desc, part))
+		return 0;
+
+	if (fs_get_type() == FS_TYPE_ANY)
+		return 0;
+
+	fs_close();
+
+	return 1;
+}
+
 /*
  * Create a handle for a partition or disk
  *
@@ -315,7 +329,7 @@  static efi_status_t efi_disk_add_dev(
 			       diskobj->dp);
 	if (ret != EFI_SUCCESS)
 		return ret;
-	if (part >= 1) {
+	if (part >= 1 && efi_fs_exists(desc, part)) {
 		diskobj->volume = efi_simple_file_system(desc, part,
 							 diskobj->dp);
 		ret = efi_add_protocol(&diskobj->header,