Re: [PATCH v3 0/3] x86/boot: Introduce the kernel_info et consortes
On Wed, Oct 09, 2019 at 12:53:55PM +0200, Daniel Kiper wrote: > Hi, > > Due to very limited space in the setup_header this patch series introduces new > kernel_info struct which will be used to convey information from the kernel to > the bootloader. This way the boot protocol can be extended regardless of the > setup_header limitations. Additionally, the patch series introduces some > convenience features like the setup_indirect struct and the > kernel_info.setup_type_max field. > > Daniel > > Documentation/x86/boot.rst | 168 > ++ > arch/x86/boot/Makefile | 2 +- > arch/x86/boot/compressed/Makefile | 4 +- > arch/x86/boot/compressed/kaslr.c | 12 ++ > arch/x86/boot/compressed/kernel_info.S | 22 +++ > arch/x86/boot/header.S | 3 +- > arch/x86/boot/tools/build.c| 5 +++ > arch/x86/include/uapi/asm/bootparam.h | 16 +++- > arch/x86/kernel/e820.c | 11 ++ > arch/x86/kernel/kdebugfs.c | 20 -- > arch/x86/kernel/ksysfs.c | 30 ++ > arch/x86/kernel/setup.c| 4 ++ > arch/x86/mm/ioremap.c | 11 ++ > 13 files changed, 292 insertions(+), 16 deletions(-) > > Daniel Kiper (3): > x86/boot: Introduce the kernel_info > x86/boot: Introduce the kernel_info.setup_type_max > x86/boot: Introduce the setup_indirect hpa, ping? Daniel
Re: [PATCH v3 1/3] x86/boot: Introduce the kernel_info
On Wed, Oct 09, 2019 at 05:43:31PM -0700, Randy Dunlap wrote: > Hi, > > Questions and comments below... > Thanks. > > On 10/9/19 3:53 AM, Daniel Kiper wrote: > > > Suggested-by: H. Peter Anvin > > Signed-off-by: Daniel Kiper > > Reviewed-by: Konrad Rzeszutek Wilk > > Reviewed-by: Ross Philipson > > --- > > > --- > > Documentation/x86/boot.rst | 121 > > + > > arch/x86/boot/Makefile | 2 +- > > arch/x86/boot/compressed/Makefile | 4 +- > > arch/x86/boot/compressed/kernel_info.S | 17 + > > arch/x86/boot/header.S | 1 + > > arch/x86/boot/tools/build.c| 5 ++ > > arch/x86/include/uapi/asm/bootparam.h | 1 + > > 7 files changed, 148 insertions(+), 3 deletions(-) > > create mode 100644 arch/x86/boot/compressed/kernel_info.S > > > > diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst > > index 08a2f100c0e6..d5323a39f5e3 100644 > > --- a/Documentation/x86/boot.rst > > +++ b/Documentation/x86/boot.rst > > @@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field > > and extension fields > > Protocol 2.13 (Kernel 3.14) Support 32- and 64-bit flags being set in > > xloadflags to support booting a 64-bit kernel from 32-bit > > EFI > > + > > +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.5) Added the kernel_info. > > = > > > > > > +.. 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 kernel_info are changed. Additionally, it is recommended to use > > + xloadflags (in this case the protocol version number should not be > > + updated either) or kernel_info 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 kernel_info. > > + > > > > Memory Layout > > = > > @@ -207,6 +224,7 @@ Offset/Size Proto Name > > Meaning > > 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+ kernel_info_offset Offset of the > > kernel_info > > ==== > > > > > > .. note:: > > @@ -855,6 +873,109 @@ Offset/size: 0x264/4 > > > >See EFI HANDOVER PROTOCOL below for more details. > > > > + == > > +Field name:kernel_info_offset > > +Type: read > > +Offset/size: 0x268/4 > > +Protocol: 2.15+ > > + == > > + > > + This field is the offset from the beginning of the kernel image to the > > + kernel_info. It is embedded in the Linux image in the uncompressed > ^^ >What does It refer to, please? s/It/The kernel_info structure/ Is it better? > > + protected mode region. > > + > > + > > +The 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 -- > > inertia. > > +Also, the BIOS stub is responsible for creating boot_params, so it isn't > > +available to a BIOS-based loader (setup_data is, though
[PATCH v3 0/3] x86/boot: Introduce the kernel_info et consortes
Hi, Due to very limited space in the setup_header this patch series introduces new kernel_info struct which will be used to convey information from the kernel to the bootloader. This way the boot protocol can be extended regardless of the setup_header limitations. Additionally, the patch series introduces some convenience features like the setup_indirect struct and the kernel_info.setup_type_max field. Daniel Documentation/x86/boot.rst | 168 ++ arch/x86/boot/Makefile | 2 +- arch/x86/boot/compressed/Makefile | 4 +- arch/x86/boot/compressed/kaslr.c | 12 ++ arch/x86/boot/compressed/kernel_info.S | 22 +++ arch/x86/boot/header.S | 3 +- arch/x86/boot/tools/build.c| 5 +++ arch/x86/include/uapi/asm/bootparam.h | 16 +++- arch/x86/kernel/e820.c | 11 ++ arch/x86/kernel/kdebugfs.c | 20 -- arch/x86/kernel/ksysfs.c | 30 ++ arch/x86/kernel/setup.c| 4 ++ arch/x86/mm/ioremap.c | 11 ++ 13 files changed, 292 insertions(+), 16 deletions(-) Daniel Kiper (3): x86/boot: Introduce the kernel_info x86/boot: Introduce the kernel_info.setup_type_max x86/boot: Introduce the setup_indirect
[PATCH v3 2/3] x86/boot: Introduce the kernel_info.setup_type_max
This field contains maximal allowed type for setup_data. Now bump the setup_header version in arch/x86/boot/header.S. Suggested-by: H. Peter Anvin Signed-off-by: Daniel Kiper Reviewed-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Philipson --- Documentation/x86/boot.rst | 9 - arch/x86/boot/compressed/kernel_info.S | 5 + arch/x86/boot/header.S | 2 +- arch/x86/include/uapi/asm/bootparam.h | 3 +++ 4 files changed, 17 insertions(+), 2 deletions(-) diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst index d5323a39f5e3..4c536bc8816d 100644 --- a/Documentation/x86/boot.rst +++ b/Documentation/x86/boot.rst @@ -73,7 +73,7 @@ Protocol 2.14:BURNT BY INCORRECT COMMIT ae7e1238e68f2a472a125673ab506d49158c188 (x86/boot: Add ACPI RSDP address to setup_header) DO NOT USE!!! ASSUME SAME AS 2.13. -Protocol 2.15: (Kernel 5.5) Added the kernel_info. +Protocol 2.15: (Kernel 5.5) Added the kernel_info and kernel_info.setup_type_max. = .. note:: @@ -976,6 +976,13 @@ Offset/size: 0x0008/4 This field contains the size of the kernel_info including kernel_info.header and kernel_info.kernel_info_var_len_data. + == +Field name:setup_type_max +Offset/size: 0x0008/4 + == + + This field contains maximal allowed type for setup_data and setup_indirect structs. + The Image Checksum == diff --git a/arch/x86/boot/compressed/kernel_info.S b/arch/x86/boot/compressed/kernel_info.S index 8ea6f6e3feef..f818ee8fba38 100644 --- a/arch/x86/boot/compressed/kernel_info.S +++ b/arch/x86/boot/compressed/kernel_info.S @@ -1,5 +1,7 @@ /* SPDX-License-Identifier: GPL-2.0 */ +#include + .section ".rodata.kernel_info", "a" .global kernel_info @@ -12,6 +14,9 @@ kernel_info: /* Size total. */ .long kernel_info_end - kernel_info + /* Maximal allowed type for setup_data and setup_indirect structs. */ + .long SETUP_TYPE_MAX + kernel_info_var_len_data: /* Empty for time being... */ kernel_info_end: diff --git a/arch/x86/boot/header.S b/arch/x86/boot/header.S index 22dcecaaa898..97d9b6d6c1af 100644 --- a/arch/x86/boot/header.S +++ b/arch/x86/boot/header.S @@ -300,7 +300,7 @@ _start: # Part 2 of the header, from the old setup.S .ascii "HdrS" # header signature - .word 0x020d # header version number (>= 0x0105) + .word 0x020f # header version number (>= 0x0105) # or else old loadlin-1.5 will fail) .globl realmode_swtch realmode_swtch:.word 0, 0# default_switch, SETUPSEG diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index a1ebcd7a991c..dbb41128e5a0 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -11,6 +11,9 @@ #define SETUP_APPLE_PROPERTIES 5 #define SETUP_JAILHOUSE6 +/* max(SETUP_*) */ +#define SETUP_TYPE_MAX SETUP_JAILHOUSE + /* ram_size flags */ #define RAMDISK_IMAGE_START_MASK 0x07FF #define RAMDISK_PROMPT_FLAG0x8000 -- 2.11.0
[PATCH v3 1/3] x86/boot: Introduce the 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 -- inertia. 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. This patch does not bump setup_header version in arch/x86/boot/header.S because it will be followed by additional changes coming into the Linux/x86 boot protocol. Suggested-by: H. Peter Anvin Signed-off-by: Daniel Kiper Reviewed-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Philipson --- v3 - suggestions/fixes: - split kernel_info data into fixed and variable sized regions, (suggested by H. Peter Anvin), - change kernel_info.header value to "LToP" (0x506f544c), (suggested by H. Peter Anvin), - improve the comments, - improve the documentation. v2 - suggestions/fixes: - rename setup_header2 to kernel_info, (suggested by H. Peter Anvin), - change kernel_info.header value to "InfO" (0x4f666e49), - new kernel_info description in Documentation/x86/boot.rst, (suggested by H. Peter Anvin), - drop kernel_info_offset_update() as an overkill and update kernel_info offset directly from main(), (suggested by Eric Snowberg), - new commit message (suggested by H. Peter Anvin), - fix some commit message misspellings (suggested by Eric Snowberg). --- Documentation/x86/boot.rst | 121 + arch/x86/boot/Makefile | 2 +- arch/x86/boot/compressed/Makefile | 4 +- arch/x86/boot/compressed/kernel_info.S | 17 + arch/x86/boot/header.S | 1 + arch/x86/boot/tools/build.c| 5 ++ arch/x86/include/uapi/asm/bootparam.h | 1 + 7 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 arch/x86/boot/compressed/kernel_info.S diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst index 08a2f100c0e6..d5323a39f5e3 100644 --- a/Documentation/x86/boot.rst +++ b/Documentation/x86/boot.rst @@ -68,8 +68,25 @@ Protocol 2.12(Kernel 3.8) Added the xloadflags field and extension fields Protocol 2.13 (Kernel 3.14) Support 32- and 64-bit flags being set in xloadflags to support booting a 64-bit kernel from 32-bit EFI + +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.5) Added the kernel_info. = +.. 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 kernel_info are changed. Additionally, it is recommended to use + xloadflags (in this case the protocol version number should not be + updated either) or kernel_info 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 kernel_info. + Memory Layout = @@ -207,6 +224,7 @@ Offset/Size 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
[PATCH v3 3/3] x86/boot: Introduce the setup_indirect
The 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. Thus we introduce an uniform way to specify such indirect data as setup_indirect struct and SETUP_INDIRECT type. Suggested-by: H. Peter Anvin Signed-off-by: Daniel Kiper Acked-by: Konrad Rzeszutek Wilk Reviewed-by: Ross Philipson --- v3 - suggestions/fixes: - add setup_indirect mapping/KASLR avoidance/etc. code (suggested by H. Peter Anvin), - the SETUP_INDIRECT sets most significant bit right now; this way it is possible to differentiate regular setup_data and setup_indirect objects in the debugfs filesystem. v2 - suggestions/fixes: - add setup_indirect usage example (suggested by Eric Snowberg and Ross Philipson). --- Documentation/x86/boot.rst| 40 +++ arch/x86/boot/compressed/kaslr.c | 12 +++ arch/x86/include/uapi/asm/bootparam.h | 16 +++--- arch/x86/kernel/e820.c| 11 ++ arch/x86/kernel/kdebugfs.c| 20 ++ arch/x86/kernel/ksysfs.c | 30 -- arch/x86/kernel/setup.c | 4 arch/x86/mm/ioremap.c | 11 ++ 8 files changed, 130 insertions(+), 14 deletions(-) diff --git a/Documentation/x86/boot.rst b/Documentation/x86/boot.rst index 4c536bc8816d..d6d03b00b594 100644 --- a/Documentation/x86/boot.rst +++ b/Documentation/x86/boot.rst @@ -827,6 +827,46 @@ Protocol: 2.09+ sure to consider the case where the linked list already contains entries. + The 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. + + Thus setup_indirect struct and SETUP_INDIRECT type were introduced in + protocol 2.15. + + struct setup_indirect { +__u32 type; +__u32 reserved; /* Reserved, must be set to zero. */ +__u64 len; +__u64 addr; + }; + + The type member is a SETUP_INDIRECT | SETUP_* type. However, it cannot be + SETUP_INDIRECT itself since making the setup_indirect 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. + + Let's give an example how to point to SETUP_E820_EXT data using setup_indirect. + In this case setup_data and setup_indirect will look like this: + + struct setup_data { +__u64 next = 0 or ; +__u32 type = SETUP_INDIRECT; +__u32 len = sizeof(setup_data); +__u8 data[sizeof(setup_indirect)] = struct setup_indirect { + __u32 type = SETUP_INDIRECT | SETUP_E820_EXT; + __u32 reserved = 0; + __u64 len = ; + __u64 addr = ; +} + } + + Note: SETUP_INDIRECT | SETUP_NONE objects cannot be properly distinguished +from SETUP_INDIRECT itself. So, this kind of objects cannot be provided +by the bootloaders. + Field name:pref_address Type: read (reloc) diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c index 2e53c056ba20..bb9bfef174ae 100644 --- a/arch/x86/boot/compressed/kaslr.c +++ b/arch/x86/boot/compressed/kaslr.c @@ -459,6 +459,18 @@ static bool mem_avoid_overlap(struct mem_vector *img, is_overlapping = true; } + if (ptr->type == SETUP_INDIRECT && + ((struct setup_indirect *)ptr->data)->type != SETUP_INDIRECT) { + avoid.start = ((struct setup_indirect *)ptr->data)->addr; + avoid.size = ((struct setup_indirect *)ptr->data)->len; + + if (mem_overlaps(img, ) && (avoid.start < earliest)) { + *overlap = avoid; + earliest = overlap->start; + is_overlapping = true; + } + } + ptr = (struct setup_data *)(unsigned long)ptr->next; } diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h index dbb41128e5a0..949066b5398a 100644 --- a/arch/x86/include/uapi/asm/bootparam.h +++ b/arch/x86/include/uapi/asm/bootparam.h @@ -2,7 +2,7 @@ #ifndef _ASM_X86_BOOTPARAM_H #define _ASM_X86_BOOTPARAM_H -/* setup_data types */ +/* setup_data/setup_indirect types */ #define SETUP_NONE 0 #define SETUP_E820_EXT 1 #define SETUP_DTB
Re: [efi:next 1/1] arch/x86/xen/efi.c:133:35: sparse: incorrect type in argument 1 (different type sizes)
On Mon, Apr 16, 2018 at 02:07:25PM +0200, Ard Biesheuvel wrote: > > On 16 Apr 2018, at 13:40, Daniel Kiper <daniel.ki...@oracle.com> wrote: > >> On Mon, Apr 16, 2018 at 07:02:15PM +0800, kbuild test robot wrote: > >> tree: https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next > >> head: 2c603650ee23c4596f4852e43ae40e42a0c771e1 > >> commit: 2c603650ee23c4596f4852e43ae40e42a0c771e1 [1/1] x86/xen/efi: > >> Initialize UEFI secure boot state during dom0 boot > >> reproduce: > >># apt-get install sparse > >>git checkout 2c603650ee23c4596f4852e43ae40e42a0c771e1 > >>make ARCH=x86_64 allmodconfig > >>make C=1 CF=-D__CHECK_ENDIAN__ > >> > >> > >> sparse warnings: (new ones prefixed by >>) > >> > >>>> arch/x86/xen/efi.c:133:35: sparse: incorrect type in argument 1 > >>>> (different type sizes) @@expected unsigned short [usertype] *name @@ > >>>>got ype] *name @@ > >> arch/x86/xen/efi.c:133:35:expected unsigned short [usertype] *name > >> arch/x86/xen/efi.c:133:35:got char * > >> arch/x86/xen/efi.c:143:35: sparse: incorrect type in argument 1 > >> (different type sizes) @@expected unsigned short [usertype] *name @@ > >> got ype] *name @@ > >> arch/x86/xen/efi.c:143:35:expected unsigned short [usertype] *name > >> arch/x86/xen/efi.c:143:35:got char * > >> arch/x86/xen/efi.c:154:35: sparse: incorrect type in argument 1 > >> (different type sizes) @@expected unsigned short [usertype] *name @@ > >> got ype] *name @@ > >> arch/x86/xen/efi.c:154:35:expected unsigned short [usertype] *name > >> arch/x86/xen/efi.c:154:35:got char * > > > > Ugh... It looks that compiler does not like L"SecureBoot". > > I have to use this cryptic form from > > drivers/firmware/efi/libstub/secureboot.c... :-((( > > I will send the fix for this tomorrow. > > > > Please ignore: this is sparse, not gcc. You could report it with the sparse > project if you want. OK, I will ignore that. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [efi:next 1/1] arch/x86/xen/efi.c:133:35: sparse: incorrect type in argument 1 (different type sizes)
On Mon, Apr 16, 2018 at 07:02:15PM +0800, kbuild test robot wrote: > tree: https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git next > head: 2c603650ee23c4596f4852e43ae40e42a0c771e1 > commit: 2c603650ee23c4596f4852e43ae40e42a0c771e1 [1/1] x86/xen/efi: > Initialize UEFI secure boot state during dom0 boot > reproduce: > # apt-get install sparse > git checkout 2c603650ee23c4596f4852e43ae40e42a0c771e1 > make ARCH=x86_64 allmodconfig > make C=1 CF=-D__CHECK_ENDIAN__ > > > sparse warnings: (new ones prefixed by >>) > > >> arch/x86/xen/efi.c:133:35: sparse: incorrect type in argument 1 (different > >> type sizes) @@expected unsigned short [usertype] *name @@got ype] > >> *name @@ >arch/x86/xen/efi.c:133:35:expected unsigned short [usertype] *name >arch/x86/xen/efi.c:133:35:got char * >arch/x86/xen/efi.c:143:35: sparse: incorrect type in argument 1 (different > type sizes) @@expected unsigned short [usertype] *name @@got ype] > *name @@ >arch/x86/xen/efi.c:143:35:expected unsigned short [usertype] *name >arch/x86/xen/efi.c:143:35:got char * >arch/x86/xen/efi.c:154:35: sparse: incorrect type in argument 1 (different > type sizes) @@expected unsigned short [usertype] *name @@got ype] > *name @@ >arch/x86/xen/efi.c:154:35:expected unsigned short [usertype] *name >arch/x86/xen/efi.c:154:35:got char * Ugh... It looks that compiler does not like L"SecureBoot". I have to use this cryptic form from drivers/firmware/efi/libstub/secureboot.c... :-((( I will send the fix for this tomorrow. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] 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 -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On Mon, Sep 14, 2015 at 10:25:19AM +0100, Mark Rutland wrote: > On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote: > > On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote: > > > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote: > > > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: > > > > [...] > > > > > > > What's troublesome with the boot services? > > > > > > > > > > What can't be simulated? > > > > > > > > How do you want to access bare metal EFI boot services from dom0 if they > > > > were shutdown long time ago before loading dom0 image? > > > > > > I don't want to. > > > > > > I asked "What can't be simulated?" because I assumed everything > > > necessary/mandatory could be simulated without needinng access to any > > > real EFI boot services. > > > > > > As far as I can see all that's necessary is to provide a compatible > > > interface. > > > > Could you be more precise what do you need? Please enumerate. UEFI spec has > > more than 2500 pages and I do not think that we need all stuff in dom0. > > > > > > What do you need from EFI boot services in dom0? > > > > > > The ability to call ExitBootServices() and SetVirtualAddressMap() on a > > > _virtual_ address map for _virtual_ services provided by the hypervisor. > > > > I am confused. Why do you need that? Please remember, EFI is owned and > > operated by Xen hypervisor. dom0 does not have direct access to EFI. > > Let's take a step back. > > My objection here is to passing the Dom0 kernel properties as if it were > booted with direct access to a full UEFI, then later fixing that up > (when Xen is detected and we apply its hypercall EFI implementation). > > If the kernel cannot use EFI natively, why pretend to the kernel that it > can? The hypercall implementation is _not_ EFI (though it provides > access to some services). > > The two ways I can see providing Dom0 with EFI services are: > > * Have Xen create shims for any services, in which any hypercalls live, > and pass these to the kernel with a virtual system table. This keeps > the interface to the kernel the same regardless of Xen. > > * Have the kernel detect Xen EFI capability via Xen, without passing the > usual native EFI parameters. This can then be installed into the > kernel in a Xen-specific manner, and we know from the outset that > Xen-specific caveats apply. It works on x86 in that way and I suppose that it can work in that way on ARM too. So, just go and reuse existing code. That is all. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On Mon, Sep 14, 2015 at 11:43:27AM +0200, Ard Biesheuvel wrote: > On 14 September 2015 at 11:25, Mark Rutland <mark.rutl...@arm.com> wrote: > > On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote: > >> On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote: > >> > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote: > >> > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: > >> > >> [...] > >> > >> > > > What's troublesome with the boot services? > >> > > > > >> > > > What can't be simulated? > >> > > > >> > > How do you want to access bare metal EFI boot services from dom0 if > >> > > they > >> > > were shutdown long time ago before loading dom0 image? > >> > > >> > I don't want to. > >> > > >> > I asked "What can't be simulated?" because I assumed everything > >> > necessary/mandatory could be simulated without needinng access to any > >> > real EFI boot services. > >> > > >> > As far as I can see all that's necessary is to provide a compatible > >> > interface. > >> > >> Could you be more precise what do you need? Please enumerate. UEFI spec has > >> more than 2500 pages and I do not think that we need all stuff in dom0. > >> > >> > > What do you need from EFI boot services in dom0? > >> > > >> > The ability to call ExitBootServices() and SetVirtualAddressMap() on a > >> > _virtual_ address map for _virtual_ services provided by the hypervisor. > >> > >> I am confused. Why do you need that? Please remember, EFI is owned and > >> operated by Xen hypervisor. dom0 does not have direct access to EFI. > > > > Let's take a step back. > > > > My objection here is to passing the Dom0 kernel properties as if it were > > booted with direct access to a full UEFI, then later fixing that up > > (when Xen is detected and we apply its hypercall EFI implementation). > > > > To be honest, I don't think that has ever been suggested here. What > was suggested is to provide a minimal EFI like environment that allows > the post-stub EFI code to proceed and find the ACPI root pointer. > > > If the kernel cannot use EFI natively, why pretend to the kernel that it > > can? The hypercall implementation is _not_ EFI (though it provides > > access to some services). > > > > To get access to the ACPI root pointer, for which there is only one > specified way of obtaining it on ARM, which is via the UEFI > configuration table database > > > The two ways I can see providing Dom0 with EFI services are: > > > > * Have Xen create shims for any services, in which any hypercalls live, > > and pass these to the kernel with a virtual system table. This keeps > > the interface to the kernel the same regardless of Xen. > > > > * Have the kernel detect Xen EFI capability via Xen, without passing the > > usual native EFI parameters. This can then be installed into the > > kernel in a Xen-specific manner, and we know from the outset that > > Xen-specific caveats apply. > > > > As per my original email, I'm not against the renaming of the stub > > parameters if we standardise the rest of the details, but I believe > > that's orthogonal to the Xen Dom0 case. > > > > Xen will not boot the kernel via the stub, but directly. It needs to > supply a EFI like environment so that the kernel can boot via ACPI. > There is no reason whatsoever to mock up boot services or other pieces > of UEFI functionality that are not needed. The core kernel does not > call any boot services or SetVirtualAddressMap/ConvertPointer, and > there is already paravirtualized plumbing in place for the remaining > runtime services. > > Hence my claim earlier that we should cope with the runtime services > pointer being NULL, since that is really the only thing standing in I suppose that you thought about EFI_INVALID_TABLE_ADDR... > the way from the kernel side. If you feel that violates the spec in If yes then you should know that dom0 on x86 EFI platform works with efi.runtime == EFI_INVALID_TABLE_ADDR without any issue. So, I think that all problems are solved. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On Mon, Sep 14, 2015 at 03:09:34PM +0200, Ard Biesheuvel wrote: > On 14 September 2015 at 14:28, Daniel Kiper <daniel.ki...@oracle.com> wrote: > > On Mon, Sep 14, 2015 at 11:43:27AM +0200, Ard Biesheuvel wrote: > >> On 14 September 2015 at 11:25, Mark Rutland <mark.rutl...@arm.com> wrote: > >> > On Sat, Sep 12, 2015 at 12:36:55PM +0100, Daniel Kiper wrote: > >> >> On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote: > >> >> > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote: > >> >> > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: > >> >> > >> >> [...] > >> >> > >> >> > > > What's troublesome with the boot services? > >> >> > > > > >> >> > > > What can't be simulated? > >> >> > > > >> >> > > How do you want to access bare metal EFI boot services from dom0 if > >> >> > > they > >> >> > > were shutdown long time ago before loading dom0 image? > >> >> > > >> >> > I don't want to. > >> >> > > >> >> > I asked "What can't be simulated?" because I assumed everything > >> >> > necessary/mandatory could be simulated without needinng access to any > >> >> > real EFI boot services. > >> >> > > >> >> > As far as I can see all that's necessary is to provide a compatible > >> >> > interface. > >> >> > >> >> Could you be more precise what do you need? Please enumerate. UEFI spec > >> >> has > >> >> more than 2500 pages and I do not think that we need all stuff in dom0. > >> >> > >> >> > > What do you need from EFI boot services in dom0? > >> >> > > >> >> > The ability to call ExitBootServices() and SetVirtualAddressMap() on a > >> >> > _virtual_ address map for _virtual_ services provided by the > >> >> > hypervisor. > >> >> > >> >> I am confused. Why do you need that? Please remember, EFI is owned and > >> >> operated by Xen hypervisor. dom0 does not have direct access to EFI. > >> > > >> > Let's take a step back. > >> > > >> > My objection here is to passing the Dom0 kernel properties as if it were > >> > booted with direct access to a full UEFI, then later fixing that up > >> > (when Xen is detected and we apply its hypercall EFI implementation). > >> > > >> > >> To be honest, I don't think that has ever been suggested here. What > >> was suggested is to provide a minimal EFI like environment that allows > >> the post-stub EFI code to proceed and find the ACPI root pointer. > >> > >> > If the kernel cannot use EFI natively, why pretend to the kernel that it > >> > can? The hypercall implementation is _not_ EFI (though it provides > >> > access to some services). > >> > > >> > >> To get access to the ACPI root pointer, for which there is only one > >> specified way of obtaining it on ARM, which is via the UEFI > >> configuration table database > >> > >> > The two ways I can see providing Dom0 with EFI services are: > >> > > >> > * Have Xen create shims for any services, in which any hypercalls live, > >> > and pass these to the kernel with a virtual system table. This keeps > >> > the interface to the kernel the same regardless of Xen. > >> > > >> > * Have the kernel detect Xen EFI capability via Xen, without passing the > >> > usual native EFI parameters. This can then be installed into the > >> > kernel in a Xen-specific manner, and we know from the outset that > >> > Xen-specific caveats apply. > >> > > >> > As per my original email, I'm not against the renaming of the stub > >> > parameters if we standardise the rest of the details, but I believe > >> > that's orthogonal to the Xen Dom0 case. > >> > > >> > >> Xen will not boot the kernel via the stub, but directly. It needs to > >> supply a EFI like environment so that the kernel can boot via ACPI. > >> There is no reason whatsoever to mock up boot services or other pieces > >> of UEFI functionality that are not needed. The core kernel does not > >> call any boot services or SetVirtualAddres
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On Fri, Sep 11, 2015 at 05:25:59PM +0100, Mark Rutland wrote: > On Fri, Sep 11, 2015 at 01:46:43PM +0100, Daniel Kiper wrote: > > On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: [...] > > > What's troublesome with the boot services? > > > > > > What can't be simulated? > > > > How do you want to access bare metal EFI boot services from dom0 if they > > were shutdown long time ago before loading dom0 image? > > I don't want to. > > I asked "What can't be simulated?" because I assumed everything > necessary/mandatory could be simulated without needinng access to any > real EFI boot services. > > As far as I can see all that's necessary is to provide a compatible > interface. Could you be more precise what do you need? Please enumerate. UEFI spec has more than 2500 pages and I do not think that we need all stuff in dom0. > > What do you need from EFI boot services in dom0? > > The ability to call ExitBootServices() and SetVirtualAddressMap() on a > _virtual_ address map for _virtual_ services provided by the hypervisor. I am confused. Why do you need that? Please remember, EFI is owned and operated by Xen hypervisor. dom0 does not have direct access to EFI. All stuff required in dom0 is provided via hypercalls. If you need an extra data form EFI in dom0 please extend currently exiting API. Do not emulate whole EFI if you need one or a few things from spec. > A console so that I can log things early on. IIUC, log from dom0. Please use machinery provided by Xen hypervisor. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: > > > C) When you could go: > > > > > >DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI > > > discovery > > > > I take you mean discovering Xen with the usual Xen hypervisor node on > > device tree. I think that C) is a good option actually. I like it. Not > > sure why we didn't think about this earlier. Is there anything EFI or > > ACPI which is needed before Xen support is discovered by > > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()? > > Currently lots (including the memory map). With the stuff to support > SPCR, the ACPI discovery would be moved before xen_early_init(). > > > If not, we could just go for this. A lot of complexity would go away. > > I suspect this would still be fairly complex, but would at least prevent > the Xen-specific EFI handling from adversely affecting the native case. > > > > D) If you want to be generic: > > >EFI -> EFI application -> EFI tables -> ACPI tables -> Xen-specific > > > stuff > > > \--/ > > > (virtualize these, provide shims to Dom0, but handle > > > everything in Xen itself) > > > > I think that this is good in theory but could turn out to be a lot of > > work in practice. We could probably virtualize the RuntimeServices but > > the BootServices are troublesome. > > What's troublesome with the boot services? > > What can't be simulated? How do you want to access bare metal EFI boot services from dom0 if they were shutdown long time ago before loading dom0 image? What do you need from EFI boot services in dom0? Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] efi/libstub/fdt: Standardize the names of EFI stub parameters
On Fri, Sep 11, 2015 at 03:30:15PM +0200, Ard Biesheuvel wrote: > On 11 September 2015 at 15:14, Stefano Stabellini > <stefano.stabell...@eu.citrix.com> wrote: > > On Fri, 11 Sep 2015, Daniel Kiper wrote: > >> On Thu, Sep 10, 2015 at 05:23:02PM +0100, Mark Rutland wrote: > >> > > > C) When you could go: > >> > > > > >> > > >DT -> Discover Xen -> Xen-specific stuff -> Xen-specific EFI/ACPI > >> > > > discovery > >> > > > >> > > I take you mean discovering Xen with the usual Xen hypervisor node on > >> > > device tree. I think that C) is a good option actually. I like it. Not > >> > > sure why we didn't think about this earlier. Is there anything EFI or > >> > > ACPI which is needed before Xen support is discovered by > >> > > arch/arm64/kernel/setup.c:setup_arch -> xen_early_init()? > >> > > >> > Currently lots (including the memory map). With the stuff to support > >> > SPCR, the ACPI discovery would be moved before xen_early_init(). > >> > > >> > > If not, we could just go for this. A lot of complexity would go away. > >> > > >> > I suspect this would still be fairly complex, but would at least prevent > >> > the Xen-specific EFI handling from adversely affecting the native case. > >> > > >> > > > D) If you want to be generic: > >> > > >EFI -> EFI application -> EFI tables -> ACPI tables -> > >> > > > Xen-specific stuff > >> > > > \--/ > >> > > >(virtualize these, provide shims to Dom0, but handle > >> > > > everything in Xen itself) > >> > > > >> > > I think that this is good in theory but could turn out to be a lot of > >> > > work in practice. We could probably virtualize the RuntimeServices but > >> > > the BootServices are troublesome. > >> > > >> > What's troublesome with the boot services? > >> > > >> > What can't be simulated? > >> > >> How do you want to access bare metal EFI boot services from dom0 if they > >> were shutdown long time ago before loading dom0 image? What do you need > >> from EFI boot services in dom0? > > > > That's right. Trying to emulate BootServices after the real > > ExitBootServices has already been called seems like a very bad plan. > > > > I think that whatever interface we come up with, would need to be past > > ExitBootServices. > > It feels like this discussion is going in circles. > > When we discussed this six months ago, we already concluded that, > since UEFI is the only specified way that the presence of ACPI is > advertised on an ARM system, we need to emulate UEFI to some extent. > > So we need the EFI system table to expose the UEFI configuration table > that carries the ACPI root pointer. > > Since ACPI support also relies on the UEFI memory map (I think?), we > need that as well. > > These two items are exactly what we pass via the UEFI DT properties, > so we should indeed promote the current de-facto binding to a proper > binding, and renaming the properties makes sense in that context. > > I agree that this should also include a description of the expected > state of the firmware, i.e., that ExitBootServices() has been called, > and that the memory map has been populated with virtual address, which > have been installed using SetVirtualAddressMap() if they differ from > the physical addresses. (The current implementation on the kernel side > is perfectly capable of dealing with a 1:1 mapping). > > Beyond that, there is no point in pretending to be a full UEFI > implementation, imo. Boot services are not required, nor are runtime > services (only the current EFI init code on arm needs to be modified > to deal with a NULL runtime services pointer) Taking into account above I think that you have most of the code in place. Please take a look at linux/arch/x86/xen/efi.c, linux/drivers/acpi/osl.c and linux/drivers/xen/efi.c (maybe somewhere else). In general you should create ARM version of xen_efi_init() (x86 version you can find in linux/drivers/xen/efi.c; it is very simple thing), maybe add some code in a few places and voila. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 00/10] xen: Add EFI support
On Mon, Jul 07, 2014 at 08:49:14PM +0100, Matt Fleming wrote: On Mon, 30 Jun, at 07:52:54PM, Daniel Kiper wrote: Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. Daniel arch/ia64/include/asm/io.h |1 + arch/x86/kernel/setup.c |4 +- arch/x86/platform/efi/efi.c | 106 ++-- arch/x86/xen/enlighten.c | 15 drivers/firmware/efi/efi.c | 25 +++--- drivers/xen/Kconfig |4 + drivers/xen/Makefile |3 + drivers/xen/efi.c| 368 +++ include/linux/efi.h |4 +- include/xen/interface/platform.h | 123 include/xen/xen-ops.h| 11 +++ 11 files changed, 587 insertions(+), 77 deletions(-) Daniel Kiper (10): arch/ia64: Define early_memunmap() efi: Use early_mem*() instead of early_io*() arch/x86: Do not access EFI memory map if it is not available efi: Introduce EFI_PARAVIRT flag arch/x86: Remove redundant set_bit(EFI_SYSTEM_TABLES) call arch/x86: Remove redundant set_bit(EFI_MEMMAP) call xen: Define EFI related stuff xen: Put EFI machinery in place arch/x86: Replace plain strings with constants arch/x86: Remove efi_set_rtc_mmss() Thanks Daniel. I've included Stefano's Acks where they were given and queued this up for v3.17 in the EFI 'next' branch at, git://git.kernel.org/pub/scm/linux/kernel/git/mfleming/efi.git Great! Thanks a lot. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 00/10] xen: Add EFI support
On Tue, Jul 01, 2014 at 02:13:10PM +0100, David Vrabel wrote: On 30/06/14 18:52, Daniel Kiper wrote: Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. What's changed in this version? I have added arch/ia64: Define early_memunmap() patch, moved xen_efi_probe() declaration to include/xen/xen-ops.h as Stefano asked, added yours Reviewed-by to Xen stuff and did more tests. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 05/10] arch/x86: Remove redundant set_bit(EFI_SYSTEM_TABLES) call
Remove redundant set_bit(EFI_SYSTEM_TABLES, efi.flags) call. It is executed earlier in efi_systab_init(). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c |2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index b9c23d7..ae3d398 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -744,8 +744,6 @@ void __init efi_init(void) if (efi_systab_init(efi_phys.systab)) return; - set_bit(EFI_SYSTEM_TABLES, efi.flags); - efi.config_table = (unsigned long)efi.systab-tables; efi.fw_vendor= (unsigned long)efi.systab-fw_vendor; efi.runtime = (unsigned long)efi.systab-runtime; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 07/10] xen: Define EFI related stuff
Define constants and structures which are needed to properly execute EFI related hypercall in Xen dom0. This patch is based on Jan Beulich and Tang Liang work. Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com Reviewed-by: David Vrabel david.vra...@citrix.com --- v6 - suggestions/fixes: - fix indentation (suggested by David Vrabel). v5 - suggestions/fixes: - improve commit message (suggested by David Vrabel). v4 - suggestions/fixes: - change some types from generic to Xen specific ones (suggested by Stefano Stabellini), - do some formating changes (suggested by Jan Beulich). --- include/xen/interface/platform.h | 123 ++ 1 file changed, 123 insertions(+) diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h index f1331e3..5cc49ea 100644 --- a/include/xen/interface/platform.h +++ b/include/xen/interface/platform.h @@ -108,11 +108,113 @@ struct xenpf_platform_quirk { }; DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t); +#define XENPF_efi_runtime_call49 +#define XEN_EFI_get_time 1 +#define XEN_EFI_set_time 2 +#define XEN_EFI_get_wakeup_time 3 +#define XEN_EFI_set_wakeup_time 4 +#define XEN_EFI_get_next_high_monotonic_count 5 +#define XEN_EFI_get_variable 6 +#define XEN_EFI_set_variable 7 +#define XEN_EFI_get_next_variable_name8 +#define XEN_EFI_query_variable_info 9 +#define XEN_EFI_query_capsule_capabilities 10 +#define XEN_EFI_update_capsule 11 + +struct xenpf_efi_runtime_call { + uint32_t function; + /* +* This field is generally used for per sub-function flags (defined +* below), except for the XEN_EFI_get_next_high_monotonic_count case, +* where it holds the single returned value. +*/ + uint32_t misc; + xen_ulong_t status; + union { +#define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x0001 + struct { + struct xenpf_efi_time { + uint16_t year; + uint8_t month; + uint8_t day; + uint8_t hour; + uint8_t min; + uint8_t sec; + uint32_t ns; + int16_t tz; + uint8_t daylight; + } time; + uint32_t resolution; + uint32_t accuracy; + } get_time; + + struct xenpf_efi_time set_time; + +#define XEN_EFI_GET_WAKEUP_TIME_ENABLED 0x0001 +#define XEN_EFI_GET_WAKEUP_TIME_PENDING 0x0002 + struct xenpf_efi_time get_wakeup_time; + +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE 0x0001 +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY 0x0002 + struct xenpf_efi_time set_wakeup_time; + +#define XEN_EFI_VARIABLE_NON_VOLATILE 0x0001 +#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002 +#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x0004 + struct { + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + xen_ulong_t size; + GUEST_HANDLE(void) data; + struct xenpf_efi_guid { + uint32_t data1; + uint16_t data2; + uint16_t data3; + uint8_t data4[8]; + } vendor_guid; + } get_variable, set_variable; + + struct { + xen_ulong_t size; + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + struct xenpf_efi_guid vendor_guid; + } get_next_variable_name; + + struct { + uint32_t attr; + uint64_t max_store_size; + uint64_t remain_store_size; + uint64_t max_size; + } query_variable_info; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t max_capsule_size; + uint32_t reset_type; + } query_capsule_capabilities; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t sg_list; /* machine address */ + } update_capsule; + } u; +}; +DEFINE_GUEST_HANDLE_STRUCT(xenpf_efi_runtime_call); + +#define XEN_FW_EFI_VERSION0 +#define XEN_FW_EFI_CONFIG_TABLE 1
[PATCH v7 10/10] arch/x86: Remove efi_set_rtc_mmss()
efi_set_rtc_mmss() is never used to set RTC due to bugs found on many EFI platforms. It is set directly by mach_set_rtc_mmss(). Hence, remove unused efi_set_rtc_mmss() function. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - remove efi_set_rtc_mmss() instead of commenting out it (suggested by Stefano Stabellini, Juergen Gross, H. Peter Anvin and Matt Fleming). --- arch/x86/platform/efi/efi.c | 36 include/linux/efi.h |1 - 2 files changed, 37 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index da15df9..d569eea 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -244,42 +244,6 @@ static efi_status_t __init phys_efi_set_virtual_address_map( return status; } -int efi_set_rtc_mmss(const struct timespec *now) -{ - unsigned long nowtime = now-tv_sec; - efi_status_tstatus; - efi_time_t eft; - efi_time_cap_t cap; - struct rtc_time tm; - - status = efi.get_time(eft, cap); - if (status != EFI_SUCCESS) { - pr_err(Oops: efitime: can't read time!\n); - return -1; - } - - rtc_time_to_tm(nowtime, tm); - if (!rtc_valid_tm(tm)) { - eft.year = tm.tm_year + 1900; - eft.month = tm.tm_mon + 1; - eft.day = tm.tm_mday; - eft.minute = tm.tm_min; - eft.second = tm.tm_sec; - eft.nanosecond = 0; - } else { - pr_err(%s: Invalid EFI RTC value: write of %lx to EFI RTC failed\n, - __func__, nowtime); - return -1; - } - - status = efi.set_time(eft); - if (status != EFI_SUCCESS) { - pr_err(Oops: efitime: can't write time!\n); - return -1; - } - return 0; -} - void efi_get_time(struct timespec *now) { efi_status_t status; diff --git a/include/linux/efi.h b/include/linux/efi.h index 713a4f1..322366b 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -870,7 +870,6 @@ extern int __init efi_uart_console_only (void); extern void efi_initialize_iomem_resources(struct resource *code_resource, struct resource *data_resource, struct resource *bss_resource); extern void efi_get_time(struct timespec *now); -extern int efi_set_rtc_mmss(const struct timespec *now); extern void efi_reserve_boot_services(void); extern int efi_get_fdt_params(struct efi_fdt_params *params, int verbose); extern struct efi_memory_map memmap; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 08/10] xen: Put EFI machinery in place
This patch enables EFI usage under Xen dom0. Standard EFI Linux Kernel infrastructure cannot be used because it requires direct access to EFI data and code. However, in dom0 case it is not possible because above mentioned EFI stuff is fully owned and controlled by Xen hypervisor. In this case all calls from dom0 to EFI must be requested via special hypercall which in turn executes relevant EFI code in behalf of dom0. When dom0 kernel boots it checks for EFI availability on a machine. If it is detected then artificial EFI system table is filled. Native EFI callas are replaced by functions which mimics them by calling relevant hypercall. Later pointer to EFI system table is passed to standard EFI machinery and it continues EFI subsystem initialization taking into account that there is no direct access to EFI boot services, runtime, tables, structures, etc. After that system runs as usual. This patch is based on Jan Beulich and Tang Liang work. Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com Reviewed-by: David Vrabel david.vra...@citrix.com --- v7 - suggestions/fixes: - move xen_efi_probe() declaration to include/xen/xen-ops.h (suggested by Stefano Stabellini). v6 - suggestions/fixes: - remove unneeded BUG() call (suggested by Stefano Stabellini). v5 - suggestions/fixes: - improve macro usage readability (suggested by Andrew Cooper and David Vrabel), - conditions cleanup (suggested by David Vrabel), - use -fshort-wchar option (suggested by Jan Beulich), - Kconfig rule cleanup (suggested by Jan Beulich), - forward port fixes from SUSE kernel (suggested by Jan Beulich), - improve commit message (suggested by David Vrabel). v4 - suggestions/fixes: - just populate an efi_system_table_t object (suggested by Matt Fleming). --- arch/x86/xen/enlighten.c | 15 ++ drivers/xen/Kconfig |4 + drivers/xen/Makefile |3 + drivers/xen/efi.c| 368 ++ include/xen/xen-ops.h| 11 ++ 5 files changed, 401 insertions(+) create mode 100644 drivers/xen/efi.c diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index ffb101e..e27d2fe 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -32,6 +32,7 @@ #include linux/gfp.h #include linux/memblock.h #include linux/edd.h +#include linux/efi.h #include xen/xen.h #include xen/events.h @@ -1520,6 +1521,7 @@ asmlinkage __visible void __init xen_start_kernel(void) { struct physdev_set_iopl set_iopl; int rc; + efi_system_table_t *efi_systab_xen; if (!xen_start_info) return; @@ -1718,6 +1720,19 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) 32); + + set_bit(EFI_BOOT, efi.flags); + set_bit(EFI_PARAVIRT, efi.flags); + set_bit(EFI_64BIT, efi.flags); + } + /* Start the world */ #ifdef CONFIG_X86_32 i386_start_kernel(); diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 38fb36e..8bc0183 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -240,4 +240,8 @@ config XEN_MCE_LOG config XEN_HAVE_PVMMU bool +config XEN_EFI + def_bool y + depends on X86_64 EFI + endmenu diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 45e00af..84044b5 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -9,6 +9,8 @@ obj-y += xenbus/ nostackp := $(call cc-option, -fno-stack-protector) CFLAGS_features.o := $(nostackp) +CFLAGS_efi.o += -fshort-wchar + dom0-$(CONFIG_PCI) += pci.o dom0-$(CONFIG_USB_SUPPORT) += dbgp.o dom0-$(CONFIG_ACPI) += acpi.o $(xen-pad-y) @@ -33,6 +35,7 @@ obj-$(CONFIG_XEN_STUB)+= xen-stub.o obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY) += xen-acpi-memhotplug.o obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU) += xen-acpi-cpuhotplug.o obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o +obj-$(CONFIG_XEN_EFI) += efi.o xen-evtchn-y := evtchn.o xen-gntdev-y := gntdev.o xen-gntalloc-y := gntalloc.o diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c new file mode 100644 index 000..31f618a --- /dev/null +++ b/drivers/xen/efi.c @@ -0,0 +1,368 @@ +/* + * EFI support for Xen. + * + * Copyright (C) 1999 VA Linux Systems
[PATCH v7 09/10] arch/x86: Replace plain strings with constants
We've got constants, so let's use them instead of hard-coded values. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - improve commit message (suggested by Matt Fleming). --- arch/x86/kernel/setup.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 78a0e62..41ead8d 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -924,10 +924,10 @@ void __init setup_arch(char **cmdline_p) #endif #ifdef CONFIG_EFI if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL32, 4)) { +EFI32_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); } else if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL64, 4)) { +EFI64_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); set_bit(EFI_64BIT, efi.flags); } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 06/10] arch/x86: Remove redundant set_bit(EFI_MEMMAP) call
Remove redundant set_bit(EFI_MEMMAP, efi.flags) call. It is executed earlier in efi_memmap_init(). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c |2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index ae3d398..da15df9 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -784,8 +784,6 @@ void __init efi_init(void) if (efi_memmap_init()) return; - set_bit(EFI_MEMMAP, efi.flags); - print_efi_memmap(); } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 03/10] arch/x86: Do not access EFI memory map if it is not available
Do not access EFI memory map if it is not available. At least Xen dom0 EFI implementation does not have an access to it. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - create this separate patch from main EFI_PARAVIRT patch (suggested by Matt Fleming). --- arch/x86/platform/efi/efi.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 0ee1f46..d2d3c41 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -1220,6 +1220,9 @@ u64 efi_mem_attributes(unsigned long phys_addr) efi_memory_desc_t *md; void *p; + if (!efi_enabled(EFI_MEMMAP)) + return 0; + for (p = memmap.map; p memmap.map_end; p += memmap.desc_size) { md = p; if ((md-phys_addr = phys_addr) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 00/10] xen: Add EFI support
Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. Daniel arch/ia64/include/asm/io.h |1 + arch/x86/kernel/setup.c |4 +- arch/x86/platform/efi/efi.c | 106 ++-- arch/x86/xen/enlighten.c | 15 drivers/firmware/efi/efi.c | 25 +++--- drivers/xen/Kconfig |4 + drivers/xen/Makefile |3 + drivers/xen/efi.c| 368 +++ include/linux/efi.h |4 +- include/xen/interface/platform.h | 123 include/xen/xen-ops.h| 11 +++ 11 files changed, 587 insertions(+), 77 deletions(-) Daniel Kiper (10): arch/ia64: Define early_memunmap() efi: Use early_mem*() instead of early_io*() arch/x86: Do not access EFI memory map if it is not available efi: Introduce EFI_PARAVIRT flag arch/x86: Remove redundant set_bit(EFI_SYSTEM_TABLES) call arch/x86: Remove redundant set_bit(EFI_MEMMAP) call xen: Define EFI related stuff xen: Put EFI machinery in place arch/x86: Replace plain strings with constants arch/x86: Remove efi_set_rtc_mmss() -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 01/10] arch/ia64: Define early_memunmap()
This is odd to use early_iounmap() function do tear down mapping created by early_memremap() function, even if it works right now, because they belong to different set of functions. The former is I/O related function and the later is memory related. So, create early_memunmap() macro which in real is early_iounmap(). This thing will help to not confuse code readers longer by mixing functions from different classes. EFI patches following this patch uses that functionality. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/ia64/include/asm/io.h |1 + 1 file changed, 1 insertion(+) diff --git a/arch/ia64/include/asm/io.h b/arch/ia64/include/asm/io.h index 0d2bcb3..bee0acd 100644 --- a/arch/ia64/include/asm/io.h +++ b/arch/ia64/include/asm/io.h @@ -426,6 +426,7 @@ extern void iounmap (volatile void __iomem *addr); extern void __iomem * early_ioremap (unsigned long phys_addr, unsigned long size); #define early_memremap(phys_addr, size)early_ioremap(phys_addr, size) extern void early_iounmap (volatile void __iomem *addr, unsigned long size); +#define early_memunmap(addr, size) early_iounmap(addr, size) static inline void __iomem * ioremap_cache (unsigned long phys_addr, unsigned long size) { return ioremap(phys_addr, size); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 04/10] efi: Introduce EFI_PARAVIRT flag
Introduce EFI_PARAVIRT flag. If it is set then kernel runs on EFI platform but it has not direct control on EFI stuff like EFI runtime, tables, structures, etc. If not this means that Linux Kernel has direct access to EFI infrastructure and everything runs as usual. This functionality is used in Xen dom0 because hypervisor has full control on EFI stuff and all calls from dom0 to EFI must be requested via special hypercall which in turn executes relevant EFI code in behalf of dom0. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - rename EFI_NO_DIRECT to EFI_PARAVIRT (suggested by David Vrabel), - improve code comments (suggested by Matt Fleming). v5 - suggestions/fixes: - rename EFI_DIRECT to EFI_NO_DIRECT (suggested by David Vrabel), - limit EFI_NO_DIRECT usage (suggested by Jan Beulich and Matt Fleming), - improve commit message (suggested by David Vrabel). --- arch/x86/platform/efi/efi.c | 31 +-- drivers/firmware/efi/efi.c | 21 - include/linux/efi.h |3 ++- 3 files changed, 39 insertions(+), 16 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index d2d3c41..b9c23d7 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -350,6 +350,9 @@ int __init efi_memblock_x86_reserve_range(void) struct efi_info *e = boot_params.efi_info; unsigned long pmap; + if (efi_enabled(EFI_PARAVIRT)) + return 0; + #ifdef CONFIG_X86_32 /* Can't handle data above 4GB at this time */ if (e-efi_memmap_hi) { @@ -616,14 +619,24 @@ static int __init efi_runtime_init(void) * the runtime services table so that we can grab the physical * address of several of the EFI runtime functions, needed to * set the firmware into virtual mode. +* +* When EFI_PARAVIRT is in force then we could not map runtime +* service memory region because we do not have direct access to it. +* However, runtime services are available through proxy functions +* (e.g. in case of Xen dom0 EFI implementation they call special +* hypercall which executes relevant EFI functions) and that is why +* they are always enabled. */ - if (efi_enabled(EFI_64BIT)) - rv = efi_runtime_init64(); - else - rv = efi_runtime_init32(); - if (rv) - return rv; + if (!efi_enabled(EFI_PARAVIRT)) { + if (efi_enabled(EFI_64BIT)) + rv = efi_runtime_init64(); + else + rv = efi_runtime_init32(); + + if (rv) + return rv; + } set_bit(EFI_RUNTIME_SERVICES, efi.flags); @@ -632,6 +645,9 @@ static int __init efi_runtime_init(void) static int __init efi_memmap_init(void) { + if (efi_enabled(EFI_PARAVIRT)) + return 0; + /* Map the EFI memory map */ memmap.map = early_memremap((unsigned long)memmap.phys_map, memmap.nr_map * memmap.desc_size); @@ -1188,6 +1204,9 @@ static void __init __efi_enter_virtual_mode(void) void __init efi_enter_virtual_mode(void) { + if (efi_enabled(EFI_PARAVIRT)) + return; + if (efi_setup) kexec_enter_virtual_mode(); else diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 4fd17ca..07329d1 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -104,16 +104,19 @@ static struct attribute *efi_subsys_attrs[] = { static umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n) { - umode_t mode = attr-mode; - - if (attr == efi_attr_fw_vendor.attr) - return (efi.fw_vendor == EFI_INVALID_TABLE_ADDR) ? 0 : mode; - else if (attr == efi_attr_runtime.attr) - return (efi.runtime == EFI_INVALID_TABLE_ADDR) ? 0 : mode; - else if (attr == efi_attr_config_table.attr) - return (efi.config_table == EFI_INVALID_TABLE_ADDR) ? 0 : mode; + if (attr == efi_attr_fw_vendor.attr) { + if (efi_enabled(EFI_PARAVIRT) || + efi.fw_vendor == EFI_INVALID_TABLE_ADDR) + return 0; + } else if (attr == efi_attr_runtime.attr) { + if (efi.runtime == EFI_INVALID_TABLE_ADDR) + return 0; + } else if (attr == efi_attr_config_table.attr) { + if (efi.config_table == EFI_INVALID_TABLE_ADDR) + return 0; + } - return mode; + return attr-mode; } static struct attribute_group efi_subsys_attr_group = { diff --git a/include/linux/efi.h b/include/linux/efi.h index 41bbf8b..713a4f1 100644 --- a/include/linux/efi.h +++ b/include
[PATCH v7 02/10] efi: Use early_mem*() instead of early_io*()
Use early_mem*() instead of early_io*() because all mapped EFI regions are memory (usually RAM but they could also be ROM, EPROM, EEPROM, flash, etc.) not I/O regions. Additionally, I/O family calls do not work correctly under Xen in our case. early_ioremap() skips the PFN to MFN conversion when building the PTE. Using it for memory will attempt to map the wrong machine frame. However, all artificial EFI structures created under Xen live in dom0 memory and should be mapped/unmapped using early_mem*() family calls which map domain memory. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - improve commit message (suggested by Matthew Garrett and Matt Fleming), - do not change indentation (suggested by Matt Fleming). --- arch/x86/platform/efi/efi.c | 32 drivers/firmware/efi/efi.c |4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 87fc96b..0ee1f46 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void) { clear_bit(EFI_MEMMAP, efi.flags); if (memmap.map) { - early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size); + early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size); memmap.map = NULL; } } @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys) if (!data) return -ENOMEM; } - systab64 = early_ioremap((unsigned long)phys, + systab64 = early_memremap((unsigned long)phys, sizeof(*systab64)); if (systab64 == NULL) { pr_err(Couldn't map the system table!\n); if (data) - early_iounmap(data, sizeof(*data)); + early_memunmap(data, sizeof(*data)); return -ENOMEM; } @@ -504,9 +504,9 @@ static int __init efi_systab_init(void *phys) systab64-tables; tmp |= data ? data-tables : systab64-tables; - early_iounmap(systab64, sizeof(*systab64)); + early_memunmap(systab64, sizeof(*systab64)); if (data) - early_iounmap(data, sizeof(*data)); + early_memunmap(data, sizeof(*data)); #ifdef CONFIG_X86_32 if (tmp 32) { pr_err(EFI data located above 4GB, disabling EFI.\n); @@ -516,7 +516,7 @@ static int __init efi_systab_init(void *phys) } else { efi_system_table_32_t *systab32; - systab32 = early_ioremap((unsigned long)phys, + systab32 = early_memremap((unsigned long)phys, sizeof(*systab32)); if (systab32 == NULL) { pr_err(Couldn't map the system table!\n); @@ -537,7 +537,7 @@ static int __init efi_systab_init(void *phys) efi_systab.nr_tables = systab32-nr_tables; efi_systab.tables = systab32-tables; - early_iounmap(systab32, sizeof(*systab32)); + early_memunmap(systab32, sizeof(*systab32)); } efi.systab = efi_systab; @@ -563,7 +563,7 @@ static int __init efi_runtime_init32(void) { efi_runtime_services_32_t *runtime; - runtime = early_ioremap((unsigned long)efi.systab-runtime, + runtime = early_memremap((unsigned long)efi.systab-runtime, sizeof(efi_runtime_services_32_t)); if (!runtime) { pr_err(Could not map the runtime service table!\n); @@ -578,7 +578,7 @@ static int __init efi_runtime_init32(void) efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) (unsigned long)runtime-set_virtual_address_map; - early_iounmap(runtime, sizeof(efi_runtime_services_32_t)); + early_memunmap(runtime, sizeof(efi_runtime_services_32_t)); return 0; } @@ -587,7 +587,7 @@ static int __init efi_runtime_init64(void) { efi_runtime_services_64_t *runtime; - runtime = early_ioremap((unsigned long)efi.systab-runtime, + runtime = early_memremap((unsigned long)efi.systab-runtime, sizeof(efi_runtime_services_64_t)); if (!runtime) { pr_err(Could not map the runtime service table!\n); @@ -602,7 +602,7 @@ static int __init efi_runtime_init64(void) efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) (unsigned long)runtime-set_virtual_address_map; - early_iounmap(runtime, sizeof(efi_runtime_services_64_t
Re: [PATCH v6 1/9] efi: Use early_mem*() instead of early_io*()
I am CC'ing IA-64 guys. On Mon, Jun 23, 2014 at 08:19:00AM +0100, Jan Beulich wrote: On 20.06.14 at 23:29, daniel.ki...@oracle.com wrote: --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -298,7 +298,7 @@ int __init efi_config_init(efi_config_table_type_t *arch_tables) if (table64 32) { pr_cont(\n); pr_err(Table located above 4GB, disabling EFI.\n); - early_iounmap(config_tables, + early_memunmap(config_tables, efi.systab-nr_tables * sz); return -EINVAL; } @@ -314,7 +314,7 @@ int __init efi_config_init(efi_config_table_type_t *arch_tables) tablep += sz; } pr_cont(\n); - early_iounmap(config_tables, efi.systab-nr_tables * sz); + early_memunmap(config_tables, efi.systab-nr_tables * sz); set_bit(EFI_CONFIG_TABLES, efi.flags); If these two changes are really deemed necessary (there's the implied assumption currently in place that early_iounmap() can undo early_memremap() mappings), then ia64 will need a definition added for early_memunmap() or its build will break. I know that early_memunmap() == early_iounmap() in general. However, I think that it is less confusing if use relevant functions in pairs (i.e. early_memremap() with early_memunmap(), ...) than mix them up. We have following choices here: - leave early_iounmap() as is in drivers/firmware/efi/efi.c (arch/x86/platform/efi/efi.c:early_iounmap() - early_memunmap() changes should be left as is), - include asm/early_ioremap.h in arch/ia64/include/asm/io.h (as I can see the same think is done for x86 and arm64). I prefer second solution but I do not insist. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Xen-devel] [PATCH v6 7/9] xen: Put EFI machinery in place
On Mon, Jun 23, 2014 at 10:57:31AM +0100, David Vrabel wrote: On 20/06/14 22:29, Daniel Kiper wrote: This patch enables EFI usage under Xen dom0. Standard EFI Linux Kernel infrastructure cannot be used because it requires direct access to EFI data and code. However, in dom0 case it is not possible because above mentioned EFI stuff is fully owned and controlled by Xen hypervisor. In this case all calls from dom0 to EFI must be requested via special hypercall which in turn executes relevant EFI code in behalf of dom0. When dom0 kernel boots it checks for EFI availability on a machine. If it is detected then artificial EFI system table is filled. Native EFI callas are replaced by functions which mimics them by calling relevant hypercall. Later pointer to EFI system table is passed to standard EFI machinery and it continues EFI subsystem initialization taking into account that there is no direct access to EFI boot services, runtime, tables, structures, etc. After that system runs as usual. Reviewed-by: David Vrabel david.vra...@citrix.com Thanks for this and second one. (With or without the change suggested by Stefano). I am going to take into account Stefano's idea. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 2/9] arch/x86: Do not access EFI memory map if it is not available
On Mon, Jun 23, 2014 at 12:00:01PM +0100, Jan Beulich wrote: On 23.06.14 at 11:53, david.vra...@citrix.com wrote: On 20/06/14 22:29, Daniel Kiper wrote: Do not access EFI memory map if it is not available. At least Xen dom0 EFI implementation does not have an access to it. Could it make one based on the XENMEM_memory_map or XENMEM_machine_memory_map hypercall? No, the correct operation to implement this and efi_mem_type() similar function is XEN_FW_EFI_INFO, index XEN_FW_EFI_MEM_INFO. efi_mem_attributes() is used only on IA-64 arch. So, that is why I completely removed relevant Xen code. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag
On Thu, Jun 19, 2014 at 03:41:12PM +0100, Matt Fleming wrote: On Wed, 18 Jun, at 06:48:35PM, Daniel Kiper wrote: Why don't you want to export efi.fw_vendor, etc? Rationale please. I am exporting real addresses (machine addresses) of things which I am able to get. Stuff which was created artificially and lives in dom0 address space or does not exist are not exported. So, wouldn't it be easier to just leave those fields as EFI_INVALID_TABLE_ADDR? If they're not usable why fill out an address? It looks that EFI_PARAVIRT check is needed for efi.fw_vendor only. Probably I prepared this chunk of code before filling efi_systab_xen.runtime with EFI_INVALID_TABLE_ADDR. efi.fw_vendor contains pointer to vendor name which lives in dom0 memory and it is a copy of name living in EFI memory data. So, this pointer is useless for potential user of /sys/firmware/efi/fw_vendor. efi.fw_vendor is used in init code only. Hence, I was thinking once about wiping this pointer with EFI_INVALID_TABLE_ADDR. However, it requires some changes in arch/x86/platform/efi/efi.c (wipe it if EFI_PARAVIRT is set) and it is a hack for me. I prefer to live with current solution. Hmmm... I do not know what is wrong with this minimal shuffling. We are playing here with internal stuff which is not visible outside of any given kernel. Additionally, as I saw in a few places arch bits are defined in following way: #define ARCH_1 10 #define A_ARCH_CONST ARCH_1 #define B_ARCH_CONST (ARCH_1 + 1) #define C_ARCH_CONST (ARCH_1 + 2) ... So I think addition is more natural here than subtraction. No, because we hit the same problem if we need more non-arch bits after bit 9, it moves the problem, it doesn't fix it. Though admittedly, using 10 is just an example. It could be an arbitrary value. However, you are right. This just moves the problem. this level of indirection (going through ARCH_1) instead of using constants does reduce the problem. Yes, these bits are internal, and yes the shuffling is minimal, but we can do better. As always, it is a place for an improvement. However, I am not sure it is worth fighting with that so hard. I'm not suggesting you need to modify your patch. I'm really just thinking out loud. I'll take care of fixing this up later. OK. Thanks. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/9] xen: Add EFI support
Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. Daniel arch/x86/kernel/setup.c |4 +- arch/x86/platform/efi/efi.c | 106 ++-- arch/x86/xen/enlighten.c | 24 ++ drivers/firmware/efi/efi.c | 25 +++--- drivers/xen/Kconfig |4 + drivers/xen/Makefile |3 + drivers/xen/efi.c| 367 +++ include/linux/efi.h |4 +- include/xen/interface/platform.h | 123 9 files changed, 583 insertions(+), 77 deletions(-) Daniel Kiper (9): efi: Use early_mem*() instead of early_io*() arch/x86: Do not access EFI memory map if it is not available efi: Introduce EFI_PARAVIRT flag arch/x86: Remove redundant set_bit(EFI_SYSTEM_TABLES) call arch/x86: Remove redundant set_bit(EFI_MEMMAP) call xen: Define EFI related stuff xen: Put EFI machinery in place arch/x86: Replace plain strings with constants arch/x86: Remove efi_set_rtc_mmss() -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 7/9] xen: Put EFI machinery in place
This patch enables EFI usage under Xen dom0. Standard EFI Linux Kernel infrastructure cannot be used because it requires direct access to EFI data and code. However, in dom0 case it is not possible because above mentioned EFI stuff is fully owned and controlled by Xen hypervisor. In this case all calls from dom0 to EFI must be requested via special hypercall which in turn executes relevant EFI code in behalf of dom0. When dom0 kernel boots it checks for EFI availability on a machine. If it is detected then artificial EFI system table is filled. Native EFI callas are replaced by functions which mimics them by calling relevant hypercall. Later pointer to EFI system table is passed to standard EFI machinery and it continues EFI subsystem initialization taking into account that there is no direct access to EFI boot services, runtime, tables, structures, etc. After that system runs as usual. This patch is based on Jan Beulich and Tang Liang work. Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - remove unneeded BUG() call (suggested by Stefano Stabellini). v5 - suggestions/fixes: - improve macro usage readability (suggested by Andrew Cooper and David Vrabel), - conditions cleanup (suggested by David Vrabel), - use -fshort-wchar option (suggested by Jan Beulich), - Kconfig rule cleanup (suggested by Jan Beulich), - forward port fixes from SUSE kernel (suggested by Jan Beulich), - improve commit message (suggested by David Vrabel). v4 - suggestions/fixes: - just populate an efi_system_table_t object (suggested by Matt Fleming). --- arch/x86/xen/enlighten.c | 24 +++ drivers/xen/Kconfig |4 + drivers/xen/Makefile |3 + drivers/xen/efi.c| 367 ++ 4 files changed, 398 insertions(+) create mode 100644 drivers/xen/efi.c diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index ffb101e..04611ea 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -32,6 +32,7 @@ #include linux/gfp.h #include linux/memblock.h #include linux/edd.h +#include linux/efi.h #include xen/xen.h #include xen/events.h @@ -150,6 +151,15 @@ struct shared_info *HYPERVISOR_shared_info = xen_dummy_shared_info; */ static int have_vcpu_info_placement = 1; +#ifdef CONFIG_XEN_EFI +extern efi_system_table_t __init *xen_efi_probe(void); +#else +static efi_system_table_t __init *xen_efi_probe(void) +{ + return NULL; +} +#endif + struct tls_descs { struct desc_struct desc[3]; }; @@ -1520,6 +1530,7 @@ asmlinkage __visible void __init xen_start_kernel(void) { struct physdev_set_iopl set_iopl; int rc; + efi_system_table_t *efi_systab_xen; if (!xen_start_info) return; @@ -1718,6 +1729,19 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)__pa(efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)(__pa(efi_systab_xen) 32); + + set_bit(EFI_BOOT, efi.flags); + set_bit(EFI_PARAVIRT, efi.flags); + set_bit(EFI_64BIT, efi.flags); + } + /* Start the world */ #ifdef CONFIG_X86_32 i386_start_kernel(); diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 38fb36e..8bc0183 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -240,4 +240,8 @@ config XEN_MCE_LOG config XEN_HAVE_PVMMU bool +config XEN_EFI + def_bool y + depends on X86_64 EFI + endmenu diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 45e00af..84044b5 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -9,6 +9,8 @@ obj-y += xenbus/ nostackp := $(call cc-option, -fno-stack-protector) CFLAGS_features.o := $(nostackp) +CFLAGS_efi.o += -fshort-wchar + dom0-$(CONFIG_PCI) += pci.o dom0-$(CONFIG_USB_SUPPORT) += dbgp.o dom0-$(CONFIG_ACPI) += acpi.o $(xen-pad-y) @@ -33,6 +35,7 @@ obj-$(CONFIG_XEN_STUB)+= xen-stub.o obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY) += xen-acpi-memhotplug.o obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU) += xen-acpi-cpuhotplug.o obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o +obj-$(CONFIG_XEN_EFI) += efi.o xen-evtchn-y := evtchn.o xen-gntdev-y := gntdev.o xen-gntalloc-y := gntalloc.o diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c new file mode 100644
[PATCH v6 8/9] arch/x86: Replace plain strings with constants
We've got constants, so let's use them instead of hard-coded values. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - improve commit message (suggested by Matt Fleming). --- arch/x86/kernel/setup.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 78a0e62..41ead8d 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -924,10 +924,10 @@ void __init setup_arch(char **cmdline_p) #endif #ifdef CONFIG_EFI if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL32, 4)) { +EFI32_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); } else if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL64, 4)) { +EFI64_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); set_bit(EFI_64BIT, efi.flags); } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 9/9] arch/x86: Remove efi_set_rtc_mmss()
efi_set_rtc_mmss() is never used to set RTC due to bugs found on many EFI platforms. It is set directly by mach_set_rtc_mmss(). Hence, remove unused efi_set_rtc_mmss() function. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - remove efi_set_rtc_mmss() instead of commenting out it (suggested by Stefano Stabellini, Juergen Gross, H. Peter Anvin and Matt Fleming). --- arch/x86/platform/efi/efi.c | 36 include/linux/efi.h |1 - 2 files changed, 37 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index da15df9..d569eea 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -244,42 +244,6 @@ static efi_status_t __init phys_efi_set_virtual_address_map( return status; } -int efi_set_rtc_mmss(const struct timespec *now) -{ - unsigned long nowtime = now-tv_sec; - efi_status_tstatus; - efi_time_t eft; - efi_time_cap_t cap; - struct rtc_time tm; - - status = efi.get_time(eft, cap); - if (status != EFI_SUCCESS) { - pr_err(Oops: efitime: can't read time!\n); - return -1; - } - - rtc_time_to_tm(nowtime, tm); - if (!rtc_valid_tm(tm)) { - eft.year = tm.tm_year + 1900; - eft.month = tm.tm_mon + 1; - eft.day = tm.tm_mday; - eft.minute = tm.tm_min; - eft.second = tm.tm_sec; - eft.nanosecond = 0; - } else { - pr_err(%s: Invalid EFI RTC value: write of %lx to EFI RTC failed\n, - __func__, nowtime); - return -1; - } - - status = efi.set_time(eft); - if (status != EFI_SUCCESS) { - pr_err(Oops: efitime: can't write time!\n); - return -1; - } - return 0; -} - void efi_get_time(struct timespec *now) { efi_status_t status; diff --git a/include/linux/efi.h b/include/linux/efi.h index 713a4f1..322366b 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -870,7 +870,6 @@ extern int __init efi_uart_console_only (void); extern void efi_initialize_iomem_resources(struct resource *code_resource, struct resource *data_resource, struct resource *bss_resource); extern void efi_get_time(struct timespec *now); -extern int efi_set_rtc_mmss(const struct timespec *now); extern void efi_reserve_boot_services(void); extern int efi_get_fdt_params(struct efi_fdt_params *params, int verbose); extern struct efi_memory_map memmap; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 2/9] arch/x86: Do not access EFI memory map if it is not available
Do not access EFI memory map if it is not available. At least Xen dom0 EFI implementation does not have an access to it. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - create this separate patch from main EFI_PARAVIRT patch (suggested by Matt Fleming). --- arch/x86/platform/efi/efi.c |3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 0ee1f46..d2d3c41 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -1220,6 +1220,9 @@ u64 efi_mem_attributes(unsigned long phys_addr) efi_memory_desc_t *md; void *p; + if (!efi_enabled(EFI_MEMMAP)) + return 0; + for (p = memmap.map; p memmap.map_end; p += memmap.desc_size) { md = p; if ((md-phys_addr = phys_addr) -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 6/9] xen: Define EFI related stuff
Define constants and structures which are needed to properly execute EFI related hypercall in Xen dom0. This patch is based on Jan Beulich and Tang Liang work. Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - fix indentation (suggested by David Vrabel). v5 - suggestions/fixes: - improve commit message (suggested by David Vrabel). v4 - suggestions/fixes: - change some types from generic to Xen specific ones (suggested by Stefano Stabellini), - do some formating changes (suggested by Jan Beulich). --- include/xen/interface/platform.h | 123 ++ 1 file changed, 123 insertions(+) diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h index f1331e3..5cc49ea 100644 --- a/include/xen/interface/platform.h +++ b/include/xen/interface/platform.h @@ -108,11 +108,113 @@ struct xenpf_platform_quirk { }; DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t); +#define XENPF_efi_runtime_call49 +#define XEN_EFI_get_time 1 +#define XEN_EFI_set_time 2 +#define XEN_EFI_get_wakeup_time 3 +#define XEN_EFI_set_wakeup_time 4 +#define XEN_EFI_get_next_high_monotonic_count 5 +#define XEN_EFI_get_variable 6 +#define XEN_EFI_set_variable 7 +#define XEN_EFI_get_next_variable_name8 +#define XEN_EFI_query_variable_info 9 +#define XEN_EFI_query_capsule_capabilities 10 +#define XEN_EFI_update_capsule 11 + +struct xenpf_efi_runtime_call { + uint32_t function; + /* +* This field is generally used for per sub-function flags (defined +* below), except for the XEN_EFI_get_next_high_monotonic_count case, +* where it holds the single returned value. +*/ + uint32_t misc; + xen_ulong_t status; + union { +#define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x0001 + struct { + struct xenpf_efi_time { + uint16_t year; + uint8_t month; + uint8_t day; + uint8_t hour; + uint8_t min; + uint8_t sec; + uint32_t ns; + int16_t tz; + uint8_t daylight; + } time; + uint32_t resolution; + uint32_t accuracy; + } get_time; + + struct xenpf_efi_time set_time; + +#define XEN_EFI_GET_WAKEUP_TIME_ENABLED 0x0001 +#define XEN_EFI_GET_WAKEUP_TIME_PENDING 0x0002 + struct xenpf_efi_time get_wakeup_time; + +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE 0x0001 +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY 0x0002 + struct xenpf_efi_time set_wakeup_time; + +#define XEN_EFI_VARIABLE_NON_VOLATILE 0x0001 +#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002 +#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x0004 + struct { + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + xen_ulong_t size; + GUEST_HANDLE(void) data; + struct xenpf_efi_guid { + uint32_t data1; + uint16_t data2; + uint16_t data3; + uint8_t data4[8]; + } vendor_guid; + } get_variable, set_variable; + + struct { + xen_ulong_t size; + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + struct xenpf_efi_guid vendor_guid; + } get_next_variable_name; + + struct { + uint32_t attr; + uint64_t max_store_size; + uint64_t remain_store_size; + uint64_t max_size; + } query_variable_info; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t max_capsule_size; + uint32_t reset_type; + } query_capsule_capabilities; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t sg_list; /* machine address */ + } update_capsule; + } u; +}; +DEFINE_GUEST_HANDLE_STRUCT(xenpf_efi_runtime_call); + +#define XEN_FW_EFI_VERSION0 +#define XEN_FW_EFI_CONFIG_TABLE 1 +#define XEN_FW_EFI_VENDOR 2 +#define
[PATCH v6 4/9] arch/x86: Remove redundant set_bit(EFI_SYSTEM_TABLES) call
Remove redundant set_bit(EFI_SYSTEM_TABLES, efi.flags) call. It is executed earlier in efi_systab_init(). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c |2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index b9c23d7..ae3d398 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -744,8 +744,6 @@ void __init efi_init(void) if (efi_systab_init(efi_phys.systab)) return; - set_bit(EFI_SYSTEM_TABLES, efi.flags); - efi.config_table = (unsigned long)efi.systab-tables; efi.fw_vendor= (unsigned long)efi.systab-fw_vendor; efi.runtime = (unsigned long)efi.systab-runtime; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/9] arch/x86: Remove redundant set_bit(EFI_MEMMAP) call
Remove redundant set_bit(EFI_MEMMAP, efi.flags) call. It is executed earlier in efi_memmap_init(). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c |2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index ae3d398..da15df9 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -784,8 +784,6 @@ void __init efi_init(void) if (efi_memmap_init()) return; - set_bit(EFI_MEMMAP, efi.flags); - print_efi_memmap(); } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/9] efi: Use early_mem*() instead of early_io*()
Use early_mem*() instead of early_io*() because all mapped EFI regions are memory (usually RAM but they could also be ROM, EPROM, EEPROM, flash, etc.) not I/O regions. Additionally, I/O family calls do not work correctly under Xen in our case. early_ioremap() skips the PFN to MFN conversion when building the PTE. Using it for memory will attempt to map the wrong machine frame. However, all artificial EFI structures created under Xen live in dom0 memory and should be mapped/unmapped using early_mem*() family calls which map domain memory. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- v6 - suggestions/fixes: - improve commit message (suggested by Matthew Garrett and Matt Fleming), - do not change indentation (suggested by Matt Fleming). --- arch/x86/platform/efi/efi.c | 32 drivers/firmware/efi/efi.c |4 ++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 87fc96b..0ee1f46 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void) { clear_bit(EFI_MEMMAP, efi.flags); if (memmap.map) { - early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size); + early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size); memmap.map = NULL; } } @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys) if (!data) return -ENOMEM; } - systab64 = early_ioremap((unsigned long)phys, + systab64 = early_memremap((unsigned long)phys, sizeof(*systab64)); if (systab64 == NULL) { pr_err(Couldn't map the system table!\n); if (data) - early_iounmap(data, sizeof(*data)); + early_memunmap(data, sizeof(*data)); return -ENOMEM; } @@ -504,9 +504,9 @@ static int __init efi_systab_init(void *phys) systab64-tables; tmp |= data ? data-tables : systab64-tables; - early_iounmap(systab64, sizeof(*systab64)); + early_memunmap(systab64, sizeof(*systab64)); if (data) - early_iounmap(data, sizeof(*data)); + early_memunmap(data, sizeof(*data)); #ifdef CONFIG_X86_32 if (tmp 32) { pr_err(EFI data located above 4GB, disabling EFI.\n); @@ -516,7 +516,7 @@ static int __init efi_systab_init(void *phys) } else { efi_system_table_32_t *systab32; - systab32 = early_ioremap((unsigned long)phys, + systab32 = early_memremap((unsigned long)phys, sizeof(*systab32)); if (systab32 == NULL) { pr_err(Couldn't map the system table!\n); @@ -537,7 +537,7 @@ static int __init efi_systab_init(void *phys) efi_systab.nr_tables = systab32-nr_tables; efi_systab.tables = systab32-tables; - early_iounmap(systab32, sizeof(*systab32)); + early_memunmap(systab32, sizeof(*systab32)); } efi.systab = efi_systab; @@ -563,7 +563,7 @@ static int __init efi_runtime_init32(void) { efi_runtime_services_32_t *runtime; - runtime = early_ioremap((unsigned long)efi.systab-runtime, + runtime = early_memremap((unsigned long)efi.systab-runtime, sizeof(efi_runtime_services_32_t)); if (!runtime) { pr_err(Could not map the runtime service table!\n); @@ -578,7 +578,7 @@ static int __init efi_runtime_init32(void) efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) (unsigned long)runtime-set_virtual_address_map; - early_iounmap(runtime, sizeof(efi_runtime_services_32_t)); + early_memunmap(runtime, sizeof(efi_runtime_services_32_t)); return 0; } @@ -587,7 +587,7 @@ static int __init efi_runtime_init64(void) { efi_runtime_services_64_t *runtime; - runtime = early_ioremap((unsigned long)efi.systab-runtime, + runtime = early_memremap((unsigned long)efi.systab-runtime, sizeof(efi_runtime_services_64_t)); if (!runtime) { pr_err(Could not map the runtime service table!\n); @@ -602,7 +602,7 @@ static int __init efi_runtime_init64(void) efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) (unsigned long)runtime-set_virtual_address_map; - early_iounmap(runtime, sizeof(efi_runtime_services_64_t
Re: [PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()
On Wed, Jun 18, 2014 at 01:17:09PM +0100, Matt Fleming wrote: (Pulling in Mark Salter for early_ioremap() knowledge) On Fri, 13 Jun, at 07:00:17PM, Daniel Kiper wrote: Use early_mem*() instead of early_io*() because all mapped EFI regions are true RAM not I/O regions. Additionally, I/O family calls do not work correctly under Xen in our case. AIUI, early_io*() maps/unmaps real machine addresses. However, all artificial EFI structures created under Xen live in dom0 memory and should be mapped/unmapped using early_mem*() family calls which map domain memory. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c | 42 +- drivers/firmware/efi/efi.c |4 ++-- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 87fc96b..dd1e351 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void) { clear_bit(EFI_MEMMAP, efi.flags); if (memmap.map) { - early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size); + early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size); memmap.map = NULL; } } @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys) if (!data) return -ENOMEM; } - systab64 = early_ioremap((unsigned long)phys, -sizeof(*systab64)); + systab64 = early_memremap((unsigned long)phys, + sizeof(*systab64)); Please don't misalign the arguments :-( This makes the diff harder to read when all you're doing is changing the function call. You're indentation isn't an improvement. I think that it improves readability a bit but if you wish I will not do that in the future. As Matthew pointed out we may also need to access EFI mapped flash devices. Right, but I think it does not change a lot in that case. Flash is simply slower type of memory used as permanent storage. Am I missing something? Now, the only difference between early_memremap() and early_ioremap(), at least on x86, is PAGE_KERNEL vs. PAGE_KERNEL_IO, where PAGE_KERNEL_IO has the additional _PAGE_BIT_IOMAP bit set in the pte. But that's a software bit for x86. So, even though this change should be harmless, it's not clear to me why this change is needed. You say I/O family calls do not work correctly under Xen in our case, but could you provide some more explanation as to why they don't work correctly? As I saw David provided better explanation. If you wish I could include it in commit message. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag
On Wed, Jun 18, 2014 at 03:30:25PM +0100, Stefano Stabellini wrote: On Wed, 18 Jun 2014, Jan Beulich wrote: On 18.06.14 at 15:52, m...@console-pimps.org wrote: EFI_PARAVIRT will be usable by architectures other than x86, correct? If your intention is for it only ever to be used by x86, then it should probably be EFI_ARCH_2. I would expect ARM, once it gets UEFI support on the Xen side, to be able to handle most of this identically to x86. Which raises the question whether most of the new Xen-specific code (in one of the other patches) wouldn't better live under drivers/xen/. I was thinking the same thing. However this patch series doesn't add much code outside drivers/xen/efi.c and include/xen/interface/platform.h. I think it wouldn't be fair to ask Daniel to refactor the efi code currently under arch/x86 to an arch-independent location. Whoever comes in later and adds EFI Xen support for ARM can do that. I was doing those EFI patches with ARM support in mind from the beginning. That is why I moved everything, which was possible, to arch independent place. There are only some simple init calls from arch/x86. Just a few required lines. So I think that it will be quite easy to use this EFI code on ARM platform. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/7] efi: Introduce EFI_NO_DIRECT flag
On Wed, Jun 18, 2014 at 02:52:29PM +0100, Matt Fleming wrote: On Fri, 13 Jun, at 07:00:18PM, Daniel Kiper wrote: Introduce EFI_NO_DIRECT flag. If it is set then kernel runs on EFI platform but it has not direct control on EFI stuff like EFI runtime, tables, structures, etc. If not this means that Linux Kernel has direct access to EFI infrastructure and everything runs as usual. This functionality is used in Xen dom0 because hypervisor has full control on EFI stuff and all calls from dom0 to EFI must be requested via special hypercall which in turn executes relevant EFI code in behalf of dom0. v5 - suggestions/fixes: - rename EFI_DIRECT to EFI_NO_DIRECT (suggested by David Vrabel), - limit EFI_NO_DIRECT usage (suggested by Jan Beulich and Matt Fleming), - improve commit message (suggested by David Vrabel). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c | 27 +-- drivers/firmware/efi/efi.c | 22 +- include/linux/efi.h |3 ++- 3 files changed, 36 insertions(+), 16 deletions(-) [...] @@ -617,13 +620,16 @@ static int __init efi_runtime_init(void) * address of several of the EFI runtime functions, needed to * set the firmware into virtual mode. */ - if (efi_enabled(EFI_64BIT)) - rv = efi_runtime_init64(); - else - rv = efi_runtime_init32(); - if (rv) - return rv; + if (!efi_enabled(EFI_NO_DIRECT)) { + if (efi_enabled(EFI_64BIT)) + rv = efi_runtime_init64(); + else + rv = efi_runtime_init32(); + + if (rv) + return rv; + } set_bit(EFI_RUNTIME_SERVICES, efi.flags); This could do with some comments to explain why you want to set EFI_RUNTIME_SERVICES even though you're skipping efi_runtime_init*(), e.g. that for Xen things are already mapped. I'm not likely to remember the rationale for this in 6 months time, and anyone else hacking on this code that isn't part of this thread also may not realise at first glance. Comments would go a long way to fixing that. OK, I will add relevant comment here. @@ -1220,6 +1232,9 @@ u64 efi_mem_attributes(unsigned long phys_addr) efi_memory_desc_t *md; void *p; + if (!efi_enabled(EFI_MEMMAP)) + return 0; + for (p = memmap.map; p memmap.map_end; p += memmap.desc_size) { md = p; if ((md-phys_addr = phys_addr) This should be a separate patch, please. OK. I was not sure about that. diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 023937a..8bb1075 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -104,16 +104,20 @@ static struct attribute *efi_subsys_attrs[] = { static umode_t efi_attr_is_visible(struct kobject *kobj, struct attribute *attr, int n) { - umode_t mode = attr-mode; - - if (attr == efi_attr_fw_vendor.attr) - return (efi.fw_vendor == EFI_INVALID_TABLE_ADDR) ? 0 : mode; - else if (attr == efi_attr_runtime.attr) - return (efi.runtime == EFI_INVALID_TABLE_ADDR) ? 0 : mode; - else if (attr == efi_attr_config_table.attr) - return (efi.config_table == EFI_INVALID_TABLE_ADDR) ? 0 : mode; + if (attr == efi_attr_fw_vendor.attr) { + if (efi_enabled(EFI_NO_DIRECT) || + efi.fw_vendor == EFI_INVALID_TABLE_ADDR) + return 0; + } else if (attr == efi_attr_runtime.attr) { + if (efi_enabled(EFI_NO_DIRECT) || + efi.runtime == EFI_INVALID_TABLE_ADDR) + return 0; + } else if (attr == efi_attr_config_table.attr) { + if (efi.config_table == EFI_INVALID_TABLE_ADDR) + return 0; + } - return mode; + return attr-mode; } Why don't you want to export efi.fw_vendor, etc? Rationale please. I am exporting real addresses (machine addresses) of things which I am able to get. Stuff which was created artificially and lives in dom0 address space or does not exist are not exported. static struct attribute_group efi_subsys_attr_group = { diff --git a/include/linux/efi.h b/include/linux/efi.h index 41bbf8b..e917c4a 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -916,7 +916,8 @@ extern int __init efi_setup_pcdp_console(char *); #define EFI_RUNTIME_SERVICES 3 /* Can we use runtime services? */ #define EFI_MEMMAP 4 /* Can we use EFI memory map? */ #define EFI_64BIT 5 /* Is the firmware 64-bit? */ -#define EFI_ARCH_1 6 /* First arch-specific bit */ +#define EFI_NO_DIRECT 6 /* Can we access EFI directly? */ +#define
Re: [PATCH v5 4/7] xen: Put EFI machinery in place
On Mon, Jun 16, 2014 at 12:55:35PM +0100, Stefano Stabellini wrote: On Fri, 13 Jun 2014, Daniel Kiper wrote: This patch enables EFI usage under Xen dom0. Standard EFI Linux Kernel infrastructure cannot be used because it requires direct access to EFI data and code. However, in dom0 case it is not possible because above mentioned EFI stuff is fully owned and controlled by Xen hypervisor. In this case all calls from dom0 to EFI must be requested via special hypercall which in turn executes relevant EFI code in behalf of dom0. When dom0 kernel boots it checks for EFI availability on a machine. If it is detected then artificial EFI system table is filled. Native EFI callas are replaced by functions which mimics them by calling relevant hypercall. Later pointer to EFI system table is passed to standard EFI machinery and it continues EFI subsystem initialization taking into account that there is no direct access to EFI boot services, runtime, tables, structures, etc. After that system runs as usual. This patch is based on Jan Beulich and Tang Liang work. v5 - suggestions/fixes: - improve macro usage readability (suggested by Andrew Cooper and David Vrabel), - conditions cleanup (suggested by David Vrabel), - use -fshort-wchar option (suggested by Jan Beulich), - Kconfig rule cleanup (suggested by Jan Beulich), - forward port fixes from SUSE kernel (suggested by Jan Beulich), - improve commit message (suggested by David Vrabel). v4 - suggestions/fixes: - just populate an efi_system_table_t object (suggested by Matt Fleming). Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com Sorry for commenting on your patches so late in the review cycle. No problem. [...] +efi_system_table_t __init *xen_efi_probe(void) +{ + struct xen_platform_op op = { + .cmd = XENPF_firmware_info, + .u.firmware_info = { + .type = XEN_FW_EFI_INFO, + .index = XEN_FW_EFI_CONFIG_TABLE + } + }; + union xenpf_efi_info *info = op.u.firmware_info.u.efi_info; + + if (!xen_initial_domain() || HYPERVISOR_dom0_op(op) 0) + return NULL; + + /* Here we know that Xen runs on EFI platform. */ + + efi = efi_xen; + + op.cmd = XENPF_firmware_info; + op.u.firmware_info.type = XEN_FW_EFI_INFO; + op.u.firmware_info.index = XEN_FW_EFI_VENDOR; + info-vendor.bufsz = sizeof(vendor); + set_xen_guest_handle(info-vendor.name, vendor); + + if (HYPERVISOR_dom0_op(op) == 0) { + efi_systab_xen.fw_vendor = __pa_symbol(vendor); + efi_systab_xen.fw_revision = info-vendor.revision; + } else + efi_systab_xen.fw_vendor = __pa_symbol(LUNKNOWN); + + op.cmd = XENPF_firmware_info; + op.u.firmware_info.type = XEN_FW_EFI_INFO; + op.u.firmware_info.index = XEN_FW_EFI_VERSION; + + if (HYPERVISOR_dom0_op(op) == 0) + efi_systab_xen.hdr.revision = info-version; + + op.cmd = XENPF_firmware_info; + op.u.firmware_info.type = XEN_FW_EFI_INFO; + op.u.firmware_info.index = XEN_FW_EFI_RT_VERSION; + + if (HYPERVISOR_dom0_op(op) == 0) + efi.runtime_version = info-version; + + op.cmd = XENPF_firmware_info; + op.u.firmware_info.type = XEN_FW_EFI_INFO; + op.u.firmware_info.index = XEN_FW_EFI_CONFIG_TABLE; + + if (HYPERVISOR_dom0_op(op) 0) + BUG(); Is it really worth of a BUG()? Can't we just print a warning and return NULL? We could still boot without EFI support. Earlier the same hypercall function succeeded so if here it failed it means that something is really broken. However, I will try remove this call and get all data from earlier one. This way we avoid this BUG() call. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 4/7] xen: Put EFI machinery in place
On Mon, Jun 16, 2014 at 01:00:29PM +0100, David Vrabel wrote: On 13/06/14 18:00, Daniel Kiper wrote: v5 - suggestions/fixes: Put after a --- marker. Why? You mean: --- v5 - suggestions/fixes: ... +static efi_char16_t vendor[100] __initdata; Why 100? Well... Quite arbitrary value. The same thing is used in arch/x86/platform/efi/efi.c:efi_init(). Sadly UEFI sepec says nothing about maximum vendor name length. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Why efi_set_rtc_mmss() is never used?
On Mon, Jun 09, 2014 at 01:46:35PM +0100, One Thousand Gnomes wrote: On Mon, 9 Jun 2014 13:08:00 +0200 Daniel Kiper daniel.ki...@oracle.com wrote: Hi, As I can see efi_set_rtc_mmss() is never used to set RTC. Even on EFI platform mach_set_rtc_mmss() is called and does that thing. Why it is done in that way? It is a bug or it was done intentionally. Am I missing something? EFI services generally don't work properly. The less they are used the better. We can realistically only make an EFI call with the same parameters windows does under the same kind of circumstances because the firmware on many motherboards is complete sh*te. Thanks. Sadly it makes sens. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 6/7] arch/x86: Remove redundant set_bit() call
Remove redundant set_bit(EFI_SYSTEM_TABLES, efi.flags) call. It is executed earlier in efi_systab_init(). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c |2 -- 1 file changed, 2 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index e3d9d76..deb4f05 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -737,8 +737,6 @@ void __init efi_init(void) if (efi_systab_init(efi_phys.systab)) return; - set_bit(EFI_SYSTEM_TABLES, efi.flags); - efi.config_table = (unsigned long)efi.systab-tables; efi.fw_vendor= (unsigned long)efi.systab-fw_vendor; efi.runtime = (unsigned long)efi.systab-runtime; -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 7/7] arch/x86: Comment out efi_set_rtc_mmss()
efi_set_rtc_mmss() is never used to set RTC due to bugs found on many EFI platforms. It is set directly by mach_set_rtc_mmss(). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c |6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index deb4f05..bd3e080 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -244,6 +244,11 @@ static efi_status_t __init phys_efi_set_virtual_address_map( return status; } +#if 0 +/* + * efi_set_rtc_mmss() is never used to set RTC due to bugs found + * on many EFI platforms. It is set directly by mach_set_rtc_mmss(). + */ int efi_set_rtc_mmss(const struct timespec *now) { unsigned long nowtime = now-tv_sec; @@ -279,6 +284,7 @@ int efi_set_rtc_mmss(const struct timespec *now) } return 0; } +#endif void efi_get_time(struct timespec *now) { -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/7] xen: Add EFI support
Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. Daniel arch/x86/kernel/setup.c |4 +- arch/x86/platform/efi/efi.c | 77 ++--- arch/x86/xen/enlighten.c | 24 ++ drivers/firmware/efi/efi.c | 26 +++--- drivers/xen/Kconfig |4 + drivers/xen/Makefile |3 + drivers/xen/efi.c| 374 +++ include/linux/efi.h |3 +- include/xen/interface/platform.h | 123 +++ 9 files changed, 595 insertions(+), 43 deletions(-) Daniel Kiper (7): efi: Use early_mem*() instead of early_io*() efi: Introduce EFI_NO_DIRECT flag xen: Define EFI related stuff xen: Put EFI machinery in place arch/x86: Replace plain strings with constants arch/x86: Remove redundant set_bit() call arch/x86: Comment out efi_set_rtc_mmss() -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/7] arch/x86: Replace plain strings with constants
v5 - suggestions/fixes: - do not change indentation (suggested by Matt Fleming). Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/kernel/setup.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 78a0e62..41ead8d 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -924,10 +924,10 @@ void __init setup_arch(char **cmdline_p) #endif #ifdef CONFIG_EFI if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL32, 4)) { +EFI32_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); } else if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL64, 4)) { +EFI64_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); set_bit(EFI_64BIT, efi.flags); } -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/7] efi: Use early_mem*() instead of early_io*()
Use early_mem*() instead of early_io*() because all mapped EFI regions are true RAM not I/O regions. Additionally, I/O family calls do not work correctly under Xen in our case. AIUI, early_io*() maps/unmaps real machine addresses. However, all artificial EFI structures created under Xen live in dom0 memory and should be mapped/unmapped using early_mem*() family calls which map domain memory. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/platform/efi/efi.c | 42 +- drivers/firmware/efi/efi.c |4 ++-- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 87fc96b..dd1e351 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -427,7 +427,7 @@ void __init efi_unmap_memmap(void) { clear_bit(EFI_MEMMAP, efi.flags); if (memmap.map) { - early_iounmap(memmap.map, memmap.nr_map * memmap.desc_size); + early_memunmap(memmap.map, memmap.nr_map * memmap.desc_size); memmap.map = NULL; } } @@ -467,12 +467,12 @@ static int __init efi_systab_init(void *phys) if (!data) return -ENOMEM; } - systab64 = early_ioremap((unsigned long)phys, -sizeof(*systab64)); + systab64 = early_memremap((unsigned long)phys, + sizeof(*systab64)); if (systab64 == NULL) { pr_err(Couldn't map the system table!\n); if (data) - early_iounmap(data, sizeof(*data)); + early_memunmap(data, sizeof(*data)); return -ENOMEM; } @@ -504,9 +504,9 @@ static int __init efi_systab_init(void *phys) systab64-tables; tmp |= data ? data-tables : systab64-tables; - early_iounmap(systab64, sizeof(*systab64)); + early_memunmap(systab64, sizeof(*systab64)); if (data) - early_iounmap(data, sizeof(*data)); + early_memunmap(data, sizeof(*data)); #ifdef CONFIG_X86_32 if (tmp 32) { pr_err(EFI data located above 4GB, disabling EFI.\n); @@ -516,8 +516,8 @@ static int __init efi_systab_init(void *phys) } else { efi_system_table_32_t *systab32; - systab32 = early_ioremap((unsigned long)phys, -sizeof(*systab32)); + systab32 = early_memremap((unsigned long)phys, + sizeof(*systab32)); if (systab32 == NULL) { pr_err(Couldn't map the system table!\n); return -ENOMEM; @@ -537,7 +537,7 @@ static int __init efi_systab_init(void *phys) efi_systab.nr_tables = systab32-nr_tables; efi_systab.tables = systab32-tables; - early_iounmap(systab32, sizeof(*systab32)); + early_memunmap(systab32, sizeof(*systab32)); } efi.systab = efi_systab; @@ -563,8 +563,8 @@ static int __init efi_runtime_init32(void) { efi_runtime_services_32_t *runtime; - runtime = early_ioremap((unsigned long)efi.systab-runtime, - sizeof(efi_runtime_services_32_t)); + runtime = early_memremap((unsigned long)efi.systab-runtime, + sizeof(efi_runtime_services_32_t)); if (!runtime) { pr_err(Could not map the runtime service table!\n); return -ENOMEM; @@ -578,7 +578,7 @@ static int __init efi_runtime_init32(void) efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) (unsigned long)runtime-set_virtual_address_map; - early_iounmap(runtime, sizeof(efi_runtime_services_32_t)); + early_memunmap(runtime, sizeof(efi_runtime_services_32_t)); return 0; } @@ -587,8 +587,8 @@ static int __init efi_runtime_init64(void) { efi_runtime_services_64_t *runtime; - runtime = early_ioremap((unsigned long)efi.systab-runtime, - sizeof(efi_runtime_services_64_t)); + runtime = early_memremap((unsigned long)efi.systab-runtime, + sizeof(efi_runtime_services_64_t)); if (!runtime) { pr_err(Could not map the runtime service table!\n); return -ENOMEM; @@ -602,7 +602,7 @@ static int __init efi_runtime_init64(void) efi_phys.set_virtual_address_map = (efi_set_virtual_address_map_t *) (unsigned long)runtime-set_virtual_address_map
[PATCH v5 3/7] xen: Define EFI related stuff
Define constants and structures which are needed to properly execute EFI related hypercall in Xen dom0. This patch is based on Jan Beulich and Tang Liang work. v5 - suggestions/fixes: - improve commit message (suggested by David Vrabel). v4 - suggestions/fixes: - change some types from generic to Xen specific ones (suggested by Stefano Stabellini), - do some formating changes (suggested by Jan Beulich). Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- include/xen/interface/platform.h | 123 ++ 1 file changed, 123 insertions(+) diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h index f1331e3..55deeb7 100644 --- a/include/xen/interface/platform.h +++ b/include/xen/interface/platform.h @@ -108,11 +108,113 @@ struct xenpf_platform_quirk { }; DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t); +#define XENPF_efi_runtime_call49 +#define XEN_EFI_get_time 1 +#define XEN_EFI_set_time 2 +#define XEN_EFI_get_wakeup_time 3 +#define XEN_EFI_set_wakeup_time 4 +#define XEN_EFI_get_next_high_monotonic_count 5 +#define XEN_EFI_get_variable 6 +#define XEN_EFI_set_variable 7 +#define XEN_EFI_get_next_variable_name8 +#define XEN_EFI_query_variable_info 9 +#define XEN_EFI_query_capsule_capabilities 10 +#define XEN_EFI_update_capsule 11 + +struct xenpf_efi_runtime_call { + uint32_t function; + /* +* This field is generally used for per sub-function flags (defined +* below), except for the XEN_EFI_get_next_high_monotonic_count case, +* where it holds the single returned value. +*/ + uint32_t misc; + xen_ulong_t status; + union { +#define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x0001 + struct { + struct xenpf_efi_time { + uint16_t year; + uint8_t month; + uint8_t day; + uint8_t hour; + uint8_t min; + uint8_t sec; + uint32_t ns; + int16_t tz; + uint8_t daylight; + } time; + uint32_t resolution; + uint32_t accuracy; + } get_time; + + struct xenpf_efi_time set_time; + +#define XEN_EFI_GET_WAKEUP_TIME_ENABLED 0x0001 +#define XEN_EFI_GET_WAKEUP_TIME_PENDING 0x0002 + struct xenpf_efi_time get_wakeup_time; + +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE 0x0001 +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY 0x0002 + struct xenpf_efi_time set_wakeup_time; + +#define XEN_EFI_VARIABLE_NON_VOLATILE 0x0001 +#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002 +#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x0004 + struct { + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + xen_ulong_t size; + GUEST_HANDLE(void) data; + struct xenpf_efi_guid { + uint32_t data1; + uint16_t data2; + uint16_t data3; + uint8_t data4[8]; + } vendor_guid; + } get_variable, set_variable; + + struct { + xen_ulong_t size; + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + struct xenpf_efi_guid vendor_guid; + } get_next_variable_name; + + struct { + uint32_t attr; + uint64_t max_store_size; + uint64_t remain_store_size; + uint64_t max_size; + } query_variable_info; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t max_capsule_size; + uint32_t reset_type; + } query_capsule_capabilities; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t sg_list; /* machine address */ + } update_capsule; + } u; +}; +DEFINE_GUEST_HANDLE_STRUCT(xenpf_efi_runtime_call); + +#define XEN_FW_EFI_VERSION0 +#define XEN_FW_EFI_CONFIG_TABLE 1 +#define XEN_FW_EFI_VENDOR 2 +#define XEN_FW_EFI_MEM_INFO 3 +#define XEN_FW_EFI_RT_VERSION 4 + #define XENPF_firmware_info 50 #define XEN_FW_DISK_INFO 1 /* from int 13 AH=08/41/48 */ #define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */ #define XEN_FW_VBEDDC_INFO3 /* from int 10 AX=4f15 */ +#define XEN_FW_EFI_INFO 4 /* from EFI */ #define XEN_FW_KBD_SHIFT_FLAGS5 /* Int16, Fn02: Get keyboard shift flags. */ + struct
Why efi_set_rtc_mmss() is never used?
Hi, As I can see efi_set_rtc_mmss() is never used to set RTC. Even on EFI platform mach_set_rtc_mmss() is called and does that thing. Why it is done in that way? It is a bug or it was done intentionally. Am I missing something? Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 3/5] xen: Put EFI machinery in place
On Tue, May 20, 2014 at 10:47:00AM +0100, David Vrabel wrote: On 16/05/14 21:41, Daniel Kiper wrote: Put EFI machinery for Xen in place. Put what machinery to do what? @@ -1714,6 +1725,21 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)((__u64)efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)((__u64)efi_systab_xen 32); + + x86_platform.get_wallclock = efi_get_time; x86_platform.get_wallclock should always be xen_get_wallclock(). Hmmm... Make sens... Jan, why did you replace x86_platform.get_wallclock with efi_get_time() in your implementation? + x86_platform.set_wallclock = efi_set_rtc_mmss; Now I am not sure even about that. As I can see Linux Kernel running on bare metal EFI platform directly sets RTC omitting efi_set_rtc_mmss(). Am I missing something? IIRC, there was discussion about that once. But where and when? Anyway, now I think that this initialization should be done in arch/x86/xen/time.c if needed. + + set_bit(EFI_BOOT, efi.flags); + set_bit(EFI_64BIT, efi.flags); + } + /* Start the world */ #ifdef CONFIG_X86_32 i386_start_kernel(); --- /dev/null +++ b/drivers/xen/efi.c @@ -0,0 +1,374 @@ + * Copyright (c) 2014 Daniel Kiper, Oracle Corporation Is this really copyright by you personally and not Oracle? As I know it is correct but I will double check it. +#define call (op.u.efi_runtime_call) +#define DECLARE_CALL(what) \ + struct xen_platform_op op; \ + op.cmd = XENPF_efi_runtime_call; \ + call.function = XEN_EFI_##what; \ + call.misc = 0 Macros that declare local variables are awful. Use what Andrew suggested and something like struct xen_blah *call = op.u.efi_runtime_call; +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, + .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 = NULL, /* Functionality provided by Xen. */ Xen provides functionality to reset (just maybe not via an EFI call). Should an implementation be provided that does this? I do not think so. efi.reset_system() would not be called on Xen because all reset related stuff is replaced by Xen reset specific functions. Please check arch/x86/xen/enlighten.c. Additionally, I think that situation is a bit similar to time issue. If we are running on Xen then we should ask Xen to do reset because Xen controls platform and it knows how to do that. + .set_virtual_address_map = NULL, /* Not used under Xen. */ + .memmap = NULL, /* Not used under Xen. */ + .flags= 0 /* Initialized later. */ +}; + +efi_system_table_t __init *xen_efi_probe(void) +{ + struct xen_platform_op op = { + .cmd = XENPF_firmware_info, + .u.firmware_info = { + .type = XEN_FW_EFI_INFO, + .index = XEN_FW_EFI_CONFIG_TABLE + } + }; + union xenpf_efi_info *info = op.u.firmware_info.u.efi_info; + + if (!xen_initial_domain() || HYPERVISOR_dom0_op(op)) + return NULL; if (!xen_initial_domain()) return NULL; if (HYPERVISOR_dom0_op(op) 0) return
[PATCH v4 0/5] xen: Add EFI support
Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. I am still not sure what to do with /sys/firmware/efi/config_table, /sys/firmware/efi/{fw_vendor,runtime,systab} files. On bare metal they contain physical addresses of relevant structures. However, in Xen case they does not make sens. So maybe they should contain invalid values (e.g. 0) or should not appear at all on Xen (I prefer last one). What do you think about that? Daniel arch/x86/kernel/setup.c |6 +- arch/x86/platform/efi/efi.c | 60 ++ arch/x86/xen/enlighten.c | 26 ++ drivers/xen/Kconfig |3 + drivers/xen/Makefile |1 + drivers/xen/efi.c| 374 +++ include/linux/efi.h | 13 +-- include/xen/interface/platform.h | 123 +++ 8 files changed, 582 insertions(+), 24 deletions(-) Daniel Kiper (5): efi: Introduce EFI_DIRECT flag xen: Define EFI related stuff xen: Put EFI machinery in place arch/x86: Replace plain strings with constants arch/x86: Remove redundant set_bit() call -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 3/5] xen: Put EFI machinery in place
Put EFI machinery for Xen in place. This patch is based on Jan Beulich and Tang Liang work. v4 - suggestions/fixes: - just populate an efi_system_table_t object (suggested by Matt Fleming). Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/xen/enlighten.c | 26 drivers/xen/Kconfig |3 + drivers/xen/Makefile |1 + drivers/xen/efi.c| 374 ++ 4 files changed, 404 insertions(+) create mode 100644 drivers/xen/efi.c diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index c34bfc4..d5cc21f 100644 --- a/arch/x86/xen/enlighten.c +++ b/arch/x86/xen/enlighten.c @@ -32,6 +32,7 @@ #include linux/gfp.h #include linux/memblock.h #include linux/edd.h +#include linux/efi.h #include xen/xen.h #include xen/events.h @@ -150,6 +151,15 @@ struct shared_info *HYPERVISOR_shared_info = xen_dummy_shared_info; */ static int have_vcpu_info_placement = 1; +#ifdef CONFIG_XEN_EFI +extern efi_system_table_t __init *xen_efi_probe(void); +#else +static efi_system_table_t __init *xen_efi_probe(void) +{ + return NULL; +} +#endif + struct tls_descs { struct desc_struct desc[3]; }; @@ -1519,6 +1529,7 @@ asmlinkage __visible void __init xen_start_kernel(void) { struct physdev_set_iopl set_iopl; int rc; + efi_system_table_t *efi_systab_xen; if (!xen_start_info) return; @@ -1714,6 +1725,21 @@ asmlinkage __visible void __init xen_start_kernel(void) xen_setup_runstate_info(0); + efi_systab_xen = xen_efi_probe(); + + if (efi_systab_xen) { + strncpy((char *)boot_params.efi_info.efi_loader_signature, Xen, + sizeof(boot_params.efi_info.efi_loader_signature)); + boot_params.efi_info.efi_systab = (__u32)((__u64)efi_systab_xen); + boot_params.efi_info.efi_systab_hi = (__u32)((__u64)efi_systab_xen 32); + + x86_platform.get_wallclock = efi_get_time; + x86_platform.set_wallclock = efi_set_rtc_mmss; + + set_bit(EFI_BOOT, efi.flags); + set_bit(EFI_64BIT, efi.flags); + } + /* Start the world */ #ifdef CONFIG_X86_32 i386_start_kernel(); diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig index 38fb36e..cead283 100644 --- a/drivers/xen/Kconfig +++ b/drivers/xen/Kconfig @@ -240,4 +240,7 @@ config XEN_MCE_LOG config XEN_HAVE_PVMMU bool +config XEN_EFI + def_bool X86_64 EFI + endmenu diff --git a/drivers/xen/Makefile b/drivers/xen/Makefile index 45e00af..c35de02 100644 --- a/drivers/xen/Makefile +++ b/drivers/xen/Makefile @@ -33,6 +33,7 @@ obj-$(CONFIG_XEN_STUB)+= xen-stub.o obj-$(CONFIG_XEN_ACPI_HOTPLUG_MEMORY) += xen-acpi-memhotplug.o obj-$(CONFIG_XEN_ACPI_HOTPLUG_CPU) += xen-acpi-cpuhotplug.o obj-$(CONFIG_XEN_ACPI_PROCESSOR) += xen-acpi-processor.o +obj-$(CONFIG_XEN_EFI) += efi.o xen-evtchn-y := evtchn.o xen-gntdev-y := gntdev.o xen-gntalloc-y := gntalloc.o diff --git a/drivers/xen/efi.c b/drivers/xen/efi.c new file mode 100644 index 000..d81458a --- /dev/null +++ b/drivers/xen/efi.c @@ -0,0 +1,374 @@ +/* + * EFI support for Xen. + * + * Copyright (C) 1999 VA Linux Systems + * Copyright (C) 1999 Walt Drummond drumm...@valinux.com + * Copyright (C) 1999-2002 Hewlett-Packard Co. + * David Mosberger-Tang dav...@hpl.hp.com + * Stephane Eranian eran...@hpl.hp.com + * Copyright (C) 2005-2008 Intel Co. + * Fenghua Yu fenghua...@intel.com + * Bibo Mao bibo@intel.com + * Chandramouli Narayanan mo...@linux.intel.com + * Huang Ying ying.hu...@intel.com + * Copyright (C) 2011 Novell Co. + * Jan Beulich jbeul...@suse.com + * Copyright (C) 2011-2012 Oracle Co. + * Liang Tang liang.t...@oracle.com + * Copyright (c) 2014 Daniel Kiper, Oracle Corporation + */ + +#include linux/bug.h +#include linux/efi.h +#include linux/init.h +#include linux/string.h + +#include xen/interface/xen.h +#include xen/interface/platform.h + +#include asm/xen/hypercall.h + +#define call (op.u.efi_runtime_call) +#define DECLARE_CALL(what) \ + struct xen_platform_op op; \ + op.cmd = XENPF_efi_runtime_call; \ + call.function = XEN_EFI_##what; \ + call.misc = 0 + +static efi_status_t xen_efi_get_time(efi_time_t *tm, efi_time_cap_t *tc) +{ + int err; + DECLARE_CALL(get_time); + + err = HYPERVISOR_dom0_op(op); + if (err) + return EFI_UNSUPPORTED; + + if (tm) { + BUILD_BUG_ON(sizeof(*tm) != sizeof(call.u.get_time.time)); + memcpy(tm, call.u.get_time.time, sizeof(*tm)); + } + + if (tc) { + tc-resolution = call.u.get_time.resolution
[PATCH v4 4/5] arch/x86: Replace plain strings with constants
Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/kernel/setup.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index f41f648..7a67f5d 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -924,11 +924,11 @@ void __init setup_arch(char **cmdline_p) #endif #ifdef CONFIG_EFI if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL32, 4)) { + EFI32_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); set_bit(EFI_DIRECT, efi.flags); } else if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, -EL64, 4)) { + EFI64_LOADER_SIGNATURE, 4)) { set_bit(EFI_BOOT, efi.flags); set_bit(EFI_DIRECT, efi.flags); set_bit(EFI_64BIT, efi.flags); -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v4 2/5] xen: Define EFI related stuff
Define EFI related stuff for Xen. This patch is based on Jan Beulich and Tang Liang work. v4 - suggestions/fixes: - change some types from generic to Xen specific ones (suggested by Stefano Stabellini), - do some formating changes (suggested by Jan Beulich). Signed-off-by: Jan Beulich jbeul...@suse.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- include/xen/interface/platform.h | 123 ++ 1 file changed, 123 insertions(+) diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h index f1331e3..55deeb7 100644 --- a/include/xen/interface/platform.h +++ b/include/xen/interface/platform.h @@ -108,11 +108,113 @@ struct xenpf_platform_quirk { }; DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t); +#define XENPF_efi_runtime_call49 +#define XEN_EFI_get_time 1 +#define XEN_EFI_set_time 2 +#define XEN_EFI_get_wakeup_time 3 +#define XEN_EFI_set_wakeup_time 4 +#define XEN_EFI_get_next_high_monotonic_count 5 +#define XEN_EFI_get_variable 6 +#define XEN_EFI_set_variable 7 +#define XEN_EFI_get_next_variable_name8 +#define XEN_EFI_query_variable_info 9 +#define XEN_EFI_query_capsule_capabilities 10 +#define XEN_EFI_update_capsule 11 + +struct xenpf_efi_runtime_call { + uint32_t function; + /* +* This field is generally used for per sub-function flags (defined +* below), except for the XEN_EFI_get_next_high_monotonic_count case, +* where it holds the single returned value. +*/ + uint32_t misc; + xen_ulong_t status; + union { +#define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x0001 + struct { + struct xenpf_efi_time { + uint16_t year; + uint8_t month; + uint8_t day; + uint8_t hour; + uint8_t min; + uint8_t sec; + uint32_t ns; + int16_t tz; + uint8_t daylight; + } time; + uint32_t resolution; + uint32_t accuracy; + } get_time; + + struct xenpf_efi_time set_time; + +#define XEN_EFI_GET_WAKEUP_TIME_ENABLED 0x0001 +#define XEN_EFI_GET_WAKEUP_TIME_PENDING 0x0002 + struct xenpf_efi_time get_wakeup_time; + +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE 0x0001 +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY 0x0002 + struct xenpf_efi_time set_wakeup_time; + +#define XEN_EFI_VARIABLE_NON_VOLATILE 0x0001 +#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002 +#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x0004 + struct { + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + xen_ulong_t size; + GUEST_HANDLE(void) data; + struct xenpf_efi_guid { + uint32_t data1; + uint16_t data2; + uint16_t data3; + uint8_t data4[8]; + } vendor_guid; + } get_variable, set_variable; + + struct { + xen_ulong_t size; + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + struct xenpf_efi_guid vendor_guid; + } get_next_variable_name; + + struct { + uint32_t attr; + uint64_t max_store_size; + uint64_t remain_store_size; + uint64_t max_size; + } query_variable_info; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t max_capsule_size; + uint32_t reset_type; + } query_capsule_capabilities; + + struct { + GUEST_HANDLE(void) capsule_header_array; + xen_ulong_t capsule_count; + uint64_t sg_list; /* machine address */ + } update_capsule; + } u; +}; +DEFINE_GUEST_HANDLE_STRUCT(xenpf_efi_runtime_call); + +#define XEN_FW_EFI_VERSION0 +#define XEN_FW_EFI_CONFIG_TABLE 1 +#define XEN_FW_EFI_VENDOR 2 +#define XEN_FW_EFI_MEM_INFO 3 +#define XEN_FW_EFI_RT_VERSION 4 + #define XENPF_firmware_info 50 #define XEN_FW_DISK_INFO 1 /* from int 13 AH=08/41/48 */ #define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */ #define XEN_FW_VBEDDC_INFO3 /* from int 10 AX=4f15 */ +#define XEN_FW_EFI_INFO 4 /* from EFI */ #define XEN_FW_KBD_SHIFT_FLAGS5 /* Int16, Fn02: Get keyboard shift flags. */ + struct xenpf_firmware_info { /* IN variables. */ uint32_t type; @@ -144,6 +246,26 @@ struct xenpf_firmware_info { GUEST_HANDLE
Re: [PATCH v3 3/5] x86: Call efi_memblock_x86_reserve_range() on native EFI platform only
On Wed, Mar 26, 2014 at 01:39:42PM +, Jan Beulich wrote: On 26.03.14 at 14:31, m...@console-pimps.org wrote: On Wed, 26 Mar, at 01:22:49PM, Jan Beulich wrote: On 26.03.14 at 14:00, m...@console-pimps.org wrote: This could do with a little bit more explanation. Why is it not necessary to mark the EFI memory map that was passed to the kernel as reserved in memblock? Because that's in memory Dom0 doesn't even see: The EFI memory map is visible to the hypervisor only. So where does boot_params.efi_info.efi_memmap point? If nowhere (i.e. it's NULL) that's no problem because memblock_reserve() handles zero size regions just fine. That's a question to Daniel - in our implementation (with a separate Xen kernel that can't run on bare hardware) boot_params as a whole simply doesn't exist. On my machine this function crashes on Xen so that is why I have changed condition. However, if you say that this issue could be solved in another way I will investigate it further. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] efi: Add efi_init_ops variable
On Wed, Mar 26, 2014 at 12:56:23PM +, Matt Fleming wrote: On Tue, 25 Mar, at 09:57:52PM, Daniel Kiper wrote: Add efi_init_ops variable which allows us to replace EFI init functions on Xen with its specific stuff. This patch is based on Jan Beulich and Tang Liang work. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com Signed-off-by: Tang Liang liang.t...@oracle.com --- arch/ia64/kernel/efi.c | 30 +- arch/x86/platform/efi/efi.c | 18 +- drivers/firmware/efi/efi.c | 26 ++ include/linux/efi.h |8 4 files changed, 68 insertions(+), 14 deletions(-) [...] @@ -1118,6 +1118,14 @@ u64 efi_mem_attributes(unsigned long phys_addr) return 0; } +struct efi_init_ops efi_init_ops = { + .efi_init = efi_init_generic, + .efi_reserve_boot_services = efi_reserve_boot_services_generic, + .efi_enter_virtual_mode = efi_enter_virtual_mode_generic, + .efi_mem_type = efi_mem_type_generic, + .efi_mem_attributes = efi_mem_attributes_generic +}; Please don't create another struct of EFI function pointers. After this we'll have 3 global efi structures defintions and 4 global efi objects on x86, - struct efi_early [in the boot stub] - struct efi ['efi_phys' before/'efi' after SetVirtualAddressMap()] - struct efi_init_ops [for the benefit of xen] What do you suggest? Should we fill all EFI related structures in xen_efi_probe() (called from xen_start_kernel()) and later they should be parsed by generic/x86 EFI initialization code? I suppose that many variables will have NULL (or something relevant) in Xen dom0 and it will require some changes in EFI initialization code. I have a big problem with exposing .efi_reserve_boot_services as an API. Why? efi_reserve_boot_services() is public right now. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] efi: Export arch_tables variable
On Wed, Mar 26, 2014 at 01:21:19PM +, Jan Beulich wrote: On 25.03.14 at 21:57, daniel.ki...@oracle.com wrote: Export arch_tables variable. Xen init function calls efi_config_init() which takes it as an argument. Additionally, put __initdata in place suggested by include/linux/init.h. Which isn't necessarily the most appropriate place. Why? If comments in include/linux/init.h are not valid they should be changed. --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -70,7 +70,7 @@ static efi_system_table_t efi_systab __initdata; unsigned long x86_efi_facility; -static __initdata efi_config_table_type_t arch_tables[] = { +efi_config_table_type_t arch_tables[] __initdata = { efi_config_table_type_t __initdata arch_tables[] = { would be what I'd recommend. --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -583,6 +583,8 @@ extern struct efi { struct efi_memory_map *memmap; } efi; +extern efi_config_table_type_t arch_tables[] __initdata; And section placement annotations are bogus on declarations. Hmmm... I am not sure which approach is better. I saw that in many places declarations have annotations. Could you point me some docs which states (and explains) that this is wrong idea. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] x86: Call efi_memblock_x86_reserve_range() on native EFI platform only
On Wed, Mar 26, 2014 at 01:57:23PM +, Matt Fleming wrote: On Wed, 26 Mar, at 02:48:45PM, Daniel Kiper wrote: On my machine this function crashes on Xen so that is why I have changed condition. However, if you say that this issue could be solved in another way I will investigate it further. Daniel, could you paste the crash? Do you get a stack trace? Here it is: [...] mapping kernel into physical memory about to get started... (XEN) traps.c:458:d0v0 Unhandled divide error fault/trap [#0] on VCPU 0 [ec=] (XEN) domain_crash_sync called from entry.S: fault at 82d080229d30 int80_direct_trap+0x200/0x210 (XEN) Domain 0 (vcpu#0) crashed on cpu#0: (XEN) [ Xen-4.5-unstable x86_64 debug=y Tainted:C ] (XEN) CPU:0 (XEN) RIP:e033:[816987c5] (XEN) RFLAGS: 0246 EM: 1 CONTEXT: pv guest (XEN) rax: rbx: 0100 rcx: (XEN) rdx: rsi: 814d7e7e rdi: (XEN) rbp: 81601e88 rsp: 81601e48 r8: (XEN) r9: 7ff0 r10: deadbeef r11: 81601df8 (XEN) r12: 816fe010 r13: 81601f00 r14: (XEN) r15: cr0: 80050033 cr4: 26f0 (XEN) cr3: 7b60d000 cr2: (XEN) ds: es: fs: gs: ss: e02b cs: e033 (XEN) Guest stack trace from rsp=81601e48: (XEN) 81601df8 816987c5 0001e030 (XEN)00010046 81601e88 e02b 81601f00 (XEN)81601ed8 8168a1a4 81601ee8 81601ea8 (XEN)880001e16490 816fe010 (XEN) 81601f28 81685a3e (XEN) (XEN)880001e16000 (XEN)81601f38 816854c8 81601ff8 81687442 (XEN)0001 08000623 0789cbf580802001 03010032 (XEN)0005 (XEN) (XEN) (XEN) (XEN) 0f0060c0c748 (XEN)c305 (XEN) (XEN) (XEN) (XEN) (XEN) (XEN) Domain 0 crashed: rebooting machine in 5 seconds. Some addresses are solved to: 0x816987c5: efi_memblock_x86_reserve_range at arch/x86/platform/efi/efi.c:393 0x8168a1a4: setup_arch at arch/x86/kernel/setup.c:940 0x81685a3e: setup_command_line at init/main.c:353 0x816854c8: x86_64_start_reservations at arch/x86/kernel/head64.c:194 0x81687442: xen_start_kernel at arch/x86/xen/enlighten.c:1733 I am using Linus tree with latest commit b098d6726bbfb94c06d6e1097466187afddae61f (Linux 3.14-rc8) with my patches applied excluding patch 3. Daniel -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/5] xen: Define EFI related stuff
Define EFI related stuff for Xen. This patch is based on Jan Beulich and Tang Liang work. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com Signed-off-by: Tang Liang liang.t...@oracle.com Signed-off-by: Jan Beulich jbeul...@suse.com --- include/xen/interface/platform.h | 123 +- 1 file changed, 122 insertions(+), 1 deletion(-) diff --git a/include/xen/interface/platform.h b/include/xen/interface/platform.h index f1331e3..bb3dfad 100644 --- a/include/xen/interface/platform.h +++ b/include/xen/interface/platform.h @@ -108,11 +108,113 @@ struct xenpf_platform_quirk { }; DEFINE_GUEST_HANDLE_STRUCT(xenpf_platform_quirk_t); +#define XENPF_efi_runtime_call49 +#define XEN_EFI_get_time 1 +#define XEN_EFI_set_time 2 +#define XEN_EFI_get_wakeup_time 3 +#define XEN_EFI_set_wakeup_time 4 +#define XEN_EFI_get_next_high_monotonic_count 5 +#define XEN_EFI_get_variable 6 +#define XEN_EFI_set_variable 7 +#define XEN_EFI_get_next_variable_name8 +#define XEN_EFI_query_variable_info 9 +#define XEN_EFI_query_capsule_capabilities 10 +#define XEN_EFI_update_capsule 11 + +struct xenpf_efi_runtime_call { + uint32_t function; +/* + * This field is generally used for per sub-function flags (defined + * below), except for the XEN_EFI_get_next_high_monotonic_count case, + * where it holds the single returned value. + */ + uint32_t misc; + unsigned long status; + union { +#define XEN_EFI_GET_TIME_SET_CLEARS_NS 0x0001 + struct { + struct xenpf_efi_time { + uint16_t year; + uint8_t month; + uint8_t day; + uint8_t hour; + uint8_t min; + uint8_t sec; + uint32_t ns; + int16_t tz; + uint8_t daylight; + } time; + uint32_t resolution; + uint32_t accuracy; + } get_time; + + struct xenpf_efi_time set_time; + +#define XEN_EFI_GET_WAKEUP_TIME_ENABLED 0x0001 +#define XEN_EFI_GET_WAKEUP_TIME_PENDING 0x0002 + struct xenpf_efi_time get_wakeup_time; + +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE 0x0001 +#define XEN_EFI_SET_WAKEUP_TIME_ENABLE_ONLY 0x0002 + struct xenpf_efi_time set_wakeup_time; + +#define XEN_EFI_VARIABLE_NON_VOLATILE 0x0001 +#define XEN_EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0002 +#define XEN_EFI_VARIABLE_RUNTIME_ACCESS 0x0004 + struct { + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + unsigned long size; + GUEST_HANDLE(void) data; + struct xenpf_efi_guid { + uint32_t data1; + uint16_t data2; + uint16_t data3; + uint8_t data4[8]; + } vendor_guid; + } get_variable, set_variable; + + struct { + unsigned long size; + GUEST_HANDLE(void) name; /* UCS-2/UTF-16 string */ + struct xenpf_efi_guid vendor_guid; + } get_next_variable_name; + + struct { + uint32_t attr; + uint64_t max_store_size; + uint64_t remain_store_size; + uint64_t max_size; + } query_variable_info; + + struct { + GUEST_HANDLE(void) capsule_header_array; + unsigned long capsule_count; + uint64_t max_capsule_size; + unsigned int reset_type; + } query_capsule_capabilities; + + struct { + GUEST_HANDLE(void) capsule_header_array; + unsigned long capsule_count; + uint64_t sg_list; /* machine address */ + } update_capsule; + } u; +}; +DEFINE_GUEST_HANDLE_STRUCT(xenpf_efi_runtime_call); + +#define XEN_FW_EFI_VERSION0 +#define XEN_FW_EFI_CONFIG_TABLE 1 +#define XEN_FW_EFI_VENDOR 2 +#define XEN_FW_EFI_MEM_INFO 3 +#define XEN_FW_EFI_RT_VERSION 4 + #define XENPF_firmware_info 50 #define XEN_FW_DISK_INFO 1 /* from int 13 AH=08/41/48 */ #define XEN_FW_DISK_MBR_SIGNATURE 2 /* from MBR offset 0x1b8 */ #define XEN_FW_VBEDDC_INFO3 /* from int 10 AX=4f15 */ +#define XEN_FW_EFI_INFO 4 /* from EFI */ #define XEN_FW_KBD_SHIFT_FLAGS5 /* Int16, Fn02: Get keyboard shift flags. */ + struct xenpf_firmware_info { /* IN variables. */ uint32_t type; @@ -143,7 +245,25 @@ struct xenpf_firmware_info { /* must refer to 128-byte buffer */ GUEST_HANDLE(uchar) edid; } vbeddc_info; /* XEN_FW_VBEDDC_INFO */ - + union xenpf_efi_info
[PATCH v3 3/5] x86: Call efi_memblock_x86_reserve_range() on native EFI platform only
Call efi_memblock_x86_reserve_range() on native EFI platform only. This is not needed and even it should not be called on platforms which wraps EFI infrastructure like Xen. Signed-off-by: Daniel Kiper daniel.ki...@oracle.com --- arch/x86/kernel/setup.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index ce72964..992b67a 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -933,7 +933,7 @@ void __init setup_arch(char **cmdline_p) set_bit(EFI_64BIT, x86_efi_facility); } - if (efi_enabled(EFI_BOOT)) + if (!strncmp((char *)boot_params.efi_info.efi_loader_signature, EL, 2)) efi_memblock_x86_reserve_range(); #endif -- 1.7.10.4 -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] xen: Add EFI support
Hey, This patch series adds EFI support for Xen dom0 guests. It is based on Jan Beulich and Tang Liang work. I was trying to take into account all previous comments, however, if I missed something sorry for that. Additionally, I attempted to leave all credits as is and put SOB in relevant places. If I made any mistake drop me a line and I will fix it in next version. Feel free to play with it. I am looking forward for your comments. Daniel arch/ia64/kernel/efi.c | 30 -- arch/x86/kernel/setup.c |2 +- arch/x86/platform/efi/efi.c | 20 ++-- arch/x86/xen/enlighten.c | 10 ++ drivers/firmware/efi/efi.c | 26 ++ drivers/xen/Kconfig |3 + drivers/xen/Makefile |1 + drivers/xen/efi.c| 425 +++ include/linux/efi.h | 10 ++ include/xen/interface/platform.h | 123 +++- 10 files changed, 633 insertions(+), 17 deletions(-) Daniel Kiper (5): efi: Add efi_init_ops variable efi: Export arch_tables variable x86: Call efi_memblock_x86_reserve_range() on native EFI platform only xen: Define EFI related stuff xen: Put EFI machinery in place -- To unsubscribe from this list: send the line unsubscribe linux-efi in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html