[PATCH v2 0/8] i386: memory/MSR/CR code updates

3 views
Skip to first unread message

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:21 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
Hi,

These are generally useful x86-related changes which were originally posted as
part of DRTM patchset [0]. This version took comments there into account,
hence v2. The changes can also be viewed on GitHub [1].

Best regards,
Sergii

[0]: https://lists.gnu.org/archive/html/grub-devel/2024-08/msg00088.html
[1]: https://github.com/TrenchBoot/grub/compare/e1bee15...tb-generic-2.12-72-v2

Daniel Kiper (6):
i386/msr: Merge rdmsr.h and wrmsr.h into msr.h
i386/msr: Rename grub_msr_read() and grub_msr_write()
i386/msr: Extract and improve MSR support detection code
i386/memory: Rename PAGE_SHIFT to GRUB_PAGE_SHIFT
i386/memory: Rename PAGE_SIZE to GRUB_PAGE_SIZE and make it global
mmap: Add grub_mmap_get_lowest() and grub_mmap_get_highest()

Krystian Hebel (1):
i386/memory: Define GRUB_PAGE_MASK constant and GRUB_PAGE_{UP,DOWN}
macros

Ross Philipson (1):
i386: Add CRx, MMIO, MSR and extend CPUID definitions

grub-core/commands/i386/rdmsr.c | 25 ++---
grub-core/commands/i386/wrmsr.c | 25 ++---
grub-core/lib/i386/xen/relocator.S | 6 +-
grub-core/lib/x86_64/xen/relocator.S | 4 +-
grub-core/loader/i386/xen.c | 61 ++++++------
grub-core/mmap/mmap.c | 83 ++++++++++++++++
include/grub/i386/cpuid.h | 11 +++
include/grub/i386/crfr.h | 127 +++++++++++++++++++++++++
include/grub/i386/memory.h | 8 +-
include/grub/i386/mmio.h | 72 ++++++++++++++
include/grub/i386/msr.h | 137 +++++++++++++++++++++++++++
include/grub/i386/rdmsr.h | 37 --------
include/grub/i386/wrmsr.h | 35 -------
include/grub/memory.h | 3 +
14 files changed, 489 insertions(+), 145 deletions(-)
create mode 100644 include/grub/i386/crfr.h
create mode 100644 include/grub/i386/mmio.h
create mode 100644 include/grub/i386/msr.h
delete mode 100644 include/grub/i386/rdmsr.h
delete mode 100644 include/grub/i386/wrmsr.h

--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:22 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

It does not make sense to have separate headers for individual static
functions. Additionally, we have to add some constants with MSR
addresses in subsequent patches. So, make one common place to store
them.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/commands/i386/rdmsr.c | 2 +-
grub-core/commands/i386/wrmsr.c | 2 +-
include/grub/i386/{wrmsr.h => msr.h} | 16 +++++++++---
include/grub/i386/rdmsr.h | 37 ----------------------------
4 files changed, 15 insertions(+), 42 deletions(-)
rename include/grub/i386/{wrmsr.h => msr.h} (78%)
delete mode 100644 include/grub/i386/rdmsr.h

diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index 46c4346da..fa4622f9e 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -26,7 +26,7 @@
#include <grub/extcmd.h>
#include <grub/i18n.h>
#include <grub/i386/cpuid.h>
-#include <grub/i386/rdmsr.h>
+#include <grub/i386/msr.h>

GRUB_MOD_LICENSE("GPLv3+");

diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index 1b143b888..8f352f205 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -27,7 +27,7 @@
#include <grub/lockdown.h>
#include <grub/i18n.h>
#include <grub/i386/cpuid.h>
-#include <grub/i386/wrmsr.h>
+#include <grub/i386/msr.h>

GRUB_MOD_LICENSE("GPLv3+");

diff --git a/include/grub/i386/wrmsr.h b/include/grub/i386/msr.h
similarity index 78%
rename from include/grub/i386/wrmsr.h
rename to include/grub/i386/msr.h
index dea60aed1..7b52b5d61 100644
--- a/include/grub/i386/wrmsr.h
+++ b/include/grub/i386/msr.h
@@ -16,14 +16,24 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/

-#ifndef GRUB_WRMSR_H
-#define GRUB_WRMSR_H 1
+#ifndef GRUB_I386_MSR_H
+#define GRUB_I386_MSR_H 1

/*
* TODO: Add a general protection exception handler.
* Accessing a reserved or unimplemented MSR address results in a GP#.
*/

+static inline grub_uint64_t
+grub_msr_read (grub_uint32_t msr_id)
+{
+ grub_uint32_t low, high;
+
+ asm volatile ("rdmsr" : "=a" (low), "=d" (high) : "c" (msr_id));
+
+ return ((grub_uint64_t) high << 32) | low;
+}
+
static inline void
grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
{
@@ -32,4 +42,4 @@ grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
asm volatile ("wrmsr" : : "c" (msr_id), "a" (low), "d" (high));
}

-#endif /* GRUB_WRMSR_H */
+#endif /* GRUB_I386_MSR_H */
diff --git a/include/grub/i386/rdmsr.h b/include/grub/i386/rdmsr.h
deleted file mode 100644
index c0a0c717a..000000000
--- a/include/grub/i386/rdmsr.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * GRUB -- GRand Unified Bootloader
- * Copyright (C) 2019 Free Software Foundation, Inc.
- *
- * GRUB is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * GRUB is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef GRUB_RDMSR_H
-#define GRUB_RDMSR_H 1
-
-/*
- * TODO: Add a general protection exception handler.
- * Accessing a reserved or unimplemented MSR address results in a GP#.
- */
-
-static inline grub_uint64_t
-grub_msr_read (grub_uint32_t msr_id)
-{
- grub_uint32_t low, high;
-
- asm volatile ("rdmsr" : "=a" (low), "=d" (high) : "c" (msr_id));
-
- return ((grub_uint64_t)high << 32) | low;
-}
-
-#endif /* GRUB_RDMSR_H */
--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:24 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

Use more obvious names which match corresponding instructions:
* grub_msr_read() => grub_rdmsr()
* grub_msr_write() => grub_wrmsr()

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/commands/i386/rdmsr.c | 2 +-
grub-core/commands/i386/wrmsr.c | 2 +-
include/grub/i386/msr.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index fa4622f9e..89ece7657 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -76,7 +76,7 @@ grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char **argv)
if (*ptr != '\0')
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));

- value = grub_msr_read (addr);
+ value = grub_rdmsr (addr);

if (ctxt->state[0].set)
{
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index 8f352f205..cf6bf6c8f 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -77,7 +77,7 @@ grub_cmd_msr_write (grub_command_t cmd __attribute__ ((unused)), int argc, char
if (*ptr != '\0')
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));

- grub_msr_write (addr, value);
+ grub_wrmsr (addr, value);

return GRUB_ERR_NONE;
}
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 7b52b5d61..4fba1b8e0 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -25,7 +25,7 @@
*/

static inline grub_uint64_t
-grub_msr_read (grub_uint32_t msr_id)
+grub_rdmsr (grub_uint32_t msr_id)
{
grub_uint32_t low, high;

@@ -35,7 +35,7 @@ grub_msr_read (grub_uint32_t msr_id)
}

static inline void
-grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
+grub_wrmsr (grub_uint32_t msr_id, grub_uint64_t msr_value)
{
grub_uint32_t low = msr_value, high = msr_value >> 32;

--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:26 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

Currently rdmsr and wrmsr commands have own MSR support detection code.
This code is the same. So, it is duplicated. Additionally, this code
cannot be reused by others. Hence, extract this code to a function and
make it public. By the way, improve a code a bit.

Additionally, use GRUB_ERR_BAD_DEVICE instead of GRUB_ERR_BUG to signal
an error because errors encountered by this new routine are not bugs.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/commands/i386/rdmsr.c | 21 +++++----------------
grub-core/commands/i386/wrmsr.c | 21 +++++----------------
include/grub/i386/msr.h | 29 +++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index 89ece7657..2e42f6197 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -42,27 +42,16 @@ static const struct grub_arg_option options[] =
static grub_err_t
grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char **argv)
{
- grub_uint32_t manufacturer[3], max_cpuid, a, b, c, features, addr;
+ grub_err_t err;
+ grub_uint32_t addr;
grub_uint64_t value;
const char *ptr;
char buf[sizeof("1122334455667788")];

- /*
- * The CPUID instruction should be used to determine whether MSRs
- * are supported. (CPUID.01H:EDX[5] = 1)
- */
- if (! grub_cpu_is_cpuid_supported ())
- return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ err = grub_cpu_is_msr_supported ();

- grub_cpuid (0, max_cpuid, manufacturer[0], manufacturer[2], manufacturer[1]);
-
- if (max_cpuid < 1)
- return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
-
- grub_cpuid (1, a, b, c, features);
-
- if (!(features & (1 << 5)))
- return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ if (err != GRUB_ERR_NONE)
+ return grub_error (err, N_("RDMSR is unsupported"));

if (argc != 1)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("one argument expected"));
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index cf6bf6c8f..7fbedaed9 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -36,26 +36,15 @@ static grub_command_t cmd_write;
static grub_err_t
grub_cmd_msr_write (grub_command_t cmd __attribute__ ((unused)), int argc, char **argv)
{
- grub_uint32_t manufacturer[3], max_cpuid, a, b, c, features, addr;
+ grub_err_t err;
+ grub_uint32_t addr;
grub_uint64_t value;
const char *ptr;

- /*
- * The CPUID instruction should be used to determine whether MSRs
- * are supported. (CPUID.01H:EDX[5] = 1)
- */
- if (!grub_cpu_is_cpuid_supported ())
- return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ err = grub_cpu_is_msr_supported ();

- grub_cpuid (0, max_cpuid, manufacturer[0], manufacturer[2], manufacturer[1]);
-
- if (max_cpuid < 1)
- return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
-
- grub_cpuid (1, a, b, c, features);
-
- if (!(features & (1 << 5)))
- return grub_error (GRUB_ERR_BUG, N_("unsupported instruction"));
+ if (err != GRUB_ERR_NONE)
+ return grub_error (err, N_("WRMSR is unsupported"));

if (argc != 2)
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("two arguments expected"));
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 4fba1b8e0..1e838c022 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -19,6 +19,35 @@
#ifndef GRUB_I386_MSR_H
#define GRUB_I386_MSR_H 1

+#include <grub/err.h>
+#include <grub/i386/cpuid.h>
+#include <grub/types.h>
+
+static inline grub_err_t
+grub_cpu_is_msr_supported (void)
+{
+ grub_uint32_t eax, ebx, ecx, edx;
+
+ /*
+ * The CPUID instruction should be used to determine whether MSRs
+ * are supported, CPUID.01H:EDX[5] = 1.
+ */
+ if (!grub_cpu_is_cpuid_supported ())
+ return GRUB_ERR_BAD_DEVICE;
+
+ grub_cpuid (0, eax, ebx, ecx, edx);
+
+ if (eax < 1)
+ return GRUB_ERR_BAD_DEVICE;
+
+ grub_cpuid (1, eax, ebx, ecx, edx);
+
+ if (!(edx & (1 << 5)))
+ return GRUB_ERR_BAD_DEVICE;
+
+ return GRUB_ERR_NONE;
+}
+
/*
* TODO: Add a general protection exception handler.
* Accessing a reserved or unimplemented MSR address results in a GP#.
--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:28 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

This fixes naming inconsistency that goes against coding style as well
as helps to avoid potential conflicts and confusion.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/lib/i386/xen/relocator.S | 6 +++---
grub-core/lib/x86_64/xen/relocator.S | 4 ++--
grub-core/loader/i386/xen.c | 28 ++++++++++++++--------------
include/grub/i386/memory.h | 2 +-
4 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/grub-core/lib/i386/xen/relocator.S b/grub-core/lib/i386/xen/relocator.S
index 96e51b59a..dab4d8ace 100644
--- a/grub-core/lib/i386/xen/relocator.S
+++ b/grub-core/lib/i386/xen/relocator.S
@@ -75,10 +75,10 @@ VARIABLE(grub_relocator_xen_mfn_list)
.long 0
movl 0(%eax, %ebp, 4), %ecx /* mfn */
movl %ebp, %ebx
- shll $PAGE_SHIFT, %ebx /* virtual address (1:1 mapping) */
+ shll $GRUB_PAGE_SHIFT, %ebx /* virtual address (1:1 mapping) */
movl %ecx, %edx
- shll $PAGE_SHIFT, %ecx /* prepare pte low part */
- shrl $(32 - PAGE_SHIFT), %edx /* pte high part */
+ shll $GRUB_PAGE_SHIFT, %ecx /* prepare pte low part */
+ shrl $(32 - GRUB_PAGE_SHIFT), %edx /* pte high part */
orl $(GRUB_PAGE_PRESENT | GRUB_PAGE_USER), %ecx /* pte low */
movl $UVMF_INVLPG, %esi
movl $__HYPERVISOR_update_va_mapping, %eax
diff --git a/grub-core/lib/x86_64/xen/relocator.S b/grub-core/lib/x86_64/xen/relocator.S
index f5364ed0f..852cd40aa 100644
--- a/grub-core/lib/x86_64/xen/relocator.S
+++ b/grub-core/lib/x86_64/xen/relocator.S
@@ -60,9 +60,9 @@ LOCAL(cont):
jz 3f
2:
movq %r12, %rdi
- shlq $PAGE_SHIFT, %rdi /* virtual address (1:1 mapping) */
+ shlq $GRUB_PAGE_SHIFT, %rdi /* virtual address (1:1 mapping) */
movq (%rbx, %r12, 8), %rsi /* mfn */
- shlq $PAGE_SHIFT, %rsi
+ shlq $GRUB_PAGE_SHIFT, %rsi
orq $(GRUB_PAGE_PRESENT | GRUB_PAGE_USER), %rsi /* Build pte */
movq $UVMF_INVLPG, %rdx
movq %rcx, %r9 /* %rcx clobbered by hypercall */
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index 3b856e842..520367732 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -92,7 +92,7 @@ static struct xen_loader_state xen_state;

static grub_dl_t my_mod;

-#define PAGE_SIZE (1UL << PAGE_SHIFT)
+#define PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
#define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
#define STACK_SIZE 1048576
#define ADDITIONAL_SIZE (1 << 19)
@@ -103,7 +103,7 @@ static grub_dl_t my_mod;
static grub_uint64_t
page2offset (grub_uint64_t page)
{
- return page << PAGE_SHIFT;
+ return page << GRUB_PAGE_SHIFT;
}

static grub_err_t
@@ -142,7 +142,7 @@ get_pgtable_size (grub_uint64_t from, grub_uint64_t to, grub_uint64_t pfn)
continue;
}

- bits = PAGE_SHIFT + (i + 1) * LOG_POINTERS_PER_PAGE;
+ bits = GRUB_PAGE_SHIFT + (i + 1) * LOG_POINTERS_PER_PAGE;
mask = (1ULL << bits) - 1;
map->lvls[i].virt_start = map->area.virt_start & ~mask;
map->lvls[i].virt_end = map->area.virt_end | mask;
@@ -247,11 +247,11 @@ generate_page_table (grub_xen_mfn_t *mfn_list)
if (lvl->virt_start >= end || lvl->virt_end <= start)
continue;
p_s = (grub_max (start, lvl->virt_start) - start) >>
- (PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
+ (GRUB_PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
p_e = (grub_min (end, lvl->virt_end) - start) >>
- (PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
+ (GRUB_PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE);
pfn = ((grub_max (start, lvl->virt_start) - lvl->virt_start) >>
- (PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE)) + lvl->pfn_start;
+ (GRUB_PAGE_SHIFT + l * LOG_POINTERS_PER_PAGE)) + lvl->pfn_start;
grub_dprintf ("xen", "write page table entries level %d pg %p "
"mapping %d/%d index %lx-%lx pfn %llx\n",
l, pg, m1, m2, p_s, p_e, (unsigned long long) pfn);
@@ -329,16 +329,16 @@ grub_xen_p2m_alloc (void)
{
err = get_pgtable_size (xen_state.xen_inf.p2m_base,
xen_state.xen_inf.p2m_base + p2msize,
- (xen_state.max_addr + p2msize) >> PAGE_SHIFT);
+ (xen_state.max_addr + p2msize) >> GRUB_PAGE_SHIFT);
if (err)
return err;

- map->area.pfn_start = xen_state.max_addr >> PAGE_SHIFT;
+ map->area.pfn_start = xen_state.max_addr >> GRUB_PAGE_SHIFT;
p2malloc = p2msize + page2offset (map->area.n_pt_pages);
xen_state.n_mappings++;
xen_state.next_start.mfn_list = xen_state.xen_inf.p2m_base;
xen_state.next_start.first_p2m_pfn = map->area.pfn_start;
- xen_state.next_start.nr_p2m_frames = p2malloc >> PAGE_SHIFT;
+ xen_state.next_start.nr_p2m_frames = p2malloc >> GRUB_PAGE_SHIFT;
}
else
{
@@ -381,7 +381,7 @@ grub_xen_special_alloc (void)
xen_state.virt_start_info = get_virtual_current_address (ch);
xen_state.max_addr =
ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE);
- xen_state.console_pfn = xen_state.max_addr >> PAGE_SHIFT;
+ xen_state.console_pfn = xen_state.max_addr >> GRUB_PAGE_SHIFT;
xen_state.max_addr += 2 * PAGE_SIZE;

xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages;
@@ -413,7 +413,7 @@ grub_xen_pt_alloc (void)

xen_state.next_start.pt_base =
xen_state.max_addr + xen_state.xen_inf.virt_base;
- nr_info_pages = xen_state.max_addr >> PAGE_SHIFT;
+ nr_info_pages = xen_state.max_addr >> GRUB_PAGE_SHIFT;
nr_need_pages = nr_info_pages;

while (1)
@@ -461,7 +461,7 @@ grub_xen_pt_alloc (void)
xen_state.max_addr + STACK_SIZE + xen_state.xen_inf.virt_base;
xen_state.next_start.nr_pt_frames = nr_need_pages;
xen_state.max_addr = try_virt_end - xen_state.xen_inf.virt_base;
- xen_state.pgtbl_end = xen_state.max_addr >> PAGE_SHIFT;
+ xen_state.pgtbl_end = xen_state.max_addr >> GRUB_PAGE_SHIFT;
xen_state.map_reloc->where = (grub_uint64_t *) ((char *) map->where +
page2offset (map->area.n_pt_pages));

@@ -515,7 +515,7 @@ grub_xen_boot (void)
if (err)
return err;

- nr_pages = xen_state.max_addr >> PAGE_SHIFT;
+ nr_pages = xen_state.max_addr >> GRUB_PAGE_SHIFT;

grub_dprintf ("xen", "bootstrap domain %llx+%llx\n",
(unsigned long long) xen_state.xen_inf.virt_base,
@@ -818,7 +818,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
if (xen_state.xen_inf.unmapped_initrd)
{
xen_state.next_start.flags |= SIF_MOD_START_PFN;
- xen_state.next_start.mod_start = xen_state.max_addr >> PAGE_SHIFT;
+ xen_state.next_start.mod_start = xen_state.max_addr >> GRUB_PAGE_SHIFT;
}
else
xen_state.next_start.mod_start =
diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
index 5cb607fb4..7be57d6d7 100644
--- a/include/grub/i386/memory.h
+++ b/include/grub/i386/memory.h
@@ -20,7 +20,7 @@
#ifndef GRUB_MEMORY_CPU_HEADER
#define GRUB_MEMORY_CPU_HEADER 1

-#define PAGE_SHIFT 12
+#define GRUB_PAGE_SHIFT 12

/* The flag for protected mode. */
#define GRUB_MEMORY_CPU_CR0_PE_ON 0x1
--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:30 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

Subsequent patches will use that constant.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/loader/i386/xen.c | 35 +++++++++++++++++------------------
include/grub/i386/memory.h | 1 +
2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index 520367732..dcdf005df 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -92,8 +92,7 @@ static struct xen_loader_state xen_state;

static grub_dl_t my_mod;

-#define PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
-#define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
+#define MAX_MODULES (GRUB_PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
#define STACK_SIZE 1048576
#define ADDITIONAL_SIZE (1 << 19)
#define ALIGN_SIZE (1 << 22)
@@ -229,7 +228,7 @@ generate_page_table (grub_xen_mfn_t *mfn_list)

for (m1 = 0; m1 < xen_state.n_mappings; m1++)
grub_memset (xen_state.mappings[m1].where, 0,
- xen_state.mappings[m1].area.n_pt_pages * PAGE_SIZE);
+ xen_state.mappings[m1].area.n_pt_pages * GRUB_PAGE_SIZE);

for (l = NUMBER_OF_LEVELS - 1; l >= 0; l--)
{
@@ -324,7 +323,7 @@ grub_xen_p2m_alloc (void)

map = xen_state.mappings + xen_state.n_mappings;
p2msize = ALIGN_UP (sizeof (grub_xen_mfn_t) *
- grub_xen_start_page_addr->nr_pages, PAGE_SIZE);
+ grub_xen_start_page_addr->nr_pages, GRUB_PAGE_SIZE);
if (xen_state.xen_inf.has_p2m_base)
{
err = get_pgtable_size (xen_state.xen_inf.p2m_base,
@@ -380,9 +379,9 @@ grub_xen_special_alloc (void)
xen_state.state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base;
xen_state.virt_start_info = get_virtual_current_address (ch);
xen_state.max_addr =
- ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE);
+ ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), GRUB_PAGE_SIZE);
xen_state.console_pfn = xen_state.max_addr >> GRUB_PAGE_SHIFT;
- xen_state.max_addr += 2 * PAGE_SIZE;
+ xen_state.max_addr += 2 * GRUB_PAGE_SIZE;

