[U-Boot] [PATCH v3 0/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk

[U-Boot] [PATCH v3 0/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk

[AKASHI Takahiro]

Changes in v3 (Oct 4, 2019)
* add patch#1
* use newly-added fs_get_type() instead fo fs_get_type_name()
* check for FS_TYPE_ANY to confirm if a file system exists or not
* remove a redundant check on whether a device is a partition
  or a whole disk

Changes in v2 (Sept 12, 2019)
* add patch#1 and #2
* install the protocol only if a file system does exist

AKASHI Takahiro (4):
  fs: export fs_close()
  fs: add fs_get_type() for current filesystem type
  efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available
  efi_loader: disk: install file system protocol to a whole disk

 fs/fs.c                   | 15 ++++++++++++++-
 include/fs.h              | 17 +++++++++++++++++
 lib/efi_loader/efi_disk.c | 17 ++++++++++++++++-
 3 files changed, 47 insertions(+), 2 deletions(-)

-- 
2.21.0

[U-Boot] [PATCH v3 1/4] fs: export fs_close()

[AKASHI Takahiro]

This function is always paired with either fs_set_blk_desc() or
fs_set_blk_desc_with_part(). So just export it.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fs.c      | 2 +-
 include/fs.h | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index d8a4ced4698e..64ba25fea8bf 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -389,7 +389,7 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part)
 	return -1;
 }
 
-static void fs_close(void)
+void fs_close(void)
 {
 	struct fstype_info *info = fs_get_info(fs_type);
 
diff --git a/include/fs.h b/include/fs.h
index 7601b0343bcd..5a1244d57fd2 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -37,6 +37,13 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);
  */
 int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
 
+/**
+ * fs_close() - Unset current block device and partition
+ *
+ * Should be paired with either fs_set_blk_dev() or fs_set_dev_with_part()
+ */
+void fs_close(void);
+
 /**
  * fs_get_type_name() - Get type of current filesystem
  *
-- 
2.21.0

[U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type

[AKASHI Takahiro]

This function is a variant of fs_get_type_name() and returns a filesystem
type with which the current device is associated.
We don't want to export fs_type variable directly because we have to take
care of it consistently within fs.c.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 fs/fs.c      | 13 +++++++++++++
 include/fs.h | 10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/fs/fs.c b/fs/fs.c
index 64ba25fea8bf..e5307dbeaa37 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -307,6 +307,19 @@ static struct fstype_info *fs_get_info(int fstype)
 	return info;
 }
 
+/**
+ * fs_get_type() - Get type of current filesystem
+ *
+ * Return: filesystem type
+ *
+ * Returns filesystem type representing the current filesystem, or
+ * FS_TYPE_ANY for any unrecognised filesystem.
+ */
+int fs_get_type(void)
+{
+	return fs_type;
+}
+
 /**
  * fs_get_type_name() - Get type of current filesystem
  *
diff --git a/include/fs.h b/include/fs.h
index 5a1244d57fd2..6dfdb5c5307a 100644
--- a/include/fs.h
+++ b/include/fs.h
@@ -44,6 +44,16 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
  */
 void fs_close(void);
 
+/**
+ * fs_get_type() - Get type of current filesystem
+ *
+ * Return: filesystem type
+ *
+ * Returns filesystem type representing the current filesystem, or
+ * FS_TYPE_ANY for any unrecognised filesystem.
+ */
+int fs_get_type(void);
+
 /**
  * fs_get_type_name() - Get type of current filesystem
  *
-- 
2.21.0

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

[AKASHI Takahiro]

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

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,
-- 
2.21.0

[U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk

[AKASHI Takahiro]

Currently, a whole disk without any partitions is not associated
with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some
file system, there is a chance that we may not be able to access
it, particularly, when accesses are to be attempted after searching
that protocol against a device handle.

With this patch, EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is installed
to such a disk if part_get_info() shows there is no partition
table installed on it.

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

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 27329cadb6f1..4e1a1efed7ec 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -329,7 +329,8 @@ static efi_status_t efi_disk_add_dev(
 			       diskobj->dp);
 	if (ret != EFI_SUCCESS)
 		return ret;
-	if (part >= 1 && efi_fs_exists(desc, part)) {
+	/* partitions or whole disk without partitions */
+	if (efi_fs_exists(desc, part)) {
 		diskobj->volume = efi_simple_file_system(desc, part,
 							 diskobj->dp);
 		ret = efi_add_protocol(&diskobj->header,
-- 
2.21.0

[U-Boot] [PATCH v3 1/4] fs: export fs_close()

[Heinrich Schuchardt]

On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> This function is always paired with either fs_set_blk_desc() or
> fs_set_blk_desc_with_part(). So just export it.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  fs/fs.c      | 2 +-
>  include/fs.h | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index d8a4ced4698e..64ba25fea8bf 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -389,7 +389,7 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part)
>  	return -1;
>  }
>
> -static void fs_close(void)
> +void fs_close(void)
>  {
>  	struct fstype_info *info = fs_get_info(fs_type);
>
> diff --git a/include/fs.h b/include/fs.h
> index 7601b0343bcd..5a1244d57fd2 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -37,6 +37,13 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);
>   */
>  int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
>
> +/**
> + * fs_close() - Unset current block device and partition
> + *
> + * Should be paired with either fs_set_blk_dev() or fs_set_dev_with_part()

This "paired" implies that one should call one of the name functions
afterwards which may not be necessary. I would suggest:

"fs_close() closes the connection to a file system which opened with
either fs_set_blk_dev() or fs_set_dev_with_part(). Many file functions
implicitly call fs_close(), e.g. fs_closedir(), fs_exist(), fs_ln(),
fs_ls(), fs_mkdir(), fs_read(), fs_size(), fs_write(), fs_unlink()."

Best regards

Heinrich Schuchardt

> + */
> +void fs_close(void);
> +
>  /**
>   * fs_get_type_name() - Get type of current filesystem
>   *
>

[U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type

[Heinrich Schuchardt]

On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> This function is a variant of fs_get_type_name() and returns a filesystem
> type with which the current device is associated.
> We don't want to export fs_type variable directly because we have to take
> care of it consistently within fs.c.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Looking at fs/fs.c there seems to be a lot of inconsistency in the usage
of FS_TYPE_ANY. Some of the file system functions set fs_type =
FS_TYPE_ANY before calling fs_close(). Others don't. Shouldn't we move
those assignments to fs_close() to get consistency?

FS_TYPE_ANY seems to be misnomer and could be replaced by FS_TYPE_NONE.

Please, use scripts/get_maintainer in future to determine the addressees
of patches. I have put the missing ones on CC now.

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


> ---
>  fs/fs.c      | 13 +++++++++++++
>  include/fs.h | 10 ++++++++++
>  2 files changed, 23 insertions(+)
>
> diff --git a/fs/fs.c b/fs/fs.c
> index 64ba25fea8bf..e5307dbeaa37 100644
> --- a/fs/fs.c
> +++ b/fs/fs.c
> @@ -307,6 +307,19 @@ static struct fstype_info *fs_get_info(int fstype)
>  	return info;
>  }
>
> +/**
> + * fs_get_type() - Get type of current filesystem
> + *
> + * Return: filesystem type
> + *
> + * Returns filesystem type representing the current filesystem, or
> + * FS_TYPE_ANY for any unrecognised filesystem.
> + */
> +int fs_get_type(void)
> +{
> +	return fs_type;
> +}
> +
>  /**
>   * fs_get_type_name() - Get type of current filesystem
>   *
> diff --git a/include/fs.h b/include/fs.h
> index 5a1244d57fd2..6dfdb5c5307a 100644
> --- a/include/fs.h
> +++ b/include/fs.h
> @@ -44,6 +44,16 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
>   */
>  void fs_close(void);
>
> +/**
> + * fs_get_type() - Get type of current filesystem
> + *
> + * Return: filesystem type
> + *
> + * Returns filesystem type representing the current filesystem, or
> + * FS_TYPE_ANY for any unrecognised filesystem.
> + */
> +int fs_get_type(void);
> +
>  /**
>   * fs_get_type_name() - Get type of current filesystem
>   *
>

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

[Heinrich Schuchardt]

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,
>

[U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk

[Heinrich Schuchardt]

On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> Currently, a whole disk without any partitions is not associated
> with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some
> file system, there is a chance that we may not be able to access
> it, particularly, when accesses are to be attempted after searching
> that protocol against a device handle.
>
> With this patch, EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is installed
> to such a disk if part_get_info() shows there is no partition
> table installed on it.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
>  lib/efi_loader/efi_disk.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 27329cadb6f1..4e1a1efed7ec 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -329,7 +329,8 @@ static efi_status_t efi_disk_add_dev(
>  			       diskobj->dp);
>  	if (ret != EFI_SUCCESS)
>  		return ret;
> -	if (part >= 1 && efi_fs_exists(desc, part)) {
> +	/* partitions or whole disk without partitions */
> +	if (efi_fs_exists(desc, part)) {

As described in response to an earlier version of the patch series we
should only install the EFI protocols if no partition table exits.

Best regards

Heinrich Schuchardt

>  		diskobj->volume = efi_simple_file_system(desc, part,
>  							 diskobj->dp);
>  		ret = efi_add_protocol(&diskobj->header,
>

[U-Boot] [PATCH v3 1/4] fs: export fs_close()

[AKASHI Takahiro]

On Fri, Oct 04, 2019 at 08:53:00PM +0200, Heinrich Schuchardt wrote:
> On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> > This function is always paired with either fs_set_blk_desc() or
> > fs_set_blk_desc_with_part(). So just export it.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  fs/fs.c      | 2 +-
> >  include/fs.h | 7 +++++++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/fs/fs.c b/fs/fs.c
> > index d8a4ced4698e..64ba25fea8bf 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -389,7 +389,7 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part)
> >  	return -1;
> >  }
> >
> > -static void fs_close(void)
> > +void fs_close(void)
> >  {
> >  	struct fstype_info *info = fs_get_info(fs_type);
> >
> > diff --git a/include/fs.h b/include/fs.h
> > index 7601b0343bcd..5a1244d57fd2 100644
> > --- a/include/fs.h
> > +++ b/include/fs.h
> > @@ -37,6 +37,13 @@ int fs_set_blk_dev(const char *ifname, const char *dev_part_str, int fstype);
> >   */
> >  int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
> >
> > +/**
> > + * fs_close() - Unset current block device and partition
> > + *
> > + * Should be paired with either fs_set_blk_dev() or fs_set_dev_with_part()
> 
> This "paired" implies that one should call one of the name functions
> afterwards which may not be necessary. I would suggest:
> 
> "fs_close() closes the connection to a file system which opened with
> either fs_set_blk_dev() or fs_set_dev_with_part(). Many file functions
> implicitly call fs_close(), e.g. fs_closedir(), fs_exist(), fs_ln(),
> fs_ls(), fs_mkdir(), fs_read(), fs_size(), fs_write(), fs_unlink()."

Okay.

-Takahiro Akashi


> Best regards
> 
> Heinrich Schuchardt
> 
> > + */
> > +void fs_close(void);
> > +
> >  /**
> >   * fs_get_type_name() - Get type of current filesystem
> >   *
> >
> 

[U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type

[AKASHI Takahiro]

On Fri, Oct 04, 2019 at 09:04:59PM +0200, Heinrich Schuchardt wrote:
> On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> > This function is a variant of fs_get_type_name() and returns a filesystem
> > type with which the current device is associated.
> > We don't want to export fs_type variable directly because we have to take
> > care of it consistently within fs.c.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> 
> Looking at fs/fs.c there seems to be a lot of inconsistency in the usage
> of FS_TYPE_ANY. Some of the file system functions set fs_type =
> FS_TYPE_ANY before calling fs_close(). Others don't. Shouldn't we move
> those assignments to fs_close() to get consistency?

Another patch.

> FS_TYPE_ANY seems to be misnomer and could be replaced by FS_TYPE_NONE.
> 
> Please, use scripts/get_maintainer in future to determine the addressees
> of patches. I have put the missing ones on CC now.

There is no dedicated maintainer for "fs" sub-system,
so Tom was included here after MAINTAINERS.

-Takahiro Akashi


> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> 
> 
> > ---
> >  fs/fs.c      | 13 +++++++++++++
> >  include/fs.h | 10 ++++++++++
> >  2 files changed, 23 insertions(+)
> >
> > diff --git a/fs/fs.c b/fs/fs.c
> > index 64ba25fea8bf..e5307dbeaa37 100644
> > --- a/fs/fs.c
> > +++ b/fs/fs.c
> > @@ -307,6 +307,19 @@ static struct fstype_info *fs_get_info(int fstype)
> >  	return info;
> >  }
> >
> > +/**
> > + * fs_get_type() - Get type of current filesystem
> > + *
> > + * Return: filesystem type
> > + *
> > + * Returns filesystem type representing the current filesystem, or
> > + * FS_TYPE_ANY for any unrecognised filesystem.
> > + */
> > +int fs_get_type(void)
> > +{
> > +	return fs_type;
> > +}
> > +
> >  /**
> >   * fs_get_type_name() - Get type of current filesystem
> >   *
> > diff --git a/include/fs.h b/include/fs.h
> > index 5a1244d57fd2..6dfdb5c5307a 100644
> > --- a/include/fs.h
> > +++ b/include/fs.h
> > @@ -44,6 +44,16 @@ int fs_set_blk_dev_with_part(struct blk_desc *desc, int part);
> >   */
> >  void fs_close(void);
> >
> > +/**
> > + * fs_get_type() - Get type of current filesystem
> > + *
> > + * Return: filesystem type
> > + *
> > + * Returns filesystem type representing the current filesystem, or
> > + * FS_TYPE_ANY for any unrecognised filesystem.
> > + */
> > +int fs_get_type(void);
> > +
> >  /**
> >   * fs_get_type_name() - Get type of current filesystem
> >   *
> >
> 

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

[AKASHI Takahiro]

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,
> >
> 

[U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk

[AKASHI Takahiro]

On Fri, Oct 04, 2019 at 09:15:19PM +0200, Heinrich Schuchardt wrote:
> On 10/4/19 5:05 AM, AKASHI Takahiro wrote:
> > Currently, a whole disk without any partitions is not associated
> > with EFI_SIMPLE_FILE_SYSTEM_PROTOCOL. So even if it houses some
> > file system, there is a chance that we may not be able to access
> > it, particularly, when accesses are to be attempted after searching
> > that protocol against a device handle.
> >
> > With this patch, EFI_SIMPLE_FILE_SYSTEM_PROTOCOL is installed
> > to such a disk if part_get_info() shows there is no partition
> > table installed on it.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > ---
> >  lib/efi_loader/efi_disk.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 27329cadb6f1..4e1a1efed7ec 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -329,7 +329,8 @@ static efi_status_t efi_disk_add_dev(
> >  			       diskobj->dp);
> >  	if (ret != EFI_SUCCESS)
> >  		return ret;
> > -	if (part >= 1 && efi_fs_exists(desc, part)) {
> > +	/* partitions or whole disk without partitions */
> > +	if (efi_fs_exists(desc, part)) {
> 
> As described in response to an earlier version of the patch series we
> should only install the EFI protocols if no partition table exits.

That is why I removed "part >= 1" check here.
efi_fs_exists() should work both for a whole disk (part == 0)
or any partitions on a disk.

Thanks,
-Takahiro Akashi

> Best regards
> 
> Heinrich Schuchardt
> 
> >  		diskobj->volume = efi_simple_file_system(desc, part,
> >  							 diskobj->dp);
> >  		ret = efi_add_protocol(&diskobj->header,
> >
>