Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-10-03 Thread AKASHI Takahiro
On Thu, Oct 03, 2019 at 03:44:54PM +0200, Heinrich Schuchardt wrote:
> On 10/3/19 9:09 AM, AKASHI Takahiro wrote:
> >Heinrich,
> >
> >On Thu, Sep 12, 2019 at 06:51:35PM +0900, AKASHI Takahiro wrote:
> >>On Thu, Sep 12, 2019 at 11:43:07AM +0200, Heinrich Schuchardt wrote:
> >>>On 9/12/19 11:17 AM, AKASHI Takahiro wrote:
> On Thu, Sep 12, 2019 at 10:57:20AM +0200, Heinrich Schuchardt wrote:
> >On 9/12/19 6:51 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 FAT file system
> >>exists.
> >>
> >>Signed-off-by: AKASHI Takahiro 
> >>---
> >>  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 7a6b06821a47..d72f455901f2 100644
> >>--- a/lib/efi_loader/efi_disk.c
> >>+++ b/lib/efi_loader/efi_disk.c
> >>@@ -9,6 +9,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >>+#include 
> >>  #include 
> >>  #include 
> >>
> >>@@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))
> >
> >Before your patch we could use any supported file system (e.g. EXT2). I
> >see no need for a restriction to FAT. You could compare the string to
> >"unsupported":
> 
> No. As far as you want to stick to compliance to UEFI specification,
> "fat" is the only file system supported by UEFI.
> >>>
> >>>In the case of device path node VenHw() there is a direct rule in the
> >>>spec indicating how it should be rendered. I have not seen anything in
> >>>the UEFI spec saying that you should not support file systems besides
> >>>FAT. So there is no compliance issue. I would be reluctant to remove an
> >>>existing capability of U-Boot.
> >>
> >>See section 13.3. It says,
> >> The file system supported by the Extensible Firmware Interface is
> >> based on the FAT file system.
> >
> >Any comments here?
> 
> The spec does not say: Thou shalt not support other file systems.

If we admit such an excuse for arbitrary extension,
it will easily break compatibility in general.

-Takahiro Akashi


