* [PATCH 0/2] efi_loader: multi part device paths to text
@ 2021-02-18 17:30 Heinrich Schuchardt
2021-02-18 17:30 ` [PATCH 1/2] " Heinrich Schuchardt
2021-02-18 17:30 ` [PATCH 2/2] efi_selftest: multi part device path " Heinrich Schuchardt
0 siblings, 2 replies; 7+ messages in thread
From: Heinrich Schuchardt @ 2021-02-18 17:30 UTC (permalink / raw)
To: u-boot
Our current implementation of
EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi
part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly
described in the UEFI spec but mimics what EDK II does.
Provide a unit test for multi part device paths.
Heinrich Schuchardt (2):
efi_loader: multi part device paths to text
efi_selftest: multi part device path to text
lib/efi_loader/efi_device_path_to_text.c | 17 ++++--
lib/efi_selftest/efi_selftest_devicepath.c | 65 ++++++++++++++++++++++
2 files changed, 77 insertions(+), 5 deletions(-)
--
2.30.0
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] efi_loader: multi part device paths to text
2021-02-18 17:30 [PATCH 0/2] efi_loader: multi part device paths to text Heinrich Schuchardt
@ 2021-02-18 17:30 ` Heinrich Schuchardt
2021-02-19 10:13 ` Ilias Apalodimas
2021-02-18 17:30 ` [PATCH 2/2] efi_selftest: multi part device path " Heinrich Schuchardt
1 sibling, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2021-02-18 17:30 UTC (permalink / raw)
To: u-boot
Our current implementation of
EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi
part device paths after the first part. We should convert all parts.
Render device path instance ends as commas. This is not explicitly
described in the UEFI spec but mimics what EDK II does.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++-----
1 file changed, 12 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index 1aaa9f94fa..81b8ac23ba 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
if (!device_path)
goto out;
- while (device_path &&
- str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
- *str++ = '/';
- str = efi_convert_single_device_node_to_text(str, device_path);
- device_path = efi_dp_next(device_path);
+ while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
+ if (device_path->type == DEVICE_PATH_TYPE_END) {
+ if (device_path->sub_type !=
+ DEVICE_PATH_SUB_TYPE_INSTANCE_END)
+ break;
+ *str++ = ',';
+ } else {
+ *str++ = '/';
+ str = efi_convert_single_device_node_to_text(
+ str, device_path);
+ }
+ *(u8 **)&device_path += device_path->length;
}
text = efi_str_to_u16(buffer);
--
2.30.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] efi_selftest: multi part device path to text
2021-02-18 17:30 [PATCH 0/2] efi_loader: multi part device paths to text Heinrich Schuchardt
2021-02-18 17:30 ` [PATCH 1/2] " Heinrich Schuchardt
@ 2021-02-18 17:30 ` Heinrich Schuchardt
2021-02-19 10:15 ` Ilias Apalodimas
1 sibling, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2021-02-18 17:30 UTC (permalink / raw)
To: u-boot
Test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() for a multi
part device path.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
lib/efi_selftest/efi_selftest_devicepath.c | 65 ++++++++++++++++++++++
1 file changed, 65 insertions(+)
diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c
index 4ce3fad895..d87b9f7dcd 100644
--- a/lib/efi_selftest/efi_selftest_devicepath.c
+++ b/lib/efi_selftest/efi_selftest_devicepath.c
@@ -45,6 +45,55 @@ static u8 *dp1;
static u8 *dp2;
static u8 *dp3;
+static struct {
+ struct efi_device_path_sd_mmc_path sd1;
+ struct efi_device_path sep1;
+ struct efi_device_path_sd_mmc_path sd2;
+ struct efi_device_path sep2;
+ struct efi_device_path_sd_mmc_path sd3;
+ struct efi_device_path end;
+} multi_part_dp = {
+ {
+ {
+ DEVICE_PATH_TYPE_MESSAGING_DEVICE,
+ DEVICE_PATH_SUB_TYPE_MSG_SD,
+ sizeof(struct efi_device_path_sd_mmc_path),
+ },
+ 0,
+ },
+ {
+ DEVICE_PATH_TYPE_END,
+ DEVICE_PATH_SUB_TYPE_INSTANCE_END,
+ sizeof(struct efi_device_path),
+ },
+ {
+ {
+ DEVICE_PATH_TYPE_MESSAGING_DEVICE,
+ DEVICE_PATH_SUB_TYPE_MSG_SD,
+ sizeof(struct efi_device_path_sd_mmc_path),
+ },
+ 1,
+ },
+ {
+ DEVICE_PATH_TYPE_END,
+ DEVICE_PATH_SUB_TYPE_INSTANCE_END,
+ sizeof(struct efi_device_path),
+ },
+ {
+ {
+ DEVICE_PATH_TYPE_MESSAGING_DEVICE,
+ DEVICE_PATH_SUB_TYPE_MSG_SD,
+ sizeof(struct efi_device_path_sd_mmc_path),
+ },
+ 2,
+ },
+ {
+ DEVICE_PATH_TYPE_END,
+ DEVICE_PATH_SUB_TYPE_END,
+ sizeof(struct efi_device_path),
+ },
+};
+
struct efi_device_path_to_text_protocol *device_path_to_text;
/*
@@ -340,6 +389,22 @@ static int execute(void)
return EFI_ST_FAILURE;
}
+ string = device_path_to_text->convert_device_path_to_text(
+ (struct efi_device_path *)&multi_part_dp, true, false);
+ if (efi_st_strcmp_16_8(
+ string,
+ "/SD(0),/SD(1),/SD(2)")
+ ) {
+ efi_st_printf("multi_part_dp: %ps\n", string);
+ efi_st_error("Incorrect text from ConvertDevicePathToText\n");
+ return EFI_ST_FAILURE;
+ }
+ ret = boottime->free_pool(string);
+ if (ret != EFI_SUCCESS) {
+ efi_st_error("FreePool failed\n");
+ return EFI_ST_FAILURE;
+ }
+
/* Test ConvertDeviceNodeToText */
string = device_path_to_text->convert_device_node_to_text(
(struct efi_device_path *)&dp_node, true, false);
--
2.30.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 1/2] efi_loader: multi part device paths to text
2021-02-18 17:30 ` [PATCH 1/2] " Heinrich Schuchardt
@ 2021-02-19 10:13 ` Ilias Apalodimas
2021-02-19 11:45 ` Heinrich Schuchardt
0 siblings, 1 reply; 7+ messages in thread
From: Ilias Apalodimas @ 2021-02-19 10:13 UTC (permalink / raw)
To: u-boot
Hi Heinrich
On Thu, Feb 18, 2021 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
> Our current implementation of
> EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi
> part device paths after the first part. We should convert all parts.
>
> Render device path instance ends as commas. This is not explicitly
> described in the UEFI spec but mimics what EDK II does.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> index 1aaa9f94fa..81b8ac23ba 100644
> --- a/lib/efi_loader/efi_device_path_to_text.c
> +++ b/lib/efi_loader/efi_device_path_to_text.c
> @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
>
> if (!device_path)
> goto out;
> - while (device_path &&
> - str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
> - *str++ = '/';
> - str = efi_convert_single_device_node_to_text(str, device_path);
> - device_path = efi_dp_next(device_path);
> + while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
> + if (device_path->type == DEVICE_PATH_TYPE_END) {
> + if (device_path->sub_type !=
> + DEVICE_PATH_SUB_TYPE_INSTANCE_END)
maybe it's better to check explicitly for '== DEVICE_PATH_SUB_TYPE_END'?
> + break;
> + *str++ = ',';
> + } else {
> + *str++ = '/';
> + str = efi_convert_single_device_node_to_text(
> + str, device_path);
> + }
> + *(u8 **)&device_path += device_path->length;
> }
>
> text = efi_str_to_u16(buffer);
> --
> 2.30.0
>
Cheers
/Ilias
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] efi_selftest: multi part device path to text
2021-02-18 17:30 ` [PATCH 2/2] efi_selftest: multi part device path " Heinrich Schuchardt
@ 2021-02-19 10:15 ` Ilias Apalodimas
0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-02-19 10:15 UTC (permalink / raw)
To: u-boot
On Thu, Feb 18, 2021 at 06:30:44PM +0100, Heinrich Schuchardt wrote:
> Test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() for a multi
> part device path.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> lib/efi_selftest/efi_selftest_devicepath.c | 65 ++++++++++++++++++++++
> 1 file changed, 65 insertions(+)
>
> diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c
> index 4ce3fad895..d87b9f7dcd 100644
> --- a/lib/efi_selftest/efi_selftest_devicepath.c
> +++ b/lib/efi_selftest/efi_selftest_devicepath.c
> @@ -45,6 +45,55 @@ static u8 *dp1;
> static u8 *dp2;
> static u8 *dp3;
>
> +static struct {
> + struct efi_device_path_sd_mmc_path sd1;
> + struct efi_device_path sep1;
> + struct efi_device_path_sd_mmc_path sd2;
> + struct efi_device_path sep2;
> + struct efi_device_path_sd_mmc_path sd3;
> + struct efi_device_path end;
> +} multi_part_dp = {
> + {
> + {
> + DEVICE_PATH_TYPE_MESSAGING_DEVICE,
> + DEVICE_PATH_SUB_TYPE_MSG_SD,
> + sizeof(struct efi_device_path_sd_mmc_path),
> + },
> + 0,
> + },
> + {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_INSTANCE_END,
> + sizeof(struct efi_device_path),
> + },
> + {
> + {
> + DEVICE_PATH_TYPE_MESSAGING_DEVICE,
> + DEVICE_PATH_SUB_TYPE_MSG_SD,
> + sizeof(struct efi_device_path_sd_mmc_path),
> + },
> + 1,
> + },
> + {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_INSTANCE_END,
> + sizeof(struct efi_device_path),
> + },
> + {
> + {
> + DEVICE_PATH_TYPE_MESSAGING_DEVICE,
> + DEVICE_PATH_SUB_TYPE_MSG_SD,
> + sizeof(struct efi_device_path_sd_mmc_path),
> + },
> + 2,
> + },
> + {
> + DEVICE_PATH_TYPE_END,
> + DEVICE_PATH_SUB_TYPE_END,
> + sizeof(struct efi_device_path),
> + },
> +};
> +
> struct efi_device_path_to_text_protocol *device_path_to_text;
>
> /*
> @@ -340,6 +389,22 @@ static int execute(void)
> return EFI_ST_FAILURE;
> }
>
> + string = device_path_to_text->convert_device_path_to_text(
> + (struct efi_device_path *)&multi_part_dp, true, false);
> + if (efi_st_strcmp_16_8(
> + string,
> + "/SD(0),/SD(1),/SD(2)")
> + ) {
> + efi_st_printf("multi_part_dp: %ps\n", string);
> + efi_st_error("Incorrect text from ConvertDevicePathToText\n");
> + return EFI_ST_FAILURE;
> + }
> + ret = boottime->free_pool(string);
> + if (ret != EFI_SUCCESS) {
> + efi_st_error("FreePool failed\n");
> + return EFI_ST_FAILURE;
> + }
> +
> /* Test ConvertDeviceNodeToText */
> string = device_path_to_text->convert_device_node_to_text(
> (struct efi_device_path *)&dp_node, true, false);
> --
> 2.30.0
>
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] efi_loader: multi part device paths to text
2021-02-19 10:13 ` Ilias Apalodimas
@ 2021-02-19 11:45 ` Heinrich Schuchardt
2021-02-19 12:02 ` Ilias Apalodimas
0 siblings, 1 reply; 7+ messages in thread
From: Heinrich Schuchardt @ 2021-02-19 11:45 UTC (permalink / raw)
To: u-boot
On 19.02.21 11:13, Ilias Apalodimas wrote:
> Hi Heinrich
>
> On Thu, Feb 18, 2021 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
>> Our current implementation of
>> EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi
>> part device paths after the first part. We should convert all parts.
>>
>> Render device path instance ends as commas. This is not explicitly
>> described in the UEFI spec but mimics what EDK II does.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++-----
>> 1 file changed, 12 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
>> index 1aaa9f94fa..81b8ac23ba 100644
>> --- a/lib/efi_loader/efi_device_path_to_text.c
>> +++ b/lib/efi_loader/efi_device_path_to_text.c
>> @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
>>
>> if (!device_path)
>> goto out;
>> - while (device_path &&
>> - str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
>> - *str++ = '/';
>> - str = efi_convert_single_device_node_to_text(str, device_path);
>> - device_path = efi_dp_next(device_path);
>> + while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
>> + if (device_path->type == DEVICE_PATH_TYPE_END) {
>> + if (device_path->sub_type !=
>> + DEVICE_PATH_SUB_TYPE_INSTANCE_END)
>
> maybe it's better to check explicitly for '== DEVICE_PATH_SUB_TYPE_END'?
Thank you for reviewing.
I want to leave the loop when an unexpected value occurs.
Best regards
Heinrich
>
>> + break;
>> + *str++ = ',';
>> + } else {
>> + *str++ = '/';
>> + str = efi_convert_single_device_node_to_text(
>> + str, device_path);
>> + }
>> + *(u8 **)&device_path += device_path->length;
>> }
>>
>> text = efi_str_to_u16(buffer);
>> --
>> 2.30.0
>>
>
> Cheers
> /Ilias
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] efi_loader: multi part device paths to text
2021-02-19 11:45 ` Heinrich Schuchardt
@ 2021-02-19 12:02 ` Ilias Apalodimas
0 siblings, 0 replies; 7+ messages in thread
From: Ilias Apalodimas @ 2021-02-19 12:02 UTC (permalink / raw)
To: u-boot
On Fri, Feb 19, 2021 at 12:45:38PM +0100, Heinrich Schuchardt wrote:
> On 19.02.21 11:13, Ilias Apalodimas wrote:
> > Hi Heinrich
> >
> > On Thu, Feb 18, 2021 at 06:30:43PM +0100, Heinrich Schuchardt wrote:
> >> Our current implementation of
> >> EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() truncates multi
> >> part device paths after the first part. We should convert all parts.
> >>
> >> Render device path instance ends as commas. This is not explicitly
> >> described in the UEFI spec but mimics what EDK II does.
> >>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >> lib/efi_loader/efi_device_path_to_text.c | 17 ++++++++++++-----
> >> 1 file changed, 12 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
> >> index 1aaa9f94fa..81b8ac23ba 100644
> >> --- a/lib/efi_loader/efi_device_path_to_text.c
> >> +++ b/lib/efi_loader/efi_device_path_to_text.c
> >> @@ -369,11 +369,18 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
> >>
> >> if (!device_path)
> >> goto out;
> >> - while (device_path &&
> >> - str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
> >> - *str++ = '/';
> >> - str = efi_convert_single_device_node_to_text(str, device_path);
> >> - device_path = efi_dp_next(device_path);
> >> + while (device_path && str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
> >> + if (device_path->type == DEVICE_PATH_TYPE_END) {
> >> + if (device_path->sub_type !=
> >> + DEVICE_PATH_SUB_TYPE_INSTANCE_END)
> >
> > maybe it's better to check explicitly for '== DEVICE_PATH_SUB_TYPE_END'?
>
> Thank you for reviewing.
>
> I want to leave the loop when an unexpected value occurs.
Fair enough
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
> Best regards
>
> Heinrich
>
> >
> >> + break;
> >> + *str++ = ',';
> >> + } else {
> >> + *str++ = '/';
> >> + str = efi_convert_single_device_node_to_text(
> >> + str, device_path);
> >> + }
> >> + *(u8 **)&device_path += device_path->length;
> >> }
> >>
> >> text = efi_str_to_u16(buffer);
> >> --
> >> 2.30.0
> >>
> >
> > Cheers
> > /Ilias
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-02-19 12:02 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-18 17:30 [PATCH 0/2] efi_loader: multi part device paths to text Heinrich Schuchardt
2021-02-18 17:30 ` [PATCH 1/2] " Heinrich Schuchardt
2021-02-19 10:13 ` Ilias Apalodimas
2021-02-19 11:45 ` Heinrich Schuchardt
2021-02-19 12:02 ` Ilias Apalodimas
2021-02-18 17:30 ` [PATCH 2/2] efi_selftest: multi part device path " Heinrich Schuchardt
2021-02-19 10:15 ` Ilias Apalodimas
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.