diff mbox series

uefirtmisc: add checking the resources for testing

Message ID 20220119083341.33613-1-ivan.hu@canonical.com
State Accepted
Headers show
Series uefirtmisc: add checking the resources for testing | expand

Commit Message

Ivan Hu Jan. 19, 2022, 8:33 a.m. UTC
BugLink: https://bugs.launchpad.net/fwts/+bug/1958206

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
---
 src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Sunny Wang Jan. 19, 2022, 9:23 a.m. UTC | #1
Hi Ivan,

Thanks for working on this.

However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.
For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.

Best Regards,
Sunny

-----Original Message-----
From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
Sent: 19 January 2022 08:34
To: fwts-devel@lists.ubuntu.com
Subject: [PATCH] uefirtmisc: add checking the resources for testing

BugLink: https://bugs.launchpad.net/fwts/+bug/1958206

Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
---
 src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
index f6038f5f..db33ad66 100644
--- a/src/uefi/uefirtmisc/uefirtmisc.c
+++ b/src/uefi/uefirtmisc/uefirtmisc.c
@@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
                                        "service is not supported on this platform.");
                                return FWTS_SKIP;
                        }
+                       if (status == EFI_OUT_OF_RESOURCES) {
+                               fwts_skipped(fw, "Skipping test, run out of resources for "
+                                       "GetNextHighMonotonicCount runtime service test.");
+                               return FWTS_SKIP;
+                       }
                        fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
                                "Failed to get high monotonic count with UEFI runtime service.");
                        fwts_uefi_print_status_info(fw, status);
--
2.25.1


--
fwts-devel mailing list
fwts-devel@lists.ubuntu.com
Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Ivan Hu Jan. 20, 2022, 1:52 a.m. UTC | #2
Hi Sunny,

On 1/19/22 5:23 PM, Sunny Wang wrote:
> Hi Ivan,
> 
> Thanks for working on this.
> 
> However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.

They are supposed to "SKIP" as well when running out of resources. If
not, file another bug for it and attach the result.log.

> For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
> It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.

It is not my intention to SKIP the tests. It is due to run out of
resources, we cannot get the conclusion of the result, PASS or FAIL, so
SKIP instead. Besides, it is not a good idea to test a runtime service
which depends on another runtime service, we cannot make sure all
runtime services work fine, it might get more false alarms. I prefer
keeping it simple.

Cheers,
Ivan

> 
> Best Regards,
> Sunny
> 
> -----Original Message-----
> From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
> Sent: 19 January 2022 08:34
> To: fwts-devel@lists.ubuntu.com
> Subject: [PATCH] uefirtmisc: add checking the resources for testing
> 
> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
> 
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index f6038f5f..db33ad66 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>                                         "service is not supported on this platform.");
>                                 return FWTS_SKIP;
>                         }
> +                       if (status == EFI_OUT_OF_RESOURCES) {
> +                               fwts_skipped(fw, "Skipping test, run out of resources for "
> +                                       "GetNextHighMonotonicCount runtime service test.");
> +                               return FWTS_SKIP;
> +                       }
>                         fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>                                 "Failed to get high monotonic count with UEFI runtime service.");
>                         fwts_uefi_print_status_info(fw, status);
> --
> 2.25.1
> 
> 
> --
> fwts-devel mailing list
> fwts-devel@lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
Sunny Wang Jan. 20, 2022, 10:56 a.m. UTC | #3
Thanks for clarifying, Ivan.

The problem is that we may NOT know the details about the firmware implementation of runtime services. Any runtime service test can be related to SetVariable, so the returned error status may not always be EFI_OUT_OF_RESOURCES.
For the edk2 implementation, this issue (using up UEFI variable storage space) caused different error statuses from the following test cases. For proprietary implementation, that will be more and more possibilities.
   - For uefirtvariable, it gets EFI_BAD_BUFFER_SIZE.
   - For uefirttime, it gets EFI_DEVICE_ERROR because the implementation tries to emulate the time device by using UEFI variable storage.
  -  For uefirtmisc, it gets EFI_OUT_OF_RESOURCES. Actually, the EFI_OUT_OF_RESOURCES is NOT one of the returned statuses in UEFI spec GetNextHighMonotonicCount() section, so the current solution may not make completely sense.

In other words, the current solution doesn't solve the difficult situation that we currently face with. When the problem occurs (after running the stress test), we will still run into the issues below:
   - System will still no longer be able to run any test or run any applications/commands that are related to SetVariable (even no longer be able to install an OS or change setup option as the SetVariable won’t work). In the worst case, it would look like the FWTS test bricks the system. For recovering the system, we have to clear UEFI variable storage, which would require a reboot for using a tool or an option in the setup menu or re-flashing the firmware. For some ARM systems, we even need to use HW flasher/JTAG programmer to recover the system.
   - We would still get the false positive with some tests after uefirtmisc. Again, we don't know whether the firmware implementation is based on the UEFI variable or not and don't know what error status will be returned. This would also cause difficulty in debugging the failures.

