LinuxLists.cc - [PATCH 0/5] replace magic numbers in GDT descriptors

2023-12-19 15:15:05

Subject: [PATCH 0/5] replace magic numbers in GDT descriptors

Linus suggested replacing the magic numbers in the GDT descriptors
using preprocessor macros [1].

[1] https://lore.kernel.org/all/CAHk-=wib5XLebuEra7y2YH96wxdk=8vJnA8XoVq0FExpzVvN=Q@mail.gmail.com/

For patch 3 ("x86: replace magic numbers in GDT descriptors, part 2") I've
verified it to the best of my abilities on 32-bit and 64-bit by ensuring
that the object files are bitwise identical before and after applying the
patch and ensuring that all the object files were rebuilt on at least one
of the two configs:

32-bit:
arch/x86/boot/pm.o -- no change
arch/x86/kernel/apm_32.o -- no change
arch/x86/kernel/cpu/common.o -- no change
arch/x86/kernel/head64.o -- not built for 32
arch/x86/kernel/setup_percpu.o -- no change
arch/x86/platform/pvh/head.o -- not built for 32
arch/x86/realmode/rm/reboot.o -- no change
drivers/firmware/efi/libstub/x86-5lvl.o -- not built for 32
drivers/pnp/pnpbios/bioscalls.o -- no change

64-bit:
arch/x86/boot/pm.o -- no change
arch/x86/kernel/apm_32.o -- not built for 64
arch/x86/kernel/cpu/common.o -- no change
arch/x86/kernel/head64.o -- no change
arch/x86/kernel/setup_percpu.o -- no change
arch/x86/platform/pvh/head.o -- no change
arch/x86/realmode/rm/reboot.o -- no change
drivers/firmware/efi/libstub/x86-5lvl.o -- no change
drivers/pnp/pnpbios/bioscalls.o -- not built for 64

Patches 2+3 can be squashed to a single commit, but I've submitted them
separately because it makes verifying correctness easier.

I've done basic boot testing on both 32-bit and 64-bit with all of the
patches.

Vegard Nossum (5):
x86: provide new infrastructure for GDT descriptors
x86: replace magic numbers in GDT descriptors, part 1
x86: replace magic numbers in GDT descriptors, part 2
x86: always set A (accessed) flag in GDT descriptors
x86: add DB flag to 32-bit percpu GDT entry

arch/x86/boot/pm.c | 7 +--
arch/x86/include/asm/desc_defs.h | 68 ++++++++++++++++++++++---
arch/x86/kernel/apm_32.c | 2 +-
arch/x86/kernel/cpu/common.c | 50 ++++++++----------
arch/x86/kernel/head64.c | 6 +--
arch/x86/kernel/setup_percpu.c | 4 +-
arch/x86/platform/pvh/head.S | 7 +--
arch/x86/realmode/rm/reboot.S | 3 +-
drivers/firmware/efi/libstub/x86-5lvl.c | 4 +-
drivers/pnp/pnpbios/bioscalls.c | 2 +-
10 files changed, 100 insertions(+), 53 deletions(-)

--
2.34.1

This is the script I used for verifying no binary changes (pass rev of
patch 3 to build as the only argumnet):

#! /bin/bash

set -e
set -u

rev=$(git rev-parse $1)

# get the paths that changed
paths="$(git diff --name-only $rev^- | grep '\.[cS]$' | sed 's/\.[cS]$/.o/')"

build() {
id=$1

# build without patch and save result as <path>.pre-<id>
git checkout -d $rev^
rm -f $paths
make -j128
for p in $paths
do
if [ -e $p ]
then
mv -f $p $p.pre-$id
fi
done

# build with patch and save result as <path>.post-<id>
git checkout -d $rev
rm -f $paths
make -j128
for p in $paths
do
if [ -e $p ]
then
mv -f $p $p.post-$id
fi
done
}

# build i386
make defconfig
scripts/config --disable 64BIT
scripts/config --disable MODULES
# for arch/x86/kernel/apm_32.o
scripts/config --enable APM
# for drivers/pnp/pnpbios/bioscalls.o
scripts/config --enable ISA --enable PNP --enable PNPBIOS
make olddefconfig
build 32

# build x86_64
make defconfig
scripts/config --disable MODULES
# for arch/x86/platform/pvh/head.o
scripts/config --enable XEN --enable XEN_PVHVM --enable PVH
make olddefconfig
build 64

echo
echo results:
echo

for id in 32 64
do
echo $id:

for p in $paths
do
if [ -e $p.pre-$id ] && [ -e $p.post-$id ]
then
if cmp -s $p.pre-$id $p.post-$id
then
echo $p -- no change
else
echo $p -- differences:
diff -U3 <(objdump -dr $p.pre-$id | tail -n +3) <(objdump -dr $p.post-$id | tail -n +3) || true
fi
else
echo $p -- not built for $id
fi
done

echo
done

2023-12-19 15:16:20

by Vegard Nossum

[permalink] [raw]

Subject: [PATCH 2/5] x86: replace magic numbers in GDT descriptors, part 1

We'd like to replace all the magic numbers in various GDT descriptors
with new, semantically meaningful, symbolic values.

In order to be able to verify that the change doesn't cause any actual
changes to the compiled binary code, I've split the change into two
patches:

Part 1 (this commit): everything _but_ actually replacing the numbers
Part 2 (the following commit): _only_ replacing the numbers

These two commits may be squashed together when merged.

The reason we need this split for verification is that including new
headers causes some spurious changes to the object files, mostly line
number changes in the debug info but occasionally other subtle codegen
changes.

