Re: Regression from efi: call get_event_log before ExitBootServices
On Mon, Mar 12, 2018 at 11:41:25AM +0100, Paul Menzel wrote: > Dear Jarkko, > > > On 03/12/18 11:17, Jarkko Sakkinen wrote: > > On Sat, 2018-03-10 at 10:45 +, Thiebaud Weksteen wrote: > > > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Clinewrote: > > > > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" > > > > > > Thanks. Well, it looks like the memory that is supposedly allocated is not > > > usable. I'm thinking this is a firmware bug. > > > Ard, would you agree on this assumption? Thoughts on how to proceed? > > > > Check if the BIOS is up to date? > > How would that help? The no regression policy demands, that Linux continues > working on systems, where it worked before. So upgrading the firmware is no > solution, is it? Until a solution is found, the commits should be reverted. Nope. You are right. /Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 13 March 2018 at 13:41, Jeremy Clinewrote: > On 03/13/2018 03:59 AM, Ard Biesheuvel wrote: >> On 13 March 2018 at 07:47, Hans de Goede wrote: >>> Hi, >>> >>> >>> On 12-03-18 20:55, Thiebaud Weksteen wrote: >> ... Hans, you said you configured the tablet to use the 32-bit version of grub instead of 64. Why's that? >>> >>> >>> Because this tablet, like (almost?) all Bay Trail hardware has a 32 bit >>> UEFI, >>> even though the processor is 64 bit capable (AFAIK 64 bit Windows drivers >>> were >>> not ready in time so all Bay Trail devices shipped with a 32 bit Windows). >>> >>> So this is running a 32 bit grub which boots a 64 bit kernel. >>> Jeremy, could you confirm if you are building the kernel in 64bit mode? Is your Android install working? (that is, what happens if you boot Boot)? >>> >>> >>> AFAIK the kernel on Jeremy's tablet (which I initially installed) is 64 bit. >>> >>> Could the problem perhaps be that the new code for the TPM event-log is >>> missing some handling to deal with running on a 32 bit firmware? I know the >>> rest of the kernel has special code to deal with this. >>> >> >> That is a very good point, and I missed that this is a 64-bit kernel >> running on 32-bit UEFI. >> >> The TPM code does use efi_call_proto() directly, and now I am thinking >> it is perhaps the allocate_pages() call that simply only initializes >> the low 32-bits of log_tbl. >> >> Jeremy, could you please try initializing tcg2_protocol and log_tbl to >> NULL at the start of the function? >> > > That was it, it boots when those are initialized to NULL. > Thanks for confirming. I'll send out a patch. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/13/2018 03:59 AM, Ard Biesheuvel wrote: > On 13 March 2018 at 07:47, Hans de Goedewrote: >> Hi, >> >> >> On 12-03-18 20:55, Thiebaud Weksteen wrote: >>> > ... >>> >>> Hans, you said you configured the tablet to use the 32-bit version of grub >>> instead >>> of 64. Why's that? >> >> >> Because this tablet, like (almost?) all Bay Trail hardware has a 32 bit >> UEFI, >> even though the processor is 64 bit capable (AFAIK 64 bit Windows drivers >> were >> not ready in time so all Bay Trail devices shipped with a 32 bit Windows). >> >> So this is running a 32 bit grub which boots a 64 bit kernel. >> >>> Jeremy, could you confirm if you are building the kernel in 64bit mode? Is >>> your Android install working? (that is, what happens if you boot >>> Boot)? >> >> >> AFAIK the kernel on Jeremy's tablet (which I initially installed) is 64 bit. >> >> Could the problem perhaps be that the new code for the TPM event-log is >> missing some handling to deal with running on a 32 bit firmware? I know the >> rest of the kernel has special code to deal with this. >> > > That is a very good point, and I missed that this is a 64-bit kernel > running on 32-bit UEFI. > > The TPM code does use efi_call_proto() directly, and now I am thinking > it is perhaps the allocate_pages() call that simply only initializes > the low 32-bits of log_tbl. > > Jeremy, could you please try initializing tcg2_protocol and log_tbl to > NULL at the start of the function? > That was it, it boots when those are initialized to NULL. Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Tue, Mar 13, 2018 at 9:47 AM, Hans de Goedewrote: > On 12-03-18 20:55, Thiebaud Weksteen wrote: >> Hans, you said you configured the tablet to use the 32-bit version of grub >> instead >> of 64. Why's that? > Because this tablet, like (almost?) all Bay Trail hardware has a 32 bit > UEFI, > even though the processor is 64 bit capable (AFAIK 64 bit Windows drivers > were > not ready in time so all Bay Trail devices shipped with a 32 bit Windows). Almost. Lenovo Thinkpad 10 tablet has 64-bit firmware. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 13 March 2018 at 10:23, Thiebaud Weksteenwrote: > On Tue, Mar 13, 2018 at 8:59 AM Ard Biesheuvel > wrote: > >> On 13 March 2018 at 07:47, Hans de Goede wrote: ... >> > Could the problem perhaps be that the new code for the TPM event-log is >> > missing some handling to deal with running on a 32 bit firmware? I know > the >> > rest of the kernel has special code to deal with this. >> > > > Yes, that was my guess as well. > >> That is a very good point, and I missed that this is a 64-bit kernel >> running on 32-bit UEFI. > >> The TPM code does use efi_call_proto() directly, and now I am thinking >> it is perhaps the allocate_pages() call that simply only initializes >> the low 32-bits of log_tbl. > > That make sense. Would you know what happens to the arguments of the > function in this case? (I'm thinking _location ?) log_location and log_last_entry are always 64-bit quantities, and efi_bool_t is always u8, so that shouldn't matter. > Would it be safer to skip the code completely on EFI_MIXED systems? > Obviously, but I would like to avoid that if possible. Let's see first if we've pinpointed it now. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Tue, Mar 13, 2018 at 8:59 AM Ard Biesheuvelwrote: > On 13 March 2018 at 07:47, Hans de Goede wrote: > > Hi, > > > > > > On 12-03-18 20:55, Thiebaud Weksteen wrote: > >> > ... > >> > >> Hans, you said you configured the tablet to use the 32-bit version of grub > >> instead > >> of 64. Why's that? > > > > > > Because this tablet, like (almost?) all Bay Trail hardware has a 32 bit > > UEFI, > > even though the processor is 64 bit capable (AFAIK 64 bit Windows drivers > > were > > not ready in time so all Bay Trail devices shipped with a 32 bit Windows). > > > > So this is running a 32 bit grub which boots a 64 bit kernel. > > > >> Jeremy, could you confirm if you are building the kernel in 64bit mode? Is > >> your Android install working? (that is, what happens if you boot > >> Boot)? > > > > > > AFAIK the kernel on Jeremy's tablet (which I initially installed) is 64 bit. > > > > Could the problem perhaps be that the new code for the TPM event-log is > > missing some handling to deal with running on a 32 bit firmware? I know the > > rest of the kernel has special code to deal with this. > > Yes, that was my guess as well. > That is a very good point, and I missed that this is a 64-bit kernel > running on 32-bit UEFI. > The TPM code does use efi_call_proto() directly, and now I am thinking > it is perhaps the allocate_pages() call that simply only initializes > the low 32-bits of log_tbl. That make sense. Would you know what happens to the arguments of the function in this case? (I'm thinking _location ?) Would it be safer to skip the code completely on EFI_MIXED systems? > Jeremy, could you please try initializing tcg2_protocol and log_tbl to > NULL at the start of the function? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi, On 12-03-18 22:02, Ard Biesheuvel wrote: On 12 March 2018 at 19:55, Thiebaud Weksteenwrote: On Mon, Mar 12, 2018 at 7:33 PM Jeremy Cline wrote: On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel < ard.biesheu...@linaro.org> wrote: On 12 March 2018 at 17:01, Jeremy Cline wrote: On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: On 12 March 2018 at 14:30, Jeremy Cline wrote: On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: Thanks a lot for trying out the patch! Please don't modify your install at this stage, I think we are hitting a firmware bug and that would be awesome if we can fix how we are handling it. So, if we reach that stage in the function it could either be that: * The allocation did not succeed, somehow, but the firmware still returned EFI_SUCCEED. * The size requested is incorrect (I'm thinking something like a 1G of log). This would be due to either a miscalculation of log_size (possible) or; the returned values of GetEventLog are not correct. I'm sending a patch to add checks for these. Could you please apply and retest? Again, thanks for helping debugging this. No problem, thanks for the help :) With the new patch: Locating the TCG2Protocol Calling GetEventLog on TCG2Protocol Log returned log_location is not empty log_size != 0 log_size < 1M Allocating memory for storing the logs Returned from memory allocation Copying log to new location And then it hangs. I added a couple more print statements: diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index ee3fac109078..1ab5638bc50e 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -148,8 +148,11 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) efi_printk(sys_table_arg, "Copying log to new location\n"); memset(log_tbl, 0, sizeof(*log_tbl) + log_size); + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); log_tbl->size = log_size; + efi_printk(sys_table_arg, "Set log_tbl->size\n"); log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + efi_printk(sys_table_arg, "Set log_tbl-version\n"); memcpy(log_tbl->log, (void *) first_entry_addr, log_size); efi_printk(sys_table_arg, "Installing the log into the configuration table\n"); and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" Thanks. Well, it looks like the memory that is supposedly allocated is not usable. I'm thinking this is a firmware bug. Ard, would you agree on this assumption? Thoughts on how to proceed? I am rather puzzled why the allocate_pool() should succeed and the subsequent memset() should fail. This does not look like an issue that is intimately related to TPM2 support, rather an issue in the firmware that happens to get tickled after the change. Would you mind trying replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA in the allocate_pool() call? Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the memset() call. Could you try the following please? (attached as well in case gmail mangles it) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 2298560cea72..30d960a344b7 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -70,6 +70,8 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) size_t log_size, last_entry_size; efi_bool_t truncated; void *tcg2_protocol; + unsigned long num_pages; + efi_physical_addr_t log_tbl_alloc; status = efi_call_early(locate_protocol, _guid, NULL, _protocol); @@ -104,9 +106,12 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) } /* Allocate space for the logs and copy them. */ - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, - sizeof(*log_tbl) + log_size, - (void **) _tbl); + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); + status = efi_call_early(allocate_pages, + EFI_ALLOCATE_ANY_PAGES, + EFI_LOADER_DATA, + num_pages, + _tbl_alloc); if (status != EFI_SUCCESS) { efi_printk(sys_table_arg, @@ -114,6 +119,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; } + log_tbl =
Re: Regression from efi: call get_event_log before ExitBootServices
On 13 March 2018 at 07:59, Ard Biesheuvelwrote: > On 13 March 2018 at 07:47, Hans de Goede wrote: >> Hi, >> >> >> On 12-03-18 20:55, Thiebaud Weksteen wrote: >>> > ... >>> >>> Hans, you said you configured the tablet to use the 32-bit version of grub >>> instead >>> of 64. Why's that? >> >> >> Because this tablet, like (almost?) all Bay Trail hardware has a 32 bit >> UEFI, >> even though the processor is 64 bit capable (AFAIK 64 bit Windows drivers >> were >> not ready in time so all Bay Trail devices shipped with a 32 bit Windows). >> >> So this is running a 32 bit grub which boots a 64 bit kernel. >> >>> Jeremy, could you confirm if you are building the kernel in 64bit mode? Is >>> your Android install working? (that is, what happens if you boot >>> Boot)? >> >> >> AFAIK the kernel on Jeremy's tablet (which I initially installed) is 64 bit. >> >> Could the problem perhaps be that the new code for the TPM event-log is >> missing some handling to deal with running on a 32 bit firmware? I know the >> rest of the kernel has special code to deal with this. >> > > That is a very good point, and I missed that this is a 64-bit kernel > running on 32-bit UEFI. > > The TPM code does use efi_call_proto() directly, *correctly* > and now I am thinking > it is perhaps the allocate_pages() call that simply only initializes > the low 32-bits of log_tbl. > > Jeremy, could you please try initializing tcg2_protocol and log_tbl to > NULL at the start of the function? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 13 March 2018 at 07:47, Hans de Goedewrote: > Hi, > > > On 12-03-18 20:55, Thiebaud Weksteen wrote: >> ... >> >> Hans, you said you configured the tablet to use the 32-bit version of grub >> instead >> of 64. Why's that? > > > Because this tablet, like (almost?) all Bay Trail hardware has a 32 bit > UEFI, > even though the processor is 64 bit capable (AFAIK 64 bit Windows drivers > were > not ready in time so all Bay Trail devices shipped with a 32 bit Windows). > > So this is running a 32 bit grub which boots a 64 bit kernel. > >> Jeremy, could you confirm if you are building the kernel in 64bit mode? Is >> your Android install working? (that is, what happens if you boot >> Boot)? > > > AFAIK the kernel on Jeremy's tablet (which I initially installed) is 64 bit. > > Could the problem perhaps be that the new code for the TPM event-log is > missing some handling to deal with running on a 32 bit firmware? I know the > rest of the kernel has special code to deal with this. > That is a very good point, and I missed that this is a 64-bit kernel running on 32-bit UEFI. The TPM code does use efi_call_proto() directly, and now I am thinking it is perhaps the allocate_pages() call that simply only initializes the low 32-bits of log_tbl. Jeremy, could you please try initializing tcg2_protocol and log_tbl to NULL at the start of the function? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi, On 12-03-18 20:55, Thiebaud Weksteen wrote: On Mon, Mar 12, 2018 at 7:33 PM Jeremy Clinewrote: On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel < ard.biesheu...@linaro.org> wrote: On 12 March 2018 at 17:01, Jeremy Cline wrote: On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: On 12 March 2018 at 14:30, Jeremy Cline wrote: On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: Thanks a lot for trying out the patch! Please don't modify your install at this stage, I think we are hitting a firmware bug and that would be awesome if we can fix how we are handling it. So, if we reach that stage in the function it could either be that: * The allocation did not succeed, somehow, but the firmware still returned EFI_SUCCEED. * The size requested is incorrect (I'm thinking something like a 1G of log). This would be due to either a miscalculation of log_size (possible) or; the returned values of GetEventLog are not correct. I'm sending a patch to add checks for these. Could you please apply and retest? Again, thanks for helping debugging this. No problem, thanks for the help :) With the new patch: Locating the TCG2Protocol Calling GetEventLog on TCG2Protocol Log returned log_location is not empty log_size != 0 log_size < 1M Allocating memory for storing the logs Returned from memory allocation Copying log to new location And then it hangs. I added a couple more print statements: diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index ee3fac109078..1ab5638bc50e 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -148,8 +148,11 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) efi_printk(sys_table_arg, "Copying log to new location\n"); memset(log_tbl, 0, sizeof(*log_tbl) + log_size); + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); log_tbl->size = log_size; + efi_printk(sys_table_arg, "Set log_tbl->size\n"); log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + efi_printk(sys_table_arg, "Set log_tbl-version\n"); memcpy(log_tbl->log, (void *) first_entry_addr, log_size); efi_printk(sys_table_arg, "Installing the log into the configuration table\n"); and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" Thanks. Well, it looks like the memory that is supposedly allocated is not usable. I'm thinking this is a firmware bug. Ard, would you agree on this assumption? Thoughts on how to proceed? I am rather puzzled why the allocate_pool() should succeed and the subsequent memset() should fail. This does not look like an issue that is intimately related to TPM2 support, rather an issue in the firmware that happens to get tickled after the change. Would you mind trying replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA in the allocate_pool() call? Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the memset() call. Could you try the following please? (attached as well in case gmail mangles it) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 2298560cea72..30d960a344b7 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -70,6 +70,8 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) size_t log_size, last_entry_size; efi_bool_t truncated; void *tcg2_protocol; + unsigned long num_pages; + efi_physical_addr_t log_tbl_alloc; status = efi_call_early(locate_protocol, _guid, NULL, _protocol); @@ -104,9 +106,12 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) } /* Allocate space for the logs and copy them. */ - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, - sizeof(*log_tbl) + log_size, - (void **) _tbl); + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); + status = efi_call_early(allocate_pages, + EFI_ALLOCATE_ANY_PAGES, + EFI_LOADER_DATA, + num_pages, + _tbl_alloc); if (status != EFI_SUCCESS) { efi_printk(sys_table_arg, @@ -114,6 +119,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; } + log_tbl = (struct linux_efi_tpm_eventlog *)(unsigned long)log_tbl_alloc;
Re: Regression from efi: call get_event_log before ExitBootServices
On Mon, Mar 12, 2018 at 10:03 PM Ard Biesheuvelwrote: > On 12 March 2018 at 19:55, Thiebaud Weksteen wrote: > > On Mon, Mar 12, 2018 at 7:33 PM Jeremy Cline wrote: > > > >> On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: > >> > On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel < > > ard.biesheu...@linaro.org> > >> > wrote: > >> > > >> >> On 12 March 2018 at 17:01, Jeremy Cline wrote: > >> >>> On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: > >> On 12 March 2018 at 14:30, Jeremy Cline wrote: > >> > On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: > >> >> On 10 March 2018 at 10:45, Thiebaud Weksteen > >> > wrote: > >> >>> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline > >> > wrote: > >> >>> > >> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen > > wrote: > >> > Thanks a lot for trying out the patch! > >> > > >> > Please don't modify your install at this stage, I think we are > >> > hitting a > >> > firmware bug and that would be awesome if we can fix how we are > >> >>> handling it. > >> > So, if we reach that stage in the function it could either be > >> > that: > >> > * The allocation did not succeed, somehow, but the firmware > > still > >> >>> returned > >> > EFI_SUCCEED. > >> > * The size requested is incorrect (I'm thinking something like a > >> > 1G of > >> > log). This would be due to either a miscalculation of log_size > >> >>> (possible) > >> > or; the returned values of GetEventLog are not correct. > >> > I'm sending a patch to add checks for these. Could you please > >> > apply and > >> > retest? > >> > Again, thanks for helping debugging this. > >> >>> > >> No problem, thanks for the help :) > >> >>> > >> With the new patch: > >> >>> > >> Locating the TCG2Protocol > >> Calling GetEventLog on TCG2Protocol > >> Log returned > >> log_location is not empty > >> log_size != 0 > >> log_size < 1M > >> Allocating memory for storing the logs > >> Returned from memory allocation > >> Copying log to new location > >> >>> > >> And then it hangs. I added a couple more print statements: > >> >>> > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > >> >>> b/drivers/firmware/efi/libstub/tpm.c > >> index ee3fac109078..1ab5638bc50e 100644 > >> --- a/drivers/firmware/efi/libstub/tpm.c > >> +++ b/drivers/firmware/efi/libstub/tpm.c > >> @@ -148,8 +148,11 @@ void > >> >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > >> efi_printk(sys_table_arg, "Copying log to new > >> > location\n"); > >> >>> > >> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); > >> + efi_printk(sys_table_arg, "Successfully memset log_tbl to > >> > 0\n"); > >> log_tbl->size = log_size; > >> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); > >> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; > >> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); > >> memcpy(log_tbl->log, (void *) first_entry_addr, > > log_size); > >> >>> > >> efi_printk(sys_table_arg, "Installing the log into the > >> >>> configuration table\n"); > >> >>> > >> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + > >> > log_size);" > >> >>> > >> >>> Thanks. Well, it looks like the memory that is supposedly > > allocated > >> > is not > >> >>> usable. I'm thinking this is a firmware bug. > >> >>> Ard, would you agree on this assumption? Thoughts on how to > > proceed? > >> >>> > >> >> > >> >> I am rather puzzled why the allocate_pool() should succeed and the > >> >> subsequent memset() should fail. This does not look like an issue > >> > that > >> >> is intimately related to TPM2 support, rather an issue in the > >> > firmware > >> >> that happens to get tickled after the change. > >> >> > >> >> Would you mind trying replacing EFI_LOADER_DATA with > >> >> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? > >> > > >> > Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at > >> > the > >> > memset() call. > >> > > >> > >> Could you try the following please? (attached as well in case gmail > >> > mangles it) > >> > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > >> b/drivers/firmware/efi/libstub/tpm.c > >> index 2298560cea72..30d960a344b7 100644 > >> --- a/drivers/firmware/efi/libstub/tpm.c > >> +++ b/drivers/firmware/efi/libstub/tpm.c > >> @@ -70,6 +70,8 @@ void > >>
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/12/2018 03:55 PM, Thiebaud Weksteen wrote: > On Mon, Mar 12, 2018 at 7:33 PM Jeremy Clinewrote: > >> On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: >>> On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel < > ard.biesheu...@linaro.org> >>> wrote: >>> On 12 March 2018 at 17:01, Jeremy Cline wrote: > On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: >> On 12 March 2018 at 14:30, Jeremy Cline wrote: >>> On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: On 10 March 2018 at 10:45, Thiebaud Weksteen >>> wrote: > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline >>> wrote: > >> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen > wrote: >>> Thanks a lot for trying out the patch! >>> >>> Please don't modify your install at this stage, I think we are >>> hitting a >>> firmware bug and that would be awesome if we can fix how we are > handling it. >>> So, if we reach that stage in the function it could either be >>> that: >>> * The allocation did not succeed, somehow, but the firmware > still > returned >>> EFI_SUCCEED. >>> * The size requested is incorrect (I'm thinking something like a >>> 1G of >>> log). This would be due to either a miscalculation of log_size > (possible) >>> or; the returned values of GetEventLog are not correct. >>> I'm sending a patch to add checks for these. Could you please >>> apply and >>> retest? >>> Again, thanks for helping debugging this. > >> No problem, thanks for the help :) > >> With the new patch: > >> Locating the TCG2Protocol >> Calling GetEventLog on TCG2Protocol >> Log returned >> log_location is not empty >> log_size != 0 >> log_size < 1M >> Allocating memory for storing the logs >> Returned from memory allocation >> Copying log to new location > >> And then it hangs. I added a couple more print statements: > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > b/drivers/firmware/efi/libstub/tpm.c >> index ee3fac109078..1ab5638bc50e 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -148,8 +148,11 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> efi_printk(sys_table_arg, "Copying log to new >>> location\n"); > >> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >> + efi_printk(sys_table_arg, "Successfully memset log_tbl to >>> 0\n"); >> log_tbl->size = log_size; >> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >> memcpy(log_tbl->log, (void *) first_entry_addr, > log_size); > >> efi_printk(sys_table_arg, "Installing the log into the > configuration table\n"); > >> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + >>> log_size);" > > Thanks. Well, it looks like the memory that is supposedly > allocated >>> is not > usable. I'm thinking this is a firmware bug. > Ard, would you agree on this assumption? Thoughts on how to > proceed? > I am rather puzzled why the allocate_pool() should succeed and the subsequent memset() should fail. This does not look like an issue >>> that is intimately related to TPM2 support, rather an issue in the >>> firmware that happens to get tickled after the change. Would you mind trying replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA in the allocate_pool() call? >>> >>> Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at >>> the >>> memset() call. >>> >> >> Could you try the following please? (attached as well in case gmail >>> mangles it) >> >> diff --git a/drivers/firmware/efi/libstub/tpm.c >> b/drivers/firmware/efi/libstub/tpm.c >> index 2298560cea72..30d960a344b7 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -70,6 +70,8 @@ void >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> size_t log_size, last_entry_size; >> efi_bool_t truncated; >> void *tcg2_protocol; >> + unsigned long num_pages; >> + efi_physical_addr_t log_tbl_alloc; >> >> status = efi_call_early(locate_protocol, _guid, NULL, >> _protocol); >> @@ -104,9 +106,12 @@ void
Re: Regression from efi: call get_event_log before ExitBootServices
On 12 March 2018 at 19:55, Thiebaud Weksteenwrote: > On Mon, Mar 12, 2018 at 7:33 PM Jeremy Cline wrote: > >> On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: >> > On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel < > ard.biesheu...@linaro.org> >> > wrote: >> > >> >> On 12 March 2018 at 17:01, Jeremy Cline wrote: >> >>> On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: >> On 12 March 2018 at 14:30, Jeremy Cline wrote: >> > On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: >> >> On 10 March 2018 at 10:45, Thiebaud Weksteen >> > wrote: >> >>> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline >> > wrote: >> >>> >> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen > wrote: >> > Thanks a lot for trying out the patch! >> > >> > Please don't modify your install at this stage, I think we are >> > hitting a >> > firmware bug and that would be awesome if we can fix how we are >> >>> handling it. >> > So, if we reach that stage in the function it could either be >> > that: >> > * The allocation did not succeed, somehow, but the firmware > still >> >>> returned >> > EFI_SUCCEED. >> > * The size requested is incorrect (I'm thinking something like a >> > 1G of >> > log). This would be due to either a miscalculation of log_size >> >>> (possible) >> > or; the returned values of GetEventLog are not correct. >> > I'm sending a patch to add checks for these. Could you please >> > apply and >> > retest? >> > Again, thanks for helping debugging this. >> >>> >> No problem, thanks for the help :) >> >>> >> With the new patch: >> >>> >> Locating the TCG2Protocol >> Calling GetEventLog on TCG2Protocol >> Log returned >> log_location is not empty >> log_size != 0 >> log_size < 1M >> Allocating memory for storing the logs >> Returned from memory allocation >> Copying log to new location >> >>> >> And then it hangs. I added a couple more print statements: >> >>> >> diff --git a/drivers/firmware/efi/libstub/tpm.c >> >>> b/drivers/firmware/efi/libstub/tpm.c >> index ee3fac109078..1ab5638bc50e 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -148,8 +148,11 @@ void >> >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> efi_printk(sys_table_arg, "Copying log to new >> > location\n"); >> >>> >> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >> + efi_printk(sys_table_arg, "Successfully memset log_tbl to >> > 0\n"); >> log_tbl->size = log_size; >> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >> memcpy(log_tbl->log, (void *) first_entry_addr, > log_size); >> >>> >> efi_printk(sys_table_arg, "Installing the log into the >> >>> configuration table\n"); >> >>> >> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + >> > log_size);" >> >>> >> >>> Thanks. Well, it looks like the memory that is supposedly > allocated >> > is not >> >>> usable. I'm thinking this is a firmware bug. >> >>> Ard, would you agree on this assumption? Thoughts on how to > proceed? >> >>> >> >> >> >> I am rather puzzled why the allocate_pool() should succeed and the >> >> subsequent memset() should fail. This does not look like an issue >> > that >> >> is intimately related to TPM2 support, rather an issue in the >> > firmware >> >> that happens to get tickled after the change. >> >> >> >> Would you mind trying replacing EFI_LOADER_DATA with >> >> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? >> > >> > Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at >> > the >> > memset() call. >> > >> >> Could you try the following please? (attached as well in case gmail >> > mangles it) >> >> diff --git a/drivers/firmware/efi/libstub/tpm.c >> b/drivers/firmware/efi/libstub/tpm.c >> index 2298560cea72..30d960a344b7 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -70,6 +70,8 @@ void >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> size_t log_size, last_entry_size; >> efi_bool_t truncated; >> void *tcg2_protocol; >> + unsigned long num_pages; >> + efi_physical_addr_t log_tbl_alloc; >> >>
Re: Regression from efi: call get_event_log before ExitBootServices
On Mon, Mar 12, 2018 at 7:33 PM Jeremy Clinewrote: > On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: > > On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel < ard.biesheu...@linaro.org> > > wrote: > > > >> On 12 March 2018 at 17:01, Jeremy Cline wrote: > >>> On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: > On 12 March 2018 at 14:30, Jeremy Cline wrote: > > On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: > >> On 10 March 2018 at 10:45, Thiebaud Weksteen > > wrote: > >>> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline > > wrote: > >>> > On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: > > Thanks a lot for trying out the patch! > > > > Please don't modify your install at this stage, I think we are > > hitting a > > firmware bug and that would be awesome if we can fix how we are > >>> handling it. > > So, if we reach that stage in the function it could either be > > that: > > * The allocation did not succeed, somehow, but the firmware still > >>> returned > > EFI_SUCCEED. > > * The size requested is incorrect (I'm thinking something like a > > 1G of > > log). This would be due to either a miscalculation of log_size > >>> (possible) > > or; the returned values of GetEventLog are not correct. > > I'm sending a patch to add checks for these. Could you please > > apply and > > retest? > > Again, thanks for helping debugging this. > >>> > No problem, thanks for the help :) > >>> > With the new patch: > >>> > Locating the TCG2Protocol > Calling GetEventLog on TCG2Protocol > Log returned > log_location is not empty > log_size != 0 > log_size < 1M > Allocating memory for storing the logs > Returned from memory allocation > Copying log to new location > >>> > And then it hangs. I added a couple more print statements: > >>> > diff --git a/drivers/firmware/efi/libstub/tpm.c > >>> b/drivers/firmware/efi/libstub/tpm.c > index ee3fac109078..1ab5638bc50e 100644 > --- a/drivers/firmware/efi/libstub/tpm.c > +++ b/drivers/firmware/efi/libstub/tpm.c > @@ -148,8 +148,11 @@ void > >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > efi_printk(sys_table_arg, "Copying log to new > > location\n"); > >>> > memset(log_tbl, 0, sizeof(*log_tbl) + log_size); > + efi_printk(sys_table_arg, "Successfully memset log_tbl to > > 0\n"); > log_tbl->size = log_size; > + efi_printk(sys_table_arg, "Set log_tbl->size\n"); > log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; > + efi_printk(sys_table_arg, "Set log_tbl-version\n"); > memcpy(log_tbl->log, (void *) first_entry_addr, log_size); > >>> > efi_printk(sys_table_arg, "Installing the log into the > >>> configuration table\n"); > >>> > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + > > log_size);" > >>> > >>> Thanks. Well, it looks like the memory that is supposedly allocated > > is not > >>> usable. I'm thinking this is a firmware bug. > >>> Ard, would you agree on this assumption? Thoughts on how to proceed? > >>> > >> > >> I am rather puzzled why the allocate_pool() should succeed and the > >> subsequent memset() should fail. This does not look like an issue > > that > >> is intimately related to TPM2 support, rather an issue in the > > firmware > >> that happens to get tickled after the change. > >> > >> Would you mind trying replacing EFI_LOADER_DATA with > >> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? > > > > Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at > > the > > memset() call. > > > > Could you try the following please? (attached as well in case gmail > > mangles it) > > diff --git a/drivers/firmware/efi/libstub/tpm.c > b/drivers/firmware/efi/libstub/tpm.c > index 2298560cea72..30d960a344b7 100644 > --- a/drivers/firmware/efi/libstub/tpm.c > +++ b/drivers/firmware/efi/libstub/tpm.c > @@ -70,6 +70,8 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > size_t log_size, last_entry_size; > efi_bool_t truncated; > void *tcg2_protocol; > + unsigned long num_pages; > + efi_physical_addr_t log_tbl_alloc; > > status = efi_call_early(locate_protocol, _guid, NULL, > _protocol); > @@ -104,9 +106,12 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/12/2018 02:29 PM, Thiebaud Weksteen wrote: > On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvel> wrote: > >> On 12 March 2018 at 17:01, Jeremy Cline wrote: >>> On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: On 12 March 2018 at 14:30, Jeremy Cline wrote: > On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: >> On 10 March 2018 at 10:45, Thiebaud Weksteen > wrote: >>> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline > wrote: >>> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: > Thanks a lot for trying out the patch! > > Please don't modify your install at this stage, I think we are > hitting a > firmware bug and that would be awesome if we can fix how we are >>> handling it. > So, if we reach that stage in the function it could either be > that: > * The allocation did not succeed, somehow, but the firmware still >>> returned > EFI_SUCCEED. > * The size requested is incorrect (I'm thinking something like a > 1G of > log). This would be due to either a miscalculation of log_size >>> (possible) > or; the returned values of GetEventLog are not correct. > I'm sending a patch to add checks for these. Could you please > apply and > retest? > Again, thanks for helping debugging this. >>> No problem, thanks for the help :) >>> With the new patch: >>> Locating the TCG2Protocol Calling GetEventLog on TCG2Protocol Log returned log_location is not empty log_size != 0 log_size < 1M Allocating memory for storing the logs Returned from memory allocation Copying log to new location >>> And then it hangs. I added a couple more print statements: >>> diff --git a/drivers/firmware/efi/libstub/tpm.c >>> b/drivers/firmware/efi/libstub/tpm.c index ee3fac109078..1ab5638bc50e 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -148,8 +148,11 @@ void >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) efi_printk(sys_table_arg, "Copying log to new > location\n"); >>> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); + efi_printk(sys_table_arg, "Successfully memset log_tbl to > 0\n"); log_tbl->size = log_size; + efi_printk(sys_table_arg, "Set log_tbl->size\n"); log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + efi_printk(sys_table_arg, "Set log_tbl-version\n"); memcpy(log_tbl->log, (void *) first_entry_addr, log_size); >>> efi_printk(sys_table_arg, "Installing the log into the >>> configuration table\n"); >>> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + > log_size);" >>> >>> Thanks. Well, it looks like the memory that is supposedly allocated > is not >>> usable. I'm thinking this is a firmware bug. >>> Ard, would you agree on this assumption? Thoughts on how to proceed? >>> >> >> I am rather puzzled why the allocate_pool() should succeed and the >> subsequent memset() should fail. This does not look like an issue > that >> is intimately related to TPM2 support, rather an issue in the > firmware >> that happens to get tickled after the change. >> >> Would you mind trying replacing EFI_LOADER_DATA with >> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? > > Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at > the > memset() call. > Could you try the following please? (attached as well in case gmail > mangles it) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 2298560cea72..30d960a344b7 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -70,6 +70,8 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) size_t log_size, last_entry_size; efi_bool_t truncated; void *tcg2_protocol; + unsigned long num_pages; + efi_physical_addr_t log_tbl_alloc; status = efi_call_early(locate_protocol, _guid, NULL, _protocol); @@ -104,9 +106,12 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) } /* Allocate space for the logs and copy them. */ - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, - sizeof(*log_tbl) + log_size, - (void **) _tbl); + num_pages =
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/12/2018 01:30 PM, Ard Biesheuvel wrote: > On 12 March 2018 at 17:01, Jeremy Clinewrote: >> On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: >>> On 12 March 2018 at 14:30, Jeremy Cline wrote: On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: > On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: >> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: >> >>> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: Thanks a lot for trying out the patch! Please don't modify your install at this stage, I think we are hitting a firmware bug and that would be awesome if we can fix how we are >> handling it. So, if we reach that stage in the function it could either be that: * The allocation did not succeed, somehow, but the firmware still >> returned EFI_SUCCEED. * The size requested is incorrect (I'm thinking something like a 1G of log). This would be due to either a miscalculation of log_size >> (possible) or; the returned values of GetEventLog are not correct. I'm sending a patch to add checks for these. Could you please apply and retest? Again, thanks for helping debugging this. >> >>> No problem, thanks for the help :) >> >>> With the new patch: >> >>> Locating the TCG2Protocol >>> Calling GetEventLog on TCG2Protocol >>> Log returned >>> log_location is not empty >>> log_size != 0 >>> log_size < 1M >>> Allocating memory for storing the logs >>> Returned from memory allocation >>> Copying log to new location >> >>> And then it hangs. I added a couple more print statements: >> >>> diff --git a/drivers/firmware/efi/libstub/tpm.c >> b/drivers/firmware/efi/libstub/tpm.c >>> index ee3fac109078..1ab5638bc50e 100644 >>> --- a/drivers/firmware/efi/libstub/tpm.c >>> +++ b/drivers/firmware/efi/libstub/tpm.c >>> @@ -148,8 +148,11 @@ void >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >>> efi_printk(sys_table_arg, "Copying log to new location\n"); >> >>> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >>> + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); >>> log_tbl->size = log_size; >>> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >>> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >>> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >>> memcpy(log_tbl->log, (void *) first_entry_addr, log_size); >> >>> efi_printk(sys_table_arg, "Installing the log into the >> configuration table\n"); >> >>> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" >> >> Thanks. Well, it looks like the memory that is supposedly allocated is >> not >> usable. I'm thinking this is a firmware bug. >> Ard, would you agree on this assumption? Thoughts on how to proceed? >> > > I am rather puzzled why the allocate_pool() should succeed and the > subsequent memset() should fail. This does not look like an issue that > is intimately related to TPM2 support, rather an issue in the firmware > that happens to get tickled after the change. > > Would you mind trying replacing EFI_LOADER_DATA with > EFI_BOOT_SERVICES_DATA in the allocate_pool() call? Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the memset() call. >>> >>> Could you try the following please? (attached as well in case gmail mangles >>> it) >>> >>> diff --git a/drivers/firmware/efi/libstub/tpm.c >>> b/drivers/firmware/efi/libstub/tpm.c >>> index 2298560cea72..30d960a344b7 100644 >>> --- a/drivers/firmware/efi/libstub/tpm.c >>> +++ b/drivers/firmware/efi/libstub/tpm.c >>> @@ -70,6 +70,8 @@ void >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >>> size_t log_size, last_entry_size; >>> efi_bool_t truncated; >>> void *tcg2_protocol; >>> + unsigned long num_pages; >>> + efi_physical_addr_t log_tbl_alloc; >>> >>> status = efi_call_early(locate_protocol, _guid, NULL, >>> _protocol); >>> @@ -104,9 +106,12 @@ void >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >>> } >>> >>> /* Allocate space for the logs and copy them. */ >>> - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, >>> - sizeof(*log_tbl) + log_size, >>> - (void **) _tbl); >>> + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, >>> EFI_PAGE_SIZE); >>> + status = efi_call_early(allocate_pages, >>> + EFI_ALLOCATE_ANY_PAGES, >>> +
Re: Regression from efi: call get_event_log before ExitBootServices
On Mon, Mar 12, 2018 at 6:30 PM Ard Biesheuvelwrote: > On 12 March 2018 at 17:01, Jeremy Cline wrote: > > On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: > >> On 12 March 2018 at 14:30, Jeremy Cline wrote: > >>> On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: > On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: > > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: > > > >> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: > >>> Thanks a lot for trying out the patch! > >>> > >>> Please don't modify your install at this stage, I think we are hitting a > >>> firmware bug and that would be awesome if we can fix how we are > > handling it. > >>> So, if we reach that stage in the function it could either be that: > >>> * The allocation did not succeed, somehow, but the firmware still > > returned > >>> EFI_SUCCEED. > >>> * The size requested is incorrect (I'm thinking something like a 1G of > >>> log). This would be due to either a miscalculation of log_size > > (possible) > >>> or; the returned values of GetEventLog are not correct. > >>> I'm sending a patch to add checks for these. Could you please apply and > >>> retest? > >>> Again, thanks for helping debugging this. > > > >> No problem, thanks for the help :) > > > >> With the new patch: > > > >> Locating the TCG2Protocol > >> Calling GetEventLog on TCG2Protocol > >> Log returned > >> log_location is not empty > >> log_size != 0 > >> log_size < 1M > >> Allocating memory for storing the logs > >> Returned from memory allocation > >> Copying log to new location > > > >> And then it hangs. I added a couple more print statements: > > > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > > b/drivers/firmware/efi/libstub/tpm.c > >> index ee3fac109078..1ab5638bc50e 100644 > >> --- a/drivers/firmware/efi/libstub/tpm.c > >> +++ b/drivers/firmware/efi/libstub/tpm.c > >> @@ -148,8 +148,11 @@ void > > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > >> efi_printk(sys_table_arg, "Copying log to new location\n"); > > > >> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); > >> + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); > >> log_tbl->size = log_size; > >> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); > >> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; > >> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); > >> memcpy(log_tbl->log, (void *) first_entry_addr, log_size); > > > >> efi_printk(sys_table_arg, "Installing the log into the > > configuration table\n"); > > > >> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" > > > > Thanks. Well, it looks like the memory that is supposedly allocated is not > > usable. I'm thinking this is a firmware bug. > > Ard, would you agree on this assumption? Thoughts on how to proceed? > > > > I am rather puzzled why the allocate_pool() should succeed and the > subsequent memset() should fail. This does not look like an issue that > is intimately related to TPM2 support, rather an issue in the firmware > that happens to get tickled after the change. > > Would you mind trying replacing EFI_LOADER_DATA with > EFI_BOOT_SERVICES_DATA in the allocate_pool() call? > >>> > >>> Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the > >>> memset() call. > >>> > >> > >> Could you try the following please? (attached as well in case gmail mangles it) > >> > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > >> b/drivers/firmware/efi/libstub/tpm.c > >> index 2298560cea72..30d960a344b7 100644 > >> --- a/drivers/firmware/efi/libstub/tpm.c > >> +++ b/drivers/firmware/efi/libstub/tpm.c > >> @@ -70,6 +70,8 @@ void > >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > >> size_t log_size, last_entry_size; > >> efi_bool_t truncated; > >> void *tcg2_protocol; > >> + unsigned long num_pages; > >> + efi_physical_addr_t log_tbl_alloc; > >> > >> status = efi_call_early(locate_protocol, _guid, NULL, > >> _protocol); > >> @@ -104,9 +106,12 @@ void > >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > >> } > >> > >> /* Allocate space for the logs and copy them. */ > >> - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, > >> - sizeof(*log_tbl) + log_size, > >> - (void **) _tbl); > >> + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); > >> + status =
Re: Regression from efi: call get_event_log before ExitBootServices
On 12 March 2018 at 17:01, Jeremy Clinewrote: > On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: >> On 12 March 2018 at 14:30, Jeremy Cline wrote: >>> On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: > >> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: >>> Thanks a lot for trying out the patch! >>> >>> Please don't modify your install at this stage, I think we are hitting a >>> firmware bug and that would be awesome if we can fix how we are > handling it. >>> So, if we reach that stage in the function it could either be that: >>> * The allocation did not succeed, somehow, but the firmware still > returned >>> EFI_SUCCEED. >>> * The size requested is incorrect (I'm thinking something like a 1G of >>> log). This would be due to either a miscalculation of log_size > (possible) >>> or; the returned values of GetEventLog are not correct. >>> I'm sending a patch to add checks for these. Could you please apply and >>> retest? >>> Again, thanks for helping debugging this. > >> No problem, thanks for the help :) > >> With the new patch: > >> Locating the TCG2Protocol >> Calling GetEventLog on TCG2Protocol >> Log returned >> log_location is not empty >> log_size != 0 >> log_size < 1M >> Allocating memory for storing the logs >> Returned from memory allocation >> Copying log to new location > >> And then it hangs. I added a couple more print statements: > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > b/drivers/firmware/efi/libstub/tpm.c >> index ee3fac109078..1ab5638bc50e 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -148,8 +148,11 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> efi_printk(sys_table_arg, "Copying log to new location\n"); > >> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >> + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); >> log_tbl->size = log_size; >> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >> memcpy(log_tbl->log, (void *) first_entry_addr, log_size); > >> efi_printk(sys_table_arg, "Installing the log into the > configuration table\n"); > >> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" > > Thanks. Well, it looks like the memory that is supposedly allocated is not > usable. I'm thinking this is a firmware bug. > Ard, would you agree on this assumption? Thoughts on how to proceed? > I am rather puzzled why the allocate_pool() should succeed and the subsequent memset() should fail. This does not look like an issue that is intimately related to TPM2 support, rather an issue in the firmware that happens to get tickled after the change. Would you mind trying replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA in the allocate_pool() call? >>> >>> Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the >>> memset() call. >>> >> >> Could you try the following please? (attached as well in case gmail mangles >> it) >> >> diff --git a/drivers/firmware/efi/libstub/tpm.c >> b/drivers/firmware/efi/libstub/tpm.c >> index 2298560cea72..30d960a344b7 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -70,6 +70,8 @@ void >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> size_t log_size, last_entry_size; >> efi_bool_t truncated; >> void *tcg2_protocol; >> + unsigned long num_pages; >> + efi_physical_addr_t log_tbl_alloc; >> >> status = efi_call_early(locate_protocol, _guid, NULL, >> _protocol); >> @@ -104,9 +106,12 @@ void >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> } >> >> /* Allocate space for the logs and copy them. */ >> - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, >> - sizeof(*log_tbl) + log_size, >> - (void **) _tbl); >> + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); >> + status = efi_call_early(allocate_pages, >> + EFI_ALLOCATE_ANY_PAGES, >> + EFI_LOADER_DATA, >> + num_pages, >> + _tbl_alloc); >> >> if (status != EFI_SUCCESS) { >>
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/12/2018 10:56 AM, Ard Biesheuvel wrote: > On 12 March 2018 at 14:30, Jeremy Clinewrote: >> On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: >>> On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: > On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: >> Thanks a lot for trying out the patch! >> >> Please don't modify your install at this stage, I think we are hitting a >> firmware bug and that would be awesome if we can fix how we are handling it. >> So, if we reach that stage in the function it could either be that: >> * The allocation did not succeed, somehow, but the firmware still returned >> EFI_SUCCEED. >> * The size requested is incorrect (I'm thinking something like a 1G of >> log). This would be due to either a miscalculation of log_size (possible) >> or; the returned values of GetEventLog are not correct. >> I'm sending a patch to add checks for these. Could you please apply and >> retest? >> Again, thanks for helping debugging this. > No problem, thanks for the help :) > With the new patch: > Locating the TCG2Protocol > Calling GetEventLog on TCG2Protocol > Log returned > log_location is not empty > log_size != 0 > log_size < 1M > Allocating memory for storing the logs > Returned from memory allocation > Copying log to new location > And then it hangs. I added a couple more print statements: > diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c > index ee3fac109078..1ab5638bc50e 100644 > --- a/drivers/firmware/efi/libstub/tpm.c > +++ b/drivers/firmware/efi/libstub/tpm.c > @@ -148,8 +148,11 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > efi_printk(sys_table_arg, "Copying log to new location\n"); > memset(log_tbl, 0, sizeof(*log_tbl) + log_size); > + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); > log_tbl->size = log_size; > + efi_printk(sys_table_arg, "Set log_tbl->size\n"); > log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; > + efi_printk(sys_table_arg, "Set log_tbl-version\n"); > memcpy(log_tbl->log, (void *) first_entry_addr, log_size); > efi_printk(sys_table_arg, "Installing the log into the configuration table\n"); > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" Thanks. Well, it looks like the memory that is supposedly allocated is not usable. I'm thinking this is a firmware bug. Ard, would you agree on this assumption? Thoughts on how to proceed? >>> >>> I am rather puzzled why the allocate_pool() should succeed and the >>> subsequent memset() should fail. This does not look like an issue that >>> is intimately related to TPM2 support, rather an issue in the firmware >>> that happens to get tickled after the change. >>> >>> Would you mind trying replacing EFI_LOADER_DATA with >>> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? >> >> Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the >> memset() call. >> > > Could you try the following please? (attached as well in case gmail mangles > it) > > diff --git a/drivers/firmware/efi/libstub/tpm.c > b/drivers/firmware/efi/libstub/tpm.c > index 2298560cea72..30d960a344b7 100644 > --- a/drivers/firmware/efi/libstub/tpm.c > +++ b/drivers/firmware/efi/libstub/tpm.c > @@ -70,6 +70,8 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > size_t log_size, last_entry_size; > efi_bool_t truncated; > void *tcg2_protocol; > + unsigned long num_pages; > + efi_physical_addr_t log_tbl_alloc; > > status = efi_call_early(locate_protocol, _guid, NULL, > _protocol); > @@ -104,9 +106,12 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > } > > /* Allocate space for the logs and copy them. */ > - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, > - sizeof(*log_tbl) + log_size, > - (void **) _tbl); > + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); > + status = efi_call_early(allocate_pages, > + EFI_ALLOCATE_ANY_PAGES, > + EFI_LOADER_DATA, > + num_pages, > + _tbl_alloc); > > if (status != EFI_SUCCESS) { > efi_printk(sys_table_arg, > @@ -114,6 +119,7 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > return; > } > > + log_tbl =
Re: Regression from efi: call get_event_log before ExitBootServices
On 12 March 2018 at 14:30, Jeremy Clinewrote: > On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: >> On 10 March 2018 at 10:45, Thiebaud Weksteen wrote: >>> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: >>> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: > Thanks a lot for trying out the patch! > > Please don't modify your install at this stage, I think we are hitting a > firmware bug and that would be awesome if we can fix how we are >>> handling it. > So, if we reach that stage in the function it could either be that: > * The allocation did not succeed, somehow, but the firmware still >>> returned > EFI_SUCCEED. > * The size requested is incorrect (I'm thinking something like a 1G of > log). This would be due to either a miscalculation of log_size >>> (possible) > or; the returned values of GetEventLog are not correct. > I'm sending a patch to add checks for these. Could you please apply and > retest? > Again, thanks for helping debugging this. >>> No problem, thanks for the help :) >>> With the new patch: >>> Locating the TCG2Protocol Calling GetEventLog on TCG2Protocol Log returned log_location is not empty log_size != 0 log_size < 1M Allocating memory for storing the logs Returned from memory allocation Copying log to new location >>> And then it hangs. I added a couple more print statements: >>> diff --git a/drivers/firmware/efi/libstub/tpm.c >>> b/drivers/firmware/efi/libstub/tpm.c index ee3fac109078..1ab5638bc50e 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -148,8 +148,11 @@ void >>> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) efi_printk(sys_table_arg, "Copying log to new location\n"); >>> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); log_tbl->size = log_size; + efi_printk(sys_table_arg, "Set log_tbl->size\n"); log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + efi_printk(sys_table_arg, "Set log_tbl-version\n"); memcpy(log_tbl->log, (void *) first_entry_addr, log_size); >>> efi_printk(sys_table_arg, "Installing the log into the >>> configuration table\n"); >>> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" >>> >>> Thanks. Well, it looks like the memory that is supposedly allocated is not >>> usable. I'm thinking this is a firmware bug. >>> Ard, would you agree on this assumption? Thoughts on how to proceed? >>> >> >> I am rather puzzled why the allocate_pool() should succeed and the >> subsequent memset() should fail. This does not look like an issue that >> is intimately related to TPM2 support, rather an issue in the firmware >> that happens to get tickled after the change. >> >> Would you mind trying replacing EFI_LOADER_DATA with >> EFI_BOOT_SERVICES_DATA in the allocate_pool() call? > > Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the > memset() call. > Could you try the following please? (attached as well in case gmail mangles it) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 2298560cea72..30d960a344b7 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -70,6 +70,8 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) size_t log_size, last_entry_size; efi_bool_t truncated; void *tcg2_protocol; + unsigned long num_pages; + efi_physical_addr_t log_tbl_alloc; status = efi_call_early(locate_protocol, _guid, NULL, _protocol); @@ -104,9 +106,12 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) } /* Allocate space for the logs and copy them. */ - status = efi_call_early(allocate_pool, EFI_LOADER_DATA, - sizeof(*log_tbl) + log_size, - (void **) _tbl); + num_pages = DIV_ROUND_UP(sizeof(*log_tbl) + log_size, EFI_PAGE_SIZE); + status = efi_call_early(allocate_pages, + EFI_ALLOCATE_ANY_PAGES, + EFI_LOADER_DATA, + num_pages, + _tbl_alloc); if (status != EFI_SUCCESS) { efi_printk(sys_table_arg, @@ -114,6 +119,7 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) return; } + log_tbl = (struct linux_efi_tpm_eventlog *)(unsigned long)log_tbl_alloc; memset(log_tbl, 0, sizeof(*log_tbl) + log_size); log_tbl->size = log_size; log_tbl->version =
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/12/2018 07:08 AM, Ard Biesheuvel wrote: > On 10 March 2018 at 10:45, Thiebaud Weksteenwrote: >> On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: >> >>> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: Thanks a lot for trying out the patch! Please don't modify your install at this stage, I think we are hitting a firmware bug and that would be awesome if we can fix how we are >> handling it. So, if we reach that stage in the function it could either be that: * The allocation did not succeed, somehow, but the firmware still >> returned EFI_SUCCEED. * The size requested is incorrect (I'm thinking something like a 1G of log). This would be due to either a miscalculation of log_size >> (possible) or; the returned values of GetEventLog are not correct. I'm sending a patch to add checks for these. Could you please apply and retest? Again, thanks for helping debugging this. >> >>> No problem, thanks for the help :) >> >>> With the new patch: >> >>> Locating the TCG2Protocol >>> Calling GetEventLog on TCG2Protocol >>> Log returned >>> log_location is not empty >>> log_size != 0 >>> log_size < 1M >>> Allocating memory for storing the logs >>> Returned from memory allocation >>> Copying log to new location >> >>> And then it hangs. I added a couple more print statements: >> >>> diff --git a/drivers/firmware/efi/libstub/tpm.c >> b/drivers/firmware/efi/libstub/tpm.c >>> index ee3fac109078..1ab5638bc50e 100644 >>> --- a/drivers/firmware/efi/libstub/tpm.c >>> +++ b/drivers/firmware/efi/libstub/tpm.c >>> @@ -148,8 +148,11 @@ void >> efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >>> efi_printk(sys_table_arg, "Copying log to new location\n"); >> >>> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >>> + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); >>> log_tbl->size = log_size; >>> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >>> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >>> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >>> memcpy(log_tbl->log, (void *) first_entry_addr, log_size); >> >>> efi_printk(sys_table_arg, "Installing the log into the >> configuration table\n"); >> >>> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" >> >> Thanks. Well, it looks like the memory that is supposedly allocated is not >> usable. I'm thinking this is a firmware bug. >> Ard, would you agree on this assumption? Thoughts on how to proceed? >> > > I am rather puzzled why the allocate_pool() should succeed and the > subsequent memset() should fail. This does not look like an issue that > is intimately related to TPM2 support, rather an issue in the firmware > that happens to get tickled after the change. > > Would you mind trying replacing EFI_LOADER_DATA with > EFI_BOOT_SERVICES_DATA in the allocate_pool() call? Replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA still hangs at the memset() call. Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 10 March 2018 at 10:45, Thiebaud Weksteenwrote: > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Cline wrote: > >> On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: >> > Thanks a lot for trying out the patch! >> > >> > Please don't modify your install at this stage, I think we are hitting a >> > firmware bug and that would be awesome if we can fix how we are > handling it. >> > So, if we reach that stage in the function it could either be that: >> > * The allocation did not succeed, somehow, but the firmware still > returned >> > EFI_SUCCEED. >> > * The size requested is incorrect (I'm thinking something like a 1G of >> > log). This would be due to either a miscalculation of log_size > (possible) >> > or; the returned values of GetEventLog are not correct. >> > I'm sending a patch to add checks for these. Could you please apply and >> > retest? >> > Again, thanks for helping debugging this. > >> No problem, thanks for the help :) > >> With the new patch: > >> Locating the TCG2Protocol >> Calling GetEventLog on TCG2Protocol >> Log returned >> log_location is not empty >> log_size != 0 >> log_size < 1M >> Allocating memory for storing the logs >> Returned from memory allocation >> Copying log to new location > >> And then it hangs. I added a couple more print statements: > >> diff --git a/drivers/firmware/efi/libstub/tpm.c > b/drivers/firmware/efi/libstub/tpm.c >> index ee3fac109078..1ab5638bc50e 100644 >> --- a/drivers/firmware/efi/libstub/tpm.c >> +++ b/drivers/firmware/efi/libstub/tpm.c >> @@ -148,8 +148,11 @@ void > efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) >> efi_printk(sys_table_arg, "Copying log to new location\n"); > >> memset(log_tbl, 0, sizeof(*log_tbl) + log_size); >> + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); >> log_tbl->size = log_size; >> + efi_printk(sys_table_arg, "Set log_tbl->size\n"); >> log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; >> + efi_printk(sys_table_arg, "Set log_tbl-version\n"); >> memcpy(log_tbl->log, (void *) first_entry_addr, log_size); > >> efi_printk(sys_table_arg, "Installing the log into the > configuration table\n"); > >> and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" > > Thanks. Well, it looks like the memory that is supposedly allocated is not > usable. I'm thinking this is a firmware bug. > Ard, would you agree on this assumption? Thoughts on how to proceed? > I am rather puzzled why the allocate_pool() should succeed and the subsequent memset() should fail. This does not look like an issue that is intimately related to TPM2 support, rather an issue in the firmware that happens to get tickled after the change. Would you mind trying replacing EFI_LOADER_DATA with EFI_BOOT_SERVICES_DATA in the allocate_pool() call? -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Dear Jarkko, On 03/12/18 11:17, Jarkko Sakkinen wrote: On Sat, 2018-03-10 at 10:45 +, Thiebaud Weksteen wrote: On Fri, Mar 9, 2018 at 5:54 PM Jeremy Clinewrote: and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" Thanks. Well, it looks like the memory that is supposedly allocated is not usable. I'm thinking this is a firmware bug. Ard, would you agree on this assumption? Thoughts on how to proceed? Check if the BIOS is up to date? How would that help? The no regression policy demands, that Linux continues working on systems, where it worked before. So upgrading the firmware is no solution, is it? Until a solution is found, the commits should be reverted. Kind regards, Paul smime.p7s Description: S/MIME Cryptographic Signature
Re: Regression from efi: call get_event_log before ExitBootServices
On Sat, 2018-03-10 at 10:45 +, Thiebaud Weksteen wrote: > On Fri, Mar 9, 2018 at 5:54 PM Jeremy Clinewrote: > > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" > > Thanks. Well, it looks like the memory that is supposedly allocated is not > usable. I'm thinking this is a firmware bug. > Ard, would you agree on this assumption? Thoughts on how to proceed? Check if the BIOS is up to date? /Jarkko -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Fri, Mar 9, 2018 at 5:54 PM Jeremy Clinewrote: > On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: > > Thanks a lot for trying out the patch! > > > > Please don't modify your install at this stage, I think we are hitting a > > firmware bug and that would be awesome if we can fix how we are handling it. > > So, if we reach that stage in the function it could either be that: > > * The allocation did not succeed, somehow, but the firmware still returned > > EFI_SUCCEED. > > * The size requested is incorrect (I'm thinking something like a 1G of > > log). This would be due to either a miscalculation of log_size (possible) > > or; the returned values of GetEventLog are not correct. > > I'm sending a patch to add checks for these. Could you please apply and > > retest? > > Again, thanks for helping debugging this. > No problem, thanks for the help :) > With the new patch: > Locating the TCG2Protocol > Calling GetEventLog on TCG2Protocol > Log returned > log_location is not empty > log_size != 0 > log_size < 1M > Allocating memory for storing the logs > Returned from memory allocation > Copying log to new location > And then it hangs. I added a couple more print statements: > diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c > index ee3fac109078..1ab5638bc50e 100644 > --- a/drivers/firmware/efi/libstub/tpm.c > +++ b/drivers/firmware/efi/libstub/tpm.c > @@ -148,8 +148,11 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) > efi_printk(sys_table_arg, "Copying log to new location\n"); > memset(log_tbl, 0, sizeof(*log_tbl) + log_size); > + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); > log_tbl->size = log_size; > + efi_printk(sys_table_arg, "Set log_tbl->size\n"); > log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; > + efi_printk(sys_table_arg, "Set log_tbl-version\n"); > memcpy(log_tbl->log, (void *) first_entry_addr, log_size); > efi_printk(sys_table_arg, "Installing the log into the configuration table\n"); > and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" Thanks. Well, it looks like the memory that is supposedly allocated is not usable. I'm thinking this is a firmware bug. Ard, would you agree on this assumption? Thoughts on how to proceed? Thanks > Regards, > Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Fri, 2018-03-09 at 10:29 +0100, Hans de Goede wrote: > Hi, > > On 08-03-18 18:26, Jeremy Cline wrote: > > > > On 03/08/2018 11:50 AM, Hans de Goede wrote: [...] > > > > The UEFI firmware does some measurements and so does shim. So > > > > you should have some event logs. What version of shim are you > > > > using? And also would be good to know if it's the same shim > > > > version that Jeremy is using. > > > > > > That is a very good question, I'm using: shim-ia32-13-0.7.x86_64, > > > which is the last version for F27 AFAICT. > > > > All my tablet has installed is shim-0.8-10.x86_64, no shim-ia32. > > Yes my bad, although if the kernel changes break booting on systems > without the shim that is still good to know and something which > we probably ought to fix. My laptop is set up with secure boot but without shim using a shim protocol thin layer to check the kernel signature against db variables: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/efitools.git/tree/ShimReplace.c and I haven't seen any breakage, so not having a shim that does measurements works for me all the way up to -rc4. James -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Fri, Mar 09, 2018 at 10:43:50AM +, Thiebaud Weksteen wrote: > Thanks a lot for trying out the patch! > > Please don't modify your install at this stage, I think we are hitting a > firmware bug and that would be awesome if we can fix how we are handling it. > So, if we reach that stage in the function it could either be that: > * The allocation did not succeed, somehow, but the firmware still returned > EFI_SUCCEED. > * The size requested is incorrect (I'm thinking something like a 1G of > log). This would be due to either a miscalculation of log_size (possible) > or; the returned values of GetEventLog are not correct. > I'm sending a patch to add checks for these. Could you please apply and > retest? > Again, thanks for helping debugging this. No problem, thanks for the help :) With the new patch: Locating the TCG2Protocol Calling GetEventLog on TCG2Protocol Log returned log_location is not empty log_size != 0 log_size < 1M Allocating memory for storing the logs Returned from memory allocation Copying log to new location And then it hangs. I added a couple more print statements: diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index ee3fac109078..1ab5638bc50e 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -148,8 +148,11 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) efi_printk(sys_table_arg, "Copying log to new location\n"); memset(log_tbl, 0, sizeof(*log_tbl) + log_size); + efi_printk(sys_table_arg, "Successfully memset log_tbl to 0\n"); log_tbl->size = log_size; + efi_printk(sys_table_arg, "Set log_tbl->size\n"); log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + efi_printk(sys_table_arg, "Set log_tbl-version\n"); memcpy(log_tbl->log, (void *) first_entry_addr, log_size); efi_printk(sys_table_arg, "Installing the log into the configuration table\n"); and it's hanging at "memset(log_tbl, 0, sizeof(*log_tbl) + log_size);" Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
--- drivers/firmware/efi/libstub/tpm.c | 21 + 1 file changed, 21 insertions(+) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index 773afcd6a37c..ee3fac109078 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -112,6 +112,22 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) log_size = log_last_entry - log_location + last_entry_size; } + if (log_size == 0) { + efi_printk(sys_table_arg, "log_size == 0\n"); + } + else if (log_size < 1 * 1024 * 1024) { + efi_printk(sys_table_arg, "log_size < 1M\n"); + } + else if (log_size < 500 * 1024 * 1024) { + efi_printk(sys_table_arg, "log_size < 500M\n"); + } + else if (log_size < 1000 * 1024 * 1024) { + efi_printk(sys_table_arg, "log_size < 1G\n"); + } + else { + efi_printk(sys_table_arg, "log_size > 1G\n"); + } + efi_printk(sys_table_arg, "Allocating memory for storing the logs\n"); /* Allocate space for the logs and copy them. */ status = efi_call_early(allocate_pool, EFI_LOADER_DATA, @@ -124,6 +140,11 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) "Unable to allocate memory for event log\n"); return; } + if (!log_tbl) { + efi_printk(sys_table_arg, "Pointer returned from allocation is NULL!\n"); + return; + } + efi_printk(sys_table_arg, "Copying log to new location\n"); memset(log_tbl, 0, sizeof(*log_tbl) + log_size); -- 2.16.2.395.g2e18187dfd-goog -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Fri, Mar 9, 2018 at 10:29 AM Hans de Goedewrote: > Hi, > On 08-03-18 18:26, Jeremy Cline wrote: > > On 03/08/2018 11:50 AM, Hans de Goede wrote: > >> >> added these now> > >> > >> Hi, > >> > >> On 07-03-18 12:34, Javier Martinez Canillas wrote: > > > > > > > >>> Are you also able to read the TPM event logs? > >>> > >>> $ hexdump /sys/kernel/security/tpm0/binary_bios_measurements > >> > >> Yes for me that outputs a lot of hex :) > > > > For me, /sys/kernel/security/tmp0 doesn't exist on 4.15.6 or 4.16 with > > the patch reverted. > Hmm, have you re-enabled the TPM in the BIOS? > >>> The UEFI firmware does some measurements and so does shim. So you should > >>> have some event logs. What version of shim are you using? And also would > >>> be good to know if it's the same shim version that Jeremy is using. > >> > >> That is a very good question, I'm using: shim-ia32-13-0.7.x86_64, which is > >> the last version for F27 AFAICT. > > > > All my tablet has installed is shim-0.8-10.x86_64, no shim-ia32. > Yes my bad, although if the kernel changes break booting on systems > without the shim that is still good to know and something which > we probably ought to fix. > >> But Jeremy's tablet might very well be not using the shim at all, as > >> I manually installed Fedora 25 on the tablet he now has, before Fedora > >> supported > >> machines with 32 bit EFI. I then later did a "dnf distro-sync" to > >> Fedora-27. > >> > >> Jeremy might also very well still be booting using a grub binary I build > >> manually back then, without any shim being involved. > >> > >> Jeremy what does efibootmgr -v output on your device ? > > > > # efibootmgr -v > > BootCurrent: 0003 > > Timeout: 4 seconds > > BootOrder: 0003,,0001,2001,2002,2003 > > Boot* Android X64 OS > > HD(1,GPT,215e6cf3-e97d-4735-9c4e-7338c8f5a645,0x800,0x32000)/File(\EFI\BOOT\bootx64.efi)RC > > Boot0001* Internal EFI Shell > > FvVol(a881d567-6cb0-4eee-8435-2e72d33e45b5)/FvFile(c57ad6b7-0515-40a8-9d21-551652854e37)RCM&". > > Boot0003* Fedora > > HD(1,GPT,215e6cf3-e97d-4735-9c4e-7338c8f5a645,0x800,0x32000)/File(\EFI\fedora\grubx64.efi) > > Boot2001* EFI USB Device RC > > Boot2002* EFI DVD/CDROM RC > > Boot2003* EFI Network RC > > Boot8087* Udm > > FvVol(a881d567-6cb0-4eee-8435-2e72d33e45b5)/FvFile(9a9ab4c1-ee1b-488b-b300-24544a7bd418) > > > > I think you're right about it using the old grub binary. I'm > > embarrassingly unfamiliar with both UEFI and grub, but I'm guessing you > > set the location of grub.cfg at compile time? When I boot > > \EFI\fedora\grubx64.efi, it's pulling the grub.cfg from > > \EFI\redhat\grub.cfg. > Ah yes, so I did not build my own grub I took one from RHEL as that had > 32 bit UEFI support before Fedora got it and as I was lazy I copied the > 32 bit binary over the 64 bit one, so don't let the filename fool you. > What you could do is install grub2-efi-ia32 from the Fedora 27 repos > and then use efibootmgr to add an entry pointing to \EFI\fedora\grubia32.efi > note that one will look at \EFI\fedora\grub.cfg . > Then see if the problem persists. A second step would be to also install > shim-ia32 and point to that... Thanks a lot for trying out the patch! Please don't modify your install at this stage, I think we are hitting a firmware bug and that would be awesome if we can fix how we are handling it. So, if we reach that stage in the function it could either be that: * The allocation did not succeed, somehow, but the firmware still returned EFI_SUCCEED. * The size requested is incorrect (I'm thinking something like a 1G of log). This would be due to either a miscalculation of log_size (possible) or; the returned values of GetEventLog are not correct. I'm sending a patch to add checks for these. Could you please apply and retest? Again, thanks for helping debugging this. > Regards, > Hans -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi, On 08-03-18 18:26, Jeremy Cline wrote: On 03/08/2018 11:50 AM, Hans de Goede wrote: Hi, On 07-03-18 12:34, Javier Martinez Canillas wrote: Are you also able to read the TPM event logs? $ hexdump /sys/kernel/security/tpm0/binary_bios_measurements Yes for me that outputs a lot of hex :) For me, /sys/kernel/security/tmp0 doesn't exist on 4.15.6 or 4.16 with the patch reverted. Hmm, have you re-enabled the TPM in the BIOS? The UEFI firmware does some measurements and so does shim. So you should have some event logs. What version of shim are you using? And also would be good to know if it's the same shim version that Jeremy is using. That is a very good question, I'm using: shim-ia32-13-0.7.x86_64, which is the last version for F27 AFAICT. All my tablet has installed is shim-0.8-10.x86_64, no shim-ia32. Yes my bad, although if the kernel changes break booting on systems without the shim that is still good to know and something which we probably ought to fix. But Jeremy's tablet might very well be not using the shim at all, as I manually installed Fedora 25 on the tablet he now has, before Fedora supported machines with 32 bit EFI. I then later did a "dnf distro-sync" to Fedora-27. Jeremy might also very well still be booting using a grub binary I build manually back then, without any shim being involved. Jeremy what does efibootmgr -v output on your device ? # efibootmgr -v BootCurrent: 0003 Timeout: 4 seconds BootOrder: 0003,,0001,2001,2002,2003 Boot* Android X64 OS HD(1,GPT,215e6cf3-e97d-4735-9c4e-7338c8f5a645,0x800,0x32000)/File(\EFI\BOOT\bootx64.efi)RC Boot0001* Internal EFI Shell FvVol(a881d567-6cb0-4eee-8435-2e72d33e45b5)/FvFile(c57ad6b7-0515-40a8-9d21-551652854e37)RCM&". Boot0003* Fedora HD(1,GPT,215e6cf3-e97d-4735-9c4e-7338c8f5a645,0x800,0x32000)/File(\EFI\fedora\grubx64.efi) Boot2001* EFI USB DeviceRC Boot2002* EFI DVD/CDROM RC Boot2003* EFI Network RC Boot8087* Udm FvVol(a881d567-6cb0-4eee-8435-2e72d33e45b5)/FvFile(9a9ab4c1-ee1b-488b-b300-24544a7bd418) I think you're right about it using the old grub binary. I'm embarrassingly unfamiliar with both UEFI and grub, but I'm guessing you set the location of grub.cfg at compile time? When I boot \EFI\fedora\grubx64.efi, it's pulling the grub.cfg from \EFI\redhat\grub.cfg. Ah yes, so I did not build my own grub I took one from RHEL as that had 32 bit UEFI support before Fedora got it and as I was lazy I copied the 32 bit binary over the 64 bit one, so don't let the filename fool you. What you could do is install grub2-efi-ia32 from the Fedora 27 repos and then use efibootmgr to add an entry pointing to \EFI\fedora\grubia32.efi note that one will look at \EFI\fedora\grub.cfg . Then see if the problem persists. A second step would be to also install shim-ia32 and point to that... Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/08/2018 03:45 AM, Thiebaud Weksteen wrote: > Jeremy, Hans, could you both describe precisely how your boot is > configured? This feature is only triggered when booting the EFI stub of the > kernel so this may be not executed if you are using something else in > between. I put everything I know in the other sub-thread. > Jeremy, would you be able to modify the efi_retrieve_tpm2_eventlog_1_2 > function to add multiple efi_printk(sys_table_arg, "message\n"); to test: > if you get the output on your screen; and isolate which call might be the > cause of the hang? > I can forward a debug patch if that helps. Thanks for the patch, here's the output: Locating the TCG2Protocol Calling GetEventLog on TCG2Protocol Log returned log_location is not empty log_size != 0 Allocating memory for storing the logs Returned from memory allocation Copying log to new location At this point it hangs. Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On 03/08/2018 11:50 AM, Hans de Goede wrote: > added these now> > > Hi, > > On 07-03-18 12:34, Javier Martinez Canillas wrote: >> Are you also able to read the TPM event logs? >> >> $ hexdump /sys/kernel/security/tpm0/binary_bios_measurements > > Yes for me that outputs a lot of hex :) For me, /sys/kernel/security/tmp0 doesn't exist on 4.15.6 or 4.16 with the patch reverted. >> The UEFI firmware does some measurements and so does shim. So you should >> have some event logs. What version of shim are you using? And also would >> be good to know if it's the same shim version that Jeremy is using. > > That is a very good question, I'm using: shim-ia32-13-0.7.x86_64, which is > the last version for F27 AFAICT. All my tablet has installed is shim-0.8-10.x86_64, no shim-ia32. > > But Jeremy's tablet might very well be not using the shim at all, as > I manually installed Fedora 25 on the tablet he now has, before Fedora > supported > machines with 32 bit EFI. I then later did a "dnf distro-sync" to > Fedora-27. > > Jeremy might also very well still be booting using a grub binary I build > manually back then, without any shim being involved. > > Jeremy what does efibootmgr -v output on your device ? # efibootmgr -v BootCurrent: 0003 Timeout: 4 seconds BootOrder: 0003,,0001,2001,2002,2003 Boot* Android X64 OS HD(1,GPT,215e6cf3-e97d-4735-9c4e-7338c8f5a645,0x800,0x32000)/File(\EFI\BOOT\bootx64.efi)RC Boot0001* Internal EFI Shell FvVol(a881d567-6cb0-4eee-8435-2e72d33e45b5)/FvFile(c57ad6b7-0515-40a8-9d21-551652854e37)RCM&". Boot0003* Fedora HD(1,GPT,215e6cf3-e97d-4735-9c4e-7338c8f5a645,0x800,0x32000)/File(\EFI\fedora\grubx64.efi) Boot2001* EFI USB DeviceRC Boot2002* EFI DVD/CDROM RC Boot2003* EFI Network RC Boot8087* Udm FvVol(a881d567-6cb0-4eee-8435-2e72d33e45b5)/FvFile(9a9ab4c1-ee1b-488b-b300-24544a7bd418) I think you're right about it using the old grub binary. I'm embarrassingly unfamiliar with both UEFI and grub, but I'm guessing you set the location of grub.cfg at compile time? When I boot \EFI\fedora\grubx64.efi, it's pulling the grub.cfg from \EFI\redhat\grub.cfg. Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi, On 07-03-18 12:34, Javier Martinez Canillas wrote: On 03/07/2018 12:10 PM, Hans de Goede wrote: Both according to the BIOS and to the /sys/class/tpm/tpm0/device/description file it is a TPM 2.0. I see, so you can choose enabling the TPM 1.2 or TPM 2.0 device? At least that's the case on my X1 Carbon laptop. I've both a hardware TPM 1.2 and a firmware TPM 2.0 that's implemented as an Intel ME application (AFAIU). This device only has the firmware TPM 2.0 implementation. I'm actually amazed that this machine has a TPM at all, a quick internet search shows that it is a software implemented TPM running as part of the TXE firmware. A quick search suggests that it comes with Windows 10? Yes, it comes with Windows 10. For start, can you please check if you can boot a v4.16-rcX kernel with the TPM device enabled? That way we will know that at least that it consistently fails on this machine and is not and isolated issue. I just tried and v4.16-rc3 boots fine for me, repeatedly. That's an interesting data point. I guess Jeremy's model may actually have something in the TPM log I don't think so. The UEFI firmware already does some measurements and also does shim. So you *should* have some logs. while my TPM log is empty... Is there anyway to make sure the TPM log has some info to retreive? Are you also able to read the TPM event logs? $ hexdump /sys/kernel/security/tpm0/binary_bios_measurements Yes for me that outputs a lot of hex :) The UEFI firmware does some measurements and so does shim. So you should have some event logs. What version of shim are you using? And also would be good to know if it's the same shim version that Jeremy is using. That is a very good question, I'm using: shim-ia32-13-0.7.x86_64, which is the last version for F27 AFAICT. But Jeremy's tablet might very well be not using the shim at all, as I manually installed Fedora 25 on the tablet he now has, before Fedora supported machines with 32 bit EFI. I then later did a "dnf distro-sync" to Fedora-27. Jeremy might also very well still be booting using a grub binary I build manually back then, without any shim being involved. Jeremy what does efibootmgr -v output on your device ? Regards, Hans -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
--- drivers/firmware/efi/libstub/tpm.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/firmware/efi/libstub/tpm.c b/drivers/firmware/efi/libstub/tpm.c index da661bf8cb96..773afcd6a37c 100644 --- a/drivers/firmware/efi/libstub/tpm.c +++ b/drivers/firmware/efi/libstub/tpm.c @@ -74,19 +74,23 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) efi_bool_t truncated; void *tcg2_protocol; + efi_printk(sys_table_arg, "Locating the TCG2Protocol\n"); status = efi_call_early(locate_protocol, _guid, NULL, _protocol); if (status != EFI_SUCCESS) return; + efi_printk(sys_table_arg, "Calling GetEventLog on TCG2Protocol\n"); status = efi_call_proto(efi_tcg2_protocol, get_event_log, tcg2_protocol, EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2, _location, _last_entry, ); if (status != EFI_SUCCESS) return; + efi_printk(sys_table_arg, "Log returned\n"); if (!log_location) return; + efi_printk(sys_table_arg, "log_location is not empty\n"); first_entry_addr = (unsigned long) log_location; /* @@ -94,7 +98,9 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) */ if (!log_last_entry) { log_size = 0; + efi_printk(sys_table_arg, "log_size = 0\n"); } else { + efi_printk(sys_table_arg, "log_size != 0\n"); last_entry_addr = (unsigned long) log_last_entry; /* * get_event_log only returns the address of the last entry. @@ -106,26 +112,31 @@ void efi_retrieve_tpm2_eventlog_1_2(efi_system_table_t *sys_table_arg) log_size = log_last_entry - log_location + last_entry_size; } + efi_printk(sys_table_arg, "Allocating memory for storing the logs\n"); /* Allocate space for the logs and copy them. */ status = efi_call_early(allocate_pool, EFI_LOADER_DATA, sizeof(*log_tbl) + log_size, (void **) _tbl); + efi_printk(sys_table_arg, "Returned from memory allocation\n"); if (status != EFI_SUCCESS) { efi_printk(sys_table_arg, "Unable to allocate memory for event log\n"); return; } + efi_printk(sys_table_arg, "Copying log to new location\n"); memset(log_tbl, 0, sizeof(*log_tbl) + log_size); log_tbl->size = log_size; log_tbl->version = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; memcpy(log_tbl->log, (void *) first_entry_addr, log_size); + efi_printk(sys_table_arg, "Installing the log into the configuration table\n"); status = efi_call_early(install_configuration_table, _eventlog_guid, log_tbl); if (status != EFI_SUCCESS) goto err_free; + efi_printk(sys_table_arg, "Done\n"); return; err_free: -- 2.16.2.395.g2e18187dfd-goog -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
On Wed, Mar 7, 2018 at 6:33 PM Jeremy Clinewrote: > On 03/07/2018 03:41 AM, Thiebaud Weksteen wrote: > > Hi, > > > > Thanks for testing and sending this report! This patch relies heavily on > > the functions exposed by the firmware. My first guess would be that some of > > these may not be implemented correctly by the manufacturer. > > > > Could you share more information on this specific device? > > Do you have any link to the manufacturer website (I found [1] but it is > > based on an ARM CPU)? > > Do you have the option to update your firmware? Is a copy of the firmware > > available from the manufacturer? > I couldn't find a copy of the firmware, unfortunately. No worries, thanks for looking that up. > > On your side, I assume no error message got displayed on the screen when > > booting. Would you be able to try to boot in an UEFI shell [2] and execute > > the command "dh -v"? > Yup, no errors on the screen. I've attached the output of dh -v from the > UEFI shell. Great, thanks for that. There is a module that exposes the EfiTcg2Protocol (TrEEDxe). So I'm going to assume this is properly located and then called. Unfortunately, this is so early in the boot that we can only rely on the EFI functions for logging/debugging. Jeremy, Hans, could you both describe precisely how your boot is configured? This feature is only triggered when booting the EFI stub of the kernel so this may be not executed if you are using something else in between. Jeremy, would you be able to modify the efi_retrieve_tpm2_eventlog_1_2 function to add multiple efi_printk(sys_table_arg, "message\n"); to test: if you get the output on your screen; and isolate which call might be the cause of the hang? I can forward a debug patch if that helps. Thanks > Regards, > Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi Hans, On 03/07/2018 12:16 PM, Hans de Goede wrote: > Hi, > > On 07-03-18 09:41, Thiebaud Weksteen wrote: >> Hi, >> >> Thanks for testing and sending this report! This patch relies heavily on >> the functions exposed by the firmware. My first guess would be that some of >> these may not be implemented correctly by the manufacturer. >> >> Could you share more information on this specific device? > > I've the same device as Jeremy, but I just tried a 4.16-rc3 kernel > and I'm not seeing this problem, BIOS settings all default (I loaded > the BIOS defaults to make sure). > >> Do you have any link to the manufacturer website (I found [1] but it is >> based on an ARM CPU)? >> Do you have the option to update your firmware? Is a copy of the firmware >> available from the manufacturer? > > This is a really cheap Windows tablet which was given away for free in > the Netherlands with some home-schooling language courses, or something > similar. > > Both mine and Jeremy tablets come from a website in the Netherlands > where people can buy/sell used goods. > > Most relevant for this discussion I guess is that this device is > based on a Bay Trail Z3735G SoC, on which according to the internets: > https://embedded.communities.intel.com/thread/7868 > > The TPM 2.0 it contains is implemented as part of the TXE firmware. > > Since I cannot reproduce I'm thinking that maybe Jeremy actually has > some log messages in the TPM log, where as mine is empty. Is there a > way to make sure some messages are in there? > The UEFI firmware does some measurements and so does shim. So you should have some event logs. What version of shim are you using? And also would be good to know if it's the same shim version that Jeremy is using. > Regards, > > Hans > Best regards, -- Javier Martinez Canillas Software Engineer - Desktop Hardware Enablement Red Hat -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi, On 07-03-18 09:41, Thiebaud Weksteen wrote: Hi, Thanks for testing and sending this report! This patch relies heavily on the functions exposed by the firmware. My first guess would be that some of these may not be implemented correctly by the manufacturer. Could you share more information on this specific device? I've the same device as Jeremy, but I just tried a 4.16-rc3 kernel and I'm not seeing this problem, BIOS settings all default (I loaded the BIOS defaults to make sure). Do you have any link to the manufacturer website (I found [1] but it is based on an ARM CPU)? Do you have the option to update your firmware? Is a copy of the firmware available from the manufacturer? This is a really cheap Windows tablet which was given away for free in the Netherlands with some home-schooling language courses, or something similar. Both mine and Jeremy tablets come from a website in the Netherlands where people can buy/sell used goods. Most relevant for this discussion I guess is that this device is based on a Bay Trail Z3735G SoC, on which according to the internets: https://embedded.communities.intel.com/thread/7868 The TPM 2.0 it contains is implemented as part of the TXE firmware. Since I cannot reproduce I'm thinking that maybe Jeremy actually has some log messages in the TPM log, where as mine is empty. Is there a way to make sure some messages are in there? Regards, Hans On your side, I assume no error message got displayed on the screen when booting. Would you be able to try to boot in an UEFI shell [2] and execute the command "dh -v"? Thanks, Thiebaud [1] https://www.gp-electronic.nl/product/7inchtablet [2] https://wiki.archlinux.org/index.php/Unified_Extensible_Firmware_Interface#UEFI_Shell On Tue, Mar 6, 2018 at 5:00 PM Jeremy Clinewrote: Hi folks, Commit 33b6d03469b2 ("efi: call get_event_log before ExitBootServices") causes my GP-electronic T701 tablet to hang when booting. Reverting the patch series or hiding the TPM in the BIOS fixes the problem. I've never fiddled with TPMs before so I'm not sure what what debugging information to provide. It's got an Atom Z3735G and the UEFI firmware is InsydeH20 version BYT70A.YNCHENG.WIN.007. Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Regression from efi: call get_event_log before ExitBootServices
Hi, Thanks for testing and sending this report! This patch relies heavily on the functions exposed by the firmware. My first guess would be that some of these may not be implemented correctly by the manufacturer. Could you share more information on this specific device? Do you have any link to the manufacturer website (I found [1] but it is based on an ARM CPU)? Do you have the option to update your firmware? Is a copy of the firmware available from the manufacturer? On your side, I assume no error message got displayed on the screen when booting. Would you be able to try to boot in an UEFI shell [2] and execute the command "dh -v"? Thanks, Thiebaud [1] https://www.gp-electronic.nl/product/7inchtablet [2] https://wiki.archlinux.org/index.php/Unified_Extensible_Firmware_Interface#UEFI_Shell On Tue, Mar 6, 2018 at 5:00 PM Jeremy Clinewrote: > Hi folks, > Commit 33b6d03469b2 ("efi: call get_event_log before ExitBootServices") > causes my GP-electronic T701 tablet to hang when booting. Reverting the > patch series or hiding the TPM in the BIOS fixes the problem. > I've never fiddled with TPMs before so I'm not sure what what debugging > information to provide. It's got an Atom Z3735G and the UEFI firmware is > InsydeH20 version BYT70A.YNCHENG.WIN.007. > Regards, > Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Regression from efi: call get_event_log before ExitBootServices
Hi folks, Commit 33b6d03469b2 ("efi: call get_event_log before ExitBootServices") causes my GP-electronic T701 tablet to hang when booting. Reverting the patch series or hiding the TPM in the BIOS fixes the problem. I've never fiddled with TPMs before so I'm not sure what what debugging information to provide. It's got an Atom Z3735G and the UEFI firmware is InsydeH20 version BYT70A.YNCHENG.WIN.007. Regards, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html