Therefore, I would still like to know why we need the stress tests. If we don't have a reason to do this, can we remove them? If the stress test is not for any requirement in UEFI, ACPI, or other specs, can we enable/skip that by using a flag/option? If we have a concern about skipping the stress test, can we reduce the number/count of runs for the stress test?

Best Regards,
Sunny
-----Original Message-----
From: ivanhu <ivan.hu@canonical.com>
Sent: 20 January 2022 01:53
To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing

Hi Sunny,

On 1/19/22 5:23 PM, Sunny Wang wrote:
> Hi Ivan,
>
> Thanks for working on this.
>
> However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.

They are supposed to "SKIP" as well when running out of resources. If
not, file another bug for it and attach the result.log.

> For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
> It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.

It is not my intention to SKIP the tests. It is due to run out of
resources, we cannot get the conclusion of the result, PASS or FAIL, so
SKIP instead. Besides, it is not a good idea to test a runtime service
which depends on another runtime service, we cannot make sure all
runtime services work fine, it might get more false alarms. I prefer
keeping it simple.

Cheers,
Ivan

>
> Best Regards,
> Sunny
>
> -----Original Message-----
> From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
> Sent: 19 January 2022 08:34
> To: fwts-devel@lists.ubuntu.com
> Subject: [PATCH] uefirtmisc: add checking the resources for testing
>
> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
>
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>  src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index f6038f5f..db33ad66 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>                                         "service is not supported on this platform.");
>                                 return FWTS_SKIP;
>                         }
> +                       if (status == EFI_OUT_OF_RESOURCES) {
> +                               fwts_skipped(fw, "Skipping test, run out of resources for "
> +                                       "GetNextHighMonotonicCount runtime service test.");
> +                               return FWTS_SKIP;
> +                       }
>                         fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>                                 "Failed to get high monotonic count with UEFI runtime service.");
>                         fwts_uefi_print_status_info(fw, status);
> --
> 2.25.1
>
>
> --
> fwts-devel mailing list
> fwts-devel@lists.ubuntu.com
> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Ivan Hu Jan. 21, 2022, 1:39 a.m. UTC | #4
Hi Sunny,

On 1/20/22 6:56 PM, Sunny Wang wrote:
> Thanks for clarifying, Ivan.
> 
> The problem is that we may NOT know the details about the firmware implementation of runtime services. Any runtime service test can be related to SetVariable, so the returned error status may not always be EFI_OUT_OF_RESOURCES.
> For the edk2 implementation, this issue (using up UEFI variable storage space) caused different error statuses from the following test cases. For proprietary implementation, that will be more and more possibilities.
>    - For uefirtvariable, it gets EFI_BAD_BUFFER_SIZE.
This failure could also be checked.

>    - For uefirttime, it gets EFI_DEVICE_ERROR because the implementation tries to emulate the time device by using UEFI variable storage.
No idea why time service need to use variable storage.
>   -  For uefirtmisc, it gets EFI_OUT_OF_RESOURCES. Actually, the EFI_OUT_OF_RESOURCES is NOT one of the returned statuses in UEFI spec GetNextHighMonotonicCount() section, so the current solution may not make completely sense.
What else status is not actually error?
> 
> In other words, the current solution doesn't solve the difficult situation that we currently face with. When the problem occurs (after running the stress test), we will still run into the issues below:
>    - System will still no longer be able to run any test or run any applications/commands that are related to SetVariable (even no longer be able to install an OS or change setup option as the SetVariable won’t work). In the worst case, it would look like the FWTS test bricks the system. For recovering the system, we have to clear UEFI variable storage, which would require a reboot for using a tool or an option in the setup menu or re-flashing the firmware. For some ARM systems, we even need to use HW flasher/JTAG programmer to recover the system.
SetVariable with the same name and GUID, only data is different, it
won't actually reduce the storage, reboot should solve your problem.
>    - We would still get the false positive with some tests after uefirtmisc. Again, we don't know whether the firmware implementation is based on the UEFI variable or not and don't know what error status will be returned. This would also cause difficulty in debugging the failures.
FWTS tests can be run separately, it might not be a problem for debugging.

> 
> Therefore, I would still like to know why we need the stress tests. If we don't have a reason to do this, can we remove them? If the stress test is not for any requirement in UEFI, ACPI, or other specs, can we enable/skip that by using a flag/option? If we have a concern about skipping the stress test, can we reduce the number/count of runs for the stress test?

FWTS is a only tool for checking firmware implementation, the stress
test concept is from UEFI SCT, you can run whatever tests you want.
I'm happy to sent another patch for reduce the number of runs the stress
for those pure storage platform.
How many NV basically for general ARM platforms? and what number of runs
for the stress tests suits the platforms?


Cheers,
Ivan