xen_state.next_start.nr_pages = grub_xen_start_page_addr->nr_pages;
grub_memcpy (xen_state.next_start.magic, grub_xen_start_page_addr->magic,
@@ -431,9 +430,9 @@ grub_xen_pt_alloc (void)
/* Map the relocator page either at virtual 0 or after end of area. */
nr_need_pages = nr_info_pages + map->area.n_pt_pages;
if (xen_state.xen_inf.virt_base)
- err = get_pgtable_size (0, PAGE_SIZE, nr_need_pages);
+ err = get_pgtable_size (0, GRUB_PAGE_SIZE, nr_need_pages);
else
- err = get_pgtable_size (try_virt_end, try_virt_end + PAGE_SIZE,
+ err = get_pgtable_size (try_virt_end, try_virt_end + GRUB_PAGE_SIZE,
nr_need_pages);
if (err)
return err;
@@ -538,7 +537,7 @@ grub_xen_boot (void)

return grub_relocator_xen_boot (xen_state.relocator, xen_state.state, nr_pages,
xen_state.xen_inf.virt_base <
- PAGE_SIZE ? page2offset (nr_pages) : 0,
+ GRUB_PAGE_SIZE ? page2offset (nr_pages) : 0,
xen_state.pgtbl_end - 1,
page2offset (xen_state.pgtbl_end - 1) +
xen_state.xen_inf.virt_base);
@@ -677,7 +676,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
goto fail;
}

- if (xen_state.xen_inf.virt_base & (PAGE_SIZE - 1))
+ if (xen_state.xen_inf.virt_base & (GRUB_PAGE_SIZE - 1))
{
grub_error (GRUB_ERR_BAD_OS, "unaligned virt_base");
goto fail;
@@ -700,10 +699,10 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
kern_start = grub_min (kern_start, xen_state.xen_inf.hypercall_page -
xen_state.xen_inf.virt_base);
kern_end = grub_max (kern_end, xen_state.xen_inf.hypercall_page -
- xen_state.xen_inf.virt_base + PAGE_SIZE);
+ xen_state.xen_inf.virt_base + GRUB_PAGE_SIZE);
}

