Re: [PATCH 0/4] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On 12 January 2018 at 11:24, Daniel Kiper wrote: > Hi Ard, > > On Thu, Jan 11, 2018 at 12:51:07PM +, Ard Biesheuvel wrote: >> On 9 January 2018 at 14:22, Daniel Kiper wrote: >> > Hi, >> > >> > Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel >> > may not even know that it runs on secure boot enabled platform. >> >> Hi Daniel, >> >> I must say, I am not too thrilled with the approach you have chosen >> here. #including .c files in other .c files, and using #defines to >> override C functions or other stub functionality is rather fragile. In > > TBH I do not like it too. Sadly I have not find a better solution for > that. I wish to avoid code duplication as much as possible because > otherwise it will fall out of sync sooner or later (usually sooner). > Similar thing happened in different part of Xen EFI code a few months ago. > >> particular, it means we have to start caring about not breaking >> Xen/x86 code when making modifications to the EFI stub, and that code >> is already difficult enough to maintain, given that it is shared >> between ARM, arm64 and x86, and runs either from the decompressor or >> the kernel proper (arm64) but in the context of the UEFI firmware. > > I understand that. > >> None of the stub code currently runs in ordinary kernel context. > > Yep. > >> So please, could you try to find another way to do this? > > I am happy to improve the situation, however, I am afraid that it is > difficult here. Stub and kernel proper are separate entities and simple > linking does not work. So, It seems to me that only play with includes > will allow us to not duplicate the code. However, if you have a better > idea I am happy to implement it. > Actually, there is another reason why it does not make sense to reuse that code. This code /* * See if a user has put the shim into insecure mode. If so, and if the * variable doesn't have the runtime attribute set, we might as well * honor that. */ size = sizeof(moksbstate); status = get_efi_var(L"MokSBState", &shim_guid, &attr, &size, &moksbstate); /* If it fails, we don't care why. Default to secure */ if (status != EFI_SUCCESS) goto secure_boot_enabled; if (!(attr & EFI_VARIABLE_RUNTIME_ACCESS) && moksbstate == 1) return efi_secureboot_mode_disabled; will always fail after exiting boot services, so it makes no sense to call it from xen_efi_init(). So I suggest you just clone the function and only keep the pieces that make sense for Xen. -- Ard. -- 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: [PATCH] efi/arm*: Only register page tables when they exist
On 17 January 2018 at 12:01, Ard Biesheuvel wrote: > On 17 January 2018 at 10:19, Will Deacon wrote: >> On Tue, Jan 16, 2018 at 02:08:32PM +, Mark Rutland wrote: >>> Currently the arm/arm64 runtime code registers the runtime servies >>> pagetables with ptdump regardless of whether runtime services page >>> tables have been created. >>> >>> As efi_mm.pgd is NULL in these cases, attempting to dump the efi page >>> tables results in a NULL pointer dereference in the ptdump code: >> >> Acked-by: Will Deacon >> >> Ard -- please can you pick this one up? >> > > I'll see if I can manage applying it. > Applied for -next, thanks. -- 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: [PATCH v1] efi/apple-properties: Device core takes care of empty properties
On 22 January 2018 at 21:46, Lukas Wunner wrote: > On Mon, Jan 22, 2018 at 04:17:09PM +0200, Andy Shevchenko wrote: >> There is no need to artificially supply a property length and fake data >> if property has type of boolean. > > You're right, I just didn't know better. > > I've verified that with this patch applied, boolean properties are > still listed with the "dump_apple_properties" command line option > and I've also tested that it's still possible to test presence of > boolean properties with device_property_present(), so FWIW: > > Reviewed-and-tested-by: Lukas Wunner > > Thanks Andy! Ard, could you pick this up please? > Applied, thanks. -- 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: [PATCH] x86/efi: Fix trailing semicolons
On 23 January 2018 at 13:20, Luis de Bethencourt wrote: > The trailing semicolon is an empty statement that does no operation. > Removing them since they don't do anything. > > Signed-off-by: Luis de Bethencourt > --- > > Hi, > > After fixing the same thing in drivers/staging/rtl8723bs/, Joe Perches > suggested I fix it treewide [0]. > > Best regards > Luis > Applied, thanks. > > [0] > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115410.html > [1] > http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115390.html > > arch/x86/boot/compressed/eboot.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/boot/compressed/eboot.c > b/arch/x86/boot/compressed/eboot.c > index 353e20c3f114..886a9115af62 100644 > --- a/arch/x86/boot/compressed/eboot.c > +++ b/arch/x86/boot/compressed/eboot.c > @@ -439,7 +439,7 @@ setup_uga32(void **uga_handle, unsigned long size, u32 > *width, u32 *height) > struct efi_uga_draw_protocol *uga = NULL, *first_uga; > efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID; > unsigned long nr_ugas; > - u32 *handles = (u32 *)uga_handle;; > + u32 *handles = (u32 *)uga_handle; > efi_status_t status = EFI_INVALID_PARAMETER; > int i; > > @@ -484,7 +484,7 @@ setup_uga64(void **uga_handle, unsigned long size, u32 > *width, u32 *height) > struct efi_uga_draw_protocol *uga = NULL, *first_uga; > efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID; > unsigned long nr_ugas; > - u64 *handles = (u64 *)uga_handle;; > + u64 *handles = (u64 *)uga_handle; > efi_status_t status = EFI_INVALID_PARAMETER; > int i; > > -- > 2.15.1 > -- 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
[PATCH] x86/efi: Fix trailing semicolons
The trailing semicolon is an empty statement that does no operation. Removing them since they don't do anything. Signed-off-by: Luis de Bethencourt --- Hi, After fixing the same thing in drivers/staging/rtl8723bs/, Joe Perches suggested I fix it treewide [0]. Best regards Luis [0] http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115410.html [1] http://driverdev.linuxdriverproject.org/pipermail/driverdev-devel/2018-January/115390.html arch/x86/boot/compressed/eboot.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 353e20c3f114..886a9115af62 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -439,7 +439,7 @@ setup_uga32(void **uga_handle, unsigned long size, u32 *width, u32 *height) struct efi_uga_draw_protocol *uga = NULL, *first_uga; efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID; unsigned long nr_ugas; - u32 *handles = (u32 *)uga_handle;; + u32 *handles = (u32 *)uga_handle; efi_status_t status = EFI_INVALID_PARAMETER; int i; @@ -484,7 +484,7 @@ setup_uga64(void **uga_handle, unsigned long size, u32 *width, u32 *height) struct efi_uga_draw_protocol *uga = NULL, *first_uga; efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID; unsigned long nr_ugas; - u64 *handles = (u64 *)uga_handle;; + u64 *handles = (u64 *)uga_handle; efi_status_t status = EFI_INVALID_PARAMETER; int i; -- 2.15.1 -- 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