> 
> Best Regards,
> Sunny
> -----Original Message-----
> From: ivanhu <ivan.hu@canonical.com>
> Sent: 20 January 2022 01:53
> To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
> Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing
> 
> Hi Sunny,
> 
> On 1/19/22 5:23 PM, Sunny Wang wrote:
>> Hi Ivan,
>>
>> Thanks for working on this.
>>
>> However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.
> 
> They are supposed to "SKIP" as well when running out of resources. If
> not, file another bug for it and attach the result.log.
> 
>> For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
>> It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.
> 
> It is not my intention to SKIP the tests. It is due to run out of
> resources, we cannot get the conclusion of the result, PASS or FAIL, so
> SKIP instead. Besides, it is not a good idea to test a runtime service
> which depends on another runtime service, we cannot make sure all
> runtime services work fine, it might get more false alarms. I prefer
> keeping it simple.
> 
> Cheers,
> Ivan
> 
>>
>> Best Regards,
>> Sunny
>>
>> -----Original Message-----
>> From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
>> Sent: 19 January 2022 08:34
>> To: fwts-devel@lists.ubuntu.com
>> Subject: [PATCH] uefirtmisc: add checking the resources for testing
>>
>> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
>>
>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
>> ---
>>  src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
>> index f6038f5f..db33ad66 100644
>> --- a/src/uefi/uefirtmisc/uefirtmisc.c
>> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
>> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>>                                         "service is not supported on this platform.");
>>                                 return FWTS_SKIP;
>>                         }
>> +                       if (status == EFI_OUT_OF_RESOURCES) {
>> +                               fwts_skipped(fw, "Skipping test, run out of resources for "
>> +                                       "GetNextHighMonotonicCount runtime service test.");
>> +                               return FWTS_SKIP;
>> +                       }
>>                         fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>>                                 "Failed to get high monotonic count with UEFI runtime service.");
>>                         fwts_uefi_print_status_info(fw, status);
>> --
>> 2.25.1
>>
>>
>> --
>> fwts-devel mailing list
>> fwts-devel@lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
Sunny Wang Jan. 21, 2022, 10:36 a.m. UTC | #5
Hi Ivan,

Thanks for further checking this.

If the test concept is from SCT, it seems better to add a flag/a new option for controlling the stress test so that FWTS can be consistent with SCT. For SCT, it doesn't run the stress test by default. If user wants to run the stress test, he/she has to re-build the SCT with EFI_TEST_EXHAUSTIVE.
Actually, using a flag/new option and disabling stress test by default makes more sense to me because I couldn't find a requirement in UEFI and ACPI spec. The main purpose of using SCT and FWTS is to check the requirements and interfaces defined in the specs, so it is better to run stress tests on demand than always.

If you have a concern about using the flag/new option, we can change the number of runs as the following:
       - For GetNextHighMonotonicCount and SetTime, SCT don't have stress test for them, so I think we can remove them. Or we can set the number to 1. Then, for anyone who need stress test, they can still change the number and rebuild the FWTS.
       - For SetVariable, SCT only calls SetVariable with same data for 10 times and different data for another 10 times, so we can set similar number for FWTS.

For other questions, I put my comments inline with [Sunny].

Best Regards,
Sunny

-----Original Message-----
From: ivanhu <ivan.hu@canonical.com>
Sent: 21 January 2022 01:39
To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing

Hi Sunny,

On 1/20/22 6:56 PM, Sunny Wang wrote:
> Thanks for clarifying, Ivan.
>
> The problem is that we may NOT know the details about the firmware implementation of runtime services. Any runtime service test can be related to SetVariable, so the returned error status may not always be EFI_OUT_OF_RESOURCES.
> For the edk2 implementation, this issue (using up UEFI variable storage space) caused different error statuses from the following test cases. For proprietary implementation, that will be more and more possibilities.
>    - For uefirtvariable, it gets EFI_BAD_BUFFER_SIZE.
This failure could also be checked.

>    - For uefirttime, it gets EFI_DEVICE_ERROR because the implementation tries to emulate the time device by using UEFI variable storage.
No idea why time service need to use variable storage.
>   -  For uefirtmisc, it gets EFI_OUT_OF_RESOURCES. Actually, the EFI_OUT_OF_RESOURCES is NOT one of the returned statuses in UEFI spec GetNextHighMonotonicCount() section, so the current solution may not make completely sense.
What else status is not actually error?
[Sunny] Unfortunately, UEFI spec doesn't cover this case, so no defined status code can perfectly be used for this case.

>
> In other words, the current solution doesn't solve the difficult situation that we currently face with. When the problem occurs (after running the stress test), we will still run into the issues below:
>    - System will still no longer be able to run any test or run any applications/commands that are related to SetVariable (even no longer be able to install an OS or change setup option as the SetVariable won’t work). In the worst case, it would look like the FWTS test bricks the system. For recovering the system, we have to clear UEFI variable storage, which would require a reboot for using a tool or an option in the setup menu or re-flashing the firmware. For some ARM systems, we even need to use HW flasher/JTAG programmer to recover the system.
SetVariable with the same name and GUID, only data is different, it
won't actually reduce the storage, reboot should solve your problem.
[Sunny] Actually, it did. I added code to call QueryVariableInfo() and print out the RemainingVariableStorageSize in FWTS. The RemainingVariableStorageSize keeps reduced after calling GetNextHighMonotonicCount() and SetVariable().  I just uploaded the logs to https://bugs.launchpad.net/fwts/+bug/1958206 for your reference. Reboot won't solve the problem. We already ran into this problem on several Arm systems including RPi4. Most of them require re-flashing the firmware.


>    - We would still get the false positive with some tests after uefirtmisc. Again, we don't know whether the firmware implementation is based on the UEFI variable or not and don't know what error status will be returned. This would also cause difficulty in debugging the failures.
FWTS tests can be run separately, it might not be a problem for debugging.
[Sunny] It causes the additional effort. Besides separately running the tests to narrow down the real failed test case, we also have to check if the firmware implementation is related to UEFI variable. The connection between test cases and UEFI variable are not directly visible in FWTS error message. The message even causes misleading if the person who debug this issue has no idea about connection between test cases and variable storage. Another problem is that some people may run into problem with OS installation and changing setup option, and may have no idea why this suddenly happened. The root cause is also the stress test. When the FWTS test almost uses up the UEFI variable storage, users may easily run into this issue after doing some other things related to UEFI variables.


>
> Therefore, I would still like to know why we need the stress tests. If we don't have a reason to do this, can we remove them? If the stress test is not for any requirement in UEFI, ACPI, or other specs, can we enable/skip that by using a flag/option? If we have a concern about skipping the stress test, can we reduce the number/count of runs for the stress test?

FWTS is a only tool for checking firmware implementation, the stress
test concept is from UEFI SCT, you can run whatever tests you want.
I'm happy to sent another patch for reduce the number of runs the stress
for those pure storage platform.
How many NV basically for general ARM platforms? and what number of runs
for the stress tests suits the platforms?

Cheers,
Ivan

>
> Best Regards,
> Sunny
> -----Original Message-----
> From: ivanhu <ivan.hu@canonical.com>
> Sent: 20 January 2022 01:53
> To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
> Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing
>
> Hi Sunny,
>
> On 1/19/22 5:23 PM, Sunny Wang wrote:
>> Hi Ivan,
>>
>> Thanks for working on this.
>>
>> However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.
>
> They are supposed to "SKIP" as well when running out of resources. If
> not, file another bug for it and attach the result.log.
>
>> For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
>> It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.
>
> It is not my intention to SKIP the tests. It is due to run out of
> resources, we cannot get the conclusion of the result, PASS or FAIL, so
> SKIP instead. Besides, it is not a good idea to test a runtime service
> which depends on another runtime service, we cannot make sure all
> runtime services work fine, it might get more false alarms. I prefer
> keeping it simple.
>
> Cheers,
> Ivan
>
>>
>> Best Regards,
>> Sunny
>>
>> -----Original Message-----
>> From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
>> Sent: 19 January 2022 08:34
>> To: fwts-devel@lists.ubuntu.com
>> Subject: [PATCH] uefirtmisc: add checking the resources for testing
>>
>> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
>>
>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
>> ---
>>  src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
>> index f6038f5f..db33ad66 100644
>> --- a/src/uefi/uefirtmisc/uefirtmisc.c
>> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
>> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>>                                         "service is not supported on this platform.");
>>                                 return FWTS_SKIP;
>>                         }
>> +                       if (status == EFI_OUT_OF_RESOURCES) {
>> +                               fwts_skipped(fw, "Skipping test, run out of resources for "
>> +                                       "GetNextHighMonotonicCount runtime service test.");
>> +                               return FWTS_SKIP;
>> +                       }
>>                         fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>>                                 "Failed to get high monotonic count with UEFI runtime service.");
>>                         fwts_uefi_print_status_info(fw, status);
>> --
>> 2.25.1
>>
>>
>> --
>> fwts-devel mailing list
>> fwts-devel@lists.ubuntu.com
>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Ivan Hu Jan. 24, 2022, 9:13 a.m. UTC | #6
On 1/21/22 18:36, Sunny Wang wrote:
> Hi Ivan,
>
> Thanks for further checking this.
>
> If the test concept is from SCT, it seems better to add a flag/a new option for controlling the stress test so that FWTS can be consistent with SCT. For SCT, it doesn't run the stress test by default. If user wants to run the stress test, he/she has to re-build the SCT with EFI_TEST_EXHAUSTIVE.
> Actually, using a flag/new option and disabling stress test by default makes more sense to me because I couldn't find a requirement in UEFI and ACPI spec. The main purpose of using SCT and FWTS is to check the requirements and interfaces defined in the specs, so it is better to run stress tests on demand than always.