- xen_state.max_addr = ALIGN_UP (kern_end, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (kern_end, GRUB_PAGE_SIZE);


if (grub_sub (kern_end, kern_start, &sz))
@@ -730,7 +729,7 @@ grub_cmd_xen (grub_command_t cmd __attribute__ ((unused)),
if (xen_state.xen_inf.has_hypercall_page)
{
unsigned i;
- for (i = 0; i < PAGE_SIZE / HYPERCALL_INTERFACE_SIZE; i++)
+ for (i = 0; i < GRUB_PAGE_SIZE / HYPERCALL_INTERFACE_SIZE; i++)
set_hypercall_interface ((grub_uint8_t *) kern_chunk_src +
i * HYPERCALL_INTERFACE_SIZE +
xen_state.xen_inf.hypercall_page -
@@ -828,7 +827,7 @@ grub_cmd_initrd (grub_command_t cmd __attribute__ ((unused)),
(unsigned) (xen_state.max_addr + xen_state.xen_inf.virt_base),
(unsigned) size);

- xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, GRUB_PAGE_SIZE);

fail:
grub_initrd_close (&initrd_ctx);
@@ -882,7 +881,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
{
xen_state.xen_inf.unmapped_initrd = 0;
xen_state.n_modules = 0;
- xen_state.max_addr = ALIGN_UP (xen_state.max_addr, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (xen_state.max_addr, GRUB_PAGE_SIZE);
xen_state.modules_target_start = xen_state.max_addr;
xen_state.next_start.mod_start =
xen_state.max_addr + xen_state.xen_inf.virt_base;
@@ -902,7 +901,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
MAX_MODULES * sizeof (xen_state.module_info_page[0]);
}

- xen_state.max_addr = ALIGN_UP (xen_state.max_addr, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (xen_state.max_addr, GRUB_PAGE_SIZE);

file = grub_file_open (argv[0], GRUB_FILE_TYPE_LINUX_INITRD |
(nounzip ? GRUB_FILE_TYPE_NO_DECOMPRESS : GRUB_FILE_TYPE_NONE));
@@ -925,7 +924,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),

xen_state.module_info_page[xen_state.n_modules].cmdline =
xen_state.max_addr - xen_state.modules_target_start;
- xen_state.max_addr = ALIGN_UP (xen_state.max_addr + cmdline_len, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (xen_state.max_addr + cmdline_len, GRUB_PAGE_SIZE);

if (size)
{
@@ -952,7 +951,7 @@ grub_cmd_module (grub_command_t cmd __attribute__ ((unused)),
xen_state.n_modules++;
grub_dprintf ("xen", "module, addr=0x%x, size=0x%x\n",
(unsigned) xen_state.max_addr, (unsigned) size);
- xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, GRUB_PAGE_SIZE);


fail:
diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
index 7be57d6d7..c64529630 100644
--- a/include/grub/i386/memory.h
+++ b/include/grub/i386/memory.h
@@ -21,6 +21,7 @@
#define GRUB_MEMORY_CPU_HEADER 1

#define GRUB_PAGE_SHIFT 12
+#define GRUB_PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:32 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Krystian Hebel <krystia...@3mdeb.com>

Subsequent patches will use those macros and constant.

Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
include/grub/i386/memory.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
index c64529630..56f64855b 100644
--- a/include/grub/i386/memory.h
+++ b/include/grub/i386/memory.h
@@ -22,6 +22,7 @@

#define GRUB_PAGE_SHIFT 12
#define GRUB_PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
+#define GRUB_PAGE_MASK (~(GRUB_PAGE_SIZE - 1))

/* The flag for protected mode. */
#define GRUB_MEMORY_CPU_CR0_PE_ON 0x1
@@ -43,8 +44,12 @@

#define GRUB_MMAP_MALLOC_LOW 1

+#include <grub/misc.h>
#include <grub/types.h>

+#define GRUB_PAGE_UP(p) ALIGN_UP (p, GRUB_PAGE_SIZE)
+#define GRUB_PAGE_DOWN(p) ALIGN_DOWN (p, GRUB_PAGE_SIZE)
+
struct grub_e820_mmap_entry
{
grub_uint64_t addr;
--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:33 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

The functions calculate lowest and highest available RAM
addresses respectively.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/mmap/mmap.c | 83 +++++++++++++++++++++++++++++++++++++++++++
include/grub/memory.h | 3 ++
2 files changed, 86 insertions(+)

diff --git a/grub-core/mmap/mmap.c b/grub-core/mmap/mmap.c
index c8c8312c5..80d6c60b8 100644
--- a/grub-core/mmap/mmap.c
+++ b/grub-core/mmap/mmap.c
@@ -26,6 +26,7 @@
#include <grub/command.h>
#include <grub/dl.h>
#include <grub/i18n.h>
+#include <grub/safemath.h>

GRUB_MOD_LICENSE ("GPLv3+");

@@ -343,6 +344,88 @@ grub_mmap_unregister (int handle)

#endif /* ! GRUB_MMAP_REGISTER_BY_FIRMWARE */

+typedef struct
+{
+ grub_uint64_t addr;
+ grub_uint64_t limit;
+} addr_limit_t;
+
+/* Helper for grub_mmap_get_lowest(). */
+static int
+lowest_hook (grub_uint64_t addr, grub_uint64_t size, grub_memory_type_t type,
+ void *data)
+{
+ addr_limit_t *al = data;
+ grub_uint64_t end;
+
+ if (type != GRUB_MEMORY_AVAILABLE)
+ return 0;
+
+ if (grub_add (addr, size, &end))
+ return 0;
+
+ if (addr >= al->limit)
+ al->addr = grub_min (al->addr, addr);
+
+ if ((addr < al->limit) && (end > al->limit))
+ al->addr = al->limit;
+
+ return 0;
+}
+
+/*
+ * This function calculates lowest available RAM address that is at or above
+ * the passed limit. If no RAM exists above the limit, ~0 is returned.
+ */
+grub_uint64_t
+grub_mmap_get_lowest (grub_uint64_t limit)
+{
+ addr_limit_t al = {~0, limit};
+
+ grub_mmap_iterate (lowest_hook, &al);
+
+ return al.addr;
+}
+
+/* Helper for grub_mmap_get_highest(). */
+static int
+highest_hook (grub_uint64_t addr, grub_uint64_t size, grub_memory_type_t type,
+ void *data)
+{
+ addr_limit_t *al = data;
+ grub_uint64_t end;
+
+ if (type != GRUB_MEMORY_AVAILABLE)
+ return 0;
+
+ if (grub_add (addr, size, &end))
+ return 0;
+
+ if (end < al->limit)
+ al->addr = grub_max (al->addr, end);
+
+ if ((addr < al->limit) && (end >= al->limit))
+ al->addr = al->limit;
+
+ return 0;
+}
+
+/*
+ * This function calculates highest available RAM address that is below the
+ * passed limit. Returned address is either one byte after last byte of RAM or
+ * equal to limit, whichever is lower. If no RAM exists below limit, 0 is
+ * returned.
+ */
+grub_uint64_t
+grub_mmap_get_highest (grub_uint64_t limit)
+{
+ addr_limit_t al = {0, limit};
+
+ grub_mmap_iterate (highest_hook, &al);
+
+ return al.addr;
+}
+
#define CHUNK_SIZE 0x400

struct badram_entry {
diff --git a/include/grub/memory.h b/include/grub/memory.h
index 6da114a1b..8f22f7525 100644
--- a/include/grub/memory.h
+++ b/include/grub/memory.h
@@ -69,6 +69,9 @@ void *grub_mmap_malign_and_register (grub_uint64_t align, grub_uint64_t size,

void grub_mmap_free_and_unregister (int handle);

+extern grub_uint64_t grub_mmap_get_lowest (grub_uint64_t limit);
+extern grub_uint64_t grub_mmap_get_highest (grub_uint64_t limit);
+
#ifndef GRUB_MMAP_REGISTER_BY_FIRMWARE

struct grub_mmap_region
--
2.46.1

Sergii Dmytruk

unread,
Sep 19, 2024, 6:02:34 PM9/19/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Ross Philipson <ross.ph...@oracle.com>

Signed-off-by: Ross Philipson <ross.ph...@oracle.com>
Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
include/grub/i386/cpuid.h | 11 ++++
include/grub/i386/crfr.h | 127 ++++++++++++++++++++++++++++++++++++++
include/grub/i386/mmio.h | 72 +++++++++++++++++++++
include/grub/i386/msr.h | 63 +++++++++++++++++++
4 files changed, 273 insertions(+)
create mode 100644 include/grub/i386/crfr.h
create mode 100644 include/grub/i386/mmio.h

diff --git a/include/grub/i386/cpuid.h b/include/grub/i386/cpuid.h
index f7ae4b0a4..bf2a39ec3 100644
--- a/include/grub/i386/cpuid.h
+++ b/include/grub/i386/cpuid.h
@@ -19,6 +19,17 @@
#ifndef GRUB_CPU_CPUID_HEADER
#define GRUB_CPU_CPUID_HEADER 1

+/* General */
+#define GRUB_X86_CPUID_VENDOR 0x00000000
+#define GRUB_X86_CPUID_FEATURES 0x00000001
+/* Intel */
+#define GRUB_X86_CPUID_FEATURES_ECX_VMX (1<<5)
+#define GRUB_X86_CPUID_FEATURES_ECX_SMX (1<<6)
+/* AMD */
+#define GRUB_AMD_CPUID_FEATURES 0x80000001
+#define GRUB_AMD_CPUID_FEATURES_ECX_SVM (1<<2)
+#define GRUB_AMD_CPUID_FUNC 0x8000000a
+
extern unsigned char grub_cpuid_has_longmode;
extern unsigned char grub_cpuid_has_pae;

diff --git a/include/grub/i386/crfr.h b/include/grub/i386/crfr.h
new file mode 100644
index 000000000..aeb696a91
--- /dev/null
+++ b/include/grub/i386/crfr.h
@@ -0,0 +1,127 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_CRFR_H
+#define GRUB_CRFR_H 1
+
+#include <grub/types.h>
+
+/* Routines for R/W of control and flags registers */
+
+#define GRUB_CR0_X86_PE 0x00000001 /* Enable Protected Mode */
+#define GRUB_CR0_X86_MP 0x00000002 /* "Math" (FPU) Present */
+#define GRUB_CR0_X86_EM 0x00000004 /* EMulate FPU */
+#define GRUB_CR0_X86_TS 0x00000008 /* Task Switched */
+#define GRUB_CR0_X86_PG 0x80000000 /* Enable PaGing */
+
+#define GRUB_CR0_X86_NE 0x00000020 /* Numeric Error enable (EX16 vs IRQ13) */
+#define GRUB_CR0_X86_WP 0x00010000 /* Write Protect */
+#define GRUB_CR0_X86_AM 0x00040000 /* Alignment Mask */
+#define GRUB_CR0_X86_NW 0x20000000 /* Not Write-through */
+#define GRUB_CR0_X86_CD 0x40000000 /* Cache Disable */
+
+#define GRUB_CR4_X86_VME 0x00000001 /* Virtual 8086 mode extensions */
+#define GRUB_CR4_X86_PVI 0x00000002 /* Protected-mode virtual interrupts */
+#define GRUB_CR4_X86_TSD 0x00000004 /* Time stamp disable */
+#define GRUB_CR4_X86_DE 0x00000008 /* Debugging extensions */
+#define GRUB_CR4_X86_PSE 0x00000010 /* Page size extensions */
+#define GRUB_CR4_X86_PAE 0x00000020 /* Physical address extension */
+#define GRUB_CR4_X86_MCE 0x00000040 /* Enable Machine check enable */
+#define GRUB_CR4_X86_PGE 0x00000080 /* Enable Page global */
+#define GRUB_CR4_X86_PCE 0x00000100 /* Enable Performance monitoring counter */
+#define GRUB_CR4_X86_FXSR 0x00000200 /* Fast FPU save/restore */
+#define GRUB_CR4_X86_XMM 0x00000400 /* Generate #XM instead of #UD for SIMD */
+#define GRUB_CR4_X86_VMXE 0x00002000 /* Enable VMX */
+#define GRUB_CR4_X86_SMXE 0x00004000 /* Enable SMX */
+#define GRUB_CR4_X86_PCIDE 0x00020000 /* Enable PCID */
+
+static inline unsigned long
+grub_read_cr0 (void)
+{
+ unsigned long val;
+
+ asm volatile ("mov %%cr0, %0" : "=r" (val) : : "memory");
+
+ return val;
+}
+
+static inline void
+grub_write_cr0 (unsigned long val)
+{
+ asm volatile ("mov %0, %%cr0" : : "r" (val) : "memory");
+}
+
+static inline unsigned long
+grub_read_cr4 (void)
+{
+ unsigned long val;
+
+ asm volatile ("mov %%cr4, %0" : "=r" (val) : : "memory");
+
+ return val;
+}
+
+static inline void
+grub_write_cr4 (unsigned long val)
+{
+ asm volatile ("mov %0, %%cr4" : : "r" (val) : "memory");
+}
+
+#define GRUB_EFLAGS_X86_CF 0x00000001 /* Carry Flag */
+#define GRUB_EFLAGS_X86_PF 0x00000004 /* Parity Flag */
+#define GRUB_EFLAGS_X86_AF 0x00000010 /* Auxillary carry Flag */
+#define GRUB_EFLAGS_X86_ZF 0x00000040 /* Zero Flag */
+#define GRUB_EFLAGS_X86_SF 0x00000080 /* Sign Flag */
+#define GRUB_EFLAGS_X86_TF 0x00000100 /* Trap Flag */
+#define GRUB_EFLAGS_X86_IF 0x00000200 /* Interrupt Flag */
+#define GRUB_EFLAGS_X86_DF 0x00000400 /* Direction Flag */
+#define GRUB_EFLAGS_X86_OF 0x00000800 /* Overflow Flag */
+#define GRUB_EFLAGS_X86_IOPL 0x00003000 /* IOPL mask */
+#define GRUB_EFLAGS_X86_NT 0x00004000 /* Nested Task */
+#define GRUB_EFLAGS_X86_RF 0x00010000 /* Resume Flag */
+#define GRUB_EFLAGS_X86_VM 0x00020000 /* Virtual Mode */
+#define GRUB_EFLAGS_X86_AC 0x00040000 /* Alignment Check */
+#define GRUB_EFLAGS_X86_VIF 0x00080000 /* Virtual Interrupt Flag */
+#define GRUB_EFLAGS_X86_VIP 0x00100000 /* Virtual Interrupt Pending */
+#define GRUB_EFLAGS_X86_ID 0x00200000 /* CPUID detection flag */
+
+static inline unsigned long
+grub_read_flags_register (void)
+{
+ unsigned long flags;
+
+#ifdef __x86_64__
+ asm volatile ("pushfq; popq %0" : "=r" (flags));
+#else
+ asm volatile ("pushfl; popl %0" : "=r" (flags));
+#endif
+
+ return flags;
+}
+
+static inline void
+grub_write_flags_register (unsigned long flags)
+{
+#ifdef __x86_64__
+ asm volatile ("pushq %0; popfq" : : "r" (flags));
+#else
+ asm volatile ("pushl %0; popfl" : : "r" (flags));
+#endif
+}
+
+#endif
diff --git a/include/grub/i386/mmio.h b/include/grub/i386/mmio.h
new file mode 100644
index 000000000..97a30f7d8
--- /dev/null
+++ b/include/grub/i386/mmio.h
@@ -0,0 +1,72 @@
+/*
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_I386_MMIO_H
+#define GRUB_I386_MMIO_H 1
+
+#include <grub/types.h>
+
+static inline grub_uint8_t
+grub_read8 (const grub_addr_t addr)
+{
+ grub_uint8_t val;
+
+ val = (*(volatile grub_uint8_t *) (addr));
+
+ return val;
+}
+
+static inline grub_uint32_t
+grub_read32 (const grub_addr_t addr)
+{
+ grub_uint32_t val;
+
+ val = (*(volatile grub_uint32_t *) (addr));
+
+ return val;
+}
+
+static inline grub_uint64_t
+grub_read64 (const grub_addr_t addr)
+{
+ grub_uint64_t val;
+
+ val = (*(volatile grub_uint64_t *) (addr));
+
+ return val;
+}
+
+static inline void
+grub_write8 (grub_uint8_t val, grub_addr_t addr)
+{
+ (*(volatile grub_uint8_t *) (addr)) = val;
+}
+
+static inline void
+grub_write32 (grub_uint32_t val, grub_addr_t addr)
+{
+ (*(volatile grub_uint32_t *) (addr)) = val;
+}
+
+static inline void
+grub_write64 (grub_uint64_t val, grub_addr_t addr)
+{
+ (*(volatile grub_uint64_t *) (addr)) = val;
+}
+
+#endif /* GRUB_I386_MMIO_H */
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 1e838c022..05a0fd2f6 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -2,6 +2,9 @@
* GRUB -- GRand Unified Bootloader
* Copyright (C) 2019 Free Software Foundation, Inc.
*
+ * Some definitions in this header are extracted from the Trusted Computing
+ * Group's "TPM Main Specification", Parts 1-3.
+ *
* GRUB is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
@@ -19,6 +22,64 @@
#ifndef GRUB_I386_MSR_H
#define GRUB_I386_MSR_H 1

+/* General */
+#define GRUB_MSR_X86_PLATFORM_ID 0x00000017
+
+#define GRUB_MSR_X86_APICBASE 0x0000001b
+#define GRUB_MSR_X86_APICBASE_BSP (1<<8)
+#define GRUB_MSR_X86_APICBASE_ENABLE (1<<11)
+#define GRUB_MSR_X86_APICBASE_BASE (0xfffff<<12) /* Mask for APIC base address */
+
+#define GRUB_MSR_X86_FEATURE_CONTROL 0x0000003a
+#define GRUB_MSR_X86_FEATURE_CTRL_LOCK (1<<0) /* Lock writes to this register */
+#define GRUB_MSR_X86_ENABLE_VMX_IN_SMX (1<<1) /* Enable VMX inside SMX */
+#define GRUB_MSR_X86_ENABLE_VMX_OUT_SMX (1<<2) /* Enable VMX outside SMX */
+#define GRUB_MSR_X86_SENTER_FUNCTIONS (0x7f<<8) /* Bitmap of SENTER function enables */
+#define GRUB_MSR_X86_SENTER_ENABLE (1<<15) /* SENTER global enable */
+
+#define GRUB_MSR_X86_MTRRCAP 0x000000fe
+#define GRUB_MSR_X86_VCNT_MASK 0xff /* Number of variable MTRRs */
+
+#define GRUB_MSR_X86_MCG_CAP 0x00000179
+#define GRUB_MSR_MCG_BANKCNT_MASK 0xff /* Number of banks */
+#define GRUB_MSR_X86_MCG_STATUS 0x0000017a
+#define GRUB_MSR_MCG_STATUS_MCIP (1ULL<<2) /* MC in progress */
+
+#define GRUB_MSR_X86_MISC_ENABLE 0x000001a0
+#define GRUB_MSR_X86_ENABLE_MONITOR_FSM (1<<18)
+
+#define GRUB_MSR_X86_MTRR_PHYSBASE0 0x00000200
+#define GRUB_MSR_X86_MTRR_PHYSMASK0 0x00000201
+#define GRUB_MSR_X86_MTRR_PHYSBASE(n) (GRUB_MSR_X86_MTRR_PHYSBASE0 + 2 * (n))
+#define GRUB_MSR_X86_MTRR_PHYSMASK(n) (GRUB_MSR_X86_MTRR_PHYSMASK0 + 2 * (n))
+#define GRUB_MSR_X86_BASE_DEF_TYPE_MASK 0xff
+#define GRUB_MSR_X86_MASK_VALID (1<<11)
+
+#define GRUB_MSR_X86_MTRR_DEF_TYPE 0x000002ff
+#define GRUB_MSR_X86_DEF_TYPE_MASK 0xff
+#define GRUB_MSR_X86_MTRR_ENABLE_FIXED (1<<10)
+#define GRUB_MSR_X86_MTRR_ENABLE (1<<11)
+
+#define GRUB_MSR_X86_MC0_STATUS 0x00000401
+
+#define GRUB_MSR_X86_EFER 0xc0000080 /* Extended features */
+#define GRUB_MSR_EFER_LME (1<<8) /* Enable Long Mode/IA-32e */
+#define GRUB_MSR_EFER_LMA (1<<10) /* Long Mode/IA-32e Active */
+#define GRUB_MSR_EFER_SVME (1<<12) /* Enable SVM (AMD-V) */
+
+/* AMD Specific */
+#define GRUB_MSR_AMD64_VM_CR 0xc0010114 /* SVM control register */
+#define GRUB_MSR_SVM_VM_CR_SVM_DISABLE (1<<4) /* Disable writes to EFER.SVME */
+
+/* MTRR Specific */
+#define GRUB_MTRR_MEMORY_TYPE_UC 0
+#define GRUB_MTRR_MEMORY_TYPE_WC 1
+#define GRUB_MTRR_MEMORY_TYPE_WT 4
+#define GRUB_MTRR_MEMORY_TYPE_WP 5
+#define GRUB_MTRR_MEMORY_TYPE_WB 6
+
+#ifndef ASM_FILE
+
#include <grub/err.h>
#include <grub/i386/cpuid.h>
#include <grub/types.h>
@@ -71,4 +132,6 @@ grub_wrmsr (grub_uint32_t msr_id, grub_uint64_t msr_value)
asm volatile ("wrmsr" : : "c" (msr_id), "a" (low), "d" (high));
}

+#endif /* ASM_FILE */
+
#endif /* GRUB_I386_MSR_H */
--
2.46.1

Frediano Ziglio

unread,
Sep 20, 2024, 9:40:36 AM9/20/24
to The development of GNU GRUB, trenchbo...@googlegroups.com
On Thu, Sep 19, 2024 at 11:03 PM Sergii Dmytruk
<sergii....@3mdeb.com> wrote:
>
> From: Krystian Hebel <krystia...@3mdeb.com>
>
> Subsequent patches will use those macros and constant.
>

Minor, but "Define GRUB_PAGE_MASK constant and GRUB_PAGE_{UP, DOWN}
macros" subject sounds a bit confusing to me. I mean, at the end they
are all defined as macros so why mixing constant and macros?

> Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
> ---
> include/grub/i386/memory.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
> index c64529630..56f64855b 100644
> --- a/include/grub/i386/memory.h
> +++ b/include/grub/i386/memory.h
> @@ -22,6 +22,7 @@
>
> #define GRUB_PAGE_SHIFT 12
> #define GRUB_PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
> +#define GRUB_PAGE_MASK (~(GRUB_PAGE_SIZE - 1))
>

Why not use signed numbers so it could extend as needed?

> /* The flag for protected mode. */
> #define GRUB_MEMORY_CPU_CR0_PE_ON 0x1
> @@ -43,8 +44,12 @@
>
> #define GRUB_MMAP_MALLOC_LOW 1
>
> +#include <grub/misc.h>
> #include <grub/types.h>
>
> +#define GRUB_PAGE_UP(p) ALIGN_UP (p, GRUB_PAGE_SIZE)
> +#define GRUB_PAGE_DOWN(p) ALIGN_DOWN (p, GRUB_PAGE_SIZE)
> +
> struct grub_e820_mmap_entry
> {
> grub_uint64_t addr;

Frediano

Sergii Dmytruk

unread,
Sep 20, 2024, 12:12:27 PM9/20/24
to The development of GNU GRUB, Frediano Ziglio, trenchbo...@googlegroups.com
On Fri, Sep 20, 2024 at 02:40:22PM +0100, Frediano Ziglio via Grub-devel wrote:
> On Thu, Sep 19, 2024 at 11:03 PM Sergii Dmytruk
> <sergii....@3mdeb.com> wrote:
> >
> > From: Krystian Hebel <krystia...@3mdeb.com>
> >
> > Subsequent patches will use those macros and constant.
> >
>
> Minor, but "Define GRUB_PAGE_MASK constant and GRUB_PAGE_{UP, DOWN}
> macros" subject sounds a bit confusing to me. I mean, at the end they
> are all defined as macros so why mixing constant and macros?

The latter two are function-like macros while the first one isn't and
always expands to the same value. That's how I interpreted the title.

> > Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
> > Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
> > ---
> > include/grub/i386/memory.h | 5 +++++
> > 1 file changed, 5 insertions(+)
> >
> > diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
> > index c64529630..56f64855b 100644
> > --- a/include/grub/i386/memory.h
> > +++ b/include/grub/i386/memory.h
> > @@ -22,6 +22,7 @@
> >
> > #define GRUB_PAGE_SHIFT 12
> > #define GRUB_PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
> > +#define GRUB_PAGE_MASK (~(GRUB_PAGE_SIZE - 1))
> >
>
> Why not use signed numbers so it could extend as needed?

I don't think signed numbers were avoided on purpose, masks just tend to
be unsigned (e.g., to avoid UB on shifts). Can do

#define GRUB_PAGE_MASK (~((1L << GRUB_PAGE_SHIFT) - 1))

instead.

> > /* The flag for protected mode. */
> > #define GRUB_MEMORY_CPU_CR0_PE_ON 0x1
> > @@ -43,8 +44,12 @@
> >
> > #define GRUB_MMAP_MALLOC_LOW 1
> >
> > +#include <grub/misc.h>
> > #include <grub/types.h>
> >
> > +#define GRUB_PAGE_UP(p) ALIGN_UP (p, GRUB_PAGE_SIZE)
> > +#define GRUB_PAGE_DOWN(p) ALIGN_DOWN (p, GRUB_PAGE_SIZE)
> > +
> > struct grub_e820_mmap_entry
> > {
> > grub_uint64_t addr;
>
> Frediano

Sergii

ross.ph...@oracle.com

unread,
Sep 20, 2024, 4:15:29 PM9/20/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/19/24 3:02 PM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> It does not make sense to have separate headers for individual static
> functions. Additionally, we have to add some constants with MSR
> addresses in subsequent patches. So, make one common place to store
> them.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>
> * along with GRUB. If not, see <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!ACWV5N9M2RV99hQ!PhOYc-x84ufXqmXQr2vfpRbZ4VlqZkzY-LEOnFzoJ4Gq1PFNGt-Gh3X9gk07ES2VzMoPmpzAxWtX19IQQvJZt_cL4MXg$ >.
> - * along with GRUB. If not, see <https://urldefense.com/v3/__http://www.gnu.org/licenses/__;!!ACWV5N9M2RV99hQ!PhOYc-x84ufXqmXQr2vfpRbZ4VlqZkzY-LEOnFzoJ4Gq1PFNGt-Gh3X9gk07ES2VzMoPmpzAxWtX19IQQvJZt_cL4MXg$ >.

ross.ph...@oracle.com

unread,
Sep 20, 2024, 4:15:46 PM9/20/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/19/24 3:02 PM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> Use more obvious names which match corresponding instructions:
> * grub_msr_read() => grub_rdmsr()
> * grub_msr_write() => grub_wrmsr()
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

ross.ph...@oracle.com

unread,
Sep 20, 2024, 4:16:23 PM9/20/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/19/24 3:02 PM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> Currently rdmsr and wrmsr commands have own MSR support detection code.
> This code is the same. So, it is duplicated. Additionally, this code
> cannot be reused by others. Hence, extract this code to a function and
> make it public. By the way, improve a code a bit.
>
> Additionally, use GRUB_ERR_BAD_DEVICE instead of GRUB_ERR_BUG to signal
> an error because errors encountered by this new routine are not bugs.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

ross.ph...@oracle.com

unread,
Sep 20, 2024, 4:26:55 PM9/20/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/19/24 3:02 PM, Sergii Dmytruk wrote:
> From: Ross Philipson <ross.ph...@oracle.com>

I can't really give an R-b on this once since it originated with me. I
think it looks fine though perhaps adding something of a commit message
might be desirable, maybe just listing what was added here.

Thanks
Ross
> + * along with GRUB. If not, see <https://urldefense.com/v3/__https://www.gnu.org/licenses/__;!!ACWV5N9M2RV99hQ!OmET6KeJ8ePGpqn99mxcceGKAosl42D1tEaZ0mZErgF8wzcbOkDu7XhmRVfaAHt9NxwJoNqz20y9qZLMY8g1bUGctD5i$ >.
> + * along with GRUB. If not, see <https://urldefense.com/v3/__https://www.gnu.org/licenses/__;!!ACWV5N9M2RV99hQ!OmET6KeJ8ePGpqn99mxcceGKAosl42D1tEaZ0mZErgF8wzcbOkDu7XhmRVfaAHt9NxwJoNqz20y9qZLMY8g1bUGctD5i$ >.

ross.ph...@oracle.com

unread,
Sep 20, 2024, 4:35:18 PM9/20/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/19/24 3:02 PM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> The functions calculate lowest and highest available RAM
> addresses respectively.

It seems that the functions do a bit more than this. They find the
lowest and highest values wrt to the limit you pass in. In the case of
passing a low limit of 0 and a high limit of ~0, it seems the above
functionality would be achieved. The commit message should reflect this.

Thanks
Ross

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:06 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com, Frediano Ziglio, Ross Philipson
Hi,

These are generally useful x86-related changes which were originally posted as
part of DRTM patchset [0]. The changes can also be viewed on GitHub [1].

Best regards,
Sergii

Changes in v2:
- updated commit messages
- added missing Signed-off-by

Changes in v3:
- patch 6: updated commit message and made page mask signed
- patch 7: more accurate commit message
- patch 8: add overview of changes to commit message

[0]: https://lists.gnu.org/archive/html/grub-devel/2024-08/msg00088.html
[1]: https://github.com/TrenchBoot/grub/compare/e1bee15...tb-generic-2.12-72-v3

Daniel Kiper (6):
i386/msr: Merge rdmsr.h and wrmsr.h into msr.h
i386/msr: Rename grub_msr_read() and grub_msr_write()
i386/msr: Extract and improve MSR support detection code
i386/memory: Rename PAGE_SHIFT to GRUB_PAGE_SHIFT
i386/memory: Rename PAGE_SIZE to GRUB_PAGE_SIZE and make it global
mmap: Add grub_mmap_get_lowest() and grub_mmap_get_highest()

Krystian Hebel (1):
i386/memory: Define GRUB_PAGE_MASK and GRUB_PAGE_{UP,DOWN} macros

Ross Philipson (1):
i386: Add CRx, MMIO, MSR and extend CPUID definitions

grub-core/commands/i386/rdmsr.c | 25 ++---
grub-core/commands/i386/wrmsr.c | 25 ++---
grub-core/lib/i386/xen/relocator.S | 6 +-
grub-core/lib/x86_64/xen/relocator.S | 4 +-
grub-core/loader/i386/xen.c | 61 ++++++------
grub-core/mmap/mmap.c | 83 ++++++++++++++++
include/grub/i386/cpuid.h | 11 +++
include/grub/i386/crfr.h | 127 +++++++++++++++++++++++++
include/grub/i386/memory.h | 8 +-
include/grub/i386/mmio.h | 72 ++++++++++++++
include/grub/i386/msr.h | 137 +++++++++++++++++++++++++++
include/grub/i386/rdmsr.h | 37 --------
include/grub/i386/wrmsr.h | 35 -------
include/grub/memory.h | 3 +
14 files changed, 489 insertions(+), 145 deletions(-)
create mode 100644 include/grub/i386/crfr.h
create mode 100644 include/grub/i386/mmio.h
create mode 100644 include/grub/i386/msr.h
delete mode 100644 include/grub/i386/rdmsr.h
delete mode 100644 include/grub/i386/wrmsr.h


base-commit: 9c34d56c2dafcd2737db0e3e49df63bce4d8b504
prerequisite-patch-id: 9255568f81dbfebc40e49a086e40c643ce2e79b9
--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:09 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

It does not make sense to have separate headers for individual static
functions. Additionally, we have to add some constants with MSR
addresses in subsequent patches. So, make one common place to store
them.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
Reviewed-by: Ross Philipson <ross.ph...@oracle.com>
---
grub-core/commands/i386/rdmsr.c | 2 +-
grub-core/commands/i386/wrmsr.c | 2 +-
include/grub/i386/{wrmsr.h => msr.h} | 16 +++++++++---
include/grub/i386/rdmsr.h | 37 ----------------------------
4 files changed, 15 insertions(+), 42 deletions(-)
rename include/grub/i386/{wrmsr.h => msr.h} (78%)
delete mode 100644 include/grub/i386/rdmsr.h

diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index 46c4346da..fa4622f9e 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -26,7 +26,7 @@
#include <grub/extcmd.h>
#include <grub/i18n.h>
#include <grub/i386/cpuid.h>
-#include <grub/i386/rdmsr.h>
+#include <grub/i386/msr.h>

GRUB_MOD_LICENSE("GPLv3+");

diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index 1b143b888..8f352f205 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -27,7 +27,7 @@
#include <grub/lockdown.h>
#include <grub/i18n.h>
#include <grub/i386/cpuid.h>
-#include <grub/i386/wrmsr.h>
+#include <grub/i386/msr.h>

GRUB_MOD_LICENSE("GPLv3+");

diff --git a/include/grub/i386/wrmsr.h b/include/grub/i386/msr.h
similarity index 78%
rename from include/grub/i386/wrmsr.h
rename to include/grub/i386/msr.h
index dea60aed1..7b52b5d61 100644
--- a/include/grub/i386/wrmsr.h
+++ b/include/grub/i386/msr.h
@@ -16,14 +16,24 @@
* along with GRUB. If not, see <http://www.gnu.org/licenses/>.
*/

-#ifndef GRUB_WRMSR_H
-#define GRUB_WRMSR_H 1
+#ifndef GRUB_I386_MSR_H
+#define GRUB_I386_MSR_H 1

/*
* TODO: Add a general protection exception handler.
* Accessing a reserved or unimplemented MSR address results in a GP#.
*/

+static inline grub_uint64_t
+grub_msr_read (grub_uint32_t msr_id)
+{
+ grub_uint32_t low, high;
+
+ asm volatile ("rdmsr" : "=a" (low), "=d" (high) : "c" (msr_id));
+
+ return ((grub_uint64_t) high << 32) | low;
+}
+
static inline void
grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
{
@@ -32,4 +42,4 @@ grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
asm volatile ("wrmsr" : : "c" (msr_id), "a" (low), "d" (high));
}

-#endif /* GRUB_WRMSR_H */
+#endif /* GRUB_I386_MSR_H */
diff --git a/include/grub/i386/rdmsr.h b/include/grub/i386/rdmsr.h
deleted file mode 100644
index c0a0c717a..000000000
--- a/include/grub/i386/rdmsr.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * GRUB -- GRand Unified Bootloader
- * Copyright (C) 2019 Free Software Foundation, Inc.
- *
- * GRUB is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * GRUB is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with GRUB. If not, see <http://www.gnu.org/licenses/>.
- */
-
-#ifndef GRUB_RDMSR_H
-#define GRUB_RDMSR_H 1
-
-/*
- * TODO: Add a general protection exception handler.
- * Accessing a reserved or unimplemented MSR address results in a GP#.
- */
-
-static inline grub_uint64_t
-grub_msr_read (grub_uint32_t msr_id)
-{
- grub_uint32_t low, high;
-
- asm volatile ("rdmsr" : "=a" (low), "=d" (high) : "c" (msr_id));
-
- return ((grub_uint64_t)high << 32) | low;
-}
-
-#endif /* GRUB_RDMSR_H */
--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:14 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

Use more obvious names which match corresponding instructions:
* grub_msr_read() => grub_rdmsr()
* grub_msr_write() => grub_wrmsr()

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
Reviewed-by: Ross Philipson <ross.ph...@oracle.com>
---
grub-core/commands/i386/rdmsr.c | 2 +-
grub-core/commands/i386/wrmsr.c | 2 +-
include/grub/i386/msr.h | 4 ++--
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index fa4622f9e..89ece7657 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -76,7 +76,7 @@ grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char **argv)
if (*ptr != '\0')
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));

- value = grub_msr_read (addr);
+ value = grub_rdmsr (addr);

if (ctxt->state[0].set)
{
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index 8f352f205..cf6bf6c8f 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
@@ -77,7 +77,7 @@ grub_cmd_msr_write (grub_command_t cmd __attribute__ ((unused)), int argc, char
if (*ptr != '\0')
return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("invalid argument"));

- grub_msr_write (addr, value);
+ grub_wrmsr (addr, value);

return GRUB_ERR_NONE;
}
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 7b52b5d61..4fba1b8e0 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -25,7 +25,7 @@
*/

static inline grub_uint64_t
-grub_msr_read (grub_uint32_t msr_id)
+grub_rdmsr (grub_uint32_t msr_id)
{
grub_uint32_t low, high;

@@ -35,7 +35,7 @@ grub_msr_read (grub_uint32_t msr_id)
}

static inline void
-grub_msr_write(grub_uint32_t msr_id, grub_uint64_t msr_value)
+grub_wrmsr (grub_uint32_t msr_id, grub_uint64_t msr_value)
{
grub_uint32_t low = msr_value, high = msr_value >> 32;

--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:16 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

Currently rdmsr and wrmsr commands have own MSR support detection code.
This code is the same. So, it is duplicated. Additionally, this code
cannot be reused by others. Hence, extract this code to a function and
make it public. By the way, improve a code a bit.

Additionally, use GRUB_ERR_BAD_DEVICE instead of GRUB_ERR_BUG to signal
an error because errors encountered by this new routine are not bugs.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
Reviewed-by: Ross Philipson <ross.ph...@oracle.com>
---
grub-core/commands/i386/rdmsr.c | 21 +++++----------------
grub-core/commands/i386/wrmsr.c | 21 +++++----------------
include/grub/i386/msr.h | 29 +++++++++++++++++++++++++++++
3 files changed, 39 insertions(+), 32 deletions(-)

diff --git a/grub-core/commands/i386/rdmsr.c b/grub-core/commands/i386/rdmsr.c
index 89ece7657..2e42f6197 100644
--- a/grub-core/commands/i386/rdmsr.c
+++ b/grub-core/commands/i386/rdmsr.c
@@ -42,27 +42,16 @@ static const struct grub_arg_option options[] =
static grub_err_t
grub_cmd_msr_read (grub_extcmd_context_t ctxt, int argc, char **argv)
diff --git a/grub-core/commands/i386/wrmsr.c b/grub-core/commands/i386/wrmsr.c
index cf6bf6c8f..7fbedaed9 100644
--- a/grub-core/commands/i386/wrmsr.c
+++ b/grub-core/commands/i386/wrmsr.c
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 4fba1b8e0..1e838c022 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -19,6 +19,35 @@
#ifndef GRUB_I386_MSR_H
#define GRUB_I386_MSR_H 1

+#include <grub/err.h>
+#include <grub/i386/cpuid.h>
+#include <grub/types.h>
+
/*
* TODO: Add a general protection exception handler.
* Accessing a reserved or unimplemented MSR address results in a GP#.
--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:18 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

This fixes naming inconsistency that goes against coding style as well
as helps to avoid potential conflicts and confusion.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index 3b856e842..520367732 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -92,7 +92,7 @@ static struct xen_loader_state xen_state;

static grub_dl_t my_mod;

-#define PAGE_SIZE (1UL << PAGE_SHIFT)
+#define PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
#define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
#define STACK_SIZE 1048576
#define ADDITIONAL_SIZE (1 << 19)
xen_state.virt_start_info = get_virtual_current_address (ch);
xen_state.max_addr =
ALIGN_UP (xen_state.max_addr + sizeof (xen_state.next_start), PAGE_SIZE);
- xen_state.console_pfn = xen_state.max_addr >> PAGE_SHIFT;
+ xen_state.console_pfn = xen_state.max_addr >> GRUB_PAGE_SHIFT;
xen_state.max_addr += 2 * PAGE_SIZE;

diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
index 5cb607fb4..7be57d6d7 100644
--- a/include/grub/i386/memory.h
+++ b/include/grub/i386/memory.h
@@ -20,7 +20,7 @@
#ifndef GRUB_MEMORY_CPU_HEADER
#define GRUB_MEMORY_CPU_HEADER 1

-#define PAGE_SHIFT 12
+#define GRUB_PAGE_SHIFT 12

/* The flag for protected mode. */
#define GRUB_MEMORY_CPU_CR0_PE_ON 0x1
--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:20 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

Subsequent patches will use that constant.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
grub-core/loader/i386/xen.c | 35 +++++++++++++++++------------------
include/grub/i386/memory.h | 1 +
2 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/grub-core/loader/i386/xen.c b/grub-core/loader/i386/xen.c
index 520367732..dcdf005df 100644
--- a/grub-core/loader/i386/xen.c
+++ b/grub-core/loader/i386/xen.c
@@ -92,8 +92,7 @@ static struct xen_loader_state xen_state;

static grub_dl_t my_mod;

-#define PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
-#define MAX_MODULES (PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
+#define MAX_MODULES (GRUB_PAGE_SIZE / sizeof (struct xen_multiboot_mod_list))
#define STACK_SIZE 1048576
#define ADDITIONAL_SIZE (1 << 19)
#define ALIGN_SIZE (1 << 22)
@@ -229,7 +228,7 @@ generate_page_table (grub_xen_mfn_t *mfn_list)

for (m1 = 0; m1 < xen_state.n_mappings; m1++)
grub_memset (xen_state.mappings[m1].where, 0,
- xen_state.mappings[m1].area.n_pt_pages * PAGE_SIZE);
+ xen_state.mappings[m1].area.n_pt_pages * GRUB_PAGE_SIZE);

for (l = NUMBER_OF_LEVELS - 1; l >= 0; l--)
{
@@ -324,7 +323,7 @@ grub_xen_p2m_alloc (void)

map = xen_state.mappings + xen_state.n_mappings;
p2msize = ALIGN_UP (sizeof (grub_xen_mfn_t) *
- grub_xen_start_page_addr->nr_pages, PAGE_SIZE);
+ grub_xen_start_page_addr->nr_pages, GRUB_PAGE_SIZE);
if (xen_state.xen_inf.has_p2m_base)
{
err = get_pgtable_size (xen_state.xen_inf.p2m_base,
@@ -380,9 +379,9 @@ grub_xen_special_alloc (void)
xen_state.state.start_info = xen_state.max_addr + xen_state.xen_inf.virt_base;
xen_state.virt_start_info = get_virtual_current_address (ch);
xen_state.max_addr =
- xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, PAGE_SIZE);
+ xen_state.max_addr = ALIGN_UP (xen_state.max_addr + size, GRUB_PAGE_SIZE);


fail:
diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
index 7be57d6d7..c64529630 100644
--- a/include/grub/i386/memory.h
+++ b/include/grub/i386/memory.h
@@ -21,6 +21,7 @@
#define GRUB_MEMORY_CPU_HEADER 1

#define GRUB_PAGE_SHIFT 12
+#define GRUB_PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:22 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Krystian Hebel <krystia...@3mdeb.com>

GRUB_PAGE_MASK is intentionally signed to make use of sign extension.

Subsequent patches will use them.

Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
include/grub/i386/memory.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/include/grub/i386/memory.h b/include/grub/i386/memory.h
index c64529630..ca4509497 100644
--- a/include/grub/i386/memory.h
+++ b/include/grub/i386/memory.h
@@ -22,6 +22,7 @@

#define GRUB_PAGE_SHIFT 12
#define GRUB_PAGE_SIZE (1UL << GRUB_PAGE_SHIFT)
+#define GRUB_PAGE_MASK (~((1L << GRUB_PAGE_SHIFT) - 1))

/* The flag for protected mode. */
#define GRUB_MEMORY_CPU_CR0_PE_ON 0x1
@@ -43,8 +44,12 @@

#define GRUB_MMAP_MALLOC_LOW 1

+#include <grub/misc.h>
#include <grub/types.h>

+#define GRUB_PAGE_UP(p) ALIGN_UP (p, GRUB_PAGE_SIZE)
+#define GRUB_PAGE_DOWN(p) ALIGN_DOWN (p, GRUB_PAGE_SIZE)
+
struct grub_e820_mmap_entry
{
grub_uint64_t addr;
--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:25 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

The functions find the lowest and highest values with regard to the
passed in limit. Passing a low limit of 0 or a high limit of ~0
calculates lowest and highest available RAM addresses respectively.

Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
--
2.46.1

Sergii Dmytruk

unread,
Sep 22, 2024, 1:18:27 PM9/22/24
to grub-...@gnu.org, trenchbo...@googlegroups.com
From: Ross Philipson <ross.ph...@oracle.com>

Control registers and flags:
- CR0 read/write and flags (PE, MP, EM, TS, PG, NE, WP, AM, NW, CD)
- CR4 read/write and flags (VME, PVI, TSD, DE, PSE, PAE, MCE, PGE, PCE,
FXSR, XMM, VMXE, SMXE, PCIDE)
- EFLAGS read/write and flags (CF, PF, AF, ZF, SF, TF, IF, DF, OF,
IOPL, NT, RF, VM, AC, VIF, VIP, ID)

MMIO:
- read/write 8bit values
- read/write 32bit values
- read/write 64bit values

MSRs:
- platform ID
- APIC base
- feature control
- MTRR (capability, bases, masks, types)
- MCG (global machine check; capability, status)
- MISC_ENABLE
- MC0 (machine check error reporting status)
- EFER (LME, LMA, SVEM (AMD-V))
- AMD: SVM control

CPUID:
- flags for availability of vendor, features
- Intel: VMX, SMX
- AMD: SVM

Signed-off-by: Ross Philipson <ross.ph...@oracle.com>
Signed-off-by: Daniel Kiper <daniel...@oracle.com>
Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
---
include/grub/i386/cpuid.h | 11 ++++
include/grub/i386/crfr.h | 127 ++++++++++++++++++++++++++++++++++++++
include/grub/i386/mmio.h | 72 +++++++++++++++++++++
include/grub/i386/msr.h | 63 +++++++++++++++++++
4 files changed, 273 insertions(+)
create mode 100644 include/grub/i386/crfr.h
create mode 100644 include/grub/i386/mmio.h

+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <https://www.gnu.org/licenses/>.
+
+ asm volatile ("mov %%cr0, %0" : "=r" (val) : : "memory");
+
+ return val;
+}
+
+static inline void
+grub_write_cr0 (unsigned long val)
+{
+ asm volatile ("mov %0, %%cr0" : : "r" (val) : "memory");
+}
+
+static inline unsigned long
+grub_read_cr4 (void)
+{
+ unsigned long val;
+
+ asm volatile ("mov %%cr4, %0" : "=r" (val) : : "memory");
+
+ return val;
+}
+
+static inline void
+grub_write_cr4 (unsigned long val)
+{
+ * GRUB -- GRand Unified Bootloader
+ * Copyright (C) 2020 Oracle and/or its affiliates.
+ *
+ * GRUB is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GRUB is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GRUB. If not, see <https://www.gnu.org/licenses/>.
+ */
+
+#ifndef GRUB_I386_MMIO_H
+#define GRUB_I386_MMIO_H 1
+
+#include <grub/types.h>
+
diff --git a/include/grub/i386/msr.h b/include/grub/i386/msr.h
index 1e838c022..05a0fd2f6 100644
--- a/include/grub/i386/msr.h
+++ b/include/grub/i386/msr.h
@@ -2,6 +2,9 @@
* GRUB -- GRand Unified Bootloader
* Copyright (C) 2019 Free Software Foundation, Inc.
*
+ * Some definitions in this header are extracted from the Trusted Computing
+ * Group's "TPM Main Specification", Parts 1-3.
+ *
* GRUB is free software: you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
@@ -19,6 +22,64 @@
#ifndef GRUB_I386_MSR_H
#define GRUB_I386_MSR_H 1

@@ -71,4 +132,6 @@ grub_wrmsr (grub_uint32_t msr_id, grub_uint64_t msr_value)
asm volatile ("wrmsr" : : "c" (msr_id), "a" (low), "d" (high));
}

+#endif /* ASM_FILE */
+
#endif /* GRUB_I386_MSR_H */
--
2.46.1

ross.ph...@oracle.com

unread,
Sep 23, 2024, 12:56:42 PM9/23/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/22/24 10:17 AM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> This fixes naming inconsistency that goes against coding style as well
> as helps to avoid potential conflicts and confusion.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

ross.ph...@oracle.com

unread,
Sep 23, 2024, 1:04:19 PM9/23/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/22/24 10:17 AM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> Subsequent patches will use that constant.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

ross.ph...@oracle.com

unread,
Sep 23, 2024, 1:04:55 PM9/23/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/22/24 10:17 AM, Sergii Dmytruk wrote:
> From: Krystian Hebel <krystia...@3mdeb.com>
>
> GRUB_PAGE_MASK is intentionally signed to make use of sign extension.
>
> Subsequent patches will use them.
>
> Signed-off-by: Krystian Hebel <krystia...@3mdeb.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

ross.ph...@oracle.com

unread,
Sep 23, 2024, 1:05:21 PM9/23/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On 9/22/24 10:17 AM, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> The functions find the lowest and highest values with regard to the
> passed in limit. Passing a low limit of 0 or a high limit of ~0
> calculates lowest and highest available RAM addresses respectively.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

Sergii Dmytruk

unread,
Sep 30, 2024, 9:30:47 AM9/30/24
to The development of GNU GRUB, trenchbo...@googlegroups.com
Ping. 7 other patches of this series already have Reviewed-by.

Best regards,
Sergii

Alec Brown

unread,
Oct 1, 2024, 12:19:34 PM10/1/24
to The development of GNU GRUB, trenchbo...@googlegroups.com
Reviewed-by: Alec Brown <alec.r...@oracle.com>
> <https://urldefense.com/v3/__https://www.gnu.org/licenses/__;!!ACWV5N9M2RV99hQ!Kp-
> 6csQGLflhBa0f91yu7uYC6I4_2epaqrkQiX07KlWjbI_XcuW9yg4OjYxptQIRM_000Klz9NGpDqSuzdu4XXISwiwO$ >.
> <https://urldefense.com/v3/__https://www.gnu.org/licenses/__;!!ACWV5N9M2RV99hQ!Kp-
> 6csQGLflhBa0f91yu7uYC6I4_2epaqrkQiX07KlWjbI_XcuW9yg4OjYxptQIRM_000Klz9NGpDqSuzdu4XXISwiwO$ >.
> _______________________________________________
> Grub-devel mailing list
> Grub-...@gnu.org
> https://urldefense.com/v3/__https://lists.gnu.org/mailman/listinfo/grub-
> devel__;!!ACWV5N9M2RV99hQ!Kp-
> 6csQGLflhBa0f91yu7uYC6I4_2epaqrkQiX07KlWjbI_XcuW9yg4OjYxptQIRM_000Klz9NGpDqSuzdu4Xbyw-yr-$

Daniel Kiper

unread,
Oct 3, 2024, 9:57:02 AM10/3/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On Sun, Sep 22, 2024 at 08:17:47PM +0300, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> It does not make sense to have separate headers for individual static
> functions. Additionally, we have to add some constants with MSR
> addresses in subsequent patches. So, make one common place to store

The second sentence does not make a lot of sense due to lack of DRTM
patches in this patch set. Please drop it. When you do it you can add
my RB below.

> them.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
> Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

Daniel

Daniel Kiper

unread,
Oct 3, 2024, 9:58:42 AM10/3/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On Sun, Sep 22, 2024 at 08:17:48PM +0300, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> Use more obvious names which match corresponding instructions:
> * grub_msr_read() => grub_rdmsr()
> * grub_msr_write() => grub_wrmsr()
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
> Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

Reviewed-by: Daniel Kiper <daniel...@oracle.com>

Daniel

Daniel Kiper

unread,
Oct 3, 2024, 10:34:32 AM10/3/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On Sun, Sep 22, 2024 at 08:17:49PM +0300, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> Currently rdmsr and wrmsr commands have own MSR support detection code.
> This code is the same. So, it is duplicated. Additionally, this code
> cannot be reused by others. Hence, extract this code to a function and
> make it public. By the way, improve a code a bit.
>
> Additionally, use GRUB_ERR_BAD_DEVICE instead of GRUB_ERR_BUG to signal
> an error because errors encountered by this new routine are not bugs.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
> Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

Daniel Kiper

unread,
Oct 3, 2024, 10:35:45 AM10/3/24
to ross.ph...@oracle.com, Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On Mon, Sep 23, 2024 at 09:56:22AM -0700, ross.philipson via trenchboot-devel wrote:
> On 9/22/24 10:17 AM, Sergii Dmytruk wrote:
> > From: Daniel Kiper <daniel...@oracle.com>
> >
> > This fixes naming inconsistency that goes against coding style as well
> > as helps to avoid potential conflicts and confusion.
> >
> > Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> > Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
>
> Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

Daniel Kiper

unread,
Oct 3, 2024, 10:46:38 AM10/3/24
to ross.ph...@oracle.com, Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On Mon, Sep 23, 2024 at 10:04:04AM -0700, ross.philipson via trenchboot-devel wrote:
> On 9/22/24 10:17 AM, Sergii Dmytruk wrote:
> > From: Daniel Kiper <daniel...@oracle.com>
> >
> > Subsequent patches will use that constant.

This sentence is not true due to lack of DRTM patches. I think we should
replace it with something like that:

In general this is x86 specific thing and should be available globally.

Same comment applies to previous patch.

> > Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> > Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>
>
> Reviewed-by: Ross Philipson <ross.ph...@oracle.com>

Daniel Kiper

unread,
Oct 3, 2024, 10:56:30 AM10/3/24
to Sergii Dmytruk, grub-...@gnu.org, trenchbo...@googlegroups.com
On Sun, Sep 22, 2024 at 08:17:53PM +0300, Sergii Dmytruk wrote:
> From: Daniel Kiper <daniel...@oracle.com>
>
> The functions find the lowest and highest values with regard to the
> passed in limit. Passing a low limit of 0 or a high limit of ~0
> calculates lowest and highest available RAM addresses respectively.
>
> Signed-off-by: Daniel Kiper <daniel...@oracle.com>
> Signed-off-by: Sergii Dmytruk <sergii....@3mdeb.com>

This patch and next one should not be a part of this series. I do not
want to have a code in the GRUB which is unused.

Daniel
Reply all
Reply to author
Forward
0 new messages