* [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 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 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
* [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 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
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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox