[PATCH v4 0/6] i386: memory/MSR/CR code updates

0 views
Skip to first unread message

Sergii Dmytruk

unread,
Oct 6, 2024, 10:15:08 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, trenchbo...@googlegroups.com
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:
- patches 1-3: added reviewed-by
- 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

Changes in v4:
- patches 1,4,5: updated commit messages
- patches 1-6: added reviewed-by
- patches 7,8: removed from the series to not add unused code

[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-v4

Daniel Kiper (5):
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

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

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 ++++++++++++++--------------
include/grub/i386/memory.h | 8 +++-
include/grub/i386/{wrmsr.h => msr.h} | 47 +++++++++++++++++++--
include/grub/i386/rdmsr.h | 37 -----------------
8 files changed, 99 insertions(+), 114 deletions(-)
rename include/grub/i386/{wrmsr.h => msr.h} (52%)
delete mode 100644 include/grub/i386/rdmsr.h


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

Sergii Dmytruk

unread,
Oct 6, 2024, 10:15:11 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

It does not make sense to have separate headers for individual static
functions. 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>
Reviewed-by: Daniel Kiper <daniel...@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.2

Sergii Dmytruk

unread,
Oct 6, 2024, 10:15:13 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, 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>
Reviewed-by: Daniel Kiper <daniel...@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.2

Sergii Dmytruk

unread,
Oct 6, 2024, 10:15:16 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, 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>
Reviewed-by: Daniel Kiper <daniel...@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)
{
- 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.2

Sergii Dmytruk

unread,
Oct 6, 2024, 10:15:20 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, 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 as this constant is
used in multiple places.

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>
---
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.2

Sergii Dmytruk

unread,
Oct 6, 2024, 10:15:24 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, trenchbo...@googlegroups.com
From: Daniel Kiper <daniel...@oracle.com>

This is an x86-specific thing and should be available globally.

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>
---
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,
Oct 6, 2024, 10:15:26 AMOct 6
to grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, 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>
Reviewed-by: Ross Philipson <ross.ph...@oracle.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.2

Daniel Kiper

unread,
Oct 8, 2024, 11:38:37 AMOct 8
to Sergii Dmytruk, grub-...@gnu.org, Alex Burmashev, Vladimir 'phcoder' Serbinenko, trenchbo...@googlegroups.com
Hey,

On Sun, Oct 06, 2024 at 05:14:41PM +0300, Sergii Dmytruk wrote:
> 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].

I will take everything up to patch #5. I am not sure why patch #6 is
part of this series... 🤔

Thank you for posting these patches!

Daniel
Reply all
Reply to author
Forward
0 new messages