Re: [PATCH 00/18] x86: multiboot2 protocol support
On Fri, Jan 30, 2015 at 06:54:04PM +0100, Daniel Kiper wrote: Hi, I am sending, long awaited, first version of multiboot2 protocol support for legacy BIOS and EFI platforms. New version with relocatable Xen early boot code is under tests now. I hope that I will release new version in 2-3 weeks after polishing some details and taking into account your comments. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 08/18] efi: build xen.gz with EFI code
On Mon, Mar 02, 2015 at 04:14:22PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: --- a/xen/arch/x86/efi/Makefile +++ b/xen/arch/x86/efi/Makefile @@ -1,14 +1,14 @@ CFLAGS += -fshort-wchar -obj-y += stub.o +obj-y += boot.o +obj-y += compat.o +obj-y += runtime.o So how is this going to work with a compiler not new enough to support the MS ABI (via function attribute)? README says: First, there are a number of prerequisites for building a Xen source release. Make sure you have all the following installed, either by visiting the project webpage or installing a pre-built package provided by your OS distributor: * GCC v4.1 or later IIRC, MS ABI is supported starting from GCC v4.0. Should we care about people using compilers older than we require? Though another question arises. Is it possible to build GCC v4.0 and newer with MS ABI disabled? If yes then we should take into account that thing. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 04/18] xen/x86: add multiboot2 protocol support
On 27.03.15 at 11:56, daniel.ki...@oracle.com wrote:
On Fri, Feb 20, 2015 at 04:06:26PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -1,6 +1,7 @@
obj-bin-y += head.o
-RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h
$(BASEDIR)/include/xen/multiboot.h
+RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h
$(BASEDIR)/include/xen/compiler.h \
+ $(BASEDIR)/include/xen/multiboot.h
$(BASEDIR)/include/xen/multiboot2.h
Perhaps we should rather have the compiler generate the
dependencies for us when compiling reloc.o?
Hmmm... I am a bit confused. Here
http://lists.xen.org/archives/html/xen-devel/2014-10/msg02094.html
you told a bit different thing and relevant patch was accepted as commit
c42070df66c9fcedf477959b8371b85aa4ac4933
(x86/boot: fix reloc.S build dependencies). I can try to do what you
suggest, this is not a problem
for me, however, I would like to be sure what is your final decision in that
case.
First of all I wasn't anticipating that this list of dependencies would
further grow. And then I didn't say don't do this, I only pointed
out (albeit maybe a little too implicitly) that this would be a more
complex patch.
+.long MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO
+.long MULTIBOOT2_TAG_TYPE_MMAP
+.Lmultiboot2_info_req_end:
+
+.align MULTIBOOT2_TAG_ALIGN
+.short MULTIBOOT2_HEADER_TAG_MODULE_ALIGN
+.short MULTIBOOT2_HEADER_TAG_REQUIRED
+.long 8 /* Tag size. */
... here and further down you hard-code it. Please be consistent
(and if you go the calculation route, perhaps introduce some
redundancy reducing macro).
I did this deliberately. I calculate size using labels when it is really
needed, e.g. in tags which size depends on number of elements. However,
most tags have strictly defined sizes. So, that is why I dropped labels
and entered simple numbers. Though I agree that it can be improved.
I think that we can define relevant tag structures in multiboot2.h and
generate relevant constants using asm-offsets.c. Is it OK for you?
That would mean exposing stuff to other parts of the tree which
doesn't need to be exposed. I don't see why you can't just do
proper size calculations here.
@@ -81,10 +135,51 @@ __start:
mov %ecx,%es
mov %ecx,%ss
+/* Bootloaders may set multiboot[12].mem_lower to a nonzero value
*/
Above you meet coding style requirements, but here you stop to do
so (ongoing below)? Even if pre-existing neighboring comments aren't
well formed, please don't make the situation worse.
Do you ask about ending fullstops? Am I right?
Yes.
@@ -31,7 +38,16 @@ asm (
);
typedef unsigned int u32;
+typedef unsigned long long u64;
+
+#include ../../../include/xen/compiler.h
Why?
static multiboot_info_t __used *reloc(void *mbi_in, u32 mb_magic)
Because of this -- ^^
And what keeps you from leaving out the static, making the
__used unnecessary?
+
+typedef struct {
+u32 type;
+u32 size;
+u32 mem_lower;
+u32 mem_upper;
+} multiboot2_tag_basic_meminfo_t;
+
+typedef struct __packed {
Why __packed when all the others aren't?
Ha! This thing was taken from GRUB2 source.
I was asking this question myself many times.
I have not found real explanation for this
but if you wish I can dive into code and
try to find one (if it exists).
Or even better just drop the __packed.
Jan
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 04/18] xen/x86: add multiboot2 protocol support
On Fri, Feb 20, 2015 at 04:06:26PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -1,6 +1,7 @@
obj-bin-y += head.o
-RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h
$(BASEDIR)/include/xen/multiboot.h
+RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h
$(BASEDIR)/include/xen/compiler.h \
+$(BASEDIR)/include/xen/multiboot.h
$(BASEDIR)/include/xen/multiboot2.h
Perhaps we should rather have the compiler generate the
dependencies for us when compiling reloc.o?
Hmmm... I am a bit confused. Here
http://lists.xen.org/archives/html/xen-devel/2014-10/msg02094.html
you told a bit different thing and relevant patch was accepted as commit
c42070df66c9fcedf477959b8371b85aa4ac4933
(x86/boot: fix reloc.S build dependencies). I can try to do what you suggest,
this is not a problem
for me, however, I would like to be sure what is your final decision in that
case.
@@ -32,6 +33,59 @@ ENTRY(start)
.long MULTIBOOT_HEADER_FLAGS
/* Checksum: must be the negated sum of the first two fields. */
.long -(MULTIBOOT_HEADER_MAGIC + MULTIBOOT_HEADER_FLAGS)
+
+/*** MULTIBOOT2 HEADER /
+/* Some ideas are taken from grub-2.00/grub-core/tests/boot/kernel-i386.S
file. */
+.align MULTIBOOT2_HEADER_ALIGN
+
+.Lmultiboot2_header:
+/* Magic number indicating a Multiboot2 header. */
+.long MULTIBOOT2_HEADER_MAGIC
+/* Architecture: i386. */
+.long MULTIBOOT2_ARCHITECTURE_I386
+/* Multiboot2 header length. */
+.long .Lmultiboot2_header_end - .Lmultiboot2_header
+/* Multiboot2 header checksum. */
+.long -(MULTIBOOT2_HEADER_MAGIC + MULTIBOOT2_ARCHITECTURE_I386 +
\
+(.Lmultiboot2_header_end - .Lmultiboot2_header))
So here ...
+/* Multiboot2 tags... */
+.Lmultiboot2_info_req:
+/* Multiboot2 information request tag. */
+.short MULTIBOOT2_HEADER_TAG_INFORMATION_REQUEST
+.short MULTIBOOT2_HEADER_TAG_REQUIRED
+.long .Lmultiboot2_info_req_end - .Lmultiboot2_info_req
.. and here you properly calculate the length, while ...
+.long MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO
+.long MULTIBOOT2_TAG_TYPE_MMAP
+.Lmultiboot2_info_req_end:
+
+.align MULTIBOOT2_TAG_ALIGN
+.short MULTIBOOT2_HEADER_TAG_MODULE_ALIGN
+.short MULTIBOOT2_HEADER_TAG_REQUIRED
+.long 8 /* Tag size. */
... here and further down you hard-code it. Please be consistent
(and if you go the calculation route, perhaps introduce some
redundancy reducing macro).
I did this deliberately. I calculate size using labels when it is really
needed, e.g. in tags which size depends on number of elements. However,
most tags have strictly defined sizes. So, that is why I dropped labels
and entered simple numbers. Though I agree that it can be improved.
I think that we can define relevant tag structures in multiboot2.h and
generate relevant constants using asm-offsets.c. Is it OK for you?
+/* Console flags tag. */
+.align MULTIBOOT2_TAG_ALIGN
+.short MULTIBOOT2_HEADER_TAG_CONSOLE_FLAGS
+.short MULTIBOOT2_HEADER_TAG_OPTIONAL
+.long 12 /* Tag size. */
+.long MULTIBOOT2_CONSOLE_FLAGS_EGA_TEXT_SUPPORTED
+
+/* Framebuffer tag. */
+.align MULTIBOOT2_TAG_ALIGN
+.short MULTIBOOT2_HEADER_TAG_FRAMEBUFFER
+.short MULTIBOOT2_HEADER_TAG_OPTIONAL
+.long 20 /* Tag size. */
+.long 0 /* Number of the columns - no preference. */
+.long 0 /* Number of the lines - no preference. */
+.long 0 /* Number of bits per pixel - no preference. */
+
+/* Multiboot2 header end tag. */
+.align MULTIBOOT2_TAG_ALIGN
+.short MULTIBOOT2_HEADER_TAG_END
+.short 0
+.long 8 /* Tag size. */
+.Lmultiboot2_header_end:
.section .init.rodata, a, @progbits
.align 4
@@ -81,10 +135,51 @@ __start:
mov %ecx,%es
mov %ecx,%ss
+/* Bootloaders may set multiboot[12].mem_lower to a nonzero value
*/
Above you meet coding style requirements, but here you stop to do
so (ongoing below)? Even if pre-existing neighboring comments aren't
well formed, please don't make the situation worse.
Do you ask about ending fullstops? Am I right?
Also please don't say 12 here - I first even mistook this as an array
index, but you seem to mean 1 or 2. Please use {1,2} instead.
OK.
+xor %edx,%edx
+
/* Check for Multiboot bootloader */
cmp $MULTIBOOT_BOOTLOADER_MAGIC,%eax
-jne not_multiboot
+je multiboot1_proto
+/* Check for Multiboot2 bootloader */
+cmp
Re: [PATCH 17/18] x86/efi: create new early memory allocator
On Mon, Mar 02, 2015 at 05:23:49PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -103,9 +103,35 @@ static void __init relocate_trampoline(unsigned long
phys)
*(u16 *)(*trampoline_ptr + (long)trampoline_ptr) = phys 4;
}
+#define __MALLOC_SIZE 65536
+
+static char __malloc_mem[__MALLOC_SIZE];
+static char *__malloc_free = NULL;
+
+static void __init *__malloc(size_t size)
Please do away with all these prefixing underscores.
+{
+void *ptr;
+
+/*
+ * Init __malloc_free on runtime. Static initialization
+ * will not work because it puts virtual address there.
+ */
+if ( __malloc_free == NULL )
+__malloc_free = __malloc_mem;
+
+ptr = __malloc_free;
+
+__malloc_free += size;
+
+if ( __malloc_free - __malloc_mem sizeof(__malloc_mem) )
+blexit(LOut of static memory\r\n);
+
+return ptr;
+}
You're ignoring alignment requirements here altogether.
I can understand why __malloc_mem should be let's say page aligned
but I am not sure why we should care here about alignment inside
of __malloc_mem array like...
@@ -192,12 +218,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
static void *__init efi_arch_allocate_mmap_buffer(UINTN *map_size)
{
-place_string(mbi.mem_upper, NULL);
-mbi.mem_upper -= *map_size;
-mbi.mem_upper = -__alignof__(EFI_MEMORY_DESCRIPTOR);
...here...
-if ( mbi.mem_upper xen_phys_start )
-return NULL;
-return (void *)(long)mbi.mem_upper;
+return __malloc(*map_size);
}
Which then even suggests that _if_ we go this route, this could be
shared with ARM (and hence become common code again).
So, go or no go this route? If not what do you suggest?
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3] Add a module for retrieving SMBIOS information
On 23.03.2015 03:01, David Michael wrote:
The following are two use cases from Rajat Jainrajatj...@juniper.net:
1) We have a board that boots Linux and this board itself can be plugged into one of
different chassis types. We need to pass different parameters to the kernel based on the
CHASSIS_TYPE information that is passed by the bios in the DMI / SMBIOS
tables.
2) We may have a USB stick that can go into multiple boards, and the exact
kernel to be loaded depends on the machine information (PRODUCT_NAME etc)
passed via the DMI.
---
Changes since v2:
* Switched to language like string set and SMBIOS structure to use
terminology consistent with the specification.
To address points suggested by Andrei Borzenkov:
* Dropped ChangeLog text from the commit message.
* Changed to long options in the documentation.
* Renamed --variable to --set.
* Exit with an error when given out-of-range option values instead of
resetting the option.
* Functions were added to retrieve data types that should avoid
alignment and endianness issues.
* Force string set searches to terminate at end of table.
* Each data type now has a separate size/bounds test.
* Error messages have better explanations.
docs/grub.texi | 65 +++
grub-core/Makefile.core.def | 7 +
grub-core/commands/smbios.c | 445
3 files changed, 517 insertions(+)
create mode 100644 grub-core/commands/smbios.c
diff --git a/docs/grub.texi b/docs/grub.texi
index 46b9e7f..73f0909 100644
--- a/docs/grub.texi
+++ b/docs/grub.texi
@@ -3830,6 +3830,7 @@ you forget a command, you can run the command
@command{help}
* sha256sum:: Compute or check SHA256 hash
* sha512sum:: Compute or check SHA512 hash
* sleep:: Wait for a specified number of seconds
+* smbios:: Retrieve SMBIOS information
* source:: Read a configuration file in same context
* test::Check file types and compare values
* true::Do nothing, successfully
@@ -4944,6 +4945,70 @@ if timeout was interrupted by @key{ESC}.
@end deffn
+@node smbios
+@subsection smbios
+
+@deffn Command smbios @
+ [@option{--type} @var{type}] @
+ [@option{--handle} @var{handle}] @
+ [@option{--match} @var{match}] @
+ [(@option{--get-byte} | @option{--get-word} | @option{--get-dword} | @
+ @option{--get-qword} | @option{--get-string}) @
+ @var{offset} [@option{--set} @var{variable}]]
+Retrieve SMBIOS information. This command is only available on x86 and EFI
+systems.
+
Could we avoid exposing such details as offset in structures? It's way
too technical perhaps something like
smbios [--handle=HANDLE|--instance=N] [--set VAR] [TABLE.VARNAME]
where table and varname will be string identifiers and would be
translated using dictionaries. TABLE.VARNAME as whole string can be the
key, that format is just to avoid conflicts when similar fields are
exported in different tables. You can just put the vars you care about
in the dictionary.
+/*
+ * In order for any of this module to function, it needs to find an entry point
+ * structure. This returns a pointer to a struct that identifies the fields of
+ * the EPS, or NULL if it cannot be located.
+ */
+static const struct grub_smbios_eps *
+grub_smbios_locate_eps (void)
+{
+#ifdef GRUB_MACHINE_EFI
+ static const grub_efi_guid_t smbios_guid = GRUB_EFI_SMBIOS_TABLE_GUID;
+ const grub_efi_system_table_t *st = grub_efi_system_table;
+ const grub_efi_configuration_table_t *t = st-configuration_table;
+ unsigned int i;
+
+ for (i = 0; i st-num_table_entries; i++)
+if (grub_memcmp (smbios_guid, t-vendor_guid, sizeof (smbios_guid)) == 0)
+ {
+grub_dprintf (smbios, Found entry point structure at %p\n,
+ t-vendor_table);
+return (const struct grub_smbios_eps *)t-vendor_table;
+ }
+else
+ t++;
+#else
+ grub_uint8_t *ptr;
+
+ for (ptr = (grub_uint8_t *)0x000F;
+ ptr (grub_uint8_t *)0x0010;
+ ptr += 16)
+if (grub_memcmp (ptr, _SM_, 4) == 0
+ grub_byte_checksum (ptr, ptr[5]) == 0)
+ {
+grub_dprintf (smbios, Found entry point structure at %p\n, ptr);
+return (const struct grub_smbios_eps *)ptr;
+ }
+#endif
+
This is already present in grub-core/efiemu/*/cfgtables.c. It should
rather be moved to separate file than duplicated.
+/*
+ * Given a pointer to an SMBIOS structure, return the unsigned little-endian
+ * value of the requested number of bytes. These functions avoid alignment and
+ * endianness issues.
+ */
+static inline grub_uint8_t
+grub_smbios_get_byte (const grub_uint8_t *structure, grub_uint8_t offset)
+{
+ return structure[offset];
+}
+
+static inline grub_uint16_t
+grub_smbios_get_word (const grub_uint8_t *structure, grub_uint8_t offset)
+{
+ return (grub_uint16_t)(grub_smbios_get_byte (structure,
Re: [PATCH 16/18] efi: create efi_exit_boot()
On Mon, Mar 02, 2015 at 04:45:47PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: ..which gets memory map and calls ExitBootServices(). We need this to support multiboot2 protocol on EFI platforms. Patches from 9 up to here all make sense on the basis that patch 18 does and assuming that you really need all this code moved out to separate functions. How much different is efi_multiboot2() introduced in #18 from what is left of efi_start() at this point? I.e. is splitting out More or less efi_multiboot2() does not parse command line and do not load modules itself as efi_start() does. all of this code really needed? I think that it is worth doing. First of all efi_start() is huge and its analysis is very difficult right now. So, splitting code into smaller chunks will improve readability a lot (I am still thinking about extracting command line parser and module loader from efi_start() even if both functions will be used only in efi_start(); this way we will have very simple functions doing one thing easy to understand). Additionally, we create pieces which are very easy to reuse in efi_multiboot2() which is very simple and again easy for analysis. Potentially we can reuse efi_start() in multiboot2 case. However, I prefer to have separate function because this way it is clear that multiboot2 case is different thing then native EFI loader stuff. Additionally, efi_start() is architecture independent and efi_multiboot2() is x86 only and it should live in x86 files. If it is, please don't title all these patches create ... but split out ... or some such - you don't really create the code. Similarly the second sentence above is too imprecise for my taste - we want to re-use this code to support ... would seem more to the point. OK. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 16/18] efi: create efi_exit_boot()
On 27.03.15 at 13:00, daniel.ki...@oracle.com wrote: Additionally, efi_start() is architecture independent and efi_multiboot2() is x86 only and it should live in x86 files. Is that really the case? Looking at the grub2 sources I see support for other than x86... Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] use stock embedded timestamp 2015-01-01T0000+0000
On 22.03.2015 20:33, Daniel Kahn Gillmor wrote:
Variant timestamps make some grub platforms produce non-deterministic
core images. This makes it difficult to use simple tools to audit the
stability of a system with grub installed.
This patch selects a single timestamp to use for these embedded
timestamps so that the core images will be replicable.
---
util/mkimage.c | 9 ++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/util/mkimage.c b/util/mkimage.c
index 7821dc5..adc1706 100644
--- a/util/mkimage.c
+++ b/util/mkimage.c
@@ -55,6 +55,9 @@
#define TARGET_NO_FIELD 0x
+/* use 2015-01-01T00:00:00+ as a stock timestamp */
+#define STABLE_EMBEDDING_TIMESTAMP 1420070400
+
struct grub_install_image_target_desc
{
const char *dirname;
@@ -1439,7 +1442,7 @@ grub_install_generate_image (const char *dir, const char
*prefix,
c-machine = grub_host_to_target16 (image_target-pe_target);
c-num_sections = grub_host_to_target16 (4);
- c-time = grub_host_to_target32 (time (0));
+ c-time = grub_host_to_target32 (STABLE_EMBEDDING_TIMESTAMP);
c-characteristics = grub_host_to_target16 (GRUB_PE32_EXECUTABLE_IMAGE
|
GRUB_PE32_LINE_NUMS_STRIPPED
|
((image_target-voidp_sizeof == 4)
@@ -1782,7 +1785,7 @@ grub_install_generate_image (const char *dir, const char
*prefix,
memset (hdr, 0, sizeof (*hdr));
hdr-ih_magic = grub_cpu_to_be32_compile_time (GRUB_UBOOT_IH_MAGIC);
- hdr-ih_time = grub_cpu_to_be32 (time (0));
+ hdr-ih_time = grub_cpu_to_be32 (STABLE_EMBEDDING_TIMESTAMP);
hdr-ih_size = grub_cpu_to_be32 (core_size);
hdr-ih_load = grub_cpu_to_be32 (image_target-link_addr);
hdr-ih_ep = grub_cpu_to_be32 (image_target-link_addr);
@@ -1856,7 +1859,7 @@ grub_install_generate_image (const char *dir, const char
*prefix,
head-magic = image_target-bigendian ? grub_host_to_target16 (0x160)
: grub_host_to_target16 (0x166);
head-nsec = grub_host_to_target16 (1);
- head-time = grub_host_to_target32 (0);
+ head-time = grub_host_to_target32 (STABLE_EMBEDDING_TIMESTAMP);
I dropped this hunk as it's just changing one const to another.
head-opt = grub_host_to_target16 (0x38);
head-flags = image_target-bigendian
? grub_host_to_target16 (0x207)
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 04/18] xen/x86: add multiboot2 protocol support
On 27.03.15 at 13:22, daniel.ki...@oracle.com wrote:
On Fri, Mar 27, 2015 at 11:20:10AM +, Jan Beulich wrote:
On 27.03.15 at 11:56, daniel.ki...@oracle.com wrote:
On Fri, Feb 20, 2015 at 04:06:26PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
@@ -31,7 +38,16 @@ asm (
);
typedef unsigned int u32;
+typedef unsigned long long u64;
+
+#include ../../../include/xen/compiler.h
Why?
static multiboot_info_t __used *reloc(void *mbi_in, u32 mb_magic)
Because of this -- ^^
And what keeps you from leaving out the static, making the
__used unnecessary?
This func is clearly static. Why do not mark it as is and use __used.
What is wrong with that?
#include-s of the above kind generally indicate badness, so we
should try to limit them to the absolute minimum.
+
+typedef struct {
+u32 type;
+u32 size;
+u32 mem_lower;
+u32 mem_upper;
+} multiboot2_tag_basic_meminfo_t;
+
+typedef struct __packed {
Why __packed when all the others aren't?
Ha! This thing was taken from GRUB2 source.
I was asking this question myself many times.
I have not found real explanation for this
but if you wish I can dive into code and
try to find one (if it exists).
Or even better just drop the __packed.
Should not we be in line with GRUB2 source?
Even it sounds strange. Anyway, I will check
GRUB2 source and maybe we can also remove __packed
from it. This way everything will be OK.
Now that's the right course of action.
Jan
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 08/18] efi: build xen.gz with EFI code
On 27.03.15 at 12:14, daniel.ki...@oracle.com wrote: IIRC, MS ABI is supported starting from GCC v4.0. Where did you find that? From all I know __attribute__((__ms_abi__)) is being supported only by 4.5 and newer. The mere support of the MS ABI via command line option doesn't help us, as we need to be able to mix ABIs within a single source file. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 08/18] efi: build xen.gz with EFI code
On 27/03/15 11:46, Jan Beulich wrote: On 27.03.15 at 12:14, daniel.ki...@oracle.com wrote: IIRC, MS ABI is supported starting from GCC v4.0. Where did you find that? From all I know __attribute__((__ms_abi__)) is being supported only by 4.5 and newer. The mere support of the MS ABI via command line option doesn't help us, as we need to be able to mix ABIs within a single source file. Jan As I have indicated elsewhere (but can't seem to locate - it was a while ago), I think it is perfectly reasonable to have a CONFIG_EFI which is only enabled if the makefile detects GCC = 4.5 That way, older build environments don't get EFI support, while newer ones do, and newer systems can safely use __attribute__((__ms_abi__)) to make a mixed abi binary. ~Andrew ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 04/18] xen/x86: add multiboot2 protocol support
On Fri, Mar 27, 2015 at 11:20:10AM +, Jan Beulich wrote:
On 27.03.15 at 11:56, daniel.ki...@oracle.com wrote:
On Fri, Feb 20, 2015 at 04:06:26PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
--- a/xen/arch/x86/boot/Makefile
+++ b/xen/arch/x86/boot/Makefile
@@ -1,6 +1,7 @@
obj-bin-y += head.o
-RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h
$(BASEDIR)/include/xen/multiboot.h
+RELOC_DEPS = $(BASEDIR)/include/asm-x86/config.h
$(BASEDIR)/include/xen/compiler.h \
+ $(BASEDIR)/include/xen/multiboot.h
$(BASEDIR)/include/xen/multiboot2.h
Perhaps we should rather have the compiler generate the
dependencies for us when compiling reloc.o?
Hmmm... I am a bit confused. Here
http://lists.xen.org/archives/html/xen-devel/2014-10/msg02094.html
you told a bit different thing and relevant patch was accepted as commit
c42070df66c9fcedf477959b8371b85aa4ac4933
(x86/boot: fix reloc.S build dependencies). I can try to do what you
suggest, this is not a problem
for me, however, I would like to be sure what is your final decision in that
case.
First of all I wasn't anticipating that this list of dependencies would
further grow. And then I didn't say don't do this, I only pointed
out (albeit maybe a little too implicitly) that this would be a more
complex patch.
So, I understand this as Go for it!.
+.long MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO
+.long MULTIBOOT2_TAG_TYPE_MMAP
+.Lmultiboot2_info_req_end:
+
+.align MULTIBOOT2_TAG_ALIGN
+.short MULTIBOOT2_HEADER_TAG_MODULE_ALIGN
+.short MULTIBOOT2_HEADER_TAG_REQUIRED
+.long 8 /* Tag size. */
... here and further down you hard-code it. Please be consistent
(and if you go the calculation route, perhaps introduce some
redundancy reducing macro).
I did this deliberately. I calculate size using labels when it is really
needed, e.g. in tags which size depends on number of elements. However,
most tags have strictly defined sizes. So, that is why I dropped labels
and entered simple numbers. Though I agree that it can be improved.
I think that we can define relevant tag structures in multiboot2.h and
generate relevant constants using asm-offsets.c. Is it OK for you?
That would mean exposing stuff to other parts of the tree which
doesn't need to be exposed. I don't see why you can't just do
proper size calculations here.
OK, I will do this as you wish.
@@ -81,10 +135,51 @@ __start:
mov %ecx,%es
mov %ecx,%ss
+/* Bootloaders may set multiboot[12].mem_lower to a nonzero
value */
Above you meet coding style requirements, but here you stop to do
so (ongoing below)? Even if pre-existing neighboring comments aren't
well formed, please don't make the situation worse.
Do you ask about ending fullstops? Am I right?
Yes.
@@ -31,7 +38,16 @@ asm (
);
typedef unsigned int u32;
+typedef unsigned long long u64;
+
+#include ../../../include/xen/compiler.h
Why?
static multiboot_info_t __used *reloc(void *mbi_in, u32 mb_magic)
Because of this -- ^^
And what keeps you from leaving out the static, making the
__used unnecessary?
This func is clearly static. Why do not mark it as is and use __used.
What is wrong with that?
+
+typedef struct {
+u32 type;
+u32 size;
+u32 mem_lower;
+u32 mem_upper;
+} multiboot2_tag_basic_meminfo_t;
+
+typedef struct __packed {
Why __packed when all the others aren't?
Ha! This thing was taken from GRUB2 source.
I was asking this question myself many times.
I have not found real explanation for this
but if you wish I can dive into code and
try to find one (if it exists).
Or even better just drop the __packed.
Should not we be in line with GRUB2 source?
Even it sounds strange. Anyway, I will check
GRUB2 source and maybe we can also remove __packed
from it. This way everything will be OK.
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 16/18] efi: create efi_exit_boot()
On Fri, Mar 27, 2015 at 12:10:56PM +, Jan Beulich wrote: On 27.03.15 at 13:00, daniel.ki...@oracle.com wrote: Additionally, efi_start() is architecture independent and efi_multiboot2() is x86 only and it should live in x86 files. Is that really the case? Looking at the grub2 sources I see support for other than x86... Well... In theory multiboot protocol was designed as arch independet. However, docs define more precisely stuff for i386 and MIPS (multiboot2 protocol only). As I know we do not care about MIPS. Additionally, so called muliboot protocol for ARM is completely different thing then multiboot protocols mentioned above (it is a mixture of EFI native loader with DT). So, it looks that from our point of view efi_multiboot2() is x86 only stuff (right now, I am not fortune teller, so, I am not able to tell what happens in the future ;-))) ). Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: @@ -94,6 +111,17 @@ ENTRY(start) gdt_boot_descr: .word 6*8-1 .long sym_phys(trampoline_gdt) +.long 0 /* Needed for 64-bit lgdt */ + +cs32_switch_addr: +.long sym_phys(cs32_switch) +.long BOOT_CS32 + +efi_st: +.quad 0 + +efi_ih: +.quad 0 .Lbad_cpu_msg: .asciz ERR: Not a 64-bit CPU! .Lbad_ldr_msg: .asciz ERR: Not a Multiboot bootloader! @@ -124,6 +152,133 @@ print_err: .Lhalt: hlt jmp .Lhalt +.code64 + +__efi64_start: +cld + +/* Bootloaders may set multiboot[12].mem_lower to a nonzero value */ +xor %edx,%edx + +/* Check for Multiboot2 bootloader */ +cmp $MULTIBOOT2_BOOTLOADER_MAGIC,%eax +je efi_multiboot2_proto + +jmp not_multiboot + +efi_multiboot2_proto: jne not_multiboot (and efi_multiboot2_proto dropped altogether) +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. + +0: +/* Get mem_lower from Multiboot2 information */ +cmpl$MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO,(%ecx) +jne 1f + +mov MB2_mem_lower(%ecx),%edx +jmp 4f + +1: +/* Get EFI SystemTable address from Multiboot2 information */ +cmpl$MULTIBOOT2_TAG_TYPE_EFI64,(%ecx) +jne 2f + +lea MB2_efi64_st(%ecx),%esi +lea efi_st(%rip),%edi +movsq A simple pair of mov-s please, assuming all of this really needs to be done in assembly in the first place. Also please use .Lname labels in this assembly coded switch statement to ease reading. + +/* Do not go into real mode on EFI platform */ +movb$1,skip_realmode(%rip) + +jmp 4f + +2: +/* Get EFI ImageHandle address from Multiboot2 information */ +cmpl$MULTIBOOT2_TAG_TYPE_EFI64_IH,(%ecx) +jne 3f + +lea MB2_efi64_ih(%ecx),%esi +lea efi_ih(%rip),%edi +movsq +jmp 4f + +3: +/* Is it the end of Multiboot2 information? */ +cmpl$MULTIBOOT2_TAG_TYPE_END,(%ecx) +je run_bs + +4: Please switch the order so that 2 can fall through into 4 (and you then won't need the run_bs label, which otherwise should also becom .Lrun_bs). +/* Go to next Multiboot2 information tag */ +add MB2_tag_size(%ecx),%ecx +add $(MULTIBOOT2_TAG_ALIGN-1),%ecx +and $~(MULTIBOOT2_TAG_ALIGN-1),%ecx +jmp 0b + +run_bs: +push%rax +push%rdx + +/* Initialize BSS (no nasty surprises!) */ +lea __bss_start(%rip),%rdi +lea _end(%rip),%rcx +sub %rdi,%rcx +xor %rax,%rax +rep stosb rep stosb + +mov efi_ih(%rip),%rdi /* EFI ImageHandle */ +mov efi_st(%rip),%rsi /* EFI SystemTable */ +callefi_multiboot2 With efi_multiboot2 being a C function it really looks like much of the above doesn't need to be done in assembly. Potentially make sense. I will try to do that. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 16/18] efi: create efi_exit_boot()
On Fri, 2015-03-27 at 13:43 +0100, Daniel Kiper wrote: On Fri, Mar 27, 2015 at 12:10:56PM +, Jan Beulich wrote: On 27.03.15 at 13:00, daniel.ki...@oracle.com wrote: Additionally, efi_start() is architecture independent and efi_multiboot2() is x86 only and it should live in x86 files. Is that really the case? Looking at the grub2 sources I see support for other than x86... Well... In theory multiboot protocol was designed as arch independet. However, docs define more precisely stuff for i386 and MIPS (multiboot2 protocol only). As I know we do not care about MIPS. Additionally, so called muliboot protocol for ARM is completely different thing then multiboot protocols mentioned above The ARM multiboot DT thing fits into the multiboot1 namespace, since it is unlikely there will ever be an ARM multiboot1. It's possible that someone might spec and implement multiboot2 for ARM as well, although whether than has any impact on the comments made in this threadlet I've no idea.. Ian. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 17/18] x86/efi: create new early memory allocator
On 27.03.15 at 13:57, daniel.ki...@oracle.com wrote:
On Mon, Mar 02, 2015 at 05:23:49PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
+{
+void *ptr;
+
+/*
+ * Init __malloc_free on runtime. Static initialization
+ * will not work because it puts virtual address there.
+ */
+if ( __malloc_free == NULL )
+__malloc_free = __malloc_mem;
+
+ptr = __malloc_free;
+
+__malloc_free += size;
+
+if ( __malloc_free - __malloc_mem sizeof(__malloc_mem) )
+blexit(LOut of static memory\r\n);
+
+return ptr;
+}
You're ignoring alignment requirements here altogether.
I can understand why __malloc_mem should be let's say page aligned
but I am not sure why we should care here about alignment inside
of __malloc_mem array like...
@@ -192,12 +218,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
static void *__init efi_arch_allocate_mmap_buffer(UINTN *map_size)
{
-place_string(mbi.mem_upper, NULL);
-mbi.mem_upper -= *map_size;
-mbi.mem_upper = -__alignof__(EFI_MEMORY_DESCRIPTOR);
...here...
Specifically with the later mentioned potential for sharing this with
ARM I think you have to make sure that things are suitably aligned,
or else you cause aborts.
-if ( mbi.mem_upper xen_phys_start )
-return NULL;
-return (void *)(long)mbi.mem_upper;
+return __malloc(*map_size);
}
Which then even suggests that _if_ we go this route, this could be
shared with ARM (and hence become common code again).
So, go or no go this route?
As long as it's being done properly, I'm not wildly opposed.
Jan
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3] Add a module for retrieving SMBIOS information
В Fri, 27 Mar 2015 13:59:06 +0100 Vladimir 'φ-coder/phcoder' Serbinenko phco...@gmail.com пишет: Could we avoid exposing such details as offset in structures? It's way too technical perhaps something like smbios [--handle=HANDLE|--instance=N] [--set VAR] [TABLE.VARNAME] where table and varname will be string identifiers and would be translated using dictionaries. TABLE.VARNAME as whole string can be the key, that format is just to avoid conflicts when similar fields are exported in different tables. You can just put the vars you care about in the dictionary. Do you mean something like BIOS Information.BIOS Characteristics Extension Bytes? Because otherwise where these names should come from and how users will know them? SMBIOS at the end is defined in terms of structure offsets. I suppose that any user of this command will know them. Either we will limit ourselves to known names documented in grub manual, but even then documentation will have to say foo.bar refers to table bar at offset XX. Not sure how is it better. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. seem the right approach. [...] --- a/xen/include/xen/efi.h +++ b/xen/include/xen/efi.h @@ -5,7 +5,11 @@ #include xen/types.h #endif -extern const bool_t efi_enabled; +/* If true then Xen runs on EFI platform. */ +extern bool_t efi_platform; + +/* If true then Xen was loaded by native EFI loader as PE application. */ +extern bool_t efi_loader; I don't see why you're losing the constness. Both of them could be set during runtime. Please look above for more details. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On 27.03.15 at 14:06, daniel.ki...@oracle.com wrote: On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. See the 32-bit register used for addressing here (and in many more places)? Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On 27/03/15 13:43, Jan Beulich wrote: On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. I suppose built with efi support is known because of CONFIG_EFI or not, and doesn't need a variable. However, booted legacy vs booted EFI does need distinguishing at runtime, as either is possible. ~Andrew ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On 27.03.15 at 14:53, andrew.coop...@citrix.com wrote: On 27/03/15 13:43, Jan Beulich wrote: On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. I suppose built with efi support is known because of CONFIG_EFI or not, and doesn't need a variable. However, booted legacy vs booted EFI does need distinguishing at runtime, as either is possible. Right, but that's what efi_enabled is supposed to express after the change; there's no need to express built with EFI support. It just so happens that right now, without all these changes, built-with-EFI-support == runs-on-EFI. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Fri, Mar 27, 2015 at 02:04:22PM +, Jan Beulich wrote: On 27.03.15 at 14:53, andrew.coop...@citrix.com wrote: On 27/03/15 13:43, Jan Beulich wrote: On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. I suppose built with efi support is known because of CONFIG_EFI or not, and doesn't need a variable. However, booted legacy vs booted EFI does need distinguishing at runtime, as either is possible. Right, but that's what efi_enabled is supposed to express after the change; there's no need to express built with EFI support. It just so happens that right now, without all these changes, built-with-EFI-support == runs-on-EFI. Then how about 'efi_booted' as the variable name. -- Len Sorensen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On Fri, Mar 27, 2015 at 02:19:44PM +, Jan Beulich wrote: On 27.03.15 at 15:09, lsore...@csclub.uwaterloo.ca wrote: On Fri, Mar 27, 2015 at 02:04:22PM +, Jan Beulich wrote: On 27.03.15 at 14:53, andrew.coop...@citrix.com wrote: On 27/03/15 13:43, Jan Beulich wrote: On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. I suppose built with efi support is known because of CONFIG_EFI or not, and doesn't need a variable. However, booted legacy vs booted EFI does need distinguishing at runtime, as either is possible. Right, but that's what efi_enabled is supposed to express after the change; there's no need to express built with EFI support. It just so happens that right now, without all these changes, built-with-EFI-support == runs-on-EFI. Then how about 'efi_booted' as the variable name. Why should we rename a variable that's perfectly suitable for the purposes we have? Even more so that we don't just want to express that we were booted from EFI, but also that we're running in a respective environment, including using tables coming from EFI and using runtime services (unless specifically disabled). If anything we could follow Linux and make efi_enabled a bit mask. OK, so it isn't just to tell that you booted from EFI. -- Len Sorensen ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On 27.03.15 at 15:26, daniel.ki...@oracle.com wrote: On Fri, Mar 27, 2015 at 01:36:32PM +, Jan Beulich wrote: On 27.03.15 at 14:06, daniel.ki...@oracle.com wrote: On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. See the 32-bit register used for addressing here (and in many more places)? This is what I expected but I was confused because you were referring only here to this problem. Anyway, is it possible to do this in different way? Should we care if image is always loaded at 0x10 right now? Even with Xen early boot code being relocatable loader could not load image higher than 0x - 14 MiB. I don't understand what you're alluding to. Just use 64-bit registers for memory accesses and LEAs, and be done. This will result in smaller code as a benefit. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [Xen-devel] [PATCH 05/18] efi: split efi_enabled to efi_platform and efi_loader
On 27.03.15 at 15:09, lsore...@csclub.uwaterloo.ca wrote: On Fri, Mar 27, 2015 at 02:04:22PM +, Jan Beulich wrote: On 27.03.15 at 14:53, andrew.coop...@citrix.com wrote: On 27/03/15 13:43, Jan Beulich wrote: On 27.03.15 at 14:32, daniel.ki...@oracle.com wrote: On Fri, Feb 20, 2015 at 04:17:35PM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: We need more fine grained knowledge about EFI environment and check for EFI platform and EFI loader separately to properly support multiboot2 protocol. ... because of ... (i.e. I can't see from the description what the separation is good for). Looking at the comments you placed aside the variables doesn't help me either. In general Xen loaded by this protocol uses memory mappings and loaded modules in simliar way to Xen loaded by multiboot (v1) protocol. Hence, split efi_enabled to efi_platform and efi_loader. And if I'm guessing things right, then introducing efi_loader but leaving efi_enabled alone (only converting where needed) would efi_enabled is not fortunate name for new usage. Currently it means that Xen binary have or does not have EFI support build in. However, if we build in multiboot2 protocol into xen.gz then it means that it can ran on legacy BIOS or EFI platform. So, I think that we should rename efi_enabled to efi_platform because it will mean that Xen runs on EFI platform or not. I disagree here. efi_loader is used to differentiate between EFI native loader and multiboot2 protocol. And I agree here. I suppose built with efi support is known because of CONFIG_EFI or not, and doesn't need a variable. However, booted legacy vs booted EFI does need distinguishing at runtime, as either is possible. Right, but that's what efi_enabled is supposed to express after the change; there's no need to express built with EFI support. It just so happens that right now, without all these changes, built-with-EFI-support == runs-on-EFI. Then how about 'efi_booted' as the variable name. Why should we rename a variable that's perfectly suitable for the purposes we have? Even more so that we don't just want to express that we were booted from EFI, but also that we're running in a respective environment, including using tables coming from EFI and using runtime services (unless specifically disabled). If anything we could follow Linux and make efi_enabled a bit mask. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 17/18] x86/efi: create new early memory allocator
On Fri, Mar 27, 2015 at 01:35:11PM +, Jan Beulich wrote:
On 27.03.15 at 13:57, daniel.ki...@oracle.com wrote:
On Mon, Mar 02, 2015 at 05:23:49PM +, Jan Beulich wrote:
On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote:
+{
+void *ptr;
+
+/*
+ * Init __malloc_free on runtime. Static initialization
+ * will not work because it puts virtual address there.
+ */
+if ( __malloc_free == NULL )
+__malloc_free = __malloc_mem;
+
+ptr = __malloc_free;
+
+__malloc_free += size;
+
+if ( __malloc_free - __malloc_mem sizeof(__malloc_mem) )
+blexit(LOut of static memory\r\n);
+
+return ptr;
+}
You're ignoring alignment requirements here altogether.
I can understand why __malloc_mem should be let's say page aligned
but I am not sure why we should care here about alignment inside
of __malloc_mem array like...
@@ -192,12 +218,7 @@ static void __init
efi_arch_process_memory_map(EFI_SYSTEM_TABLE *SystemTable,
static void *__init efi_arch_allocate_mmap_buffer(UINTN *map_size)
{
-place_string(mbi.mem_upper, NULL);
-mbi.mem_upper -= *map_size;
-mbi.mem_upper = -__alignof__(EFI_MEMORY_DESCRIPTOR);
...here...
Specifically with the later mentioned potential for sharing this with
ARM I think you have to make sure that things are suitably aligned,
or else you cause aborts.
-if ( mbi.mem_upper xen_phys_start )
-return NULL;
-return (void *)(long)mbi.mem_upper;
+return __malloc(*map_size);
}
Which then even suggests that _if_ we go this route, this could be
shared with ARM (and hence become common code again).
So, go or no go this route?
As long as it's being done properly, I'm not wildly opposed.
So, AIUI, general idea is OK but fix all stuff which should be fixed. Right?
Daniel
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On Fri, Mar 27, 2015 at 01:36:32PM +, Jan Beulich wrote: On 27.03.15 at 14:06, daniel.ki...@oracle.com wrote: On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. See the 32-bit register used for addressing here (and in many more places)? This is what I expected but I was confused because you were referring only here to this problem. Anyway, is it possible to do this in different way? Should we care if image is always loaded at 0x10 right now? Even with Xen early boot code being relocatable loader could not load image higher than 0x - 14 MiB. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On Fri, Mar 27, 2015 at 02:34:19PM +, Jan Beulich wrote: On 27.03.15 at 15:26, daniel.ki...@oracle.com wrote: On Fri, Mar 27, 2015 at 01:36:32PM +, Jan Beulich wrote: On 27.03.15 at 14:06, daniel.ki...@oracle.com wrote: On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. See the 32-bit register used for addressing here (and in many more places)? This is what I expected but I was confused because you were referring only here to this problem. Anyway, is it possible to do this in different way? Should we care if image is always loaded at 0x10 right now? Even with Xen early boot code being relocatable loader could not load image higher than 0x - 14 MiB. I don't understand what you're alluding to. Just use 64-bit registers for memory accesses and LEAs, and be done. This will result in smaller code as a benefit. Well... How can I do that here if processor is in 32-bit mode? Maybe, we could that things after switching to 64-bit mode. However, I think this requires separate patch to do these changes. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On 27.03.15 at 15:57, daniel.ki...@oracle.com wrote: On Fri, Mar 27, 2015 at 02:34:19PM +, Jan Beulich wrote: On 27.03.15 at 15:26, daniel.ki...@oracle.com wrote: On Fri, Mar 27, 2015 at 01:36:32PM +, Jan Beulich wrote: On 27.03.15 at 14:06, daniel.ki...@oracle.com wrote: On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. See the 32-bit register used for addressing here (and in many more places)? This is what I expected but I was confused because you were referring only here to this problem. Anyway, is it possible to do this in different way? Should we care if image is always loaded at 0x10 right now? Even with Xen early boot code being relocatable loader could not load image higher than 0x - 14 MiB. I don't understand what you're alluding to. Just use 64-bit registers for memory accesses and LEAs, and be done. This will result in smaller code as a benefit. Well... How can I do that here if processor is in 32-bit mode? Maybe, we could that things after switching to 64-bit mode. However, I think this requires separate patch to do these changes. No, if the processor is in 32-bit mode, then using 32-bit registers is fine of course. But I'm pretty certain I spotted at least some cases where it looked like you used 32-bit registers in 64-bit mode. Jan ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 18/18] x86: add multiboot2 protocol support for EFI platforms
On Fri, Mar 27, 2015 at 03:06:43PM +, Jan Beulich wrote: On 27.03.15 at 15:57, daniel.ki...@oracle.com wrote: On Fri, Mar 27, 2015 at 02:34:19PM +, Jan Beulich wrote: On 27.03.15 at 15:26, daniel.ki...@oracle.com wrote: On Fri, Mar 27, 2015 at 01:36:32PM +, Jan Beulich wrote: On 27.03.15 at 14:06, daniel.ki...@oracle.com wrote: On Tue, Mar 17, 2015 at 10:32:01AM +, Jan Beulich wrote: On 30.01.15 at 18:54, daniel.ki...@oracle.com wrote: +/* Skip Multiboot2 information fixed part */ +lea MB2_fixed_sizeof(%ebx),%ecx Let's please not add more assumptions than necessary about stuff being below 4G. I am not sure what do you mean by that. See the 32-bit register used for addressing here (and in many more places)? This is what I expected but I was confused because you were referring only here to this problem. Anyway, is it possible to do this in different way? Should we care if image is always loaded at 0x10 right now? Even with Xen early boot code being relocatable loader could not load image higher than 0x - 14 MiB. I don't understand what you're alluding to. Just use 64-bit registers for memory accesses and LEAs, and be done. This will result in smaller code as a benefit. Well... How can I do that here if processor is in 32-bit mode? Maybe, we could that things after switching to 64-bit mode. However, I think this requires separate patch to do these changes. No, if the processor is in 32-bit mode, then using 32-bit registers is fine of course. But I'm pretty certain I spotted at least some cases where it looked like you used 32-bit registers in 64-bit mode. OK, I will double check. If I find something then I will fix it. Daniel ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
RE: [PATCH v3] Add a module for retrieving SMBIOS information
[+Arthur]
Hello Folks,
I'm trying to get a binary / raw copy of this patch but could not find one.
David, is it possible to please send us a binary / raw copy of this patch?
Thanks,
Rajat
-Original Message-
From: Andrei Borzenkov [mailto:arvidj...@gmail.com]
Sent: Friday, March 27, 2015 8:24 AM
To: David Michael
Cc: grub-devel@gnu.org; Rajat Jain; Prarit Bhargava; Leif Lindholm; Sanjay
Jain; Raghuraman Thirumalairajan; Stu Grossman
Subject: Re: [PATCH v3] Add a module for retrieving SMBIOS information
В Sun, 22 Mar 2015 22:01:49 -0400
David Michael fedora@gmail.com пишет:
+struct __attribute__ ((packed)) grub_smbios_eps
+ {
+grub_uint8_t anchor[4]; /* _SM_ */
any plans to implement SMBIOS 3.0 (64 bit address) support?
+grub_uint8_t checksum;
+grub_uint8_t length;
+grub_uint8_t version_major;
+grub_uint8_t version_minor;
+grub_uint16_t maximum_structure_size;
+grub_uint8_t revision;
+grub_uint8_t formatted[5];
+struct grub_smbios_ieps intermediate; };
+
+#define eps_table_begin(eps)
+((grub_addr_t)((eps)-intermediate.table_address))
+#define eps_table_end(eps) \
+ ((grub_addr_t)((eps)-intermediate.table_address + \
+ (eps)-intermediate.table_length))
+
To make adding 64 bit SMBIOS easier, may be extract entry point and size
(and other relevant fields) instead of referring to them directly.
Then we'd just need to add additional search for 3.0 entry point and all other
code won't need to be changed - tables themselves remain the same as far
as I can tell.
...
+
+static grub_err_t
+grub_cmd_smbios (grub_extcmd_context_t ctxt,
+ int argc __attribute__ ((unused)),
+ char **argv __attribute__ ((unused))) {
+ struct grub_arg_list *state = ctxt-state;
+
+ grub_int16_t type = -1;
+ grub_int32_t handle = -1;
+ grub_uint16_t match = 0;
+ grub_uint8_t offset = 0;
+
+ grub_int32_t option;
+ const grub_uint8_t *structure;
+ grub_uint8_t accessors;
+ grub_uint8_t i;
+ char buffer[24]; /* 64-bit number - maximum 20 decimal digits */
+ const char *value = buffer;
+
Could we avoid this aliasing? It is extremely confusing to see buffer used
everywhere and then suddenly value in the last line. What is the reason?
+
+ /* Store or print the requested value. */ if (state[8].set)
+{
+ grub_env_set (state[8].arg, value);
+ grub_env_export (state[8].arg);
Why export variable here? It is up to user what to do with it later.
+static const struct grub_arg_option options[] =
+ {
+{type, 't', 0, N_(Match entries with the given type.),
+ N_(type), ARG_TYPE_INT},
+{handle, 'h', 0, N_(Match entries with the given handle.),
+ N_(handle), ARG_TYPE_INT},
+{match, 'm', 0, N_(Select a structure when several match.),
+ N_(match), ARG_TYPE_INT},
+{get-byte, 'b', 0, N_(Get the byte's value at the given offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-word, 'w', 0, N_(Get two bytes' value at the given offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-dword, 'd', 0, N_(Get four bytes' value at the given
offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-qword, 'q', 0, N_(Get eight bytes' value at the given
offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-string, 's', 0, N_(Get the string specified at the given
offset.),
+ N_(offset), ARG_TYPE_INT},
+{set, '\0', 0, N_(Store the value in the given variable
name.),
+ N_(variable), ARG_TYPE_STRING},
+{0, 0, 0, 0, 0, 0}
+ };
One non-trivial structure field that is rather awkward to get otherwise is
UUID.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v3] Add a module for retrieving SMBIOS information
В Sun, 22 Mar 2015 22:01:49 -0400
David Michael fedora@gmail.com пишет:
+struct __attribute__ ((packed)) grub_smbios_eps
+ {
+grub_uint8_t anchor[4]; /* _SM_ */
any plans to implement SMBIOS 3.0 (64 bit address) support?
+grub_uint8_t checksum;
+grub_uint8_t length;
+grub_uint8_t version_major;
+grub_uint8_t version_minor;
+grub_uint16_t maximum_structure_size;
+grub_uint8_t revision;
+grub_uint8_t formatted[5];
+struct grub_smbios_ieps intermediate;
+ };
+
+#define eps_table_begin(eps)
((grub_addr_t)((eps)-intermediate.table_address))
+#define eps_table_end(eps) \
+ ((grub_addr_t)((eps)-intermediate.table_address + \
+ (eps)-intermediate.table_length))
+
To make adding 64 bit SMBIOS easier, may be extract entry point and
size (and other relevant fields) instead of referring to them directly.
Then we'd just need to add additional search for 3.0 entry point and
all other code won't need to be changed - tables themselves remain the
same as far as I can tell.
...
+
+static grub_err_t
+grub_cmd_smbios (grub_extcmd_context_t ctxt,
+ int argc __attribute__ ((unused)),
+ char **argv __attribute__ ((unused)))
+{
+ struct grub_arg_list *state = ctxt-state;
+
+ grub_int16_t type = -1;
+ grub_int32_t handle = -1;
+ grub_uint16_t match = 0;
+ grub_uint8_t offset = 0;
+
+ grub_int32_t option;
+ const grub_uint8_t *structure;
+ grub_uint8_t accessors;
+ grub_uint8_t i;
+ char buffer[24]; /* 64-bit number - maximum 20 decimal digits */
+ const char *value = buffer;
+
Could we avoid this aliasing? It is extremely confusing to see buffer
used everywhere and then suddenly value in the last line. What is the
reason?
+
+ /* Store or print the requested value. */
+ if (state[8].set)
+{
+ grub_env_set (state[8].arg, value);
+ grub_env_export (state[8].arg);
Why export variable here? It is up to user what to do with it later.
+static const struct grub_arg_option options[] =
+ {
+{type, 't', 0, N_(Match entries with the given type.),
+ N_(type), ARG_TYPE_INT},
+{handle, 'h', 0, N_(Match entries with the given handle.),
+ N_(handle), ARG_TYPE_INT},
+{match, 'm', 0, N_(Select a structure when several match.),
+ N_(match), ARG_TYPE_INT},
+{get-byte, 'b', 0, N_(Get the byte's value at the given offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-word, 'w', 0, N_(Get two bytes' value at the given offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-dword, 'd', 0, N_(Get four bytes' value at the given offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-qword, 'q', 0, N_(Get eight bytes' value at the given offset.),
+ N_(offset), ARG_TYPE_INT},
+{get-string, 's', 0, N_(Get the string specified at the given
offset.),
+ N_(offset), ARG_TYPE_INT},
+{set, '\0', 0, N_(Store the value in the given variable name.),
+ N_(variable), ARG_TYPE_STRING},
+{0, 0, 0, 0, 0, 0}
+ };
One non-trivial structure field that is rather awkward to get
otherwise is UUID.
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: Query about GRUB_ENABLE_CRYPTODISK
Please commit this patch
On 10.12.2014 04:25, Andrei Borzenkov wrote:
В Tue, 09 Dec 2014 23:27:49 +
Barry Jackson zen25...@zen.co.uk пишет:
On 09/12/14 22:36, Barry Jackson wrote:
On 09/12/14 18:27, Andrei Borzenkov wrote:
В Tue, 09 Dec 2014 12:35:20 +
Barry Jackson zen25...@zen.co.uk пишет:
Hello,
In Mageia it has been proposed that GRUB_ENABLE_CRYPTODISK=y be made the
default setting in /etc/default/grub for all installations, whether they
use encryption or not.
The discussion happens every now and then.
http://lists.gnu.org/archive/html/grub-devel/2013-12/msg00112.html
OK, thanks for the link.
In the case of Mageia the default installation puts everything required
by grub under /boot, so AFAICT this should not cause a problem.
... but it does.
I found time to do some testing, and adding it to the config on a system
with no encryption at all causes an error message:
error: device name required.
this seems to be triggered by this line which grub-mkconfig adds to
grub.cfg:
cryptomount -u
Does patch below help?
From: Andrei Borzenkov arvidj...@gmail.com
Subject: [PATCH] do not emit cryptomount without crypto UUID
---
util/grub-mkconfig_lib.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/util/grub-mkconfig_lib.in b/util/grub-mkconfig_lib.in
index 29ef865..60b31ca 100644
--- a/util/grub-mkconfig_lib.in
+++ b/util/grub-mkconfig_lib.in
@@ -145,7 +145,7 @@ prepare_grub_to_access_device ()
done
if [ x$GRUB_ENABLE_CRYPTODISK = xy ]; then
- for uuid in `${grub_probe} --device $@ --target=cryptodisk_uuid`; do
+ for uuid in `${grub_probe} --device $@ --target=cryptodisk_uuid`; do
echo cryptomount -u $uuid
done
fi
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH v2] grub-install: Install PV Xen binaries into the upstream specified path
On 23.10.2014 23:28, Ian Campbell wrote:
Upstream have defined a specification for where guests ought to place their
xenpv grub binaries in order to facilitate chainloading from a stage 1 grub
loaded from dom0.
http://xenbits.xen.org/docs/unstable-staging/misc/x86-xenpv-bootloader.html
The spec calls for installation into /boot/xen/pvboot-i386.elf or
/boot/xen/pvboot-x86_64.elf.
Signed-off-by: Ian Campbell i...@hellion.org.uk
---
v2: Respect bootdir, create /boot/xen as needed.
---
util/grub-install.c | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/util/grub-install.c b/util/grub-install.c
index 70f514c..7a7734e 100644
--- a/util/grub-install.c
+++ b/util/grub-install.c
@@ -1979,6 +1979,28 @@ main (int argc, char *argv[])
}
break;
+case GRUB_INSTALL_PLATFORM_I386_XEN:
+ {
+ char *path = grub_util_path_concat (2, bootdir, xen);
+ char *dst = grub_util_path_concat (2, path, pvboot-i386.elf);
+ grub_install_mkdir_p (path);
+ grub_install_copy_file (imgfile, dst, 1);
+ free (dst);
+ free (path);
+ }
+ break;
+
+case GRUB_INSTALL_PLATFORM_X86_64_XEN:
+ {
+ char *path = grub_util_path_concat (2, bootdir, xen);
Bootdir is not necessarily /boot. We also might want to specify xen dir
to e.g. /mnt/ext2/boot/xen while still keeping GRUB itself in
/boot/grub. Can you update patch to allow separate specification of
bootdir and xendir?
+ char *dst = grub_util_path_concat (2, path, pvboot-x86_64.elf);
+ grub_install_mkdir_p (path);
+ grub_install_copy_file (imgfile, dst, 1);
+ free (dst);
+ free (path);
+ }
+ break;
+
case GRUB_INSTALL_PLATFORM_MIPSEL_LOONGSON:
case GRUB_INSTALL_PLATFORM_MIPSEL_QEMU_MIPS:
case GRUB_INSTALL_PLATFORM_MIPS_QEMU_MIPS:
@@ -1987,8 +2009,6 @@ main (int argc, char *argv[])
case GRUB_INSTALL_PLATFORM_MIPSEL_ARC:
case GRUB_INSTALL_PLATFORM_ARM_UBOOT:
case GRUB_INSTALL_PLATFORM_I386_QEMU:
-case GRUB_INSTALL_PLATFORM_I386_XEN:
-case GRUB_INSTALL_PLATFORM_X86_64_XEN:
grub_util_warn (%s,
_(WARNING: no platform-specific install was performed));
break;
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] grub-core/loader/i386/xen.c: Initialized initrd_ctx so we don't free a random pointer from the stack.
Signed-off-by: Sarah Newman s...@prgmr.com
---
grub-core/loader/i386/xen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index c16b4b2..c4d9689 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -521,7 +521,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__
((unused)),
{
grub_size_t size = 0;
grub_err_t err;
- struct grub_linux_initrd_context initrd_ctx;
+ struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
grub_relocator_chunk_t ch;
if (argc == 0)
--
1.9.1
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] grub-core/loader/i386/xen.c: Initialized initrd_ctx so we don't free a random pointer from the stack.
В Fri, 27 Mar 2015 12:56:43 -0700
Sarah Newman s...@prgmr.com пишет:
Signed-off-by: Sarah Newman s...@prgmr.com
---
grub-core/loader/i386/xen.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index c16b4b2..c4d9689 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -521,7 +521,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__
((unused)),
{
grub_size_t size = 0;
grub_err_t err;
- struct grub_linux_initrd_context initrd_ctx;
+ struct grub_linux_initrd_context initrd_ctx = { 0, 0, 0 };
grub_relocator_chunk_t ch;
if (argc == 0)
Applied. Thanks!
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