Again, the main goal of FWTS is to help to find out the potential
firmware implement issue, I prefer doing the tests as manyas we can. It
is not a certify tool, so even without requirements of the
specification, we hope it can be tested.

And results(Pass or Fail) are only to help you to review your firmware
implementation, if you think your firmware implementation is fine, you
can just ignore the fail results.

> If you have a concern about using the flag/new option, we can change the number of runs as the following:
>        - For GetNextHighMonotonicCount and SetTime, SCT don't have stress test for them, so I think we can remove them. Or we can set the number to 1. Then, for anyone who need stress test, they can still change the number and rebuild the FWTS.
>        - For SetVariable, SCT only calls SetVariable with same data for 10 times and different data for another 10 times, so we can set similar number for FWTS.

"Set the number to 1" doesn't make scenes, there is no meaning to test
only one time for stress test.

I've check the SCT, they set the test times to 50,

#define MULTIPLE_TEST_TIMES         50

So, I might follow it and set all stress test number to 50.

Let's see if it work with your platforms

>
> For other questions, I put my comments inline with [Sunny].
>
> Best Regards,
> Sunny
>
> -----Original Message-----
> From: ivanhu <ivan.hu@canonical.com>
> Sent: 21 January 2022 01:39
> To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
> Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing
>
> Hi Sunny,
>
> On 1/20/22 6:56 PM, Sunny Wang wrote:
>> Thanks for clarifying, Ivan.
>>
>> The problem is that we may NOT know the details about the firmware implementation of runtime services. Any runtime service test can be related to SetVariable, so the returned error status may not always be EFI_OUT_OF_RESOURCES.
>> For the edk2 implementation, this issue (using up UEFI variable storage space) caused different error statuses from the following test cases. For proprietary implementation, that will be more and more possibilities.
>>    - For uefirtvariable, it gets EFI_BAD_BUFFER_SIZE.
> This failure could also be checked.
>
>>    - For uefirttime, it gets EFI_DEVICE_ERROR because the implementation tries to emulate the time device by using UEFI variable storage.
> No idea why time service need to use variable storage.
>>   -  For uefirtmisc, it gets EFI_OUT_OF_RESOURCES. Actually, the EFI_OUT_OF_RESOURCES is NOT one of the returned statuses in UEFI spec GetNextHighMonotonicCount() section, so the current solution may not make completely sense.
> What else status is not actually error?
> [Sunny] Unfortunately, UEFI spec doesn't cover this case, so no defined status code can perfectly be used for this case.

Maybe you should send an ECR to UEFI forum for clarify this.

>
>> In other words, the current solution doesn't solve the difficult situation that we currently face with. When the problem occurs (after running the stress test), we will still run into the issues below:
>>    - System will still no longer be able to run any test or run any applications/commands that are related to SetVariable (even no longer be able to install an OS or change setup option as the SetVariable won’t work). In the worst case, it would look like the FWTS test bricks the system. For recovering the system, we have to clear UEFI variable storage, which would require a reboot for using a tool or an option in the setup menu or re-flashing the firmware. For some ARM systems, we even need to use HW flasher/JTAG programmer to recover the system.
> SetVariable with the same name and GUID, only data is different, it
> won't actually reduce the storage, reboot should solve your problem.
> [Sunny] Actually, it did. I added code to call QueryVariableInfo() and print out the RemainingVariableStorageSize in FWTS. The RemainingVariableStorageSize keeps reduced after calling GetNextHighMonotonicCount() and SetVariable().  I just uploaded the logs to https://bugs.launchpad.net/fwts/+bug/1958206 for your reference. Reboot won't solve the problem. We already ran into this problem on several Arm systems including RPi4. Most of them require re-flashing the firmware.

It seems a critical issue on these platforms firmware implementation for
cleaning the unused storage, it sounds like users may get their
platforms bricked after they do a lot of setvariable(even only change
variable data).

I've tested with two platforms, include Intel EDK2 UEFI develop
kit(RainbowPass).

Running test uefirtmisc several times till the insufficient remaining
storage.

Fortunately, after reboot, they recover the remaining storage.

Please refer to the result log,
https://bugs.launchpad.net/fwts/+bug/1958206/comments/5


Cheers,

Ivan

>
>
>>    - We would still get the false positive with some tests after uefirtmisc. Again, we don't know whether the firmware implementation is based on the UEFI variable or not and don't know what error status will be returned. This would also cause difficulty in debugging the failures.
> FWTS tests can be run separately, it might not be a problem for debugging.
> [Sunny] It causes the additional effort. Besides separately running the tests to narrow down the real failed test case, we also have to check if the firmware implementation is related to UEFI variable. The connection between test cases and UEFI variable are not directly visible in FWTS error message. The message even causes misleading if the person who debug this issue has no idea about connection between test cases and variable storage. Another problem is that some people may run into problem with OS installation and changing setup option, and may have no idea why this suddenly happened. The root cause is also the stress test. When the FWTS test almost uses up the UEFI variable storage, users may easily run into this issue after doing some other things related to UEFI variables.
>
>
>> Therefore, I would still like to know why we need the stress tests. If we don't have a reason to do this, can we remove them? If the stress test is not for any requirement in UEFI, ACPI, or other specs, can we enable/skip that by using a flag/option? If we have a concern about skipping the stress test, can we reduce the number/count of runs for the stress test?
> FWTS is a only tool for checking firmware implementation, the stress
> test concept is from UEFI SCT, you can run whatever tests you want.
> I'm happy to sent another patch for reduce the number of runs the stress
> for those pure storage platform.
> How many NV basically for general ARM platforms? and what number of runs
> for the stress tests suits the platforms?
>
> Cheers,
> Ivan
>
>> Best Regards,
>> Sunny
>> -----Original Message-----
>> From: ivanhu <ivan.hu@canonical.com>
>> Sent: 20 January 2022 01:53
>> To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
>> Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing
>>
>> Hi Sunny,
>>
>> On 1/19/22 5:23 PM, Sunny Wang wrote:
>>> Hi Ivan,
>>>
>>> Thanks for working on this.
>>>
>>> However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.
>> They are supposed to "SKIP" as well when running out of resources. If
>> not, file another bug for it and attach the result.log.
>>
>>> For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
>>> It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.
>> It is not my intention to SKIP the tests. It is due to run out of
>> resources, we cannot get the conclusion of the result, PASS or FAIL, so
>> SKIP instead. Besides, it is not a good idea to test a runtime service
>> which depends on another runtime service, we cannot make sure all
>> runtime services work fine, it might get more false alarms. I prefer
>> keeping it simple.
>>
>> Cheers,
>> Ivan
>>
>>> Best Regards,
>>> Sunny
>>>
>>> -----Original Message-----
>>> From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
>>> Sent: 19 January 2022 08:34
>>> To: fwts-devel@lists.ubuntu.com
>>> Subject: [PATCH] uefirtmisc: add checking the resources for testing
>>>
>>> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
>>>
>>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
>>> ---
>>>  src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
>>> index f6038f5f..db33ad66 100644
>>> --- a/src/uefi/uefirtmisc/uefirtmisc.c
>>> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
>>> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>>>                                         "service is not supported on this platform.");
>>>                                 return FWTS_SKIP;
>>>                         }
>>> +                       if (status == EFI_OUT_OF_RESOURCES) {
>>> +                               fwts_skipped(fw, "Skipping test, run out of resources for "
>>> +                                       "GetNextHighMonotonicCount runtime service test.");
>>> +                               return FWTS_SKIP;
>>> +                       }
>>>                         fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>>>                                 "Failed to get high monotonic count with UEFI runtime service.");
>>>                         fwts_uefi_print_status_info(fw, status);
>>> --
>>> 2.25.1
>>>
>>>
>>> --
>>> fwts-devel mailing list
>>> fwts-devel@lists.ubuntu.com
>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
>>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>>>
>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Sunny Wang Jan. 27, 2022, 3:15 p.m. UTC | #7
Hi Ivan,

Sorry for the late response. I was stuck in other things in the past few days.
Thanks for further clarifying. Yeah, having stress tests make sense now. I missed the point that FTWS is designed not only for catching the spec related issue but also for catching the non-spec-related issues, so some of my previous proposals (removing and setting the number to 1) don't make sense.

Checking your new patch now...

Best Regards,
Sunny
-----Original Message-----
From: ivanhu <ivan.hu@canonical.com>
Sent: 24 January 2022 09:14
To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing


On 1/21/22 18:36, Sunny Wang wrote:
> Hi Ivan,
>
> Thanks for further checking this.
>
> If the test concept is from SCT, it seems better to add a flag/a new option for controlling the stress test so that FWTS can be consistent with SCT. For SCT, it doesn't run the stress test by default. If user wants to run the stress test, he/she has to re-build the SCT with EFI_TEST_EXHAUSTIVE.
> Actually, using a flag/new option and disabling stress test by default makes more sense to me because I couldn't find a requirement in UEFI and ACPI spec. The main purpose of using SCT and FWTS is to check the requirements and interfaces defined in the specs, so it is better to run stress tests on demand than always.

Again, the main goal of FWTS is to help to find out the potential
firmware implement issue, I prefer doing the tests as manyas we can. It
is not a certify tool, so even without requirements of the
specification, we hope it can be tested.

And results(Pass or Fail) are only to help you to review your firmware
implementation, if you think your firmware implementation is fine, you
can just ignore the fail results.

> If you have a concern about using the flag/new option, we can change the number of runs as the following:
>        - For GetNextHighMonotonicCount and SetTime, SCT don't have stress test for them, so I think we can remove them. Or we can set the number to 1. Then, for anyone who need stress test, they can still change the number and rebuild the FWTS.
>        - For SetVariable, SCT only calls SetVariable with same data for 10 times and different data for another 10 times, so we can set similar number for FWTS.

"Set the number to 1" doesn't make scenes, there is no meaning to test
only one time for stress test.

I've check the SCT, they set the test times to 50,

#define MULTIPLE_TEST_TIMES         50

So, I might follow it and set all stress test number to 50.

Let's see if it work with your platforms

>
> For other questions, I put my comments inline with [Sunny].
>
> Best Regards,
> Sunny
>
> -----Original Message-----
> From: ivanhu <ivan.hu@canonical.com>
> Sent: 21 January 2022 01:39
> To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
> Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing
>
> Hi Sunny,
>
> On 1/20/22 6:56 PM, Sunny Wang wrote:
>> Thanks for clarifying, Ivan.
>>
>> The problem is that we may NOT know the details about the firmware implementation of runtime services. Any runtime service test can be related to SetVariable, so the returned error status may not always be EFI_OUT_OF_RESOURCES.
>> For the edk2 implementation, this issue (using up UEFI variable storage space) caused different error statuses from the following test cases. For proprietary implementation, that will be more and more possibilities.
>>    - For uefirtvariable, it gets EFI_BAD_BUFFER_SIZE.
> This failure could also be checked.
>
>>    - For uefirttime, it gets EFI_DEVICE_ERROR because the implementation tries to emulate the time device by using UEFI variable storage.
> No idea why time service need to use variable storage.
>>   -  For uefirtmisc, it gets EFI_OUT_OF_RESOURCES. Actually, the EFI_OUT_OF_RESOURCES is NOT one of the returned statuses in UEFI spec GetNextHighMonotonicCount() section, so the current solution may not make completely sense.
> What else status is not actually error?
> [Sunny] Unfortunately, UEFI spec doesn't cover this case, so no defined status code can perfectly be used for this case.

Maybe you should send an ECR to UEFI forum for clarify this.

>
>> In other words, the current solution doesn't solve the difficult situation that we currently face with. When the problem occurs (after running the stress test), we will still run into the issues below:
>>    - System will still no longer be able to run any test or run any applications/commands that are related to SetVariable (even no longer be able to install an OS or change setup option as the SetVariable won’t work). In the worst case, it would look like the FWTS test bricks the system. For recovering the system, we have to clear UEFI variable storage, which would require a reboot for using a tool or an option in the setup menu or re-flashing the firmware. For some ARM systems, we even need to use HW flasher/JTAG programmer to recover the system.
> SetVariable with the same name and GUID, only data is different, it
> won't actually reduce the storage, reboot should solve your problem.
> [Sunny] Actually, it did. I added code to call QueryVariableInfo() and print out the RemainingVariableStorageSize in FWTS. The RemainingVariableStorageSize keeps reduced after calling GetNextHighMonotonicCount() and SetVariable().  I just uploaded the logs to https://bugs.launchpad.net/fwts/+bug/1958206 for your reference. Reboot won't solve the problem. We already ran into this problem on several Arm systems including RPi4. Most of them require re-flashing the firmware.

It seems a critical issue on these platforms firmware implementation for
cleaning the unused storage, it sounds like users may get their
platforms bricked after they do a lot of setvariable(even only change
variable data).

I've tested with two platforms, include Intel EDK2 UEFI develop
kit(RainbowPass).

Running test uefirtmisc several times till the insufficient remaining
storage.

Fortunately, after reboot, they recover the remaining storage.

Please refer to the result log,
https://bugs.launchpad.net/fwts/+bug/1958206/comments/5


Cheers,

Ivan

>
>
>>    - We would still get the false positive with some tests after uefirtmisc. Again, we don't know whether the firmware implementation is based on the UEFI variable or not and don't know what error status will be returned. This would also cause difficulty in debugging the failures.
> FWTS tests can be run separately, it might not be a problem for debugging.
> [Sunny] It causes the additional effort. Besides separately running the tests to narrow down the real failed test case, we also have to check if the firmware implementation is related to UEFI variable. The connection between test cases and UEFI variable are not directly visible in FWTS error message. The message even causes misleading if the person who debug this issue has no idea about connection between test cases and variable storage. Another problem is that some people may run into problem with OS installation and changing setup option, and may have no idea why this suddenly happened. The root cause is also the stress test. When the FWTS test almost uses up the UEFI variable storage, users may easily run into this issue after doing some other things related to UEFI variables.
>
>
>> Therefore, I would still like to know why we need the stress tests. If we don't have a reason to do this, can we remove them? If the stress test is not for any requirement in UEFI, ACPI, or other specs, can we enable/skip that by using a flag/option? If we have a concern about skipping the stress test, can we reduce the number/count of runs for the stress test?
> FWTS is a only tool for checking firmware implementation, the stress
> test concept is from UEFI SCT, you can run whatever tests you want.
> I'm happy to sent another patch for reduce the number of runs the stress
> for those pure storage platform.
> How many NV basically for general ARM platforms? and what number of runs
> for the stress tests suits the platforms?
>
> Cheers,
> Ivan
>
>> Best Regards,
>> Sunny
>> -----Original Message-----
>> From: ivanhu <ivan.hu@canonical.com>
>> Sent: 20 January 2022 01:53
>> To: Sunny Wang <Sunny.Wang@arm.com>; fwts-devel@lists.ubuntu.com
>> Subject: Re: [PATCH] uefirtmisc: add checking the resources for testing
>>
>> Hi Sunny,
>>
>> On 1/19/22 5:23 PM, Sunny Wang wrote:
>>> Hi Ivan,
>>>
>>> Thanks for working on this.
>>>
>>> However, if my understanding is correct, this solution won't work. The UEFI variable storage space is still used up by uefirtmisc test. Then, any SetVariable related test following uefirtmisc would still fail due to no remaining space.
>> They are supposed to "SKIP" as well when running out of resources. If
>> not, file another bug for it and attach the result.log.
>>
>>> For example, when we run " fwts -r stdout -q --sbbr", uefirtvariable uefirttime will be run after uefirtmisc. Then, both tests would still fail.
>>> It looks like you would like to skip the stress test for the system that doesn't have large UEFI variable storage space. If so, I think you can call QueryVariableInfo() and check the RemainingVariableStorageSize. It will be good to skip all the stress test if the RemainingVariableStorageSize is smaller than 64k.
>> It is not my intention to SKIP the tests. It is due to run out of
>> resources, we cannot get the conclusion of the result, PASS or FAIL, so
>> SKIP instead. Besides, it is not a good idea to test a runtime service
>> which depends on another runtime service, we cannot make sure all
>> runtime services work fine, it might get more false alarms. I prefer
>> keeping it simple.
>>
>> Cheers,
>> Ivan
>>
>>> Best Regards,
>>> Sunny
>>>
>>> -----Original Message-----
>>> From: fwts-devel <fwts-devel-bounces@lists.ubuntu.com> On Behalf Of Ivan Hu
>>> Sent: 19 January 2022 08:34
>>> To: fwts-devel@lists.ubuntu.com
>>> Subject: [PATCH] uefirtmisc: add checking the resources for testing
>>>
>>> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
>>>
>>> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
>>> ---
>>>  src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
>>> index f6038f5f..db33ad66 100644
>>> --- a/src/uefi/uefirtmisc/uefirtmisc.c
>>> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
>>> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>>>                                         "service is not supported on this platform.");
>>>                                 return FWTS_SKIP;
>>>                         }
>>> +                       if (status == EFI_OUT_OF_RESOURCES) {
>>> +                               fwts_skipped(fw, "Skipping test, run out of resources for "
>>> +                                       "GetNextHighMonotonicCount runtime service test.");
>>> +                               return FWTS_SKIP;
>>> +                       }
>>>                         fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>>>                                 "Failed to get high monotonic count with UEFI runtime service.");
>>>                         fwts_uefi_print_status_info(fw, status);
>>> --
>>> 2.25.1
>>>
>>>
>>> --
>>> fwts-devel mailing list
>>> fwts-devel@lists.ubuntu.com
>>> Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/fwts-devel
>>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>>>
>> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
>>
> IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Alex Hung Jan. 29, 2022, 2:31 a.m. UTC | #8
On 2022-01-19 01:33, Ivan Hu wrote:
> BugLink: https://bugs.launchpad.net/fwts/+bug/1958206
> 
> Signed-off-by: Ivan Hu <ivan.hu@canonical.com>
> ---
>   src/uefi/uefirtmisc/uefirtmisc.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index f6038f5f..db33ad66 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -91,6 +91,11 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
>   					"service is not supported on this platform.");
>   				return FWTS_SKIP;
>   			}
> +			if (status == EFI_OUT_OF_RESOURCES) {
> +				fwts_skipped(fw, "Skipping test, run out of resources for "
> +					"GetNextHighMonotonicCount runtime service test.");
> +				return FWTS_SKIP;
> +			}
>   			fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
>   				"Failed to get high monotonic count with UEFI runtime service.");
>   			fwts_uefi_print_status_info(fw, status);


Acked-by: Alex Hung <alex.hung@canonical.com>
diff mbox series

Patch

diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
index f6038f5f..db33ad66 100644
--- a/src/uefi/uefirtmisc/uefirtmisc.c
+++ b/src/uefi/uefirtmisc/uefirtmisc.c
@@ -91,6 +91,11 @@  static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
 					"service is not supported on this platform.");
 				return FWTS_SKIP;
 			}
+			if (status == EFI_OUT_OF_RESOURCES) {
+				fwts_skipped(fw, "Skipping test, run out of resources for "
+					"GetNextHighMonotonicCount runtime service test.");
+				return FWTS_SKIP;
+			}
 			fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeGetNextHighMonotonicCount",
 				"Failed to get high monotonic count with UEFI runtime service.");
 			fwts_uefi_print_status_info(fw, status);