Link: https://lore.kernel.org/all/CAHk-=wib5XLebuEra7y2YH96wxdk=8vJnA8XoVq0FExpzVvN=Q@mail.gmail.com/
Signed-off-by: Vegard Nossum <[email protected]>
---
arch/x86/boot/pm.c | 1 +
arch/x86/include/asm/desc_defs.h | 2 ++
arch/x86/kernel/cpu/common.c | 8 --------
arch/x86/platform/pvh/head.S | 1 +
arch/x86/realmode/rm/reboot.S | 1 +
5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
index 40031a614712..0361b5307bd8 100644
--- a/arch/x86/boot/pm.c
+++ b/arch/x86/boot/pm.c
@@ -11,6 +11,7 @@
*/

#include "boot.h"
+#include <asm/desc_defs.h>
#include <asm/segment.h>

/*
diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index b33f5bb240eb..014878e584fe 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -144,6 +144,7 @@ struct gate_struct {

typedef struct gate_struct gate_desc;

+#ifndef _SETUP
static inline unsigned long gate_offset(const gate_desc *g)
{
#ifdef CONFIG_X86_64
@@ -158,6 +159,7 @@ static inline unsigned long gate_segment(const gate_desc *g)
{
return g->segment;
}
+#endif

struct desc_ptr {
unsigned short size;
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index b14fc8c1c953..ceb6e4b6d57e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -204,25 +204,17 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page) = { .gdt = {
* They code segments and data segments have fixed 64k limits,
* the transfer segment sizes are set at run time.
*/
- /* 32-bit code */
[GDT_ENTRY_PNPBIOS_CS32] = GDT_ENTRY_INIT(0x409a, 0, 0xffff),
- /* 16-bit code */
[GDT_ENTRY_PNPBIOS_CS16] = GDT_ENTRY_INIT(0x009a, 0, 0xffff),
- /* 16-bit data */
[GDT_ENTRY_PNPBIOS_DS] = GDT_ENTRY_INIT(0x0092, 0, 0xffff),
- /* 16-bit data */
[GDT_ENTRY_PNPBIOS_TS1] = GDT_ENTRY_INIT(0x0092, 0, 0),
- /* 16-bit data */
[GDT_ENTRY_PNPBIOS_TS2] = GDT_ENTRY_INIT(0x0092, 0, 0),
/*
* The APM segments have byte granularity and their bases
* are set at run time. All have 64k limits.
*/
- /* 32-bit code */
[GDT_ENTRY_APMBIOS_BASE] = GDT_ENTRY_INIT(0x409a, 0, 0xffff),
- /* 16-bit code */
[GDT_ENTRY_APMBIOS_BASE+1] = GDT_ENTRY_INIT(0x009a, 0, 0xffff),
- /* data */
[GDT_ENTRY_APMBIOS_BASE+2] = GDT_ENTRY_INIT(0x4092, 0, 0xffff),

[GDT_ENTRY_ESPFIX_SS] = GDT_ENTRY_INIT(0xc092, 0, 0xfffff),
diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S
index c4365a05ab83..9bcafdded2a1 100644
--- a/arch/x86/platform/pvh/head.S
+++ b/arch/x86/platform/pvh/head.S
@@ -11,6 +11,7 @@
#include <linux/elfnote.h>
#include <linux/init.h>
#include <linux/linkage.h>
+#include <asm/desc_defs.h>
#include <asm/segment.h>
#include <asm/asm.h>
#include <asm/boot.h>
diff --git a/arch/x86/realmode/rm/reboot.S b/arch/x86/realmode/rm/reboot.S
index f10515b10e0a..447641820a8d 100644
--- a/arch/x86/realmode/rm/reboot.S
+++ b/arch/x86/realmode/rm/reboot.S
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
#include <linux/linkage.h>
+#include <asm/desc_defs.h>
#include <asm/segment.h>
#include <asm/page_types.h>
#include <asm/processor-flags.h>
--
2.34.1

2023-12-19 15:16:42

by Vegard Nossum

[permalink] [raw]

Subject: [PATCH 4/5] x86: always set A (accessed) flag in GDT descriptors

We have no known use for having the CPU track whether GDT descriptors
have been accessed or not.

Simplify the code by adding the flag to the common flags and removing
it everywhere else.

Link: https://lore.kernel.org/all/CAHk-=wib5XLebuEra7y2YH96wxdk=8vJnA8XoVq0FExpzVvN=Q@mail.gmail.com/
Signed-off-by: Vegard Nossum <[email protected]>
---
arch/x86/boot/pm.c | 4 ++--
arch/x86/include/asm/desc_defs.h | 4 ++--
arch/x86/kernel/cpu/common.c | 12 ++++++------
arch/x86/kernel/head64.c | 6 +++---
arch/x86/realmode/rm/reboot.S | 2 +-
drivers/firmware/efi/libstub/x86-5lvl.c | 4 ++--
6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
index ab35b52d2c4b..5941f930f6c5 100644
--- a/arch/x86/boot/pm.c
+++ b/arch/x86/boot/pm.c
@@ -68,9 +68,9 @@ static void setup_gdt(void)
being 8-byte unaligned. Intel recommends 16 byte alignment. */
static const u64 boot_gdt[] __attribute__((aligned(16))) = {
/* CS: code, read/execute, 4 GB, base 0 */
- [GDT_ENTRY_BOOT_CS] = GDT_ENTRY(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_BOOT_CS] = GDT_ENTRY(DESC_CODE32, 0, 0xfffff),
/* DS: data, read/write, 4 GB, base 0 */
- [GDT_ENTRY_BOOT_DS] = GDT_ENTRY(DESC_DATA32 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_BOOT_DS] = GDT_ENTRY(DESC_DATA32, 0, 0xfffff),
/* TSS: 32-bit tss, 104 bytes, base 4096 */
/* We only have a TSS here to keep Intel VT happy;
we don't actually use it for anything. */
diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index 014878e584fe..f9282bcb0a91 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -37,9 +37,9 @@
* of flags
*/

-#define _DESC_DATA (_DESC_S | _DESC_PRESENT | \
+#define _DESC_DATA (_DESC_S | _DESC_PRESENT | _DESC_ACCESSED | \
_DESC_DATA_WRITABLE)
-#define _DESC_CODE (_DESC_S | _DESC_PRESENT | \
+#define _DESC_CODE (_DESC_S | _DESC_PRESENT | _DESC_ACCESSED | \
_DESC_CODE_READABLE | _DESC_CODE_EXECUTABLE)