> >
> 
> >if (!strcmp(fs_get_type_name(), "unsupported"))
> > return 0;
> >
> >But wouldn't it be preferable to have a function to access fs_type (in
> >fs/fs.c) directly instead of a string representation?
> 
> Agree, but there is no direct function in fs/fs.c.
> I'm reluctant to invent a new function just for this purpose.
> >>>
> >>>In that case we should compare to a string that is defined as constant
> >>>in fs.h.
> >
> >Are you saying that we should add new macros in fs.h?
> >All the file system names are initialized in fs/fs.c now.
> >
> >>-Takahiro Akashi
> >>
> >>>
> >>>Best regards
> >>>
> >>>Heinrich
> >>>
> 
> -Takahiro Akashi
> 
> 
> >Otherwise we should convert the string "unsupported" of fstypes[] into a
> >constant in fs.h so that we can be sure we are using the same value.
> >
> >Best regards
> >
> >Heinrich
> >
> >>+   return 0;
> >>+
> >>+   fs_close();
> >>+
> >>+   return 1;
> >>+}
> >>+
> >>  /*
> >>   * Create a handle for a partition or disk
> >>   *
> >>@@ -270,7 +284,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(>header,
> >>
> >
> 
> >>>
> >
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-10-03 Thread Heinrich Schuchardt

On 10/3/19 9:09 AM, AKASHI Takahiro wrote:

Heinrich,

On Thu, Sep 12, 2019 at 06:51:35PM +0900, AKASHI Takahiro wrote:

On Thu, Sep 12, 2019 at 11:43:07AM +0200, Heinrich Schuchardt wrote:

On 9/12/19 11:17 AM, AKASHI Takahiro wrote:

On Thu, Sep 12, 2019 at 10:57:20AM +0200, Heinrich Schuchardt wrote:

On 9/12/19 6:51 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 FAT file system
exists.

Signed-off-by: AKASHI Takahiro 
---
  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 7a6b06821a47..d72f455901f2 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -9,6 +9,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 

@@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))


Before your patch we could use any supported file system (e.g. EXT2). I
see no need for a restriction to FAT. You could compare the string to
"unsupported":


No. As far as you want to stick to compliance to UEFI specification,
"fat" is the only file system supported by UEFI.


In the case of device path node VenHw() there is a direct rule in the
spec indicating how it should be rendered. I have not seen anything in
the UEFI spec saying that you should not support file systems besides
FAT. So there is no compliance issue. I would be reluctant to remove an
existing capability of U-Boot.


See section 13.3. It says,
 The file system supported by the Extensible Firmware Interface is
 based on the FAT file system.


Any comments here?


The spec does not say: Thou shalt not support other file systems.






if (!strcmp(fs_get_type_name(), "unsupported"))
return 0;

But wouldn't it be preferable to have a function to access fs_type (in
fs/fs.c) directly instead of a string representation?


Agree, but there is no direct function in fs/fs.c.
I'm reluctant to invent a new function just for this purpose.


In that case we should compare to a string that is defined as constant
in fs.h.


Are you saying that we should add new macros in fs.h?
All the file system names are initialized in fs/fs.c now.


-Takahiro Akashi



Best regards

Heinrich



-Takahiro Akashi



Otherwise we should convert the string "unsupported" of fstypes[] into a
constant in fs.h so that we can be sure we are using the same value.

Best regards

Heinrich


+   return 0;
+
+   fs_close();
+
+   return 1;
+}
+
  /*
   * Create a handle for a partition or disk
   *
@@ -270,7 +284,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(>header,











___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-10-03 Thread AKASHI Takahiro
Heinrich,

On Thu, Sep 12, 2019 at 06:51:35PM +0900, AKASHI Takahiro wrote:
> On Thu, Sep 12, 2019 at 11:43:07AM +0200, Heinrich Schuchardt wrote:
> > On 9/12/19 11:17 AM, AKASHI Takahiro wrote:
> > > On Thu, Sep 12, 2019 at 10:57:20AM +0200, Heinrich Schuchardt wrote:
> > >> On 9/12/19 6:51 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 FAT file system
> > >>> exists.
> > >>>
> > >>> Signed-off-by: AKASHI Takahiro 
> > >>> ---
> > >>>  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 7a6b06821a47..d72f455901f2 100644
> > >>> --- a/lib/efi_loader/efi_disk.c
> > >>> +++ b/lib/efi_loader/efi_disk.c
> > >>> @@ -9,6 +9,7 @@
> > >>>  #include 
> > >>>  #include 
> > >>>  #include 
> > >>> +#include 
> > >>>  #include 
> > >>>  #include 
> > >>>
> > >>> @@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))
> > >>
> > >> Before your patch we could use any supported file system (e.g. EXT2). I
> > >> see no need for a restriction to FAT. You could compare the string to
> > >> "unsupported":
> > >
> > > No. As far as you want to stick to compliance to UEFI specification,
> > > "fat" is the only file system supported by UEFI.
> > 
> > In the case of device path node VenHw() there is a direct rule in the
> > spec indicating how it should be rendered. I have not seen anything in
> > the UEFI spec saying that you should not support file systems besides
> > FAT. So there is no compliance issue. I would be reluctant to remove an
> > existing capability of U-Boot.
> 
> See section 13.3. It says,
> The file system supported by the Extensible Firmware Interface is
> based on the FAT file system.

Any comments here?

> > >
> > >> if (!strcmp(fs_get_type_name(), "unsupported"))
> > >>  return 0;
> > >>
> > >> But wouldn't it be preferable to have a function to access fs_type (in
> > >> fs/fs.c) directly instead of a string representation?
> > >
> > > Agree, but there is no direct function in fs/fs.c.
> > > I'm reluctant to invent a new function just for this purpose.
> > 
> > In that case we should compare to a string that is defined as constant
> > in fs.h.

Are you saying that we should add new macros in fs.h?
All the file system names are initialized in fs/fs.c now.

> -Takahiro Akashi
> 
> > 
> > Best regards
> > 
> > Heinrich
> > 
> > >
> > > -Takahiro Akashi
> > >
> > >
> > >> Otherwise we should convert the string "unsupported" of fstypes[] into a
> > >> constant in fs.h so that we can be sure we are using the same value.
> > >>
> > >> Best regards
> > >>
> > >> Heinrich
> > >>
> > >>> +   return 0;
> > >>> +
> > >>> +   fs_close();
> > >>> +
> > >>> +   return 1;
> > >>> +}
> > >>> +
> > >>>  /*
> > >>>   * Create a handle for a partition or disk
> > >>>   *
> > >>> @@ -270,7 +284,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(>header,
> > >>>
> > >>
> > >
> > 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-09-12 Thread AKASHI Takahiro
On Thu, Sep 12, 2019 at 11:43:07AM +0200, Heinrich Schuchardt wrote:
> On 9/12/19 11:17 AM, AKASHI Takahiro wrote:
> > On Thu, Sep 12, 2019 at 10:57:20AM +0200, Heinrich Schuchardt wrote:
> >> On 9/12/19 6:51 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 FAT file system
> >>> exists.
> >>>
> >>> Signed-off-by: AKASHI Takahiro 
> >>> ---
> >>>  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 7a6b06821a47..d72f455901f2 100644
> >>> --- a/lib/efi_loader/efi_disk.c
> >>> +++ b/lib/efi_loader/efi_disk.c
> >>> @@ -9,6 +9,7 @@
> >>>  #include 
> >>>  #include 
> >>>  #include 
> >>> +#include 
> >>>  #include 
> >>>  #include 
> >>>
> >>> @@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))
> >>
> >> Before your patch we could use any supported file system (e.g. EXT2). I
> >> see no need for a restriction to FAT. You could compare the string to
> >> "unsupported":
> >
> > No. As far as you want to stick to compliance to UEFI specification,
> > "fat" is the only file system supported by UEFI.
> 
> In the case of device path node VenHw() there is a direct rule in the
> spec indicating how it should be rendered. I have not seen anything in
> the UEFI spec saying that you should not support file systems besides
> FAT. So there is no compliance issue. I would be reluctant to remove an
> existing capability of U-Boot.

See section 13.3. It says,
The file system supported by the Extensible Firmware Interface is
based on the FAT file system.

-Takahiro Akashi

> >
> >> if (!strcmp(fs_get_type_name(), "unsupported"))
> >>return 0;
> >>
> >> But wouldn't it be preferable to have a function to access fs_type (in
> >> fs/fs.c) directly instead of a string representation?
> >
> > Agree, but there is no direct function in fs/fs.c.
> > I'm reluctant to invent a new function just for this purpose.
> 
> In that case we should compare to a string that is defined as constant
> in fs.h.
> 
> Best regards
> 
> Heinrich
> 
> >
> > -Takahiro Akashi
> >
> >
> >> Otherwise we should convert the string "unsupported" of fstypes[] into a
> >> constant in fs.h so that we can be sure we are using the same value.
> >>
> >> Best regards
> >>
> >> Heinrich
> >>
> >>> + return 0;
> >>> +
> >>> + fs_close();
> >>> +
> >>> + return 1;
> >>> +}
> >>> +
> >>>  /*
> >>>   * Create a handle for a partition or disk
> >>>   *
> >>> @@ -270,7 +284,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(>header,
> >>>
> >>
> >
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-09-12 Thread Heinrich Schuchardt
On 9/12/19 11:17 AM, AKASHI Takahiro wrote:
> On Thu, Sep 12, 2019 at 10:57:20AM +0200, Heinrich Schuchardt wrote:
>> On 9/12/19 6:51 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 FAT file system
>>> exists.
>>>
>>> Signed-off-by: AKASHI Takahiro 
>>> ---
>>>  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 7a6b06821a47..d72f455901f2 100644
>>> --- a/lib/efi_loader/efi_disk.c
>>> +++ b/lib/efi_loader/efi_disk.c
>>> @@ -9,6 +9,7 @@
>>>  #include 
>>>  #include 
>>>  #include 
>>> +#include 
>>>  #include 
>>>  #include 
>>>
>>> @@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))
>>
>> Before your patch we could use any supported file system (e.g. EXT2). I
>> see no need for a restriction to FAT. You could compare the string to
>> "unsupported":
>
> No. As far as you want to stick to compliance to UEFI specification,
> "fat" is the only file system supported by UEFI.

In the case of device path node VenHw() there is a direct rule in the
spec indicating how it should be rendered. I have not seen anything in
the UEFI spec saying that you should not support file systems besides
FAT. So there is no compliance issue. I would be reluctant to remove an
existing capability of U-Boot.

>
>> if (!strcmp(fs_get_type_name(), "unsupported"))
>>  return 0;
>>
>> But wouldn't it be preferable to have a function to access fs_type (in
>> fs/fs.c) directly instead of a string representation?
>
> Agree, but there is no direct function in fs/fs.c.
> I'm reluctant to invent a new function just for this purpose.

In that case we should compare to a string that is defined as constant
in fs.h.

Best regards

Heinrich

>
> -Takahiro Akashi
>
>
>> Otherwise we should convert the string "unsupported" of fstypes[] into a
>> constant in fs.h so that we can be sure we are using the same value.
>>
>> Best regards
>>
>> Heinrich
>>
>>> +   return 0;
>>> +
>>> +   fs_close();
>>> +
>>> +   return 1;
>>> +}
>>> +
>>>  /*
>>>   * Create a handle for a partition or disk
>>>   *
>>> @@ -270,7 +284,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(>header,
>>>
>>
>

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-09-12 Thread AKASHI Takahiro
On Thu, Sep 12, 2019 at 10:57:20AM +0200, Heinrich Schuchardt wrote:
> On 9/12/19 6:51 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 FAT file system
> > exists.
> >
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >  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 7a6b06821a47..d72f455901f2 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -9,6 +9,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))
> 
> Before your patch we could use any supported file system (e.g. EXT2). I
> see no need for a restriction to FAT. You could compare the string to
> "unsupported":

No. As far as you want to stick to compliance to UEFI specification,
"fat" is the only file system supported by UEFI.

> if (!strcmp(fs_get_type_name(), "unsupported"))
>   return 0;
> 
> But wouldn't it be preferable to have a function to access fs_type (in
> fs/fs.c) directly instead of a string representation?

Agree, but there is no direct function in fs/fs.c.
I'm reluctant to invent a new function just for this purpose.

-Takahiro Akashi


> Otherwise we should convert the string "unsupported" of fstypes[] into a
> constant in fs.h so that we can be sure we are using the same value.
> 
> Best regards
> 
> Heinrich
> 
> > +   return 0;
> > +
> > +   fs_close();
> > +
> > +   return 1;
> > +}
> > +
> >  /*
> >   * Create a handle for a partition or disk
> >   *
> > @@ -270,7 +284,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(>header,
> >
> 
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


Re: [U-Boot] [PATCH v2 2/3] efi_loader: disk: install FILE_SYSTEM_PROTOCOL only if available

2019-09-12 Thread Heinrich Schuchardt
On 9/12/19 6:51 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 FAT file system
> exists.
>
> Signed-off-by: AKASHI Takahiro 
> ---
>  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 7a6b06821a47..d72f455901f2 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
> @@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))

Before your patch we could use any supported file system (e.g. EXT2). I
see no need for a restriction to FAT. You could compare the string to
"unsupported":

if (!strcmp(fs_get_type_name(), "unsupported"))
return 0;

But wouldn't it be preferable to have a function to access fs_type (in
fs/fs.c) directly instead of a string representation?

Otherwise we should convert the string "unsupported" of fstypes[] into a
constant in fs.h so that we can be sure we are using the same value.

Best regards

Heinrich

> + return 0;
> +
> + fs_close();
> +
> + return 1;
> +}
> +
>  /*
>   * Create a handle for a partition or disk
>   *
> @@ -270,7 +284,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(>header,
>

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


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

2019-09-11 Thread 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 FAT file system
exists.

Signed-off-by: AKASHI Takahiro 
---
 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 7a6b06821a47..d72f455901f2 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -217,6 +218,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 (strcmp(fs_get_type_name(), "fat"))
+   return 0;
+
+   fs_close();
+
+   return 1;
+}
+
 /*
  * Create a handle for a partition or disk
  *
@@ -270,7 +284,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(>header,
-- 
2.21.0

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot