* [U-Boot] [PATCH v3 1/4] fs: export fs_close()
2019-10-04 3:05 [U-Boot] [PATCH v3 0/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro
@ 2019-10-04 3:05 ` AKASHI Takahiro
2019-10-04 18:53 ` Heinrich Schuchardt
2019-10-04 3:05 ` [U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type AKASHI Takahiro
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-04 3:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 1/4] fs: export fs_close()
2019-10-04 3:05 ` [U-Boot] [PATCH v3 1/4] fs: export fs_close() AKASHI Takahiro
@ 2019-10-04 18:53 ` Heinrich Schuchardt
2019-10-07 2:08 ` AKASHI Takahiro
0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2019-10-04 18:53 UTC (permalink / raw)
To: u-boot
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
> *
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 1/4] fs: export fs_close()
2019-10-04 18:53 ` Heinrich Schuchardt
@ 2019-10-07 2:08 ` AKASHI Takahiro
0 siblings, 0 replies; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-07 2:08 UTC (permalink / raw)
To: u-boot
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
> > *
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type
2019-10-04 3:05 [U-Boot] [PATCH v3 0/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro
2019-10-04 3:05 ` [U-Boot] [PATCH v3 1/4] fs: export fs_close() AKASHI Takahiro
@ 2019-10-04 3:05 ` AKASHI Takahiro
2019-10-04 19:04 ` Heinrich Schuchardt
2019-10-04 3:05 ` [U-Boot] [PATCH v3 3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro
2019-10-04 3:05 ` [U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro
3 siblings, 1 reply; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-04 3:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type
2019-10-04 3:05 ` [U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type AKASHI Takahiro
@ 2019-10-04 19:04 ` Heinrich Schuchardt
2019-10-07 2:13 ` AKASHI Takahiro
0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2019-10-04 19:04 UTC (permalink / raw)
To: u-boot
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
> *
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type
2019-10-04 19:04 ` Heinrich Schuchardt
@ 2019-10-07 2:13 ` AKASHI Takahiro
0 siblings, 0 replies; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-07 2:13 UTC (permalink / raw)
To: u-boot
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
> > *
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3 3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available
2019-10-04 3:05 [U-Boot] [PATCH v3 0/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro
2019-10-04 3:05 ` [U-Boot] [PATCH v3 1/4] fs: export fs_close() AKASHI Takahiro
2019-10-04 3:05 ` [U-Boot] [PATCH v3 2/4] fs: add fs_get_type() for current filesystem type AKASHI Takahiro
@ 2019-10-04 3:05 ` AKASHI Takahiro
2019-10-04 19:13 ` Heinrich Schuchardt
2019-10-04 3:05 ` [U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro
3 siblings, 1 reply; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-04 3:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available
2019-10-04 3:05 ` [U-Boot] [PATCH v3 3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro
@ 2019-10-04 19:13 ` Heinrich Schuchardt
2019-10-07 2:15 ` AKASHI Takahiro
0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2019-10-04 19:13 UTC (permalink / raw)
To: u-boot
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,
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available
2019-10-04 19:13 ` Heinrich Schuchardt
@ 2019-10-07 2:15 ` AKASHI Takahiro
0 siblings, 0 replies; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-07 2:15 UTC (permalink / raw)
To: u-boot
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,
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk
2019-10-04 3:05 [U-Boot] [PATCH v3 0/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL to whole disk AKASHI Takahiro
` (2 preceding siblings ...)
2019-10-04 3:05 ` [U-Boot] [PATCH v3 3/4] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available AKASHI Takahiro
@ 2019-10-04 3:05 ` AKASHI Takahiro
2019-10-04 19:15 ` Heinrich Schuchardt
3 siblings, 1 reply; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-04 3:05 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk
2019-10-04 3:05 ` [U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk AKASHI Takahiro
@ 2019-10-04 19:15 ` Heinrich Schuchardt
2019-10-07 2:27 ` AKASHI Takahiro
0 siblings, 1 reply; 13+ messages in thread
From: Heinrich Schuchardt @ 2019-10-04 19:15 UTC (permalink / raw)
To: u-boot
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,
>
^ permalink raw reply [flat|nested] 13+ messages in thread* [U-Boot] [PATCH v3 4/4] efi_loader: disk: install file system protocol to a whole disk
2019-10-04 19:15 ` Heinrich Schuchardt
@ 2019-10-07 2:27 ` AKASHI Takahiro
0 siblings, 0 replies; 13+ messages in thread
From: AKASHI Takahiro @ 2019-10-07 2:27 UTC (permalink / raw)
To: u-boot
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,
> >
>
^ permalink raw reply [flat|nested] 13+ messages in thread