Re: [PATCH] Documentation/x86/boot.rst: Correct the example of SETUP_INDIRECT
On Wed, Jan 27, 2021 at 04:49:11PM +0800, Cao jin wrote: > struct setup_data.len is the length of data field. In case of > SETUP_INDIRECT, it should be sizeof(setup_indirect). > > Signed-off-by: Cao jin Reviewed-by: Daniel Kiper Daniel
[SPECIFICATION RFC] The firmware and bootloader log specification
Hey,
This is next attempt to create firmware and bootloader log specification.
Due to high interest among industry it is an extension to the initial
bootloader log only specification. It takes into the account most of the
comments which I got up until now.
The goal is to pass all logs produced by various boot components to the
running OS. The OS kernel should expose these logs to the user space
and/or process them internally if needed. The content of these logs
should be human readable. However, they should also contain the
information which allows admins to do e.g. boot time analysis.
The log specification should be as much as possible platform agnostic
and self contained. The final version of this spec should be merged into
existing specifications, e.g. UEFI, ACPI, Multiboot2, or be a standalone
spec, e.g. as a part of OASIS Standards. The former seems better but is
not perfect too...
Here is the description (pseudocode) of the structures which will be
used to store the log data.
struct bf_log
{
uint32_t version;
char producer[64];
uint64_t flags;
uint64_t next_bf_log_addr;
uint32_t next_msg_off;
bf_log_msg msgs[];
}
struct bf_log_msg
{
uint32_t size;
uint64_t ts_nsec;
uint32_t level;
uint32_t facility;
uint32_t msg_off;
char strings[];
}
The members of struct bf_log:
- version: the firmware and bootloader log format version number, 1 for now,
- producer: the producer/firmware/bootloader/... type; the length
allows ASCII UUID storage if somebody needs that functionality,
- flags: it can be used to store information about log state, e.g.
it was truncated or not (does it make sense to have an information
about the number of lost messages?),
- next_bf_log_addr: address of next bf_log struct; none if zero (I think
newer spec versions should not change anything in first 5 bf_log members;
this way older log parsers will be able to traverse/copy all logs regardless
of version used in one log or another),
- next_msg_off: the offset, in bytes, from the beginning of the bf_log struct,
of the next byte after the last log message in the msgs[]; i.e. the offset
of the next available log message slot; it is equal to the total size of
the log buffer including the bf_log struct,
- msgs: the array of log messages,
- should we add CRC or hash or signatures here?
The members of struct bf_log_msg:
- size: total size of bf_log_msg struct,
- ts_nsec: timestamp expressed in nanoseconds starting from 0,
- level: similar to syslog meaning; can be used to differentiate normal
messages
from debug messages; the exact interpretation depends on the current
producer
type specified in the bf_log.producer,
- facility: similar to syslog meaning; can be used to differentiate the
sources of
the messages, e.g. message produced by networking module; the exact
interpretation
depends on the current producer type specified in the bf_log.producer,
- msg_off: the log message offset in strings[],
- strings[0]: the beginning of log message type, similar to the facility
member but
NUL terminated string instead of integer; this will be used by, e.g., the
GRUB2
for messages printed using grub_dprintf(),
- strings[msg_off]: the beginning of log message, NUL terminated string.
Note: The producers are free to use/ignore any given set of level, facility
and/or
log type members. Though the usage of these members has to be clearly
defined.
Ignored integer members should be set to 0. Ignored log message type
should
contain an empty NUL terminated string. The log message is mandatory but
can
be an empty NUL terminated string.
There is still not fully solved problem how the logs should be presented to the
OS.
On the UEFI platforms we can use config tables to do that. Then probably
bf_log.next_bf_log_addr should not be used. On the ACPI and Device Tree
platforms
we can use these mechanisms to present the logs to the OSes. The situation gets
more
difficult if neither of these mechanisms are present. However, maybe we should
not
bother too much about that because probably these platforms getting less and
less
common.
Anyway, I am aware that this is not specification per se. The goal of this
email is
to continue the discussion about the idea of the firmware and booloader log and
to
find out where the final specification should land. Of course taking into the
account
assumptions made above.
You can find previous discussions about related topics at [1], [2] and [3].
Additionally, I am going to present this during GRUB mini-summit session on
Tuesday,
17th of November at 15:45 UTC. So, if you want to discuss the log design please
join
us. You can find more details here [4].
Daniel
[1] https://lists.gnu.org/archive/html/grub-devel/2019-10/msg00107.html
[2] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00079.html
[3]
Re: [PATCH 07/13] x86: Secure Launch kernel early boot stub
On Mon, Oct 19, 2020 at 01:18:22PM -0400, Arvind Sankar wrote:
> On Mon, Oct 19, 2020 at 04:51:53PM +0200, Daniel Kiper wrote:
> > On Fri, Oct 16, 2020 at 04:51:51PM -0400, Arvind Sankar wrote:
> > > On Thu, Oct 15, 2020 at 08:26:54PM +0200, Daniel Kiper wrote:
> > > >
> > > > I am discussing with Ross the other option. We can create
> > > > .rodata.mle_header section and put it at fixed offset as
> > > > kernel_info is. So, we would have, e.g.:
> > > >
> > > > arch/x86/boot/compressed/vmlinux.lds.S:
> > > > .rodata.kernel_info KERNEL_INFO_OFFSET : {
> > > > *(.rodata.kernel_info)
> > > > }
> > > > ASSERT(ABSOLUTE(kernel_info) == KERNEL_INFO_OFFSET,
> > > > "kernel_info at bad address!")
> > > >
> > > > .rodata.mle_header MLE_HEADER_OFFSET : {
> > > > *(.rodata.mle_header)
> > > > }
> > > > ASSERT(ABSOLUTE(mle_header) == MLE_HEADER_OFFSET, "mle_header
> > > > at bad address!")
> > > >
> > > > arch/x86/boot/compressed/sl_stub.S:
> > > > #define mleh_rva(X) (((X) - mle_header) + MLE_HEADER_OFFSET)
> > > >
> > > > .section ".rodata.mle_header", "a"
> > > >
> > > > SYM_DATA_START(mle_header)
> > > > .long 0x9082ac5a/* UUID0 */
> > > > .long 0x74a7476f/* UUID1 */
> > > > .long 0xa2555c0f/* UUID2 */
> > > > .long 0x42b651cb/* UUID3 */
> > > > .long 0x0034/* MLE header size */
> > > > .long 0x00020002/* MLE version 2.2 */
> > > > .long mleh_rva(sl_stub_entry)/* Linear entry point of MLE
> > > > (virt. address) */
> > > > .long 0x/* First valid page of MLE */
> > > > .long 0x/* Offset within binary of first byte of
> > > > MLE */
> > > > .long 0x/* Offset within binary of last byte + 1
> > > > of MLE */
> > > > .long 0x0223/* Bit vector of MLE-supported
> > > > capabilities */
> > > > .long 0x/* Starting linear address of command
> > > > line (unused) */
> > > > .long 0x/* Ending linear address of command line
> > > > (unused) */
> > > > SYM_DATA_END(mle_header)
> > > >
> > > > Of course MLE_HEADER_OFFSET has to be defined as a constant somewhere.
> > > > Anyway, is it acceptable?
> >
> > What do you think about my MLE_HEADER_OFFSET and related stuff proposal?
> >
>
> I'm wondering if it would be easier to just allow relocations in these
> special "header" sections. I need to check how easy/hard it is to do
> that without triggering linker warnings.
Ross and I still bouncing some ideas. We came to the conclusion that
putting mle_header into kernel .rodata.kernel_info section or even
arch/x86/boot/compressed/kernel_info.S file would be the easiest thing
to do at this point. Of course I would suggest some renaming too. E.g.
.rodata.kernel_info to .rodata.kernel_headers, etc. Does it make sense
for you?
Daniel
Re: [PATCH 07/13] x86: Secure Launch kernel early boot stub
On Fri, Oct 16, 2020 at 04:51:51PM -0400, Arvind Sankar wrote:
> On Thu, Oct 15, 2020 at 08:26:54PM +0200, Daniel Kiper wrote:
> >
> > I am discussing with Ross the other option. We can create
> > .rodata.mle_header section and put it at fixed offset as
> > kernel_info is. So, we would have, e.g.:
> >
> > arch/x86/boot/compressed/vmlinux.lds.S:
> > .rodata.kernel_info KERNEL_INFO_OFFSET : {
> > *(.rodata.kernel_info)
> > }
> > ASSERT(ABSOLUTE(kernel_info) == KERNEL_INFO_OFFSET, "kernel_info at
> > bad address!")
> >
> > .rodata.mle_header MLE_HEADER_OFFSET : {
> > *(.rodata.mle_header)
> > }
> > ASSERT(ABSOLUTE(mle_header) == MLE_HEADER_OFFSET, "mle_header at
> > bad address!")
> >
> > arch/x86/boot/compressed/sl_stub.S:
> > #define mleh_rva(X) (((X) - mle_header) + MLE_HEADER_OFFSET)
> >
> > .section ".rodata.mle_header", "a"
> >
> > SYM_DATA_START(mle_header)
> > .long 0x9082ac5a/* UUID0 */
> > .long 0x74a7476f/* UUID1 */
> > .long 0xa2555c0f/* UUID2 */
> > .long 0x42b651cb/* UUID3 */
> > .long 0x0034/* MLE header size */
> > .long 0x00020002/* MLE version 2.2 */
> > .long mleh_rva(sl_stub_entry)/* Linear entry point of MLE
> > (virt. address) */
> > .long 0x/* First valid page of MLE */
> > .long 0x/* Offset within binary of first byte of MLE
> > */
> > .long 0x/* Offset within binary of last byte + 1 of
> > MLE */
> > .long 0x0223/* Bit vector of MLE-supported capabilities */
> > .long 0x/* Starting linear address of command line
> > (unused) */
> > .long 0x/* Ending linear address of command line
> > (unused) */
> > SYM_DATA_END(mle_header)
> >
> > Of course MLE_HEADER_OFFSET has to be defined as a constant somewhere.
> > Anyway, is it acceptable?
What do you think about my MLE_HEADER_OFFSET and related stuff proposal?
> > There is also another problem. We have to put into mle_header size of
> > the Linux kernel image. Currently it is done by the bootloader but
> > I think it is not a role of the bootloader. The kernel image should
> > provide all data describing its properties and do not rely on the
> > bootloader to do that. Ross and I investigated various options but we
> > did not find a good/simple way to do that. Could you suggest how we
> > should do that or at least where we should take a look to get some
> > ideas?
> >
> > Daniel
>
> What exactly is the size you need here? Is it just the size of the
> protected mode image, that's startup_32 to _edata. Or is it the size of
> the whole bzImage file, or something else? I guess the same question
> applies to "first valid page of MLE" and "first byte of MLE", and the
> linear entry point -- are those all relative to startup_32 or do they
> need to be relative to the start of the bzImage, i.e. you have to add
> the size of the real-mode boot stub?
>
> If you need to include the size of the bzImage file, that's not known
> when the files in boot/compressed are built. It's only known after the
> real-mode stub is linked. arch/x86/boot/tools/build.c fills in various
> details in the setup header and creates the bzImage file, but it does
> not currently modify anything in the protected-mode portion of the
> compressed kernel (i.e. arch/x86/boot/compressed/vmlinux, which then
> gets converted to binary format as arch/x86/boot/vmlinux.bin), so it
> would need to be extended if you need to modify the MLE header to
> include the bzImage size or anything depending on the size of the
> real-mode stub.
Ross clarified this. So, I not have to add much here.
Daniel
Re: [PATCH 07/13] x86: Secure Launch kernel early boot stub
On Tue, Sep 29, 2020 at 10:03:47AM -0400, Ross Philipson wrote:
> On 9/25/20 3:18 PM, Arvind Sankar wrote:
[...]
> > You should see them if you do
> > readelf -r arch/x86/boot/compressed/vmlinux
> >
> > In terms of the code, things like:
> >
> > addl%ebx, (sl_gdt_desc + 2)(%ebx)
> >
> > will create a relocation, because the linker interprets this as wanting
> > the runtime address of sl_gdt_desc, rather than just the offset from
> > startup_32.
> >
> > https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/tree/arch/x86/boot/compressed/head_64.S*n48__;Iw!!GqivPVa7Brio!JpZWv1cCPZdjD2jbCCGT7P9UIVl_lhX7YjckAnUcvi927jwZI7X3nX0MpIAZOyktJds$
> >
> > has a comment with some explanation and a macro that the 32-bit code in
> > startup_32 uses to avoid creating relocations.
> >
> > Since the SL code is in a different assembler file (and a different
> > section), you can't directly use the same macro. I would suggest getting
> > rid of sl_stub_entry and entering directly at sl_stub, and then the code
> > in sl_stub.S can use sl_stub for the base address, defining the rva()
> > macro there as
> >
> > #define rva(X) ((X) - sl_stub)
> >
> > You will also need to avoid initializing data with symbol addresses.
> >
> > .long mle_header
> > .long sl_stub_entry
> > .long sl_gdt
> >
> > will create relocations. The third one is easy, just replace it with
> > sl_gdt - sl_gdt_desc and initialize it at runtime with
> >
> > lealrva(sl_gdt_desc)(%ebx), %eax
> > addl%eax, 2(%eax)
> > lgdt(%eax)
> >
> > The other two are more messy, unfortunately there is no easy way to tell
> > the linker what we want here. The other entry point addresses (for the
> > EFI stub) are populated in a post-processing step after the compressed
> > kernel has been linked, we could teach it to also update kernel_info.
> >
> > Without that, for kernel_info, you could change it to store the offset
> > of the MLE header from kernel_info, instead of from the start of the
> > image.
> >
> > For the MLE header, it could be moved to .head.text in head_64.S, and
> > initialized with
> > .long rva(sl_stub)
> > This will also let it be placed at a fixed offset from startup_32, so
> > that kernel_info can just be populated with a constant.
I am discussing with Ross the other option. We can create
.rodata.mle_header section and put it at fixed offset as
kernel_info is. So, we would have, e.g.:
arch/x86/boot/compressed/vmlinux.lds.S:
.rodata.kernel_info KERNEL_INFO_OFFSET : {
*(.rodata.kernel_info)
}
ASSERT(ABSOLUTE(kernel_info) == KERNEL_INFO_OFFSET, "kernel_info at bad
address!")
.rodata.mle_header MLE_HEADER_OFFSET : {
*(.rodata.mle_header)
}
ASSERT(ABSOLUTE(mle_header) == MLE_HEADER_OFFSET, "mle_header at bad
address!")
arch/x86/boot/compressed/sl_stub.S:
#define mleh_rva(X) (((X) - mle_header) + MLE_HEADER_OFFSET)
.section ".rodata.mle_header", "a"
SYM_DATA_START(mle_header)
.long 0x9082ac5a/* UUID0 */
.long 0x74a7476f/* UUID1 */
.long 0xa2555c0f/* UUID2 */
.long 0x42b651cb/* UUID3 */
.long 0x0034/* MLE header size */
.long 0x00020002/* MLE version 2.2 */
.long mleh_rva(sl_stub_entry)/* Linear entry point of MLE (virt.
address) */
.long 0x/* First valid page of MLE */
.long 0x/* Offset within binary of first byte of MLE */
.long 0x/* Offset within binary of last byte + 1 of MLE */
.long 0x0223/* Bit vector of MLE-supported capabilities */
.long 0x/* Starting linear address of command line
(unused) */
.long 0x/* Ending linear address of command line (unused)
*/
SYM_DATA_END(mle_header)
Of course MLE_HEADER_OFFSET has to be defined as a constant somewhere.
Anyway, is it acceptable?
There is also another problem. We have to put into mle_header size of
the Linux kernel image. Currently it is done by the bootloader but
I think it is not a role of the bootloader. The kernel image should
provide all data describing its properties and do not rely on the
bootloader to do that. Ross and I investigated various options but we
did not find a good/simple way to do that. Could you suggest how we
should do that or at least where we should take a look to get some
ideas?
Daniel
Re: [BOOTLOADER SPECIFICATION RFC] The bootloader log format for TrenchBoot and others
On Fri, May 29, 2020 at 10:11:40AM -0700, Andy Lutomirski wrote: > > On May 29, 2020, at 4:30 AM, Daniel Kiper wrote: > > > > Hey, > > > > Below you can find my rough idea of the bootloader log format which is > > generic thing but initially will be used for TrenchBoot work. I discussed > > this proposal with Ross and Daniel S. So, the idea went through initial > > sanitization. Now I would like to take feedback from other folks too. > > So, please take a look and complain... > > > In general we want to pass the messages produced by the bootloader to the OS > > kernel and finally to the userspace for further processing and analysis. > > Below > > is the description of the structures which will be used for this thing. > > Is the intent for a human to read these, or to get them into the > system log file, or are they intended to actually change the behavior > of the system? > > IOW, what is this for? In general the idea is for a human to read these. There will be a separate tool which reads the bootloader log exposed via Linux kernel sysfs and displays its contents to the user. The tool will allow user to do various kinds of filtering. We are not going to put the contents of the bootloader log into any of system logs. However, if somebody wants to do that, in sensible way, I am OK with that. Daniel
[BOOTLOADER SPECIFICATION RFC] The bootloader log format for TrenchBoot and others
Hey,
Below you can find my rough idea of the bootloader log format which is
generic thing but initially will be used for TrenchBoot work. I discussed
this proposal with Ross and Daniel S. So, the idea went through initial
sanitization. Now I would like to take feedback from other folks too.
So, please take a look and complain...
In general we want to pass the messages produced by the bootloader to the OS
kernel and finally to the userspace for further processing and analysis. Below
is the description of the structures which will be used for this thing.
struct bootloader_log_msgs
{
grub_uint32_t level;
grub_uint32_t facility;
char type[];
char msg[];
}
struct bootloader_log
{
grub_uint32_t version;
grub_uint32_t producer;
grub_uint32_t size;
grub_uint32_t next_off;
bootloader_log_msgs msgs[];
}
The members of struct bootloader_log:
- version: the bootloader log format version number, 1 for now,
- producer: the producer/bootloader type; we can steal some values from
linux/Documentation/x86/boot.rst:type_of_loader,
- size: total size of the log buffer including the bootloader_log struct,
- next_off: offset in bytes, from start of the bootloader_log struct,
of the next byte after the last log message in the msgs[];
i.e. the offset of the next available log message slot,
- msgs: the array of log messages.
The members of struct bootloader_log_msgs:
- level: similar to syslog meaning; can be used to differentiate
normal messages from debug messages; exact interpretation depends
on the current producer/bootloader type specified in the
bootloader_log.producer,
- facility: similar to syslog meaning; can be used to differentiate
the sources of the messages, e.g. message produced by networking
module; exact interpretation depends on the current producer/bootloader
type specified in the bootloader_log.producer,
- type: similar to the facility member but NUL terminated string instead of
integer;
this will be used by GRUB for messages printed using grub_dprintf(),
- msg: the bootloader log message, NUL terminated string.
Note: The bootloaders are free to use/ignore any given set of level,
facility and/or type members. Though the usage of these members
has to be clearly defined. Ignored integer members should be set
to 0. Ignored type member should contain an empty NUL terminated
string. msg member is mandatory but can be an empty NUL terminated
string.
Taking into account [1] and [2] I want to make this functionality generic
as much as possible. So, this bootloader log can be used with any bootloader
and OS kernel. However, initially the functionality will be implemented for
the Linux kernel and its boot protocol.
In case of Linux kernel the pointer to the bootloader_log struct should
be passed from the bootloader to the kernel through the boot_params and
the bootloader_log struct contents should be exposed via sysfs. E.g.
somewhere at /sys/kernel/debug or /sys/kernel/tracing or maybe we should
create new /sys/bootloader/log node.
If everybody is OK with this rough proposal then I will start working
on making it a part of Multiboot2 specification (the text above is just
raw description of the idea; it is not final text which land into the
spec). If you see better place for this thing just drop me a line.
Daniel
[1] https://lists.gnu.org/archive/html/grub-devel/2019-10/msg00107.html
[2] https://lists.gnu.org/archive/html/grub-devel/2019-11/msg00079.html
Re: [GRUB PATCH RFC 00/18] i386: Intel TXT secure launcher
Hi Łukasz,
On Tue, May 05, 2020 at 04:38:02PM +0200, Lukasz Hawrylko wrote:
> On Tue, 2020-05-05 at 01:21 +0200, Daniel Kiper wrote:
> > Hi,
> >
> > This is an RFC patchset for the GRUB introducing the Intel TXT secure
> > launcher.
> > This is a part of larger work known as the TrenchBoot. Patchset can be split
> > into two distinct parts:
> > - 01-12: preparatory patches,
> > - 13-18: the Intel TXT secure launcher itself.
> >
> > The initial implementation of the Intel TXT secure launcher works. However,
> > there are still some missing bits and pieces, e.g.:
> > - SINIT ACM auto loader,
> > - lack of RMRR support,
> > - lack of support for MLEs larger than 1 GiB,
> > - lack of TPM 1.2 support.
> > - various fixes and cleanups.
> >
> > Commands introduced by this patchset: tpm_type, slaunch, slaunch_module (not
> > required on server platforms) and slaunch_state (useful for checking
> > platform
> > configuration and state; based on tboot's txt-stat).
> >
> > Daniel
> >
>
> Hi Daniel
>
> Your patch looks promising, however I have few concerns.
Below I will be referring to the Intel Trusted Execution Technology
(Intel TXT), Software Development Guide, December 2019, Revision 016.
So, the latest and greatest...
> In OS-MLE table there is a buffer for TPM event log, however I see that
> you are not using it, but instead allocate space somewhere in the
I think that this part requires more discussion. In my opinion we should
have this region dynamically allocated because it gives us more flexibility.
Of course there is a question about the size of this buffer too. I am
not sure about that because I have not checked yet how many log entries
are created by the SINIT ACM. Though probably it should not be large...
> memory. I am just wondering if, from security perspective, it will be
> better to use memory from TXT heap for event log, like we do in TBOOT.
Appendix F, TPM Event Log, has following sentence: There are no
requirements for event log to be in DMA protected memory – SINIT will
not enforce it.
I was thinking about it and it seems to me that the TPM event log does
not require any special protections. Any changes in it can be quickly
detected by comparing hashes with the TPM PCRs. Does not it?
> There is a function that verifies if platform is TXT capable
> -grub_txt_verify_platform(), it only checks SMX and GETSEC features.
> Although BIOS should enforce both VMX and VT-d enabled when enabling
> TXT, I think that adding these check here as redundancy may be a good
The TXT spec has the following pseudocode:
//
// Intel TXT detection
// Execute on all logical processors for compatibility with
// multiple processor systems
//
1. CPUID(EAX=1);
2. IF (SMX not supported) OR (VMX not supported) {
3. Fail measured environment startup;
4. }
However, a few lines above you can find this:
Lines 1 - 4: Before attempting to launch the measured environment, the
system software should check that all logical processors support VMX and
SMX (the check for VMX support is not necessary if the environment to be
launched will not use VMX).
Hence, AIUI, I am allowed to check SMX only. And I do not think that the
bootloader should enforce VMX. If the kernel wants VMX then it should
check the platform config. The booloader should just look for features
which are really required to properly execute GETSEC[SENTER].
> idea. The same situation is with TPM presence.
The TPM presence is checked in init_txt_heap(). However, we can do it earlier.
> I suggest to add possibility to skip TXT launch when last boot ended
> with TXT error. This option can avoid boot loops when something goes
> wrong.
grub_txt_verify_platform() checks TXT_RESET.STS bit and fails if it is
set. This produces an error during boot. Well, but it does not prevent
it... :-( Probably I have to fix it...
> How will you read LCP from storage? I see that there is slaunch_module
> command that currently you are using only for loading SINIT. In the
> future it can be expanded to support LCP file too, what do you think?
I think that we should add e.g. slaunch_lcp command and do not overload
slaunch_module command. However, I am not planning support for it in the
near feature. I mean I will not be working on it...
> Do not forget to apply changes required by latest Intel's platforms, you
> should check following commits in TBOOT's repository: 2f03b57ffdba,
> fe2742dc.
Sure, will take that into account.
Thank you for your comments,
Daniel
PS By the way, I found an issue in TXT spec. TXT.VER.FSBIF refers to
TXT.VER.EMIF which does not exist in spec. I suppose that it is
remnant from previous TXT spec versions. It seems to me that it
should be changed to TXT.VER.QPIIF. TXT.VER.QPIIF descriptions
properly, IMO, refers back to TXT.VER.FSBIF.
Re: [GRUB PATCH RFC 12/18] i386/efi: Report UEFI Secure Boot status to the Linux kernel
On Wed, May 06, 2020 at 11:36:49AM -0700, Matthew Garrett wrote: > On Wed, May 6, 2020 at 6:33 AM Daniel Kiper wrote: > > > > On Tue, May 05, 2020 at 10:29:05AM -0700, Matthew Garrett wrote: > > > On Mon, May 4, 2020 at 4:25 PM Daniel Kiper > > > wrote: > > > > > > > > Otherwise the kernel does not know its state and cannot enable various > > > > security features depending on UEFI Secure Boot. > > > > > > I think this needs more context. If the kernel is loaded via the EFI > > > boot stub, the kernel is aware of the UEFI secure boot state. Why > > > duplicate this functionality in order to avoid the EFI stub? > > > > It seems to me that this issue was discussed here [1] and here [2]. > > So, if you want me to improve the commit message I am OK with that. > > Yes, I think just providing an explanation for why it's currently > necessary for you to duplicate this is reasonable. Sure, will do! Daniel
Re: [GRUB PATCH RFC 12/18] i386/efi: Report UEFI Secure Boot status to the Linux kernel
On Tue, May 05, 2020 at 10:29:05AM -0700, Matthew Garrett wrote: > On Mon, May 4, 2020 at 4:25 PM Daniel Kiper wrote: > > > > Otherwise the kernel does not know its state and cannot enable various > > security features depending on UEFI Secure Boot. > > I think this needs more context. If the kernel is loaded via the EFI > boot stub, the kernel is aware of the UEFI secure boot state. Why > duplicate this functionality in order to avoid the EFI stub? It seems to me that this issue was discussed here [1] and here [2]. So, if you want me to improve the commit message I am OK with that. Additionally, FYI I am not happy with that patch too. So, if somebody has better idea how to do that then I am happy to discuss it here. Daniel [1] https://lkml.org/lkml/2020/3/25/982 [2] https://lkml.org/lkml/2020/3/26/985
[GRUB PATCH RFC 15/18] i386/txt: Add Intel TXT core implementation
From: Ross Philipson
Signed-off-by: Ross Philipson
Signed-off-by: Daniel Kiper
---
grub-core/loader/i386/txt/txt.c | 887
include/grub/i386/memory.h | 5 +
2 files changed, 892 insertions(+)
create mode 100644 grub-core/loader/i386/txt/txt.c
diff --git a/grub-core/loader/i386/txt/txt.c b/grub-core/loader/i386/txt/txt.c
new file mode 100644
index 0..3e25890c3
--- /dev/null
+++ b/grub-core/loader/i386/txt/txt.c
@@ -0,0 +1,887 @@
+/*
+ * txt.c: Intel(r) TXT support functions, including initiating measured
+ *launch, post-launch, AP wakeup, etc.
+ *
+ * Copyright (c) 2003-2011, Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ * * Neither the name of the Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define OS_SINIT_DATA_TPM_12_VER 6
+#define OS_SINIT_DATA_TPM_20_VER 7
+
+#define OS_SINIT_DATA_MIN_VER OS_SINIT_DATA_TPM_12_VER
+
+static grub_err_t
+enable_smx_mode (void)
+{
+ grub_uint32_t caps;
+
+ /* Enable SMX mode. */
+ grub_write_cr4 (grub_read_cr4 () | GRUB_CR4_X86_SMXE);
+
+ caps = grub_txt_getsec_capabilities (0);
+
+ if (!(caps & GRUB_SMX_CAPABILITY_CHIPSET_PRESENT))
+{
+ grub_error (GRUB_ERR_BAD_DEVICE, N_("TXT-capable chipset is not
present"));
+ goto fail;
+}
+
+ if (!(caps & GRUB_SMX_CAPABILITY_SENTER))
+{
+ grub_error (GRUB_ERR_BAD_DEVICE, N_("GETSEC[SENTER] is not available"));
+ goto fail;
+}
+
+ if (!(caps & GRUB_SMX_CAPABILITY_PARAMETERS))
+{
+ grub_error (GRUB_ERR_BAD_DEVICE, N_("GETSEC[PARAMETERS] is not
available"));
+ goto fail;
+}
+
+ return GRUB_ERR_NONE;
+
+ fail:
+ /* Disable SMX mode on failure. */
+ grub_write_cr4 (grub_read_cr4 () & ~GRUB_CR4_X86_SMXE);
+
+ return grub_errno;
+}
+
+static void
+grub_txt_smx_parameters (struct grub_smx_parameters *params)
+{
+ grub_uint32_t index = 0, eax, ebx, ecx, param_type;
+
+ grub_memset (params, 0, sizeof(struct grub_smx_supported_versions));
+
+ params->max_acm_size = GRUB_SMX_DEFAULT_MAX_ACM_SIZE;
+ params->acm_memory_types = GRUB_SMX_DEFAULT_ACM_MEMORY_TYPE;
+ params->senter_controls = GRUB_SMX_DEFAULT_SENTER_CONTROLS;
+
+ do
+{
+ grub_txt_getsec_parameters (index, , , );
+ param_type = eax & GRUB_SMX_PARAMETER_TYPE_MASK;
+
+ switch (param_type)
+{
+case GRUB_SMX_PARAMETER_NULL:
+ break; /* This means done. */
+
+case GRUB_SMX_PARAMETER_ACM_VERSIO
[GRUB PATCH RFC 12/18] i386/efi: Report UEFI Secure Boot status to the Linux kernel
Otherwise the kernel does not know its state and cannot enable various
security features depending on UEFI Secure Boot.
Signed-off-by: Ignat Korchagin
Signed-off-by: Daniel Kiper
---
grub-core/loader/i386/linux.c | 86 ++-
include/grub/i386/linux.h | 14 ++-
2 files changed, 97 insertions(+), 3 deletions(-)
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index ac1fae72e..952eb1191 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -397,6 +397,87 @@ grub_linux_boot_mmap_fill (grub_uint64_t addr,
grub_uint64_t size,
return 0;
}
+#ifdef GRUB_MACHINE_EFI
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with the Linux kernel,
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static grub_uint8_t
+grub_efi_get_secureboot (void)
+{
+ grub_efi_guid_t efi_variable_guid = GRUB_EFI_GLOBAL_VARIABLE_GUID;
+ grub_efi_guid_t efi_shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
+ grub_efi_status_t status;
+ grub_efi_uint32_t attr = 0;
+ grub_size_t size = 0;
+ grub_uint8_t *secboot = NULL;
+ grub_uint8_t *setupmode = NULL;
+ grub_uint8_t *moksbstate = NULL;
+ grub_uint8_t secureboot = GRUB_LINUX_EFI_SECUREBOOT_MODE_UNKNOWN;
+ const char *secureboot_str = "UNKNOWN";
+
+ status = grub_efi_get_variable ("SecureBoot", _variable_guid,
+ , (void **) );
+
+ if (status == GRUB_EFI_NOT_FOUND)
+{
+ secureboot = GRUB_LINUX_EFI_SECUREBOOT_MODE_DISABLED;
+ goto out;
+}
+
+ if (status != GRUB_EFI_SUCCESS)
+goto out;
+
+ status = grub_efi_get_variable ("SetupMode", _variable_guid,
+ , (void **) );
+
+ if (status != GRUB_EFI_SUCCESS)
+goto out;
+
+ if ((*secboot == 0) || (*setupmode == 1))
+{
+ secureboot = GRUB_LINUX_EFI_SECUREBOOT_MODE_DISABLED;
+ goto out;
+}
+
+ /*
+ * See if a user has put the shim into insecure mode. If so, and if the
+ * variable doesn't have the runtime attribute set, we might as well
+ * honor that.
+ */
+ status = grub_efi_get_variable_with_attributes ("MokSBState",
_shim_lock_guid,
+ , (void **) ,
);
+
+ /* If it fails, we don't care why. Default to secure. */
+ if (status != GRUB_EFI_SUCCESS)
+{
+ secureboot = GRUB_LINUX_EFI_SECUREBOOT_MODE_ENABLED;
+ goto out;
+}
+
+ if (!(attr & GRUB_EFI_VARIABLE_RUNTIME_ACCESS) && *moksbstate == 1)
+secureboot = GRUB_LINUX_EFI_SECUREBOOT_MODE_DISABLED;
+
+ secureboot = GRUB_LINUX_EFI_SECUREBOOT_MODE_ENABLED;
+
+ out:
+ grub_free (moksbstate);
+ grub_free (setupmode);
+ grub_free (secboot);
+
+ if (secureboot == GRUB_LINUX_EFI_SECUREBOOT_MODE_DISABLED)
+secureboot_str = "Disabled";
+ else if (secureboot == GRUB_LINUX_EFI_SECUREBOOT_MODE_ENABLED)
+secureboot_str = "Enabled";
+
+ grub_dprintf ("linux", "UEFI Secure Boot state: %s\n", secureboot_str);
+
+ return secureboot;
+}
+#endif
+
static grub_err_t
grub_linux_boot (void)
{
@@ -579,6 +660,9 @@ grub_linux_boot (void)
grub_efi_uintn_t efi_desc_size;
grub_size_t efi_mmap_target;
grub_efi_uint32_t efi_desc_version;
+
+ctx.params->secure_boot = grub_efi_get_secureboot ();
+
err = grub_efi_finish_boot_services (_mmap_size, efi_mmap_buf, NULL,
_desc_size, _desc_version);
if (err)
@@ -790,7 +874,7 @@ grub_cmd_linux (grub_command_t cmd __attribute__ ((unused)),
linux_params.code32_start = prot_mode_target + lh.code32_start -
GRUB_LINUX_BZIMAGE_ADDR;
linux_params.kernel_alignment = (1 << align);
- linux_params.ps_mouse = linux_params.padding10 = 0;
+ linux_params.ps_mouse = linux_params.padding11 = 0;
linux_params.type_of_loader = GRUB_LINUX_BOOT_LOADER_TYPE;
/* These two are used (instead of cmd_line_ptr) by older versions of Linux,
diff --git a/include/grub/i386/linux.h b/include/grub/i386/linux.h
index ce30e7fb0..6aea73ddb 100644
--- a/include/grub/i386/linux.h
+++ b/include/grub/i386/linux.h
@@ -49,6 +49,12 @@
/* Maximum number of MBR signatures to store. */
#define EDD_MBR_SIG_MAX16
+/* Possible values for Linux secure_boot kernel parameter. */
+#define GRUB_LINUX_EFI_SECUREBOOT_MODE_UNSET 0
+#define GRUB_LINUX_EFI_SECUREBOOT_MODE_UNKNOWN 1
+#define GRUB_LINUX_EFI_SECUREBOOT_MODE_DISABLED2
+#define GRUB_LINUX_EFI_SECUREBOOT_MODE_ENABLED 3
+
#ifdef __x86_64__
#define GRUB_LINUX_EFI_SIGNATURE \
@@ -275,7 +281,11 @@ struct linux_kernel_params
grub_uint8_t mmap_size; /* 1e8 */
- grub_uint8_t padding9[0x1f1 - 0x1e9];
+ grub_uint8_t padding9[0x1ec - 0x1e9];
+
+ grub_uint8_t secure_boot; /* 1ec */
+
+ grub_uint8_t padding10[0x1f1 - 0x1ed];
/* Linux setu
[GRUB PATCH RFC 08/18] i386/tpm: Add TPM TIS and CRB driver
It will be used by Intel TXT secure launcher introduced
by subsequent patches.
Signed-off-by: Daniel Kiper
---
grub-core/commands/i386/tpm.c | 182 ++
include/grub/i386/tpm.h | 36 +
2 files changed, 218 insertions(+)
create mode 100644 grub-core/commands/i386/tpm.c
create mode 100644 include/grub/i386/tpm.h
diff --git a/grub-core/commands/i386/tpm.c b/grub-core/commands/i386/tpm.c
new file mode 100644
index 0..ff29c2e85
--- /dev/null
+++ b/grub-core/commands/i386/tpm.c
@@ -0,0 +1,182 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Free Software Foundation, Inc.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * TPM TIS and CRB driver.
+ *
+ * Note: It is suggested to not use this driver together with UEFI TPM driver.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+GRUB_MOD_LICENSE ("GPLv3+");
+
+#define TPM_MMIO_BASE 0xfed4
+
+/* TIS registers. */
+#define TPM_ACCESS 0x
+#define TPM_INTF_CAPABILITY0x0014
+#define TPM_INTERFACE_ID 0x0030
+
+/* CRB registers. */
+#define TPM_LOC_CTRL 0x0008
+
+#define TPM_12_TIS_INTF_12 0x0
+#define TPM_12_TIS_INTF_13 0x2
+#define TPM_20_TIS_INTF_13 0x3
+
+#define TPM_CRB_INTF_ACTIVE0x1
+
+#define TIS_RELINQUISH_LCL 0x20
+#define CRB_RELINQUISH_LCL 0x0002
+
+/* TODO: Do we need GRUB_PACKED for unions below??? */
+
+union tpm_interface_id
+{
+ grub_uint32_t raw;
+ struct
+ {
+grub_uint32_t interface_type:4;
+grub_uint32_t interface_version:4;
+grub_uint32_t cap_locality:1;
+grub_uint32_t reserved_0:4;
+grub_uint32_t cap_tis:1;
+grub_uint32_t cap_crb:1;
+grub_uint32_t cap_ifres:2;
+grub_uint32_t interface_selector:2;
+grub_uint32_t intf_sel_lock:1;
+grub_uint32_t reserved_1:4;
+grub_uint32_t reserved_2:8;
+ };
+} GRUB_PACKED;
+typedef union tpm_interface_id tpm_interface_id_t;
+
+union tpm_intf_capability
+{
+ grub_uint32_t raw;
+ struct
+ {
+grub_uint32_t data_avail_int_support:1;
+grub_uint32_t sts_valid_int_support:1;
+grub_uint32_t locality_change_int_support:1;
+grub_uint32_t interrupt_level_high:1;
+grub_uint32_t interrupt_level_low:1;
+grub_uint32_t interrupt_edge_rising:1;
+grub_uint32_t interrupt_edge_falling:1;
+grub_uint32_t command_ready_int_support:1;
+grub_uint32_t burst_count_static:1;
+grub_uint32_t data_transfer_size_support:2;
+grub_uint32_t reserved_0:17;
+grub_uint32_t interface_version:3;
+grub_uint32_t reserved_1:1;
+ };
+} GRUB_PACKED;
+typedef union tpm_intf_capability tpm_intf_capability_t;
+
+typedef enum
+ {
+TPM_INTF_NONE = 0,
+TPM_INTF_TIS,
+TPM_INTF_CRB
+ }
+tpm_intf_t;
+
+static grub_tpm_ver_t tpm_ver = GRUB_TPM_NONE;
+static tpm_intf_t tpm_intf = TPM_INTF_NONE;
+
+grub_tpm_ver_t
+grub_get_tpm_ver (void)
+{
+ return tpm_ver;
+}
+
+/* Localities 0-4 are supported only. */
+void
+grub_tpm_relinquish_lcl (grub_uint8_t lcl)
+{
+ grub_addr_t addr = TPM_MMIO_BASE + lcl * GRUB_PAGE_SIZE;
+
+ if (tpm_intf == TPM_INTF_TIS)
+grub_writeb (TIS_RELINQUISH_LCL, (void *) (addr + TPM_ACCESS));
+ else if (tpm_intf == TPM_INTF_CRB)
+grub_writel (CRB_RELINQUISH_LCL, (void *) (addr + TPM_LOC_CTRL));
+}
+
+static grub_err_t
+grub_cmd_tpm_type (grub_command_t cmd __attribute__ ((unused)),
+ int argc __attribute__ ((unused)),
+ char *argv[] __attribute__ ((unused)))
+{
+ const char *tpm_ver_s = "NONE";
+ const char *tpm_intf_s = "NONE";
+
+ if (tpm_ver == GRUB_TPM_12)
+tpm_ver_s = "1.2";
+ else if (tpm_ver == GRUB_TPM_20)
+tpm_ver_s = "2.0";
+
+ if (tpm_intf == TPM_INTF_TIS)
+tpm_intf_s = "TIS";
+ else if (tpm_intf == TPM_INTF_CRB)
+tpm_intf_s = "CRB";
+
+ grub_printf ("TPM VER: %s\nTPM INTF: %s\n", tpm_ver_s, tpm_intf_s);
+
+ return GRUB_ERR_NONE;
+}
+
+static grub_command_t cmd_tpm_type;
+
+GRUB_MOD_INIT (tpm)
+{
+ tpm_interface_id_t intf_id;
+ tpm_intf_capability_t intf_cap;
+
+ cmd_tpm_type = grub_register_command ("tpm_type", grub_cmd_tpm_type,
+ NULL, N_("Show TPM version and
interface type."));
+
+ intf_cap.raw = grub
[GRUB PATCH RFC 07/18] i386/tpm: Rename tpm module to tpm_verifier
..to avoid naming collision with TPM TIS and CRB driver introduced
by subsequent patch.
Signed-off-by: Daniel Kiper
---
docs/grub.texi | 15 ---
grub-core/Makefile.core.def | 4 ++--
grub-core/commands/{tpm.c => tpm_verifier.c} | 6 +++---
3 files changed, 13 insertions(+), 12 deletions(-)
rename grub-core/commands/{tpm.c => tpm_verifier.c} (96%)
diff --git a/docs/grub.texi b/docs/grub.texi
index d6408d242..395431bce 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -5940,10 +5940,10 @@ it cannot be unloaded if it was loaded into the memory.
@node Measured Boot
@section Measuring boot components
-If the tpm module is loaded and the platform has a Trusted Platform Module
-installed, GRUB will log each command executed and each file loaded into the
-TPM event log and extend the PCR values in the TPM correspondingly. All events
-will be logged into the PCR described below with a type of EV_IPL and an
+If the tpm_verifier module is loaded and the platform has a Trusted Platform
+Module installed, GRUB will log each command executed and each file loaded into
+the TPM event log and extend the PCR values in the TPM correspondingly. All
+events will be logged into the PCR described below with a type of EV_IPL and an
event description as described below.
@multitable @columnfractions 0.3 0.1 0.6
@@ -5968,9 +5968,10 @@ corresponding to the filename.
GRUB will not measure its own @file{core.img} - it is expected that firmware
will carry this out. GRUB will also not perform any measurements until the
-tpm module is loaded. As such it is recommended that the tpm module be built
-into @file{core.img} in order to avoid a potential gap in measurement between
-@file{core.img} being loaded and the tpm module being loaded.
+tpm_verifier module is loaded. As such it is recommended that the tpm_verifier
+module be built into @file{core.img} in order to avoid a potential gap in
+measurement between @file{core.img} being loaded and the tpm_verifier module
+being loaded.
Measured boot is currently only supported on EFI platforms.
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index 48b82e763..b74a34f0c 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -2497,8 +2497,8 @@ module = {
};
module = {
- name = tpm;
- common = commands/tpm.c;
+ name = tpm_verifier;
+ common = commands/tpm_verifier.c;
efi = commands/efi/tpm.c;
enable = x86_64_efi;
};
diff --git a/grub-core/commands/tpm.c b/grub-core/commands/tpm_verifier.c
similarity index 96%
rename from grub-core/commands/tpm.c
rename to grub-core/commands/tpm_verifier.c
index 1441c494d..6fff17720 100644
--- a/grub-core/commands/tpm.c
+++ b/grub-core/commands/tpm_verifier.c
@@ -85,18 +85,18 @@ grub_tpm_verify_string (char *str, enum
grub_verify_string_type type)
}
struct grub_file_verifier grub_tpm_verifier = {
- .name = "tpm",
+ .name = "tpm_verifier",
.init = grub_tpm_verify_init,
.write = grub_tpm_verify_write,
.verify_string = grub_tpm_verify_string,
};
-GRUB_MOD_INIT (tpm)
+GRUB_MOD_INIT (tpm_verifier)
{
grub_verifier_register (_tpm_verifier);
}
-GRUB_MOD_FINI (tpm)
+GRUB_MOD_FINI (tpm_verifier)
{
grub_verifier_unregister (_tpm_verifier);
}
--
2.11.0
[GRUB PATCH RFC 02/18] i386/msr: Rename grub_msr_read() and grub_msr_write()
.. to grub_rdmsr() and grub_wrmsr() respectively. New names are more
obvious than older ones.
Signed-off-by: Daniel Kiper
---
grub-core/commands/i386/rdmsr.c | 2 +-
grub-core/commands/i386/wrmsr.c | 2 +-
include/grub/i386/msr.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index fa4622f9e..89ece7657 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -76,7 +76,7 @@ grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char
**argv)
if (*ptr != '\0')
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));
- value = grub_msr_read (addr);
+ value = grub_rdmsr (addr);
if (ctxt->state[0].set)
{
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index 9b7abba7c..e3e9f2ee3 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -76,7 +76,7 @@ grub_cmd_msr_write (grub_command_t cmd __attribute__
((unused)), int argc, char
if (*ptr != '\0')
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));
- grub_msr_write (addr, value);
+ grub_wrmsr (addr, value);
return GRUB_ERR_NONE;
}
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 7b52b5d61..4fba1b8e0 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -25,7 +25,7 @@
*/
static inline grub_uint64_t
-grub_msr_read (grub_uint32_t msr_id)
+grub_rdmsr (grub_uint32_t msr_id)
{
grub_uint32_t low, high;
@@ -35,7 +35,7 @@ grub_msr_read (grub_uint32_t msr_id)
}
static inline void
-grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
+grub_wrmsr (grub_uint32_t msr_id, grub_uint64_t msr_value)
{
grub_uint32_t low = msr_value, high = msr_value >> 32;
--
2.11.0
[GRUB PATCH RFC 14/18] i386/txt: Add Intel TXT definitions header file
From: Ross Philipson
Signed-off-by: Ross Philipson
Signed-off-by: Daniel Kiper
---
include/grub/i386/txt.h | 690
1 file changed, 690 insertions(+)
create mode 100644 include/grub/i386/txt.h
diff --git a/include/grub/i386/txt.h b/include/grub/i386/txt.h
new file mode 100644
index 0..8280e461e
--- /dev/null
+++ b/include/grub/i386/txt.h
@@ -0,0 +1,690 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Intel TXT definitions header file.
+ */
+
+#ifndef GRUB_TXT_H
+#define GRUB_TXT_H 1
+
+#include
+#include
+#include
+#include
+#include
+
+/* Intel TXT Software Developers Guide */
+
+/* Chapter 2, Table 2 MLE/SINIT Capabilities Field Bit Definitions */
+
+#define GRUB_TXT_PCR_EXT_MAX_AGILITY_POLICY0
+#define GRUB_TXT_PCR_EXT_MAX_PERF_POLICY 1
+
+#define GRUB_TXT_PLATFORM_TYPE_LEGACY 0
+#define GRUB_TXT_PLATFORM_TYPE_CLIENT 1
+#define GRUB_TXT_PLATFORM_TYPE_SERVER 2
+#define GRUB_TXT_PLATFORM_TYPE_RESERVED3
+
+#define GRUB_TXT_CAPS_GETSEC_WAKE_SUPPORT (1<<0)
+#define GRUB_TXT_CAPS_MONITOR_SUPPORT (1<<1)
+#define GRUB_TXT_CAPS_ECX_PT_SUPPORT (1<<2)
+#define GRUB_TXT_CAPS_STM_SUPPORT (1<<3)
+#define GRUB_TXT_CAPS_TPM_12_NO_LEGACY_PCR_USAGE (1<<4)
+#define GRUB_TXT_CAPS_TPM_12_AUTH_PCR_USAGE(1<<5)
+#define GRUB_TXT_CAPS_PLATFORM_TYPE(3<<6)
+#define GRUB_TXT_CAPS_MAXPHYSADDR_SUPPORT (1<<8)
+#define GRUB_TXT_CAPS_TPM_20_EVTLOG_SUPPORT(1<<9)
+#define GRUB_TXT_CAPS_CBNT_SUPPORT (1<<10)
+/* Rest is reserved */
+
+/* Appendix A TXT Execution Technology Authenticated Code Modules */
+/* A.1 Authenticated Code Module Format */
+
+#define GRUB_TXT_ACM_MODULE_TYPE 2
+
+#define GRUB_TXT_ACM_MODULE_SUB_TYPE_TXT_ACM 0
+#define GRUB_TXT_ACM_MODULE_SUB_TYPE_S_ACM 1
+
+#define GRUB_TXT_ACM_HEADER_LEN_0_0161
+#define GRUB_TXT_ACM_HEADER_LEN_3_0224
+
+#define GRUB_TXT_ACM_HEADER_VERSION_0_00x
+#define GRUB_TXT_ACM_HEADER_VERSION_3_00x0300
+
+#define GRUB_TXT_ACM_FLAG_PREPRODUCTION(1<<14)
+#define GRUB_TXT_ACM_FLAG_DEBUG_SIGNED (1<<15)
+
+#define GRUB_TXT_ACM_MODULE_VENDOR_INTEL 0x8086
+
+#define GRUB_TXT_MLE_MAX_SIZE 0x4000
+
+#define GRUB_MLE_AP_WAKE_BLOCK_SIZEGRUB_PAGE_SIZE
+
+struct grub_txt_acm_header
+{
+ grub_uint16_t module_type;
+ grub_uint16_t module_sub_type;
+ grub_uint32_t header_len;
+ grub_uint32_t header_version;
+ grub_uint16_t chipset_id;
+ grub_uint16_t flags;
+ grub_uint32_t module_vendor;
+ grub_uint32_t date; /* e.g 20131231H == December 31, 2013 */
+ grub_uint32_t size; /* multiples of 4 bytes */
+ grub_uint16_t txt_svn;
+ grub_uint16_t se_svn;
+ grub_uint32_t code_control;
+ grub_uint32_t error_entry_point;
+ grub_uint32_t gdt_limit;
+ grub_uint32_t gdt_base;
+ grub_uint32_t seg_sel;
+ grub_uint32_t entry_point;
+ grub_uint8_t reserved2[64];
+ grub_uint32_t key_size;
+ grub_uint32_t scratch_size;
+ /* RSA Pub Key and Signature */
+} GRUB_PACKED;
+
+#define GRUB_TXT_ACM_UUID
"\xaa\x3a\xc0\x7f\xa7\x46\xdb\x18\x2e\xac\x69\x8f\x8d\x41\x7f\x5a"
+
+#define GRUB_TXT_ACM_CHIPSET_TYPE_BIOS 0
+#define GRUB_TXT_ACM_CHIPSET_TYPE_SINIT1
+#define GRUB_TXT_ACM_CHIPSET_TYPE_BIOS_RACM8
+#define GRUB_TXT_ACM_CHIPSET_TYPE_SINIT_RACM 9
+
+struct grub_txt_acm_info_table
+{
+ grub_uint8_t uuid[16];
+ grub_uint8_t chipset_acm_type;
+ grub_uint8_t version;
+ grub_uint16_t length;
+ grub_uint32_t chipset_id_list;
+ grub_uint32_t os_sinit_data_ver;
+ grub_uint32_t min_mle_header_ver;
+ grub_uint32_t capabilities;
+ grub_uint32_t acm_version_revision;
+ grub_uint32_t processor_id_list;
+ /* Version >= 5 */
+ grub_uint32_t tpm_info_list;
+} GRUB_PACKED;
+
+struct grub_txt_acm_chipset_id_list
+{
+ grub_uint32_t count;
+/* Array of chipset ID structs */
+} GRUB_PACKED;
+
+#define GRUB_TXT_ACM_REVISION_ID_MASK (1<<0)
+
+struct grub_txt_acm_chipset_id
+{
+ grub_uint32_t flags;
+ grub_uint16_t vendor_id;
+ grub_uint16_t
[GRUB PATCH RFC 16/18] i386/txt: Add Intel TXT ACM module support
From: Ross Philipson
Signed-off-by: Ross Philipson
Signed-off-by: Daniel Kiper
---
grub-core/loader/i386/txt/acmod.c | 575 ++
1 file changed, 575 insertions(+)
create mode 100644 grub-core/loader/i386/txt/acmod.c
diff --git a/grub-core/loader/i386/txt/acmod.c
b/grub-core/loader/i386/txt/acmod.c
new file mode 100644
index 0..ed2fbab7b
--- /dev/null
+++ b/grub-core/loader/i386/txt/acmod.c
@@ -0,0 +1,575 @@
+/*
+ * acmod.c: support functions for use of Intel(r) TXT Authenticated
+ * Code (AC) Modules
+ *
+ * Copyright (c) 2003-2011, Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ * * Neither the name of the Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * This checks to see if two numbers multiplied together are larger
+ * than the type that they are. Returns TRUE if OVERFLOWING.
+ * If the first parameter "x" is greater than zero and
+ * if that is true, that the largest possible value 0x / "x"
+ * is less than the second parameter "y". If "y" is zero then
+ * it will also fail because no unsigned number is less than zero.
+ */
+static inline int
+multiply_overflow_u32 (grub_uint32_t x, grub_uint32_t y)
+{
+ /* Use x instead of (x > 0)? */
+ return (x > 0) ? grub_uint32_t)(~0))/x) < y) : 0;
+}
+
+/*
+ * These three "plus overflow" functions take a "x" value
+ *and add the "y" value to it and if the two values are
+ *greater than the size of the variable type, they will
+ *overflow the type and end up with a smaller value and
+ *return TRUE - that they did overflow. i.e.
+ */
+static inline int plus_overflow_u32 (grub_uint32_t x, grub_uint32_t y)
+{
+ return grub_uint32_t)(~0)) - x) < y);
+}
+
+static struct grub_txt_acm_info_table*
+get_acmod_info_table (struct grub_txt_acm_header* hdr)
+{
+ grub_uint32_t user_area_off;
+
+ /* Overflow? */
+ if ( plus_overflow_u32 (hdr->header_len, hdr->scratch_size) )
+{
+ grub_error (GRUB_ERR_OUT_OF_RANGE, N_("ACM header length plus scratch
size overflows"));
+ return NULL;
+}
+
+ if ( multiply_overflow_u32 ((hdr->header_len + hdr->scratch_size), 4) )
+{
+ grub_error (GRUB_ERR_OUT_OF_RANGE, N_("ACM header length and scratch
size in bytes overflows"));
+ return NULL;
+}
+
+ /*
+ * This fn assumes that the ACM has already passed at least the initial
+ * is_acmod() checks.
+ */
+
+ user_area_off = (hdr->header_len + hdr->scratch_size) * 4;
+
+ /* Overflow?
[GRUB PATCH RFC 18/18] i386/slaunch: Add secure launch framework and commands
From: Ross Philipson
Signed-off-by: Ross Philipson
Signed-off-by: Daniel Kiper
---
grub-core/Makefile.am| 3 +
grub-core/Makefile.core.def | 15 +++
grub-core/lib/i386/relocator32.S | 8 ++
grub-core/loader/i386/bsd.c | 7 ++
grub-core/loader/i386/linux.c| 206 ---
grub-core/loader/i386/slaunch.c | 194
grub-core/loader/i386/xnu.c | 3 +
grub-core/loader/multiboot.c | 5 +
include/grub/file.h | 3 +
include/grub/i386/linux.h| 14 ++-
include/grub/i386/slaunch.h | 56 +++
11 files changed, 500 insertions(+), 14 deletions(-)
create mode 100644 grub-core/loader/i386/slaunch.c
create mode 100644 include/grub/i386/slaunch.h
diff --git a/grub-core/Makefile.am b/grub-core/Makefile.am
index 3ea8e7ff4..ae6676e6f 100644
--- a/grub-core/Makefile.am
+++ b/grub-core/Makefile.am
@@ -99,6 +99,7 @@ KERNEL_HEADER_FILES +=
$(top_builddir)/include/grub/machine/kernel.h
KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/pxe.h
KERNEL_HEADER_FILES += $(top_builddir)/include/grub/machine/int.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/slaunch.h
endif
if COND_i386_xen_pvh
@@ -118,6 +119,7 @@ KERNEL_HEADER_FILES +=
$(top_builddir)/include/grub/machine/kernel.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/slaunch.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/acpi.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/pci.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/pmtimer.h
@@ -179,6 +181,7 @@ KERNEL_HEADER_FILES +=
$(top_builddir)/include/grub/machine/kernel.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/efi.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/efi/disk.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/tsc.h
+KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/slaunch.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/pci.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/acpi.h
KERNEL_HEADER_FILES += $(top_srcdir)/include/grub/i386/pmtimer.h
diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def
index b74a34f0c..a07811b51 100644
--- a/grub-core/Makefile.core.def
+++ b/grub-core/Makefile.core.def
@@ -1824,6 +1824,15 @@ module = {
};
module = {
+ name = slaunch;
+ x86 = loader/i386/slaunch.c;
+ x86 = loader/i386/txt/txt.c;
+ x86 = loader/i386/txt/acmod.c;
+ x86 = loader/i386/txt/verify.c;
+ enable = x86;
+};
+
+module = {
name = fdt;
efi = loader/efi/fdt.c;
common = lib/fdt.c;
@@ -2497,6 +2506,12 @@ module = {
};
module = {
+ name = tpm;
+ x86 = commands/i386/tpm.c;
+ enable = x86;
+};
+
+module = {
name = tpm_verifier;
common = commands/tpm_verifier.c;
efi = commands/efi/tpm.c;
diff --git a/grub-core/lib/i386/relocator32.S b/grub-core/lib/i386/relocator32.S
index 09ce56ad0..a2b377197 100644
--- a/grub-core/lib/i386/relocator32.S
+++ b/grub-core/lib/i386/relocator32.S
@@ -24,6 +24,8 @@
#include "relocator_common.S"
+#include
+
.p2align4 /* force 16-byte alignment */
VARIABLE(grub_relocator32_start)
@@ -110,11 +112,17 @@ VARIABLE(grub_relocator32_edx)
payload and makes this implementation easier. */
cld
+ cmpl$SLP_INTEL_TXT, %edi
+ je LOCAL(intel_txt)
+
.byte 0xea
VARIABLE(grub_relocator32_eip)
.long 0
.word CODE_SEGMENT
+LOCAL(intel_txt):
+ getsec
+
/* GDT. Copied from loader/i386/linux.c. */
.p2align4
LOCAL(gdt):
diff --git a/grub-core/loader/i386/bsd.c b/grub-core/loader/i386/bsd.c
index eb82391db..53bb2af93 100644
--- a/grub-core/loader/i386/bsd.c
+++ b/grub-core/loader/i386/bsd.c
@@ -21,6 +21,10 @@
#include
#include
#include
+#if 0
+#include
+#endif
+#define SLP_NONE 0
#include
#include
#include
@@ -791,6 +795,7 @@ grub_freebsd_boot (void)
#endif
grub_memcpy ([9], , sizeof (bi));
+ state.edi = SLP_NONE;
state.eip = entry;
state.esp = stack_target;
state.ebp = stack_target;
@@ -906,6 +911,7 @@ grub_openbsd_boot (void)
return err;
#endif
+ state.edi = SLP_NONE;
state.eip = entry;
state.ebp = state.esp
= ((grub_uint8_t *) stack - (grub_uint8_t *) buf0) + buf_target;
@@ -1223,6 +1229,7 @@ grub_netbsd_boot (void)
return err;
#endif
+ state.edi = SLP_NONE;
state.eip = entry;
state.esp = stack_target;
state.ebp = stack_target;
diff --git a/grub-core/loader/i386/linux.c b/grub-core/loader/i386/linux.c
index 952eb1191..da8be621e 100644
--- a/grub-core/loader/i386/linux.c
+++ b/grub-core/loader/i386/linux.c
@@ -34,6 +34,8 @@
#include
#include
[GRUB PATCH RFC 06/18] mmap: Add grub_mmap_get_lowest() and grub_mmap_get_highest()
The functions calculate lowest and highest available RAM
addresses respectively.
Both functions are needed to calculate PMR boundaries for
Intel TXT secure launcher introduced by subsequent patches.
Signed-off-by: Daniel Kiper
---
grub-core/mmap/mmap.c | 64 +++
include/grub/memory.h | 3 +++
2 files changed, 67 insertions(+)
diff --git a/grub-core/mmap/mmap.c b/grub-core/mmap/mmap.c
index b569cb23b..cf4b69f56 100644
--- a/grub-core/mmap/mmap.c
+++ b/grub-core/mmap/mmap.c
@@ -340,6 +340,70 @@ grub_mmap_unregister (int handle)
#endif /* ! GRUB_MMAP_REGISTER_BY_FIRMWARE */
+typedef struct
+{
+ grub_uint64_t addr;
+ grub_uint64_t limit;
+} addr_limit_t;
+
+/* Helper for grub_mmap_get_lowest(). */
+static int
+lowest_hook (grub_uint64_t addr, grub_uint64_t size, grub_memory_type_t type,
+void *data)
+{
+ addr_limit_t *al = data;
+
+ if (type != GRUB_MEMORY_AVAILABLE)
+return 0;
+
+ if (addr >= al->limit)
+al->addr = grub_min (al->addr, addr);
+
+ if ((addr < al->limit) && ((addr + size) > al->limit))
+al->addr = al->limit;
+
+ return 0;
+}
+
+grub_uint64_t
+grub_mmap_get_lowest (grub_uint64_t limit)
+{
+ addr_limit_t al = {~0, limit};
+
+ grub_mmap_iterate (lowest_hook, );
+
+ return al.addr;
+}
+
+/* Helper for grub_mmap_get_highest(). */
+static int
+highest_hook (grub_uint64_t addr, grub_uint64_t size, grub_memory_type_t type,
+ void *data)
+{
+ addr_limit_t *al = data;
+
+ if (type != GRUB_MEMORY_AVAILABLE)
+return 0;
+
+ if ((addr + size) < al->limit)
+al->addr = grub_max (al->addr, addr + size);
+
+ if ((addr < al->limit) && ((addr + size) >= al->limit))
+al->addr = al->limit;
+
+ return 0;
+}
+
+grub_uint64_t
+grub_mmap_get_highest (grub_uint64_t limit)
+{
+ addr_limit_t al = {0, limit};
+
+ grub_mmap_iterate (highest_hook, );
+
+ return al.addr;
+}
+
#define CHUNK_SIZE 0x400
struct badram_entry {
diff --git a/include/grub/memory.h b/include/grub/memory.h
index 6da114a1b..8f22f7525 100644
--- a/include/grub/memory.h
+++ b/include/grub/memory.h
@@ -69,6 +69,9 @@ void *grub_mmap_malign_and_register (grub_uint64_t align,
grub_uint64_t size,
void grub_mmap_free_and_unregister (int handle);
+extern grub_uint64_t grub_mmap_get_lowest (grub_uint64_t limit);
+extern grub_uint64_t grub_mmap_get_highest (grub_uint64_t limit);
+
#ifndef GRUB_MMAP_REGISTER_BY_FIRMWARE
struct grub_mmap_region
--
2.11.0
[GRUB PATCH RFC 13/18] i386/slaunch: Add basic platform support for secure launch
From: Ross Philipson
Signed-off-by: Ross Philipson
Signed-off-by: Daniel Kiper
---
include/grub/i386/cpuid.h | 13
include/grub/i386/crfr.h | 186 ++
include/grub/i386/mmio.h | 90 ++
include/grub/i386/msr.h | 61 +++
4 files changed, 350 insertions(+)
create mode 100644 include/grub/i386/crfr.h
create mode 100644 include/grub/i386/mmio.h
diff --git a/include/grub/i386/cpuid.h b/include/grub/i386/cpuid.h
index f7ae4b0a4..8176e5d11 100644
--- a/include/grub/i386/cpuid.h
+++ b/include/grub/i386/cpuid.h
@@ -19,6 +19,19 @@
#ifndef GRUB_CPU_CPUID_HEADER
#define GRUB_CPU_CPUID_HEADER 1
+/* General */
+#define GRUB_X86_CPUID_VENDOR 0x
+#define GRUB_X86_CPUID_FEATURES0x0001
+
+/* Intel */
+#define GRUB_VMX_CPUID_FEATURE (1<<5)
+#define GRUB_SMX_CPUID_FEATURE (1<<6)
+
+/* AMD */
+#define GRUB_AMD_CPUID_FEATURES0x8001
+#define GRUB_SVM_CPUID_FEATURE (1<<2)
+#define GRUB_AMD_CPUID_FUNC0x800a
+
extern unsigned char grub_cpuid_has_longmode;
extern unsigned char grub_cpuid_has_pae;
diff --git a/include/grub/i386/crfr.h b/include/grub/i386/crfr.h
new file mode 100644
index 0..284d6967b
--- /dev/null
+++ b/include/grub/i386/crfr.h
@@ -0,0 +1,186 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_CRFR_H
+#define GRUB_CRFR_H 1
+
+#include
+
+/* Routines for R/W of control and flags registers */
+
+#define GRUB_CR0_X86_PE0x0001 /* Enable Protected Mode */
+#define GRUB_CR0_X86_MP0x0002 /* "Math" (FPU) Present */
+#define GRUB_CR0_X86_EM0x0004 /* EMulate FPU */
+#define GRUB_CR0_X86_TS0x0008 /* Task Switched */
+#define GRUB_CR0_X86_PG0x8000 /* Enable PaGing */
+
+#define GRUB_CR0_X86_NE0x0020 /* Numeric Error enable
(EX16 vs IRQ13) */
+#define GRUB_CR0_X86_WP0x0001 /* Write Protect */
+#define GRUB_CR0_X86_AM0x0004 /* Alignment Mask */
+#define GRUB_CR0_X86_NW0x2000 /* Not Write-through */
+#define GRUB_CR0_X86_CD0x4000 /* Cache Disable */
+
+#define GRUB_CR4_X86_VME 0x0001 /* Virtual 8086 mode extensions */
+#define GRUB_CR4_X86_PVI 0x0002 /* Protected-mode virtual interrupts
*/
+#define GRUB_CR4_X86_TSD 0x0004 /* Time stamp disable */
+#define GRUB_CR4_X86_DE0x0008 /* Debugging extensions */
+#define GRUB_CR4_X86_PSE 0x0010 /* Page size extensions */
+#define GRUB_CR4_X86_PAE 0x0020 /* Physical address extension */
+#define GRUB_CR4_X86_MCE 0x0040 /* Enable Machine check enable */
+#define GRUB_CR4_X86_PGE 0x0080 /* Enable Page global */
+#define GRUB_CR4_X86_PCE 0x0100 /* Enable Performance monitoring
counter */
+#define GRUB_CR4_X86_FXSR 0x0200 /* Fast FPU save/restore */
+#define GRUB_CR4_X86_XMM 0x0400 /* Enable SIMD/MMX2 to use except 16
*/
+#define GRUB_CR4_X86_VMXE 0x2000 /* Enable VMX */
+#define GRUB_CR4_X86_SMXE 0x4000 /* Enable SMX */
+#define GRUB_CR4_X86_PCIDE 0x0002 /* Enable PCID */
+
+static inline unsigned long
+grub_read_cr4 (void)
+{
+ unsigned long val;
+
+ asm volatile ("mov %%cr4, %0" : "=r" (val) : : "memory");
+
+ return val;
+}
+
+static inline void
+grub_write_cr4 (unsigned long val)
+{
+ asm volatile ("mov %0, %%cr4" : : "r" (val) : "memory");
+}
+
+#define GRUB_CR0 0
+#define GRUB_CR1 1
+#define GRUB_CR2 2
+#define GRUB_CR3 3
+#define GRUB_CR4 4
+
+#ifdef __x86_64__
+#define read_cr(r, d) asm volatile ("movq %%cr" r ", %0" : "=r" (d))
+#else
+#define read_cr(r, d) asm volatile ("movl %%cr" r ", %0" : "=r" (d))
+#endif
+
+static inline unsigned long
+grub_read_control_register(grub_uint8_t reg)
+{
+ unsigned long data;
+
+ switch (reg)
+ {
+ case GRUB_CR0:
+read_cr("0", data);
+break;
+ case GRUB_CR1:
+read_cr("1", data);
+break;
+ case GRUB_CR2:
+read_cr(&
[GRUB PATCH RFC 11/18] efi: Add a function to read EFI variables with attributes
It will be used to properly detect and report UEFI Secure Boot status to
the x86 Linux kernel. The functionality will be added by subsequent patches.
Signed-off-by: Ignat Korchagin
Signed-off-by: Daniel Kiper
---
grub-core/kern/efi/efi.c | 16 +---
include/grub/efi/efi.h | 5 +
2 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 81bc49f84..7eeac6019 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -223,8 +223,11 @@ grub_efi_set_variable(const char *var, const
grub_efi_guid_t *guid,
}
grub_efi_status_t
-grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
- grub_size_t *datasize_out, void **data_out)
+grub_efi_get_variable_with_attributes (const char *var,
+ const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out,
+ void **data_out,
+ grub_efi_uint32_t *attributes)
{
grub_efi_status_t status;
grub_efi_uintn_t datasize = 0;
@@ -261,7 +264,7 @@ grub_efi_get_variable (const char *var, const
grub_efi_guid_t *guid,
return GRUB_EFI_OUT_OF_RESOURCES;
}
- status = efi_call_5 (r->get_variable, var16, guid, NULL, , data);
+ status = efi_call_5 (r->get_variable, var16, guid, attributes, ,
data);
grub_free (var16);
if (status == GRUB_EFI_SUCCESS)
@@ -275,6 +278,13 @@ grub_efi_get_variable (const char *var, const
grub_efi_guid_t *guid,
return status;
}
+grub_efi_status_t
+grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
+ grub_size_t *datasize_out, void **data_out)
+{
+ return grub_efi_get_variable_with_attributes (var, guid, datasize_out,
data_out, NULL);
+}
+
#pragma GCC diagnostic ignored "-Wcast-align"
/* Search the mods section from the PE32/PE32+ image. This code uses
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index 8b2a0f1f5..83d958f99 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -74,6 +74,11 @@ grub_err_t EXPORT_FUNC (grub_efi_set_virtual_address_map)
(grub_efi_uintn_t memo
grub_efi_uintn_t
descriptor_size,
grub_efi_uint32_t
descriptor_version,
grub_efi_memory_descriptor_t *virtual_map);
+grub_efi_status_t EXPORT_FUNC (grub_efi_get_variable_with_attributes) (const
char *variable,
+ const
grub_efi_guid_t *guid,
+
grub_size_t *datasize_out,
+ void
**data_out,
+
grub_efi_uint32_t *attributes);
grub_efi_status_t EXPORT_FUNC (grub_efi_get_variable) (const char *variable,
const grub_efi_guid_t
*guid,
grub_size_t
*datasize_out,
--
2.11.0
[GRUB PATCH RFC 17/18] i386/txt: Add Intel TXT verification routines
From: Ross Philipson
Signed-off-by: Ross Philipson
Signed-off-by: Daniel Kiper
---
grub-core/loader/i386/txt/verify.c | 297 +
1 file changed, 297 insertions(+)
create mode 100644 grub-core/loader/i386/txt/verify.c
diff --git a/grub-core/loader/i386/txt/verify.c
b/grub-core/loader/i386/txt/verify.c
new file mode 100644
index 0..97f3b325d
--- /dev/null
+++ b/grub-core/loader/i386/txt/verify.c
@@ -0,0 +1,297 @@
+/*
+ * verify.c: verify that platform and processor supports Intel(r) TXT
+ *
+ * Copyright (c) 2003-2010, Intel Corporation
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above
+ * copyright notice, this list of conditions and the following
+ * disclaimer in the documentation and/or other materials provided
+ * with the distribution.
+ * * Neither the name of the Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
+ * HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
+ * OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Current max that the secure launch can handle */
+#define TXT_MAX_CPUS 512
+
+static grub_err_t
+verify_bios_spec_ver_elt (struct grub_txt_heap_ext_data_element *elt)
+{
+ grub_uint8_t *ptr = (grub_uint8_t *)elt;
+ struct grub_txt_heap_bios_spec_ver_element *bios_spec_ver_elt =
+ (struct grub_txt_heap_bios_spec_ver_element *)ptr;
+
+ if ( elt->size != sizeof(*elt) + sizeof(*bios_spec_ver_elt) )
+return grub_error (GRUB_ERR_OUT_OF_RANGE,
+N_("HEAP_BIOS_SPEC_VER element has wrong size (%d)"),
+elt->size);
+
+ /* Any values are allowed */
+ return GRUB_ERR_NONE;
+}
+
+static grub_err_t
+verify_acm_elt (struct grub_txt_heap_ext_data_element *elt)
+{
+ grub_uint8_t *ptr = ((grub_uint8_t *)elt + sizeof(*elt));
+ struct grub_txt_heap_acm_element *acm_elt =
+ (struct grub_txt_heap_acm_element *)ptr;
+ grub_uint64_t *acm_addrs;
+ grub_uint32_t i;
+
+ if ( elt->size != sizeof(*elt) + sizeof(*acm_elt) +
+ acm_elt->num_acms*sizeof(grub_uint64_t) )
+return grub_error (GRUB_ERR_OUT_OF_RANGE,
+N_("HEAP_ACM element has wrong size (%d)"),
+elt->size);
+
+ /* No addrs is not error, but print warning. */
+ if ( acm_elt->num_acms == 0 )
+grub_printf ("WARNING: HEAP_ACM element has no ACM addrs\n");
+
+ acm_addrs = (grub_uint64_t *)(ptr + sizeof(*acm_elt));
+ for ( i = 0; i < acm_elt->num_acms; i++ )
+{
+ if ( acm_addrs[i] == 0 )
+return grub_error (GRUB_ERR_OUT_OF_RANGE,
+N_("HEAP_ACM element ACM addr (%d) is NULL"), i);
+
+ if ( acm_addrs[i] >= 0x1UL )
+return grub_error (GRUB_ERR_OUT_OF_RANGE,
+N_("HEAP_ACM element ACM addr (%d) is >4GB"), i);
+
+ /* Not going to check if ACM addrs are valid ACMs */
+}
+
+ return GRUB_
[GRUB PATCH RFC 10/18] efi: Return grub_efi_status_t from grub_efi_get_variable()
This is needed to properly detect and report UEFI Secure Boot status
to the x86 Linux kernel. The functionality will be added by subsequent
patches.
Signed-off-by: Daniel Kiper
---
grub-core/commands/efi/efifwsetup.c | 8
grub-core/kern/efi/efi.c| 16 +---
grub-core/video/efi_gop.c | 2 +-
include/grub/efi/efi.h | 7 ---
4 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/grub-core/commands/efi/efifwsetup.c
b/grub-core/commands/efi/efifwsetup.c
index 7a137a72a..eaca03283 100644
--- a/grub-core/commands/efi/efifwsetup.c
+++ b/grub-core/commands/efi/efifwsetup.c
@@ -38,8 +38,8 @@ grub_cmd_fwsetup (grub_command_t cmd __attribute__ ((unused)),
grub_size_t oi_size;
grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
- old_os_indications = grub_efi_get_variable ("OsIndications", ,
- _size);
+ grub_efi_get_variable ("OsIndications", , _size,
+(void **) _os_indications);
if (old_os_indications != NULL && oi_size == sizeof (os_indications))
os_indications |= *old_os_indications;
@@ -63,8 +63,8 @@ efifwsetup_is_supported (void)
grub_size_t oi_size = 0;
grub_efi_guid_t global = GRUB_EFI_GLOBAL_VARIABLE_GUID;
- os_indications_supported = grub_efi_get_variable ("OsIndicationsSupported",
- , _size);
+ grub_efi_get_variable ("OsIndicationsSupported", , _size,
+(void **) _indications_supported);
if (!os_indications_supported)
return 0;
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 3a708ed72..81bc49f84 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -222,9 +222,9 @@ grub_efi_set_variable(const char *var, const
grub_efi_guid_t *guid,
return grub_error (GRUB_ERR_IO, "could not set EFI variable `%s'", var);
}
-void *
+grub_efi_status_t
grub_efi_get_variable (const char *var, const grub_efi_guid_t *guid,
- grub_size_t *datasize_out)
+ grub_size_t *datasize_out, void **data_out)
{
grub_efi_status_t status;
grub_efi_uintn_t datasize = 0;
@@ -233,13 +233,14 @@ grub_efi_get_variable (const char *var, const
grub_efi_guid_t *guid,
void *data;
grub_size_t len, len16;
+ *data_out = NULL;
*datasize_out = 0;
len = grub_strlen (var);
len16 = len * GRUB_MAX_UTF16_PER_UTF8;
var16 = grub_malloc ((len16 + 1) * sizeof (var16[0]));
if (!var16)
-return NULL;
+return GRUB_EFI_OUT_OF_RESOURCES;
len16 = grub_utf8_to_utf16 (var16, len16, (grub_uint8_t *) var, len, NULL);
var16[len16] = 0;
@@ -250,14 +251,14 @@ grub_efi_get_variable (const char *var, const
grub_efi_guid_t *guid,
if (status != GRUB_EFI_BUFFER_TOO_SMALL || !datasize)
{
grub_free (var16);
- return NULL;
+ return status;
}
data = grub_malloc (datasize);
if (!data)
{
grub_free (var16);
- return NULL;
+ return GRUB_EFI_OUT_OF_RESOURCES;
}
status = efi_call_5 (r->get_variable, var16, guid, NULL, , data);
@@ -265,12 +266,13 @@ grub_efi_get_variable (const char *var, const
grub_efi_guid_t *guid,
if (status == GRUB_EFI_SUCCESS)
{
+ *data_out = data;
*datasize_out = datasize;
- return data;
+ return status;
}
grub_free (data);
- return NULL;
+ return status;
}
#pragma GCC diagnostic ignored "-Wcast-align"
diff --git a/grub-core/video/efi_gop.c b/grub-core/video/efi_gop.c
index be446f8d2..7fe0cdabf 100644
--- a/grub-core/video/efi_gop.c
+++ b/grub-core/video/efi_gop.c
@@ -316,7 +316,7 @@ grub_video_gop_get_edid (struct grub_video_edid_info
*edid_info)
char edidname[] = "agp-internal-edid";
grub_size_t datasize;
grub_uint8_t *data;
- data = grub_efi_get_variable (edidname, _var_guid, );
+ grub_efi_get_variable (edidname, _var_guid, , (void **)
);
if (data && datasize > 16)
{
copy_size = datasize - 16;
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index e90e00dc4..8b2a0f1f5 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -74,9 +74,10 @@ grub_err_t EXPORT_FUNC (grub_efi_set_virtual_address_map)
(grub_efi_uintn_t memo
grub_efi_uintn_t
descriptor_size,
grub_efi_uint32_t
descriptor_version,
grub_efi_memory_descriptor_t *virtual_map);
-void *EXPORT_FUNC (grub_efi_get_variable) (const char *variable,
- const grub_efi_guid_t *guid,
- grub_size_t *datasize_out);
+grub_efi_status_t EXPORT_FUNC (grub_efi_get_variable) (const char *v
[GRUB PATCH RFC 00/18] i386: Intel TXT secure launcher
Hi, This is an RFC patchset for the GRUB introducing the Intel TXT secure launcher. This is a part of larger work known as the TrenchBoot. Patchset can be split into two distinct parts: - 01-12: preparatory patches, - 13-18: the Intel TXT secure launcher itself. The initial implementation of the Intel TXT secure launcher works. However, there are still some missing bits and pieces, e.g.: - SINIT ACM auto loader, - lack of RMRR support, - lack of support for MLEs larger than 1 GiB, - lack of TPM 1.2 support. - various fixes and cleanups. Commands introduced by this patchset: tpm_type, slaunch, slaunch_module (not required on server platforms) and slaunch_state (useful for checking platform configuration and state; based on tboot's txt-stat). Daniel
[GRUB PATCH RFC 01/18] i386/msr: Merge rdmsr.h and wrmsr.h into msr.h
It does not make sense to have separate headers for separate static
functions. Additionally, we have to add some constants with MSR addresses
in subsequent patches. So, make one common place to store them.
Signed-off-by: Daniel Kiper
---
grub-core/commands/i386/rdmsr.c | 2 +-
grub-core/commands/i386/wrmsr.c | 2 +-
include/grub/i386/{wrmsr.h => msr.h} | 16 +---
include/grub/i386/rdmsr.h| 37
4 files changed, 15 insertions(+), 42 deletions(-)
rename include/grub/i386/{wrmsr.h => msr.h} (78%)
delete mode 100644 include/grub/i386/rdmsr.h
diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index 46c4346da..fa4622f9e 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -26,7 +26,7 @@
#include
#include
#include
-#include
+#include
GRUB_MOD_LICENSE("GPLv3+");
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index fa76f5aed..9b7abba7c 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -26,7 +26,7 @@
#include
#include
#include
-#include
+#include
GRUB_MOD_LICENSE("GPLv3+");
diff --git a/include/grub/i386/wrmsr.h b/include/grub/i386/msr.h
similarity index 78%
rename from include/grub/i386/wrmsr.h
rename to include/grub/i386/msr.h
index dea60aed1..7b52b5d61 100644
--- a/include/grub/i386/wrmsr.h
+++ b/include/grub/i386/msr.h
@@ -16,14 +16,24 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/
-#ifndef GRUB_WRMSR_H
-#define GRUB_WRMSR_H 1
+#ifndef GRUB_I386_MSR_H
+#define GRUB_I386_MSR_H 1
/*
* TODO: Add a general protection exception handler.
* Accessing a reserved or unimplemented MSR address results in a GP#.
*/
+static inline grub_uint64_t
+grub_msr_read (grub_uint32_t msr_id)
+{
+ grub_uint32_t low, high;
+
+ asm volatile ("rdmsr" : "=a" (low), "=d" (high) : "c" (msr_id));
+
+ return ((grub_uint64_t) high << 32) | low;
+}
+
static inline void
grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
{
@@ -32,4 +42,4 @@ grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
asm volatile ("wrmsr" : : "c" (msr_id), "a" (low), "d" (high));
}
-#endif /* GRUB_WRMSR_H */
+#endif /* GRUB_I386_MSR_H */
diff --git a/include/grub/i386/rdmsr.h b/include/grub/i386/rdmsr.h
deleted file mode 100644
index c0a0c717a..0
--- a/include/grub/i386/rdmsr.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * GRUB -- GRand Unified Bootloader
- * Copyright (C) 2019 Free Software Foundation, Inc.
- *
- * GRUB is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * GRUB is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef GRUB_RDMSR_H
-#define GRUB_RDMSR_H 1
-
-/*
- * TODO: Add a general protection exception handler.
- * Accessing a reserved or unimplemented MSR address results in a GP#.
- */
-
-static inline grub_uint64_t
-grub_msr_read (grub_uint32_t msr_id)
-{
- grub_uint32_t low, high;
-
- asm volatile ("rdmsr" : "=a" (low), "=d" (high) : "c" (msr_id));
-
- return ((grub_uint64_t)high << 32) | low;
-}
-
-#endif /* GRUB_RDMSR_H */
--
2.11.0
[GRUB PATCH RFC 05/18] i386/memory: Rename PAGE_SIZE to GRUB_PAGE_SIZE and make it global
Subsequent patches will use that constant.
Signed-off-by: Daniel Kiper
---
grub-core/loader/i386/xen.c | 35 +--
include/grub/i386/memory.h | 1 +
2 files changed, 18 insertions(+), 18 deletions(-)
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index fe92874e0..dfdddfeed 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -91,8 +91,7 @@ static struct xen_loader_state xen_state;
static grub_dl_t my_mod;
-#define PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
-#define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
+#define MAX_MODULES (GRUB_PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
#define STACK_SIZE 1048576
#define ADDITIONAL_SIZE (1 << 19)
#define ALIGN_SIZE (1 << 22)
@@ -228,7 +227,7 @@ generate_page_table (grub_xen_mfn_t *mfn_list)
for (m1 = 0; m1 < xen_state.n_mappings; m1++)
grub_memset (xen_state.mappings[m1].where, 0,
-xen_state.mappings[m1].area.n_pt_pages * PAGE_SIZE);
+xen_state.mappings[m1].area.n_pt_pages * GRUB_PAGE_SIZE);
for (l = NUMBER_OF_LEVELS - 1; l >= 0; l--)
{
@@ -323,7 +322,7 @@ grub_xen_p2m_alloc (void)
map = xen_state.mappings + xen_state.n_mappings;
p2msize = ALIGN_UP (sizeof (grub_xen_mfn_t) *
- grub_xen_start_page_addr->nr_pages, PAGE_SIZE);
+ grub_xen_start_page_addr->nr_pages, GRUB_PAGE_SIZE);
if (xen_state.xen_inf.has_p2m_base)
{
err = get_pgtable_size (xen_state.xen_inf.p2m_base,
@@ -379,9 +378,9 @@ grub_xen_special_alloc (void)
xen_state.state.start_info = xen_state.max_addr +
xen_state.xen_inf.virt_base;
xen_state.virt_start_info = get_virtual_current_address (ch);
xen_state.max_addr =
-ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE);
+ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start),
GRUB_PAGE_SIZE);
xen_state.console_pfn = xen_state.max_addr >> GRUB_PAGE_SHIFT;
- xen_state.max_addr += 2 * PAGE_SIZE;
+ xen_state.max_addr += 2 * GRUB_PAGE_SIZE;
xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages;
grub_memcpy (xen_state.next_start.magic, grub_xen_start_page_addr->magic,
@@ -430,9 +429,9 @@ grub_xen_pt_alloc (void)
/* Map the relocator page either at virtual 0 or after end of area. */
nr_need_pages = nr_info_pages + map->area.n_pt_pages;
if (xen_state.xen_inf.virt_base)
- err = get_pgtable_size (0, PAGE_SIZE, nr_need_pages);
+ err = get_pgtable_size (0, GRUB_PAGE_SIZE, nr_need_pages);
else
- err = get_pgtable_size (try_virt_end, try_virt_end + PAGE_SIZE,
+ err = get_pgtable_size (try_virt_end, try_virt_end + GRUB_PAGE_SIZE,
nr_need_pages);
if (err)
return err;
@@ -537,7 +536,7 @@ grub_xen_boot (void)
return grub_relocator_xen_boot (xen_state.relocator, xen_state.state,
nr_pages,
xen_state.xen_inf.virt_base <
- PAGE_SIZE ? page2offset (nr_pages) : 0,
+ GRUB_PAGE_SIZE ? page2offset (nr_pages) : 0,
xen_state.pgtbl_end - 1,
page2offset (xen_state.pgtbl_end - 1) +
xen_state.xen_inf.virt_base);
@@ -675,7 +674,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}
- if (xen_state.xen_inf.virt_base & (PAGE_SIZE - 1))
+ if (xen_state.xen_inf.virt_base & (GRUB_PAGE_SIZE - 1))
{
grub_error (GRUB_ERR_BAD_OS, "unaligned virt_base");
goto fail;
@@ -698,10 +697,10 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
kern_start = grub_min (kern_start, xen_state.xen_inf.hypercall_page -
xen_state.xen_inf.virt_base);
kern_end = grub_max (kern_end, xen_state.xen_inf.hypercall_page -
-xen_state.xen_inf.virt_base + PAGE_SIZE);
+xen_state.xen_inf.virt_base +
GRUB_PAGE_SIZE);
}
- xen_state.max_addr = ALIGN_UP (kern_end, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (kern_end, GRUB_PAGE_SIZE);
err = grub_relocator_alloc_chunk_addr (xen_state.relocator, , kern_start,
kern_end - kern_start);
@@ -722,7 +721,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
if (xen_state.xen_inf.has_hypercall_page)
{
unsigned i;
- for (i = 0; i < PAGE_SIZE / HYPERCALL_INTERFACE_SIZE; i++)
+ for (i = 0; i < GRUB_PAGE_SIZE / HYPERCALL_INTERFACE_SIZE; i++)
set_hypercall_interface ((grub_uint8_t *) kern_chunk_src +
i * HYPERCALL_INTERFACE_SIZE +
xen_state.xen_inf.hypercall_page -
@@ -821
[GRUB PATCH RFC 04/18] i386/memory: Rename PAGE_SHIFT to GRUB_PAGE_SHIFT
..to avoid potential conflicts and confusion.
Signed-off-by: Daniel Kiper
---
grub-core/lib/i386/xen/relocator.S | 6 +++---
grub-core/lib/x86_64/xen/relocator.S | 4 ++--
grub-core/loader/i386/xen.c | 28 ++--
include/grub/i386/memory.h | 2 +-
4 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/grub-core/lib/i386/xen/relocator.S
b/grub-core/lib/i386/xen/relocator.S
index 96e51b59a..dab4d8ace 100644
--- a/grub-core/lib/i386/xen/relocator.S
+++ b/grub-core/lib/i386/xen/relocator.S
@@ -75,10 +75,10 @@ VARIABLE(grub_relocator_xen_mfn_list)
.long 0
movl0(%eax, %ebp, 4), %ecx /* mfn */
movl%ebp, %ebx
- shll$PAGE_SHIFT, %ebx /* virtual address (1:1 mapping) */
+ shll$GRUB_PAGE_SHIFT, %ebx /* virtual address (1:1 mapping) */
movl%ecx, %edx
- shll$PAGE_SHIFT, %ecx /* prepare pte low part */
- shrl$(32 - PAGE_SHIFT), %edx /* pte high part */
+ shll$GRUB_PAGE_SHIFT, %ecx /* prepare pte low part */
+ shrl$(32 - GRUB_PAGE_SHIFT), %edx /* pte high part */
orl $(GRUB_PAGE_PRESENT | GRUB_PAGE_USER), %ecx /* pte low */
movl$UVMF_INVLPG, %esi
movl$__HYPERVISOR_update_va_mapping, %eax
diff --git a/grub-core/lib/x86_64/xen/relocator.S
b/grub-core/lib/x86_64/xen/relocator.S
index f5364ed0f..852cd40aa 100644
--- a/grub-core/lib/x86_64/xen/relocator.S
+++ b/grub-core/lib/x86_64/xen/relocator.S
@@ -60,9 +60,9 @@ LOCAL(cont):
jz 3f
2:
movq%r12, %rdi
- shlq$PAGE_SHIFT, %rdi /* virtual address (1:1 mapping) */
+ shlq$GRUB_PAGE_SHIFT, %rdi /* virtual address (1:1 mapping) */
movq(%rbx, %r12, 8), %rsi /* mfn */
- shlq$PAGE_SHIFT, %rsi
+ shlq$GRUB_PAGE_SHIFT, %rsi
orq $(GRUB_PAGE_PRESENT | GRUB_PAGE_USER), %rsi /* Build pte */
movq$UVMF_INVLPG, %rdx
movq%rcx, %r9 /* %rcx clobbered by hypercall */
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index 8f662c8ac..fe92874e0 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -91,7 +91,7 @@ static struct xen_loader_state xen_state;
static grub_dl_t my_mod;
-#define PAGE_SIZE (1UL << PAGE_SHIFT)
+#define PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
#define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
#define STACK_SIZE 1048576
#define ADDITIONAL_SIZE (1 << 19)
@@ -102,7 +102,7 @@ static grub_dl_t my_mod;
static grub_uint64_t
page2offset (grub_uint64_t page)
{
- return page << PAGE_SHIFT;
+ return page << GRUB_PAGE_SHIFT;
}
static grub_err_t
@@ -141,7 +141,7 @@ get_pgtable_size (grub_uint64_t from, grub_uint64_t to,
grub_uint64_t pfn)
continue;
}
- bits = PAGE_SHIFT + (i + 1) * LOG_POINTERS_PER_PAGE;
+ bits = GRUB_PAGE_SHIFT + (i + 1) * LOG_POINTERS_PER_PAGE;
mask = (1ULL << bits) - 1;
map->lvls[i].virt_start = map->area.virt_start & ~mask;
map->lvls[i].virt_end = map->area.virt_end | mask;
@@ -246,11 +246,11 @@ generate_page_table (grub_xen_mfn_t *mfn_list)
if (lvl->virt_start >= end || lvl->virt_end <= start)
continue;
p_s = (grub_max (start, lvl->virt_start) - start) >>
- (PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
+ (GRUB_PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
p_e = (grub_min (end, lvl->virt_end) - start) >>
- (PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
+ (GRUB_PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
pfn = ((grub_max (start, lvl->virt_start) - lvl->virt_start) >>
-(PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE)) + lvl->pfn_start;
+(GRUB_PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE)) +
lvl->pfn_start;
grub_dprintf ("xen", "write page table entries level %d pg %p "
"mapping %d/%d index %lx-%lx pfn %llx\n",
l, pg, m1, m2, p_s, p_e, (unsigned long long) pfn);
@@ -328,16 +328,16 @@ grub_xen_p2m_alloc (void)
{
err = get_pgtable_size (xen_state.xen_inf.p2m_base,
xen_state.xen_inf.p2m_base + p2msize,
- (xen_state.max_addr + p2msize) >> PAGE_SHIFT);
+ (xen_state.max_addr + p2msize) >>
GRUB_PAGE_SHIFT);
if (err)
return err;
- map->area.pfn_start = xen_state.max_addr >> PAGE_SHIFT;
+ map->area.pfn_start = xen_state.max_addr >> GRUB_PAGE_SHIFT;
p2malloc = p2msize + page2offset (map->area.n_pt_pages);
xen_state.n_mappings++;
xen_state.next_start.mfn_lis
[GRUB PATCH RFC 09/18] efi: Make shim_lock GUID and protocol type public
The GUID will be used to properly detect and report UEFI Secure Boot
status to the x86 Linux kernel. The functionality will be added by
subsequent patches. The shim_lock protocol type is made public for
completeness.
Additionally, fix formatting of four preceding GUIDs.
Signed-off-by: Daniel Kiper
---
grub-core/commands/efi/shim_lock.c | 12
include/grub/efi/api.h | 19 +++
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/grub-core/commands/efi/shim_lock.c
b/grub-core/commands/efi/shim_lock.c
index 764098cfc..d8f52d721 100644
--- a/grub-core/commands/efi/shim_lock.c
+++ b/grub-core/commands/efi/shim_lock.c
@@ -27,18 +27,6 @@
GRUB_MOD_LICENSE ("GPLv3+");
-#define GRUB_EFI_SHIM_LOCK_GUID \
- { 0x605dab50, 0xe046, 0x4300, \
-{ 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
- }
-
-struct grub_efi_shim_lock_protocol
-{
- grub_efi_status_t
- (*verify) (void *buffer, grub_uint32_t size);
-};
-typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
-
static grub_efi_guid_t shim_lock_guid = GRUB_EFI_SHIM_LOCK_GUID;
static grub_efi_shim_lock_protocol_t *sl;
diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h
index 937058d68..e634afd61 100644
--- a/include/grub/efi/api.h
+++ b/include/grub/efi/api.h
@@ -321,22 +321,27 @@
#define GRUB_EFI_SAL_TABLE_GUID \
{ 0xeb9d2d32, 0x2d88, 0x11d3, \
- { 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d } \
+{ 0x9a, 0x16, 0x0, 0x90, 0x27, 0x3f, 0xc1, 0x4d } \
}
#define GRUB_EFI_HCDP_TABLE_GUID \
{ 0xf951938d, 0x620b, 0x42ef, \
- { 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 } \
+{ 0x82, 0x79, 0xa8, 0x4b, 0x79, 0x61, 0x78, 0x98 } \
}
#define GRUB_EFI_DEVICE_TREE_GUID \
{ 0xb1b621d5, 0xf19c, 0x41a5, \
- { 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 } \
+{ 0x83, 0x0b, 0xd9, 0x15, 0x2c, 0x69, 0xaa, 0xe0 } \
}
#define GRUB_EFI_VENDOR_APPLE_GUID \
{ 0x2B0585EB, 0xD8B8, 0x49A9,\
- { 0x8B, 0x8C, 0xE2, 0x1B, 0x01, 0xAE, 0xF2, 0xB7 } \
+{ 0x8B, 0x8C, 0xE2, 0x1B, 0x01, 0xAE, 0xF2, 0xB7 } \
+ }
+
+#define GRUB_EFI_SHIM_LOCK_GUID \
+ { 0x605dab50, 0xe046, 0x4300, \
+{ 0xab, 0xb6, 0x3d, 0xd8, 0x10, 0xdd, 0x8b, 0x23 } \
}
struct grub_efi_sal_system_table
@@ -1690,6 +1695,12 @@ struct grub_efi_block_io
};
typedef struct grub_efi_block_io grub_efi_block_io_t;
+struct grub_efi_shim_lock_protocol
+{
+ grub_efi_status_t (*verify) (void *buffer, grub_uint32_t size);
+};
+typedef struct grub_efi_shim_lock_protocol grub_efi_shim_lock_protocol_t;
+
#if (GRUB_TARGET_SIZEOF_VOID_P == 4) || defined (__ia64__) \
|| defined (__aarch64__) || defined (__MINGW64__) || defined (__CYGWIN__) \
|| defined(__riscv)
--
2.11.0
[GRUB PATCH RFC 03/18] i386/msr: Extract and improve MSR support detection code
Currently rdmsr and wrmsr commands have own MSR support detection code.
This code is the same. So, it is duplicated. Additionally, this code
cannot be reused by others. Hence, extract this code to a function and
make it public. By the way, improve a code a bit.
Additionally, use GRUB_ERR_BAD_DEVICE instead of GRUB_ERR_BUG to signal
an error because errors encountered by this new routine are not bugs.
Signed-off-by: Daniel Kiper
---
grub-core/commands/i386/rdmsr.c | 21 +
grub-core/commands/i386/wrmsr.c | 21 +
include/grub/i386/msr.h | 29 +
3 files changed, 39 insertions(+), 32 deletions(-)
diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index 89ece7657..2e42f6197 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -42,27 +42,16 @@ static const struct grub_arg_option options[] =
static grub_err_t
grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char **argv)
{
- grub_uint32_t manufacturer[3], max_cpuid, a, b, c, features, addr;
+ grub_err_t err;
+ grub_uint32_t addr;
grub_uint64_t value;
const char *ptr;
char buf[sizeof("1122334455667788")];
- /*
- * The CPUID instruction should be used to determine whether MSRs
- * are supported. (CPUID.01H:EDX[5] = 1)
- */
- if (! grub_cpu_is_cpuid_supported ())
-return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ err = grub_cpu_is_msr_supported ();
- grub_cpuid (0, max_cpuid, manufacturer[0], manufacturer[2], manufacturer[1]);
-
- if (max_cpuid < 1)
-return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
-
- grub_cpuid (1, a, b, c, features);
-
- if (!(features & (1 << 5)))
-return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ if (err != GRUB_ERR_NONE)
+return grub_error (err, N_("RDMSR is unsupported"));
if (argc != 1)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index e3e9f2ee3..d14bb3176 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -35,26 +35,15 @@ static grub_command_t cmd_write;
static grub_err_t
grub_cmd_msr_write (grub_command_t cmd __attribute__ ((unused)), int argc,
char **argv)
{
- grub_uint32_t manufacturer[3], max_cpuid, a, b, c, features, addr;
+ grub_err_t err;
+ grub_uint32_t addr;
grub_uint64_t value;
const char *ptr;
- /*
- * The CPUID instruction should be used to determine whether MSRs
- * are supported. (CPUID.01H:EDX[5] = 1)
- */
- if (!grub_cpu_is_cpuid_supported ())
-return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ err = grub_cpu_is_msr_supported ();
- grub_cpuid (0, max_cpuid, manufacturer[0], manufacturer[2], manufacturer[1]);
-
- if (max_cpuid < 1)
-return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
-
- grub_cpuid (1, a, b, c, features);
-
- if (!(features & (1 << 5)))
-return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ if (err != GRUB_ERR_NONE)
+return grub_error (err, N_("WRMSR is unsupported"));
if (argc != 2)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 4fba1b8e0..1e838c022 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -19,6 +19,35 @@
#ifndef GRUB_I386_MSR_H
#define GRUB_I386_MSR_H 1
+#include
+#include
+#include
+
+static inline grub_err_t
+grub_cpu_is_msr_supported (void)
+{
+ grub_uint32_t eax, ebx, ecx, edx;
+
+ /*
+ * The CPUID instruction should be used to determine whether MSRs
+ * are supported, CPUID.01H:EDX[5] = 1.
+ */
+ if (!grub_cpu_is_cpuid_supported ())
+return GRUB_ERR_BAD_DEVICE;
+
+ grub_cpuid (0, eax, ebx, ecx, edx);
+
+ if (eax < 1)
+return GRUB_ERR_BAD_DEVICE;
+
+ grub_cpuid (1, eax, ebx, ecx, edx);
+
+ if (!(edx & (1 << 5)))
+return GRUB_ERR_BAD_DEVICE;
+
+ return GRUB_ERR_NONE;
+}
+
/*
* TODO: Add a general protection exception handler.
* Accessing a reserved or unimplemented MSR address results in a GP#.
--
2.11.0
Re: [PATCH v2 1/3] x86/boot: Introduce the kernel_info
On Tue, Oct 01, 2019 at 03:28:01PM -0700, H. Peter Anvin wrote: > On 2019-10-01 04:41, Daniel Kiper wrote: > > > > OK, so, this is more or less what I had in my v3 patch before sending > > this email. So, it looks that I am on good track. Great! Though I am not > > sure that we should have magic for chunked objects. If yes could you > > explain why? I would just leave len for chunked objects. > > > > It makes it easier to validate the contents (bugs happen...), and would allow > for multiple chunks that could come from different object files if it ever > becomes necessary for some reason. OK. > We could also just say that dynamic chunks don't even have pointers, and let > the boot loader just walk the list. Yeah... That seams simpler. I will do that. > >> Also "InfO" is a pretty hideous magic. In general, all-ASCII magics have > >> much > >> higher risk of collision than *RANDOM* binary numbers. However, for a > >> chunked > >> architecture they do have the advantage that they can be used also as a > >> human > >> name or file name for the chunk, e.,g. in sysfs, so maybe something like > >> "LnuX" or even "LToP" for the top-level chunk might make sense. > >> > >> How does that sound? > > > > Well, your proposals are more cryptic, especially the second one, than > > mine but I tend to agree that more *RANDOM* magics are better. So, > > I would choose "LToP" if you decipher it for me. Linux Top? > > > > Yes, Linux top [structure]. Thx! Daniel
Re: [PATCH RFC 1/2] x86/boot: Introduce the setup_header2
I am working on new version of patches but I have some concerns. Please
look below for more details...
On Thu, Jun 06, 2019 at 03:06:30PM -0700, H. Peter Anvin wrote:
> On 5/24/19 2:55 AM, Daniel Kiper wrote:
> > Due to limited space left in the setup header it was decided to
> > introduce the setup_header2. Its role is to communicate Linux kernel
> > supported features to the boot loader. Starting from now this is the
> > primary way to communicate things to the boot loader.
> >
> > Suggested-by: H. Peter Anvin
> > Signed-off-by: Daniel Kiper
> > Reviewed-by: Ross Philipson
> > Reviewed-by: Eric Snowberg
> > ---
> > I know that setup_header2 is not the best name. There were some
> > alternatives proposed like setup_header_extra, setup_header_addendum,
> > setup_header_more, ext_setup_header, extended_setup_header, extended_header
> > and extended_setup. Sadly, I am not happy with any of them. So,
> > leaving setup_header2 as is but still looking for better name.
> > Probably shorter == better...
>
> I would say kernel_info. The relationships between the headers are analogous
> to the various data sections:
>
> setup_header= .data
> boot_params/setup_data = .bss
>
> What is missing from the above list? That's right:
>
> kernel_info = .rodata
>
> We have been (ab)using .data for things that could go into .rodata or .bss for
> a long time, for lack of alternatives and -- especially early on -- intertia.
> Also, the BIOS stub is responsible for creating boot_params, so it isn't
> available to a BIOS-based loader (setup_data is, though.)
>
> setup_header is permanently limited to 144 bytes due to the reach of the
> 2-byte jump field, which doubles as a length field for the structure, combined
> with the size of the "hole" in struct boot_params that a protected-mode loader
> or the BIOS stub has to copy it into. It is currently 119 bytes long, which
> leaves us with 25 very precious bytes. This isn't something that can be fixed
> without revising the boot protocol entirely, breaking backwards compatibility.
>
> boot_params proper is limited to 4096 bytes, but can be arbitrarily extended
> by adding setup_data entries. It cannot be used to communicate properties of
> the kernel image, because it is .bss and has no image-provided content.
>
> kernel_info solves this by providing an extensible place for information about
> the kernel image. It is readonly, because the kernel cannot rely on a
> bootloader copying its contents anywhere, but that is OK; if it becomes
> necessary it can still contain data items that an enabled bootloader would be
> expected to copy into a setup_data chunk.
>
> ^ The above or some variant thereof may be a good thing to put both in your
> patch comments as well as in the boot protocol documentation.
Will do...
> While we are making a change that bumps the version number anyway, there is
> another change I would like to make to the boot protocol which we might as
> well do at the same time. setup_data is a bit awkward to use for extremely
> large data objects, both because the setup_data header has to be adjacent to
> the data object, and because it has a 32-bit length field. However, it is
> important that intermediate stages of the boot process have a way to identify
> which chunks of memory are occupied by kernel data.
Is not possible to "identify which chunks of memory are occupied by
kernel data" today? I think that it is. So, this does not look like
a valid point for change. Am I missing something?
> Thus I think we should introduce a uniform way to specify such indirect data.
> We define a new setup_data type we can maybe call SETUP_INDIRECT; a
> SETUP_INDIRECT data item would be an array of structures of the form:
OK.
> struct setup_indirect {
> __u32 type;
> __u32 reserved; /* Reserved, must be set to zero */
> __u64 len;
> __u64 addr;
> };
>
> ... where type is itself simply a SETUP_* type -- although we probably don't
> want to let it be SETUP_INDIRECT itself since making it a tree structure could
> require a lot of stack space in something that needs to parse it, and stack
> space can be limited in boot contexts.
Yeah...
> This would be particularly useful for having SETUP_INITRAMFS, if it becomes
> desirable to allow the kernel to parse a non-contiguous set of memory regions
> for the initramfs.
OK.
> It might be a good idea to immediately start out struct kernel_info with
> either a high mark or a bitmask of SETUP_* types that the kernel supports. A
High mark seems better for me here.
> bitmask would be more flexible, but would need provisions to be grown in the
> future.
Ye
Re: [PATCH RFC 0/2] x86/boot: Introduce the setup_header2
On Thu, Jun 06, 2019 at 01:30:46PM -0400, Konrad Rzeszutek Wilk wrote: > On Thu, Jun 06, 2019 at 01:51:08PM +0200, Daniel Kiper wrote: > > On Wed, Jun 05, 2019 at 10:01:17AM -0400, Konrad Rzeszutek Wilk wrote: > > > On Wed, Jun 05, 2019 at 03:50:31PM +0200, Daniel Kiper wrote: > > > > On Fri, May 24, 2019 at 11:55:02AM +0200, Daniel Kiper wrote: > > > > > Hi, > > > > > > > > > > This change is needed to properly start the Linux kernel in Intel TXT > > > > > mode and > > > > > is a part of the TrenchBoot project (https://github.com/TrenchBoot). > > > > > > Can you please expand more on this? > > > > > > Nice explanation of why, other alternative solutions that didn't work, > > > and so on. > > > > OK. > > > > > > > Daniel > > > > > > > > > > Documentation/x86/boot.txt | 55 > > > > > +++ > > > > > arch/x86/Kconfig | 7 +++ > > > > > arch/x86/boot/Makefile | 2 +- > > > > > arch/x86/boot/compressed/Makefile| 5 +++-- > > > > > arch/x86/boot/compressed/setup_header2.S | 18 ++ > > > > > arch/x86/boot/compressed/sl_stub.S | 28 > > > > > > > > > > arch/x86/boot/header.S | 3 ++- > > > > > arch/x86/boot/tools/build.c | 8 > > > > > arch/x86/include/uapi/asm/bootparam.h| 1 + > > > > > 9 files changed, 123 insertions(+), 4 deletions(-) > > > > > > > > > > Daniel Kiper (2): > > > > > x86/boot: Introduce the setup_header2 > > > > > x86/boot: Introduce dummy MLE header > > > > > > > > Ping? > > > > > > Can you add Ingo and Thomas to the To: next time please? > > > > OK. > > > > > Also please drop the second patch. > > > > Why? This is an example how to use the setup_header2. > > If you are going to post it as non-RFC (which I suspect you will > for the next), then why post a patch that is not to be checked in? Nope, this will be an RFC. And the second patch is an example. I hope that it eases understanding how all pieces fit together. If the idea is approved then first patch will be posted with full Intel TXT implementation and second patch will contain fully fledged MLE header. Daniel
Re: [PATCH RFC 0/2] x86/boot: Introduce the setup_header2
On Wed, Jun 05, 2019 at 10:01:17AM -0400, Konrad Rzeszutek Wilk wrote: > On Wed, Jun 05, 2019 at 03:50:31PM +0200, Daniel Kiper wrote: > > On Fri, May 24, 2019 at 11:55:02AM +0200, Daniel Kiper wrote: > > > Hi, > > > > > > This change is needed to properly start the Linux kernel in Intel TXT > > > mode and > > > is a part of the TrenchBoot project (https://github.com/TrenchBoot). > > Can you please expand more on this? > > Nice explanation of why, other alternative solutions that didn't work, and so > on. OK. > > > Daniel > > > > > > Documentation/x86/boot.txt | 55 > > > +++ > > > arch/x86/Kconfig | 7 +++ > > > arch/x86/boot/Makefile | 2 +- > > > arch/x86/boot/compressed/Makefile| 5 +++-- > > > arch/x86/boot/compressed/setup_header2.S | 18 ++ > > > arch/x86/boot/compressed/sl_stub.S | 28 > > > > > > arch/x86/boot/header.S | 3 ++- > > > arch/x86/boot/tools/build.c | 8 > > > arch/x86/include/uapi/asm/bootparam.h| 1 + > > > 9 files changed, 123 insertions(+), 4 deletions(-) > > > > > > Daniel Kiper (2): > > > x86/boot: Introduce the setup_header2 > > > x86/boot: Introduce dummy MLE header > > > > Ping? > > Can you add Ingo and Thomas to the To: next time please? OK. > Also please drop the second patch. Why? This is an example how to use the setup_header2. Daniel
Re: [PATCH RFC 0/2] x86/boot: Introduce the setup_header2
On Fri, May 24, 2019 at 11:55:02AM +0200, Daniel Kiper wrote: > Hi, > > This change is needed to properly start the Linux kernel in Intel TXT mode and > is a part of the TrenchBoot project (https://github.com/TrenchBoot). > > Daniel > > Documentation/x86/boot.txt | 55 > +++ > arch/x86/Kconfig | 7 +++ > arch/x86/boot/Makefile | 2 +- > arch/x86/boot/compressed/Makefile| 5 +++-- > arch/x86/boot/compressed/setup_header2.S | 18 ++ > arch/x86/boot/compressed/sl_stub.S | 28 > arch/x86/boot/header.S | 3 ++- > arch/x86/boot/tools/build.c | 8 > arch/x86/include/uapi/asm/bootparam.h| 1 + > 9 files changed, 123 insertions(+), 4 deletions(-) > > Daniel Kiper (2): > x86/boot: Introduce the setup_header2 > x86/boot: Introduce dummy MLE header Ping? Daniel
[PATCH RFC 1/2] x86/boot: Introduce the setup_header2
Due to limited space left in the setup header it was decided to introduce the setup_header2. Its role is to communicate Linux kernel supported features to the boot loader. Starting from now this is the primary way to communicate things to the boot loader. Suggested-by: H. Peter Anvin Signed-off-by: Daniel Kiper Reviewed-by: Ross Philipson Reviewed-by: Eric Snowberg --- I know that setup_header2 is not the best name. There were some alternatives proposed like setup_header_extra, setup_header_addendum, setup_header_more, ext_setup_header, extended_setup_header, extended_header and extended_setup. Sadly, I am not happy with any of them. So, leaving setup_header2 as is but still looking for better name. Probably shorter == better... --- Documentation/x86/boot.txt | 49 arch/x86/boot/Makefile | 2 +- arch/x86/boot/compressed/Makefile| 4 +-- arch/x86/boot/compressed/setup_header2.S | 12 arch/x86/boot/header.S | 3 +- arch/x86/boot/tools/build.c | 8 ++ arch/x86/include/uapi/asm/bootparam.h| 1 + 7 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 arch/x86/boot/compressed/setup_header2.S diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt index f4c2a97bfdbd..ff10c6116662 100644 --- a/Documentation/x86/boot.txt +++ b/Documentation/x86/boot.txt @@ -61,6 +61,22 @@ Protocol 2.12: (Kernel 3.8) Added the xloadflags field and extension fields to struct boot_params for loading bzImage and ramdisk above 4G in 64bit. +Protocol 2.14: BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c1889 + (x86/boot: Add ACPI RSDP address to setup_header) + DO NOT USE!!! ASSUME SAME AS 2.13. + +Protocol 2.15: (Kernel 5.2) Added the setup_header2. + +Note: The protocol version number should be changed only if the setup header + is changed. There is no need to update the version number if boot_params + or setup_header2 are changed. Additionally, it is recommended to use + xloadflags (in this case the protocol version number should not be + updated either) or setup_header2 to communicate supported Linux kernel + features to the boot loader. Due to very limited space available in + the original setup header every update to it should be considered + with great care. Starting from the protocol 2.15 the primary way to + communicate things to the boot loader is the setup_header2. + MEMORY LAYOUT The traditional memory map for the kernel loader, used for Image or @@ -197,6 +213,7 @@ Offset Proto NameMeaning 0258/8 2.10+ pref_addressPreferred loading address 0260/4 2.10+ init_size Linear memory required during initialization 0264/4 2.11+ handover_offset Offset of handover entry point +0268/4 2.15+ setup_header2_offset Offset of the setup_header2 (1) For backwards compatibility, if the setup_sects field contains 0, the real value is 4. @@ -744,6 +761,38 @@ Offset/size: 0x264/4 See EFI HANDOVER PROTOCOL below for more details. +Field name:setup_header2_offset +Type: read +Offset/size: 0x268/4 +Protocol: 2.15+ + + This field is the offset from the beginning of the kernel image to the + setup_header2. It is embedded in the Linux image in the uncompressed + protected mode region. + + + THE SETUP_HEADER2 + +Due to limited space left in the setup header it was decided to introduce +the setup_header2. Its role is to communicate Linux kernel supported features +to the boot loader. All fields of the setup_header2 are read only from the +boot loader point of view. The setup_header2 is supported starting from the +boot protocol version 2.15. + + + DETAILS OF THE SETUP_HEADER2 FIELDS + +Field name:header +Offset/size: 0x/4 + + Contains the magic number "hDR2" (0x68445232). + +Field name:size +Offset/size: 0x0004/4 + + This field contains the size of the setup_header2 including setup_header2.header. + It should be used by the boot loader to detect supported fields in the setup_header2. + THE IMAGE CHECKSUM diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile index e2839b5c246c..c11b57da90f6 100644 --- a/arch/x86/boot/Makefile +++ b/arch/x86/boot/Makefile @@ -87,7 +87,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE SETUP_OBJS = $(addprefix $(obj)/,$(setup-y)) -sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|input_data\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' +sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [ABCDGRSTVW] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|setup_header2\|input_data\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' quiet_cmd_zoffset = ZOFFSET $@ cmd_zoffset = $(NM
[PATCH RFC 0/2] x86/boot: Introduce the setup_header2
Hi, This change is needed to properly start the Linux kernel in Intel TXT mode and is a part of the TrenchBoot project (https://github.com/TrenchBoot). Daniel Documentation/x86/boot.txt | 55 +++ arch/x86/Kconfig | 7 +++ arch/x86/boot/Makefile | 2 +- arch/x86/boot/compressed/Makefile| 5 +++-- arch/x86/boot/compressed/setup_header2.S | 18 ++ arch/x86/boot/compressed/sl_stub.S | 28 arch/x86/boot/header.S | 3 ++- arch/x86/boot/tools/build.c | 8 arch/x86/include/uapi/asm/bootparam.h| 1 + 9 files changed, 123 insertions(+), 4 deletions(-) Daniel Kiper (2): x86/boot: Introduce the setup_header2 x86/boot: Introduce dummy MLE header
[PATCH RFC 2/2] x86/boot: Introduce dummy MLE header
DO NOT APPLY!!! THIS PATCH INTRODUCES DUMMY MLE HEADER AND SIMPLY ILLUSTRATES HOW TO EXTEND THE setup_header2 PROPERLY. DO NOT APPLY!!! Signed-off-by: Ross Philipson Signed-off-by: Daniel Kiper Reviewed-by: Ross Philipson --- Documentation/x86/boot.txt | 6 ++ arch/x86/Kconfig | 7 +++ arch/x86/boot/compressed/Makefile| 1 + arch/x86/boot/compressed/setup_header2.S | 6 ++ arch/x86/boot/compressed/sl_stub.S | 28 5 files changed, 48 insertions(+) create mode 100644 arch/x86/boot/compressed/sl_stub.S diff --git a/Documentation/x86/boot.txt b/Documentation/x86/boot.txt index ff10c6116662..09cf50d7dca2 100644 --- a/Documentation/x86/boot.txt +++ b/Documentation/x86/boot.txt @@ -793,6 +793,12 @@ Offset/size: 0x0004/4 This field contains the size of the setup_header2 including setup_header2.header. It should be used by the boot loader to detect supported fields in the setup_header2. +Field name:mle_header_offset +Offset/size: 0x0008/4 + + This field contains the MLE header offset from the beginning of the kernel image. + If it is set to zero then it means that MLE header is not build into the kernel. + THE IMAGE CHECKSUM diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index 5ad92419be19..021e274ede54 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -1961,6 +1961,13 @@ config EFI_MIXED If unsure, say N. +config SECURE_LAUNCH_STUB + bool "Secure Launch stub support" + depends on X86_64 + ---help--- + This kernel feature allows a bzImage to be loaded directly + through Intel TXT or AMD SKINIT measured launch. + config SECCOMP def_bool y prompt "Enable seccomp to safely compute untrusted bytecode" diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile index c12ccc2bd923..9722d119e19a 100644 --- a/arch/x86/boot/compressed/Makefile +++ b/arch/x86/boot/compressed/Makefile @@ -78,6 +78,7 @@ vmlinux-objs-y := $(obj)/vmlinux.lds $(obj)/setup_header2.o $(obj)/head_$(BITS). vmlinux-objs-$(CONFIG_EARLY_PRINTK) += $(obj)/early_serial_console.o vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/kaslr.o +vmlinux-objs-$(CONFIG_SECURE_LAUNCH_STUB) += $(obj)/sl_stub.o ifdef CONFIG_X86_64 vmlinux-objs-$(CONFIG_RANDOMIZE_BASE) += $(obj)/kaslr_64.o vmlinux-objs-y += $(obj)/mem_encrypt.o diff --git a/arch/x86/boot/compressed/setup_header2.S b/arch/x86/boot/compressed/setup_header2.S index 0b3963296825..eb732626fd22 100644 --- a/arch/x86/boot/compressed/setup_header2.S +++ b/arch/x86/boot/compressed/setup_header2.S @@ -9,4 +9,10 @@ setup_header2: .ascii "hDR2" /* Size. */ .long setup_header2_end - setup_header2 +/* MLE header offset. */ +#ifdef CONFIG_SECURE_LAUNCH_STUB + .long mle_header +#else + .long 0 +#endif setup_header2_end: diff --git a/arch/x86/boot/compressed/sl_stub.S b/arch/x86/boot/compressed/sl_stub.S new file mode 100644 index ..34f5000528e4 --- /dev/null +++ b/arch/x86/boot/compressed/sl_stub.S @@ -0,0 +1,28 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +/* + * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. + * + * Author(s): + * Ross Philipson + */ + .code32 + .text + + /* The MLE Header per the TXT Specification, section 4.1 */ + .global mle_header + +mle_header: + .long 0x9082ac5a/* UUID0 */ + .long 0x74a7476f/* UUID1 */ + .long 0xa2555c0f/* UUID2 */ + .long 0x42b651cb/* UUID3 */ + .long 0x0034/* MLE header size */ + .long 0x00020002/* MLE version 2.2 */ + .long 0x01234567/* Linear entry point of MLE (virt. address) */ + .long 0x/* First valid page of MLE */ + .long 0x/* Offset within binary of first byte of MLE */ + .long 0x/* Offset within binary of last byte + 1 of MLE */ + .long 0x0223/* Bit vector of MLE-supported capabilities */ + .long 0x/* Starting linear address of command line */ + .long 0x/* Ending linear address of command line */ -- 2.11.0
Re: [PATCH 0/1] [RFC] Secure Launch boot protocol
On Thu, Mar 28, 2019 at 11:15:53AM -0700, H. Peter Anvin wrote: > So, per our conversation today, lets create a new, readonly, data structure > pointed to by a single field in setup_header, in order to preserve what little > space we have left in that structure (a whopping 24 bytes...) > > The new data structure will have a header consisting of a magic number and a > length field; if we want to be really paranoid we could add a checksum/crc. > > The existence of this new readonly structure will be announced by bumping the > boot protocol to 2.15. > > The presence of your new boot launch capability (trenchboot) will be indicated > by a new bit in xloadflags. > > I thought hard about this, and I have come to the conclusion that the new > structure is better off in the .rodata section of the compressed kernel rather > than in the setup area, for the following reasons, some of which are > theoretical and unlikely to apply anywhere in the near future, but don't > actually hurt to address right off the bat: > > a. The future size of the structure would not be artificially constrained by >the 32K hard limit on the setup area; > b. It is one less level of indirection in the build tools; > c. It adds a possibly unnecessary dependency on the setup area, which could >possibly be awkward for some boot loaders (unlikely, but...); > d. It would allow this new structure to also carry information that might be >useful to the decompressor for whatever reason. Thank you for your help. I will try to post the patches next week. Daniel
Re: [PATCH] doc: add boot protocol 2.13 description to Documentation/x86/boot.txt
On Fri, Mar 08, 2019 at 12:43:10PM +0100, Juergen Gross wrote: > Documentation/x86/boot.txt is missing protocol 2.13 description. > > Signed-off-by: Juergen Gross Reviewed-by: Daniel Kiper Daniel
[tip:efi/core] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Commit-ID: a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93
Gitweb: https://git.kernel.org/tip/a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93
Author: Daniel Kiper <daniel.ki...@oracle.com>
AuthorDate: Fri, 4 May 2018 07:59:47 +0200
Committer: Ingo Molnar <mi...@kernel.org>
CommitDate: Mon, 14 May 2018 08:57:46 +0200
x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.
Note that part of drivers/firmware/efi/libstub/secureboot.c is duplicated
by this patch, only in this case, it runs in the context of the kernel
proper rather than UEFI boot context. The reason for the duplication is
that maintaining the original code to run correctly on ARM/arm64 as well
as on all the quirky x86 firmware we support is enough of a burden as it
is, and adding the x86/Xen execution context to that mix just so we can
reuse a single routine just isn't worth it.
[ardb: explain rationale for code duplication]
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Matt Fleming <m...@codeblueprint.co.uk>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: linux-...@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-2-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
arch/x86/xen/efi.c| 57 +++
drivers/firmware/efi/libstub/secureboot.c | 3 ++
2 files changed, 60 insertions(+)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703be9ead..1804b27f9632 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
return _systab_xen;
}
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+ static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+ static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+ efi_status_t status;
+ u8 moksbstate, secboot, setupmode;
+ unsigned long size;
+
+ size = sizeof(secboot);
+ status = efi.get_variable(L"SecureBoot", _variable_guid,
+ NULL, , );
+
+ if (status == EFI_NOT_FOUND)
+ return efi_secureboot_mode_disabled;
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = efi.get_variable(L"SetupMode", _variable_guid,
+ NULL, , );
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ return efi_secureboot_mode_disabled;
+
+ /* See if a user has put the shim into insecure mode. */
+ size = sizeof(moksbstate);
+ status = efi.get_variable(L"MokSBStateRT", _guid,
+ NULL, , );
+
+ /* If it fails, we don't care why. Default to secure. */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+
+ if (moksbstate == 1)
+ return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+ pr_info("UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+ pr_err("Could not determine UEFI Secure Boot status.\n");
+ return efi_secureboot_mode_unknown;
+}
+
void __init xen_efi_init(void)
{
efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
+ boot_params.secure_boot = xen_efi_get_secureboot();
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb414c00..72d9dfbebf08 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@ static const efi_char16_t shim_MokSBState_name[] =
L"MokSBState";
/*
* Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
*/
enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
[tip:efi/core] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Commit-ID: a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93
Gitweb: https://git.kernel.org/tip/a7012bdbdf406bbaa4e3de0cc3d8eb0faaacbf93
Author: Daniel Kiper
AuthorDate: Fri, 4 May 2018 07:59:47 +0200
Committer: Ingo Molnar
CommitDate: Mon, 14 May 2018 08:57:46 +0200
x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.
Note that part of drivers/firmware/efi/libstub/secureboot.c is duplicated
by this patch, only in this case, it runs in the context of the kernel
proper rather than UEFI boot context. The reason for the duplication is
that maintaining the original code to run correctly on ARM/arm64 as well
as on all the quirky x86 firmware we support is enough of a burden as it
is, and adding the x86/Xen execution context to that mix just so we can
reuse a single routine just isn't worth it.
[ardb: explain rationale for code duplication]
Signed-off-by: Daniel Kiper
Signed-off-by: Ard Biesheuvel
Cc: Linus Torvalds
Cc: Matt Fleming
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: linux-...@vger.kernel.org
Link: http://lkml.kernel.org/r/20180504060003.19618-2-ard.biesheu...@linaro.org
Signed-off-by: Ingo Molnar
---
arch/x86/xen/efi.c| 57 +++
drivers/firmware/efi/libstub/secureboot.c | 3 ++
2 files changed, 60 insertions(+)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703be9ead..1804b27f9632 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
return _systab_xen;
}
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+ static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+ static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+ efi_status_t status;
+ u8 moksbstate, secboot, setupmode;
+ unsigned long size;
+
+ size = sizeof(secboot);
+ status = efi.get_variable(L"SecureBoot", _variable_guid,
+ NULL, , );
+
+ if (status == EFI_NOT_FOUND)
+ return efi_secureboot_mode_disabled;
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = efi.get_variable(L"SetupMode", _variable_guid,
+ NULL, , );
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ return efi_secureboot_mode_disabled;
+
+ /* See if a user has put the shim into insecure mode. */
+ size = sizeof(moksbstate);
+ status = efi.get_variable(L"MokSBStateRT", _guid,
+ NULL, , );
+
+ /* If it fails, we don't care why. Default to secure. */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+
+ if (moksbstate == 1)
+ return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+ pr_info("UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+ pr_err("Could not determine UEFI Secure Boot status.\n");
+ return efi_secureboot_mode_unknown;
+}
+
void __init xen_efi_init(void)
{
efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
+ boot_params.secure_boot = xen_efi_get_secureboot();
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb414c00..72d9dfbebf08 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@ static const efi_char16_t shim_MokSBState_name[] =
L"MokSBState";
/*
* Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
*/
enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Mon, Apr 16, 2018 at 10:15:15AM +0200, Ard Biesheuvel wrote: > On 11 April 2018 at 10:56, Daniel Kiper <daniel.ki...@oracle.com> wrote: > > On Wed, Apr 04, 2018 at 12:38:24PM +0200, Daniel Kiper wrote: > >> On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote: > >> > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote: > >> > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote: > >> > >> [...] > >> > >> > > > This looks like a bad idea: you're duplicating the secure boot > >> > > > check in > >> > > > > >> > > > drivers/firmware/efi/libstub/secureboot.c > >> > > > > >> > > > Which is an implementation of policy. If we have to have policy in > >> > > > the kernel, it should really only be in one place to prevent drift; > >> > > > why can't you simply use the libstub efi_get_secureboot() so we're > >> > > > not duplicating the implementation of policy? > >> > > > >> > > Well, here is the first version of this patch: > >> > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not > >> > > happy too. In general both approaches are not perfect. More you can > >> > > find in the discussion around this patchset. If you have better idea > >> > > how to do that I am happy to implement it. > >> > > >> > One way might be simply to have the pre exit-boot-services code lay > >> > down a variable containing the state which you pick up, rather than you > >> > >> Do you mean variable in kernel proper or something like that? If yes this > >> is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI > >> infrastructure is owned and operated by Xen. Dom0 kernel can access some > >> stuff in UEFI, including variables, via hypercall. However, when dom0 > >> runs only UEFI runtime services are available. > >> > >> > calling efi code separately and trying to use the insecure RT > >> > >> I am not sure why they are insecure. > >> > >> > variables. That way there's a uniform view of the internal kernel > >> > secure boot state that everyone can use. > >> > >> That would be perfect but I have a feeling that in form proposed above > >> it is not possible. > > > > Ping? > > > > (apologies if this is a duplicate email - I thought I had replied > already but I don't see it in my sent folder) > > Queued in efi/next - thanks. Thanks a lot! Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Mon, Apr 16, 2018 at 10:15:15AM +0200, Ard Biesheuvel wrote: > On 11 April 2018 at 10:56, Daniel Kiper wrote: > > On Wed, Apr 04, 2018 at 12:38:24PM +0200, Daniel Kiper wrote: > >> On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote: > >> > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote: > >> > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote: > >> > >> [...] > >> > >> > > > This looks like a bad idea: you're duplicating the secure boot > >> > > > check in > >> > > > > >> > > > drivers/firmware/efi/libstub/secureboot.c > >> > > > > >> > > > Which is an implementation of policy. If we have to have policy in > >> > > > the kernel, it should really only be in one place to prevent drift; > >> > > > why can't you simply use the libstub efi_get_secureboot() so we're > >> > > > not duplicating the implementation of policy? > >> > > > >> > > Well, here is the first version of this patch: > >> > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not > >> > > happy too. In general both approaches are not perfect. More you can > >> > > find in the discussion around this patchset. If you have better idea > >> > > how to do that I am happy to implement it. > >> > > >> > One way might be simply to have the pre exit-boot-services code lay > >> > down a variable containing the state which you pick up, rather than you > >> > >> Do you mean variable in kernel proper or something like that? If yes this > >> is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI > >> infrastructure is owned and operated by Xen. Dom0 kernel can access some > >> stuff in UEFI, including variables, via hypercall. However, when dom0 > >> runs only UEFI runtime services are available. > >> > >> > calling efi code separately and trying to use the insecure RT > >> > >> I am not sure why they are insecure. > >> > >> > variables. That way there's a uniform view of the internal kernel > >> > secure boot state that everyone can use. > >> > >> That would be perfect but I have a feeling that in form proposed above > >> it is not possible. > > > > Ping? > > > > (apologies if this is a duplicate email - I thought I had replied > already but I don't see it in my sent folder) > > Queued in efi/next - thanks. Thanks a lot! Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Wed, Apr 04, 2018 at 12:38:24PM +0200, Daniel Kiper wrote: > On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote: > > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote: > > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote: > > [...] > > > > > This looks like a bad idea: you're duplicating the secure boot > > > > check in > > > > > > > > drivers/firmware/efi/libstub/secureboot.c > > > > > > > > Which is an implementation of policy. If we have to have policy in > > > > the kernel, it should really only be in one place to prevent drift; > > > > why can't you simply use the libstub efi_get_secureboot() so we're > > > > not duplicating the implementation of policy? > > > > > > Well, here is the first version of this patch: > > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not > > > happy too. In general both approaches are not perfect. More you can > > > find in the discussion around this patchset. If you have better idea > > > how to do that I am happy to implement it. > > > > One way might be simply to have the pre exit-boot-services code lay > > down a variable containing the state which you pick up, rather than you > > Do you mean variable in kernel proper or something like that? If yes this > is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI > infrastructure is owned and operated by Xen. Dom0 kernel can access some > stuff in UEFI, including variables, via hypercall. However, when dom0 > runs only UEFI runtime services are available. > > > calling efi code separately and trying to use the insecure RT > > I am not sure why they are insecure. > > > variables. That way there's a uniform view of the internal kernel > > secure boot state that everyone can use. > > That would be perfect but I have a feeling that in form proposed above > it is not possible. Ping? Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Wed, Apr 04, 2018 at 12:38:24PM +0200, Daniel Kiper wrote: > On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote: > > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote: > > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote: > > [...] > > > > > This looks like a bad idea: you're duplicating the secure boot > > > > check in > > > > > > > > drivers/firmware/efi/libstub/secureboot.c > > > > > > > > Which is an implementation of policy. If we have to have policy in > > > > the kernel, it should really only be in one place to prevent drift; > > > > why can't you simply use the libstub efi_get_secureboot() so we're > > > > not duplicating the implementation of policy? > > > > > > Well, here is the first version of this patch: > > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not > > > happy too. In general both approaches are not perfect. More you can > > > find in the discussion around this patchset. If you have better idea > > > how to do that I am happy to implement it. > > > > One way might be simply to have the pre exit-boot-services code lay > > down a variable containing the state which you pick up, rather than you > > Do you mean variable in kernel proper or something like that? If yes this > is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI > infrastructure is owned and operated by Xen. Dom0 kernel can access some > stuff in UEFI, including variables, via hypercall. However, when dom0 > runs only UEFI runtime services are available. > > > calling efi code separately and trying to use the insecure RT > > I am not sure why they are insecure. > > > variables. That way there's a uniform view of the internal kernel > > secure boot state that everyone can use. > > That would be perfect but I have a feeling that in form proposed above > it is not possible. Ping? Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote: > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote: > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote: [...] > > > This looks like a bad idea: you're duplicating the secure boot > > > check in > > > > > > drivers/firmware/efi/libstub/secureboot.c > > > > > > Which is an implementation of policy. If we have to have policy in > > > the kernel, it should really only be in one place to prevent drift; > > > why can't you simply use the libstub efi_get_secureboot() so we're > > > not duplicating the implementation of policy? > > > > Well, here is the first version of this patch: > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not > > happy too. In general both approaches are not perfect. More you can > > find in the discussion around this patchset. If you have better idea > > how to do that I am happy to implement it. > > One way might be simply to have the pre exit-boot-services code lay > down a variable containing the state which you pick up, rather than you Do you mean variable in kernel proper or something like that? If yes this is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI infrastructure is owned and operated by Xen. Dom0 kernel can access some stuff in UEFI, including variables, via hypercall. However, when dom0 runs only UEFI runtime services are available. > calling efi code separately and trying to use the insecure RT I am not sure why they are insecure. > variables. That way there's a uniform view of the internal kernel > secure boot state that everyone can use. That would be perfect but I have a feeling that in form proposed above it is not possible. Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, Apr 03, 2018 at 10:00:52AM -0700, James Bottomley wrote: > On Tue, 2018-04-03 at 18:07 +0200, Daniel Kiper wrote: > > On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote: [...] > > > This looks like a bad idea: you're duplicating the secure boot > > > check in > > > > > > drivers/firmware/efi/libstub/secureboot.c > > > > > > Which is an implementation of policy. If we have to have policy in > > > the kernel, it should really only be in one place to prevent drift; > > > why can't you simply use the libstub efi_get_secureboot() so we're > > > not duplicating the implementation of policy? > > > > Well, here is the first version of this patch: > > https://lkml.org/lkml/2018/1/9/496 Ard did not like it. I was not > > happy too. In general both approaches are not perfect. More you can > > find in the discussion around this patchset. If you have better idea > > how to do that I am happy to implement it. > > One way might be simply to have the pre exit-boot-services code lay > down a variable containing the state which you pick up, rather than you Do you mean variable in kernel proper or something like that? If yes this is not possible. EFI Linux stub is not executed in Xen dom0. All UEFI infrastructure is owned and operated by Xen. Dom0 kernel can access some stuff in UEFI, including variables, via hypercall. However, when dom0 runs only UEFI runtime services are available. > calling efi code separately and trying to use the insecure RT I am not sure why they are insecure. > variables. That way there's a uniform view of the internal kernel > secure boot state that everyone can use. That would be perfect but I have a feeling that in form proposed above it is not possible. Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> > Initialize UEFI secure boot state during dom0 boot. Otherwise the
> > kernel
> > may not even know that it runs on secure boot enabled platform.
> >
> > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
> > ---
> > arch/x86/xen/efi.c| 57
> > +
> > drivers/firmware/efi/libstub/secureboot.c |3 ++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index a18703b..1804b27 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -115,6 +115,61 @@ static efi_system_table_t __init
> > *xen_efi_probe(void)
> > return _systab_xen;
> > }
> >
> > +/*
> > + * Determine whether we're in secure boot mode.
> > + *
> > + * Please keep the logic in sync with
> > + * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> > + */
> > +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> > +{
> > + static efi_guid_t efi_variable_guid =
> > EFI_GLOBAL_VARIABLE_GUID;
> > + static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> > + efi_status_t status;
> > + u8 moksbstate, secboot, setupmode;
> > + unsigned long size;
> > +
> > + size = sizeof(secboot);
> > + status = efi.get_variable(L"SecureBoot", _variable_guid,
> > + NULL, , );
> > +
> > + if (status == EFI_NOT_FOUND)
> > + return efi_secureboot_mode_disabled;
> > +
> > + if (status != EFI_SUCCESS)
> > + goto out_efi_err;
> > +
> > + size = sizeof(setupmode);
> > + status = efi.get_variable(L"SetupMode", _variable_guid,
> > + NULL, , );
> > +
> > + if (status != EFI_SUCCESS)
> > + goto out_efi_err;
> > +
> > + if (secboot == 0 || setupmode == 1)
> > + return efi_secureboot_mode_disabled;
> > +
> > + /* See if a user has put the shim into insecure mode. */
> > + size = sizeof(moksbstate);
> > + status = efi.get_variable(L"MokSBStateRT", _guid,
> > + NULL, , );
> > +
> > + /* If it fails, we don't care why. Default to secure. */
> > + if (status != EFI_SUCCESS)
> > + goto secure_boot_enabled;
> > +
> > + if (moksbstate == 1)
> > + return efi_secureboot_mode_disabled;
> > +
> > + secure_boot_enabled:
> > + pr_info("UEFI Secure Boot is enabled.\n");
> > + return efi_secureboot_mode_enabled;
> > +
> > + out_efi_err:
> > + pr_err("Could not determine UEFI Secure Boot status.\n");
> > + return efi_secureboot_mode_unknown;
> > +}
> > +
>
> This looks like a bad idea: you're duplicating the secure boot check in
>
> drivers/firmware/efi/libstub/secureboot.c
>
> Which is an implementation of policy. If we have to have policy in the
> kernel, it should really only be in one place to prevent drift; why
> can't you simply use the libstub efi_get_secureboot() so we're not
> duplicating the implementation of policy?
Well, here is the first version of this patch:
https://lkml.org/lkml/2018/1/9/496
Ard did not like it. I was not happy too. In general both approaches are not
perfect.
More you can find in the discussion around this patchset. If you have better
idea
how to do that I am happy to implement it.
Daniel
Re: [PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
On Tue, Apr 03, 2018 at 08:44:41AM -0700, James Bottomley wrote:
> On Tue, 2018-04-03 at 16:39 +0200, Daniel Kiper wrote:
> > Initialize UEFI secure boot state during dom0 boot. Otherwise the
> > kernel
> > may not even know that it runs on secure boot enabled platform.
> >
> > Signed-off-by: Daniel Kiper
> > ---
> > arch/x86/xen/efi.c| 57
> > +
> > drivers/firmware/efi/libstub/secureboot.c |3 ++
> > 2 files changed, 60 insertions(+)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index a18703b..1804b27 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -115,6 +115,61 @@ static efi_system_table_t __init
> > *xen_efi_probe(void)
> > return _systab_xen;
> > }
> >
> > +/*
> > + * Determine whether we're in secure boot mode.
> > + *
> > + * Please keep the logic in sync with
> > + * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
> > + */
> > +static enum efi_secureboot_mode xen_efi_get_secureboot(void)
> > +{
> > + static efi_guid_t efi_variable_guid =
> > EFI_GLOBAL_VARIABLE_GUID;
> > + static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
> > + efi_status_t status;
> > + u8 moksbstate, secboot, setupmode;
> > + unsigned long size;
> > +
> > + size = sizeof(secboot);
> > + status = efi.get_variable(L"SecureBoot", _variable_guid,
> > + NULL, , );
> > +
> > + if (status == EFI_NOT_FOUND)
> > + return efi_secureboot_mode_disabled;
> > +
> > + if (status != EFI_SUCCESS)
> > + goto out_efi_err;
> > +
> > + size = sizeof(setupmode);
> > + status = efi.get_variable(L"SetupMode", _variable_guid,
> > + NULL, , );
> > +
> > + if (status != EFI_SUCCESS)
> > + goto out_efi_err;
> > +
> > + if (secboot == 0 || setupmode == 1)
> > + return efi_secureboot_mode_disabled;
> > +
> > + /* See if a user has put the shim into insecure mode. */
> > + size = sizeof(moksbstate);
> > + status = efi.get_variable(L"MokSBStateRT", _guid,
> > + NULL, , );
> > +
> > + /* If it fails, we don't care why. Default to secure. */
> > + if (status != EFI_SUCCESS)
> > + goto secure_boot_enabled;
> > +
> > + if (moksbstate == 1)
> > + return efi_secureboot_mode_disabled;
> > +
> > + secure_boot_enabled:
> > + pr_info("UEFI Secure Boot is enabled.\n");
> > + return efi_secureboot_mode_enabled;
> > +
> > + out_efi_err:
> > + pr_err("Could not determine UEFI Secure Boot status.\n");
> > + return efi_secureboot_mode_unknown;
> > +}
> > +
>
> This looks like a bad idea: you're duplicating the secure boot check in
>
> drivers/firmware/efi/libstub/secureboot.c
>
> Which is an implementation of policy. If we have to have policy in the
> kernel, it should really only be in one place to prevent drift; why
> can't you simply use the libstub efi_get_secureboot() so we're not
> duplicating the implementation of policy?
Well, here is the first version of this patch:
https://lkml.org/lkml/2018/1/9/496
Ard did not like it. I was not happy too. In general both approaches are not
perfect.
More you can find in the discussion around this patchset. If you have better
idea
how to do that I am happy to implement it.
Daniel
[PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
arch/x86/xen/efi.c| 57 +
drivers/firmware/efi/libstub/secureboot.c |3 ++
2 files changed, 60 insertions(+)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703b..1804b27 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
return _systab_xen;
}
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+ static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+ static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+ efi_status_t status;
+ u8 moksbstate, secboot, setupmode;
+ unsigned long size;
+
+ size = sizeof(secboot);
+ status = efi.get_variable(L"SecureBoot", _variable_guid,
+ NULL, , );
+
+ if (status == EFI_NOT_FOUND)
+ return efi_secureboot_mode_disabled;
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = efi.get_variable(L"SetupMode", _variable_guid,
+ NULL, , );
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ return efi_secureboot_mode_disabled;
+
+ /* See if a user has put the shim into insecure mode. */
+ size = sizeof(moksbstate);
+ status = efi.get_variable(L"MokSBStateRT", _guid,
+ NULL, , );
+
+ /* If it fails, we don't care why. Default to secure. */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+
+ if (moksbstate == 1)
+ return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+ pr_info("UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+ pr_err("Could not determine UEFI Secure Boot status.\n");
+ return efi_secureboot_mode_unknown;
+}
+
void __init xen_efi_init(void)
{
efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
+ boot_params.secure_boot = xen_efi_get_secureboot();
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb4..72d9dfb 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@
/*
* Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
*/
enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
--
1.7.10.4
[PATCH v2] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel
may not even know that it runs on secure boot enabled platform.
Signed-off-by: Daniel Kiper
---
arch/x86/xen/efi.c| 57 +
drivers/firmware/efi/libstub/secureboot.c |3 ++
2 files changed, 60 insertions(+)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703b..1804b27 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -115,6 +115,61 @@ static efi_system_table_t __init *xen_efi_probe(void)
return _systab_xen;
}
+/*
+ * Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * drivers/firmware/efi/libstub/secureboot.c:efi_get_secureboot().
+ */
+static enum efi_secureboot_mode xen_efi_get_secureboot(void)
+{
+ static efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+ static efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+ efi_status_t status;
+ u8 moksbstate, secboot, setupmode;
+ unsigned long size;
+
+ size = sizeof(secboot);
+ status = efi.get_variable(L"SecureBoot", _variable_guid,
+ NULL, , );
+
+ if (status == EFI_NOT_FOUND)
+ return efi_secureboot_mode_disabled;
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = efi.get_variable(L"SetupMode", _variable_guid,
+ NULL, , );
+
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ return efi_secureboot_mode_disabled;
+
+ /* See if a user has put the shim into insecure mode. */
+ size = sizeof(moksbstate);
+ status = efi.get_variable(L"MokSBStateRT", _guid,
+ NULL, , );
+
+ /* If it fails, we don't care why. Default to secure. */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+
+ if (moksbstate == 1)
+ return efi_secureboot_mode_disabled;
+
+ secure_boot_enabled:
+ pr_info("UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+ out_efi_err:
+ pr_err("Could not determine UEFI Secure Boot status.\n");
+ return efi_secureboot_mode_unknown;
+}
+
void __init xen_efi_init(void)
{
efi_system_table_t *efi_systab_xen;
@@ -129,6 +184,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
+ boot_params.secure_boot = xen_efi_get_secureboot();
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 8f07eb4..72d9dfb 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -30,6 +30,9 @@
/*
* Determine whether we're in secure boot mode.
+ *
+ * Please keep the logic in sync with
+ * arch/x86/xen/efi.c:xen_efi_get_secureboot().
*/
enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
--
1.7.10.4
Re: [PATCH] x86: vmx: Allow direct access to MSR_IA32_SPEC_CTRL
On Mon, Jan 29, 2018 at 12:31:13PM -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 29, 2018 at 08:46:03AM +, David Woodhouse wrote:
> > On Sun, 2018-01-28 at 16:39 -0800, Liran Alon wrote:
> > >
> > > Windows use IBRS and Microsoft don't have any plans to switch to
> > > retpoline.
> > > Running a Windows guest should be a pretty common use-case no?
> > >
> > > In addition, your handle of the first WRMSR intercept could be different.
> > > It could signal you to start doing the following:
> > > 1. Disable intercept on SPEC_CTRL MSR.
> > > 2. On VMEntry, Write vCPU SPEC_CTRL value into physical MSR.
> > > 3. On VMExit, read physical MSR into vCPU SPEC_CTRL value.
> > > (And if IBRS is used at host, also set physical SPEC_CTRL MSR here to 1)
> > >
> > > That way, you will both have fastest option as long as guest don't use
> > > IBRS
> > > and also won't have the 3% performance hit compared to Konrad's proposal.
> > >
> > > Am I missing something?
> >
> > Reads from the SPEC_CTRL MSR are strangely slow. I suspect a large part
> > of the 3% speedup you observe is because in the above, the vmentry path
> > doesn't need to *read* the host's value and store it; the host is
> > expected to restore it for itself anyway?
>
> Yes for at least the purpose of correctness. That is based on what I have
> heard is that you when you transition to a higher ring you have to write 1,
> then write zero
> when you transition back to lower rings. That is it is like a knob.
>
> But then I heard that on some CPUs it is more like reset button and
> just writting 1 to IBRS is fine. But again, correctness here.
>
> >
> > I'd actually quite like to repeat the benchmark on the new fixed
> > microcode, if anyone has it yet, to see if that read/swap slowness is
> > still quite as excessive. I'm certainly not ruling this out, but I'm
> > just a little wary of premature optimisation, and I'd like to make sure
> > we have everything *else* in the KVM patches right first.
> >
> > The fact that the save-and-restrict macros I have in the tip of my
> > working tree at the moment are horrid and causing 0-day nastygrams,
> > probably doesn't help persuade me to favour the approach ;)
> >
> > ... hm, the CPU actually has separate MSR save/restore lists for
> > entry/exit, doesn't it? Is there any way to sanely make use of that and
> > do the restoration manually on vmentry but let it be automatic on
> > vmexit, by having it *only* in the guest's MSR-store area to be saved
> > on exit and restored on exit, but *not* in the host's MSR-store area?
s/on exit and restored on exit/on exit and restored on entry/?
Additionally, AIUI there is no "host's MSR-store area".
> Oh. That sounds sounds interesting
That is possible but you have to split add_atomic_switch_msr() accordingly.
> > Reading the code and comparing with the SDM, I can't see where we're
> > ever setting VM_EXIT_MSR_STORE_{ADDR,COUNT} except in the nested
> > case...
>
> Right. We (well Daniel Kiper, CC-ed) implemented it for this and
> that is where we got the numbers.
>
> Daniel, you OK posting it here? Granted with the caveats thta it won't even
> compile against upstream as it was based on a distro kernel.
Please look below...
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index aa9bc4f..e7c0f8b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -162,8 +162,10 @@ module_param(ple_window_max, int, S_IRUGO);
extern const ulong vmx_return;
-#define NR_AUTOLOAD_MSRS 8
-#define VMCS02_POOL_SIZE 1
+#define NR_AUTOLOAD_MSRS 8
+#define NR_AUTOSTORE_MSRS NR_AUTOLOAD_MSRS
+
+#define VMCS02_POOL_SIZE 1
struct vmcs {
u32 revision_id;
@@ -504,6 +506,10 @@ struct vcpu_vmx {
struct vmx_msr_entry guest[NR_AUTOLOAD_MSRS];
struct vmx_msr_entry host[NR_AUTOLOAD_MSRS];
} msr_autoload;
+ struct msr_autostore {
+ unsigned nr;
+ struct vmx_msr_entry guest[NR_AUTOSTORE_MSRS];
+ } msr_autostore;
struct {
int loaded;
u16 fs_sel, gs_sel, ldt_sel;
@@ -1704,6 +1710,37 @@ static void add_atomic_switch_msr(struct vcpu_vmx *vmx,
unsigned msr,
m->host[i].value = host_val;
}
+static void add_atomic_store_msr(struct vcpu_vmx *vmx, unsigned msr)
+{
+ unsigned i;
+ struct msr_autostore *m = >msr_autostore;
+
+ for (i = 0; i < m->nr; ++i)
+ if (m->guest[i].index == msr)
+ return;
+
+ if (i == NR_AUTOSTORE_MSRS) {
+ pr_err("Not enough msr store ent
Re: [PATCH] x86: vmx: Allow direct access to MSR_IA32_SPEC_CTRL
On Mon, Jan 29, 2018 at 12:31:13PM -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Jan 29, 2018 at 08:46:03AM +, David Woodhouse wrote:
> > On Sun, 2018-01-28 at 16:39 -0800, Liran Alon wrote:
> > >
> > > Windows use IBRS and Microsoft don't have any plans to switch to
> > > retpoline.
> > > Running a Windows guest should be a pretty common use-case no?
> > >
> > > In addition, your handle of the first WRMSR intercept could be different.
> > > It could signal you to start doing the following:
> > > 1. Disable intercept on SPEC_CTRL MSR.
> > > 2. On VMEntry, Write vCPU SPEC_CTRL value into physical MSR.
> > > 3. On VMExit, read physical MSR into vCPU SPEC_CTRL value.
> > > (And if IBRS is used at host, also set physical SPEC_CTRL MSR here to 1)
> > >
> > > That way, you will both have fastest option as long as guest don't use
> > > IBRS
> > > and also won't have the 3% performance hit compared to Konrad's proposal.
> > >
> > > Am I missing something?
> >
> > Reads from the SPEC_CTRL MSR are strangely slow. I suspect a large part
> > of the 3% speedup you observe is because in the above, the vmentry path
> > doesn't need to *read* the host's value and store it; the host is
> > expected to restore it for itself anyway?
>
> Yes for at least the purpose of correctness. That is based on what I have
> heard is that you when you transition to a higher ring you have to write 1,
> then write zero
> when you transition back to lower rings. That is it is like a knob.
>
> But then I heard that on some CPUs it is more like reset button and
> just writting 1 to IBRS is fine. But again, correctness here.
>
> >
> > I'd actually quite like to repeat the benchmark on the new fixed
> > microcode, if anyone has it yet, to see if that read/swap slowness is
> > still quite as excessive. I'm certainly not ruling this out, but I'm
> > just a little wary of premature optimisation, and I'd like to make sure
> > we have everything *else* in the KVM patches right first.
> >
> > The fact that the save-and-restrict macros I have in the tip of my
> > working tree at the moment are horrid and causing 0-day nastygrams,
> > probably doesn't help persuade me to favour the approach ;)
> >
> > ... hm, the CPU actually has separate MSR save/restore lists for
> > entry/exit, doesn't it? Is there any way to sanely make use of that and
> > do the restoration manually on vmentry but let it be automatic on
> > vmexit, by having it *only* in the guest's MSR-store area to be saved
> > on exit and restored on exit, but *not* in the host's MSR-store area?
s/on exit and restored on exit/on exit and restored on entry/?
Additionally, AIUI there is no "host's MSR-store area".
> Oh. That sounds sounds interesting
That is possible but you have to split add_atomic_switch_msr() accordingly.
> > Reading the code and comparing with the SDM, I can't see where we're
> > ever setting VM_EXIT_MSR_STORE_{ADDR,COUNT} except in the nested
> > case...
>
> Right. We (well Daniel Kiper, CC-ed) implemented it for this and
> that is where we got the numbers.
>
> Daniel, you OK posting it here? Granted with the caveats thta it won't even
> compile against upstream as it was based on a distro kernel.
Please look below...
diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
index aa9bc4f..e7c0f8b 100644
--- a/arch/x86/kvm/vmx.c
+++ b/arch/x86/kvm/vmx.c
@@ -162,8 +162,10 @@ module_param(ple_window_max, int, S_IRUGO);
extern const ulong vmx_return;
-#define NR_AUTOLOAD_MSRS 8
-#define VMCS02_POOL_SIZE 1
+#define NR_AUTOLOAD_MSRS 8
+#define NR_AUTOSTORE_MSRS NR_AUTOLOAD_MSRS
+
+#define VMCS02_POOL_SIZE 1
struct vmcs {
u32 revision_id;
@@ -504,6 +506,10 @@ struct vcpu_vmx {
struct vmx_msr_entry guest[NR_AUTOLOAD_MSRS];
struct vmx_msr_entry host[NR_AUTOLOAD_MSRS];
} msr_autoload;
+ struct msr_autostore {
+ unsigned nr;
+ struct vmx_msr_entry guest[NR_AUTOSTORE_MSRS];
+ } msr_autostore;
struct {
int loaded;
u16 fs_sel, gs_sel, ldt_sel;
@@ -1704,6 +1710,37 @@ static void add_atomic_switch_msr(struct vcpu_vmx *vmx,
unsigned msr,
m->host[i].value = host_val;
}
+static void add_atomic_store_msr(struct vcpu_vmx *vmx, unsigned msr)
+{
+ unsigned i;
+ struct msr_autostore *m = >msr_autostore;
+
+ for (i = 0; i < m->nr; ++i)
+ if (m->guest[i].index == msr)
+ return;
+
+ if (i == NR_AUTOSTORE_MSRS) {
+ pr_err("Not enough msr store ent
Re: [PATCH 0/4] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Hi Ard, On Thu, Jan 11, 2018 at 12:51:07PM +, Ard Biesheuvel wrote: > On 9 January 2018 at 14:22, Daniel Kiper <daniel.ki...@oracle.com> wrote: > > Hi, > > > > Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel > > may not even know that it runs on secure boot enabled platform. > > Hi Daniel, > > I must say, I am not too thrilled with the approach you have chosen > here. #including .c files in other .c files, and using #defines to > override C functions or other stub functionality is rather fragile. In TBH I do not like it too. Sadly I have not find a better solution for that. I wish to avoid code duplication as much as possible because otherwise it will fall out of sync sooner or later (usually sooner). Similar thing happened in different part of Xen EFI code a few months ago. > particular, it means we have to start caring about not breaking > Xen/x86 code when making modifications to the EFI stub, and that code > is already difficult enough to maintain, given that it is shared > between ARM, arm64 and x86, and runs either from the decompressor or > the kernel proper (arm64) but in the context of the UEFI firmware. I understand that. > None of the stub code currently runs in ordinary kernel context. Yep. > So please, could you try to find another way to do this? I am happy to improve the situation, however, I am afraid that it is difficult here. Stub and kernel proper are separate entities and simple linking does not work. So, It seems to me that only play with includes will allow us to not duplicate the code. However, if you have a better idea I am happy to implement it. Daniel
Re: [PATCH 0/4] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Hi Ard, On Thu, Jan 11, 2018 at 12:51:07PM +, Ard Biesheuvel wrote: > On 9 January 2018 at 14:22, Daniel Kiper wrote: > > Hi, > > > > Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel > > may not even know that it runs on secure boot enabled platform. > > Hi Daniel, > > I must say, I am not too thrilled with the approach you have chosen > here. #including .c files in other .c files, and using #defines to > override C functions or other stub functionality is rather fragile. In TBH I do not like it too. Sadly I have not find a better solution for that. I wish to avoid code duplication as much as possible because otherwise it will fall out of sync sooner or later (usually sooner). Similar thing happened in different part of Xen EFI code a few months ago. > particular, it means we have to start caring about not breaking > Xen/x86 code when making modifications to the EFI stub, and that code > is already difficult enough to maintain, given that it is shared > between ARM, arm64 and x86, and runs either from the decompressor or > the kernel proper (arm64) but in the context of the UEFI firmware. I understand that. > None of the stub code currently runs in ordinary kernel context. Yep. > So please, could you try to find another way to do this? I am happy to improve the situation, however, I am afraid that it is difficult here. Stub and kernel proper are separate entities and simple linking does not work. So, It seems to me that only play with includes will allow us to not duplicate the code. However, if you have a better idea I am happy to implement it. Daniel
[PATCH 1/4] efi/stub: Extract efi_get_secureboot() to separate file
We have to call efi_get_secureboot() from early Xen dom0 boot code to properly
initialize boot_params.secure_boot. Sadly it lives in the EFI stub. Hence, it is
not readily reachable from the kernel proper. So, move efi_get_secureboot() to
separate file which can be included from the core kernel code. Subsequent patch
will add efi_get_secureboot() call from Xen dom0 boot code.
There is no functional change.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
drivers/firmware/efi/libstub/secureboot-core.c | 77
drivers/firmware/efi/libstub/secureboot.c | 66 +---
2 files changed, 78 insertions(+), 65 deletions(-)
create mode 100644 drivers/firmware/efi/libstub/secureboot-core.c
diff --git a/drivers/firmware/efi/libstub/secureboot-core.c
b/drivers/firmware/efi/libstub/secureboot-core.c
new file mode 100644
index 000..11a4feb
--- /dev/null
+++ b/drivers/firmware/efi/libstub/secureboot-core.c
@@ -0,0 +1,77 @@
+/*
+ * Secure boot handling.
+ *
+ * Copyright (C) 2013,2014 Linaro Limited
+ * Roy Franz <roy.fr...@linaro.org>
+ * Copyright (C) 2013 Red Hat, Inc.
+ * Mark Salter <msal...@redhat.com>
+ *
+ * This file is part of the Linux kernel, and is made available under the
+ * terms of the GNU General Public License version 2.
+ */
+
+/* BIOS variables */
+static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+static const efi_char16_t efi_SecureBoot_name[] = {
+ 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
+};
+static const efi_char16_t efi_SetupMode_name[] = {
+ 'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
+};
+
+/* SHIM variables */
+static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+static const efi_char16_t shim_MokSBState_name[] = {
+ 'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
+};
+
+/*
+ * Determine whether we're in secure boot mode.
+ */
+enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+{
+ u32 attr;
+ u8 secboot, setupmode, moksbstate;
+ unsigned long size;
+ efi_status_t status;
+
+ size = sizeof(secboot);
+ status = get_efi_var(efi_SecureBoot_name, _variable_guid,
+NULL, , );
+ if (status == EFI_NOT_FOUND)
+ return efi_secureboot_mode_disabled;
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = get_efi_var(efi_SetupMode_name, _variable_guid,
+NULL, , );
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ return efi_secureboot_mode_disabled;
+
+ /*
+* See if a user has put the shim into insecure mode. If so, and if the
+* variable doesn't have the runtime attribute set, we might as well
+* honor that.
+*/
+ size = sizeof(moksbstate);
+ status = get_efi_var(shim_MokSBState_name, _guid,
+, , );
+
+ /* If it fails, we don't care why. Default to secure */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+ if (!(attr & EFI_VARIABLE_RUNTIME_ACCESS) && moksbstate == 1)
+ return efi_secureboot_mode_disabled;
+
+secure_boot_enabled:
+ pr_efi(sys_table_arg, "UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+out_efi_err:
+ pr_efi_err(sys_table_arg, "Could not determine UEFI Secure Boot
status.\n");
+ return efi_secureboot_mode_unknown;
+}
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 959777e..4a6159f 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -14,73 +14,9 @@
#include "efistub.h"
-/* BIOS variables */
-static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t efi_SecureBoot_name[] = {
- 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
-};
-static const efi_char16_t efi_SetupMode_name[] = {
- 'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
-};
-
-/* SHIM variables */
-static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
-static efi_char16_t const shim_MokSBState_name[] = {
- 'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
-};
-
#define get_efi_var(name, vendor, ...) \
efi_call_runtime(get_variable, \
(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
__VA_ARGS__);
-/*
- * Determine whether we're in secure boot mode.
- */
-enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
-{
- u32 attr;
- u8 secboot, setupmode, moksbstate;
- unsigned long size;
- efi_status_t status;
-
- size = sizeof(secboot);
- status = get_efi_var(efi_Secure
[PATCH 1/4] efi/stub: Extract efi_get_secureboot() to separate file
We have to call efi_get_secureboot() from early Xen dom0 boot code to properly
initialize boot_params.secure_boot. Sadly it lives in the EFI stub. Hence, it is
not readily reachable from the kernel proper. So, move efi_get_secureboot() to
separate file which can be included from the core kernel code. Subsequent patch
will add efi_get_secureboot() call from Xen dom0 boot code.
There is no functional change.
Signed-off-by: Daniel Kiper
---
drivers/firmware/efi/libstub/secureboot-core.c | 77
drivers/firmware/efi/libstub/secureboot.c | 66 +---
2 files changed, 78 insertions(+), 65 deletions(-)
create mode 100644 drivers/firmware/efi/libstub/secureboot-core.c
diff --git a/drivers/firmware/efi/libstub/secureboot-core.c
b/drivers/firmware/efi/libstub/secureboot-core.c
new file mode 100644
index 000..11a4feb
--- /dev/null
+++ b/drivers/firmware/efi/libstub/secureboot-core.c
@@ -0,0 +1,77 @@
+/*
+ * Secure boot handling.
+ *
+ * Copyright (C) 2013,2014 Linaro Limited
+ * Roy Franz
+ * Copyright (C) 2013 Red Hat, Inc.
+ * Mark Salter
+ *
+ * This file is part of the Linux kernel, and is made available under the
+ * terms of the GNU General Public License version 2.
+ */
+
+/* BIOS variables */
+static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
+static const efi_char16_t efi_SecureBoot_name[] = {
+ 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
+};
+static const efi_char16_t efi_SetupMode_name[] = {
+ 'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
+};
+
+/* SHIM variables */
+static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
+static const efi_char16_t shim_MokSBState_name[] = {
+ 'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
+};
+
+/*
+ * Determine whether we're in secure boot mode.
+ */
+enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+{
+ u32 attr;
+ u8 secboot, setupmode, moksbstate;
+ unsigned long size;
+ efi_status_t status;
+
+ size = sizeof(secboot);
+ status = get_efi_var(efi_SecureBoot_name, _variable_guid,
+NULL, , );
+ if (status == EFI_NOT_FOUND)
+ return efi_secureboot_mode_disabled;
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ size = sizeof(setupmode);
+ status = get_efi_var(efi_SetupMode_name, _variable_guid,
+NULL, , );
+ if (status != EFI_SUCCESS)
+ goto out_efi_err;
+
+ if (secboot == 0 || setupmode == 1)
+ return efi_secureboot_mode_disabled;
+
+ /*
+* See if a user has put the shim into insecure mode. If so, and if the
+* variable doesn't have the runtime attribute set, we might as well
+* honor that.
+*/
+ size = sizeof(moksbstate);
+ status = get_efi_var(shim_MokSBState_name, _guid,
+, , );
+
+ /* If it fails, we don't care why. Default to secure */
+ if (status != EFI_SUCCESS)
+ goto secure_boot_enabled;
+ if (!(attr & EFI_VARIABLE_RUNTIME_ACCESS) && moksbstate == 1)
+ return efi_secureboot_mode_disabled;
+
+secure_boot_enabled:
+ pr_efi(sys_table_arg, "UEFI Secure Boot is enabled.\n");
+ return efi_secureboot_mode_enabled;
+
+out_efi_err:
+ pr_efi_err(sys_table_arg, "Could not determine UEFI Secure Boot
status.\n");
+ return efi_secureboot_mode_unknown;
+}
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 959777e..4a6159f 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -14,73 +14,9 @@
#include "efistub.h"
-/* BIOS variables */
-static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t efi_SecureBoot_name[] = {
- 'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
-};
-static const efi_char16_t efi_SetupMode_name[] = {
- 'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
-};
-
-/* SHIM variables */
-static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
-static efi_char16_t const shim_MokSBState_name[] = {
- 'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
-};
-
#define get_efi_var(name, vendor, ...) \
efi_call_runtime(get_variable, \
(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
__VA_ARGS__);
-/*
- * Determine whether we're in secure boot mode.
- */
-enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
-{
- u32 attr;
- u8 secboot, setupmode, moksbstate;
- unsigned long size;
- efi_status_t status;
-
- size = sizeof(secboot);
- status = get_efi_var(efi_SecureBoot_name, _variable_guid,
-NULL, , );
- if (statu
[PATCH 2/4] x86/xen/efi: Initialize boot_params.secure_boot in xen_efi_init()
Otherwise the kernel reports incorrect UEFI secure boot state in the Xen dom0.
By the way fix CFLAGS_mmu_pv.o assignment alignment.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
arch/x86/xen/Makefile |4 +++-
arch/x86/xen/efi.c| 11 +++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index d83cb54..1b07664 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -12,7 +12,9 @@ endif
# Make sure early boot has no stackprotector
nostackp := $(call cc-option, -fno-stack-protector)
CFLAGS_enlighten_pv.o := $(nostackp)
-CFLAGS_mmu_pv.o:= $(nostackp)
+CFLAGS_mmu_pv.o:= $(nostackp)
+
+CFLAGS_efi.o += -I$(srctree)/drivers/firmware
obj-y := enlighten.o multicalls.o mmu.o irq.o \
time.o xen-asm.o xen-asm_$(BITS).o \
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703b..e089fa7 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -28,6 +28,15 @@
#include
#include
+#define pr_efi(sys_table, msg)
+#define pr_efi_err(sys_table, msg)
+
+#define get_efi_var(name, vendor, attr, data_size, data) \
+ xen_efi_get_variable((efi_char16_t *)name, (efi_guid_t
*)vendor, \
+attr, data_size, data)
+
+#include
+
static efi_char16_t vendor[100] __initdata;
static efi_system_table_t efi_systab_xen __initdata = {
@@ -129,6 +138,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
+ boot_params.secure_boot = efi_get_secureboot(efi_systab_xen);
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
--
1.7.10.4
[PATCH 2/4] x86/xen/efi: Initialize boot_params.secure_boot in xen_efi_init()
Otherwise the kernel reports incorrect UEFI secure boot state in the Xen dom0.
By the way fix CFLAGS_mmu_pv.o assignment alignment.
Signed-off-by: Daniel Kiper
---
arch/x86/xen/Makefile |4 +++-
arch/x86/xen/efi.c| 11 +++
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
index d83cb54..1b07664 100644
--- a/arch/x86/xen/Makefile
+++ b/arch/x86/xen/Makefile
@@ -12,7 +12,9 @@ endif
# Make sure early boot has no stackprotector
nostackp := $(call cc-option, -fno-stack-protector)
CFLAGS_enlighten_pv.o := $(nostackp)
-CFLAGS_mmu_pv.o:= $(nostackp)
+CFLAGS_mmu_pv.o:= $(nostackp)
+
+CFLAGS_efi.o += -I$(srctree)/drivers/firmware
obj-y := enlighten.o multicalls.o mmu.o irq.o \
time.o xen-asm.o xen-asm_$(BITS).o \
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index a18703b..e089fa7 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -28,6 +28,15 @@
#include
#include
+#define pr_efi(sys_table, msg)
+#define pr_efi_err(sys_table, msg)
+
+#define get_efi_var(name, vendor, attr, data_size, data) \
+ xen_efi_get_variable((efi_char16_t *)name, (efi_guid_t
*)vendor, \
+attr, data_size, data)
+
+#include
+
static efi_char16_t vendor[100] __initdata;
static efi_system_table_t efi_systab_xen __initdata = {
@@ -129,6 +138,8 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
+ boot_params.secure_boot = efi_get_secureboot(efi_systab_xen);
+
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
set_bit(EFI_64BIT, );
--
1.7.10.4
[PATCH 3/4] efi: Tweak efi_get_secureboot() and its data section assignment
Otherwise they are not freed after the kernel proper init.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
arch/x86/xen/efi.c |3 +++
drivers/firmware/efi/libstub/secureboot-core.c | 12 ++--
drivers/firmware/efi/libstub/secureboot.c |3 +++
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index e089fa7..5ad2b8f 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -28,6 +28,9 @@
#include
#include
+#define __sb_init __init
+#define __sb_initconst __initconst
+
#define pr_efi(sys_table, msg)
#define pr_efi_err(sys_table, msg)
diff --git a/drivers/firmware/efi/libstub/secureboot-core.c
b/drivers/firmware/efi/libstub/secureboot-core.c
index 11a4feb..d503ee4 100644
--- a/drivers/firmware/efi/libstub/secureboot-core.c
+++ b/drivers/firmware/efi/libstub/secureboot-core.c
@@ -11,24 +11,24 @@
*/
/* BIOS variables */
-static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t efi_SecureBoot_name[] = {
+static const efi_guid_t efi_variable_guid __sb_initconst =
EFI_GLOBAL_VARIABLE_GUID;
+static const efi_char16_t efi_SecureBoot_name[] __sb_initconst = {
'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
};
-static const efi_char16_t efi_SetupMode_name[] = {
+static const efi_char16_t efi_SetupMode_name[] __sb_initconst = {
'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
};
/* SHIM variables */
-static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
-static const efi_char16_t shim_MokSBState_name[] = {
+static const efi_guid_t shim_guid __sb_initconst = EFI_SHIM_LOCK_GUID;
+static const efi_char16_t shim_MokSBState_name[] __sb_initconst = {
'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
};
/*
* Determine whether we're in secure boot mode.
*/
-enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+enum __sb_init efi_secureboot_mode efi_get_secureboot(efi_system_table_t
*sys_table_arg)
{
u32 attr;
u8 secboot, setupmode, moksbstate;
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 4a6159f..1142170 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -14,6 +14,9 @@
#include "efistub.h"
+#define __sb_init
+#define __sb_initconst
+
#define get_efi_var(name, vendor, ...) \
efi_call_runtime(get_variable, \
(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
--
1.7.10.4
[PATCH 3/4] efi: Tweak efi_get_secureboot() and its data section assignment
Otherwise they are not freed after the kernel proper init.
Signed-off-by: Daniel Kiper
---
arch/x86/xen/efi.c |3 +++
drivers/firmware/efi/libstub/secureboot-core.c | 12 ++--
drivers/firmware/efi/libstub/secureboot.c |3 +++
3 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index e089fa7..5ad2b8f 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -28,6 +28,9 @@
#include
#include
+#define __sb_init __init
+#define __sb_initconst __initconst
+
#define pr_efi(sys_table, msg)
#define pr_efi_err(sys_table, msg)
diff --git a/drivers/firmware/efi/libstub/secureboot-core.c
b/drivers/firmware/efi/libstub/secureboot-core.c
index 11a4feb..d503ee4 100644
--- a/drivers/firmware/efi/libstub/secureboot-core.c
+++ b/drivers/firmware/efi/libstub/secureboot-core.c
@@ -11,24 +11,24 @@
*/
/* BIOS variables */
-static const efi_guid_t efi_variable_guid = EFI_GLOBAL_VARIABLE_GUID;
-static const efi_char16_t efi_SecureBoot_name[] = {
+static const efi_guid_t efi_variable_guid __sb_initconst =
EFI_GLOBAL_VARIABLE_GUID;
+static const efi_char16_t efi_SecureBoot_name[] __sb_initconst = {
'S', 'e', 'c', 'u', 'r', 'e', 'B', 'o', 'o', 't', 0
};
-static const efi_char16_t efi_SetupMode_name[] = {
+static const efi_char16_t efi_SetupMode_name[] __sb_initconst = {
'S', 'e', 't', 'u', 'p', 'M', 'o', 'd', 'e', 0
};
/* SHIM variables */
-static const efi_guid_t shim_guid = EFI_SHIM_LOCK_GUID;
-static const efi_char16_t shim_MokSBState_name[] = {
+static const efi_guid_t shim_guid __sb_initconst = EFI_SHIM_LOCK_GUID;
+static const efi_char16_t shim_MokSBState_name[] __sb_initconst = {
'M', 'o', 'k', 'S', 'B', 'S', 't', 'a', 't', 'e', 0
};
/*
* Determine whether we're in secure boot mode.
*/
-enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+enum __sb_init efi_secureboot_mode efi_get_secureboot(efi_system_table_t
*sys_table_arg)
{
u32 attr;
u8 secboot, setupmode, moksbstate;
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 4a6159f..1142170 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -14,6 +14,9 @@
#include "efistub.h"
+#define __sb_init
+#define __sb_initconst
+
#define get_efi_var(name, vendor, ...) \
efi_call_runtime(get_variable, \
(efi_char16_t *)(name), (efi_guid_t *)(vendor), \
--
1.7.10.4
[PATCH 0/4] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Hi, Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel may not even know that it runs on secure boot enabled platform. Daniel arch/x86/xen/Makefile |4 +++- arch/x86/xen/efi.c | 14 + drivers/firmware/efi/libstub/secureboot-core.c | 77 + drivers/firmware/efi/libstub/secureboot.c | 66 +-- 4 files changed, 99 insertions(+), 62 deletions(-) Daniel Kiper (4): efi/stub: Extract efi_get_secureboot() to separate file x86/xen/efi: Initialize boot_params.secure_boot in xen_efi_init() efi: Tweak efi_get_secureboot() and its data section assignment efi: Rename efi_get_secureboot() to __efi_get_secureboot() and make it static
[PATCH 4/4] efi: Rename efi_get_secureboot() to __efi_get_secureboot() and make it static
This may help compiler to do some function call optimization.
This is rather cosmetic. If you like this patch apply.
If you do not you may ignore it.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
arch/x86/xen/efi.c |2 +-
drivers/firmware/efi/libstub/secureboot-core.c |2 +-
drivers/firmware/efi/libstub/secureboot.c |5 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 5ad2b8f..d45677f 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -141,7 +141,7 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
- boot_params.secure_boot = efi_get_secureboot(efi_systab_xen);
+ boot_params.secure_boot = __efi_get_secureboot(efi_systab_xen);
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
diff --git a/drivers/firmware/efi/libstub/secureboot-core.c
b/drivers/firmware/efi/libstub/secureboot-core.c
index d503ee4..07526a6 100644
--- a/drivers/firmware/efi/libstub/secureboot-core.c
+++ b/drivers/firmware/efi/libstub/secureboot-core.c
@@ -28,7 +28,7 @@
/*
* Determine whether we're in secure boot mode.
*/
-enum __sb_init efi_secureboot_mode efi_get_secureboot(efi_system_table_t
*sys_table_arg)
+static enum __sb_init efi_secureboot_mode
__efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
u32 attr;
u8 secboot, setupmode, moksbstate;
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 1142170..f872afd 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -23,3 +23,8 @@
__VA_ARGS__);
#include "secureboot-core.c"
+
+enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+{
+ return __efi_get_secureboot(sys_table_arg);
+}
--
1.7.10.4
[PATCH 0/4] x86/xen/efi: Initialize UEFI secure boot state during dom0 boot
Hi, Initialize UEFI secure boot state during dom0 boot. Otherwise the kernel may not even know that it runs on secure boot enabled platform. Daniel arch/x86/xen/Makefile |4 +++- arch/x86/xen/efi.c | 14 + drivers/firmware/efi/libstub/secureboot-core.c | 77 + drivers/firmware/efi/libstub/secureboot.c | 66 +-- 4 files changed, 99 insertions(+), 62 deletions(-) Daniel Kiper (4): efi/stub: Extract efi_get_secureboot() to separate file x86/xen/efi: Initialize boot_params.secure_boot in xen_efi_init() efi: Tweak efi_get_secureboot() and its data section assignment efi: Rename efi_get_secureboot() to __efi_get_secureboot() and make it static
[PATCH 4/4] efi: Rename efi_get_secureboot() to __efi_get_secureboot() and make it static
This may help compiler to do some function call optimization.
This is rather cosmetic. If you like this patch apply.
If you do not you may ignore it.
Signed-off-by: Daniel Kiper
---
arch/x86/xen/efi.c |2 +-
drivers/firmware/efi/libstub/secureboot-core.c |2 +-
drivers/firmware/efi/libstub/secureboot.c |5 +
3 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 5ad2b8f..d45677f 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -141,7 +141,7 @@ void __init xen_efi_init(void)
boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen);
boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) >>
32);
- boot_params.secure_boot = efi_get_secureboot(efi_systab_xen);
+ boot_params.secure_boot = __efi_get_secureboot(efi_systab_xen);
set_bit(EFI_BOOT, );
set_bit(EFI_PARAVIRT, );
diff --git a/drivers/firmware/efi/libstub/secureboot-core.c
b/drivers/firmware/efi/libstub/secureboot-core.c
index d503ee4..07526a6 100644
--- a/drivers/firmware/efi/libstub/secureboot-core.c
+++ b/drivers/firmware/efi/libstub/secureboot-core.c
@@ -28,7 +28,7 @@
/*
* Determine whether we're in secure boot mode.
*/
-enum __sb_init efi_secureboot_mode efi_get_secureboot(efi_system_table_t
*sys_table_arg)
+static enum __sb_init efi_secureboot_mode
__efi_get_secureboot(efi_system_table_t *sys_table_arg)
{
u32 attr;
u8 secboot, setupmode, moksbstate;
diff --git a/drivers/firmware/efi/libstub/secureboot.c
b/drivers/firmware/efi/libstub/secureboot.c
index 1142170..f872afd 100644
--- a/drivers/firmware/efi/libstub/secureboot.c
+++ b/drivers/firmware/efi/libstub/secureboot.c
@@ -23,3 +23,8 @@
__VA_ARGS__);
#include "secureboot-core.c"
+
+enum efi_secureboot_mode efi_get_secureboot(efi_system_table_t *sys_table_arg)
+{
+ return __efi_get_secureboot(sys_table_arg);
+}
--
1.7.10.4
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
On Thu, Jul 20, 2017 at 11:16:39AM +0200, Greg Kroah-Hartman wrote: > On Thu, Jul 20, 2017 at 10:39:10AM +0200, Ingo Molnar wrote: > > > > * Daniel Kiper <daniel.ki...@oracle.com> wrote: > > > > > Hey Greg, > > > > > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote: > > > > 4.12-stable review patch. If anyone has any objections, please let me > > > > know. > > > > > > Why did you skip this patch for 4.11? IMO it should be applied there too. > > > > The thing is, this patch should probaly not even be in v4.12, as it should > > only > > make any difference if there's a separate _bug_ in the kernel. > > > > This patch makes things more robust going forward, but I question that it > > needs to > > be in -stable. > > Yeah, good point, I'm going to go drop it entirely from the 4.12-stable > tree as it obviously isn't stable material, sorry for not catching that > before. Wait a minute. IIRC, Juergen told me last week that this patch fixes a bug found/assigned by/to him. Juergen? If it is true then I would apply it to stable. If I am wrong you can drop it. Thanks, Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
On Thu, Jul 20, 2017 at 11:16:39AM +0200, Greg Kroah-Hartman wrote: > On Thu, Jul 20, 2017 at 10:39:10AM +0200, Ingo Molnar wrote: > > > > * Daniel Kiper wrote: > > > > > Hey Greg, > > > > > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote: > > > > 4.12-stable review patch. If anyone has any objections, please let me > > > > know. > > > > > > Why did you skip this patch for 4.11? IMO it should be applied there too. > > > > The thing is, this patch should probaly not even be in v4.12, as it should > > only > > make any difference if there's a separate _bug_ in the kernel. > > > > This patch makes things more robust going forward, but I question that it > > needs to > > be in -stable. > > Yeah, good point, I'm going to go drop it entirely from the 4.12-stable > tree as it obviously isn't stable material, sorry for not catching that > before. Wait a minute. IIRC, Juergen told me last week that this patch fixes a bug found/assigned by/to him. Juergen? If it is true then I would apply it to stable. If I am wrong you can drop it. Thanks, Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
On Wed, Jul 19, 2017 at 01:19:58PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote:
> > > Hey Greg,
> > >
> > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > > 4.12-stable review patch. If anyone has any objections, please let me
> > > > know.
> > >
> > > Why did you skip this patch for 4.11? IMO it should be applied there too.
> >
> > Are you sure it actually applied? (hint, it did not...)
> >
> > If you want it in 4.11, or older kernels, please provide a working
> > backport.
>
> And, in the future, if you want it to be applied to older kernels, or be
> notified if it can not be, please add a kernel version number in the
> stable marking:
> Cc: sta...@vger.kernel.org # 4.0+
> or use the Fixes: tag:
> Fixes: SHASHAHSA ("short description")
> which I pick up on and let you know if the patch does not actually apply
> back to the kernel that the fixes: tag was in.
>
> hope this helps,
Sure thing! Thanks a lot!
Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
On Wed, Jul 19, 2017 at 01:19:58PM +0200, Greg Kroah-Hartman wrote:
> On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote:
> > On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote:
> > > Hey Greg,
> > >
> > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote:
> > > > 4.12-stable review patch. If anyone has any objections, please let me
> > > > know.
> > >
> > > Why did you skip this patch for 4.11? IMO it should be applied there too.
> >
> > Are you sure it actually applied? (hint, it did not...)
> >
> > If you want it in 4.11, or older kernels, please provide a working
> > backport.
>
> And, in the future, if you want it to be applied to older kernels, or be
> notified if it can not be, please add a kernel version number in the
> stable marking:
> Cc: sta...@vger.kernel.org # 4.0+
> or use the Fixes: tag:
> Fixes: SHASHAHSA ("short description")
> which I pick up on and let you know if the patch does not actually apply
> back to the kernel that the fixes: tag was in.
>
> hope this helps,
Sure thing! Thanks a lot!
Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote: > On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote: > > Hey Greg, > > > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote: > > > 4.12-stable review patch. If anyone has any objections, please let me > > > know. > > > > Why did you skip this patch for 4.11? IMO it should be applied there too. > > Are you sure it actually applied? (hint, it did not...) > > If you want it in 4.11, or older kernels, please provide a working > backport. OK, if it did not apply then probably there were some changes in the code here and there. Though, IIRC, fix itself is perfectly valid for 4.11. So, I will post updated patch for it. Thanks, Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
On Wed, Jul 19, 2017 at 01:12:14PM +0200, Greg Kroah-Hartman wrote: > On Wed, Jul 19, 2017 at 12:37:47PM +0200, Daniel Kiper wrote: > > Hey Greg, > > > > On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote: > > > 4.12-stable review patch. If anyone has any objections, please let me > > > know. > > > > Why did you skip this patch for 4.11? IMO it should be applied there too. > > Are you sure it actually applied? (hint, it did not...) > > If you want it in 4.11, or older kernels, please provide a working > backport. OK, if it did not apply then probably there were some changes in the code here and there. Though, IIRC, fix itself is perfectly valid for 4.11. So, I will post updated patch for it. Thanks, Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
Hey Greg, On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote: > 4.12-stable review patch. If anyone has any objections, please let me know. Why did you skip this patch for 4.11? IMO it should be applied there too. Daniel
Re: [PATCH 4.12 26/84] x86/xen/efi: Initialize only the EFI struct members used by Xen
Hey Greg, On Wed, Jul 19, 2017 at 11:43:32AM +0200, Greg Kroah-Hartman wrote: > 4.12-stable review patch. If anyone has any objections, please let me know. Why did you skip this patch for 4.11? IMO it should be applied there too. Daniel
[tip:efi/core] x86/xen/efi: Initialize only the EFI struct members used by Xen
Commit-ID: 6c64447ec58b0bac612732303f7ab04562124587
Gitweb: http://git.kernel.org/tip/6c64447ec58b0bac612732303f7ab04562124587
Author: Daniel Kiper <daniel.ki...@oracle.com>
AuthorDate: Thu, 22 Jun 2017 12:51:37 +0200
Committer: Ingo Molnar <mi...@kernel.org>
CommitDate: Fri, 23 Jun 2017 11:11:03 +0200
x86/xen/efi: Initialize only the EFI struct members used by Xen
The current approach, which is the wholesale efi struct initialization from
a 'efi_xen' local template is not robust. Usually if new member is defined
then it is properly initialized in drivers/firmware/efi/efi.c, but not in
arch/x86/xen/efi.c.
The effect is that the Xen initialization clears any fields the generic code
might have set and the Xen code does not know about yet.
I saw this happen a few times, so let's initialize only the EFI struct members
used by Xen and maintain no local duplicate, to avoid such issues in the future.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Reviewed-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: andrew.coop...@citrix.com
Cc: jgr...@suse.com
Cc: linux-...@vger.kernel.org
Cc: m...@codeblueprint.co.uk
Cc: sta...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Link:
http://lkml.kernel.org/r/1498128697-12943-3-git-send-email-daniel.ki...@oracle.com
[ Clarified the changelog. ]
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
arch/x86/xen/efi.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..a18703b 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@ static efi_system_table_t efi_systab_xen __initdata = {
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0,/* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info= EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab= EFI_INVALID_TABLE_ADDR,
- .fw_vendor= EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable= xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags= 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ efi.get_time = xen_efi_get_time;
+ efi.set_time = xen_efi_set_time;
+ efi.get_wakeup_time = xen_efi_get_wakeup_time;
+ efi.set_wakeup_time = xen_efi_set_wakeup_time;
+ efi.get_variable = xen_efi_get_variable;
+ efi.get_next_variable= xen_efi_get_next_variable;
+ efi.set_variable = xen_efi_set_variable;
+ efi.query_variable_info = xen_efi_query_variable_info;
+ efi.update_capsule = xen_efi_update_capsule;
+ efi.query_capsule_caps = xen_efi_query_capsule_caps;
+ efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+ efi.reset_system = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
[tip:efi/core] x86/xen/efi: Initialize only the EFI struct members used by Xen
Commit-ID: 6c64447ec58b0bac612732303f7ab04562124587
Gitweb: http://git.kernel.org/tip/6c64447ec58b0bac612732303f7ab04562124587
Author: Daniel Kiper
AuthorDate: Thu, 22 Jun 2017 12:51:37 +0200
Committer: Ingo Molnar
CommitDate: Fri, 23 Jun 2017 11:11:03 +0200
x86/xen/efi: Initialize only the EFI struct members used by Xen
The current approach, which is the wholesale efi struct initialization from
a 'efi_xen' local template is not robust. Usually if new member is defined
then it is properly initialized in drivers/firmware/efi/efi.c, but not in
arch/x86/xen/efi.c.
The effect is that the Xen initialization clears any fields the generic code
might have set and the Xen code does not know about yet.
I saw this happen a few times, so let's initialize only the EFI struct members
used by Xen and maintain no local duplicate, to avoid such issues in the future.
Signed-off-by: Daniel Kiper
Reviewed-by: Boris Ostrovsky
Acked-by: Ard Biesheuvel
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: andrew.coop...@citrix.com
Cc: jgr...@suse.com
Cc: linux-...@vger.kernel.org
Cc: m...@codeblueprint.co.uk
Cc: sta...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Link:
http://lkml.kernel.org/r/1498128697-12943-3-git-send-email-daniel.ki...@oracle.com
[ Clarified the changelog. ]
Signed-off-by: Ingo Molnar
---
arch/x86/xen/efi.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..a18703b 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@ static efi_system_table_t efi_systab_xen __initdata = {
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0,/* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info= EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab= EFI_INVALID_TABLE_ADDR,
- .fw_vendor= EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable= xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags= 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ efi.get_time = xen_efi_get_time;
+ efi.set_time = xen_efi_set_time;
+ efi.get_wakeup_time = xen_efi_get_wakeup_time;
+ efi.set_wakeup_time = xen_efi_set_wakeup_time;
+ efi.get_variable = xen_efi_get_variable;
+ efi.get_next_variable= xen_efi_get_next_variable;
+ efi.set_variable = xen_efi_set_variable;
+ efi.query_variable_info = xen_efi_query_variable_info;
+ efi.update_capsule = xen_efi_update_capsule;
+ efi.query_capsule_caps = xen_efi_query_capsule_caps;
+ efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+ efi.reset_system = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
[tip:efi/core] efi: Process the MEMATTR table only if EFI_MEMMAP is enabled
Commit-ID: 457ea3f7e97881f937136ce0ba1f29f82b9abdb0
Gitweb: http://git.kernel.org/tip/457ea3f7e97881f937136ce0ba1f29f82b9abdb0
Author: Daniel Kiper <daniel.ki...@oracle.com>
AuthorDate: Thu, 22 Jun 2017 12:51:36 +0200
Committer: Ingo Molnar <mi...@kernel.org>
CommitDate: Fri, 23 Jun 2017 11:11:02 +0200
efi: Process the MEMATTR table only if EFI_MEMMAP is enabled
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more targeted and covers more cases.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Cc: andrew.coop...@citrix.com
Cc: boris.ostrov...@oracle.com
Cc: jgr...@suse.com
Cc: linux-...@vger.kernel.org
Cc: m...@codeblueprint.co.uk
Cc: sta...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Link:
http://lkml.kernel.org/r/1498128697-12943-2-git-send-email-daniel.ki...@oracle.com
Signed-off-by: Ingo Molnar <mi...@kernel.org>
---
drivers/firmware/efi/efi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
[tip:efi/core] efi: Process the MEMATTR table only if EFI_MEMMAP is enabled
Commit-ID: 457ea3f7e97881f937136ce0ba1f29f82b9abdb0
Gitweb: http://git.kernel.org/tip/457ea3f7e97881f937136ce0ba1f29f82b9abdb0
Author: Daniel Kiper
AuthorDate: Thu, 22 Jun 2017 12:51:36 +0200
Committer: Ingo Molnar
CommitDate: Fri, 23 Jun 2017 11:11:02 +0200
efi: Process the MEMATTR table only if EFI_MEMMAP is enabled
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more targeted and covers more cases.
Signed-off-by: Daniel Kiper
Reviewed-by: Ard Biesheuvel
Cc: Linus Torvalds
Cc: Peter Zijlstra
Cc: Thomas Gleixner
Cc: andrew.coop...@citrix.com
Cc: boris.ostrov...@oracle.com
Cc: jgr...@suse.com
Cc: linux-...@vger.kernel.org
Cc: m...@codeblueprint.co.uk
Cc: sta...@vger.kernel.org
Cc: xen-de...@lists.xenproject.org
Link:
http://lkml.kernel.org/r/1498128697-12943-2-git-send-email-daniel.ki...@oracle.com
Signed-off-by: Ingo Molnar
---
drivers/firmware/efi/efi.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
[PATCH v2 2/2] x86/xen/efi: Init only efi struct members used by Xen
Current approach, wholesale efi struct initialization from efi_xen, is not
good. Usually if new member is defined then it is properly initialized in
drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Acked-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
--
Align assignments to increase readability. Suggested by Ingo Molnar.
---
arch/x86/xen/efi.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..a18703b 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0,/* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info= EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab= EFI_INVALID_TABLE_ADDR,
- .fw_vendor= EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable= xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags= 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ efi.get_time = xen_efi_get_time;
+ efi.set_time = xen_efi_set_time;
+ efi.get_wakeup_time = xen_efi_get_wakeup_time;
+ efi.set_wakeup_time = xen_efi_set_wakeup_time;
+ efi.get_variable = xen_efi_get_variable;
+ efi.get_next_variable= xen_efi_get_next_variable;
+ efi.set_variable = xen_efi_set_variable;
+ efi.query_variable_info = xen_efi_query_variable_info;
+ efi.update_capsule = xen_efi_update_capsule;
+ efi.query_capsule_caps = xen_efi_query_capsule_caps;
+ efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+ efi.reset_system = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
--
1.7.10.4
[PATCH v2 2/2] x86/xen/efi: Init only efi struct members used by Xen
Current approach, wholesale efi struct initialization from efi_xen, is not
good. Usually if new member is defined then it is properly initialized in
drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.
Signed-off-by: Daniel Kiper
Acked-by: Ard Biesheuvel
--
Align assignments to increase readability. Suggested by Ingo Molnar.
---
arch/x86/xen/efi.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..a18703b 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0,/* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info= EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab= EFI_INVALID_TABLE_ADDR,
- .fw_vendor= EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable= xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags= 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ efi.get_time = xen_efi_get_time;
+ efi.set_time = xen_efi_set_time;
+ efi.get_wakeup_time = xen_efi_get_wakeup_time;
+ efi.set_wakeup_time = xen_efi_set_wakeup_time;
+ efi.get_variable = xen_efi_get_variable;
+ efi.get_next_variable= xen_efi_get_next_variable;
+ efi.set_variable = xen_efi_set_variable;
+ efi.query_variable_info = xen_efi_query_variable_info;
+ efi.update_capsule = xen_efi_update_capsule;
+ efi.query_capsule_caps = xen_efi_query_capsule_caps;
+ efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+ efi.reset_system = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
--
1.7.10.4
[PATCH v2 1/2] efi: Process MEMATTR table only if EFI_MEMMAP
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more generic and covers more cases.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
Reviewed-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
---
drivers/firmware/efi/efi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
--
1.7.10.4
[PATCH v2 1/2] efi: Process MEMATTR table only if EFI_MEMMAP
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more generic and covers more cases.
Signed-off-by: Daniel Kiper
Reviewed-by: Ard Biesheuvel
---
drivers/firmware/efi/efi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
--
1.7.10.4
[PATCH v2 0/2] xen/efi: Fixes
Hey, Two small fixes (v2, minor cleanup) for Xen dom0 running on x86_64 EFI platforms. I am CC-ing stable maintainers because similar stuff is needed for various stable kernels too. Unfortunately, almost every version needs a bit different set of fixes. So, please treat this email more as head up than real set of patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me a line. Then I will prepare set of patches for your kernel (if needed). Ard, Andrew, Ingo, thank you for looking at the patches. Daniel arch/x86/xen/efi.c | 45 - drivers/firmware/efi/efi.c |3 ++- 2 files changed, 14 insertions(+), 34 deletions(-) Daniel Kiper (2): efi: Process MEMATTR table only if EFI_MEMMAP x86/xen/efi: Init only efi struct members used by Xen
[PATCH v2 0/2] xen/efi: Fixes
Hey, Two small fixes (v2, minor cleanup) for Xen dom0 running on x86_64 EFI platforms. I am CC-ing stable maintainers because similar stuff is needed for various stable kernels too. Unfortunately, almost every version needs a bit different set of fixes. So, please treat this email more as head up than real set of patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me a line. Then I will prepare set of patches for your kernel (if needed). Ard, Andrew, Ingo, thank you for looking at the patches. Daniel arch/x86/xen/efi.c | 45 - drivers/firmware/efi/efi.c |3 ++- 2 files changed, 14 insertions(+), 34 deletions(-) Daniel Kiper (2): efi: Process MEMATTR table only if EFI_MEMMAP x86/xen/efi: Init only efi struct members used by Xen
Re: [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
On Wed, Jun 21, 2017 at 11:24:06AM +0200, Ingo Molnar wrote:
>
> * Daniel Kiper <daniel.ki...@oracle.com> wrote:
>
> > -static const struct efi efi_xen __initconst = {
> > - .systab = NULL, /* Initialized later. */
> > - .runtime_version = 0,/* Initialized later. */
> > - .mps = EFI_INVALID_TABLE_ADDR,
> > - .acpi = EFI_INVALID_TABLE_ADDR,
> > - .acpi20 = EFI_INVALID_TABLE_ADDR,
> > - .smbios = EFI_INVALID_TABLE_ADDR,
> > - .smbios3 = EFI_INVALID_TABLE_ADDR,
> > - .sal_systab = EFI_INVALID_TABLE_ADDR,
> > - .boot_info= EFI_INVALID_TABLE_ADDR,
> > - .hcdp = EFI_INVALID_TABLE_ADDR,
> > - .uga = EFI_INVALID_TABLE_ADDR,
> > - .uv_systab= EFI_INVALID_TABLE_ADDR,
> > - .fw_vendor= EFI_INVALID_TABLE_ADDR,
> > - .runtime = EFI_INVALID_TABLE_ADDR,
> > - .config_table = EFI_INVALID_TABLE_ADDR,
> > - .get_time = xen_efi_get_time,
> > - .set_time = xen_efi_set_time,
> > - .get_wakeup_time = xen_efi_get_wakeup_time,
> > - .set_wakeup_time = xen_efi_set_wakeup_time,
> > - .get_variable = xen_efi_get_variable,
> > - .get_next_variable= xen_efi_get_next_variable,
> > - .set_variable = xen_efi_set_variable,
> > - .query_variable_info = xen_efi_query_variable_info,
> > - .update_capsule = xen_efi_update_capsule,
> > - .query_capsule_caps = xen_efi_query_capsule_caps,
> > - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> > - .reset_system = xen_efi_reset_system,
> > - .set_virtual_address_map = NULL, /* Not used under Xen. */
> > - .flags= 0 /* Initialized later. */
> > -};
> > -
> > static efi_system_table_t __init *xen_efi_probe(void)
> > {
> > struct xen_platform_op op = {
> > @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
> >
> > /* Here we know that Xen runs on EFI platform. */
> >
> > - efi = efi_xen;
> > + efi.get_time = xen_efi_get_time;
> > + efi.set_time = xen_efi_set_time;
> > + efi.get_wakeup_time = xen_efi_get_wakeup_time;
> > + efi.set_wakeup_time = xen_efi_set_wakeup_time;
> > + efi.get_variable = xen_efi_get_variable;
> > + efi.get_next_variable = xen_efi_get_next_variable;
> > + efi.set_variable = xen_efi_set_variable;
> > + efi.query_variable_info = xen_efi_query_variable_info;
> > + efi.update_capsule = xen_efi_update_capsule;
> > + efi.query_capsule_caps = xen_efi_query_capsule_caps;
> > + efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> > + efi.reset_system = xen_efi_reset_system;
>
> This is a step back stylistically, as you lost the nice vertical tabulation
> of the
> original initializer ...
If you wish and others do not object I can realign it back.
Daniel
Re: [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
On Wed, Jun 21, 2017 at 11:24:06AM +0200, Ingo Molnar wrote:
>
> * Daniel Kiper wrote:
>
> > -static const struct efi efi_xen __initconst = {
> > - .systab = NULL, /* Initialized later. */
> > - .runtime_version = 0,/* Initialized later. */
> > - .mps = EFI_INVALID_TABLE_ADDR,
> > - .acpi = EFI_INVALID_TABLE_ADDR,
> > - .acpi20 = EFI_INVALID_TABLE_ADDR,
> > - .smbios = EFI_INVALID_TABLE_ADDR,
> > - .smbios3 = EFI_INVALID_TABLE_ADDR,
> > - .sal_systab = EFI_INVALID_TABLE_ADDR,
> > - .boot_info= EFI_INVALID_TABLE_ADDR,
> > - .hcdp = EFI_INVALID_TABLE_ADDR,
> > - .uga = EFI_INVALID_TABLE_ADDR,
> > - .uv_systab= EFI_INVALID_TABLE_ADDR,
> > - .fw_vendor= EFI_INVALID_TABLE_ADDR,
> > - .runtime = EFI_INVALID_TABLE_ADDR,
> > - .config_table = EFI_INVALID_TABLE_ADDR,
> > - .get_time = xen_efi_get_time,
> > - .set_time = xen_efi_set_time,
> > - .get_wakeup_time = xen_efi_get_wakeup_time,
> > - .set_wakeup_time = xen_efi_set_wakeup_time,
> > - .get_variable = xen_efi_get_variable,
> > - .get_next_variable= xen_efi_get_next_variable,
> > - .set_variable = xen_efi_set_variable,
> > - .query_variable_info = xen_efi_query_variable_info,
> > - .update_capsule = xen_efi_update_capsule,
> > - .query_capsule_caps = xen_efi_query_capsule_caps,
> > - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> > - .reset_system = xen_efi_reset_system,
> > - .set_virtual_address_map = NULL, /* Not used under Xen. */
> > - .flags= 0 /* Initialized later. */
> > -};
> > -
> > static efi_system_table_t __init *xen_efi_probe(void)
> > {
> > struct xen_platform_op op = {
> > @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
> >
> > /* Here we know that Xen runs on EFI platform. */
> >
> > - efi = efi_xen;
> > + efi.get_time = xen_efi_get_time;
> > + efi.set_time = xen_efi_set_time;
> > + efi.get_wakeup_time = xen_efi_get_wakeup_time;
> > + efi.set_wakeup_time = xen_efi_set_wakeup_time;
> > + efi.get_variable = xen_efi_get_variable;
> > + efi.get_next_variable = xen_efi_get_next_variable;
> > + efi.set_variable = xen_efi_set_variable;
> > + efi.query_variable_info = xen_efi_query_variable_info;
> > + efi.update_capsule = xen_efi_update_capsule;
> > + efi.query_capsule_caps = xen_efi_query_capsule_caps;
> > + efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> > + efi.reset_system = xen_efi_reset_system;
>
> This is a step back stylistically, as you lost the nice vertical tabulation
> of the
> original initializer ...
If you wish and others do not object I can realign it back.
Daniel
Re: [Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
On Wed, Jun 21, 2017 at 09:10:51AM +0100, Andrew Cooper wrote:
> On 20/06/2017 21:14, Daniel Kiper wrote:
> > Current approach, wholesale efi struct initialization from efi_xen, is not
> > good. Usually if new member is defined then it is properly initialized in
> > drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it
> > happened
> > a few times until now. So, let's initialize only efi struct members used by
> > Xen to avoid such issues in the future.
> >
> > Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
> > ---
> > arch/x86/xen/efi.c | 45 -
> > 1 file changed, 12 insertions(+), 33 deletions(-)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index 30bb2e8..01b9faf 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -54,38 +54,6 @@
> > .tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
> > };
> >
> > -static const struct efi efi_xen __initconst = {
> > - .systab = NULL, /* Initialized later. */
> > - .runtime_version = 0,/* Initialized later. */
> > - .mps = EFI_INVALID_TABLE_ADDR,
> > - .acpi = EFI_INVALID_TABLE_ADDR,
> > - .acpi20 = EFI_INVALID_TABLE_ADDR,
> > - .smbios = EFI_INVALID_TABLE_ADDR,
> > - .smbios3 = EFI_INVALID_TABLE_ADDR,
> > - .sal_systab = EFI_INVALID_TABLE_ADDR,
> > - .boot_info= EFI_INVALID_TABLE_ADDR,
> > - .hcdp = EFI_INVALID_TABLE_ADDR,
> > - .uga = EFI_INVALID_TABLE_ADDR,
> > - .uv_systab= EFI_INVALID_TABLE_ADDR,
> > - .fw_vendor= EFI_INVALID_TABLE_ADDR,
> > - .runtime = EFI_INVALID_TABLE_ADDR,
> > - .config_table = EFI_INVALID_TABLE_ADDR,
> > - .get_time = xen_efi_get_time,
> > - .set_time = xen_efi_set_time,
> > - .get_wakeup_time = xen_efi_get_wakeup_time,
> > - .set_wakeup_time = xen_efi_set_wakeup_time,
> > - .get_variable = xen_efi_get_variable,
> > - .get_next_variable= xen_efi_get_next_variable,
> > - .set_variable = xen_efi_set_variable,
> > - .query_variable_info = xen_efi_query_variable_info,
> > - .update_capsule = xen_efi_update_capsule,
> > - .query_capsule_caps = xen_efi_query_capsule_caps,
> > - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> > - .reset_system = xen_efi_reset_system,
> > - .set_virtual_address_map = NULL, /* Not used under Xen. */
> > - .flags= 0 /* Initialized later. */
> > -};
> > -
> > static efi_system_table_t __init *xen_efi_probe(void)
> > {
> > struct xen_platform_op op = {
> > @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
> >
> > /* Here we know that Xen runs on EFI platform. */
> >
> > - efi = efi_xen;
> > + efi.get_time = xen_efi_get_time;
> > + efi.set_time = xen_efi_set_time;
> > + efi.get_wakeup_time = xen_efi_get_wakeup_time;
> > + efi.set_wakeup_time = xen_efi_set_wakeup_time;
> > + efi.get_variable = xen_efi_get_variable;
> > + efi.get_next_variable = xen_efi_get_next_variable;
> > + efi.set_variable = xen_efi_set_variable;
> > + efi.query_variable_info = xen_efi_query_variable_info;
> > + efi.update_capsule = xen_efi_update_capsule;
> > + efi.query_capsule_caps = xen_efi_query_capsule_caps;
> > + efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> > + efi.reset_system = xen_efi_reset_system;
>
> This presumably means that the system default values are already present
> in efi at the point that we overwrite some Xen specifics?
More or less.
> If so, surely you need to retain the clobbering of set_virtual_address_map ?
Nope, by default efi.set_virtual_address_map is NULL (please take a look
at efi struct initialization in drivers/firmware/efi/efi.c). And it is
not touched if efi_enabled(EFI_PARAVIRT).
Daniel
Re: [Xen-devel] [PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
On Wed, Jun 21, 2017 at 09:10:51AM +0100, Andrew Cooper wrote:
> On 20/06/2017 21:14, Daniel Kiper wrote:
> > Current approach, wholesale efi struct initialization from efi_xen, is not
> > good. Usually if new member is defined then it is properly initialized in
> > drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it
> > happened
> > a few times until now. So, let's initialize only efi struct members used by
> > Xen to avoid such issues in the future.
> >
> > Signed-off-by: Daniel Kiper
> > ---
> > arch/x86/xen/efi.c | 45 -
> > 1 file changed, 12 insertions(+), 33 deletions(-)
> >
> > diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
> > index 30bb2e8..01b9faf 100644
> > --- a/arch/x86/xen/efi.c
> > +++ b/arch/x86/xen/efi.c
> > @@ -54,38 +54,6 @@
> > .tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
> > };
> >
> > -static const struct efi efi_xen __initconst = {
> > - .systab = NULL, /* Initialized later. */
> > - .runtime_version = 0,/* Initialized later. */
> > - .mps = EFI_INVALID_TABLE_ADDR,
> > - .acpi = EFI_INVALID_TABLE_ADDR,
> > - .acpi20 = EFI_INVALID_TABLE_ADDR,
> > - .smbios = EFI_INVALID_TABLE_ADDR,
> > - .smbios3 = EFI_INVALID_TABLE_ADDR,
> > - .sal_systab = EFI_INVALID_TABLE_ADDR,
> > - .boot_info= EFI_INVALID_TABLE_ADDR,
> > - .hcdp = EFI_INVALID_TABLE_ADDR,
> > - .uga = EFI_INVALID_TABLE_ADDR,
> > - .uv_systab= EFI_INVALID_TABLE_ADDR,
> > - .fw_vendor= EFI_INVALID_TABLE_ADDR,
> > - .runtime = EFI_INVALID_TABLE_ADDR,
> > - .config_table = EFI_INVALID_TABLE_ADDR,
> > - .get_time = xen_efi_get_time,
> > - .set_time = xen_efi_set_time,
> > - .get_wakeup_time = xen_efi_get_wakeup_time,
> > - .set_wakeup_time = xen_efi_set_wakeup_time,
> > - .get_variable = xen_efi_get_variable,
> > - .get_next_variable= xen_efi_get_next_variable,
> > - .set_variable = xen_efi_set_variable,
> > - .query_variable_info = xen_efi_query_variable_info,
> > - .update_capsule = xen_efi_update_capsule,
> > - .query_capsule_caps = xen_efi_query_capsule_caps,
> > - .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
> > - .reset_system = xen_efi_reset_system,
> > - .set_virtual_address_map = NULL, /* Not used under Xen. */
> > - .flags= 0 /* Initialized later. */
> > -};
> > -
> > static efi_system_table_t __init *xen_efi_probe(void)
> > {
> > struct xen_platform_op op = {
> > @@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
> >
> > /* Here we know that Xen runs on EFI platform. */
> >
> > - efi = efi_xen;
> > + efi.get_time = xen_efi_get_time;
> > + efi.set_time = xen_efi_set_time;
> > + efi.get_wakeup_time = xen_efi_get_wakeup_time;
> > + efi.set_wakeup_time = xen_efi_set_wakeup_time;
> > + efi.get_variable = xen_efi_get_variable;
> > + efi.get_next_variable = xen_efi_get_next_variable;
> > + efi.set_variable = xen_efi_set_variable;
> > + efi.query_variable_info = xen_efi_query_variable_info;
> > + efi.update_capsule = xen_efi_update_capsule;
> > + efi.query_capsule_caps = xen_efi_query_capsule_caps;
> > + efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
> > + efi.reset_system = xen_efi_reset_system;
>
> This presumably means that the system default values are already present
> in efi at the point that we overwrite some Xen specifics?
More or less.
> If so, surely you need to retain the clobbering of set_virtual_address_map ?
Nope, by default efi.set_virtual_address_map is NULL (please take a look
at efi struct initialization in drivers/firmware/efi/efi.c). And it is
not touched if efi_enabled(EFI_PARAVIRT).
Daniel
[PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
Current approach, wholesale efi struct initialization from efi_xen, is not
good. Usually if new member is defined then it is properly initialized in
drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
arch/x86/xen/efi.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..01b9faf 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0,/* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info= EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab= EFI_INVALID_TABLE_ADDR,
- .fw_vendor= EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable= xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags= 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ efi.get_time = xen_efi_get_time;
+ efi.set_time = xen_efi_set_time;
+ efi.get_wakeup_time = xen_efi_get_wakeup_time;
+ efi.set_wakeup_time = xen_efi_set_wakeup_time;
+ efi.get_variable = xen_efi_get_variable;
+ efi.get_next_variable = xen_efi_get_next_variable;
+ efi.set_variable = xen_efi_set_variable;
+ efi.query_variable_info = xen_efi_query_variable_info;
+ efi.update_capsule = xen_efi_update_capsule;
+ efi.query_capsule_caps = xen_efi_query_capsule_caps;
+ efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+ efi.reset_system = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
--
1.7.10.4
[PATCH 2/2] x86/xen/efi: Init only efi struct members used by Xen
Current approach, wholesale efi struct initialization from efi_xen, is not
good. Usually if new member is defined then it is properly initialized in
drivers/firmware/efi/efi.c but not in arch/x86/xen/efi.c. As I saw it happened
a few times until now. So, let's initialize only efi struct members used by
Xen to avoid such issues in the future.
Signed-off-by: Daniel Kiper
---
arch/x86/xen/efi.c | 45 -
1 file changed, 12 insertions(+), 33 deletions(-)
diff --git a/arch/x86/xen/efi.c b/arch/x86/xen/efi.c
index 30bb2e8..01b9faf 100644
--- a/arch/x86/xen/efi.c
+++ b/arch/x86/xen/efi.c
@@ -54,38 +54,6 @@
.tables = EFI_INVALID_TABLE_ADDR /* Initialized later. */
};
-static const struct efi efi_xen __initconst = {
- .systab = NULL, /* Initialized later. */
- .runtime_version = 0,/* Initialized later. */
- .mps = EFI_INVALID_TABLE_ADDR,
- .acpi = EFI_INVALID_TABLE_ADDR,
- .acpi20 = EFI_INVALID_TABLE_ADDR,
- .smbios = EFI_INVALID_TABLE_ADDR,
- .smbios3 = EFI_INVALID_TABLE_ADDR,
- .sal_systab = EFI_INVALID_TABLE_ADDR,
- .boot_info= EFI_INVALID_TABLE_ADDR,
- .hcdp = EFI_INVALID_TABLE_ADDR,
- .uga = EFI_INVALID_TABLE_ADDR,
- .uv_systab= EFI_INVALID_TABLE_ADDR,
- .fw_vendor= EFI_INVALID_TABLE_ADDR,
- .runtime = EFI_INVALID_TABLE_ADDR,
- .config_table = EFI_INVALID_TABLE_ADDR,
- .get_time = xen_efi_get_time,
- .set_time = xen_efi_set_time,
- .get_wakeup_time = xen_efi_get_wakeup_time,
- .set_wakeup_time = xen_efi_set_wakeup_time,
- .get_variable = xen_efi_get_variable,
- .get_next_variable= xen_efi_get_next_variable,
- .set_variable = xen_efi_set_variable,
- .query_variable_info = xen_efi_query_variable_info,
- .update_capsule = xen_efi_update_capsule,
- .query_capsule_caps = xen_efi_query_capsule_caps,
- .get_next_high_mono_count = xen_efi_get_next_high_mono_count,
- .reset_system = xen_efi_reset_system,
- .set_virtual_address_map = NULL, /* Not used under Xen. */
- .flags= 0 /* Initialized later. */
-};
-
static efi_system_table_t __init *xen_efi_probe(void)
{
struct xen_platform_op op = {
@@ -102,7 +70,18 @@ static efi_system_table_t __init *xen_efi_probe(void)
/* Here we know that Xen runs on EFI platform. */
- efi = efi_xen;
+ efi.get_time = xen_efi_get_time;
+ efi.set_time = xen_efi_set_time;
+ efi.get_wakeup_time = xen_efi_get_wakeup_time;
+ efi.set_wakeup_time = xen_efi_set_wakeup_time;
+ efi.get_variable = xen_efi_get_variable;
+ efi.get_next_variable = xen_efi_get_next_variable;
+ efi.set_variable = xen_efi_set_variable;
+ efi.query_variable_info = xen_efi_query_variable_info;
+ efi.update_capsule = xen_efi_update_capsule;
+ efi.query_capsule_caps = xen_efi_query_capsule_caps;
+ efi.get_next_high_mono_count = xen_efi_get_next_high_mono_count;
+ efi.reset_system = xen_efi_reset_system;
efi_systab_xen.tables = info->cfg.addr;
efi_systab_xen.nr_tables = info->cfg.nent;
--
1.7.10.4
[PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more generic and covers more cases.
Signed-off-by: Daniel Kiper <daniel.ki...@oracle.com>
---
drivers/firmware/efi/efi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
--
1.7.10.4
[PATCH 1/2] efi: Process MEMATTR table only if EFI_MEMMAP
Otherwise e.g. Xen dom0 on x86_64 EFI platforms crashes.
In theory we can check EFI_PARAVIRT too, however,
EFI_MEMMAP looks more generic and covers more cases.
Signed-off-by: Daniel Kiper
---
drivers/firmware/efi/efi.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index b372aad..045d6d3 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -528,7 +528,8 @@ int __init efi_config_parse_tables(void *config_tables, int
count, int sz,
}
}
- efi_memattr_init();
+ if (efi_enabled(EFI_MEMMAP))
+ efi_memattr_init();
/* Parse the EFI Properties table if it exists */
if (efi.properties_table != EFI_INVALID_TABLE_ADDR) {
--
1.7.10.4
[PATCH 0/2] xen/efi: Fixes
Hey, Two small fixes for Xen dom0 running on x86_64 EFI platforms. I am CC-ing stable maintainers because similar stuff is needed for various stable kernels too. Unfortunately, almost every version needs a bit different set of fixes. So, please treat this email more as head up than real set of patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me a line. Then I will prepare set of patches for your kernel (if needed). Daniel arch/x86/xen/efi.c | 45 - drivers/firmware/efi/efi.c |3 ++- 2 files changed, 14 insertions(+), 34 deletions(-) Daniel Kiper (2): efi: Process MEMATTR table only if EFI_MEMMAP x86/xen/efi: Init only efi struct members used by Xen
[PATCH 0/2] xen/efi: Fixes
Hey, Two small fixes for Xen dom0 running on x86_64 EFI platforms. I am CC-ing stable maintainers because similar stuff is needed for various stable kernels too. Unfortunately, almost every version needs a bit different set of fixes. So, please treat this email more as head up than real set of patches for your kernel. If you wish to get Xen EFI stuff fixed just drop me a line. Then I will prepare set of patches for your kernel (if needed). Daniel arch/x86/xen/efi.c | 45 - drivers/firmware/efi/efi.c |3 ++- 2 files changed, 14 insertions(+), 34 deletions(-) Daniel Kiper (2): efi: Process MEMATTR table only if EFI_MEMMAP x86/xen/efi: Init only efi struct members used by Xen
Re: [PATCH] arm64: xen: Implement EFI reset_system callback
On Wed, Apr 19, 2017 at 08:37:38PM +0100, Matt Fleming wrote: > On Wed, 19 Apr, at 09:29:06PM, Daniel Kiper wrote: > > On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote: > > > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote: > > > > > > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper, > > > > rather than spreading it further. > > > > > > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper > > > > should provide an implementation. > > > > > > > > I don't see why you can't implement a wrapper that calls the usual Xen > > > > poweroff/reset functions. > > > > > > I realise I'm making a sweeping generalisation, but adding > > > EFI_PARAVIRT is almost always the wrong thing to do. > > > > Why? > > Because it makes paravirt a special case, and there's usually very > little reason to make it special in the EFI code. Special-casing means > more branches, more code paths, a bigger testing matrix and more > complex code. > > EFI_PARAVIRT does have its uses, like for those scenarios where we > don't have a table of function pointers that can be overidden for > paravirt. > > But we do have such a table for ->reset_system(). This is more or less what I expected. Thanks a lot for explanation. Daniel
Re: [PATCH] arm64: xen: Implement EFI reset_system callback
On Wed, Apr 19, 2017 at 08:37:38PM +0100, Matt Fleming wrote: > On Wed, 19 Apr, at 09:29:06PM, Daniel Kiper wrote: > > On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote: > > > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote: > > > > > > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper, > > > > rather than spreading it further. > > > > > > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper > > > > should provide an implementation. > > > > > > > > I don't see why you can't implement a wrapper that calls the usual Xen > > > > poweroff/reset functions. > > > > > > I realise I'm making a sweeping generalisation, but adding > > > EFI_PARAVIRT is almost always the wrong thing to do. > > > > Why? > > Because it makes paravirt a special case, and there's usually very > little reason to make it special in the EFI code. Special-casing means > more branches, more code paths, a bigger testing matrix and more > complex code. > > EFI_PARAVIRT does have its uses, like for those scenarios where we > don't have a table of function pointers that can be overidden for > paravirt. > > But we do have such a table for ->reset_system(). This is more or less what I expected. Thanks a lot for explanation. Daniel
Re: [PATCH] arm64: xen: Implement EFI reset_system callback
On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote: > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote: > > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper, > > rather than spreading it further. > > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper > > should provide an implementation. > > > > I don't see why you can't implement a wrapper that calls the usual Xen > > poweroff/reset functions. > > I realise I'm making a sweeping generalisation, but adding > EFI_PARAVIRT is almost always the wrong thing to do. Why? > I'd much prefer to see Mark's idea implemented. If you wish I do not object. Daniel
Re: [PATCH] arm64: xen: Implement EFI reset_system callback
On Tue, Apr 18, 2017 at 02:46:50PM +0100, Matt Fleming wrote: > On Thu, 06 Apr, at 04:55:11PM, Mark Rutland wrote: > > > > Please, let's keep the Xen knowledge constrained to the Xen EFI wrapper, > > rather than spreading it further. > > > > IMO, given reset_system is a *mandatory* function, the Xen wrapper > > should provide an implementation. > > > > I don't see why you can't implement a wrapper that calls the usual Xen > > poweroff/reset functions. > > I realise I'm making a sweeping generalisation, but adding > EFI_PARAVIRT is almost always the wrong thing to do. Why? > I'd much prefer to see Mark's idea implemented. If you wish I do not object. Daniel
Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()
On Fri, Apr 14, 2017 at 06:53:36PM +0200, Petr Tesarik wrote: > On Tue, 11 Apr 2017 19:20:08 +0200 > Daniel Kiper <daniel.ki...@oracle.com> wrote: > > On Tue, Apr 11, 2017 at 04:59:16PM +0200, Petr Tesarik wrote: > >[...] > > > Tested-by: Petr Tesarik <ptesa...@suse.com> > > > > > > I copied the complete /proc/vmcore to a directory on disk. Exactly > > > as expected, crash works both without the patch and with the patch, as > > > it does not use VMCOREINFO at all (instead, crash obtains the > > > information from kernel debuginfo directly). > > > > Thanks for doing the tests. I suppose that you have tested HVM guests. > > Not really. I crashed Dom0, which is in turn sent to the hypervisor, so > the result is a complete host dump, including Xen hypervisor data and > all domains. OK. > > IIRC, PV guests are not supported by crash right now due to p2m VMA > > mapping. At least it was an issue some time ago. Is it still valid? > > Yes, this is correct. I tested this behaviour a few weeks ago. Thanks for update. > > Anyway, one guy in Oracle works on fix for that issue and I do review. > > We are going to post it in 2-3 weeks. > > All right. FYI I do not plan to put much effort into it, as my focus has OK. > shifted towards libkdumpfile (https://github.com/ptesarik/libkdumpfile), > and this library can open PV guest dump files without any issues. Great! AIUI, it reminds my idea to make such think. However, I have not have time to make it happen. Is it based on makedumpfile or written from scratch? Do you plan support for Linux kernel dumps and/or Xen ones? Daniel
Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()
On Fri, Apr 14, 2017 at 06:53:36PM +0200, Petr Tesarik wrote: > On Tue, 11 Apr 2017 19:20:08 +0200 > Daniel Kiper wrote: > > On Tue, Apr 11, 2017 at 04:59:16PM +0200, Petr Tesarik wrote: > >[...] > > > Tested-by: Petr Tesarik > > > > > > I copied the complete /proc/vmcore to a directory on disk. Exactly > > > as expected, crash works both without the patch and with the patch, as > > > it does not use VMCOREINFO at all (instead, crash obtains the > > > information from kernel debuginfo directly). > > > > Thanks for doing the tests. I suppose that you have tested HVM guests. > > Not really. I crashed Dom0, which is in turn sent to the hypervisor, so > the result is a complete host dump, including Xen hypervisor data and > all domains. OK. > > IIRC, PV guests are not supported by crash right now due to p2m VMA > > mapping. At least it was an issue some time ago. Is it still valid? > > Yes, this is correct. I tested this behaviour a few weeks ago. Thanks for update. > > Anyway, one guy in Oracle works on fix for that issue and I do review. > > We are going to post it in 2-3 weeks. > > All right. FYI I do not plan to put much effort into it, as my focus has OK. > shifted towards libkdumpfile (https://github.com/ptesarik/libkdumpfile), > and this library can open PV guest dump files without any issues. Great! AIUI, it reminds my idea to make such think. However, I have not have time to make it happen. Is it based on makedumpfile or written from scratch? Do you plan support for Linux kernel dumps and/or Xen ones? Daniel
Re: [Xen-devel] [PATCH v2] xen, kdump: handle pv domain in paddr_vmcoreinfo_note()
On Tue, Apr 11, 2017 at 04:59:16PM +0200, Petr Tesarik wrote: > On Tue, 11 Apr 2017 15:00:58 +0200 > Daniel Kiper <daniel.ki...@oracle.com> wrote: > > > On Tue, Apr 11, 2017 at 02:45:43PM +0200, Juergen Gross wrote: > > > On 03/04/17 14:42, Daniel Kiper wrote: > > > > On Fri, Mar 31, 2017 at 12:14:38PM +0200, Juergen Gross wrote: > > > >> For kdump to work correctly it needs the physical address of > > > >> vmcoreinfo_note. When running as dom0 this means the virtual address > > > >> has to be translated to the related machine address. > > > >> > > > >> paddr_vmcoreinfo_note() is meant to do the translation via > > > >> __pa_symbol() only, but being attributed "weak" it can be replaced > > > >> easily in Xen case. > > > >> > > > >> Signed-off-by: Juergen Gross <jgr...@suse.com> > > > > > > > > Have you tested this patch with latest crash tool? Do dom0 and Xen > > > > hypervisor analysis work without any issue (at least basic commands > > > > like dmesg, bt, ps, etc.)? If yes for both you can add: > > > > > > > > Reviewed-by: Daniel Kiper <daniel.ki...@oracle.com> > > > > > > So can I add your R-b: now? > > > > My R-b is still valid. Though, let's wait for Petr's Tested-by. He > > did makedumpfile tests but I asked him to do crash tool tests too. > > I think it is important. > > Tested-by: Petr Tesarik <ptesa...@suse.com> > > I copied the complete /proc/vmcore to a directory on disk. Exactly > as expected, crash works both without the patch and with the patch, as > it does not use VMCOREINFO at all (instead, crash obtains the > information from kernel debuginfo directly). Thanks for doing the tests. I suppose that you have tested HVM guests. IIRC, PV guests are not supported by crash right now due to p2m VMA mapping. At least it was an issue some time ago. Is it still valid? Anyway, one guy in Oracle works on fix for that issue and I do review. We are going to post it in 2-3 weeks. Juergen, I do not have any objections any longer. You can go ahead. Ahhh... I see. You have posted v3. Good! Daniel