#define DESC_DATA16 (_DESC_DATA)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 32934a0656af..6184488a7d77 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -188,12 +188,12 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page) = { .gdt = {
* TLS descriptors are currently at a different place compared to i386.
* Hopefully nobody expects them at a fixed place (Wine?)
*/
- [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_DEFAULT_USER32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | DESC_USER | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_DEFAULT_USER_DS] = GDT_ENTRY_INIT(DESC_DATA64 | DESC_USER | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_DEFAULT_USER_CS] = GDT_ENTRY_INIT(DESC_CODE64 | DESC_USER | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64, 0, 0xfffff),
+ [GDT_ENTRY_DEFAULT_USER32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | DESC_USER, 0, 0xfffff),
+ [GDT_ENTRY_DEFAULT_USER_DS] = GDT_ENTRY_INIT(DESC_DATA64 | DESC_USER, 0, 0xfffff),
+ [GDT_ENTRY_DEFAULT_USER_CS] = GDT_ENTRY_INIT(DESC_CODE64 | DESC_USER, 0, 0xfffff),
#else
[GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
[GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA32, 0, 0xfffff),
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 00dbddfdfece..dc0956067944 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -71,9 +71,9 @@ EXPORT_SYMBOL(vmemmap_base);
* GDT used on the boot CPU before switching to virtual addresses.
*/
static struct desc_struct startup_gdt[GDT_ENTRIES] __initdata = {
- [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64, 0, 0xfffff),
};

/*
diff --git a/arch/x86/realmode/rm/reboot.S b/arch/x86/realmode/rm/reboot.S
index 5bc068b9acdd..e714b4624e36 100644
--- a/arch/x86/realmode/rm/reboot.S
+++ b/arch/x86/realmode/rm/reboot.S
@@ -154,5 +154,5 @@ SYM_DATA_START(machine_real_restart_gdt)
* base value 0x100; since this is consistent with real mode
* semantics we don't have to reload the segments once CR0.PE = 0.
*/
- .quad GDT_ENTRY(DESC_DATA16 | _DESC_ACCESSED, 0x100, 0xffff)
+ .quad GDT_ENTRY(DESC_DATA16, 0x100, 0xffff)
SYM_DATA_END(machine_real_restart_gdt)
diff --git a/drivers/firmware/efi/libstub/x86-5lvl.c b/drivers/firmware/efi/libstub/x86-5lvl.c
index 005dd9b14f95..77359e802181 100644
--- a/drivers/firmware/efi/libstub/x86-5lvl.c
+++ b/drivers/firmware/efi/libstub/x86-5lvl.c
@@ -13,8 +13,8 @@ bool efi_no5lvl;
static void (*la57_toggle)(void *cr3);

static const struct desc_struct gdt[] = {
- [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64, 0, 0xfffff),
};

/*
--
2.34.1

2023-12-19 15:20:48

by Vegard Nossum

[permalink] [raw]

Subject: [PATCH 1/5] x86: provide new infrastructure for GDT descriptors

Linus suggested replacing the magic numbers in the GDT descriptors
using preprocessor macros. Designing the interface properly is actually
pretty hard -- there are several constraints:

- you want the final expressions to be readable at a glance; something
like GDT_ENTRY_FLAGS(5, 1, 0, 1, 0, 1, 1, 0) isn't because you need
to visit the definition to understand what each parameter represents
and then match up parameters in the user and the definition (which is
hard when there are so many of them)

- you want the final expressions to be fairly short/information-dense;
something like GDT_ENTRY_PRESENT | GDT_ENTRY_DATA_WRITABLE |
GDT_ENTRY_SYSTEM | GDT_ENTRY_DB | GDT_ENTRY_GRANULARITY_4K is a bit
too verbose to write out every time and is actually hard to read as
well because of all the repetition

- you may want to assume defaults for some things (e.g. entries are
DPL-0 a.k.a. kernel segments by default) and allow the user to
override the default -- but this works best if you can OR in the
override; if you want DPL-3 by default and override with DPL-0 you
would need to start masking off bits instead of OR-ing them in and
that just becomes harder to read

- you may want to parameterize some things (e.g. CODE vs. DATA or
KERNEL vs. USER) since both values are used and you don't really
want prefer either one by default -- or DPL, which is always some
value that is always specified

This patch tries to balance these requirements and has two layers of
definitions -- low-level and high-level:

- the low-level defines are the mapping between human-readable names
and the actual bit numbers

- the high-level defines are the mapping from high-level intent to
combinations of low-level flags, representing roughly a tuple
(data/code/tss, 64/32/16-bits) plus an override for DPL-3 (= USER),
since that's relatively rare but still very important to mark
properly for those segments.

- we have *_BIOS variants for 32-bit code and data segments that don't
have the G flag set and give the limit in terms of bytes instead of
pages

Link: https://lore.kernel.org/all/CAHk-=wib5XLebuEra7y2YH96wxdk=8vJnA8XoVq0FExpzVvN=Q@mail.gmail.com/
Signed-off-by: Vegard Nossum <[email protected]>
---
arch/x86/include/asm/desc_defs.h | 66 ++++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index f7e7099af595..b33f5bb240eb 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -8,6 +8,56 @@
* archs.
*/

+/*
+ * Low-level interface mapping flags/field names to bits
+ */
+
+/* Flags for _DESC_S (non-system) descriptors */
+#define _DESC_ACCESSED 0x0001
+#define _DESC_DATA_WRITABLE 0x0002
+#define _DESC_CODE_READABLE 0x0002
+#define _DESC_DATA_EXPAND_DOWN 0x0004
+#define _DESC_CODE_CONFORMING 0x0004
+#define _DESC_CODE_EXECUTABLE 0x0008
+
+/* Common flags */
+#define _DESC_S 0x0010
+#define _DESC_DPL(dpl) ((dpl) << 5)
+#define _DESC_PRESENT 0x0080
+
+#define _DESC_LONG_CODE 0x2000
+#define _DESC_DB 0x4000
+#define _DESC_GRANULARITY_4K 0x8000
+
+/* System descriptors have a numeric "type" field instead of flags */
+#define _DESC_SYSTEM(code) (code)
+
+/*
+ * High-level interface mapping intended usage to low-level combinations
+ * of flags
+ */
+
+#define _DESC_DATA (_DESC_S | _DESC_PRESENT | \
+ _DESC_DATA_WRITABLE)
+#define _DESC_CODE (_DESC_S | _DESC_PRESENT | \
+ _DESC_CODE_READABLE | _DESC_CODE_EXECUTABLE)
+
+#define DESC_DATA16 (_DESC_DATA)
+#define DESC_CODE16 (_DESC_CODE)
+
+#define DESC_DATA32 (_DESC_DATA | _DESC_GRANULARITY_4K | _DESC_DB)
+#define DESC_DATA32_BIOS (_DESC_DATA | _DESC_DB)
+
+#define DESC_CODE32 (_DESC_CODE | _DESC_GRANULARITY_4K | _DESC_DB)
+#define DESC_CODE32_BIOS (_DESC_CODE | _DESC_DB)
+
+#define DESC_TSS32 (_DESC_SYSTEM(9) | _DESC_PRESENT)
+
+#define DESC_DATA64 (_DESC_DATA | _DESC_GRANULARITY_4K | _DESC_DB)
+#define DESC_CODE64 (_DESC_CODE | _DESC_GRANULARITY_4K | _DESC_LONG_CODE)
+
+#define DESC_USER (_DESC_DPL(3))
+
#ifndef __ASSEMBLY__

