Re: [ 105/128] efi: Make efi_enabled a function to query EFI facilities
On Tue, Feb 05, 2013 at 03:46:04AM +, Ben Hutchings wrote: > On Mon, 2013-02-04 at 16:44 +, Matt Fleming wrote: > > On Sun, 2013-02-03 at 16:15 +0100, Ben Hutchings wrote: > > > As you can see this needed quite a lot of work to backport, and I > > > haven't been able to test it yet. So I would particularly appreciate > > > careful review of this. > > > > Everything looks fine to me but I haven't actually booted with these > > changes. > > Thanks. I've now tested this on a 64-bit EFI system myself: > > x86_64 kernel build: > - Boots successfully > - x86_efi_facilities = 0x3f > - efifb works > - efivars can be loaded and creates /sys/firmware/efi/vars > > i386 kernel build: > - Boots as far as an initramfs shell, but the machine doesn't have an > i386 installation to continue with > - x86_efi_facilities = 0x21 > - efifb works > - efivars can be loaded but does nothing > > All of which I think is correct. Yeah, if it's running 64-bit EFI, that looks correct. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 1/2] export unpack_to_rootfs
On Tue, Feb 05, 2013 at 02:34:49PM +0200, Dmitry Kasatkin wrote:
> Signed-off-by: Dmitry Kasatkin
> ---
> init/do_mounts.h |2 ++
> init/initramfs.c |2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/init/do_mounts.h b/init/do_mounts.h
> index f5b978a..11829eb 100644
> --- a/init/do_mounts.h
> +++ b/init/do_mounts.h
> @@ -74,3 +74,5 @@ void md_run_setup(void);
> static inline void md_run_setup(void) {}
>
> #endif
> +
> +char * __init unpack_to_rootfs(char *buf, unsigned len);
> diff --git a/init/initramfs.c b/init/initramfs.c
> index 84c6bf1..e32bc06 100644
> --- a/init/initramfs.c
> +++ b/init/initramfs.c
> @@ -421,7 +421,7 @@ static unsigned my_inptr; /* index of next byte to be
> processed in inbuf */
>
> #include
>
> -static char * __init unpack_to_rootfs(char *buf, unsigned len)
> +char * __init unpack_to_rootfs(char *buf, unsigned len)
> {
> int written, res;
> decompress_fn decompress;
Doing this unconditionally seems to be inviting rootkit authors to use a
new and shiny tool. I also don't think it's the best way to do this,
but I'll comment on the other patch to explain why.
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/2] initramfs with digital signature protection
On Tue, Feb 05, 2013 at 02:34:50PM +0200, Dmitry Kasatkin wrote:
> Often initramfs is (re)fabricated on the machine on which it runs.
> In such cases it is impossible to sign initramfs image, because
> private key is not supposed to be available.
This doesn't address that problem at all.
> This patch adds support for additional digitaly signed initramfs images.
> Digitaly signed initramfs image can be loaded from conventional initramfs
> image or from rootfs and '/pre-init' is executed prior 'init' from initramfs
> or root file system. If 'pre-init' fails, kernel panics. Signed initramfs
> image must be located in '/initramfs-sig.img'.
So with this patch, you've effectively combined an unchecked initramfs with
a checked one that loads before it, and called the answer "secure". This is
like S&P traunching a AAA mortgage with a DDD mortgage and calling it a
AAA CDO - the result isn't as good as what's claimed.
> Digitally signed initramfs can be used to provide protected user-space
> environment for initialization purpose. For example, LSM, IMA/EVM can be
> securely initialized using such approach.
>
> Signing is done using scripts/sign-file from kernel source code and uses
> module
> signature format and module verification API. Important to note again that
> signing of such images should be done on the build machine.
>
> Bellow is an example, how to sign compressed initrd (cpio) image:
>
> scripts/sign-file -v signing_key.priv signing_key.x509 initrd.gz
> initramfs-sig.img
This means that if you've got multiple initramfs images, there's a signature
interleaved between them in the in-memory representation. We've used separate
images to separate invariant bits from e.g. plymouth from the initramfs that's
generated for each new kernel installed. In general, it'd be nice to
keep multiple initramfs images working.
It's not clear to me why we need this encapsulation - wouldn't it be
better to add another [pointer,size] pair to the bootloader protocol
with a structure like:
struct {
void *initramfs_area;
size_t initramfs_area_len;
void *signature;
size_t signature_len;
};
And make that a list (all fields zero signifying the end). That way we
can a) continue to easily support multiple initramfs images, b) allow
for /the whole initramfs to be checked/, which would be useful for e.g.
distro media? If we really need a "parts of this are checked and parts
aren't" system, you could still allow policy to support that, but it's
not clear to me why that's a useful thing to have.
> @@ -895,6 +897,8 @@ static void __init kernel_init_freeable(void)
> prepare_namespace();
> }
>
> + initramfs_sig_load();
> +
> /*
>* Ok, we have completed the initial bootup, and
>* we're essentially up and running. Get rid of the
At the very least, this call should go in kernel_init() so that it's
logically after the comment you trimmed here about how we're about to
start userland running.
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC 2/2] initramfs with digital signature protection
On Tue, Feb 05, 2013 at 02:34:50PM +0200, Dmitry Kasatkin wrote:
> +static const char *secmnt = "/root";
> +static const char *initramfs_img = "/initramfs-sig.img";
> +
> +static int __init load_image(const char *from)
> +{
...
> + err = sys_mount("tpmfs", (char *)secmnt, "tmpfs", MS_SILENT, NULL);
> + if (err) {
> + pr_err("sys_mount() = %d\n", err);
> + goto out;
> + }
> +
> + sys_unshare(CLONE_FS | CLONE_FILES);
> + sys_chdir(secmnt);
> + sys_chroot(".");
> + sys_setsid();
> +
> + pr_info("unpack start\n");
> + msg = unpack_to_rootfs(buf, offset);
...
> +}
> +
> +static int __init init_init(struct subprocess_info *info, struct cred *new)
> +{
> + return load_image(initramfs_img);
> +}
> +
> +static void init_cleanup(struct subprocess_info *info)
> +{
> + int err;
> +
> + pr_info("cleanup\n");
> +
> + err = sys_umount((char *)secmnt, MNT_DETACH);
> + if (err)
> + pr_err("unable to umount secmnt: %d\n", err);
> +}
> +
> +static int __init load_initramfs(void)
> +{
> + static char *argv[] = { "pre-init", NULL, };
> + extern char *envp_init[];
> + int err;
> +
> + /*
> + * In case that a resume from disk is carried out by linuxrc or one of
> + * its children, we need to tell the freezer not to wait for us.
> + */
> + current->flags |= PF_FREEZER_SKIP;
> +
> + err = call_usermodehelper_fns("/pre-init", argv, envp_init,
> + UMH_WAIT_PROC, init_init, init_cleanup,
> + NULL);
> +
> + current->flags &= ~PF_FREEZER_SKIP;
> +
> + pr_info("initramfs_sig /pre-init completed: %d\n", err);
> +
> + return err;
> +}
So, there's another issue here - if you do want to run a fully-signed
initramfs, and so there's nothing else in the container, you go through
this code and it mounts it. In this case the wrapper initramfs which
includes the signed image is still in memory, and there's nothing e.g.
dracut can do about it. Which means you've got two whole copies of the
initramfs in memory, and even if code later cleans up the one it sees,
you can't get rid of the other one.
(this would not be an issue without the wrapper.)
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Mon, Feb 25, 2013 at 11:45:21PM -0500, Theodore Ts'o wrote: > On Tue, Feb 26, 2013 at 02:25:55PM +1000, Dave Airlie wrote: > > > > Its a simple argument, MS can revoke our keys for whatever reason, > > reducing the surface area of reasons for them to do so seems like a > > good idea. Unless someone can read the mind of the MS guy that > > arbitrarily decides this in 5 years time, or has some sort of signed > > agreement, I tend towards protecting the users from having their Linux > > not work anymore, because we were scared of a PE loader in the kernel. > > If Microsoft will revoke keys for whatever reason they want, without > any regard to the potential PR and legal consequences to Microsoft, > there's absolutely **nothing** you can do, short of choosing to use > more open hardware (for example, like the Chromebook Pixel). No, no, no. Quit saying nobody knows. We've got a pretty good idea - we've got a contract with them, and it says they provide the signing service, and under circumstances where the thing being signed is found to enable malware that circumvents Secure Boot, we'll fix it so it can't be, and we've got a certain amount of time to do so, and processes for working with them, and then at that time blacklists will be issued. This is not the precise language from that contract, and I'm not going to go into specifics here. So in our eyes, we've got a choice between excluding unsigned modules from the start, or waiting until there's a very quickly approaching deadline. Obviously, for the distributions that Matthew, David, and I work on, we've chosen protecting them from the start, and some other distributions have chosen differently. One might guess that those who have chosen not to do so are expecting that we'll have come up with a solution before their deadline day happens. So that's where we're coming from, in terms of requiring signatures on modules at all. We're also in a position where some of our customers and hardware partners see a need to load binary-only modules, and we don't want any part in signing or distributing those, for a variety of reasons. So we're looking for a mechanism to allow that, and this is what we've come up with as a technical solution. Would I like to see the user in control? Yes, that's why I've put time and effort into toolchains for users doing their own signing from SB on down. I completely agree that any degree to which users can be convinced to manage their security would be a good thing. But I don't think I can convince all that many of my users to do so, and I think a lot of them are still going to see a need for loading modules like this. > If you're that terrified of the completely arbitrary and capricious > Microsoft guy having us by the short hairs, why aid and abet Microsoft > control-freak model? That description entirely misrepresents our concern. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Tue, Feb 26, 2013 at 10:30:46PM +0100, Florian Weimer wrote: > * Linus Torvalds: > > > So here's what I would suggest, and it is based on REAL SECURITY and > > on PUTTING THE USER FIRST instead of your continual "let's please > > microsoft by doing idiotic crap" approach. > > I think the real question is this one: Is there *any* device out there > which comes with Microsoft Secure Boot enabled, but doesn't have a > copy of Windows 8 on it? > > I guess there isn't. So Secure Boot support is only required for > supporting dual-booting Windows 8, while still retaining the automated > recovery capabilities (which might well remove the Linux installation > on the same box). > > Without dual-booting, there is currently no reason whatsoever to > enable UEFI Secure Boot (or the Microsoft variant). It prevents a form of malware which exists in the wild. I think that's enough reason to want *something*, though SB isn't necessarily what we'd have dreamed of. Nevertheless, SB is what we've got, and as such is why we've been working on how to use it meaningfully. This all seems pretty orthogonal to the question at hand, of course. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Tue, Feb 26, 2013 at 10:57:38PM +0100, Geert Uytterhoeven wrote: > BTW, I assume UEFI checks itself if enrolled hashes have been revoked, > so it must phone home to some server? That must be disabled as well. No. Quit fearmongering. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Wed, Feb 27, 2013 at 01:32:30PM +0100, Geert Uytterhoeven wrote: > On Tue, Feb 26, 2013 at 11:06 PM, Peter Jones wrote: > > On Tue, Feb 26, 2013 at 10:57:38PM +0100, Geert Uytterhoeven wrote: > > > >> BTW, I assume UEFI checks itself if enrolled hashes have been revoked, > >> so it must phone home to some server? That must be disabled as well. > > > > No. Quit fearmongering. > > Good to know, thanks! > > So revocation will only be done by the guest OS? > I.e. if I only boot my own trusted Linux, even if it's signed with the MS key, > the MS key _on my system_ will never be revoked? Something must apply the dbx update. We'll certainly do so in Fedora and RHEL, from userland, but you're certainly in a position to make it not happen in your own trusted linux, and even on a Fedora or RHEL machine you maintain. But there's no "phoning home" involved - the plan is to make that happen as a regular package update to shim-signed, so here you go: --- /etc/yum.conf.old 2013-02-27 09:10:25.181998268 -0500 +++ /etc/yum.conf 2013-02-27 09:10:34.423403583 -0500 @@ -21,3 +21,4 @@ # PUT YOUR REPOS HERE OR IN separate files named file.repo # in /etc/yum.repos.d +exclude=shim-signed And as long as you never boot Windows on the thing, you're set. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
linux-next: build warning after merge of the akpm tree
Stephen, The following email is an updated patch which should fix the warning you're seeing on architectures where sizeof is unsigned int rather than unsigned long. This completely replaces the ef25bb0fa6e2 patch. Andrew, if you'd prefer a single-line fixup patch, I can send you that instead. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] block/partitions/efi.c: ensure that the GPT header is at least the size of the structure.
UEFI 2.3.1D will include a change to the spec language mandating that a
GPT header must be greater than *or equal to* the size of the defined
structure. While verifying that this would work on Linux, I discovered
that we're not actually checking the minimum bound at all.
The result of this is that when we verify the checksum, it's possible that
on a malformed header (with header_size of 0), we won't actually verify
any data.
Signed-off-by: Peter Jones
Cc: Matt Fleming
Cc: Jens Axboe
Cc: Stephen Warren
Cc: Andrew Morton
---
block/partitions/efi.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index b62fb88..62e05cf 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -310,15 +310,23 @@ static int is_gpt_valid(struct parsed_partitions *state,
u64 lba,
goto fail;
}
- /* Check the GUID Partition Table header size */
+ /* Check the GUID Partition Table header size is too big */
if (le32_to_cpu((*gpt)->header_size) >
bdev_logical_block_size(state->bdev)) {
- pr_debug("GUID Partition Table Header size is wrong: %u > %u\n",
+ pr_debug("GUID Partition Table Header size is too large: %u >
%u\n",
le32_to_cpu((*gpt)->header_size),
bdev_logical_block_size(state->bdev));
goto fail;
}
+ /* Check the GUID Partition Table header size is too small */
+ if (le32_to_cpu((*gpt)->header_size) < sizeof(gpt_header)) {
+ pr_debug("GUID Partition Table Header size is too small: %u <
%lu\n",
+ le32_to_cpu((*gpt)->header_size),
+ (unsigned long)sizeof(gpt_header));
+ goto fail;
+ }
+
/* Check the GUID Partition Table CRC */
origcrc = le32_to_cpu((*gpt)->header_crc32);
(*gpt)->header_crc32 = 0;
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] block: partitions: efi: Typecast sizeof(gpt_header)
On Wed, Feb 20, 2013 at 11:52:10AM -0300, Fabio Estevam wrote:
> From: Fabio Estevam
>
> Since commit ef25bb0fa6e2 (block/partitions/efi.c: ensure that the GPT header
> is
> at least the size of the structure.) , when building for a 32-bit
> architecture,
> such as ARM, the following build warning is seen:
>
> block/partitions/efi.c:324:3: warning: format '%lu' expects argument of type
> 'long unsigned int', but argument 3 has type 'unsigned int' [-Wformat]
Hi Fabio,
I sent and updated version of ef25bb0fa6e2 this morning to fix this
issue. Thanks for the patch, though. Andrew, if you'd like you can
just use this instead of replacing my patch with the new one.
Signed-off-by: Peter Jones
>
> Typecast sizeof(gpt_header) to (unsigned long) to avoid such warning on
> 32-bit
> systems.
>
> Reported-by: Stephen Rothwell
> Cc: Peter Jones
> Cc: Matt Fleming
> Cc: Jens Axboe
> Cc: Andrew Morton
> Signed-off-by: Fabio Estevam
> ---
> block/partitions/efi.c |2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block/partitions/efi.c b/block/partitions/efi.c
> index a7475e7..62e05cf 100644
> --- a/block/partitions/efi.c
> +++ b/block/partitions/efi.c
> @@ -323,7 +323,7 @@ static int is_gpt_valid(struct parsed_partitions *state,
> u64 lba,
> if (le32_to_cpu((*gpt)->header_size) < sizeof(gpt_header)) {
> pr_debug("GUID Partition Table Header size is too small: %u <
> %lu\n",
> le32_to_cpu((*gpt)->header_size),
> - sizeof(gpt_header));
> + (unsigned long)sizeof(gpt_header));
> goto fail;
> }
>
> --
> 1.7.9.5
>
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Thu, Feb 21, 2013 at 10:25:47AM -0800, Linus Torvalds wrote: > - why do you bother with the MS keysigning of Linux kernel modules to > begin with? This is not actually what the patchset implements. All it's done here is using PE files as envelopes for keys. The usage this enables is to allow for whoever makes a module (binary only or merely out of tree for whatever reason) to sign it and vouch for it themselves. That could include, for example, a systemtap module. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Thu, Feb 21, 2013 at 10:56:44AM -0800, Linus Torvalds wrote: > On Thu, Feb 21, 2013 at 10:34 AM, Peter Jones wrote: > > On Thu, Feb 21, 2013 at 10:25:47AM -0800, Linus Torvalds wrote: > >> - why do you bother with the MS keysigning of Linux kernel modules to > >> begin with? > > > > This is not actually what the patchset implements. All it's done here > > is using PE files as envelopes for keys. The usage this enables is to > > allow for whoever makes a module (binary only or merely out of tree for > > whatever reason) to sign it and vouch for it themselves. That could > > include, for example, a systemtap module. > > Umm. And which part of "We already support that, using standard X.509 > certificates" did we suddenly miss? > > So no. The PE file thing makes no sense what-so-ever. What you mention > we can already do, and we already do it *better*. It's certainly true that we can use x509 signatures to chain trust from x509 keys to other x509 keys, but when we do, we don't get to use the hardware as the root of trust with any CA that actually *exists*. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] Load keys from signed PE binaries
On Thu, Feb 21, 2013 at 10:03:20AM -0800, Linus Torvalds wrote: > Besides, let's face it, Red Hat is going to sign the official nVidia > and AMD binary modules anyway. Don't even bother to pretend anything > else. I just want to make sure this doesn't go unresponded to - Red Hat will not sign kernel modules built by an outside source. We're simply not going to sign these kernel modules. That's one of the big reasons we want a setup where they can sign their own modules in the first place. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [RFC,PATCH v2] efi: Add support for a UEFI variable filesystem
On Thu, 2012-09-13 at 16:10 +0800, joeyli wrote: > Do we have plan to create a new kobject add to /sys/firmware/efi for > provide a fixed mount point to efivars fs? > e.g. /sys/firmware/efi/efivars > > Or we just direct reuse current /sys/firmeware/efi/vars? But, that means > we need think for the backward compatibility if choice reuse vars > folder. I'm not sure that's a terribly large concern - if you haven't updated your tools to use the new efivars, don't mount the new efivars. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] efi: add efivars kobject to efi sysfs folder
On Thu, Oct 04, 2012 at 05:08:48PM +0800, Jeremy Kerr wrote: > Hi Matt, > > >Jeremy, did you want to pick this up as part of your series? > > I have this in my series, yes. I'm just working on the authenticated > delete code, then will send out the next revision. > > Speaking of which - Peter: I've realised that doing a GetVariable() > after the SetVariable is a much cleaner way of checking whether we > need to drop the inode after an authenticated delete. > > Because the inode's size may not be simply updated by the write(), > we should probably be doing a GetVariable() after an APPEND_WRITE, > to read the new size. Rather than having a separate (and quite > complex) code path to check for the delete case, we may as well use > the same logic to determine if the variable has been deleted. Yeah, this is fine. > >Actually, shouldn't the new filesystem be called "efivarfs", or "efifs" > >to make it more explicit that it is a filesystem? > > I'm not too fussed about the name, but +1 for efivarfs. I like efivarfs for the name, as well. > >We also need something in Documentation/filesystems/ describing the old > >EFI variable method, why it's no longer any good, and why the new > >filesystem is favoured. Does anybody have anything to add to the > >following? > > Looks good to me. We can always elaborate later, if necessary. Yep. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Support UEFI variable append and deleting authenticated variables.
This adds support for appending to all UEFI variables, and also for
deleting authentication variables.
(Updated to address mfleming's concerns on 7/30/2012)
Signed-off-by: Peter Jones
---
drivers/firmware/efivars.c | 99
1 file changed, 91 insertions(+), 8 deletions(-)
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 47408e8..ff8b524 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -108,6 +108,27 @@ struct efi_variable {
__u32 Attributes;
} __attribute__((packed));
+struct win_certificate {
+ __u32 dwLength;
+ __u16 wRevision;
+ __u16 wCertificateType;
+ __u8wCertificate[];
+};
+
+struct win_certificate_uefi_guid {
+ struct win_certificate Hdr;
+ efi_guid_t CertType;
+};
+
+struct efi_variable_authentication {
+ __u64 MonotonicCount;
+ struct win_certificate_uefi_guidAuthInfo;
+};
+
+struct efi_variable_authentication_2 {
+ efi_time_t TimeStamp;
+ struct win_certificate_uefi_guidAuthInfo;
+};
struct efivar_entry {
struct efivars *efivars;
@@ -812,6 +833,7 @@ static ssize_t efivar_create(struct file *filp, struct
kobject *kobj,
unsigned long strsize1, strsize2;
efi_status_t status = EFI_NOT_FOUND;
int found = 0;
+ int is_append = 0;
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
@@ -839,7 +861,13 @@ static ssize_t efivar_create(struct file *filp, struct
kobject *kobj,
break;
}
}
- if (found) {
+ if (new_var->Attributes & EFI_VARIABLE_APPEND_WRITE) {
+ if (!found) {
+ spin_unlock(&efivars->lock);
+ return -EINVAL;
+ }
+ is_append = 1;
+ } else if (found) {
spin_unlock(&efivars->lock);
return -EINVAL;
}
@@ -857,20 +885,73 @@ static ssize_t efivar_create(struct file *filp, struct
kobject *kobj,
spin_unlock(&efivars->lock);
return -EIO;
}
- spin_unlock(&efivars->lock);
/* Create the entry in sysfs. Locking is not required here */
- status = efivar_create_sysfs_entry(efivars,
+ if (is_append) {
+ spin_unlock(&efivars->lock);
+ } else {
+ spin_unlock(&efivars->lock);
+ status = efivar_create_sysfs_entry(efivars,
utf16_strsize(new_var->VariableName,
1024),
new_var->VariableName,
&new_var->VendorGuid);
- if (status) {
- printk(KERN_WARNING "efivars: variable created, but sysfs entry
wasn't.\n");
+ if (status)
+ pr_warn("efivars: variable created, but sysfs entry
wasn't.\n");
}
return count;
}
+static int is_authenticated_delete(struct efi_variable *new_var)
+{
+ /*
+* If we get a set_variable() call that's got an authenticated
+* variable attribute set, and its DataSize is the same size as
+* the AuthInfo descriptor, then it's really a delete.
+*/
+ if (new_var->Attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) {
+ struct efi_variable_authentication *eva;
+ __u32 size;
+
+ if (new_var->DataSize < sizeof(*eva))
+ return 0;
+
+ eva = (struct efi_variable_authentication *)new_var->Data;
+
+ /*
+* 27.2.4 says:
+* dwLength: The length of the entire certificate, including
+* the length of the header, in bytes.
+*/
+ size = sizeof(eva->AuthInfo.CertType) +
+ eva->AuthInfo.Hdr.dwLength;
+
+ if (size == new_var->DataSize)
+ return 1;
+ } else if (new_var->Attributes
+ & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS) {
+ struct efi_variable_authentication_2 *eva;
+ __u32 size;
+
+ if (new_var->DataSize < sizeof(*eva))
+ return 0;
+
+ eva = (struct efi_variable_authentication_2 *)new_var->Data;
+
+ /*
+* 27.2.4 says:
+* dwLength: The length of the entire certificate, including
+* the length of the header, in bytes.
+*/
+ size = sizeof(eva->AuthInfo.CertType) +
+
Re: [PATCH] x86/efi: Add EFI framebuffer earlyprintk support
On Thu, Oct 10, 2013 at 07:28:44PM +0200, Ingo Molnar wrote:
>
> Is a non-32-bit framebuffer a possibility? If yes then it might be nice to
> emit an informative printk() here, so that users who try to enable EFI
> early-printk can at least see why it's not working. (Assuming they get to
> look at regular printk output, on a safe/working kernel.)
Not really - the spec allows RGBx, BGRx, and for custom bit masks, but
they're define like:
typedef struct {
UINT32 RedMask;
UINT32 GreenMask;
UINT32 BlueMask;
UINT32 ReservedMask;
} EFI_PIXEL_BITMASK;
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86/efi: Add EFI framebuffer earlyprintk support
On Thu, Oct 10, 2013 at 07:45:21PM +0200, Ingo Molnar wrote:
>
> * Peter Jones wrote:
>
> > On Thu, Oct 10, 2013 at 07:28:44PM +0200, Ingo Molnar wrote:
> > >
> > > Is a non-32-bit framebuffer a possibility? If yes then it might be nice
> > > to
> > > emit an informative printk() here, so that users who try to enable EFI
> > > early-printk can at least see why it's not working. (Assuming they get to
> > > look at regular printk output, on a safe/working kernel.)
> >
> > Not really - the spec allows RGBx, BGRx, and for custom bit masks, but
> > they're define like:
> >
> > typedef struct {
> > UINT32 RedMask;
> > UINT32 GreenMask;
> > UINT32 BlueMask;
> > UINT32 ReservedMask;
> > } EFI_PIXEL_BITMASK;
>
> Hm, that structure does not show up anywhere in the kernel that I can see.
It's the thing being interpretted in arch/x86/boot/compressed/eboot.c in
setup_gop() in the code that looks like:
if (pixel_format == PIXEL_RGB_RESERVED_8BIT_PER_COLOR) {
si->lfb_depth = 32;
si->lfb_linelength = pixels_per_scan_line * 4;
...
} else if (pixel_format == PIXEL_BGR_RESERVED_8BIT_PER_COLOR) {
...
} else if (pixel_format == PIXEL_BIT_MASK) {
find_bits(pixel_info.red_mask, &si->red_pos, &si->red_size);
...
...
> How are those mask values to be interpreted? As regular bitmasks? Are bits
> in the masks set to 1 consecutively, starting from bit 0?
So, the spec actually has some sample code in it:
INTN
GetPixelElementSize (
IN EFI_PIXEL_BITMASK *PixelBits
)
{
INTN HighestPixel = -1;
INTN BluePixel;
INTN RedPixel;
INTN GreenPixel;
INTN RsvdPixel;
BluePixel = FindHighestSetBit (PixelBits->BlueMask);
RedPixel = FindHighestSetBit (PixelBits->RedMask);
GreenPixel = FindHighestSetBit (PixelBits->GreenMask);
RsvdPixel = FindHighestSetBit (PixelBits->ReservedMask);
HighestPixel = max (BluePixel, RedPixel);
HighestPixel = max (HighestPixel, GreenPixel);
HighestPixel = max (HighestPixel, RsvdPixel);
return HighestPixel;
}
EFI_PHYSICAL_ADDRESS NewPixelAddress;
EFI_PHYSICAL_ADDRESS CurrentPixelAddress;
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION OutputInfo;
INTN PixelElementSize;
switch (OutputInfo.PixelFormat) {
case PixelBitMask:
PixelElementSize = GetPixelElementSize
(&OutputInfo.PixelInformation);
break;
case PixelBlueGreenRedReserved8BitPerColor:
case PixelRedGreenBlueReserved8BitPerColor:
PixelElementSize = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);
break;
}
Which makes this painfully clear.
> Also, the main question would be, what is the typical value for
> si->lfb_depth. 32 on almost all EFI systems? All around the map? Depends
> on what graphics state the EFI bootloader passes us?
Yes, 32 on almost all systems that implement a framebuffer console at
all.
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86: Switch to vga console only if framebuffer details are missing
On Wed, Oct 10, 2012 at 02:43:07PM +0100, Matt Fleming wrote: > On Mon, 2012-09-17 at 11:29 +0100, Matt Fleming wrote: > > From: Matt Fleming > > > > The efi_enabled variable has come to mean "Do we have EFI runtime > > services available?". However, lack of EFI runtime services does not > > mean that we should switch to using the VGA console. Provided that the > > boot loader passed the dimensions of the EFI framebuffer there is no > > reason we can't use efifb. > > > > There's also no reason to check the memory type of 0xa - whether > > or not that memory region is EFI_CONVENTIONAL_MEMORY is immaterial - > > the EFI framebuffer device will still work, and checking the EFI > > memory type of a memory region on a non-EFI machine is illogical. > > > > Cc: H. Peter Anvin > > Cc: Matthew Garrett > > Cc: Olof Johansson > > Cc: Peter Jones > > Cc: Ingo Molnar > > Signed-off-by: Matt Fleming > > --- > > arch/x86/kernel/setup.c |2 +- > > 1 files changed, 1 insertions(+), 1 deletions(-) > > Ping? Anybody got an opinion on this one? Provided "video=vga" still works to circumvent Alan's objection, I'm all for it. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Handle efifb with no lfb_base better.
Right now we get a WARN from platform_device_unregister() because our
platform_device has no ->release function. This is clearly wrong, but
we should be warning so we can figure out what happened, as this failure
results in bug reports. So WARN() about the real problem, and use the
registration function that gives us a default release() function.
This fixes the tracback reported at
https://bugzilla.redhat.com/show_bug.cgi?id=840621 , though it does not
fix the actual /problem/ the user is seeing.
Signed-off-by: Peter Jones
---
drivers/video/efifb.c | 15 ---
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c
index 50fe668..390b61b 100644
--- a/drivers/video/efifb.c
+++ b/drivers/video/efifb.c
@@ -285,6 +285,7 @@ static void efifb_destroy(struct fb_info *info)
{
if (info->screen_base)
iounmap(info->screen_base);
+ fb_dealloc_cmap(&info->cmap);
if (request_mem_succeeded)
release_mem_region(info->apertures->ranges[0].base,
info->apertures->ranges[0].size);
@@ -382,6 +383,8 @@ static int __init efifb_probe(struct platform_device *dev)
if (!screen_info.pages)
screen_info.pages = 1;
if (!screen_info.lfb_base) {
+ WARN(1, KERN_WARNING, "invalid framebuffer address for "
+ "device '%s'\n", dev_name(dev));
printk(KERN_DEBUG "efifb: invalid framebuffer address\n");
return -ENODEV;
}
@@ -544,9 +547,7 @@ static struct platform_driver efifb_driver = {
},
};
-static struct platform_device efifb_device = {
- .name = "efifb",
-};
+static struct platform_device *efifb_device;
static int __init efifb_init(void)
{
@@ -571,9 +572,9 @@ static int __init efifb_init(void)
if (!screen_info.lfb_linelength)
return -ENODEV;
- ret = platform_device_register(&efifb_device);
- if (ret)
- return ret;
+ efifb_device = platform_device_register_simple("efifb", 0, NULL, 0);
+ if (IS_ERROR(efifb_device))
+ return PTR_ERR(efifb_device);
/*
* This is not just an optimization. We will interfere
@@ -582,7 +583,7 @@ static int __init efifb_init(void)
*/
ret = platform_driver_probe(&efifb_driver, efifb_probe);
if (ret) {
- platform_device_unregister(&efifb_device);
+ platform_device_unregister(efifb_device);
return ret;
}
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Release efifb's colormap in efifb_destroy()
On Fri, Aug 16, 2013 at 03:51:34PM +0200, David Herrmann wrote: > Hi > > On Thu, Jul 25, 2013 at 5:48 PM, Peter Jones wrote: > > This was found by Alexandra Kossovsky, who noted this traceback from > > kmemleak: > > > >> unreferenced object 0x880216fcfe00 (size 512): > >> comm "swapper/0", pid 1, jiffies 4294895429 (age 1415.320s) > >> hex dump (first 32 bytes): > >> 00 00 00 00 00 00 00 00 aa aa aa aa aa aa aa aa > >> 55 55 55 55 55 55 55 55 ff ff ff ff ff ff ff ff > >> backtrace: > >> [] kmemleak_alloc+0x21/0x3e > >> [] > >> kmemleak_alloc_recursive.constprop.57+0x16/0x18 > >> [] __kmalloc+0xf9/0x144 > >> [] fb_alloc_cmap_gfp+0x47/0xe1 > >> [] fb_alloc_cmap+0xe/0x10 > >> [] efifb_probe+0x3e9/0x48f > >> [] platform_drv_probe+0x34/0x5e > >> [] driver_probe_device+0x98/0x1b4 > >> [] __driver_attach+0x4e/0x6f > >> [] bus_for_each_dev+0x57/0x8a > >> [] driver_attach+0x19/0x1b > >> [] bus_add_driver+0xde/0x201 > >> [] driver_register+0x8c/0x110 > >> [] platform_driver_register+0x41/0x43 > >> [] platform_driver_probe+0x18/0x8a > >> [] efifb_init+0x276/0x295 > > (CC'ing fbdev maintainers) > > Your signed-off-by is missing. Apart from that: > Reviewed-by: David Herrmann Indeed it is. With that in mind: Signed-off-by: Peter Jones > > Regards > David > > > --- > > drivers/video/efifb.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c > > index 390b61b..1f3eab3 100644 > > --- a/drivers/video/efifb.c > > +++ b/drivers/video/efifb.c > > @@ -289,6 +289,7 @@ static void efifb_destroy(struct fb_info *info) > > if (request_mem_succeeded) > > release_mem_region(info->apertures->ranges[0].base, > >info->apertures->ranges[0].size); > > + fb_dealloc_cmap(&info->cmap); > > framebuffer_release(info); > > } > > > > -- > > 1.8.3.1 > > > > -- -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: kmemleak warning in efifb_probe
On Thu, Jul 25, 2013 at 01:18:31PM +0100, Catalin Marinas wrote: > On 21 July 2013 16:11, Alexandra N. Kossovsky > wrote: > > I am running linux-3.10.0 with kmemleak and see following warnings > > in /sys/kernel/debug/kmemleak: > > > > unreferenced object 0x880216fcfe00 (size 512): > > comm "swapper/0", pid 1, jiffies 4294895429 (age 1415.320s) > > hex dump (first 32 bytes): > > 00 00 00 00 00 00 00 00 aa aa aa aa aa aa aa aa > > 55 55 55 55 55 55 55 55 ff ff ff ff ff ff ff ff > > backtrace: > > [] kmemleak_alloc+0x21/0x3e > > [] kmemleak_alloc_recursive.constprop.57+0x16/0x18 > > [] __kmalloc+0xf9/0x144 > > [] fb_alloc_cmap_gfp+0x47/0xe1 > > [] fb_alloc_cmap+0xe/0x10 > > [] efifb_probe+0x3e9/0x48f > > [] platform_drv_probe+0x34/0x5e > > [] driver_probe_device+0x98/0x1b4 > > [] __driver_attach+0x4e/0x6f > > [] bus_for_each_dev+0x57/0x8a > > [] driver_attach+0x19/0x1b > > [] bus_add_driver+0xde/0x201 > > [] driver_register+0x8c/0x110 > > [] platform_driver_register+0x41/0x43 > > [] platform_driver_probe+0x18/0x8a > > [] efifb_init+0x276/0x295 > > Does the efifb driver has any way to deallocate the cmap? I don't see > any explicit call to fb_dealloc_cmap() apart from the error handling. > My theory is that the efifb driver gets deregistered via > do_remove_conflicting_framebuffers(). I'm not familiar with this code, > just commenting from a kmemleak perspective. You're right, that's the typical deregister path. I'll follow up with a patch to test. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Release efifb's colormap in efifb_destroy()
This was found by Alexandra Kossovsky, who noted this traceback from kmemleak: > unreferenced object 0x880216fcfe00 (size 512): > comm "swapper/0", pid 1, jiffies 4294895429 (age 1415.320s) > hex dump (first 32 bytes): > 00 00 00 00 00 00 00 00 aa aa aa aa aa aa aa aa > 55 55 55 55 55 55 55 55 ff ff ff ff ff ff ff ff > backtrace: > [] kmemleak_alloc+0x21/0x3e > [] > kmemleak_alloc_recursive.constprop.57+0x16/0x18 > [] __kmalloc+0xf9/0x144 > [] fb_alloc_cmap_gfp+0x47/0xe1 > [] fb_alloc_cmap+0xe/0x10 > [] efifb_probe+0x3e9/0x48f > [] platform_drv_probe+0x34/0x5e > [] driver_probe_device+0x98/0x1b4 > [] __driver_attach+0x4e/0x6f > [] bus_for_each_dev+0x57/0x8a > [] driver_attach+0x19/0x1b > [] bus_add_driver+0xde/0x201 > [] driver_register+0x8c/0x110 > [] platform_driver_register+0x41/0x43 > [] platform_driver_probe+0x18/0x8a > [] efifb_init+0x276/0x295 --- drivers/video/efifb.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/video/efifb.c b/drivers/video/efifb.c index 390b61b..1f3eab3 100644 --- a/drivers/video/efifb.c +++ b/drivers/video/efifb.c @@ -289,6 +289,7 @@ static void efifb_destroy(struct fb_info *info) if (request_mem_succeeded) release_mem_region(info->apertures->ranges[0].base, info->apertures->ranges[0].size); + fb_dealloc_cmap(&info->cmap); framebuffer_release(info); } -- 1.8.3.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Print the actual UEFI error name, not just the error code.
EFI error numbers are useful, but symbol names are way easier to
understand when you're reading bug reports. And since, for the most
part, we know the names, we should show them.
Signed-off-by: Peter Jones
Reviewed-by: Adam Jackson
---
drivers/firmware/efivars.c | 19 +---
include/linux/efi.h| 75 +++---
2 files changed, 71 insertions(+), 23 deletions(-)
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 182ce94..6dc0676 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -382,8 +382,7 @@ get_var_data(struct efivars *efivars, struct efi_variable
*var)
spin_unlock_irqrestore(&efivars->lock, flags);
if (status != EFI_SUCCESS) {
- printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n",
- status);
+ efi_pr_warn(status, "efivars: get_variable() failed");
}
return status;
}
@@ -549,8 +548,7 @@ efivar_store_raw(struct efivar_entry *entry, const char
*buf, size_t count)
spin_unlock_irq(&efivars->lock);
if (status != EFI_SUCCESS) {
- printk(KERN_WARNING "efivars: set_variable() failed:
status=%lx\n",
- status);
+ efi_pr_warn(status, "efivars: set_variable() failed");
return -EIO;
}
@@ -802,8 +800,8 @@ static ssize_t efivarfs_file_write(struct file *file,
} else {
spin_unlock_irq(&efivars->lock);
- pr_warn("efivarfs: inconsistent EFI variable implementation? "
- "status = %lx\n", status);
+ efi_pr_warn(status,
+ "efivarfs: inconsistent EFI variable implementation? ");
}
out:
@@ -1549,8 +1547,7 @@ static ssize_t efivar_create(struct file *filp, struct
kobject *kobj,
new_var->Data);
if (status != EFI_SUCCESS) {
- printk(KERN_WARNING "efivars: set_variable() failed:
status=%lx\n",
- status);
+ efi_pr_warn(status, "efivars: set_variable() failed");
spin_unlock_irq(&efivars->lock);
return -EIO;
}
@@ -1614,8 +1611,7 @@ static ssize_t efivar_delete(struct file *filp, struct
kobject *kobj,
del_var->Data);
if (status != EFI_SUCCESS) {
- printk(KERN_WARNING "efivars: set_variable() failed:
status=%lx\n",
- status);
+ efi_pr_warn(status, "efivars: set_variable() failed");
spin_unlock_irq(&efivars->lock);
return -EIO;
}
@@ -2023,8 +2019,7 @@ int register_efivars(struct efivars *efivars,
case EFI_NOT_FOUND:
break;
default:
- printk(KERN_WARNING "efivars: get_next_variable:
status=%lx\n",
- status);
+ efi_pr_warn(status, "efivars: get_next_variable");
status = EFI_NOT_FOUND;
break;
}
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 3d7df3d..9f3ed60 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -23,18 +23,71 @@
#include
+#define EFI_ERROR(x) ((x) | (1UL << (BITS_PER_LONG-1)))
+#define EFI_REVERSE_ERROR(x) ((x) & ~(1UL << (BITS_PER_LONG-1)))
+
#define EFI_SUCCESS0
-#define EFI_LOAD_ERROR ( 1 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_INVALID_PARAMETER ( 2 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_UNSUPPORTED( 3 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_BAD_BUFFER_SIZE ( 4 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_BUFFER_TOO_SMALL ( 5 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_NOT_READY ( 6 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_DEVICE_ERROR ( 7 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_WRITE_PROTECTED( 8 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_OUT_OF_RESOURCES ( 9 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_NOT_FOUND (14 | (1UL << (BITS_PER_LONG-1)))
-#define EFI_SECURITY_VIOLATION (26 | (1UL << (BITS_PER_LONG-1)))
+#define EFI_LOAD_ERROR EFI_ERROR(1)
+#define EFI_INVALID_PARAMETER EFI_ERROR(2)
+#define EFI_UNSUPPORTEDEFI_ERROR(3)
+#define EFI_BAD_BUFFER_SIZEEFI_ERROR(4)
+#define EFI_BUFFER_TOO_SMALL EFI_ERROR(5)
+#define EFI_NOT_READY EFI_ERROR(6)
+#define EFI_DEVICE_ERROR EFI_ERROR(7)
+#define EFI_WRITE_PROTECTEDEFI_ERROR(8)
+#define EFI_OUT_OF_RESOURCES EFI_ERROR(9)
+#define EFI_NOT_FOUND EFI_ERRO
Re: Revert commit 5dcd14ecd4 - breaks EFI boot with SLES11 elilo.efi
On Thu, Feb 28, 2013 at 01:12:11PM -0800, H. Peter Anvin wrote: > Then make it follow the boot spec: > > > In 32-bit boot protocol, the first step in loading a Linux kernel > > should be to setup the boot parameters (struct boot_params, > > traditionally known as "zero page"). The memory for struct boot_params > > should be allocated and initialized to all zero. Then the setup header > > from offset 0x01f1 of kernel image on should be loaded into struct > > boot_params and examined. The end of setup header can be calculated as > > follow: > > > > 0x0202 + byte value at offset 0x0201 > > ... so we don't have to. So, the problem here seems to be that there's never been widespread compliance with this paragraph, but this patch assumes there has. A brief survey concludes: grub 1 on bios - loads the kernel and edits the parameters it cares about in place grub 1 on efi - allocates a buffer (fails to clear it) and modifies the parameters it cares about, then copies it back grub 2 on bios - clears the buffer, writes what it cares about grub 2 on efi (using efi boot stub) - reads the buffer, modifies fields it cares about, passes the pointer to the boot stub elilo - allocates a new buffer, copies the kernel structure in to it, allocates another buffer, clears it, copies the first structure in to it, frees the first buffer, modifies fields it cares about in the second buffer, clears some other fields in the second structure, and passes the pointer in when it calls the old entry point (It's possible that there's some newer version of elilo than 3.14, which I had handy, but I'm not going to do deeper research on a project that keeps a link to its CVS repo on the most obvious google result, lest I lose the will to live.) syslinux - I'm just going to assume that your code matches the spec. So it's certainly worth trying to find a better way to check this, but I don't think this patch is it. If we're going to enforce it, we have to make sure that a bootloader that's conforming to what was de facto the standard in 0x020b still works. Otherwise we're just breaking bootloaders for no reason, and that will end poorly. I'd suggest we add a field for the bootloader to make a positive declaration of what version it is using, and only check for the sentinel if the field claims it's doing 0x020c or newer. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Revert commit 5dcd14ecd4 - breaks EFI boot with SLES11 elilo.efi
On Wed, Mar 06, 2013 at 09:14:27AM -0800, H. Peter Anvin wrote: > On 03/06/2013 08:55 AM, Peter Jones wrote: > > > > So, the problem here seems to be that there's never been widespread > > compliance with this paragraph, but this patch assumes there has. A > > brief survey concludes: > > No, this patch doesn't assume there is widespread compliance, it is > trying to address the bits that are not complied with. Right, but that's basically every x86_64 UEFI machine ever deployed. [lots trimmed] > > So it's certainly worth trying to find a better way to check this, but I > > don't think this patch is it. If we're going to enforce it, we have to > > make sure that a bootloader that's conforming to what was de facto the > > standard in 0x020b still works. Otherwise we're just breaking > > bootloaders for no reason, and that will end poorly. > > > > I'd suggest we add a field for the bootloader to make a positive > > declaration of what version it is using, and only check for the sentinel > > if the field claims it's doing 0x020c or newer. > > Except it doesn't quite work. The problem is that these broken > bootloaders aren't just a matter of 2.11 vs 2.12, they are implicitly > assuming that the kernel image itself doesn't happen to contain anything > harmful in the fields that they don't bother initializing. This would > be nice and good, except that the demands for the boot sector space is > fairly high and it gets very cantankerous to turn that into a minefield. If your only objection is real estate, we can find a way to be clever about what we do that uses already existing space. For instance, write back the version number that's supported in the version field, but byte-swapped, so we can tell it changed (we don't anticipate ever supporting protocol 0x20b from a kernel that advertises 0xb02, right?) Just one example - we don't have to do this the exact way I said; we just need a positive assertion from the bootloader to start doing enforcement. Versions would be nice, but they're not strictly required. > In fact, your suggestion is exactly equivalent to the sentinel, except > you want it to be pre-initialized with 0x20b instead of 0x. No, I want the bootloader to communicate that it understands the boot protocol revision is 0x020c, so we can /safely/ enter a world where we're forbidding booting from an older bootloader. > As such, I don't really know anything better we can do other than: > > 1. detect the *properly working* case of the structure properly >initialized Which is easy, but it doesn't seem to be anything anybody has ever shipped on UEFI machines. > 2. doing legacy bootloader-specific clearing based on the bootloader ID >if the sentinel triggers -- if you can think of better heuristics >then that would be good; This heuristic is "all UEFI bootloaders anybody uses". You can list them individually, but it's the same as reverting the patch, just with more code. > 3. try to get bootloaders switched from case #2 to case #1. And I'm for that, but I think we should delay enforcement until they've got a way to express that. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Be explicit about what the x86 0x020c boot parameter version requires.
This should help avoid making the incorrect change in non-compliant
bootloaders.
Signed-off-by: Peter Jones
---
Documentation/x86/boot.txt | 5 +++--
arch/x86/include/asm/bootparam_utils.h | 7 +++
2 files changed, 10 insertions(+), 2 deletions(-)
diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt
index 3840b6f..72702db 100644
--- a/Documentation/x86/boot.txt
+++ b/Documentation/x86/boot.txt
@@ -1110,7 +1110,8 @@ firmware, 'table' is the EFI system table - these are the
first two
arguments of the "handoff state" as described in section 2.3 of the
UEFI specification. 'bp' is the boot loader-allocated boot params.
-The boot loader *must* fill out the following fields in bp,
+The boot loader *must* zero the entirity of bp, and then fill out the
+following fields:
o hdr.code32_start
o hdr.cmd_line_ptr
@@ -1118,4 +1119,4 @@ The boot loader *must* fill out the following fields in
bp,
o hdr.ramdisk_image (if applicable)
o hdr.ramdisk_size (if applicable)
-All other fields should be zero.
+All other fields should remain zero.
diff --git a/arch/x86/include/asm/bootparam_utils.h
b/arch/x86/include/asm/bootparam_utils.h
index 5b5e9cb..b4e8aa8 100644
--- a/arch/x86/include/asm/bootparam_utils.h
+++ b/arch/x86/include/asm/bootparam_utils.h
@@ -17,6 +17,13 @@
*/
static void sanitize_boot_params(struct boot_params *boot_params)
{
+ /* Note: do not simply clear this field. The purpose of this field is
+* to guarantee compliance with the x86 boot spec located in
+* Documentation/x86/boot.txt . That spec says that the *whole*
+* structure should be cleared. If you're having an issue because
+* the sentinel is set, you need to change the whole structure to be
+* cleared, not this (or any other) indidivual field.
+*/
if (boot_params->sentinel) {
/*fields in boot_params are not valid, clear them */
memset(&boot_params->olpc_ofw_header, 0,
--
1.8.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MODSIGN: Fix documentation of signed-nokey behavior when not enforcing.
jwboyer's previous commit changes the behavior of module signing when there's a valid signature but we don't know the public key and are in permissive mode. This updates the documentation to match. --- Documentation/module-signing.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt index d75d473..8c4bef9 100644 --- a/Documentation/module-signing.txt +++ b/Documentation/module-signing.txt @@ -185,7 +185,7 @@ This table indicates the behaviours of the various situations: MODULE STATEPERMISSIVE MODE ENFORCING MODE === === === UnsignedOk EKEYREJECTED - Signed, no public key ENOKEY ENOKEY + Signed, no public key Ok ENOKEY Validly signed, public key Ok Ok Invalidly signed, public keyEKEYREJECTEDEKEYREJECTED Validly signed, expired key EKEYEXPIRED EKEYEXPIRED -- 1.7.11.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] MODSIGN: Fix documentation of signed-nokey behavior when not enforcing.
jwboyer's previous commit changes the behavior of module signing when there's a valid signature but we don't know the public key and are in permissive mode. This updates the documentation to match. Signed-off-by: Peter Jones Acked-by: Josh Boyer --- Documentation/module-signing.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/module-signing.txt b/Documentation/module-signing.txt index d75d473..8c4bef9 100644 --- a/Documentation/module-signing.txt +++ b/Documentation/module-signing.txt @@ -185,7 +185,7 @@ This table indicates the behaviours of the various situations: MODULE STATEPERMISSIVE MODE ENFORCING MODE === === === UnsignedOk EKEYREJECTED - Signed, no public key ENOKEY ENOKEY + Signed, no public key Ok ENOKEY Validly signed, public key Ok Ok Invalidly signed, public keyEKEYREJECTEDEKEYREJECTED Validly signed, expired key EKEYEXPIRED EKEYEXPIRED -- 1.7.11.2 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 14/16] X.509: Add an ASN.1 decoder
On Tue, 2012-09-18 at 19:51 +0100, Alan Cox wrote:
> On Tue, 18 Sep 2012 18:34:12 +0100
> David Howells wrote:
>
> > Alan Cox wrote:
> >
> > > Why do this in the kernel.That appears to be completely insane.
> >
> > A number of reasons:
> >
> > (1) The UEFI signature/key database may contain ASN.1 X.509 certificates
> > and
> > we may need to use those very early in the boot process, during initrd.
>
> Ok that makes some sense. Presumably they've also got to fall within what
> you trust and sign ?
The idea is that you implicitly trust keys in the lists maintained by
your system firmware and/or shim ("mok") key databases, or else you
shouldn't have Secure Boot turned on in the first place. Using these
keys and hashes allows you to e.g. relatively easily add a key you're
using to sign a module you're currently developing, while still *ahem*
enjoying the many benefits of signed modules, kernel, and bootloader.
(Though obviously we would never recommend adding a public key whose
private half you're normally keeping on that same machine.)
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Use the UEFI shim to load grub.
For UEFI Secure Boot support, we need to install the shim pre-boot
loader, and use it to load grub2.
---
pyanaconda/bootloader.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
index ea4de5e..02d0ea8 100644
--- a/pyanaconda/bootloader.py
+++ b/pyanaconda/bootloader.py
@@ -1749,7 +1749,7 @@ class GRUB2(GRUB):
self.stage2_device.format.sync(root=ROOT_PATH)
class EFIGRUB(GRUB2):
-packages = ["grub2-efi", "efibootmgr"]
+packages = ["grub2-efi", "efibootmgr", "shim"]
can_dual_boot = False
@property
@@ -1809,7 +1809,7 @@ class EFIGRUB(GRUB2):
rc = self.efibootmgr("-c", "-w", "-L", productName,
"-d", boot_disk.path, "-p", boot_part_num,
"-l",
- self.efi_dir_as_efifs_dir + "\\grubx64.efi",
+ self.efi_dir_as_efifs_dir + "\\shim.efi",
root=ROOT_PATH,
stdout="/dev/tty5", stderr="/dev/tty5")
if rc:
--
1.7.11.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Use the UEFI shim to load grub.
Obviously, this went to the wrong list of addresses. Sorry for all the
noise.
On Wed, 2012-08-08 at 10:18 -0400, Peter Jones wrote:
> For UEFI Secure Boot support, we need to install the shim pre-boot
> loader, and use it to load grub2.
> ---
> pyanaconda/bootloader.py | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/pyanaconda/bootloader.py b/pyanaconda/bootloader.py
> index ea4de5e..02d0ea8 100644
> --- a/pyanaconda/bootloader.py
> +++ b/pyanaconda/bootloader.py
> @@ -1749,7 +1749,7 @@ class GRUB2(GRUB):
> self.stage2_device.format.sync(root=ROOT_PATH)
>
> class EFIGRUB(GRUB2):
> -packages = ["grub2-efi", "efibootmgr"]
> +packages = ["grub2-efi", "efibootmgr", "shim"]
> can_dual_boot = False
>
> @property
> @@ -1809,7 +1809,7 @@ class EFIGRUB(GRUB2):
> rc = self.efibootmgr("-c", "-w", "-L", productName,
> "-d", boot_disk.path, "-p", boot_part_num,
> "-l",
> - self.efi_dir_as_efifs_dir + "\\grubx64.efi",
> + self.efi_dir_as_efifs_dir + "\\shim.efi",
> root=ROOT_PATH,
> stdout="/dev/tty5", stderr="/dev/tty5")
> if rc:
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Ensure that the GPT header is at least the size of the structure.
UEFI 2.3.1D will include a change to the spec language mandating that a
GPT header must be greater than *or equal to* the size of the defined
structure. While verifying that this would work on Linux, I discovered
that we're not actually checking the minimum bound at all.
The result of this is that when we verify the checksum, it's possible
that on a malformed header (with header_size of 0), we won't actually
verify any data.
(ammended to fix type error in pr_debug())
Signed-off-by: Peter Jones
---
block/partitions/efi.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index b62fb88..a7475e7 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -310,15 +310,23 @@ static int is_gpt_valid(struct parsed_partitions *state,
u64 lba,
goto fail;
}
- /* Check the GUID Partition Table header size */
+ /* Check the GUID Partition Table header size is too big */
if (le32_to_cpu((*gpt)->header_size) >
bdev_logical_block_size(state->bdev)) {
- pr_debug("GUID Partition Table Header size is wrong: %u > %u\n",
+ pr_debug("GUID Partition Table Header size is too large: %u >
%u\n",
le32_to_cpu((*gpt)->header_size),
bdev_logical_block_size(state->bdev));
goto fail;
}
+ /* Check the GUID Partition Table header size is too small */
+ if (le32_to_cpu((*gpt)->header_size) < sizeof(gpt_header)) {
+ pr_debug("GUID Partition Table Header size is too small: %u <
%lu\n",
+ le32_to_cpu((*gpt)->header_size),
+ sizeof(gpt_header));
+ goto fail;
+ }
+
/* Check the GUID Partition Table CRC */
origcrc = le32_to_cpu((*gpt)->header_crc32);
(*gpt)->header_crc32 = 0;
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Ensure that the GPT header is at least the size of the structure.
UEFI 2.3.1D will include a change to the spec language mandating that a
GPT header must be greater than *or equal to* the size of the defined
structure. While verifying that this would work on Linux, I discovered
that we're not actually checking the minimum bound at all.
The result of this is that when we verify the checksum, it's possible
that on a malformed header (with header_size of 0), we won't actually
verify any data.
Signed-off-by: Peter Jones
---
block/partitions/efi.c | 12 ++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/block/partitions/efi.c b/block/partitions/efi.c
index b62fb88..5ad168c 100644
--- a/block/partitions/efi.c
+++ b/block/partitions/efi.c
@@ -310,15 +310,23 @@ static int is_gpt_valid(struct parsed_partitions *state,
u64 lba,
goto fail;
}
- /* Check the GUID Partition Table header size */
+ /* Check the GUID Partition Table header size is too big */
if (le32_to_cpu((*gpt)->header_size) >
bdev_logical_block_size(state->bdev)) {
- pr_debug("GUID Partition Table Header size is wrong: %u > %u\n",
+ pr_debug("GUID Partition Table Header size is too large: %u >
%u\n",
le32_to_cpu((*gpt)->header_size),
bdev_logical_block_size(state->bdev));
goto fail;
}
+ /* Check the GUID Partition Table header size is too small */
+ if (le32_to_cpu((*gpt)->header_size) < sizeof(gpt_header)) {
+ pr_debug("GUID Partition Table Header size is too small: %u <
%u\n",
+ le32_to_cpu((*gpt)->header_size),
+ sizeof(gpt_header));
+ goto fail;
+ }
+
/* Check the GUID Partition Table CRC */
origcrc = le32_to_cpu((*gpt)->header_crc32);
(*gpt)->header_crc32 = 0;
--
1.8.1.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [patch] ibft: add a missing break statement
On Fri, Jan 11, 2013 at 09:48:07AM +0300, Dan Carpenter wrote: > The code works the same with or without the break. It just looks a bit > cleaner to not fall through. Yeah, you're clearly right. For those reading this, so you don't have to go check yourselves, the line after the end of this diff is also a break; there's no further code in that case. > Signed-off-by: Dan Carpenter Acked-by: Peter Jones > > diff --git a/drivers/firmware/iscsi_ibft.c b/drivers/firmware/iscsi_ibft.c > index 3ee852c..c4b187c 100644 > --- a/drivers/firmware/iscsi_ibft.c > +++ b/drivers/firmware/iscsi_ibft.c > @@ -503,6 +503,7 @@ static umode_t __init ibft_check_tgt_for(void *data, int > type) > case ISCSI_BOOT_TGT_NIC_ASSOC: > case ISCSI_BOOT_TGT_CHAP_TYPE: > rc = S_IRUGO; > + break; > case ISCSI_BOOT_TGT_NAME: > if (tgt->tgt_name_len) > rc = S_IRUGO; -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [syslinux] Re: THE LINUX/I386 BOOT PROTOCOL - Breaking the 256 limit
On Wed, 2005-08-31 at 15:07 -0700, Chris Wedgwood wrote: > On Wed, Aug 31, 2005 at 03:01:57PM -0700, H. Peter Anvin wrote: > > > Maybe not. Another option would simply be to bump it up > > significantly (2x isn't really that much.) 4096, maybe. > > I wonder if we're not at the point where we need something different > to what we have now. The concept of a command-line works for passing > simple state but for more complex things it's too cumbersome. Well, for things that don't have to be done while the kernel is loading, it's entirely possible to do more complex things in initramfs. Form a second image that's got a config file in it, and make the initramfs's init set things up based on that. But obviously that only works for things that are tweakable from userland. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: IDE HPA
On Fri, 2005-09-02 at 19:44 +0200, Molle Bestefich wrote: > Related matters: > If you decide to include the HPA in one of your filesystems, is there > not a big risk that the BIOS will overwrite something there? Isn't the bigger risk that if you include the HPA in your block device, you'll overwrite your BIOS there? At least that's what happens on some thinkpads. It'd be nice if we left it as the BIOS leaves it by default, but make a straightforward way to enable/disable/alter the HPA region from software. (if there's already a straightforward way, feel free to clue me in -- but the default should almost certainly be to assume the HPA is set up correctly, shouldn't it?) -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: IDE HPA
On Fri, 2005-09-02 at 19:59 +0100, Alan Cox wrote: > On Gwe, 2005-09-02 at 14:09 -0400, Peter Jones wrote: > > (if there's already a straightforward way, feel free to clue me in -- > > but the default should almost certainly be to assume the HPA is set up > > correctly, shouldn't it?) > > The normal use of HPA is to clip drives to get them past BIOS boot > checks. Ugh. So some BIOSes use it for legitimate reasons (like thinkpads), and some use it to work around BIOS bugs. Great. > The thinkpads come with a pre-installed partition table which > will protect the HPA unless the user goes to town removing it. Mine didn't, but it does have an HPA. Thankfully we weren't disabling it yet when I installed my laptop -- I know others who weren't so lucky. So this partitioning scheme hasn't always been the case... And it seems broken anyway. The point of the HPA is to make the OS see a smaller/different disk layout, unless it's actually trying to update things that are "protected", right? If so, the partition table pointing outside of the the disk when the HPA configuration hasn't been changed from the bootup default totally broken :/ It really sounds better (to my naive mind, at least) to whitelist the known-broken BIOSes. > The ideal case would be that the partition table is considered at boot > to see if the HPA matches the partitiont table or not. You'd also then > need dynamic HPA enable/disable for installers and other tools to go > with that. Well, installers probably should be aware, yes -- that's why I mentioned userland interfaces to enabling/disabling. But to me it still seems like we want to disable the HPA during installation and bootup, but only if your BIOS is doing things wrong. > Send patches. Point taken. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: IDE HPA
On Fri, 2005-09-02 at 21:22 +0100, Alan Cox wrote: > On Gwe, 2005-09-02 at 15:14 -0400, Peter Jones wrote: > > Ugh. So some BIOSes use it for legitimate reasons (like thinkpads), and > > some use it to work around BIOS bugs. Great. > > All are legitimate uses. The partition table tells you which. > > > Mine didn't, but it does have an HPA. Thankfully we weren't disabling > > it yet when I installed my laptop -- I know others who weren't so lucky. > > So this partitioning scheme hasn't always been the case... > > You installed it on Red Hat 7 ? I think 7, may have been 6.x or earlier. You may be right -- it's likely that I shrank my windows partition on some other OS or Distro that wasn't designed with to screw up the disk. > This behaviour goes back pretty much to the creation of the ATA spec for > HPA. In fact if it was that long ago IBM shipped it with Windows so it > did have a partition table! Yes, it did have a partition table -- but the partition table did (and still does) not include partitions which overlap the HPA. Right now it still appears as unused space. > > It really sounds better (to my naive mind, at least) to whitelist the > > known-broken BIOSes. > > Not really practical. You'd have to list most older PC systems. Most older PC systems use HPA? Really? The alternatives seem to be: a) whitelist systems (in the kernel or userland) which use it the opposite way, where the space is being used for something relatively important (at least conceptually), or b) bad heuristics to figure out which is which. Both of these suck. Have I missed something? > > Well, installers probably should be aware, yes -- that's why I mentioned > > userland interfaces to enabling/disabling. But to me it still seems > > like we want to disable the HPA during installation and bootup, but only > > if your BIOS is doing things wrong. > > "Wrong" is a poor term here. > > If the system has a partition table that doesn't cover the post HPA area > and its about the right size we can be fairly sure the right choice is > to honour the HPA, if its a randomly different size its a fair bet the > disk got moved > > If the partition table exceeds the no HPA area of disk but not the full > disk then its almost certainly right the HPA should be disabled post > boot. If it exceeds both its a raid 0 volume of some form... So where would you envision this code to check the partition table, the HPA/host default disk size, and guess how things should be set up? >From a userland perspective, it's very difficult to let users know they'll be screwing themselves by partitioning the entire disk, so we really should be leaving HPA enabled if the protected area is indeed not for consumption. Also, the heuristic is harder than this -- if we reexamine the fakeraid case, then it's clear we have to look for raid metadata, figure out if the raid includes stuff inside the HPA or not, and then if it doesn't we don't care -- but that's assuming there _is_ raid metadata. Long term, many people hope, possibly unrealistically, that we'll be able to write out raid metadata for people creating raids on cards which support fakeraid, and have the BIOS grok it appropriately. So in that case, we may well have a blank (or garbage) disk, and we can't check the partition table or any raid metadata. Correct me if I'm wrong, but I don't see a simple heuristic for this case. (as a side note, I know one user who, at OLS, noticed we fail to re-initialize HPA after unsuspend, so on at least t40 the disk gets smaller when you suspend. This may or may not be fixed, I haven't checked. But it's one more sort of pain we get into by disabling it, whether justified or not.) I think if we go the heuristic route, then the *safest* option is to leave it enabled by default and let userland installers/initrd do fixups by telling the kernel to change the state. That way it can keep it correct on suspend/unsuspend, but we don't give users the opportunity to really screw themselves unless they try pretty hard, and we don't actually screw the users unless we simply get the heuristic totally wrong. It's also consistent with the more general policy of leaving policy up to the userland. But your point about how I should send you a patch certainly still applies. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support (v0.4.5)
Konrad Rzeszutek wrote: iBFT is not platform-independent; it only makes sense on platforms with ACPI (and even then, just barely; ACPI is a poor fit for it and it was probably "integrated" with ACPI for political reasons.) The spec just mentions that iBFT table has to be "compatible with an ACPI table format" and nothing else. Well, that's not quite accurate. It also mentions that OEM IDs for vendors come from the ACPI SIG, and they also reserved the "IBFT" signature with the ACPI SIG (it's in ACPI 3.0). It's also pretty clear that the "Locating the iBFT" section in the spec used to be a list with more than one entry and has been edited down to one. That being said, I don't think there's any reason to expect the table to show up on anything but i386 and x86_64, and maybe ia64. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Asynchronous scsi scanning
Dave Jones wrote: On Thu, May 17, 2007 at 03:30:43PM -0600, Matthew Wilcox wrote: > On Thu, May 17, 2007 at 03:43:26PM -0400, Benjamin LaHaise wrote: > > On Thu, May 17, 2007 at 01:39:54PM -0600, Matthew Wilcox wrote: > > > On Fri, May 18, 2007 at 12:34:40AM +0530, Satyam Sharma wrote: > > > > Hmmm, actually those other users could easily write and maintain > > > > a 20-line patch that does the wait for async scans thing for them > > > > using /proc/scsi/scsi in any case. This is one of the things we currently do. There are problems with it, not the least of which being that it's hard to know how long to wait, and waiting excessively long generates user complaints. > > > How about the three users who're bothered by this extra module being > > > built maintain a one-line patch to Kconfig and leave well enough alone? (I assume this is about scsi_wait_scan) > > The module has an added bonus that it doesn't require any new tools to > > make work. Doing it via sysfs/procfs means a new rev of whatever tool > > generates the boot initrd, plus fixing up boot scripts. Loading a module > > can be done via a simple option to the existing boot tools. This isn't really true -- loading the module requires that a user is actually running the tools and knows to do it, which is rarely (and ideally never) the case. And frankly, every single one of our users would have to do it. So really, either way means we need to update the tools. It also doesn't really solve the problem -- when I insert "usb-storage", the SCSI scan may still finish while we're still enumerating the bus for USB devices. (I'd be willing to believe I'm wrong about this specific example, but I suspect the principle still applies for some other driver.) In practice, we wind up doing the compare/timeout loop as on /proc/scsi/scsi, but on /proc/bus/usb/devices or /sys/bus/ieee1394/drivers/sbp2 instead. > That was what James and I thought. However, the distros seem unhappy > with it. Of course, they won't actually *comment* on it, they just > disable the async scan and won't talk about why. FWIW, Fedora 7 has it enabled, and afaik, Peter (mkinitrd maintainer) is happy with the current situation. It's my understanding that the latest ubuntu release has it enabled too, though obviously I can't speak for whether or not they're happy with the status quo. I wouldn't say I'm *happy* with the current situation, but we're to the point where it works for most users. At the same time, we're moving towards polling on the hotplug socket, waiting for specific devices to appear from which to build and mount "/". That should obviate the need for much of this in most cases. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Asynchronous scsi scanning
Matthew Wilcox wrote:
On Tue, May 15, 2007 at 12:26:29PM +0100, Simon Arlott wrote:
I've already suggested a sysfs attribute - or something equivalent - would
be much better. It's just one function that a user might want to run multiple
times (e.g. after adding scsi devices?) - why should loading a module be used
for this?
It's easy to suggest a sysfs attribute. What you've failed to do is
suggest the pathname of the sysfs attribute, the contents of it, or the
semantics of it (read-only? read-write? write-only? blocking?)
My personal favourite would be to add a new verb to /proc/scsi/scsi, but
James dislikes that idea.
I'd *really* like to hear from distro people. What is the most
convenient way for you to implement "load all the scsi modules, then
wait until all devices are found"? James and I had thought that loading
a new module would be the easiest way for you, but it seems inconvenient
for you.
Basically we're going to wind up listening for SUBSYSTEM=block hotplug
events. What would be helpful is if
/sys/block/$foo/device/{model,vendor} were populated when we get the
event (or, since there are good reasons why they're not, if we got
another event afterwards that said they'd been populated).
Even more helpful if there were also sysfs attributes for serial numbers
and such from INQUIRY VPD data. That would make probing for the
*correct* drive(s) much simpler.
--
Peter
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.22-rc4-mm1
Kay Sievers wrote: On Thu, 2007-06-07 at 11:41 +0200, Michal Piotrowski wrote: Kay Sievers pisze: On Thu, 2007-06-07 at 10:40 +0200, Michal Piotrowski wrote: Kay Sievers pisze: On Wed, 2007-06-06 at 20:48 +0200, Michal Piotrowski wrote: Andrew Morton pisze: ftp://ftp.kernel.org/pub/linux/kernel/people/akpm/patches/2.6/2.6.22-rc4/2.6.22-rc4-mm1/ Kay, your patch gregkh-driver-block-device.patch breaks Fedora 7 initrd http://www.stardust.webpages.pl/files/tbf/bitis-gabonica/2.6.22-rc4-mm1/initrd.jpg Please fix it ASAP, I can't test kernel... Do you have CONFIG_SYSFS_DEPRECATED set or unset? Kay cat ../linux-mm-bo/.config | grep CONFIG_SYSFS_DEPRECATED # CONFIG_SYSFS_DEPRECATED is not set Oh, could you possibly try (with the block patch included) setting it to yes and see if it works? That would help to find what's going wrong. I enabled CONFIG_SYSFS_DEPRECATED and 2.6.22-rc4-mm2 boots fine. Michal, thanks a lot for the testing. Peter, looking at your mkinitrd code, it works only with CONFIG_SYSFS_DEPRECATED enabled when block devices are converted to class devices. Any chance to replace lstat() with stat() while looking for devices in sysfs? Remember, _anything_ in sysfs can be symlink or a directory, you can't assume one or the other. When things change internally in the kernel, we can often provide symlinks for backwards compatibility, but lstat() obviously can't works here. Fixed in mkinitrd-6.0.9-6 , which I'll build now and push to updates-testing. Thanks for getting my attention here. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.22-rc4-mm1
Greg KH wrote: On Thu, Jun 07, 2007 at 02:48:05PM -0400, Bill Nottingham wrote: Greg KH ([EMAIL PROTECTED]) said: There are a number of distros out there right now who can support that option disabled. I'm pretty sure they are the following right now: Gentoo unstable (actually stable works now for me, but I'm not going to guarantee it just yet...) Debian unstable (again stable might work, but I have not heard any reports from testers.) openSUSE FACTORY (the 10.3 alpha releases) and now it sounds like Red Hat rawhide will also work properly. I suspect while it will work for mkinitrd, the installer will not work properly. Ah, but the installer doesn't matter until you do a Fedora 8 release, right? This is just for people who want to run their own kernel on an already-set-up Fedora machine. Eh, not so sure that's true. It's really kudzu rather than the installer itself, and that's used by, for example, system-config-network. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: 2.6.22-rc4-mm1
Kay Sievers wrote: Peter, any idea what it could be, that goes wrong on Andrew's box, or how to look for what exactly is going wrong? http://userweb.kernel.org/~akpm/s5000552.jpg Seems that yellowdog uses RH's nash. Yeah, but a /really/ old version -- 4.2.11 is from May 2005 :/ . I feel I should point out that that version IS using udev, but just like mkinitrd and nash, the udev there is probably just as old. At the same time, that does make it an excellent test case for this sort of situation. Andrew, is there any chance you can put that initrd.img somewhere that I can download it? That'd really help me to figure out what's going on, as the minutia of our initrds from 2005 have faded a bit in my memory. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] ACPI: don't show an error when we're not in charge of PCIe hotplug.
(Sorry for the slow response - it's deadline time over here.) On Thu, Jun 16, 2016 at 04:56:57PM +0200, Rafael J. Wysocki wrote: > On Thu, Jun 16, 2016 at 2:12 AM, Rafael J. Wysocki wrote: > > On Thu, Jun 16, 2016 at 12:15 AM, Peter Jones wrote: > >> Right now when booting, on many laptops the firmware manages the PCIe > >> bus. As a result, when we call the _OSC ACPI method, it returns an > >> error code. Unfortunately the errors are not very articulate. > > > > What exactly do you mean here? > > > >> As a result, we show: > >> > >> ACPI: PCI Root Bridge [PCI0] (domain [bus 00-fe]) > >> acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments > >> MSI] > >> \_SB_.PCI0 (33DB4D5B-1FF7-401C-9657-7441C03DD766): _OSC invalid UUID > >> _OSC request data: 1 1f 0 > > > > So _OSC told us that the UUID was invalid, didn't it? > > BTW, the above messages are KERN_DEBUG, so at least in theory they > shouldn't be visible in production runs. > > Maybe the bug to fix is that they show up when they aren't supposed to? No - the workflow that I am really trying to remedy is this: 1) S3 resume sometimes isn't working on some laptop you've got. 2) start looking at debug messages 3) this shows an error, so it looks like it's probably the problem 4) go fishing for red herring 5) if you happen to know who maintains the DSDT for the platform in question, eventually work out that this is working as intended and the bug is someplace else. 5b) if you don't know that person, eventually work out that it /might/ be someplace else... So the idea was to make it look more like an indication of status, and less like an error that's causing unrelated problems. When I talked to Mario at Dell (Cc'd), it wasn't clear to us that there's a way to distinguish the between the UUID being invalid/malformed, being merely unsupported, or being supported in some configurations but not the current one. In this particular DSDT, the machine doesn't support the OS controlling any of this if USB-C / thunderbolt are enabled. The DSDT is clearly written with the belief that you have to completely disable the handling for that UUID in this case, and googling for this looks like it's not the only one written with that belief. Reading the spec (v6.1, sections 6.2.11.3 and 6.2.11.4), it seems plausible that you can express this instead by handling the UUID but choosing each individual query/status bit in the way that accomplishes the OS doing nothing with the response. So it may well be that that's just more code that vendors have thought wasn't necessary (or wasn't correct for some reason.) Mario, want to jump in on your thinking here? -- Peter
Re: [PATCH] drivers/firmware: make efi/esrt.c driver explicitly non-modular
On Wed, Aug 26, 2015 at 06:11:28PM +0100, Matt Fleming wrote: > On Tue, 25 Aug, at 07:00:48PM, Paul Gortmaker wrote: > > The Kconfig for this driver is currently hidden with: > > > > config EFI_ESRT > > bool > > > > ...meaning that it currently is not being built as a module by anyone. > > Lets remove the modular code that is essentially orphaned, so that > > when reading the driver there is no doubt it is builtin-only. > > > > Since module_init translates to device_initcall in the non-modular > > case, the init ordering remains unchanged with this commit. > > > > We leave some tags like MODULE_AUTHOR for documentation purposes. > > > > We don't replace module.h with init.h since the file already has that. > > > > Cc: Matt Fleming > > Cc: Peter Jones > > Cc: linux-...@vger.kernel.org > > Signed-off-by: Paul Gortmaker > > Looks good to me. I know Peter is on vacation right now, so I'm still > expecting a response from him. In the meantime, I'll queue this up in > the EFI 'next' branch. Thanks! Looks fine to me as well. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] efifb: Add support for 64-bit frame buffer addresses
On Fri, Aug 28, 2015 at 01:12:19PM +0100, Matt Fleming wrote: > From: Matt Fleming > > The EFI Graphics Output Protocol uses 64-bit frame buffer addresses > but these get truncated to 32-bit by the EFI boot stub when storing > the address in the 'lfb_base' field of 'struct screen_info'. > > Add a 'ext_lfb_base' field for the upper 32-bits of the frame buffer > address and set VIDEO_TYPE_CAPABILITY_64BIT_BASE when the field is > useable. > > It turns out that the reason no one has required this support so far > is that there's actually code in tianocore to "downgrade" PCI > resources that have option ROMs and 64-bit BARS from 64-bit to 32-bit > to cope with legacy option ROMs that can't handle 64-bit addresses. > The upshot is that basically all GOP devices in the wild use a 32-bit > frame buffer address. > > Still, it is possible to build firmware that uses a full 64-bit GOP > frame buffer address. Chad did, which led to him reporting this issue. > > Add support in anticipation of GOP devices using 64-bit addresses more > widely, and so that efifb works out of the box when that happens. > > Reported-by: Chad Page > Cc: Pete Hawkins > Cc: Peter Jones > Cc: Matthew Garrett > Signed-off-by: Matt Fleming Looks good to me. Acked-by: Peter Jones -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] drivers/firmware: make efi/esrt.c driver explicitly non-modular
On Tue, Sep 01, 2015 at 09:57:08AM +0100, Matt Fleming wrote: > On Mon, 31 Aug, at 09:52:02AM, Peter Jones wrote: > > On Wed, Aug 26, 2015 at 06:11:28PM +0100, Matt Fleming wrote: > > > > > > Looks good to me. I know Peter is on vacation right now, so I'm still > > > expecting a response from him. In the meantime, I'll queue this up in > > > the EFI 'next' branch. Thanks! > > > > Looks fine to me as well. > > Thanks Peter. May I add your ACK? Yes, sorry: Acked-by: Peter Jones -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] Add support for EDD4 fields and types.
Back in 2010, t13 EDD version 4 added a couple of storage types, some
host bridge types (that are pretty much all represented identically),
and a couple of fields on existing storage types.
This change makes the driver expose those to userland.
Signed-off-by: Peter Jones
---
drivers/firmware/edd.c | 15 ++-
include/uapi/linux/edd.h | 6 +-
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/drivers/firmware/edd.c b/drivers/firmware/edd.c
index e229576..5092b92 100644
--- a/drivers/firmware/edd.c
+++ b/drivers/firmware/edd.c
@@ -152,15 +152,15 @@ edd_show_host_bus(struct edd_device *edev, char *buf)
info->params.interface_path.isa.base_address);
} else if (!strncmp(info->params.host_bus_type, "PCIX", 4) ||
!strncmp(info->params.host_bus_type, "PCI", 3) ||
- !strncmp(info->params.host_bus_type, "XPRS", 4)) {
+ !strncmp(info->params.host_bus_type, "XPRS", 4) ||
+ !strncmp(info->params.host_bus_type, "HTPT", 4)) {
p += scnprintf(p, left,
"\t%02x:%02x.%d channel: %u\n",
info->params.interface_path.pci.bus,
info->params.interface_path.pci.slot,
info->params.interface_path.pci.function,
info->params.interface_path.pci.channel);
- } else if (!strncmp(info->params.host_bus_type, "IBND", 4) ||
- !strncmp(info->params.host_bus_type, "HTPT", 4)) {
+ } else if (!strncmp(info->params.host_bus_type, "IBND", 4)) {
p += scnprintf(p, left,
"\tTBD: %llx\n",
info->params.interface_path.ibnd.reserved);
@@ -220,8 +220,13 @@ edd_show_interface(struct edd_device *edev, char *buf)
p += scnprintf(p, left, "\tidentity_tag: %x\n",
info->params.device_path.raid.array_number);
} else if (!strncmp(info->params.interface_type, "SATA", 4)) {
- p += scnprintf(p, left, "\tdevice: %u\n",
-info->params.device_path.sata.device);
+ p += scnprintf(p, left, "\tdevice: %u pmp: %u\n",
+info->params.device_path.sata.device,
+info->params.device_path.sata.pmp);
+ } else if (!strncmp(info->params.interface_type, "SAS", 4)) {
+ p += scnprintf(p, left, "\taddress: %llx lun: %llx\n",
+ info->params.device_path.sas.address,
+ info->params.device_path.sas.lun);
} else {
p += scnprintf(p, left, "\tunknown: %llx %llx\n",
info->params.device_path.unknown.reserved1,
diff --git a/include/uapi/linux/edd.h b/include/uapi/linux/edd.h
index 89240a0..aff4573 100644
--- a/include/uapi/linux/edd.h
+++ b/include/uapi/linux/edd.h
@@ -155,12 +155,16 @@ struct edd_device_params {
} __attribute__ ((packed)) raid;
struct {
__u8 device;
- __u8 reserved1;
+ __u8 pmp;
__u16 reserved2;
__u32 reserved3;
__u64 reserved4;
} __attribute__ ((packed)) sata;
struct {
+ __u64 address;
+ __u64 lun;
+ } __attribute__ ((packed)) sas;
+ struct {
__u64 reserved1;
__u64 reserved2;
} __attribute__ ((packed)) unknown;
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Thu, Apr 05, 2018 at 07:43:49AM +0200, Lukas Wunner wrote: > On Wed, Apr 04, 2018 at 01:18:36PM -0400, Peter Jones wrote: > > > On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: > > > > * Add the EFI Firmware Volume Protocol to include/linux/efi.h: > > > > > > > > https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf > > > > > > > > * Amend arch/x86/boot/compressed/eboot.c to read the files with the > > > > GUIDs you're interested in into memory and pass the files to the > > > > kernel as setup_data payloads. > > > > To be honest, I'm a bit skeptical about the firmware volume approach. > > Tools like UEFITool[0] and uefi-firmware-parser[1] have existed for > > years, still don't seem to reliably parse firmware images I see in the > > wild, and have a fairly regular need for fixes. These are tools > > maintained by smart people who are making a real effort, and it still > > looks pretty hard to do a good job that applies across a lot of > > platforms. > > > > So I'd rather use Hans's existing patches, at least for now, and if > > someone is interested in hacking on making an efi firmware volume parser > > for the kernel, switch them to that when such a thing is ready. > > Hello? As I've written in the above-quoted e-mail the kernel should > read the files using EFI_FIRMWARE_VOLUME_PROTOCOL.ReadFile(). > > *Not* by parsing the firmware volume! Okay, that's a fair point that I'd missed reading your first mail. It's worth a shot. That said, EFI_FIRMWARE_VOLUME_PROTOCOL is part of the PI spec, not the UEFI spec. It's not required in order to implement UEFI, and some implementations don't use it. Even when the implementation does include PI, there's no guarantee PI protocols are exposed after the "End of DXE" event (though edk2 will expose this, I think). > Parsing the firmware volume is only necessary to find out the GUIDs > of the files you're looking for. You only do that *once*. > > I habitually extract Apple's EFI Firmware Volumes and found the > existing tools always did a sufficiently good job to get the > information I need (such as UEFIExtract, which I've linked to, > and which is part of the UEFITool suite, which you've linked to > once more). Yeah, it's probably worth implementing your way and using it when we can. > On Wed, Apr 04, 2018 at 10:25:05PM +0200, Hans de Goede wrote: > > Lukas, thank you for your suggestions on this, but I doubt that these > > devices use the Firmware Volume stuff. > > If they're using EFI, they're using a Firmware Volume and you should > be able to use the Firmware Volume Protocol to read files from it. This is not actually the case. There is more than one implementation of UEFI, and they do not all implement PEI/DXE/etc from the PI spec. For two examples, there are still vendors that ship EDK-I implementations on some platforms, and current u-boot can be built to export a UEFI API. (It's a subset, but so is every other implementation.) > If that protocol wasn't supported, the existing EFI drivers wouldn't > be able to function as they couldn't load files from the volume. This is not correct. Not all UEFI implementations implement *any* of PI. Also, AFAIK, nothing we use in Linux so far directly depends on anything in PI. > > What you are describing sounds like significantly more work then > > the vendor just embedding the firmware as a char firmware[] in their > > EFI mouse driver. > > In such cases, read the EFI mouse driver from the firmware volume and > extract the firmware from the offset you've pre-determined. That's > never going to change since the devices never receive updates, as > you're saying. So basically you'll have a struct with GUID, offset, > length, and the name under which the firmware is made available to > Linux drivers. No, he's saying that it's really easy to implement a driver with the device firmware in a char [] in the code, so cheap ODMs who supply a UEFI driver with their hardware part are very, very likely to have done that instead of providing two things to go into the FV - a UEFI driver *and* a separate image for the device firmware. This also cuts out a point of failure between the OEM and the ODM. > > That combined with Peter's worries about difficulties parsing the > > Firmware Volume stuff, makes me believe that it is best to just > > stick with my current approach as Peter suggests. > > I don't know why you insist on a hackish solution (your own words) > even though a cleaner solution is suggested, but fortunately it's > not for me to decide what goes in and what doesn't. ;-) It already works... -- Peter
Re: [PATCH] efi/fb: Convert PCI bus address to resource if translated by the bridge
On Thu, May 17, 2018 at 09:22:23AM -0400, Sinan Kaya wrote:
> A host bridge is allowed to remap BAR addresses using _TRA attribute in
> _CRS windows.
>
> pci_bus :00: root bus resource [mem 0x8010010-0x8011fff window]
> (bus address [0x0010-0x1fff])
> pci :02:00.0: reg 0x10: [mem 0x8011e00-0x8011eff]
>
> When a VGA device is behind such a host bridge and the resource is
> translated efifb driver is trying to do ioremap against bus address
> rather than the resource address and is failing to probe.
>
> efifb driver is having difficulty locating the base address from BAR
> address when
>
> efifb: probing for efifb
> efifb: cannot reserve video memory at 0x1e00
> efifb: framebuffer at 0x1e00, using 1920k, total 1875k
> efifb: mode is 800x600x32, linelength=3200, pages=1
> efifb: scrolling: redraw
> efifb: Truecolor: size=8:8:8:8, shift=24:16:8:0
>
> Use the host bridge offset information to convert bus address to
> resource address in the fixup.
>
> Signed-off-by: Sinan Kaya
Looks reasonable to me - Ard, do you want to take this up through the
EFI tree?
Signed-off-by: Peter Jones
> ---
> drivers/video/fbdev/efifb.c | 10 ++
> 1 file changed, 10 insertions(+)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 46a4484..ea68d5c 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -428,6 +428,8 @@ static void efifb_fixup_resources(struct pci_dev *dev)
> {
> u64 base = screen_info.lfb_base;
> u64 size = screen_info.lfb_size;
> + struct pci_bus_region region;
> + struct resource res;
> int i;
>
> if (efifb_pci_dev || screen_info.orig_video_isVGA != VIDEO_TYPE_EFI)
> @@ -439,6 +441,14 @@ static void efifb_fixup_resources(struct pci_dev *dev)
> if (!base)
> return;
>
> + region.start = base;
> + region.end = base + size - 1;
> + res.start = 0;
> + res.flags = IORESOURCE_MEM;
> + pcibios_bus_to_resource(dev->bus, &res, ®ion);
> + if (res.start)
> + base = res.start;
> +
> for (i = 0; i <= PCI_STD_RESOURCE_END; i++) {
> struct resource *res = &dev->resource[i];
>
> --
> 2.7.4
>
--
Peter
Re: [PATCH] Add iSCSI iBFT support.
H. Peter Anvin wrote: Konrad Rzeszutek wrote: +config ISCSI_IBFT + tristate "iSCSI Boot Firmware Table Attributes" + depends on X86 why only on X86? PowerPC exports this data via the OpenFirmware so it already shows in the /sysfs entries. I was thinking to combine those sysfs entries under this code, but that is something in the future. In regards to all other platforms, I figured I would only make it supported under platforms that have been tested. Is there anything that stops this from working for example of IA64? Well no. The hardware that supports the iBFT is either in the BIOS or in NICs - so the SGI or HP boxes _should_ work, however I am not comfortable to make it supported unless I've tested it. It should, presumably, depend on ACPI, rather than on X86...? Actually no. That /should/ be the correct answer, but none of the hardware vendors actually provide the table via ACPI yet. Also, if they did, the support for /sys/firmware/acpi/tables/* would be sufficient instead of having this code *at all*. -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] Add iSCSI iBFT support.
H. Peter Anvin wrote: Peter Jones wrote: It should, presumably, depend on ACPI, rather than on X86...? Actually no. That /should/ be the correct answer, but none of the hardware vendors actually provide the table via ACPI yet. Also, if they did, the support for /sys/firmware/acpi/tables/* would be sufficient instead of having this code *at all*. Is there anything other than the discovery which is braindead about iBFT? If so, can the tables code be taught to look for this additional table instead of having all its own mechanism? Well, the code for the the generic ACPI table sysfs functionality is expecting to find the tables indexed in the RSDT. This is essentially what the iBFT spec's authors seem to have planned, but it's simply never been implemented in the firmware. AFAICS, it's technically feasible to remove the sysfs parts of this code entirely, make the probe code build a fake ACPI table header, and then add it explicitly if present at the end of acpi_system_sysfs_init() . I don't know how the ACPI guys would feel about that. Len, thoughts? -- Peter - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v4 2/2] efi: an sysfs interface for user to update efi firmware
On Tue, Apr 21, 2015 at 06:58:58PM -0700, Andy Lutomirski wrote:
> On Tue, Apr 21, 2015 at 6:21 PM, James Bottomley
> wrote:
> > Andy, just on the misc device idea, what about triggering the capsule
> > update from close()? In theory close returns an error code (not sure if
> > most tools actually check this, though). That means we can do the write
> > in chunks but pass it in atomically on the close and cat will work
> > (provided it checks the return code of close).
>
> I thought about this but IIRC cat doesn't check the return value from close.
I checked this for the use case we'd talked about before - gnu cat
/does/ check the error code, but it's easy to miss how, because
coreutils code has some good ole' gnu-code complexity. It'll print the
strerror() representation, but it always exits with 1 as the error
code.
Specifically the close on the output is handled by this:
---
initialize_main (&argc, &argv);
set_program_name (argv[0]);
setlocale (LC_ALL, "");
bindtextdomain (PACKAGE, LOCALEDIR);
textdomain (PACKAGE);
/* Arrange to close stdout if we exit via the
case_GETOPT_HELP_CHAR or case_GETOPT_VERSION_CHAR code.
Normally STDOUT_FILENO is used rather than stdout, so
close_stdout does nothing. */
atexit (close_stdout);
/* Parse command line options. */
while ((c = getopt_long (argc, argv, "benstuvAET", long_options, NULL))
---
Which in turn does:
---
void
close_stdout (void)
{
if (close_stream (stdout) != 0
&& !(ignore_EPIPE && errno == EPIPE))
{
char const *write_error = _("write error");
if (file_name)
error (0, errno, "%s: %s", quotearg_colon (file_name),
write_error);
else
error (0, errno, "%s", write_error);
_exit (exit_failure);
}
if (close_stream (stderr) != 0)
_exit (exit_failure);
}
---
exit_failure is a global from libcoreutils.a which cat never changes
from the default, so it's always 1.
(And of course error() is coreutils' own implementation rather than
glibc's because hey maybe you're not using glibc, but still, it's
there.)
So it's /annoying/ to propagate the error from there programatically,
but it can work.
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] efi: Work around ia64 build problem with ESRT driver.
So, I'm told this problem exists in the world:
--
Subject: Build error in -next due to 'efi: Add esrt support'
Building ia64:defconfig ... failed
--
Error log:
drivers/firmware/efi/esrt.c:28:31: fatal error: asm/early_ioremap.h: No
such file or directory
--
I'm not really sure how it's okay that we have things in asm-generic on
some platforms but not others - is having it the same everywhere not the
whole point of asm-generic?
That said, ia64 doesn't have early-ioremap.h . So instead, since it's
difficult to imagine new IA64 machines with UEFI 2.5, just don't build
this code there.
To me this looks like a workaround - doing something like:
generic-y += early_ioremap.h
in arch/ia64/include/asm/Kbuild would appear to be more correct, but
ia64 has its own early_memremap() decl in arch/ia64/include/asm/io.h ,
and it's a macro. So adding the above /and/ requiring that asm/io.h be
included /after/ asm/early_ioremap.h in all cases would fix it, but
that's pretty ugly as well. Since I'm not going to spend the rest of my
life rectifying ia64 headers vs "generic" headers that aren't generic,
it's much simpler to just not build there.
Note that I've only actually tried to build this patch on x86_64, but
esrt.o still gets built there, and that would seem to demonstrate that
the conditional building is working correctly at all the places the code
built before. I no longer have any ia64 machines handy to test that the
exclusion actually works there.
Signed-off-by: Peter Jones
---
drivers/firmware/efi/Makefile | 5 -
include/linux/efi.h | 4
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 26eabbc..81c8527 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -1,7 +1,10 @@
#
# Makefile for linux kernel
#
-obj-$(CONFIG_EFI) += efi.o esrt.o vars.o reboot.o
+obj-$(CONFIG_EFI) += efi.o vars.o reboot.o
+ifeq ($(CONFIG_IA64),)
+obj-$(CONFIG_EFI) += esrt.o
+endif
obj-$(CONFIG_EFI_VARS) += efivars.o
obj-$(CONFIG_EFI_VARS_PSTORE) += efi-pstore.o
obj-$(CONFIG_UEFI_CPER)+= cper.o
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 024c27e..1983d17 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -879,7 +879,11 @@ static inline efi_status_t efi_query_variable_store(u32
attributes, unsigned lon
#endif
extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr);
extern int efi_config_init(efi_config_table_type_t *arch_tables);
+#ifdef CONFIG_IA64
+static inline void efi_esrt_init(void) { }
+#else
extern void __init efi_esrt_init(void);
+#endif
extern int efi_config_parse_tables(void *config_tables, int count, int sz,
efi_config_table_type_t *arch_tables);
extern u64 efi_get_iobase (void);
--
2.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] efi: Work around ia64 build problem with ESRT driver.
So, I'm told this problem exists in the world:
--
Subject: Build error in -next due to 'efi: Add esrt support'
Building ia64:defconfig ... failed
--
Error log:
drivers/firmware/efi/esrt.c:28:31: fatal error: asm/early_ioremap.h: No
such file or directory
--
I'm not really sure how it's okay that we have things in asm-generic on
some platforms but not others - is having it the same everywhere not the
whole point of asm-generic?
That said, ia64 doesn't have early-ioremap.h . So instead, since it's
difficult to imagine new IA64 machines with UEFI 2.5, just don't build
this code there.
To me this looks like a workaround - doing something like:
generic-y += early_ioremap.h
in arch/ia64/include/asm/Kbuild would appear to be more correct, but
ia64 has its own early_memremap() decl in arch/ia64/include/asm/io.h ,
and it's a macro. So adding the above /and/ requiring that asm/io.h be
included /after/ asm/early_ioremap.h in all cases would fix it, but
that's pretty ugly as well. Since I'm not going to spend the rest of my
life rectifying ia64 headers vs "generic" headers that aren't generic,
it's much simpler to just not build there.
Note that I've only actually tried to build this patch on x86_64, but
esrt.o still gets built there, and that would seem to demonstrate that
the conditional building is working correctly at all the places the code
built before. I no longer have any ia64 machines handy to test that the
exclusion actually works there.
Signed-off-by: Peter Jones
Acked-by: Tony Luck
---
drivers/firmware/efi/Kconfig | 5 +
drivers/firmware/efi/Makefile | 3 ++-
include/linux/efi.h | 4
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 8de4da5..2272ff0 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -18,6 +18,11 @@ config EFI_VARS
Subsequent efibootmgr releases may be found at:
<http://github.com/vathpela/efibootmgr>
+config EFI_ESRT
+ bool
+ depends on EFI && !IA64
+ default true
+
config EFI_VARS_PSTORE
tristate "Register efivars backend for pstore"
depends on EFI_VARS && PSTORE
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 26eabbc..6fd3da9 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -1,8 +1,9 @@
#
# Makefile for linux kernel
#
-obj-$(CONFIG_EFI) += efi.o esrt.o vars.o reboot.o
+obj-$(CONFIG_EFI) += efi.o vars.o reboot.o
obj-$(CONFIG_EFI_VARS) += efivars.o
+obj-$(CONFIG_EFI_ESRT) += esrt.o
obj-$(CONFIG_EFI_VARS_PSTORE) += efi-pstore.o
obj-$(CONFIG_UEFI_CPER)+= cper.o
obj-$(CONFIG_EFI_RUNTIME_MAP) += runtime-map.o
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 024c27e..2092965 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -879,7 +879,11 @@ static inline efi_status_t efi_query_variable_store(u32
attributes, unsigned lon
#endif
extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr);
extern int efi_config_init(efi_config_table_type_t *arch_tables);
+#ifdef CONFIG_EFI_ESRT
extern void __init efi_esrt_init(void);
+#else
+static inline void efi_esrt_init(void) { }
+#endif
extern int efi_config_parse_tables(void *config_tables, int count, int sz,
efi_config_table_type_t *arch_tables);
extern u64 efi_get_iobase (void);
--
2.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] efi: Work around ia64 build problem with ESRT driver.
On Fri, Jun 05, 2015 at 02:54:35PM -0400, Peter Jones wrote: > Note that I've only actually tried to build this patch on x86_64, but > esrt.o still gets built there, and that would seem to demonstrate that > the conditional building is working correctly at all the places the code > built before. I no longer have any ia64 machines handy to test that the > exclusion actually works there. This, of course, is a total lie. I'll send a reply patch in which it's actually true. > Acked-by: Tony Luck So sorry, Tony. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] efi: Work around ia64 build problem with ESRT driver.
So, I'm told this problem exists in the world:
--
Subject: Build error in -next due to 'efi: Add esrt support'
Building ia64:defconfig ... failed
--
Error log:
drivers/firmware/efi/esrt.c:28:31: fatal error: asm/early_ioremap.h: No
such file or directory
--
I'm not really sure how it's okay that we have things in asm-generic on
some platforms but not others - is having it the same everywhere not the
whole point of asm-generic?
That said, ia64 doesn't have early-ioremap.h . So instead, since it's
difficult to imagine new IA64 machines with UEFI 2.5, just don't build
this code there.
To me this looks like a workaround - doing something like:
generic-y += early_ioremap.h
in arch/ia64/include/asm/Kbuild would appear to be more correct, but
ia64 has its own early_memremap() decl in arch/ia64/include/asm/io.h ,
and it's a macro. So adding the above /and/ requiring that asm/io.h be
included /after/ asm/early_ioremap.h in all cases would fix it, but
that's pretty ugly as well. Since I'm not going to spend the rest of my
life rectifying ia64 headers vs "generic" headers that aren't generic,
it's much simpler to just not build there.
Note that I've only actually tried to build this patch on x86_64, but
esrt.o still gets built there, and that would seem to demonstrate that
the conditional building is working correctly at all the places the code
built before. I no longer have any ia64 machines handy to test that the
exclusion actually works there.
Signed-off-by: Peter Jones
Acked-by: Tony Luck
---
drivers/firmware/efi/Kconfig | 5 +
drivers/firmware/efi/Makefile | 3 ++-
include/linux/efi.h | 4
3 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
index 8de4da5..54071c1 100644
--- a/drivers/firmware/efi/Kconfig
+++ b/drivers/firmware/efi/Kconfig
@@ -18,6 +18,11 @@ config EFI_VARS
Subsequent efibootmgr releases may be found at:
<http://github.com/vathpela/efibootmgr>
+config EFI_ESRT
+ bool
+ depends on EFI && !IA64
+ default y
+
config EFI_VARS_PSTORE
tristate "Register efivars backend for pstore"
depends on EFI_VARS && PSTORE
diff --git a/drivers/firmware/efi/Makefile b/drivers/firmware/efi/Makefile
index 26eabbc..6fd3da9 100644
--- a/drivers/firmware/efi/Makefile
+++ b/drivers/firmware/efi/Makefile
@@ -1,8 +1,9 @@
#
# Makefile for linux kernel
#
-obj-$(CONFIG_EFI) += efi.o esrt.o vars.o reboot.o
+obj-$(CONFIG_EFI) += efi.o vars.o reboot.o
obj-$(CONFIG_EFI_VARS) += efivars.o
+obj-$(CONFIG_EFI_ESRT) += esrt.o
obj-$(CONFIG_EFI_VARS_PSTORE) += efi-pstore.o
obj-$(CONFIG_UEFI_CPER)+= cper.o
obj-$(CONFIG_EFI_RUNTIME_MAP) += runtime-map.o
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 024c27e..2092965 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -879,7 +879,11 @@ static inline efi_status_t efi_query_variable_store(u32
attributes, unsigned lon
#endif
extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr);
extern int efi_config_init(efi_config_table_type_t *arch_tables);
+#ifdef CONFIG_EFI_ESRT
extern void __init efi_esrt_init(void);
+#else
+static inline void efi_esrt_init(void) { }
+#endif
extern int efi_config_parse_tables(void *config_tables, int count, int sz,
efi_config_table_type_t *arch_tables);
extern u64 efi_get_iobase (void);
--
2.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [GIT PULL] EFI changes for v4.2
On Tue, Jun 02, 2015 at 08:45:57AM +0200, Ingo Molnar wrote:
> @@ -167,7 +167,6 @@ static struct kset *esrt_kset;
>
> static int esre_create_sysfs_entry(void *esre, int entry_num)
> {
> - int rc = 0;
> struct esre_entry *entry;
> char name[20];
>
> @@ -180,13 +179,15 @@ static int esre_create_sysfs_entry(void *esre, int
> entry_num)
> entry->kobj.kset = esrt_kset;
>
> if (esrt->fw_resource_version == 1) {
> + int rc = 0;
> +
> entry->esre.esre1 = esre;
> rc = kobject_init_and_add(&entry->kobj, &esre1_ktype, NULL,
> "%s", name);
> - }
> - if (rc) {
> - kfree(entry);
> - return rc;
> + if (rc) {
> + kfree(entry);
> + return rc;
> + }
> }
>
> list_add_tail(&entry->list, &entry_list);
>
> How can a compiler ever have warned about 'rc' being uninitialized? It's
> defined
> straight at function entry, with initialization to 0. It can never be
> uninitialized.
>
> I pulled it, because I agree with the change itself, as it's always better to
> define and use variables in the narrowest scope possible, but I think it's a
> cleanup, not a compiler warning fix.
Well, apparently I failed to explain it well - the warning was about
"esre" rather than "rc". Basically before we were testing the version in
register_entries() (i.e. this function's caller) and never calling the
this function if it's not version 1. The compiler didn't figure out
that when we set "entry->esre.esre1 = esre;", esre can not be null
because the function wouldn't be called. Adding the explicit check
on the version here silenced the warning about entry plausibly being
NULL.
I'm guessing that this is because it's checking that the same
conditional test is involved - that the initialization is in the same
"...version == 1" test that the usage is. But that's just a guess.
Would you like another patch to add this email to the commit message, or
do you want to add it in your tree, or what?
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Sat, Mar 31, 2018 at 02:19:44PM +0200, Hans de Goede wrote:
> diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
> index fddc5f706fd2..1a5ea950f58f 100644
> --- a/drivers/firmware/efi/efi.c
> +++ b/drivers/firmware/efi/efi.c
> @@ -455,6 +455,7 @@ int __init efi_mem_desc_lookup(u64 phys_addr,
> efi_memory_desc_t *out_md)
> u64 end;
>
> if (!(md->attribute & EFI_MEMORY_RUNTIME) &&
> + md->type != EFI_BOOT_SERVICES_CODE &&
> md->type != EFI_BOOT_SERVICES_DATA &&
> md->type != EFI_RUNTIME_SERVICES_DATA) {
> continue;
Might be worth adding a comment here to ensure nobody comes along later
and adds something like EFI_BOOT_LOADER_DATA or other stuff that's
allocated later here. I don't want to accidentally patch our way into
having the ability to stumble across a firmware blob somebody dumped
into the middle of a grub config file, especially since you only need to
collide crc32 (within the same length) to pre-alias a match.
...
> +static int __init efi_check_md_for_embedded_firmware(
> + efi_memory_desc_t *md, const struct embedded_fw_desc *desc)
> +{
...
> + if (found_fw_count >= MAX_EMBEDDED_FIRMWARES) {
> + pr_err("Error already have %d embedded firmwares\n",
> +MAX_EMBEDDED_FIRMWARES);
> + return -ENOSPC;
> + }
Doesn't seem like this needs to be pr_err(); after all we have already
found a valid match, so the firmware vendor has done something
moderately stupid, but we have a firmware that will probably work. Of
course it still needs to return != 0, but pr_warn() or even pr_info()
seems more reasonable.
Aside from those nits, looks good to me.
Reviewed-by: Peter Jones
--
Peter
Re: [PATCH 2/2] efi: Add embedded peripheral firmware support
On Tue, Apr 03, 2018 at 06:58:48PM +, Luis R. Rodriguez wrote: > On Tue, Apr 03, 2018 at 08:07:11PM +0200, Lukas Wunner wrote: > > On Tue, Apr 03, 2018 at 10:33:25AM +0200, Hans de Goede wrote: > > > I asked Peter Jones for suggestions how to extract this during boot and > > > he suggested seeing if there was a copy of the firmware in the > > > EFI_BOOT_SERVICES_CODE memory segment, which it turns out there is. > > > > > > My patch to add support for this contains a table of device-model (dmi > > > strings), firmware header (first 64 bits), length and crc32 and then if > > > we boot on a device-model which is in the table the code scans the > > > EFI_BOOT_SERVICES_CODE for the prefix, if found checks the crc and > > > caches the firmware for later use by request-firmware. > > > > > > So I just do a brute-force search for the firmware, this really is hack, > > > nothing standard about it I'm afraid. But it works on 4 different x86 > > > tablets I have and makes the touchscreen work OOTB on them, so I believe > > > it is a worthwhile hack to have. > > > > The EFI Firmware Volume contains a kind of filesystem with files > > identified by GUIDs. Those files include EFI drivers, ACPI tables, > > DMI data and so on. It is actually quite common for vendors to > > also include device firmware on the Firmware Volume. Apple is doing > > this to ship firmware updates e.g. for the GMUX controller found on > > dual GPU MacBook Pros. If they want to update the controller's > > firmware, they include it in a BIOS update, and an EFI driver checks > > on boot if the firmware update for the controller is necessary and > > if so, flashes it. > > > > The firmware files you're looking for are almost certainly included > > on the Firmware Volume as individual files. > > What Hans implemented seems to have been for a specific x86 hack, best if we > confirm if indeed they are present on the Firmware Volume. To be honest, I'm a bit skeptical about the firmware volume approach. Tools like UEFITool[0] and uefi-firmware-parser[1] have existed for years, still don't seem to reliably parse firmware images I see in the wild, and have a fairly regular need for fixes. These are tools maintained by smart people who are making a real effort, and it still looks pretty hard to do a good job that applies across a lot of platforms. So I'd rather use Hans's existing patches, at least for now, and if someone is interested in hacking on making an efi firmware volume parser for the kernel, switch them to that when such a thing is ready. [0] g...@github.com:LongSoft/UEFITool.git [1] g...@github.com:theopolis/uefi-firmware-parser.git > > Rather than scraping > > the EFI memory for firmware, I think it would be cleaner and more > > elegant if you just retrieve the files you're interested in from > > the Firmware Volume. > > > > We're doing something similar with Apple EFI properties, see > > 58c5475aba67 and c9cc3aaa0281. > > > > Basically what you need to do to implement this approach is: > > > > * Determine the GUIDs used by vendors for the files you're interested > > in. Either dump the Firmware Volume or take an EFI update as > > shipped by the vendor, then feed it to UEFIExtract: > > https://github.com/LongSoft/UEFITool > > > > * Add the EFI Firmware Volume Protocol to include/linux/efi.h: > > > > https://www.intel.com/content/dam/doc/reference-guide/efi-firmware-file-volume-specification.pdf > > > > * Amend arch/x86/boot/compressed/eboot.c to read the files with the > > GUIDs you're interested in into memory and pass the files to the > > kernel as setup_data payloads. > > > > * Once the kernel has booted, make the files you've retrieved > > available to device drivers as firmware blobs. > > Happen to know if devices using Firmware Volumes also sign their firmware > and if hw checks the firmware at load time? It varies on a per-device basis, of course. Most new Intel machines as of Haswell *should* be verifying their system firmware via Boot Guard, which both checks an RSA signature and measures the firmware into the TPM, but as with everything of this nature, there are certainly vendors that screw it up. (I think AMD has something similar, but I'm really not sure.) -- Peter
Re: [GIT PULL] Kernel lockdown for secure boot
On Tue, Apr 03, 2018 at 02:51:23PM -0700, Andy Lutomirski wrote: > On Tue, Apr 3, 2018 at 12:29 PM, Matthew Garrett wrote: > > On Tue, Apr 3, 2018 at 9:46 AM Andy Lutomirski wrote: > >> On Tue, Apr 3, 2018 at 9:29 AM, Matthew Garrett wrote: > >> > A kernel that allows users arbitrary access to ring 0 is just an > >> > overfeatured bootloader. Why would you want secure boot in that case? > > > >> To get a chain of trust. I can provision a system with some public > >> keys, stored in UEFI authenticated variables, such that the system > >> will only boot a signed image. That signed image, can, in turn, load > >> a signed (or hashed or otherwise verfified) kernel and a verified > >> initramfs. The initramfs can run a full system from a verified (using > >> dm-verity or similar) filesystem, for example. Now it's very hard to > >> persistently attack this system. Chromium OS does something very much > >> like this, except that it doesn't use UEFI as far as I know. So does > >> iOS, and so do some Android versions. None of this requires lockdown, > >> or even a separation between usermode and kernelmode, to work > >> correctly. One could even do this on an MMU-less system if one really > >> cared to. More usefully, someone probably has done this using a > >> unikernel. > > > > That's only viable if you're the only person with the ability to sign stuff > > for your machine - the moment there are generic distributions that your > > machine trusts, an attacker can use one as a bootloader to compromise your > > trust chain. > > > If you removed "as a bootloader", then I agree with that sentence. > > Can someone please explain why the UEFI crowd cares so much about "as > a bootloader"? Once I'm able to install an OS (Linux kernel + > bootloader, Windows embedded doodad, OpenBSD, whatever) on your > machine, I can use your peripherals, read your data, write your data, > see your keystrokes, use your network connection, re-flash your BIOS > (at least as well as any OS can), run VMs, and generally own your > system. Somehow you all seem fine with all of this, except that the > fact that I can chainload something else gives UEFI people the > willies. > > Can someone explain why? There's no inherent difference, in terms of the trust chain, between compromising it to use the machine as a toaster or to run a botnet - the trust chain is compromised either way. But you're much more likely to notice if your desktop starts producing bread products than if it hides some malware and keeps on booting, and the second one is much more attractive to attackers anyway. The reason we talk about it as a bootloader is because of the model employed by malware. I'm sure you know that one kind of malware that exists in the wild, a so-called "boot kit", operates by modifying a kernel during load (or on disk before loading) so that it has some malicious payload, like exfiltrating user data or allowing a way to install software that the kernel hides or *whatever*, and incorporating some way to achieve relative persistence on the system - for example hiding the real boot settings and loading a kernel with a different than normal initramfs that loads an exploit before continuing with a normal looking boot. As Kees has pointed out, the lockdown portion of this is about separating uid-0 from ring-0. There are a lot of reasons to want to do that, of course. But the reason Secure Boot exists, and ultimately the reason we started trying to do this, is so you can't build the persistence mechanism for a boot kit by using a trusted kernel to springboard into a modified one, even if it's the same kernel just modified before kexec. If you can do that, you can use that to build the persistence mechanism in a boot kit. That is to say, as a result of the way malware has been written, our way of thinking about it is often that it's a way to build a boot loader for a malicious kernel, so that's how we wind up talking about it. Are we concerned with malware stealing your data? Yes, but Secure Boot is only indirectly about that. It's primarily about denying the malware easy mechanisms to build a persistence mechanism. The uid-0 != ring-0 aspect is useful independent of Secure Boot, but Secure Boot without it falls way short of accomplishing its goal. -- Peter
Re: [PATCH 12/12] EFI: Runtime services virtual mapping
On Sat, Oct 12, 2013 at 10:14:39AM +0800, Dave Young wrote: > CCing Peter Jones .., Peter, any idea about the grub related problem? What grub problem? As Matt was saying, grub2 isn't loading it as EfiBootServicesCode/Data. grub2 is loading it as EfiLoaderData . > > On 10/11/13 at 09:42am, Dave Young wrote: > > Matt, > > > > The kernel I referring is the boot kernel aka the 1st kernel, > > the boot loader is grub2 from Fedora 19. > > > > [sorry for top reply because of using webmail] > > > > > > - Original Message - > > From: "Matt Fleming" > > To: "Dave Young" > > Cc: "Borislav Petkov" , "X86 ML" , "LKML" > > , "Borislav Petkov" , "Matthew > > Garrett" , "H. Peter Anvin" , "James > > Bottomley" , "Vivek Goyal" > > , linux-...@vger.kernel.org, fwts-de...@lists.ubuntu.com > > Sent: Friday, October 11, 2013 6:27:06 PM > > Subject: Re: [PATCH 12/12] EFI: Runtime services virtual mapping > > > > On Fri, 11 Oct, at 02:24:37PM, Dave Young wrote: > > > For the boot efi_reserve_boot_services code, it's mainly for the > > > SetVirtualAddressMap callback use, so boot regions should not be reused > > > before SetVirtualAddressMap, but the overlapping happens before the > > > efi_reserve_boot_services, isn't it a problem? > > > > Hang on, which kernel are you referring to here? The boot kernel or the > > kexec'd kernel? I thought you were saying you noticed the overlap when > > running in the second (kexec'd) kernel? > > > > The only reason that you would see this overlap in the first (boot) > > kernel is if the bootloader messed up and allocated the kernel text as > > EfiBootServicesCode/Data. I'd like to believe no bootloaders are still > > doing that. > > > > -- > > Matt Fleming, Intel Open Source Technology Center > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majord...@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: EFI and multiboot2 devlopment work for Xen
On Mon, Oct 21, 2013 at 02:57:56PM +0200, Daniel Kiper wrote: > Hi, > > During work on multiboot2 protocol support for Xen it was discovered > that memory map passed via relevant tag could not represent wide range > of memory types available on EFI platforms. Additionally, GRUB2 > implementation calls ExitBootServices() on them just before jumping > into loaded image. In this situation loaded system could not clearly > identify reserved memory regions, EFI runtime services regions and others. I think you'll find that many distros are shipping patches to grub2 to add a "linuxefi" command that starts the kernel through its EFISTUB code. You may want to look in to that. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: EFI and multiboot2 devlopment work for Xen
On Tue, Oct 22, 2013 at 10:51:40AM -0400, Konrad Rzeszutek Wilk wrote:
> And I still haven't found the module that can launch any PE/COFF
> image from GRUB2. Maybe that is a myth.
"chainload" will do this. In fact, it doesn't do much:
static grub_err_t
grub_chainloader_boot (void)
{
grub_efi_boot_services_t *b;
grub_efi_status_t status;
grub_efi_uintn_t exit_data_size;
grub_efi_char16_t *exit_data = NULL;
b = grub_efi_system_table->boot_services;
status = efi_call_3 (b->start_image, image_handle, &exit_data_size,
&exit_data);
if (status != GRUB_EFI_SUCCESS)
...
That means, of course, that it won't use shim on Secure Boot systems.
There's probably some value in merging the two, so that chainload will
use the efi stub if and only if a) it's present, and b) it's needed for
e.g. multiple initrds or bad UGA/GOP data.
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH v2] x86:sysfb_efi:efifb_set_system: fix miss valid address range in later BARs
On Sun, May 01, 2016 at 12:21:05AM +0800, Wang YanQing wrote:
> We can't just break out when meet start is equal to zero,
> this will cause we miss valid address range in later BARs.
>
> On the other hand, it isn't enough to test start only
> for below situation:
> 0(start) <= lfb_base < end
>
> Note: this patch also add a trivial optimization,
> break out after we find the address range
> is valid without test later BARs.
>
> Signed-off-by: Wang YanQing
This looks correct to me:
Reviewed-By: Peter Jones
Cc'ing Matt Fleming, since this should probably go through the EFI tree.
> ---
> Due to the BUG this patch fix, I can't use video=efifb:
> boot parameter to get efifb on my new ThinkPad E550 for
> my old linux system hard disk with 3.10 kernel. In 3.10,
> efifb is the only choice due to DRM/I915 in it doesn't
> support the GPU.
>
> Changes:
> v1-v2:
> 1: Do a trivial code optimization.
>
> arch/x86/kernel/sysfb_efi.c | 14 --
> 1 file changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/sysfb_efi.c b/arch/x86/kernel/sysfb_efi.c
> index b285d4e..5da924b 100644
> --- a/arch/x86/kernel/sysfb_efi.c
> +++ b/arch/x86/kernel/sysfb_efi.c
> @@ -106,14 +106,24 @@ static int __init efifb_set_system(const struct
> dmi_system_id *id)
> continue;
> for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> resource_size_t start, end;
> + unsigned long flags;
> +
> + flags = pci_resource_flags(dev, i);
> + if (!(flags & IORESOURCE_MEM))
> + continue;
> +
> + if (flags & IORESOURCE_UNSET)
> + continue;
> +
> + if (pci_resource_len(dev, i) == 0)
> + continue;
>
> start = pci_resource_start(dev, i);
> - if (start == 0)
> - break;
> end = pci_resource_end(dev, i);
> if (screen_info.lfb_base >= start &&
> screen_info.lfb_base < end) {
> found_bar = 1;
> + break;
> }
> }
> }
> --
> 1.8.5.6.2.g3d8a54e.dirty
--
Peter
Re: [PATCH] efifb: Don't show the mapping VA
On Wed, May 11, 2016 at 04:57:47PM -0700, Andy Lutomirski wrote: > The framebuffer mapping virtual address leaks information about the > kernel memory layout. Stop logging it. > > Cc: Peter Jones > Cc: Jean-Christophe Plagniol-Villard > Cc: Tomi Valkeinen > Cc: linux-fb...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Andy Lutomirski In practice it also hasn't ever helped any actual debugging. Signed-off-by: Peter Jones > --- > drivers/video/fbdev/efifb.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c > index 95d293b7445a..3dcaf4e82885 100644 > --- a/drivers/video/fbdev/efifb.c > +++ b/drivers/video/fbdev/efifb.c > @@ -247,10 +247,8 @@ static int efifb_probe(struct platform_device *dev) > goto err_release_fb; > } > > - printk(KERN_INFO "efifb: framebuffer at 0x%lx, mapped to 0x%p, " > -"using %dk, total %dk\n", > -efifb_fix.smem_start, info->screen_base, > -size_remap/1024, size_total/1024); > + printk(KERN_INFO "efifb: framebuffer at 0x%lx, using %dk, total %dk\n", > +efifb_fix.smem_start, size_remap/1024, size_total/1024); > printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n", > efifb_defined.xres, efifb_defined.yres, > efifb_defined.bits_per_pixel, efifb_fix.line_length, > -- > 2.5.5 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-fbdev" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Peter
Re: [PATCH RFC 1/1] x86: fix bad memory access in fb_is_primary_device()
On Tue, Feb 16, 2016 at 01:49:18PM +, Matt Fleming wrote:
> [ Including Peter, the efifb maintainer. Original email is here,
>
> http://marc.info/?l=linux-kernel&m=145552936131335&w=2
>
> I've snipped some of the quoted text ]
>
> On Tue, 16 Feb, at 08:55:22AM, Ingo Molnar wrote:
> >
> > (I've Cc:-ed the EFI-FB and FB gents. Mail quoted below.)
> >
> > * Alexander Popov wrote:
> >
> > > Currently the code in fb_is_primary_device() contains to_pci_dev() macro
> > > which is applied to dev from struct fb_info. In some cases this causes
> > > bad memory access when fb_is_primary_device() handles fb_info of efifb.
> > > The reason is that fb dev of efifb is embedded into struct platform_device
> > > but not into struct pci_dev.
> > >
> > > We can fix this by checking fb dev bus name in fb_is_primary_device().
> > >
> > > It seems that this bug reveals some bigger problem with to_pci_dev(),
> > > to_platform_device() and others, which just do container_of() and
> > > don't check whether struct device is a part of the appropriate structure.
> > > Should we do something more about it?
> > >
> > > KASan report:
>
> [...]
>
> > >
> > > Signed-off-by: Alexander Popov
> > > ---
> > > arch/x86/video/fbdev.c | 9 +
> > > 1 file changed, 5 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/x86/video/fbdev.c b/arch/x86/video/fbdev.c
> > > index d5644bb..4999f78 100644
> > > --- a/arch/x86/video/fbdev.c
> > > +++ b/arch/x86/video/fbdev.c
> > > @@ -18,11 +18,12 @@ int fb_is_primary_device(struct fb_info *info)
> > > struct pci_dev *default_device = vga_default_device();
> > > struct resource *res = NULL;
> > >
> > > - if (device)
> > > - pci_dev = to_pci_dev(device);
> > > -
> > > - if (!pci_dev)
> > > + if (!device || !device->bus ||
> > > + !device->bus->name || strcmp(device->bus->name, "pci")) {
> > > return 0;
> > > + }
> > > +
> > > + pci_dev = to_pci_dev(device);
> > >
> > > if (default_device) {
> > > if (pci_dev == default_device)
> > > --
> > > 1.9.1
> > >
>
> I wonder if this issue could explain some of the efifb issues we've
> seen reported on bugzilla.kernel.org in the past where switching from
> efifb to some other framebuffer device caused hangs during boot. I'm
> struggling to find the relevant bugzilla entries now, though.
It's possible it could, but I don't have them handy either. I've also
wondered if some of them were due to bad data from the firmware - at
plugfests we've seen some cases where the actual video mode as measured
with a ruler is clearly not what the firmware claims it to be, so it's
entirely possible we're occasionally told a memory region that is not
what's actually mapped, or that's mapped but is only partially backed
by the actual frame buffer memory.
But aside from that diversion, I think Alexander has a legitimate
question about use of to_pci_dev(). If I ask the question: can we fix
this in efifb by making it live on a pci_dev, I have a couple of
fundamental problems:
1) technically it doesn't have to be a pci_dev at all (but, practically,
so far it always is on PCI...)
2) From EFI, we can't necessarily pin it down to a single PCI device
even if it is PCI. Before we do EFI's ExitBootServices() call, we
can try to find the PCI_IO handle our GOP instance is connected to,
but not all firmware GOP drivers use that, so it doesn't always work.
And even if it did, there can be more than one instance pointing to
the same memory with different PCI devices - lots of laptops have
this sort of thing.
3) Ignoring the EFI side and just focusing on PCI, if there's two
devices configured that could do scanout, it can be mapped to one
device's BAR but the other device be the actual device using it. In
this case either choice is probably wrong for something, and the
things that have the information to resolve which one don't include
efifb - they're the drivers we'll likely hand off to later.
So it's most likely right for efifb to be embedded in a platform_device
instead of a pci_dev. Which leads back to Alexander's question - if it
isn't in a pci_dev, that means fb_is_primary_device() needs to not
assume it is. So the patch appears correct, but so is the question -
should to_pci_dev() be checking this and returning NULL here?
--
Peter
Re: fwupdate
On Mon, Mar 09, 2015 at 06:54:12PM -0700, Roy Franz wrote: > On Mon, Mar 9, 2015 at 2:23 PM, Borislav Petkov wrote: > > + pjones. > > > > So reportedly, there is already a capsule-loading thing which doesn't > > need the kernel at all: > > > > https://github.com/rhinstaller/fwupdate > > > > So why are we even wasting energy with this discussion here? > > > > -- > > Regards/Gruss, > > Boris. > > The network boot case can be taken care of as Peter described (ie > taking network device paths, instead of just file paths), so > this should work for those cases as well. There would be some extra > setup, as for this to work the EFI firmware update program > would need to be run at boot (via the BootNext variable), but I don't > think this is unreasonable. > It looks like GnuEFI now supports Aarch64, so building the firmware > update utility shouldn't be a problem. Peter - have you you tried > building > this on Aarch64 yet? If not I'll give it a try. I tried a very early version of the code on an Aarch64 machine where I also wrote the firmware feature, and it basically seemed to work, modulo my buggy firmware ;) I haven't tried it recently, but I do have all the right makefile goo in there to build it reasonably, and I don't know of any reason it wouldn't work. That said, I haven't sent my patch to add the capsule headers to gnu-efi yet, so you won't get very far - I'll make sure and send those this week, hopefully today. Also note that fwupdate is still a very active work in progress; it's not /quite/ ready for general purpose deployment even in a "trying it out" phase, but I'm hoping to get to that point this week or next - in particular the code on github right now assumes an HD() device path with a specific partition guid. That is, it has literally worked on two machines ever. Yes, it's a thing we intend on supporting, but it's not /there/ yet. > I think there is some value in using runtime update capsule calls, as > that will help make sure the feature works in the firmware. I think > with arm64 servers in an early stage of development, we can influence > the firmware support of various features by using them. I don't know > that this is a sufficient reason, but if runtime capsules are never > used then there is a good chance that there will be many broken > implementations. That's certainly potentially valid, but it doesn't necessarily yield something we (the OS vendor deploying features to customers) can actually depend on. Usually firmware features like this become generally usable when there's some market maker causing hardware vendors to have a large interest in making sure the feature works. That means most often the carrot being dangled is MS Logo certification marketing dollars, and the testing all follows from the situations they require to work. So far, it appears MS has been using this only from Boot Services, so that's what works. It looks to me like that's probably likely to remain true. One reason is peripheral cards. My expectation (though possibly unfounded) is that MS will require peripheral cards to use this same update mechanism for option ROMs, and even if they don't I know some vendors are working on it. To do so, the card's UEFI driver (supplied on the option ROM) will be using EFI's Firmware Management Protocol (FMP) to participate in the capsule update system. The 2.5 spec drafts that include FMP and UpdateCapsule() support for FMP state that capsule updates for FMP aren't required to be available during runtime - it expressly calls out the case where CAPSULE_FLAGS_PERSIST_ACROSS_RESET is used. (I don't think I'm allowed to quote language from drafts in public, but if you have a copy, I'm talking about the last paragraph of 22.2.1) In my mind this means most implementations are going to always try to do updates from before ExitBootServices() for those option types - anything else and you're just in an error case where you have to do that anyway. So again, I'm not dead-set against having a kernel implementation, but I think its use cases are severely constrained, and most of the time most implementations will want to do this before the kernel loads. The best case for doing it in the kernel is embedded devices where a) there's no persistent storage except the flash part you're updating anyway, and b) the vendor nominally controls both the firmware and the deployed OS. -- Peter, heading back to the code. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
On Tue, Mar 10, 2015 at 12:26:52PM +, Matt Fleming wrote: > On Fri, 06 Mar, at 04:39:12PM, Peter Jones wrote: > > > > So again: do we really need or want to do this? > > One thing that we totally lose the ability to do is use the capsule > interface for things *other* than firmware updates, e.g. > > https://lkml.org/lkml/2013/10/16/327 > > Also, requiring embedded or custom OS to include fwupdate into their > existing boot solutions is a bit heavy handed when literally all they > want to do is cat a binary blob to a sysfs file. > > I don't see why we can't have both solutions. Yeah - we clearly need a kernel interface for some embedded devices, and it'd be better for every vendor to not implement it themselves. > Another thing is that ESRT isn't going to be supported by every > platform. Yeah - though I think you're *mostly* talking about the same platforms as above. > So, for the sysfs interface, let's not allow loading from /lib. Let's > not require a userland tool. Let's just do, > > # echo /path/to/my/awesome/capsule.bin > /sys/../capsule > > and be done with it. > > Hmmm? I assume you're implying a) the capsule header with the guid is embedded in the .bin there already, and b) one contiguous write(2) with error reporting coming through something like vars.c's efi_status_to_err()? If so, yes, I prefer this API. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: fwupdate
On Tue, Mar 10, 2015 at 10:56:32AM -0400, Peter Jones wrote: > That said, I haven't sent my patch to add the capsule headers to gnu-efi > yet, so you won't get very far - I'll make sure and send those this > week, hopefully today. Slight correction, I did actually do that - it's in the current upstream repo. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
> >> So, for the sysfs interface, let's not allow loading from /lib. Let's > >> not require a userland tool. Let's just do, > >> > >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule > > > >> > >> and be done with it. > >> > >> Hmmm? > > > > I assume you're implying a) the capsule header with the guid is embedded > > in the .bin there already, and b) one contiguous write(2) with error > > reporting coming through something like vars.c's efi_status_to_err()? > > > > If so, yes, I prefer this API. > > > > Is using a char device really so bad? I have a "simple_char" that > makes this really easy that's pending review. As long as there's straightforward propagation of the EFI_STATUS return from UpdateCapsule() back, sysfs file vs char device makes very little difference to me. Either way it's open(), write(), close(). Using the runtime firmware upload interface designed for wifi and scsi devices is the part I don't really like. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
On Tue, Mar 10, 2015 at 08:51:59AM -0700, Andy Lutomirski wrote:
> On Tue, Mar 10, 2015 at 8:40 AM, Peter Jones wrote:
> >
> >> >> So, for the sysfs interface, let's not allow loading from /lib. Let's
> >> >> not require a userland tool. Let's just do,
> >> >>
> >> >> # echo /path/to/my/awesome/capsule.bin > /sys/../capsule
> >> >
> >> >>
> >> >> and be done with it.
> >> >>
> >> >> Hmmm?
> >> >
> >> > I assume you're implying a) the capsule header with the guid is embedded
> >> > in the .bin there already, and b) one contiguous write(2) with error
> >> > reporting coming through something like vars.c's efi_status_to_err()?
> >> >
> >> > If so, yes, I prefer this API.
> >> >
> >>
> >> Is using a char device really so bad? I have a "simple_char" that
> >> makes this really easy that's pending review.
> >
> > As long as there's straightforward propagation of the EFI_STATUS return
> > from UpdateCapsule() back, sysfs file vs char device makes very little
> > difference to me. Either way it's open(), write(), close(). Using the
> > runtime firmware upload interface designed for wifi and scsi devices is
> > the part I don't really like.
> >
>
> I'm not 100% happy with write(2) (which is all we have in sysfs) for
> two reasons:
>
> 1. If we write a file name, eww. That's more complicated, requires
> temporary files, has annoying mount namespace issues, etc.
>
> 2. If we write the full contents, we need to do it in a single call to
> write. That means that we can't use cat, which mostly defeats the
> purpose. In fact, using cat could be actively harmful.
So if what we wind up with is:
fd = open("/sys/.../capsule", O_RDWR);
write(fd, buf, size/N);
...
write(fd, buf + M*size/N, size/N);
close(fd);
You're suggesting the error code would post on close()? My worry about
that is that I imagine a lot less code in the wild checks the error code
on close() than on write() - though gnu cat does do so on both. But
there are other questions still - will it post on fdatasync()? On
fsync()?
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
On Tue, Feb 24, 2015 at 12:49:09PM +, Kweh, Hock Leong wrote: > Hi All, > > After some internal discussion and re-design prototyping & testing on > this efi capsule interface kernel module, I would like to start a discussion > here on the new idea and wish to get input for the implementation and > eventually upstream. So do we actually *need* a kernel interface to UpdateCapsule? We've already got an implementation in progress where we use my ESRT patch to figure out what firmwares we can update, and we schedule them and do the update during boot up before the normal bootloader is run. That means never having to call UpdateCapsule() after ExitBootServices() or SetVirtualAddressMap() have been called, and only doing it across reboots that UEFI is performing itself. It also means never having to do it with multiple CPUs running. I've posted several revisions of the ESRT patch to linux-efi , and right now we're just waiting on the UEFI 2.5 spec to be finalized before I send a final copy to Matt. The command line tool and the code to apply the firmwares during boot are at https://github.com/rhinstaller/fwupdate and there's a GNOME-based UI in progress at https://github.com/hughsie/fwupd (yes we apparently stink at naming things.) I'm not sure how making this go through the kernel will make things better - it introduces a lot more things that can go wrong, and the only benefit is one reboot where BootNext is set - which actually is relatively fast even slow-POSTing machines. After that the UpdateCapsule+reboot to apply is *extremely* fast, because applying capsule updates have to happen very very early in the boot-up sequence. (That early processing is /effectively/ a requirement, since it has to happen before the block write locking on the SPI part is done.) So even on the machine that takes quite some time to POST, the time that would be saved saved is less than 1% or so of the total update time. An early version of my code worked to update a machine I got from your employer, FWIW - right now we're adding API and fixing up implementation bits I made specifically to that one proof-of-concept scenario, and there's some API parts that are in UEFI 2.5 draft releases that we're not quite handling yet. However, there's code there that's already been checked in which has successfully applied system firmware updates without having a kernel interface to UpdateCapsule(). So again: do we really need or want to do this? -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH v2 3/3] efi: Capsule update with user helper interface
On Fri, Mar 06, 2015 at 01:49:20PM -0800, Roy Franz wrote: > On Fri, Mar 6, 2015 at 1:39 PM, Peter Jones wrote: > > On Tue, Feb 24, 2015 at 12:49:09PM +, Kweh, Hock Leong wrote: > >> Hi All, > >> > >> After some internal discussion and re-design prototyping & testing on > >> this efi capsule interface kernel module, I would like to start a > >> discussion > >> here on the new idea and wish to get input for the implementation and > >> eventually upstream. > > > > So do we actually *need* a kernel interface to UpdateCapsule? We've > > already got an implementation in progress where we use my ESRT patch to > > figure out what firmwares we can update, and we schedule them and do the > > update during boot up before the normal bootloader is run. That means > > never having to call UpdateCapsule() after ExitBootServices() or > > SetVirtualAddressMap() have been called, and only doing it across > > reboots that UEFI is performing itself. It also means never having to > > do it with multiple CPUs running. > > So this does it by writing the capsules to the EFI system partition, and > having > them processed from there during the next boot? > How would this work on diskless systems? (or boot-disk-less systems) One of the things I'm currently writing is making the file we load be specified correctly by UEFI device paths - and that'll include the ability to get it from devices presented by the network drivers. On currently extant test machines that includes tftp support, just like netbooting. On UEFI 2.5 machines, where ESRT is introduced so that we actually know something about the capsules we can apply updates for, it also includes http support. Admittedly that means when you're doing deployment you'll have to have some process for putting the images someplace, but it can be the same device you're net-booting from. Just one example. If we're talking about systems that are booting entirely out of their own firmware volume, there's definitely some legitimate argument that doing this without an ESP could be valuable - so yeah, a kernel API in that case might be worthwhile. That said, the extra complexity combined with the fact that it's running after ExitBootServices() and SetVirtualAddressMap() means I'll probably avoid using it from the userland code except on machines where there are no other options. My faith that we're going to see a lot of machines where that's well tested without our address maps looking exactly like Windows's isn't very high, and we've repeatedly run into a lot of pain with that in the past. That's not the only thing that has to be exactly right, either - for instance there's no guarantee it'll work if you use the ACPI reset vector instead of the EFI runtime services ResetSystem(EfiResetWarm) call. Right now we use the ACPI method preferentially because of SetVirtualAddressMap() not working as documented on so *many* platforms. That means what happens upon reboot when we do UpdateCapsule() is undefined behavior. Of course I'm likely not the only person who will ever implement tools in userland to orchestrate firmware updates, either :) > What are the use cases for capsules that don't require a reboot? I'm > not really clear uses for those, but the kernel interface could be > better for those, as it would allow processing without a reboot. I'm really not sure if we know the answer to that yet. Things like USB devices most often won't ever be registered with firmware's FMP anyway, so they won't have capsules exposed, and they'll use more traditional USB commands to do firmware updates - stuff like hughsie's ColorHug device are in that category, and he's already supporting that with fwupd. Things like peripheral card option ROMs are potentially in that category, though I'm a little skeptical of the idea that I actually want to update the firmware on my video or network card with the kernel already running, and that when I do I'm not going to want to reboot immediately to make sure it worked right anyway. There are almost certainly lots of cases I haven't thought of, though. If we want this interface for those cases, I think we should also be enumerating the cases we think it's supporting, as well, even if only in broad strokes. I don't think we need to say "support this specific device's updates", but keeping track of the basic model of the update we're supporting would go a long way to establishing the value of supporting all the complexity. -- Peter -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] x86, boot: Allow 64bit EFI kernel to be loaded above 4G
On Wed, Feb 11, 2015 at 03:55:24PM +, Matt Fleming wrote:
> On Mon, 09 Feb, at 12:23:15PM, Yinghai Lu wrote:
> > On Mon, Feb 9, 2015 at 10:27 AM, Matt Fleming
> > wrote:
> > > On Tue, 03 Feb, at 06:03:20PM, Yinghai Lu wrote:
> > >
> > > The first thing that comes to mind is the issues we experienced last
> > > year when adding support for loading initrds above 4GB to the EFI boot
> > > stub, c.f. commit 47226ad4f4cf ("x86/efi: Only load initrd above 4g on
> > > second try").
> > >
> > > Are things going to work correctly this time?
> >
> > That should be addressed the grub2.
>
> I vaguely remember thinking that the issue was only experienced when
> using the EFI_FILE protocol, which grub2 doesn't use. So the grub
> developers may be OK, but we should at least give them a heads up.
Looks correct to me.
> > I was thinking that we may need to add mem_limit command together with
> > linuxefi and initrdefi.
> > or add linuxefi64/initrdefi64?
>
> No, we definitely do not want to add any more grub commands.
Definitely agree.
> > BTW, I tested loading kernel above grub2 on
> > virutalbox, qemu/kvm/OVMF, and real servers (ami ...) all work without
> > problem.
> >
> > wonder if we need have one black list for 64bit UEFI that does not
> > support access
> > memory above 4G.
>
> We have been successful, so far, in not introducing these kind of
> blacklists. It would be a shame to start now.
>From grub's point of view I'm not sure why we'd care - the pages kernel
and initramfs land in are both from the Boot Services allocator, so if the
machine doesn't support high addresses, they won't be there.
--
Peter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] lib: Always NUL terminate ucs2_as_utf8
On Thu, Apr 21, 2016 at 01:18:27PM +0100, Matt Fleming wrote: > ( Good Lord, I hate doing string manipulation in C ) (yep) > > On Wed, 20 Apr, at 03:25:32PM, Laszlo Ersek wrote: > > > > So, "len" does not include the room for the terminating NUL-byte here. > > When "len" is passed to ucs2_as_utf8(), with the proposed patch applied, > > a NUL byte will be produced in "name", but it will be at the price of a > > genuine character from the input variable name. > > Right, and this is a problem because we're trying to keep the names > consistent between efivarfs and the EFI variable data. Force > NUL-terminating the string is wrong, because if you have no room for > the NUL the caller should check for that. Sadly none do. > > On the flip-side, passing around non-NUL terminated strings is just > begging for these kinds of issues to come up. > > The fact is that the callers of ucs2_as_utf8() are passing it the > wrong 'len' argument. We want a NUL-terminated utf8 string and we're > passing a NUL-terminated ucs2 string. We should tell ucs2_as_utf8() it > has enough room to copy the NUL. > > Wouldn't this work (minus the return value checking)? I agree with your analysis, and your patch looks plausible. -- Peter
[PATCH] ACPI: don't show an error when we're not in charge of PCIe hotplug.
Right now when booting, on many laptops the firmware manages the PCIe
bus. As a result, when we call the _OSC ACPI method, it returns an
error code. Unfortunately the errors are not very articulate. As a
result, we show:
ACPI: PCI Root Bridge [PCI0] (domain [bus 00-fe])
acpi PNP0A08:00: _OSC: OS supports [ExtendedConfig ASPM ClockPM Segments MSI]
\_SB_.PCI0 (33DB4D5B-1FF7-401C-9657-7441C03DD766): _OSC invalid UUID
_OSC request data: 1 1f 0
acpi PNP0A08:00: _OSC failed (AE_ERROR); disabling ASPM
But we did get the capabilities mask back; the firmware is merely
managing this itself. So we really should not be showing the user a
message that looks like the firmware is broken, since it is working just
fine.
This patch supresses the error message when we're calling _OSC with the
PCIe host bridge UUID, and replaces it with a relatively innocuous
looking message that you can find if you're looking for it.
---
drivers/acpi/bus.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 262ca31..be8a91b 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -215,6 +215,8 @@ acpi_status acpi_str_to_uuid(char *str, u8 *uuid)
}
EXPORT_SYMBOL_GPL(acpi_str_to_uuid);
+static u8 pci_osc_uuid_str[] = "33DB4D5B-1FF7-401C-9657-7441C03DD766";
+
acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context)
{
acpi_status status;
@@ -267,9 +269,13 @@ acpi_status acpi_run_osc(acpi_handle handle, struct
acpi_osc_context *context)
if (errors & OSC_REQUEST_ERROR)
acpi_print_osc_error(handle, context,
"_OSC request failed");
- if (errors & OSC_INVALID_UUID_ERROR)
- acpi_print_osc_error(handle, context,
- "_OSC invalid UUID");
+ if (errors & OSC_INVALID_UUID_ERROR) {
+ if (!strcasecmp(context->uuid_str, pci_osc_uuid_str))
+ pr_info("PCI PME managed by ACPI.\n");
+ else
+ acpi_print_osc_error(handle, context,
+"_OSC invalid UUID");
+ }
if (errors & OSC_INVALID_REVISION_ERROR)
acpi_print_osc_error(handle, context,
"_OSC invalid revision");
--
2.7.4
Re: [PATCH 08/29] efi: Allow drivers to reserve boot services forever
On Tue, Jan 03, 2017 at 06:48:43PM -0800, Dan Williams wrote: > On Fri, Sep 9, 2016 at 8:18 AM, Matt Fleming wrote: > > Today, it is not possible for drivers to reserve EFI boot services for > > access after efi_free_boot_services() has been called on x86. For > > ARM/arm64 it can be done simply by calling memblock_reserve(). > > > > Having this ability for all three architectures is desirable for a > > couple of reasons, > > > > 1) It saves drivers copying data out of those regions > > 2) kexec reboot can now make use of things like ESRT > > > > Instead of using the standard memblock_reserve() which is insufficient > > to reserve the region on x86 (see efi_reserve_boot_services()), a new > > API is introduced in this patch; efi_mem_reserve(). > > > > efi.memmap now always represents which EFI memory regions are > > available. On x86 the EFI boot services regions that have not been > > reserved via efi_mem_reserve() will be removed from efi.memmap during > > efi_free_boot_services(). > > > > This has implications for kexec, since it is not possible for a newly > > kexec'd kernel to access the same boot services regions that the > > initial boot kernel had access to unless they are reserved by every > > kexec kernel in the chain. > > > > Tested-by: Dave Young [kexec/kdump] > > Tested-by: Ard Biesheuvel [arm] > > Acked-by: Ard Biesheuvel > > Cc: Leif Lindholm > > Cc: Peter Jones > > Cc: Borislav Petkov > > Cc: Mark Rutland > > Signed-off-by: Matt Fleming > > This commit appears to cause a boot regression between v4.8 and v4.9. Can you verify that the memory map looks reasonable? This looks similar (but not identical) to the traceback I was hitting on some ThinkPads with ESRT last month, so there's some chance, if it is caused by bad memory map entries, it may be fixed by 1cb209f63 on efi/next . > > BUG: unable to handle kernel paging request at 8830281bf1c8 > IP: [] __next_mem_range_rev+0x13a/0x1d6 > PGD 3193067 PUD 3196067 PTE 8030281bf060 > Oops: 1 SMP DEBUG_PAGEALLOC > Modules linked in: > CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.9.0+ #2 > task: 82011540 task.stack: 8200 > RIP: 0010:[] [] > __next_mem_range_rev+0x13a/0x1d6 > RSP: :82003dd8 EFLAGS: 00010202 > RAX: 8830281bf1e0 RBX: 82003e60 RCX: 82167490 > RDX: RSI: RDI: 00184000 > RBP: 82003e18 R08: 821674b0 R09: 008f > R10: 008f R11: 82011cf0 R12: 0004 > R13: 00304000 R14: R15: 0001 > FS: () GS:8817e080() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: 8830281bf1c8 CR3: 0200a000 CR4: 007406f0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > PKRU: > Stack: > ea001700 82003e50 > 10304000 82003e58 0180 > ffc0 > 82003e98 81a21395 82003e58 > Call Trace: > [] memblock_find_in_range_node+0x93/0x13a > [] memblock_alloc_range_nid+0x1b/0x3e > [] __memblock_alloc_base+0x15/0x17 > [] memblock_alloc_base+0x12/0x2e > [] memblock_alloc+0xb/0xd > [] efi_free_boot_services+0x46/0x180 > [] start_kernel+0x4a1/0x4cc > [] ? set_init_arg+0x55/0x55 > [] ? early_idt_handler_array+0x120/0x120 > [] x86_64_start_reservations+0x2a/0x2c > [] x86_64_start_kernel+0x14c/0x16f > Code: 18 44 89 38 41 8d 44 24 ff 49 c1 e1 20 4c 09 c8 48 89 03 e9 a0 > 00 00 00 4d 63 d1 4c 89 d0 48 c1 e0 05 49 03 40 18 45 85 c9 74 28 <48> > 8b 50 e8 48 03 50 e0 49 83 cb ff 4d 3b 10 73 03 4c 8b 18 49 ^M > RIP [] __next_mem_range_rev+0x13a/0x1d6 > > I also see that Petr may have run into it as well [1]? Petr is this > the same signature you are seeing? Can you post a boot log with > "efi=debug" on the kernel command line? > > It also fails on 4.10-rc2. However, if I revert the following commits > it boots fine: > > 4bc9f92e64c8 x86/efi-bgrt: Use efi_mem_reserve() to avoid copying image data > 8e80632fb23f efi/esrt: Use efi_mem_reserve() and avoid a kmalloc() > 816e76129ed5 efi: Allow drivers to reserve boot services forever > > [1]: https://lkml.org/lkml/2016/12/21/197 -- Peter
Re: [PATCH 9/9] MODSIGN: Allow the "db" UEFI variable to be suppressed
On Mon, Nov 21, 2016 at 04:42:45PM +, Ard Biesheuvel wrote: > On 21 November 2016 at 16:26, Josh Boyer wrote: > > On Mon, Nov 21, 2016 at 11:18 AM, Ard Biesheuvel > > wrote: > >> On 16 November 2016 at 18:11, David Howells wrote: > >>> From: Josh Boyer > >>> > >>> If a user tells shim to not use the certs/hashes in the UEFI db variable > >>> for verification purposes, shim will set a UEFI variable called > >>> MokIgnoreDB. Have the uefi import code look for this and ignore the db > >>> variable if it is found. > >>> > >> > >> Similar concern as in the previous patch: it appears to me that you > >> can DoS a machine by setting MokIgnoreDB if, e.g., its modules are > >> signed against a cert that resides in db, and shim/mokmanager are not > >> being used. > > > > If shim/mokmanager aren't used, then you can't actually modify > > MokIgnoreDB. Again, it requires physical access and a reboot into > > mokmanager to actually take effect. > > > > This does the trick as well > > printf "\x07\x00\x00\x00\x01" > > /sys/firmware/efi/efivars/MokIgnoreDB-605dab50-e046-4300-abb6-3dd810dd8b23 So that really means two things. First, kernel should only honor any of the Mok* variables if they're Boot Services-only variables. Second, to avoid the DoS, shim should create them all as Boot Services-only the first time it boots. That'll prevent them from being created post-boot. -- Peter
Re: [PATCH 9/9] MODSIGN: Allow the "db" UEFI variable to be suppressed
On Mon, Nov 21, 2016 at 08:06:44PM +0100, Ard Biesheuvel wrote: > On 21 November 2016 at 20:05, Peter Jones wrote: > > On Mon, Nov 21, 2016 at 04:42:45PM +, Ard Biesheuvel wrote: > >> On 21 November 2016 at 16:26, Josh Boyer wrote: > >> > On Mon, Nov 21, 2016 at 11:18 AM, Ard Biesheuvel > >> > wrote: > >> >> On 16 November 2016 at 18:11, David Howells wrote: > >> >>> From: Josh Boyer > >> >>> > >> >>> If a user tells shim to not use the certs/hashes in the UEFI db > >> >>> variable > >> >>> for verification purposes, shim will set a UEFI variable called > >> >>> MokIgnoreDB. Have the uefi import code look for this and ignore the db > >> >>> variable if it is found. > >> >>> > >> >> > >> >> Similar concern as in the previous patch: it appears to me that you > >> >> can DoS a machine by setting MokIgnoreDB if, e.g., its modules are > >> >> signed against a cert that resides in db, and shim/mokmanager are not > >> >> being used. > >> > > >> > If shim/mokmanager aren't used, then you can't actually modify > >> > MokIgnoreDB. Again, it requires physical access and a reboot into > >> > mokmanager to actually take effect. > >> > > >> > >> This does the trick as well > >> > >> printf "\x07\x00\x00\x00\x01" > > >> /sys/firmware/efi/efivars/MokIgnoreDB-605dab50-e046-4300-abb6-3dd810dd8b23 > > > > So that really means two things. First, kernel should only honor any of > > the Mok* variables if they're Boot Services-only variables. Second, to > > avoid the DoS, shim should create them all as Boot Services-only the > > first time it boots. That'll prevent them from being created post-boot. > > > > All of that assumes you are using shim and mokmanager in the first place. No, it doesn't. If you're not using shim, there's no DoS problem, because what would you be DoSing? And likewise, if you're not using Secure Boot at all, you have no guarantee of anything about your boot environment, starting with the idea that the boot loader isn't hostile. If you're not using Secure Boot, a hostile pre-boot driver could easily add DB entries just as easily as MokList entries, or any other variables. The fact that keys can be injected is true with or without this patch, though it does make it easier. But making a boot loader that injects keys into the kernel's built-in keyring isn't actually very difficult. If you're not using firmware enforced SB and shim, you do not have security against this. -- Peter
Re: [PATCH] [efifb] Fix 16 color palette entry calculation
On Tue, Jun 07, 2016 at 03:45:43PM +0200, Max Staudt wrote:
> When using efifb with a 16-bit (5:6:5) visual, fbcon's text is rendered
> in the wrong colors - e.g. text gray (#aa) is rendered as green
> (#50bc50) and neighboring pixels have slightly different values
> (such as #50bc78).
>
> The reason is that fbcon loads its 16 color palette through
> efifb_setcolreg(), which in turn calculates a 32-bit value to write
> into memory for each palette index.
> Until now, this code could only handle 8-bit visuals and didn't mask
> overlapping values when ORing them.
>
> With this patch, fbcon displays the correct colors when a qemu VM is
> booted in 16-bit mode (in GRUB: "set gfxpayload=800x600x16").
>
> Signed-off-by: Max Staudt
> ---
> drivers/video/fbdev/efifb.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
> index 924bad4..37a37c4 100644
> --- a/drivers/video/fbdev/efifb.c
> +++ b/drivers/video/fbdev/efifb.c
> @@ -50,9 +50,9 @@ static int efifb_setcolreg(unsigned regno, unsigned red,
> unsigned green,
> return 1;
>
> if (regno < 16) {
> - red >>= 8;
> - green >>= 8;
> - blue >>= 8;
> + red >>= 16 - info->var.red.length;
> + green >>= 16 - info->var.green.length;
> + blue >>= 16 - info->var.blue.length;
> ((u32 *)(info->pseudo_palette))[regno] =
> (red << info->var.red.offset) |
> (green << info->var.green.offset) |
> --
> 2.6.6
Looks right to me.
Acked-By: Peter Jones
--
Peter
Re: [PATCH] tpm: Require that all digests are present in TCG_PCR_EVENT2 structures
On Tue, Jun 16, 2020 at 11:08:38AM +0200, Ard Biesheuvel wrote:
> (cc Matthew and Peter)
>
> On Tue, 16 Jun 2020 at 01:28, Tyler Hicks wrote:
> >
> > Require that the TCG_PCR_EVENT2.digests.count value strictly matches the
> > value of TCG_EfiSpecIdEvent.numberOfAlgorithms in the event field of the
> > TCG_PCClientPCREvent event log header. Also require that
> > TCG_EfiSpecIdEvent.numberOfAlgorithms is non-zero.
> >
> > The TCG PC Client Platform Firmware Profile Specification section 9.1
> > (Family "2.0", Level 00 Revision 1.04) states:
> >
> > For each Hash algorithm enumerated in the TCG_PCClientPCREvent entry,
> > there SHALL be a corresponding digest in all TCG_PCR_EVENT2 structures.
> > Note: This includes EV_NO_ACTION events which do not extend the PCR.
> >
> > Section 9.4.5.1 provides this description of
> > TCG_EfiSpecIdEvent.numberOfAlgorithms:
> >
> > The number of Hash algorithms in the digestSizes field. This field MUST
> > be set to a value of 0x01 or greater.
> >
> > Enforce these restrictions, as required by the above specification, in
> > order to better identify and ignore invalid sequences of bytes at the
> > end of an otherwise valid TPM2 event log. Firmware doesn't always have
> > the means necessary to inform the kernel of the actual event log size so
> > the kernel's event log parsing code should be stringent when parsing the
> > event log for resiliency against firmware bugs. This is true, for
> > example, when firmware passes the event log to the kernel via a reserved
> > memory region described in device tree.
> >
>
> When does this happen? Do we have code in mainline that does this?
>
> > Prior to this patch, a single bit set in the offset corresponding to
> > either the TCG_PCR_EVENT2.eventType or TCG_PCR_EVENT2.eventSize fields,
> > after the last valid event log entry, could confuse the parser into
> > thinking that an additional entry is present in the event log. This
> > patch raises the bar on how difficult it is for stale memory to confuse
> > the kernel's event log parser but there's still a reliance on firmware
> > to properly initialize the remainder of the memory region reserved for
> > the event log as the parser cannot be expected to detect a stale but
> > otherwise properly formatted firmware event log entry.
> >
> > Fixes: fd5c78694f3f ("tpm: fix handling of the TPM 2.0 event logs")
> > Signed-off-by: Tyler Hicks
> > ---
>
> I am all for stringent checks, but this could potentially break
> measured boot on systems that are working fine today, right?
Seems like in that case our measurement is unreliable and can't really
be trusted. That said, having things that were using the measurements
before this suddenly stop being able to access sealed secrets is not a
great experience for the user who unwittingly bought the junk hardware.
Same with the zero-supported-hashes case. It would be nice to at log it
and have a way for them to opt-in to allowing the old measurement to go
through, so they can recover their data, though I don't know what that
method would be if the measured command line is one of their
dependencies.
--
Peter
Re: [PATCH] tpm: Require that all digests are present in TCG_PCR_EVENT2 structures
On Tue, Jun 30, 2020 at 02:23:22PM -0500, Tyler Hicks wrote: > > > I am all for stringent checks, but this could potentially break > > > measured boot on systems that are working fine today, right? > > > > Seems like in that case our measurement is unreliable and can't really > > be trusted. That said, having things that were using the measurements > > before this suddenly stop being able to access sealed secrets is not a > > great experience for the user who unwittingly bought the junk hardware. > > I haven't seen where anyone has suggested that such junk hardware > exists. Do you know of or expect any firmware that has mismatched > TCG_PCR_EVENT2.digests.count and TCG_EfiSpecIdEvent.numberOfAlgorithms > values? If nobody has seen any hardware that actually produces the values you're excluding, then I don't have a strong objection. > I would think that the userspace code that's parsing > /sys/kernel/security/tpm0/binary_bios_measurements would also have > issues with such an event log. > > > Same with the zero-supported-hashes case. > > Small but important correction: it is a zero-hashes case, not a > zero-supported-hashes case > > There's no handshake involved or anything like that. This would only > cause problems if the firmware provided no hashes, which means the > firmware event log is unusable, anyways. Indeed. -- Peter
[PATCH] Make it possible to disable efivar_ssdt entirely
In most cases, such as CONFIG_ACPI_CUSTOM_DSDT and CONFIG_ACPI_TABLE_UPGRADE, boot-time modifications to firmware tables are tied to specific Kconfig options. Currently this is not the case for modifying the ACPI SSDT via the efivar_ssdt kernel command line option and associated EFI variable. This patch adds CONFIG_EFI_CUSTOM_SSDT_OVERLAYS, which defaults disabled, in order to allow enabling or disabling that feature during the build. Signed-off-by: Peter Jones --- drivers/firmware/efi/efi.c | 2 +- drivers/firmware/efi/Kconfig | 11 +++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 48d0188936c..4b12a598ccf 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -192,7 +192,7 @@ static void generic_ops_unregister(void) efivars_unregister(&generic_efivars); } -#if IS_ENABLED(CONFIG_ACPI) +#if IS_ENABLED(CONFIG_EFI_CUSTOM_SSDT_OVERLAYS) #define EFIVAR_SSDT_NAME_MAX 16 static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; static int __init efivar_ssdt_setup(char *str) diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index 6b38f9e5d20..fe433f76b03 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -278,3 +278,14 @@ config EFI_EARLYCON depends on SERIAL_EARLYCON && !ARM && !IA64 select FONT_SUPPORT select ARCH_USE_MEMREMAP_PROT + +config EFI_CUSTOM_SSDT_OVERLAYS + bool "Load custom ACPI SSDT overlay from an EFI variable" + depends on EFI_VARS + default ACPI_TABLE_UPGRADE + help + Allow loading of an ACPI SSDT overlay from an EFI variable specified + by a kernel command line option. + + See Documentation/admin-guide/acpi/ssdt-overlays.rst for more + information. -- 2.26.2
Re: [PATCH] efifb: allow user to disable write combined mapping.
On Tue, Jul 18, 2017 at 04:09:09PM +1000, Dave Airlie wrote: > This patch allows the user to disable write combined mapping > of the efifb framebuffer console using an nowc option. > > A customer noticed major slowdowns while logging to the console > with write combining enabled, on other tasks running on the same > CPU. (10x or greater slow down on all other cores on the same CPU > as is doing the logging). > > I reproduced this on a machine with dual CPUs. > Intel(R) Xeon(R) CPU E5-2609 v3 @ 1.90GHz (6 core) > > I wrote a test that just mmaps the pci bar and writes to it in > a loop, while this was running in the background one a single > core with (taskset -c 1), building a kernel up to init/version.o > (taskset -c 8) went from 13s to 133s or so. I've yet to explain > why this occurs or what is going wrong I haven't managed to find > a perf command that in any way gives insight into this. > > 11,885,070,715 instructions #1.39 insns per cycle > vs > 12,082,592,342 instructions #0.13 insns per cycle > > is the only thing I've spotted of interest, I've tried at least: > dTLB-stores,dTLB-store-misses,L1-dcache-stores,LLC-store,LLC-store-misses,LLC-load-misses,LLC-loads,\mem-loads,mem-stores,iTLB-loads,iTLB-load-misses,cache-references,cache-misses > > For now it seems at least a good idea to allow a user to disable write > combining if they see this until we can figure it out. Well, that's kind of amazing, given 3c004b4f7eab239e switched us /to/ using ioremap_wc() for the exact same reason. I'm not against letting the user force one way or the other if it helps, though it sure would be nice to know why. Anyway, Acked-By: Peter Jones Bartlomiej, do you want to handle this in your devel tree? -- Peter
Re: [PATCH 1/2] fs/efivarfs: restrict inode permissions
On Tue, Feb 20, 2018 at 02:01:51PM -0800, Linus Torvalds wrote: > Which variables are actually used by user space tools? It sounds like > it may be exactly those boot order things that we already have flags > and special behavior for. Not that many are really part of a non-root workflow. The biggest consumer that there's a real /user/ workflow for is Matthew's tpmtotp, which lets you pick your own when you set it up as root and merely read from it as a user in perpetuity. Most workflows are of the same form as "I run efibootmgr as a user to list things and then use sudo when I actually need to make some changes." > And no, I don't believe in the "SMI can trigger a DoS" argument. Those > garbage efivars that *do* trigger SMI's should me marked and shamed, > and maybe _they_ need to be added to the list of things that you > should look out for. Root or not. It's all of them except the very few that are generated during bootup. It's worth noting, though, that EFI does not actually require this implementation behavior in any way. This is all about Intel's choice to use SMI to protect the variable store. > And just on general principlies, I don't want to see weasel-wordy > commit messages like > > "Reading certain EFI variables trigger SMIs" > > I understand *writing* them causing SMI's due to some flash protection > scheme. What's the reading thing? And why aren't we calling that > garbage out? Assuming most Intel CPUs work more or less the same as Galileo in this particular regard, what's going on is the SPI part is connected to the North Cluster, which presents an MMIO interface to program it, and SMM is configured so that touching those addresses triggers an SMI. This way they can protect the the "authenticated" variables by checking signatures inside SMM. So basically it's not "Reading certain variables trigger SMIs", it's "reading any variable that's not completely fake causes SMI". The result is any user can not merely trigger an SMI, but really more like they can hold it asserted. > So even if we do need to limit reading, I want out comments (in core > or commits) to actually explain *Why*., instead of this kind of > non-specific fear-mongering that nobody can really say yay or nay > about. Yeah, makes sense. There are other options, but they're not great either. For example, On most systems the total data is <100kB, so we could make a default-on config option to just cache it all during boot by default. Like the options suggested so far, it has downsides, but it's not the end of the world for most systems. -- Peter
Re: [PATCH] efivarfs: Limit the rate for non-root to read files
On Thu, Feb 22, 2018 at 06:11:14AM +, Luck, Tony wrote: >> On Feb 21, 2018, at 21:52, Linus Torvalds wrote: >> >> Does the error return actually break real users? Not "I can do did >> things and it acts differently" things, but actual users... > > Probably not. Peter Jones said that efibootmgr might access up to 20 > files. Assuming it is sanely reading in big chunks, it won’t hit the > rate limit that I set at 100. Typically each read looks like: openat(AT_FDCWD, "/sys/firmware/efi/efivars/Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c", O_RDONLY) = 3 read(3, "\7\0\0\0", 4) = 4 read(3, "\1\0\0\0b\0t\0e\0s\0t\0\0\0\4\1*\0\1\0\0\0\0\10\0\0\0\0\0\0"..., 4096) = 114 read(3, "", 3982) = 0 close(3)= 0 For each multiple of 4k, you'll see one more read() call. (It's two reads because libefivar's efi_get_variable() returns the attributes separately from the data, which goes in an allocation the caller is responsible for freeing, so doing it as one read means it would introduce an extra copy.) Looking at that code path, if it *does* get tripped up by EAGAIN, it should handle it fine, though maybe I should add a short randomized delay (or just sched_yield()) in that case. I don't think the 3rd read there to detect EOF hits the efivarfs code, so that's 2 reads per variable until you go over 4k, which most never do. That pattern is true of everything that uses libefivar to do its EFI variable manipulation. On my moderately typical laptop with 2 boot variables set, it looks something like: trillian:~$ strace -o efibootmgr.strace efibootmgr -v BootCurrent: 0001 Timeout: 0 seconds BootOrder: 0001, Boot* Fedora HD(1,GPT,2cf5261b-7b98-48c0-ae54-463dbd23e65b,0x800,0x64000)/File(\EFI\fedora\shimx64.efi) Boot0001* test HD(1,GPT,2cf5261b-7b98-48c0-ae54-463dbd23e65b,0x800,0x64000)/File(\EFI\fedora\shimx64.efi) trillian:~$ grep '^\(open\|read\)' efibootmgr.strace | grep -A100 sys/firmware | grep -c ^read 15 Which, if I'm write about VFS eating that last read, means 10 calls. Many machines have some default boot variables; my desktop at home has 5 completely useless variables the firmware sets. So there it winds up being 27 calls to read(2), and thus 18 calls to count towards our limit. Your limit at 100 looks sufficiently large to me. -- Peter
Re: [PATCH 0/2] efivars: reading variables can generate SMIs
On Fri, Feb 16, 2018 at 10:48:32AM -0800, Joe Konno wrote: > On Fri, Feb 16, 2018 at 11:18:12AM +, Ard Biesheuvel wrote: > > On 16 February 2018 at 11:08, Borislav Petkov wrote: > > > On Fri, Feb 16, 2018 at 10:58:47AM +, Ard Biesheuvel wrote: > > >> By your own reasoning above, that's a no-no as well. > > > > > > I'm sure we can come up with some emulation - the same way we did the > > > BIOS emulation. > > > > > >> But thanks for your input. Anyone else got something constructive to > > >> contribute? > > > > > > The not-breaking userspace is constructive contribution. The last > > > paragraph is my usual rant. > > > > > > > Fair enough. And I am not disagreeing with you either. > > > > So question to Joe: is it well defined which variables may exhibit > > this behavior? > > For brevity's sake, "not yet." Folks-- e.g. firmware writers and > equipment makers-- may use SMIs more (or less) than others. So, how many > SMIs generated by the reference shell script can, and will, vary > platform to platform. I and my colleagues are digging into this. As a first guess: anything generated during boot is probably not an SMI. Everything else is probably an SMI. In fact, I would expect that for most systems, the entire list of things that *don't* generate an SMI (aside from a few IBV specific variables) is all the variables in Table 10 of the UEFI spec that don't have the NV flag. > > Given that UEFI variables are GUID scoped, would whitelisting certain > > GUIDs (the ones userland currently relies on to be readable my > > non-privileged users) and making everything else user-only solve this > > problem as well? > > Perhaps. I don't want us chasing white rabbits just yet, but the > whitelist is but one approach under consideration. We may see some other > patches or RFCs about caching and/or shadowing variable values in > efivarfs to reduce the number of direct EFI reads, with the goal of > reducing how many SMIs are generated. > > Any obvious EFI variables that userspace tools have come to depend on-- > those which normal, unprivileged users need to read-- are helpful inputs > to this discussion. So, our big userland consumers are efibootmgr, fwupdate, and "systemctl reboot --firmware-setup". efibootmgr and fwupdate can do the "show the current status" half of their job as a user right now, but they rely on root for the other half anyway. I don't think we normally use libfwup as non-root even for reading status. So basically, the use case from userland that this will effect looks like: efibootmgr -v *scratch head* sudo su - efibootmgr -b -B efibootmgr -b -c -L "fixed entry" ... exit I don't feel really bad about people having to move the third line up to the top of that. It's also not a use case you can have very much of: it means you manually booted without any valid boot entries. fallback.efi would have created a valid boot entry if you didn't do it manually. "systemctl reboot --firmware-setup" effectively runs as root in all cases. The only thing we really must ensure to preserve the other workflows is making sure creat() and open() with 0644 doesn't return an error (it obviously won't be preserved across a reboot), because that would break the existing tools. But I don't see anything in this patchset which will cause that. tl;dr: I think changing everything to 0600 is probably completely fine, and whitelisting is probably pointless. -- Peter
Re: [PATCH 0/2] efivars: reading variables can generate SMIs
On Fri, Feb 16, 2018 at 07:32:17PM +, Luck, Tony wrote: > > tl;dr: I think changing everything to 0600 is probably completely fine, > > and whitelisting is probably pointless. > > But do you speak for all users? No, I just write their tools :) > It will just take one person complaining that efibootmgr no longer > shows them what it used to show to bring down the wrath of Linus on > our (specifically Joe's) head for breaking user space. The userland use case is gazing idly at the values without intending to do anything about them. And most of this is firmware config and firmware/OS interaction. Honestly it should never have been user readable to begin with. But also, we had a bug for quite some time where efibootmgr created everything as 0600, and as a result non-root users couldn't do e.g. "efibootmgr -v" and see the paths of new entries until a reboot occurred. Nobody ever reported it in bugzilla.redhat.com or efibootmgr or efivar's github issues pages. One person noticed it while commenting about another issue, but didn't see it as related to his actual issue or being a bug so much as "weird" that listing worked as non-root before changing something but not after. Another user asked about getting permission denied while setting the boot order on askubuntu here: https://askubuntu.com/questions/688317/getting-permission-denied-errors-from-efibootmgr The response was exactly that you have to run it as root. I think it's telling that nobody said anything about reading vs writing. > I've got someone about to start looking at making efivarfs read and save > the values during mount, so all the read-only options can continue to work > without making EFI calls. > > This will cost some memory (say 20-30 variables at up to 1K each). 71 variables on my laptop, and the 1K restriction went away a *lng* time ago. It was fully excised from the userland tools in 2013. On my laptop, 4 of those 71 variables are >5000 bytes. The total storage of all of the data in the variables is 38kB. I still think changing it to 0600 and calling this done is the right thing to do. -- Peter
Re: [PATCH 0/2] efivars: reading variables can generate SMIs
On Fri, Feb 16, 2018 at 09:09:30PM +, Luck, Tony wrote: > > That said, I'm not sure how many non-root users run the toolkit to > > extract their EFI certificates or check on the secure boot status of > > the system, but I suspect it might be non-zero: I can see the tinfoil > > hat people wanting at least to check the secure boot status when they > > log in. > > Another fix option might be to rate limit EFI calls for non-root users (on X86 > since only we have the SMI problem). That would: > > 1) Avoid using memory to cache all the variables > 2) Catch any other places where non-root users can call EFI I could get behind that as well. Currently the things I maintain do approximately this many normal accesses with invocations you can do as a user: "efibootmgr -v" - six files we always try to read, plus one per Boot entry. "fwupdate --info" - one file it always tries to read, one file for each ESRT entry. "dbxtool -l" - one file it always reads. "mokutil --sb-state" - reads the same file twice. I don't maintain this, but I'll send a patch to Gary to make it only read it once. AFAICS all of the other invocations you can currently do as a user /legitimately/ read two files, though. Some systems seem to *love* making a pile of Boot entries; I think the most I've seen is something like 16. So on that machine, one "efibootmgr -v" invocation is ~22 efivars files read. I've never seen a machine that advertised more than 2 ESRT entries, but maybe we'll get there some day. -- Peter
[tip:efi/core] efivarfs: Make efivarfs_file_ioctl() static
Commit-ID: 6c5450ef66816216e574885cf8d3ddb31ef77428
Gitweb: http://git.kernel.org/tip/6c5450ef66816216e574885cf8d3ddb31ef77428
Author: Peter Jones
AuthorDate: Fri, 6 May 2016 22:39:31 +0100
Committer: Ingo Molnar
CommitDate: Sat, 7 May 2016 07:06:13 +0200
efivarfs: Make efivarfs_file_ioctl() static
There are no callers except through the file_operations struct below
this, so it should be static like everything else here.
Signed-off-by: Peter Jones
Signed-off-by: Matt Fleming
Cc: Andy Lutomirski
Cc: Ard Biesheuvel
Cc: Borislav Petkov
Cc: Brian Gerst
Cc: Denys Vlasenko
Cc: H. Peter Anvin
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-...@vger.kernel.org
Link:
http://lkml.kernel.org/r/1462570771-13324-6-git-send-email-m...@codeblueprint.co.uk
Signed-off-by: Ingo Molnar
---
fs/efivarfs/file.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
index d48e0d2..5f22e74 100644
--- a/fs/efivarfs/file.c
+++ b/fs/efivarfs/file.c
@@ -157,7 +157,7 @@ efivarfs_ioc_setxflags(struct file *file, void __user *arg)
return 0;
}
-long
+static long
efivarfs_file_ioctl(struct file *file, unsigned int cmd, unsigned long p)
{
void __user *arg = (void __user *)p;
[tip:efi/core] efi: Reformat GUID tables to follow the format in UEFI spec
Commit-ID: 662b1d890c593673964758fe5b6f22067bffba7a Gitweb: http://git.kernel.org/tip/662b1d890c593673964758fe5b6f22067bffba7a Author: Peter Jones AuthorDate: Wed, 17 Feb 2016 12:35:54 + Committer: Ingo Molnar CommitDate: Mon, 22 Feb 2016 08:26:25 +0100 efi: Reformat GUID tables to follow the format in UEFI spec This makes it much easier to hunt for typos in the GUID definitions. It also makes checkpatch complain less about efi.h GUID additions, so that if you add another one with the same style, checkpatch won't complain about it. Signed-off-by: Peter Jones Signed-off-by: Matt Fleming Cc: Ard Biesheuvel Cc: Linus Torvalds Cc: Peter Zijlstra Cc: Thomas Gleixner Cc: linux-...@vger.kernel.org Link: http://lkml.kernel.org/r/1455712566-16727-2-git-send-email-m...@codeblueprint.co.uk Signed-off-by: Ingo Molnar --- include/linux/efi.h | 63 +++-- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/include/linux/efi.h b/include/linux/efi.h index 1acd723..42be9c9 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -536,67 +536,88 @@ void efi_native_runtime_setup(void); * EFI Configuration Table and GUID definitions */ #define NULL_GUID \ -EFI_GUID( 0x, 0x, 0x, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 ) + EFI_GUID(0x, 0x, 0x, \ +0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00) #define MPS_TABLE_GUID\ -EFI_GUID( 0xeb9d2d2f, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d ) + EFI_GUID(0xeb9d2d2f, 0x2d88, 0x11d3, \ +0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) #define ACPI_TABLE_GUID\ -EFI_GUID( 0xeb9d2d30, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d ) + EFI_GUID(0xeb9d2d30, 0x2d88, 0x11d3, \ +0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) #define ACPI_20_TABLE_GUID\ -EFI_GUID( 0x8868e871, 0xe4f1, 0x11d3, 0xbc, 0x22, 0x0, 0x80, 0xc7, 0x3c, 0x88, 0x81 ) + EFI_GUID(0x8868e871, 0xe4f1, 0x11d3, \ +0xbc, 0x22, 0x00, 0x80, 0xc7, 0x3c, 0x88, 0x81) #define SMBIOS_TABLE_GUID\ -EFI_GUID( 0xeb9d2d31, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d ) + EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3, \ +0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) #define SMBIOS3_TABLE_GUID\ -EFI_GUID( 0xf2fd1544, 0x9794, 0x4a2c, 0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94 ) + EFI_GUID(0xf2fd1544, 0x9794, 0x4a2c, \ +0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94) #define SAL_SYSTEM_TABLE_GUID\ -EFI_GUID( 0xeb9d2d32, 0x2d88, 0x11d3, 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d ) + EFI_GUID(0xeb9d2d32, 0x2d88, 0x11d3, \ +0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d) #define HCDP_TABLE_GUID\ -EFI_GUID( 0xf951938d, 0x620b, 0x42ef, 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 ) + EFI_GUID(0xf951938d, 0x620b, 0x42ef, \ +0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98) #define UGA_IO_PROTOCOL_GUID \ -EFI_GUID( 0x61a4d49e, 0x6f68, 0x4f1b, 0xb9, 0x22, 0xa8, 0x6e, 0xed, 0xb, 0x7, 0xa2 ) + EFI_GUID(0x61a4d49e, 0x6f68, 0x4f1b, \ +0xb9, 0x22, 0xa8, 0x6e, 0xed, 0x0b, 0x07, 0xa2) #define EFI_GLOBAL_VARIABLE_GUID \ -EFI_GUID( 0x8be4df61, 0x93ca, 0x11d2, 0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c ) + EFI_GUID(0x8be4df61, 0x93ca, 0x11d2, \ +0xaa, 0x0d, 0x00, 0xe0, 0x98, 0x03, 0x2b, 0x8c) #define UV_SYSTEM_TABLE_GUID \ -EFI_GUID( 0x3b13a7d4, 0x633e, 0x11dd, 0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93 ) + EFI_GUID(0x3b13a7d4, 0x633e, 0x11dd, \ +0x93, 0xec, 0xda, 0x25, 0x56, 0xd8, 0x95, 0x93) #define LINUX_EFI_CRASH_GUID \ -EFI_GUID( 0xcfc8fc79, 0xbe2e, 0x4ddc, 0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0 ) + EFI_GUID(0xcfc8fc79, 0xbe2e, 0x4ddc, \ +0x97, 0xf0, 0x9f, 0x98, 0xbf, 0xe2, 0x98, 0xa0) #define LOADED_IMAGE_PROTOCOL_GUID \ -EFI_GUID( 0x5b1b31a1, 0x9562, 0x11d2, 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b ) + EFI_GUID(0x5b1b31a1, 0x9562, 0x11d2, \ +0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) #define EFI_GRAPHICS_OUTPUT_PROTOCOL_GUID \ -EFI_GUID( 0x9042a9de, 0x23dc, 0x4a38, 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a ) + EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \ +0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a) #define EFI_UGA_PROTOCOL_GUID \ -EFI_GUID( 0x982c298b, 0xf4fa, 0x41cb, 0xb8, 0x38, 0x77, 0xaa, 0x68, 0x8f, 0xb8, 0x39 ) + EFI_GUID(0x982c298b, 0xf4fa, 0x41cb, \ +0xb8, 0x38, 0x77, 0xaa, 0x68, 0x8f, 0xb8, 0x39) #define EFI_PCI_IO_PROTOCOL_GUID \ -EFI_GUID( 0x4cf5b200, 0x68b8, 0x4ca5, 0x9e, 0xec, 0xb2, 0x3e, 0x3f, 0x50, 0x2, 0x9a ) + EFI_GUID(0x4cf
[tip:efi/core] efi: Document #define FOO_PROTOCOL_GUID layout
Commit-ID: 54fd11fee59e7d05287bc4eebccc8ec9742f2745
Gitweb: http://git.kernel.org/tip/54fd11fee59e7d05287bc4eebccc8ec9742f2745
Author: Peter Jones
AuthorDate: Sat, 25 Jun 2016 08:20:25 +0100
Committer: Ingo Molnar
CommitDate: Mon, 27 Jun 2016 13:06:55 +0200
efi: Document #define FOO_PROTOCOL_GUID layout
Add a comment documenting why EFI GUIDs are laid out like they are.
Ideally I'd like to change all the ", " to "," too, but right now the
format is such that checkpatch won't complain with new ones, and staring
at checkpatch didn't get me anywhere towards making that work.
Signed-off-by: Peter Jones
Signed-off-by: Matt Fleming
Cc: Ard Biesheuvel
Cc: Joe Perches
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-...@vger.kernel.org
Link:
http://lkml.kernel.org/r/1466839230-12781-3-git-send-email-m...@codeblueprint.co.uk
Signed-off-by: Ingo Molnar
---
include/linux/efi.h | 17 -
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/include/linux/efi.h b/include/linux/efi.h
index f196dd0..0300969 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -536,7 +536,22 @@ typedef efi_status_t efi_query_variable_store_t(u32
attributes,
void efi_native_runtime_setup(void);
/*
- * EFI Configuration Table and GUID definitions
+ * EFI Configuration Table and GUID definitions
+ *
+ * These should be formatted roughly like the ones in the UEFI SPEC has
+ * them. It makes them easier to grep for, and they look the same when
+ * you're staring at them. Here's the guide:
+ *
+ * GUID: 12345678-1234-1234-1234-123456789012
+ * Spec:
+ * #define EFI_SOME_PROTOCOL_GUID \
+ *{0x12345678,0x1234,0x1234,\
+ * {0x12,0x34,0x12,0x34,0x56,0x78,0x90,0x12}}
+ * Here:
+ * #define SOME_PROTOCOL_GUID \
+ * EFI_GUID(0x12345678, 0x1234, 0x1234, \
+ * 0x12, 0x34, 0x12, 0x34, 0x56, 0x78, 0x90, 0x12)
+ * ^ tab ^tab^ space
*/
#define NULL_GUID \
EFI_GUID(0x, 0x, 0x, \
[tip: efi/urgent] efi: Make it possible to disable efivar_ssdt entirely
The following commit has been merged into the efi/urgent branch of tip: Commit-ID: 435d1a471598752446a72ad1201b3c980526d869 Gitweb: https://git.kernel.org/tip/435d1a471598752446a72ad1201b3c980526d869 Author:Peter Jones AuthorDate:Mon, 15 Jun 2020 16:24:08 -04:00 Committer: Ard Biesheuvel CommitterDate: Tue, 16 Jun 2020 11:01:07 +02:00 efi: Make it possible to disable efivar_ssdt entirely In most cases, such as CONFIG_ACPI_CUSTOM_DSDT and CONFIG_ACPI_TABLE_UPGRADE, boot-time modifications to firmware tables are tied to specific Kconfig options. Currently this is not the case for modifying the ACPI SSDT via the efivar_ssdt kernel command line option and associated EFI variable. This patch adds CONFIG_EFI_CUSTOM_SSDT_OVERLAYS, which defaults disabled, in order to allow enabling or disabling that feature during the build. Cc: Signed-off-by: Peter Jones Link: https://lore.kernel.org/r/20200615202408.2242614-1-pjo...@redhat.com Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/Kconfig | 11 +++ drivers/firmware/efi/efi.c | 2 +- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index e6fc022..3939699 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -278,3 +278,14 @@ config EFI_EARLYCON depends on SERIAL_EARLYCON && !ARM && !IA64 select FONT_SUPPORT select ARCH_USE_MEMREMAP_PROT + +config EFI_CUSTOM_SSDT_OVERLAYS + bool "Load custom ACPI SSDT overlay from an EFI variable" + depends on EFI_VARS && ACPI + default ACPI_TABLE_UPGRADE + help + Allow loading of an ACPI SSDT overlay from an EFI variable specified + by a kernel command line option. + + See Documentation/admin-guide/acpi/ssdt-overlays.rst for more + information. diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index edc5d36..5114cae 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -189,7 +189,7 @@ static void generic_ops_unregister(void) efivars_unregister(&generic_efivars); } -#if IS_ENABLED(CONFIG_ACPI) +#ifdef CONFIG_EFI_CUSTOM_SSDT_OVERLAYS #define EFIVAR_SSDT_NAME_MAX 16 static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; static int __init efivar_ssdt_setup(char *str)
[tip:efi/core] efifb: Show framebuffer layout as device attributes
Commit-ID: 753375a881caa01112b7cec2c796749154e0bb23
Gitweb: http://git.kernel.org/tip/753375a881caa01112b7cec2c796749154e0bb23
Author: Peter Jones
AuthorDate: Tue, 18 Oct 2016 15:33:17 +0100
Committer: Ingo Molnar
CommitDate: Tue, 18 Oct 2016 17:11:19 +0200
efifb: Show framebuffer layout as device attributes
Userland sometimes needs to know what the framebuffer configuration was
when the firmware was running. This enables us to render localized
status strings during firmware updates using the data from the ACPI BGRT
table and the protocol described at the url below:
https://msdn.microsoft.com/en-us/windows/hardware/drivers/bringup/boot-screen-components
This patch also fixes up efifb's printk() usage to use pr_warn() /
pr_info() / pr_err() instead.
Tested-by: Ard Biesheuvel
Signed-off-by: Peter Jones
Signed-off-by: Ard Biesheuvel
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-...@vger.kernel.org
Link: http://lkml.kernel.org/r/20161018143318.15673-8-m...@codeblueprint.co.uk
Signed-off-by: Ingo Molnar
---
drivers/video/fbdev/efifb.c | 59 +++--
1 file changed, 46 insertions(+), 13 deletions(-)
diff --git a/drivers/video/fbdev/efifb.c b/drivers/video/fbdev/efifb.c
index 37a37c4..8c4dc1e 100644
--- a/drivers/video/fbdev/efifb.c
+++ b/drivers/video/fbdev/efifb.c
@@ -118,6 +118,31 @@ static inline bool fb_base_is_valid(void)
return false;
}
+#define efifb_attr_decl(name, fmt) \
+static ssize_t name##_show(struct device *dev, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ return sprintf(buf, fmt "\n", (screen_info.lfb_##name));\
+} \
+static DEVICE_ATTR_RO(name)
+
+efifb_attr_decl(base, "0x%x");
+efifb_attr_decl(linelength, "%u");
+efifb_attr_decl(height, "%u");
+efifb_attr_decl(width, "%u");
+efifb_attr_decl(depth, "%u");
+
+static struct attribute *efifb_attrs[] = {
+ &dev_attr_base.attr,
+ &dev_attr_linelength.attr,
+ &dev_attr_width.attr,
+ &dev_attr_height.attr,
+ &dev_attr_depth.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(efifb);
+
static int efifb_probe(struct platform_device *dev)
{
struct fb_info *info;
@@ -205,14 +230,13 @@ static int efifb_probe(struct platform_device *dev)
} else {
/* We cannot make this fatal. Sometimes this comes from magic
spaces our resource handlers simply don't know about */
- printk(KERN_WARNING
- "efifb: cannot reserve video memory at 0x%lx\n",
+ pr_warn("efifb: cannot reserve video memory at 0x%lx\n",
efifb_fix.smem_start);
}
info = framebuffer_alloc(sizeof(u32) * 16, &dev->dev);
if (!info) {
- printk(KERN_ERR "efifb: cannot allocate framebuffer\n");
+ pr_err("efifb: cannot allocate framebuffer\n");
err = -ENOMEM;
goto err_release_mem;
}
@@ -230,16 +254,15 @@ static int efifb_probe(struct platform_device *dev)
info->screen_base = ioremap_wc(efifb_fix.smem_start,
efifb_fix.smem_len);
if (!info->screen_base) {
- printk(KERN_ERR "efifb: abort, cannot ioremap video memory "
- "0x%x @ 0x%lx\n",
+ pr_err("efifb: abort, cannot ioremap video memory 0x%x @
0x%lx\n",
efifb_fix.smem_len, efifb_fix.smem_start);
err = -EIO;
goto err_release_fb;
}
- printk(KERN_INFO "efifb: framebuffer at 0x%lx, using %dk, total %dk\n",
+ pr_info("efifb: framebuffer at 0x%lx, using %dk, total %dk\n",
efifb_fix.smem_start, size_remap/1024, size_total/1024);
- printk(KERN_INFO "efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
+ pr_info("efifb: mode is %dx%dx%d, linelength=%d, pages=%d\n",
efifb_defined.xres, efifb_defined.yres,
efifb_defined.bits_per_pixel, efifb_fix.line_length,
screen_info.pages);
@@ -247,7 +270,7 @@ static int efifb_probe(struct platform_device *dev)
efifb_defined.xres_virtual = efifb_defined.xres;
efifb_defined.yres_virtual = efifb_fix.smem_len /
efifb_fix.line_length;
- printk(KERN_INFO "efifb: scrolling: redraw\n");
+ pr_info("efifb: scrolling: redraw\n");
efifb_defin
