Re: [efi:next 15/15] drivers/firmware/efi/libstub/efi-stub-helper.c:425:3: warning: cast to pointer from integer of different size
On Sat, Jul 14, 2018 at 04:57:36AM +0800, kbuild test robot wrote:
> vim +425 drivers/firmware/efi/libstub/efi-stub-helper.c
>
>415
>416static efi_status_t efi_open_volume(efi_system_table_t
> *sys_table_arg,
>417efi_loaded_image_t *image,
>418efi_file_handle_t **__fh)
>419{
>420efi_file_io_interface_t *io;
>421efi_file_handle_t *fh;
>422efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
>423efi_status_t status;
>424void *handle =
> > 425(void *)efi_table_attr(efi_loaded_image,
> device_handle, image);
^
My apologies Ard, it seems I forgot an (unsigned long) cast here.
Will give this a thorough review tomorrow morning with a fresh pair
of eyeballs.
--
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
[efi:next 15/15] drivers/firmware/efi/libstub/efi-stub-helper.c:425:3: warning: cast to pointer from integer of different size
tree: https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next
head: 44ae6f59573c50381b7ee5042bd9a0b19f6d6979
commit: 44ae6f59573c50381b7ee5042bd9a0b19f6d6979 [15/15] efi: Deduplicate
efi_open_volume()
config: i386-randconfig-x009-201827 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
git checkout 44ae6f59573c50381b7ee5042bd9a0b19f6d6979
# save the attached .config to linux build tree
make ARCH=i386
All warnings (new ones prefixed by >>):
drivers/firmware/efi/libstub/efi-stub-helper.c: In function
'efi_open_volume':
>> drivers/firmware/efi/libstub/efi-stub-helper.c:425:3: warning: cast to
>> pointer from integer of different size [-Wint-to-pointer-cast]
(void *)efi_table_attr(efi_loaded_image, device_handle, image);
^
vim +425 drivers/firmware/efi/libstub/efi-stub-helper.c
415
416 static efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
417 efi_loaded_image_t *image,
418 efi_file_handle_t **__fh)
419 {
420 efi_file_io_interface_t *io;
421 efi_file_handle_t *fh;
422 efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
423 efi_status_t status;
424 void *handle =
> 425 (void *)efi_table_attr(efi_loaded_image, device_handle,
> image);
426
427 status = efi_call_early(handle_protocol, handle,
428 _proto, (void **));
429 if (status != EFI_SUCCESS) {
430 efi_printk(sys_table_arg, "Failed to handle
fs_proto\n");
431 return status;
432 }
433
434 status = efi_call_proto(efi_file_io_interface, open_volume, io,
);
435 if (status != EFI_SUCCESS)
436 efi_printk(sys_table_arg, "Failed to open volume\n");
437 else
438 *__fh = fh;
439
440 return status;
441 }
442
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line
Hi Will, On 07/13/2018 02:56 AM, Will Deacon wrote: > On Fri, Jul 13, 2018 at 08:15:26AM +0200, Ard Biesheuvel wrote: > >> This is not my call to make, but I would be much less averse to this >> being merged if we could agree upfront on an expiration time of, say, >> 2 years (or more?), after which it will be removed (unless anyone >> makes a very good case for why it needs to be retained). This should >> be mentioned in the kernel log as well when the quirk is triggered. > > The problem with that is it all falls apart when somebody who wasn't > involved in the initial discussion crops up after we remove the quirk to > complain that their machine broke. In such a situation, we don't have a > leg to stand on in my opinion. That's the purpose of the announcement in advance. It should be several release cycles before hand to give plenty of time for feedback and consensus. I think it is generally accepted that if a kernel feature has no known users and no dedicated maintainer then that code should be removed. The powerpc celleb platform was in a similar situation. Not so many available, no real dedicated mainliner, etc. Support was removed [1], and I don't know of any problems that came of it. [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=bf4981a00636347ddcef3fc008e4dd979380a851 -Geoff -- 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] efivars: Call guid_parse() against guid_t type of variable
On 13 July 2018 at 15:10, Andy Shevchenko wrote: > uuid_le_to_bin() is deprecated API and take into consideration that variable, > to where we store parsed data, is type of guid_t we switch to guid_parse() > for sake of consistency. > > While here, add error checking to it. > > Signed-off-by: Andy Shevchenko Acked-by: Ard Biesheuvel > --- > fs/efivarfs/inode.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c > index 71ff317e..8c6ab6c95727 100644 > --- a/fs/efivarfs/inode.c > +++ b/fs/efivarfs/inode.c > @@ -86,7 +86,9 @@ static int efivarfs_create(struct inode *dir, struct dentry > *dentry, > /* length of the variable name itself: remove GUID and separator */ > namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1; > > - uuid_le_to_bin(dentry->d_name.name + namelen + 1, > >var.VendorGuid); > + err = guid_parse(dentry->d_name.name + namelen + 1, > >var.VendorGuid); > + if (err) > + goto out; > > if (efivar_variable_is_removable(var->var.VendorGuid, > dentry->d_name.name, namelen)) > -- > 2.18.0 > -- 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: EFI mixed mode fix for v4.18
Please don't remove cc's - I added them for a reason. On 13 July 2018 at 15:54, youling 257 wrote: > I tried Revert "mm/page_poison.c: make early_page_poison_param() > __init"Revert "mm/kmemleak.c: make kmemleak_boot_config() > __init" Revert "mm/page_owner.c: make early_page_owner_param() > __init" Revert "random: fix crng_ready() test" Revert "x86/efi: Use > efi_switch_mm() rather than manually > twiddling with %cr3" in three days > > not solved my problem ! i only can guess which patch cause my problem，i > still not solved my problem. > OK so if reverting that last patch did not fix your issue, why did you bring it up in the first place? Please, try to use git bisect methodically to narrow down the issue. If you do that, we will be able to help. If you don't, I am afraid there is nothing we can do. > 2018-07-13 21:50 GMT+08:00 Ard Biesheuvel : >> >> (+ Sai) >> >> On 13 July 2018 at 13:56, youling 257 wrote: >> > >> > https://github.com/torvalds/linux/commit/03781e40890c18bdea40092355b61431d0073c1d >> > x86_64 kernel and"efi=old_map" disabled because, x86_32 and efi=old_map >> > do >> > not use >> > efi_pgd, rather they use swapper_pg_dir. >> > >> > so what x86_64 kernel and efi=old_map ? >> > >> > https://forums.kali.org/archive/index.php/t-29927.html >> > Seems like the kali's 4.4 kernel doesnt support some (old)hardware >> > maybe? I >> > have an old 32bit toshiba satelite that always hang on reboot when using >> > that kernel version. >> > It goes to bios screen and hangs for like 1-2 min and then a blank >> > screen >> > with a _ blinking. I could then shutoff my computer using the power >> > button. >> > Shutting down is fine though. >> > >> > >> > Been having the same issue where I kept rebooting itself during loading >> > of >> > the kernel and would be a never ending loop of reboots. >> > >> > But I have managed to fix it by editing the grub conf file at >> > /etc/default/grub. Using your fave word editor and on the line >> > >> > GRUB_CMDLINE_LINUX_DEFAULT="quiet " add efi=old_map >> > >> > so the line should look like this roughly >> > >> > GRUB_CMDLINE_LINUX_DEFAULT="quiet efi=old_map" >> > >> >> OK, to let me see if I can summarize this correctly, because I have >> difficulty understanding what you are describing here. >> >> You have a Bay trail system that boots in mixed mode. >> You don't have a toshiba satellite, that is just an example you took >> from the internet. >> >> Your Bay Trail system sometimes fails to boot v4.17 but older kernels are >> fine. >> >> So does reverting the patch 03781e40890c18bdea40092355b61431d0073c1d >> you mention fix the issue? >> >> >> > 2018-07-13 18:12 GMT+08:00 youling 257 : >> >> >> >> I begin to used from 4.17 rc3，the problem make a headache for me two >> >> months. >> >> i can't bisect to find the reason ，problem begin with 4.17，the 4.16 >> >> work >> >> well，4.16.18 also work well. >> >> >> >> 2018-07-13 13:38 GMT+08:00 Ard Biesheuvel : >> >>> >> >>> On 13 July 2018 at 03:18, youling 257 wrote: >> >>> > Not thorough fix Bay trail 64 bit kernel on 32 bit uefi. >> >>> > sometimes，can't >> >>> > boot . >> >>> > >> >>> > I tested efi/x86: remove pointless call to PciIo->Attributes() >> >>> > efi/x86: prevent reentrant firmware calls in mixed modeefi/x86: >> >>> > merge >> >>> > setup_efi_pci32 and setup_efi_pci64 routines >> >>> > efi/x86: align efi_uga_draw_protocol typedef names to convention >> >>> > efi/x86: merge 32-bit and 64-bit UGA draw protocol setup routines >> >>> > efi/x86: add missing NULL initialization in UGA draw protocol >> >>> > discovery >> >>> > efi/x86: replace references to efi_early->is64 with efi_is_64bit() >> >>> > >> >>> > please you test me this problem reason，which one 4.17 rc patch cause >> >>> > it， >> >>> > from 4.17 rc3 to 4.18 rc4，sometimes can boot kernel，sometimes can't >> >>> > boot， >> >>> >> >>> So v4.17-rc2 works but v4.17-rc3 doesn't? Can you bisect this window? >> >> >> >> >> > > > -- 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 0/6] efi/x86 mixed mode cleanups
On 13 July 2018 at 11:57, Lukas Wunner wrote:
> On Thu, Jul 12, 2018 at 02:21:48PM +0200, Ard Biesheuvel wrote:
>> Patch #2 merges the remaining 32/64-bit specific parts of the setup_efi_pci
>> routine.
>>
>> Patches #3 and #4 do the same for the UGA draw protocol discovery routines.
>>
>> Patch #6 helps unused code paths to be optimized away on configurations
>> that don't need them (32-bit only and 64-bit only)
>
> Thanks a lot for doing this Ard, I meant to deduplicate that code
> further but didn't get around to it. FWIW I have a single unsubmitted
> deduplication patch which has been rotting on my development branch for
> more than a year now, I'm including it below in case it helps, needs a
> careful review though.
>
Thanks Lukas. I will apply that (with Ingo's comment addressed).
Did you have any thoughts about how we could at least detect
situations where we are invoking protocols that are not mixed-mode
safe (i.e., they pass 64-bit quantities by value, which our current
thunking routine does not take into account)
> -- >8 --
> From 7fa51faec20c74eea2bb3ba96ecab1bc3e18e5a8 Mon Sep 17 00:00:00 2001
> From: Lukas Wunner
> Date: Sun, 7 May 2017 14:42:51 +0200
> Subject: [PATCH] efi: Deduplicate efi_open_volume()
>
> There's one ARM, one x86_32 and one x86_64 version which can be folded
> into a single shared version by masking their differences with the
> efi_call_proto() macro introduced by commit 3552fdf29f01 ("efi: Allow
> bitness-agnostic protocol calls").
>
> To be able to dereference the device_handle attribute from the
> efi_loaded_image_t table in an arch- and bitness-agnostic manner,
> introduce the efi_table_attr() macro (which already exists for x86)
> to arm and arm64.
>
> No functional change intended.
>
> Signed-off-by: Lukas Wunner
> ---
> arch/arm/include/asm/efi.h | 3 ++
> arch/arm64/include/asm/efi.h | 3 ++
> arch/x86/boot/compressed/eboot.c | 61
> --
> drivers/firmware/efi/libstub/arm-stub.c| 25 ---
> drivers/firmware/efi/libstub/efi-stub-helper.c | 29 +++-
> drivers/firmware/efi/libstub/efistub.h | 3 --
> include/linux/efi.h| 10 +
> 7 files changed, 43 insertions(+), 91 deletions(-)
>
> diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
> index 17f1f1a..38badaa 100644
> --- a/arch/arm/include/asm/efi.h
> +++ b/arch/arm/include/asm/efi.h
> @@ -58,6 +58,9 @@ static inline void efi_set_pgd(struct mm_struct *mm)
> #define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
> #define efi_is_64bit() (false)
>
> +#define efi_table_attr(table, attr, instance) \
> + ((table##_t *)instance)->attr
> +
> #define efi_call_proto(protocol, f, instance, ...) \
> ((protocol##_t *)instance)->f(instance, ##__VA_ARGS__)
>
> diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
> index c4cd508..5a21037 100644
> --- a/arch/arm64/include/asm/efi.h
> +++ b/arch/arm64/include/asm/efi.h
> @@ -85,6 +85,9 @@ static inline unsigned long
> efi_get_max_initrd_addr(unsigned long dram_base,
> #define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
> #define efi_is_64bit() (true)
>
> +#define efi_table_attr(table, attr, instance) \
> + ((table##_t *)instance)->attr
> +
> #define efi_call_proto(protocol, f, instance, ...) \
> ((protocol##_t *)instance)->f(instance, ##__VA_ARGS__)
>
> diff --git a/arch/x86/boot/compressed/eboot.c
> b/arch/x86/boot/compressed/eboot.c
> index 9a5b6d3..2d2b599 100644
> --- a/arch/x86/boot/compressed/eboot.c
> +++ b/arch/x86/boot/compressed/eboot.c
> @@ -41,67 +41,6 @@ __pure const struct efi_config *__efi_early(void)
> BOOT_SERVICES(32);
> BOOT_SERVICES(64);
>
> -static inline efi_status_t __open_volume32(void *__image, void **__fh)
> -{
> - efi_file_io_interface_t *io;
> - efi_loaded_image_32_t *image = __image;
> - efi_file_handle_32_t *fh;
> - efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
> - efi_status_t status;
> - void *handle = (void *)(unsigned long)image->device_handle;
> - unsigned long func;
> -
> - status = efi_call_early(handle_protocol, handle,
> - _proto, (void **));
> - if (status != EFI_SUCCESS) {
> - efi_printk(sys_table, "Failed to handle fs_proto\n");
> - return status;
> - }
> -
> - func = (unsigned long)io->open_volume;
> - status = efi_early->call(func, io, );
> - if (status != EFI_SUCCESS)
> - efi_printk(sys_table, "Failed to open volume\n");
> -
> - *__fh = fh;
> - return status;
> -}
> -
> -static inline efi_status_t __open_volume64(void *__image, void **__fh)
> -{
> - efi_file_io_interface_t *io;
> -
Re: EFI mixed mode fix for v4.18
(+ Sai) On 13 July 2018 at 13:56, youling 257 wrote: > https://github.com/torvalds/linux/commit/03781e40890c18bdea40092355b61431d0073c1d > x86_64 kernel and"efi=old_map" disabled because, x86_32 and efi=old_map do > not use > efi_pgd, rather they use swapper_pg_dir. > > so what x86_64 kernel and efi=old_map ? > > https://forums.kali.org/archive/index.php/t-29927.html > Seems like the kali's 4.4 kernel doesnt support some (old)hardware maybe? I > have an old 32bit toshiba satelite that always hang on reboot when using > that kernel version. > It goes to bios screen and hangs for like 1-2 min and then a blank screen > with a _ blinking. I could then shutoff my computer using the power button. > Shutting down is fine though. > > > Been having the same issue where I kept rebooting itself during loading of > the kernel and would be a never ending loop of reboots. > > But I have managed to fix it by editing the grub conf file at > /etc/default/grub. Using your fave word editor and on the line > > GRUB_CMDLINE_LINUX_DEFAULT="quiet " add efi=old_map > > so the line should look like this roughly > > GRUB_CMDLINE_LINUX_DEFAULT="quiet efi=old_map" > OK, to let me see if I can summarize this correctly, because I have difficulty understanding what you are describing here. You have a Bay trail system that boots in mixed mode. You don't have a toshiba satellite, that is just an example you took from the internet. Your Bay Trail system sometimes fails to boot v4.17 but older kernels are fine. So does reverting the patch 03781e40890c18bdea40092355b61431d0073c1d you mention fix the issue? > 2018-07-13 18:12 GMT+08:00 youling 257 : >> >> I begin to used from 4.17 rc3，the problem make a headache for me two >> months. >> i can't bisect to find the reason ，problem begin with 4.17，the 4.16 work >> well，4.16.18 also work well. >> >> 2018-07-13 13:38 GMT+08:00 Ard Biesheuvel : >>> >>> On 13 July 2018 at 03:18, youling 257 wrote: >>> > Not thorough fix Bay trail 64 bit kernel on 32 bit uefi. >>> > sometimes，can't >>> > boot . >>> > >>> > I tested efi/x86: remove pointless call to PciIo->Attributes() >>> > efi/x86: prevent reentrant firmware calls in mixed modeefi/x86: merge >>> > setup_efi_pci32 and setup_efi_pci64 routines >>> > efi/x86: align efi_uga_draw_protocol typedef names to convention >>> > efi/x86: merge 32-bit and 64-bit UGA draw protocol setup routines >>> > efi/x86: add missing NULL initialization in UGA draw protocol discovery >>> > efi/x86: replace references to efi_early->is64 with efi_is_64bit() >>> > >>> > please you test me this problem reason，which one 4.17 rc patch cause >>> > it， >>> > from 4.17 rc3 to 4.18 rc4，sometimes can boot kernel，sometimes can't >>> > boot， >>> >>> So v4.17-rc2 works but v4.17-rc3 doesn't? Can you bisect this window? >> >> > -- 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 0/6] efi/x86 mixed mode cleanups
On 13 July 2018 at 15:29, Ingo Molnar wrote:
>
> * Lukas Wunner wrote:
>
>> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
>> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
>> @@ -413,6 +413,32 @@ static efi_status_t efi_file_close(void *handle)
>> return efi_call_proto(efi_file_handle, close, handle);
>> }
>>
>> +static efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
>> + efi_loaded_image_t *image,
>> + efi_file_handle_t **__fh)
>> +{
>> + efi_file_io_interface_t *io;
>> + efi_file_handle_t *fh;
>> + efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
>> + efi_status_t status;
>> + void *handle =
>> + (void *)efi_table_attr(efi_loaded_image, device_handle, image);
>> +
>> + status = efi_call_early(handle_protocol, handle,
>> + _proto, (void **));
>> + if (status != EFI_SUCCESS) {
>> + efi_printk(sys_table_arg, "Failed to handle fs_proto\n");
>> + return status;
>> + }
>> +
>> + status = efi_call_proto(efi_file_io_interface, open_volume, io, );
>> + if (status != EFI_SUCCESS)
>> + efi_printk(sys_table_arg, "Failed to open volume\n");
>> +
>> + *__fh = fh;
>> + return status;
>
> BTW., in the second failure branch is 'fh' guaranteed to be set by the EFI
> call?
> If not then we set *__fh to a potentially undefined value, from the kernels
> stack?
>
> (I realize that your refactoring just inherited this existing pattern, but it
> caught my attention.)
>
No, only EFI_SUCCESS guarantees that fh will have been assigned.
Since we propagate 'status' here, it does not seem to matter in
practice, but it would indeed be better not to clobber *__fh with
random junk when returning failure.
--
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 0/6] efi/x86 mixed mode cleanups
* Lukas Wunner wrote:
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -413,6 +413,32 @@ static efi_status_t efi_file_close(void *handle)
> return efi_call_proto(efi_file_handle, close, handle);
> }
>
> +static efi_status_t efi_open_volume(efi_system_table_t *sys_table_arg,
> + efi_loaded_image_t *image,
> + efi_file_handle_t **__fh)
> +{
> + efi_file_io_interface_t *io;
> + efi_file_handle_t *fh;
> + efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
> + efi_status_t status;
> + void *handle =
> + (void *)efi_table_attr(efi_loaded_image, device_handle, image);
> +
> + status = efi_call_early(handle_protocol, handle,
> + _proto, (void **));
> + if (status != EFI_SUCCESS) {
> + efi_printk(sys_table_arg, "Failed to handle fs_proto\n");
> + return status;
> + }
> +
> + status = efi_call_proto(efi_file_io_interface, open_volume, io, );
> + if (status != EFI_SUCCESS)
> + efi_printk(sys_table_arg, "Failed to open volume\n");
> +
> + *__fh = fh;
> + return status;
BTW., in the second failure branch is 'fh' guaranteed to be set by the EFI
call?
If not then we set *__fh to a potentially undefined value, from the kernels
stack?
(I realize that your refactoring just inherited this existing pattern, but it
caught my attention.)
Thanks,
Ingo
--
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 v1] efivars: Call guid_parse() against guid_t type of variable
uuid_le_to_bin() is deprecated API and take into consideration that variable, to where we store parsed data, is type of guid_t we switch to guid_parse() for sake of consistency. While here, add error checking to it. Signed-off-by: Andy Shevchenko --- fs/efivarfs/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c index 71ff317e..8c6ab6c95727 100644 --- a/fs/efivarfs/inode.c +++ b/fs/efivarfs/inode.c @@ -86,7 +86,9 @@ static int efivarfs_create(struct inode *dir, struct dentry *dentry, /* length of the variable name itself: remove GUID and separator */ namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1; - uuid_le_to_bin(dentry->d_name.name + namelen + 1, >var.VendorGuid); + err = guid_parse(dentry->d_name.name + namelen + 1, >var.VendorGuid); + if (err) + goto out; if (efivar_variable_is_removable(var->var.VendorGuid, dentry->d_name.name, namelen)) -- 2.18.0 -- 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: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line
On Fri, 2018-07-13 at 08:15 +0200, Ard Biesheuvel wrote: > I still think we should not be using DMI, though. Would a quirk be acceptable if keyed on something other than DMI? Are there some other options, perhaps some property of the ACPI tables proper? Something which can be queried from UEFI perhaps? (I'm really grasping at straws, but maybe someone has a smart idea). Ian. -- 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 0/6] efi/x86 mixed mode cleanups
On Thu, Jul 12, 2018 at 02:21:48PM +0200, Ard Biesheuvel wrote:
> Patch #2 merges the remaining 32/64-bit specific parts of the setup_efi_pci
> routine.
>
> Patches #3 and #4 do the same for the UGA draw protocol discovery routines.
>
> Patch #6 helps unused code paths to be optimized away on configurations
> that don't need them (32-bit only and 64-bit only)
Thanks a lot for doing this Ard, I meant to deduplicate that code
further but didn't get around to it. FWIW I have a single unsubmitted
deduplication patch which has been rotting on my development branch for
more than a year now, I'm including it below in case it helps, needs a
careful review though.
-- >8 --
>From 7fa51faec20c74eea2bb3ba96ecab1bc3e18e5a8 Mon Sep 17 00:00:00 2001
From: Lukas Wunner
Date: Sun, 7 May 2017 14:42:51 +0200
Subject: [PATCH] efi: Deduplicate efi_open_volume()
There's one ARM, one x86_32 and one x86_64 version which can be folded
into a single shared version by masking their differences with the
efi_call_proto() macro introduced by commit 3552fdf29f01 ("efi: Allow
bitness-agnostic protocol calls").
To be able to dereference the device_handle attribute from the
efi_loaded_image_t table in an arch- and bitness-agnostic manner,
introduce the efi_table_attr() macro (which already exists for x86)
to arm and arm64.
No functional change intended.
Signed-off-by: Lukas Wunner
---
arch/arm/include/asm/efi.h | 3 ++
arch/arm64/include/asm/efi.h | 3 ++
arch/x86/boot/compressed/eboot.c | 61 --
drivers/firmware/efi/libstub/arm-stub.c| 25 ---
drivers/firmware/efi/libstub/efi-stub-helper.c | 29 +++-
drivers/firmware/efi/libstub/efistub.h | 3 --
include/linux/efi.h| 10 +
7 files changed, 43 insertions(+), 91 deletions(-)
diff --git a/arch/arm/include/asm/efi.h b/arch/arm/include/asm/efi.h
index 17f1f1a..38badaa 100644
--- a/arch/arm/include/asm/efi.h
+++ b/arch/arm/include/asm/efi.h
@@ -58,6 +58,9 @@ static inline void efi_set_pgd(struct mm_struct *mm)
#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
#define efi_is_64bit() (false)
+#define efi_table_attr(table, attr, instance) \
+ ((table##_t *)instance)->attr
+
#define efi_call_proto(protocol, f, instance, ...) \
((protocol##_t *)instance)->f(instance, ##__VA_ARGS__)
diff --git a/arch/arm64/include/asm/efi.h b/arch/arm64/include/asm/efi.h
index c4cd508..5a21037 100644
--- a/arch/arm64/include/asm/efi.h
+++ b/arch/arm64/include/asm/efi.h
@@ -85,6 +85,9 @@ static inline unsigned long efi_get_max_initrd_addr(unsigned
long dram_base,
#define efi_call_runtime(f, ...) sys_table_arg->runtime->f(__VA_ARGS__)
#define efi_is_64bit() (true)
+#define efi_table_attr(table, attr, instance) \
+ ((table##_t *)instance)->attr
+
#define efi_call_proto(protocol, f, instance, ...) \
((protocol##_t *)instance)->f(instance, ##__VA_ARGS__)
diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 9a5b6d3..2d2b599 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -41,67 +41,6 @@ __pure const struct efi_config *__efi_early(void)
BOOT_SERVICES(32);
BOOT_SERVICES(64);
-static inline efi_status_t __open_volume32(void *__image, void **__fh)
-{
- efi_file_io_interface_t *io;
- efi_loaded_image_32_t *image = __image;
- efi_file_handle_32_t *fh;
- efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
- efi_status_t status;
- void *handle = (void *)(unsigned long)image->device_handle;
- unsigned long func;
-
- status = efi_call_early(handle_protocol, handle,
- _proto, (void **));
- if (status != EFI_SUCCESS) {
- efi_printk(sys_table, "Failed to handle fs_proto\n");
- return status;
- }
-
- func = (unsigned long)io->open_volume;
- status = efi_early->call(func, io, );
- if (status != EFI_SUCCESS)
- efi_printk(sys_table, "Failed to open volume\n");
-
- *__fh = fh;
- return status;
-}
-
-static inline efi_status_t __open_volume64(void *__image, void **__fh)
-{
- efi_file_io_interface_t *io;
- efi_loaded_image_64_t *image = __image;
- efi_file_handle_64_t *fh;
- efi_guid_t fs_proto = EFI_FILE_SYSTEM_GUID;
- efi_status_t status;
- void *handle = (void *)(unsigned long)image->device_handle;
- unsigned long func;
-
- status = efi_call_early(handle_protocol, handle,
- _proto, (void **));
- if (status != EFI_SUCCESS) {
- efi_printk(sys_table, "Failed to handle fs_proto\n");
- return status;
- }
-
- func = (unsigned long)io->open_volume;
Re: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line
On Fri, Jul 13, 2018 at 08:15:26AM +0200, Ard Biesheuvel wrote: > On 13 July 2018 at 00:22, Geoff Levand wrote: > > Hi Ard, > > > > On 07/04/2018 08:49 AM, Ard Biesheuvel wrote: > >> efi/libstub: taken contents of LinuxExtraArgs UEFI variable into > >> account > > > > To me this seems an overly complicated interface to the kernel, and > > still doesn't in itself solve the problem at hand -- make the > > generic distro kernel with APEI support run on m400 systems. > > > > As for me, I'd prefer something like my original patch. It fixes > > that problem, it is simple and self contained, and is very clear > > in what it does. Also, there is a limited life. When the time > > comes an announcement is made to the mailing lists 'Linux-XYZ will > > no longer support the m400 Moonshot'. Then, when the Linux-XYZ-rc1 > > merge window opens a patch goes in that removes the > > acpi_fixup_m400_quirks() routine. > > > > Actually, that is a very good point. One of the issues I have with > these quirks is that the burden is on the maintainers to keep them > around forever, unless they can prove that it is no longer needed. > > This is not my call to make, but I would be much less averse to this > being merged if we could agree upfront on an expiration time of, say, > 2 years (or more?), after which it will be removed (unless anyone > makes a very good case for why it needs to be retained). This should > be mentioned in the kernel log as well when the quirk is triggered. The problem with that is it all falls apart when somebody who wasn't involved in the initial discussion crops up after we remove the quirk to complain that their machine broke. In such a situation, we don't have a leg to stand on in my opinion. Will -- 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: [RFC PATCH 0/2] efi: add contents of LinuxExtraArgs EFI var to command line
On 13 July 2018 at 00:22, Geoff Levand wrote: > Hi Ard, > > On 07/04/2018 08:49 AM, Ard Biesheuvel wrote: >> efi/libstub: taken contents of LinuxExtraArgs UEFI variable into >> account > > To me this seems an overly complicated interface to the kernel, and > still doesn't in itself solve the problem at hand -- make the > generic distro kernel with APEI support run on m400 systems. > > As for me, I'd prefer something like my original patch. It fixes > that problem, it is simple and self contained, and is very clear > in what it does. Also, there is a limited life. When the time > comes an announcement is made to the mailing lists 'Linux-XYZ will > no longer support the m400 Moonshot'. Then, when the Linux-XYZ-rc1 > merge window opens a patch goes in that removes the > acpi_fixup_m400_quirks() routine. > Actually, that is a very good point. One of the issues I have with these quirks is that the burden is on the maintainers to keep them around forever, unless they can prove that it is no longer needed. This is not my call to make, but I would be much less averse to this being merged if we could agree upfront on an expiration time of, say, 2 years (or more?), after which it will be removed (unless anyone makes a very good case for why it needs to be retained). This should be mentioned in the kernel log as well when the quirk is triggered. I still think we should not be using DMI, though. -- 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