#include <linux/types.h>
@@ -27,14 +77,14 @@ struct desc_struct {
.base0 = (u16) (base), \
.base1 = ((base) >> 16) & 0xFF, \
.base2 = ((base) >> 24) & 0xFF, \
- .type = (flags & 0x0f), \
- .s = (flags >> 4) & 0x01, \
- .dpl = (flags >> 5) & 0x03, \
- .p = (flags >> 7) & 0x01, \
- .avl = (flags >> 12) & 0x01, \
- .l = (flags >> 13) & 0x01, \
- .d = (flags >> 14) & 0x01, \
- .g = (flags >> 15) & 0x01, \
+ .type = ((flags) & 0x0f), \
+ .s = ((flags) >> 4) & 0x01, \
+ .dpl = ((flags) >> 5) & 0x03, \
+ .p = ((flags) >> 7) & 0x01, \
+ .avl = ((flags) >> 12) & 0x01, \
+ .l = ((flags) >> 13) & 0x01, \
+ .d = ((flags) >> 14) & 0x01, \
+ .g = ((flags) >> 15) & 0x01, \
}

enum {
--
2.34.1

2023-12-19 17:34:08

by Linus Torvalds

[permalink] [raw]

Subject: Re: [PATCH 0/5] replace magic numbers in GDT descriptors

On Tue, 19 Dec 2023 at 07:12, Vegard Nossum <[email protected]> wrote:
>
> Vegard Nossum (5):
> x86: provide new infrastructure for GDT descriptors
> x86: replace magic numbers in GDT descriptors, part 1
> x86: replace magic numbers in GDT descriptors, part 2
> x86: always set A (accessed) flag in GDT descriptors
> x86: add DB flag to 32-bit percpu GDT entry

All these patches look fine to me, but I will again leave it to the
x86 maintainers whether they want to apply them. But feel free to add
my Ack if y ou do.

The end result does look a *lot* more legible, with something like

DESC_DATA64 | DESC_USER

instead of just a raw number like 0xc0f3.

So while this is unlikely to be a maintenance burden (since we look at
these things so seldom, and they never really change), I think it's a
nice readability improvement.

The fact that Vegard found two oddities while doing this series just
reinforces that readability issue. Neither of them were bugs, but they
were odd inconsistencies.

Linus

2023-12-20 09:51:34

by Ingo Molnar

[permalink] [raw]

Subject: Re: [PATCH 1/5] x86: provide new infrastructure for GDT descriptors

* Vegard Nossum <[email protected]> wrote:

> @@ -27,14 +77,14 @@ struct desc_struct {
> .base0 = (u16) (base), \
> .base1 = ((base) >> 16) & 0xFF, \
> .base2 = ((base) >> 24) & 0xFF, \
> - .type = (flags & 0x0f), \
> - .s = (flags >> 4) & 0x01, \
> - .dpl = (flags >> 5) & 0x03, \
> - .p = (flags >> 7) & 0x01, \
> - .avl = (flags >> 12) & 0x01, \
> - .l = (flags >> 13) & 0x01, \
> - .d = (flags >> 14) & 0x01, \
> - .g = (flags >> 15) & 0x01, \
> + .type = ((flags) & 0x0f), \
> + .s = ((flags) >> 4) & 0x01, \
> + .dpl = ((flags) >> 5) & 0x03, \
> + .p = ((flags) >> 7) & 0x01, \
> + .avl = ((flags) >> 12) & 0x01, \
> + .l = ((flags) >> 13) & 0x01, \
> + .d = ((flags) >> 14) & 0x01, \
> + .g = ((flags) >> 15) & 0x01, \

Yeah, so how about writing it like this:

#define GDT_ENTRY_INIT(flags, base, limit) \
{ \
.limit0 = ((limit) >> 0) & 0xFFFF, \
.limit1 = ((limit) >> 16) & 0x000F, \
.base0 = ((base) >> 0) & 0xFFFF, \
.base1 = ((base) >> 16) & 0x00FF, \
.base2 = ((base) >> 24) & 0x00FF, \
.type = ((flags) >> 0) & 0x000F, \
.s = ((flags) >> 4) & 0x0001, \
.dpl = ((flags) >> 5) & 0x0003, \
.p = ((flags) >> 7) & 0x0001, \
.avl = ((flags) >> 12) & 0x0001, \
.l = ((flags) >> 13) & 0x0001, \
.d = ((flags) >> 14) & 0x0001, \
.g = ((flags) >> 15) & 0x0001, \
}

This encodes it all in a very simple format, without C syntactic
variations: bit position and mask.

Thanks,

Ingo

2023-12-20 09:59:19

by Ingo Molnar

[permalink] [raw]

Subject: Re: [PATCH 0/5] replace magic numbers in GDT descriptors

* Linus Torvalds <[email protected]> wrote:

> On Tue, 19 Dec 2023 at 07:12, Vegard Nossum <[email protected]> wrote:
> >
> > Vegard Nossum (5):
> > x86: provide new infrastructure for GDT descriptors
> > x86: replace magic numbers in GDT descriptors, part 1
> > x86: replace magic numbers in GDT descriptors, part 2
> > x86: always set A (accessed) flag in GDT descriptors
> > x86: add DB flag to 32-bit percpu GDT entry
>
> All these patches look fine to me, but I will again leave it to the
> x86 maintainers whether they want to apply them. But feel free to add
> my Ack if y ou do.

Thanks - I've applied your acks. These are obviously useful cleanups,
so no objections whatsoever.

> The end result does look a *lot* more legible, with something like
>
> DESC_DATA64 | DESC_USER
>
> instead of just a raw number like 0xc0f3.
>
> So while this is unlikely to be a maintenance burden (since we look at
> these things so seldom, and they never really change), I think it's a
> nice readability improvement.

Yeah, absolutely.

> The fact that Vegard found two oddities while doing this series just
> reinforces that readability issue. Neither of them were bugs, but they
> were odd inconsistencies.

Indeed ...

Thanks,

Ingo

2023-12-20 10:06:00

by tip-bot2 for Dhananjay Ugwekar

[permalink] [raw]

Subject: [tip: x86/asm] x86/asm: Always set A (accessed) flag in GDT descriptors

The following commit has been merged into the x86/asm branch of tip:

Commit-ID: 3b184b71dfcb156e08246f8fbe0cd088c6a6efed
Gitweb: https://git.kernel.org/tip/3b184b71dfcb156e08246f8fbe0cd088c6a6efed
Author: Vegard Nossum <[email protected]>
AuthorDate: Tue, 19 Dec 2023 16:11:59 +01:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 20 Dec 2023 10:57:51 +01:00

x86/asm: Always set A (accessed) flag in GDT descriptors

We have no known use for having the CPU track whether GDT descriptors
have been accessed or not.

Simplify the code by adding the flag to the common flags and removing
it everywhere else.

Signed-off-by: Vegard Nossum <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Acked-by: Linus Torvalds <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/boot/pm.c | 4 ++--
arch/x86/include/asm/desc_defs.h | 4 ++--
arch/x86/kernel/cpu/common.c | 12 ++++++------
arch/x86/kernel/head64.c | 6 +++---
arch/x86/realmode/rm/reboot.S | 2 +-
drivers/firmware/efi/libstub/x86-5lvl.c | 4 ++--
6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
index ab35b52..5941f93 100644
--- a/arch/x86/boot/pm.c
+++ b/arch/x86/boot/pm.c
@@ -68,9 +68,9 @@ static void setup_gdt(void)
being 8-byte unaligned. Intel recommends 16 byte alignment. */
static const u64 boot_gdt[] __attribute__((aligned(16))) = {
/* CS: code, read/execute, 4 GB, base 0 */
- [GDT_ENTRY_BOOT_CS] = GDT_ENTRY(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_BOOT_CS] = GDT_ENTRY(DESC_CODE32, 0, 0xfffff),
/* DS: data, read/write, 4 GB, base 0 */
- [GDT_ENTRY_BOOT_DS] = GDT_ENTRY(DESC_DATA32 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_BOOT_DS] = GDT_ENTRY(DESC_DATA32, 0, 0xfffff),
/* TSS: 32-bit tss, 104 bytes, base 4096 */
/* We only have a TSS here to keep Intel VT happy;
we don't actually use it for anything. */
diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index 33d229e..d440a65 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -37,9 +37,9 @@
* of flags
*/

-#define _DESC_DATA (_DESC_S | _DESC_PRESENT | \
+#define _DESC_DATA (_DESC_S | _DESC_PRESENT | _DESC_ACCESSED | \
_DESC_DATA_WRITABLE)
-#define _DESC_CODE (_DESC_S | _DESC_PRESENT | \
+#define _DESC_CODE (_DESC_S | _DESC_PRESENT | _DESC_ACCESSED | \
_DESC_CODE_READABLE | _DESC_CODE_EXECUTABLE)

#define DESC_DATA16 (_DESC_DATA)
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 32934a0..6184488 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -188,12 +188,12 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page) = { .gdt = {
* TLS descriptors are currently at a different place compared to i386.
* Hopefully nobody expects them at a fixed place (Wine?)
*/
- [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_DEFAULT_USER32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | DESC_USER | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_DEFAULT_USER_DS] = GDT_ENTRY_INIT(DESC_DATA64 | DESC_USER | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_DEFAULT_USER_CS] = GDT_ENTRY_INIT(DESC_CODE64 | DESC_USER | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64, 0, 0xfffff),
+ [GDT_ENTRY_DEFAULT_USER32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | DESC_USER, 0, 0xfffff),
+ [GDT_ENTRY_DEFAULT_USER_DS] = GDT_ENTRY_INIT(DESC_DATA64 | DESC_USER, 0, 0xfffff),
+ [GDT_ENTRY_DEFAULT_USER_CS] = GDT_ENTRY_INIT(DESC_CODE64 | DESC_USER, 0, 0xfffff),
#else
[GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
[GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA32, 0, 0xfffff),
diff --git a/arch/x86/kernel/head64.c b/arch/x86/kernel/head64.c
index 00dbddf..dc09560 100644
--- a/arch/x86/kernel/head64.c
+++ b/arch/x86/kernel/head64.c
@@ -71,9 +71,9 @@ EXPORT_SYMBOL(vmemmap_base);
* GDT used on the boot CPU before switching to virtual addresses.
*/
static struct desc_struct startup_gdt[GDT_ENTRIES] __initdata = {
- [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_DS] = GDT_ENTRY_INIT(DESC_DATA64, 0, 0xfffff),
};

/*
diff --git a/arch/x86/realmode/rm/reboot.S b/arch/x86/realmode/rm/reboot.S
index 5bc068b..e714b46 100644
--- a/arch/x86/realmode/rm/reboot.S
+++ b/arch/x86/realmode/rm/reboot.S
@@ -154,5 +154,5 @@ SYM_DATA_START(machine_real_restart_gdt)
* base value 0x100; since this is consistent with real mode
* semantics we don't have to reload the segments once CR0.PE = 0.
*/
- .quad GDT_ENTRY(DESC_DATA16 | _DESC_ACCESSED, 0x100, 0xffff)
+ .quad GDT_ENTRY(DESC_DATA16, 0x100, 0xffff)
SYM_DATA_END(machine_real_restart_gdt)
diff --git a/drivers/firmware/efi/libstub/x86-5lvl.c b/drivers/firmware/efi/libstub/x86-5lvl.c
index 005dd9b..77359e8 100644
--- a/drivers/firmware/efi/libstub/x86-5lvl.c
+++ b/drivers/firmware/efi/libstub/x86-5lvl.c
@@ -13,8 +13,8 @@ bool efi_no5lvl;
static void (*la57_toggle)(void *cr3);

static const struct desc_struct gdt[] = {
- [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32 | _DESC_ACCESSED, 0, 0xfffff),
- [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64 | _DESC_ACCESSED, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL32_CS] = GDT_ENTRY_INIT(DESC_CODE32, 0, 0xfffff),
+ [GDT_ENTRY_KERNEL_CS] = GDT_ENTRY_INIT(DESC_CODE64, 0, 0xfffff),
};

/*

2023-12-20 10:06:22

by tip-bot2 for Dhananjay Ugwekar

[permalink] [raw]

Subject: [tip: x86/asm] x86/asm: Replace magic numbers in GDT descriptors, preparations

The following commit has been merged into the x86/asm branch of tip:

Commit-ID: 41ef75c848e33beb1f7b981866b62b0066f744c7
Gitweb: https://git.kernel.org/tip/41ef75c848e33beb1f7b981866b62b0066f744c7
Author: Vegard Nossum <[email protected]>
AuthorDate: Tue, 19 Dec 2023 16:11:57 +01:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 20 Dec 2023 10:57:20 +01:00

x86/asm: Replace magic numbers in GDT descriptors, preparations

We'd like to replace all the magic numbers in various GDT descriptors
with new, semantically meaningful, symbolic values.

In order to be able to verify that the change doesn't cause any actual
changes to the compiled binary code, I've split the change into two
patches:

- Part 1 (this commit): everything _but_ actually replacing the numbers
- Part 2 (the following commit): _only_ replacing the numbers

The reason we need this split for verification is that including new
headers causes some spurious changes to the object files, mostly line
number changes in the debug info but occasionally other subtle codegen
changes.

Signed-off-by: Vegard Nossum <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Acked-by: Linus Torvalds <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/boot/pm.c | 1 +
arch/x86/include/asm/desc_defs.h | 2 ++
arch/x86/kernel/cpu/common.c | 8 --------
arch/x86/platform/pvh/head.S | 1 +
arch/x86/realmode/rm/reboot.S | 1 +
5 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/arch/x86/boot/pm.c b/arch/x86/boot/pm.c
index 40031a6..0361b53 100644
--- a/arch/x86/boot/pm.c
+++ b/arch/x86/boot/pm.c
@@ -11,6 +11,7 @@
*/

#include "boot.h"
+#include <asm/desc_defs.h>
#include <asm/segment.h>

/*
diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index 7c08cbf..33d229e 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -144,6 +144,7 @@ struct gate_struct {

typedef struct gate_struct gate_desc;

+#ifndef _SETUP
static inline unsigned long gate_offset(const gate_desc *g)
{
#ifdef CONFIG_X86_64
@@ -158,6 +159,7 @@ static inline unsigned long gate_segment(const gate_desc *g)
{
return g->segment;
}
+#endif

struct desc_ptr {
unsigned short size;
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index b14fc8c..ceb6e4b 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -204,25 +204,17 @@ DEFINE_PER_CPU_PAGE_ALIGNED(struct gdt_page, gdt_page) = { .gdt = {
* They code segments and data segments have fixed 64k limits,
* the transfer segment sizes are set at run time.
*/
- /* 32-bit code */
[GDT_ENTRY_PNPBIOS_CS32] = GDT_ENTRY_INIT(0x409a, 0, 0xffff),
- /* 16-bit code */
[GDT_ENTRY_PNPBIOS_CS16] = GDT_ENTRY_INIT(0x009a, 0, 0xffff),
- /* 16-bit data */
[GDT_ENTRY_PNPBIOS_DS] = GDT_ENTRY_INIT(0x0092, 0, 0xffff),
- /* 16-bit data */
[GDT_ENTRY_PNPBIOS_TS1] = GDT_ENTRY_INIT(0x0092, 0, 0),
- /* 16-bit data */
[GDT_ENTRY_PNPBIOS_TS2] = GDT_ENTRY_INIT(0x0092, 0, 0),
/*
* The APM segments have byte granularity and their bases
* are set at run time. All have 64k limits.
*/
- /* 32-bit code */
[GDT_ENTRY_APMBIOS_BASE] = GDT_ENTRY_INIT(0x409a, 0, 0xffff),
- /* 16-bit code */
[GDT_ENTRY_APMBIOS_BASE+1] = GDT_ENTRY_INIT(0x009a, 0, 0xffff),
- /* data */
[GDT_ENTRY_APMBIOS_BASE+2] = GDT_ENTRY_INIT(0x4092, 0, 0xffff),

[GDT_ENTRY_ESPFIX_SS] = GDT_ENTRY_INIT(0xc092, 0, 0xfffff),
diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S
index c4365a0..9bcafdd 100644
--- a/arch/x86/platform/pvh/head.S
+++ b/arch/x86/platform/pvh/head.S
@@ -11,6 +11,7 @@
#include <linux/elfnote.h>
#include <linux/init.h>
#include <linux/linkage.h>
+#include <asm/desc_defs.h>
#include <asm/segment.h>
#include <asm/asm.h>
#include <asm/boot.h>
diff --git a/arch/x86/realmode/rm/reboot.S b/arch/x86/realmode/rm/reboot.S
index f10515b..4476418 100644
--- a/arch/x86/realmode/rm/reboot.S
+++ b/arch/x86/realmode/rm/reboot.S
@@ -1,5 +1,6 @@
/* SPDX-License-Identifier: GPL-2.0 */
#include <linux/linkage.h>
+#include <asm/desc_defs.h>
#include <asm/segment.h>
#include <asm/page_types.h>
#include <asm/processor-flags.h>

2023-12-20 10:06:43

by tip-bot2 for Dhananjay Ugwekar

[permalink] [raw]

Subject: [tip: x86/asm] x86/asm: Provide new infrastructure for GDT descriptors

The following commit has been merged into the x86/asm branch of tip:

Commit-ID: 016919c1f2e5b7ea3436abe6db0b73dbabd36682
Gitweb: https://git.kernel.org/tip/016919c1f2e5b7ea3436abe6db0b73dbabd36682
Author: Vegard Nossum <[email protected]>
AuthorDate: Tue, 19 Dec 2023 16:11:56 +01:00
Committer: Ingo Molnar <[email protected]>
CommitterDate: Wed, 20 Dec 2023 10:56:04 +01:00

x86/asm: Provide new infrastructure for GDT descriptors

Linus suggested replacing the magic numbers in the GDT descriptors
using preprocessor macros. Designing the interface properly is actually
pretty hard -- there are several constraints:

- you want the final expressions to be readable at a glance; something
like GDT_ENTRY_FLAGS(5, 1, 0, 1, 0, 1, 1, 0) isn't because you need
to visit the definition to understand what each parameter represents
and then match up parameters in the user and the definition (which is
hard when there are so many of them)

- you want the final expressions to be fairly short/information-dense;
something like GDT_ENTRY_PRESENT | GDT_ENTRY_DATA_WRITABLE |
GDT_ENTRY_SYSTEM | GDT_ENTRY_DB | GDT_ENTRY_GRANULARITY_4K is a bit
too verbose to write out every time and is actually hard to read as
well because of all the repetition

- you may want to assume defaults for some things (e.g. entries are
DPL-0 a.k.a. kernel segments by default) and allow the user to
override the default -- but this works best if you can OR in the
override; if you want DPL-3 by default and override with DPL-0 you
would need to start masking off bits instead of OR-ing them in and
that just becomes harder to read

- you may want to parameterize some things (e.g. CODE vs. DATA or
KERNEL vs. USER) since both values are used and you don't really
want prefer either one by default -- or DPL, which is always some
value that is always specified

This patch tries to balance these requirements and has two layers of
definitions -- low-level and high-level:

- the low-level defines are the mapping between human-readable names
and the actual bit numbers

- the high-level defines are the mapping from high-level intent to
combinations of low-level flags, representing roughly a tuple
(data/code/tss, 64/32/16-bits) plus an override for DPL-3 (= USER),
since that's relatively rare but still very important to mark
properly for those segments.

- we have *_BIOS variants for 32-bit code and data segments that don't
have the G flag set and give the limit in terms of bytes instead of
pages

[ mingo: Improved readability bit more. ]

Signed-off-by: Vegard Nossum <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Acked-by: Linus Torvalds <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/include/asm/desc_defs.h | 76 +++++++++++++++++++++++++------
1 file changed, 63 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/desc_defs.h b/arch/x86/include/asm/desc_defs.h
index f7e7099..7c08cbf 100644
--- a/arch/x86/include/asm/desc_defs.h
+++ b/arch/x86/include/asm/desc_defs.h
@@ -8,6 +8,56 @@
* archs.
*/

+/*
+ * Low-level interface mapping flags/field names to bits
+ */
+
+/* Flags for _DESC_S (non-system) descriptors */
+#define _DESC_ACCESSED 0x0001
+#define _DESC_DATA_WRITABLE 0x0002
+#define _DESC_CODE_READABLE 0x0002
+#define _DESC_DATA_EXPAND_DOWN 0x0004
+#define _DESC_CODE_CONFORMING 0x0004
+#define _DESC_CODE_EXECUTABLE 0x0008
+
+/* Common flags */
+#define _DESC_S 0x0010
+#define _DESC_DPL(dpl) ((dpl) << 5)
+#define _DESC_PRESENT 0x0080
+
+#define _DESC_LONG_CODE 0x2000
+#define _DESC_DB 0x4000
+#define _DESC_GRANULARITY_4K 0x8000
+
+/* System descriptors have a numeric "type" field instead of flags */
+#define _DESC_SYSTEM(code) (code)
+
+/*
+ * High-level interface mapping intended usage to low-level combinations
+ * of flags
+ */
+
+#define _DESC_DATA (_DESC_S | _DESC_PRESENT | \
+ _DESC_DATA_WRITABLE)
+#define _DESC_CODE (_DESC_S | _DESC_PRESENT | \
+ _DESC_CODE_READABLE | _DESC_CODE_EXECUTABLE)
+
+#define DESC_DATA16 (_DESC_DATA)
+#define DESC_CODE16 (_DESC_CODE)
+
+#define DESC_DATA32 (_DESC_DATA | _DESC_GRANULARITY_4K | _DESC_DB)
+#define DESC_DATA32_BIOS (_DESC_DATA | _DESC_DB)
+
+#define DESC_CODE32 (_DESC_CODE | _DESC_GRANULARITY_4K | _DESC_DB)
+#define DESC_CODE32_BIOS (_DESC_CODE | _DESC_DB)
+
+#define DESC_TSS32 (_DESC_SYSTEM(9) | _DESC_PRESENT)
+
+#define DESC_DATA64 (_DESC_DATA | _DESC_GRANULARITY_4K | _DESC_DB)
+#define DESC_CODE64 (_DESC_CODE | _DESC_GRANULARITY_4K | _DESC_LONG_CODE)
+
+#define DESC_USER (_DESC_DPL(3))
+
#ifndef __ASSEMBLY__

#include <linux/types.h>
@@ -22,19 +72,19 @@ struct desc_struct {

#define GDT_ENTRY_INIT(flags, base, limit) \
{ \
- .limit0 = (u16) (limit), \
- .limit1 = ((limit) >> 16) & 0x0F, \
- .base0 = (u16) (base), \
- .base1 = ((base) >> 16) & 0xFF, \
- .base2 = ((base) >> 24) & 0xFF, \
- .type = (flags & 0x0f), \
- .s = (flags >> 4) & 0x01, \
- .dpl = (flags >> 5) & 0x03, \
- .p = (flags >> 7) & 0x01, \
- .avl = (flags >> 12) & 0x01, \
- .l = (flags >> 13) & 0x01, \
- .d = (flags >> 14) & 0x01, \
- .g = (flags >> 15) & 0x01, \
+ .limit0 = ((limit) >> 0) & 0xFFFF, \
+ .limit1 = ((limit) >> 16) & 0x000F, \
+ .base0 = ((base) >> 0) & 0xFFFF, \
+ .base1 = ((base) >> 16) & 0x00FF, \
+ .base2 = ((base) >> 24) & 0x00FF, \
+ .type = ((flags) >> 0) & 0x000F, \
+ .s = ((flags) >> 4) & 0x0001, \
+ .dpl = ((flags) >> 5) & 0x0003, \
+ .p = ((flags) >> 7) & 0x0001, \
+ .avl = ((flags) >> 12) & 0x0001, \
+ .l = ((flags) >> 13) & 0x0001, \
+ .d = ((flags) >> 14) & 0x0001, \
+ .g = ((flags) >> 15) & 0x0001, \
}

enum {