[PATCH 0/2] x86: Remove direct access to internal x86 descriptor fields

[PATCH 0/2] x86: Remove direct access to internal x86 descriptor fields

[Joe Damato]

Hi -

This patch set aims to remove the direct access to the 'a' and 'b' fields of 32-bit x86 descriptors. Static inline getters/setters are provided.

This is a resumbmit of a patch set I submitted several months ago[1]. My last patch set was a bit large, so I've tried to take a smaller bite this time and slowly build the cleanup effort over a series of patch sets.

Any and all comments, suggestions, and questions are welcome.

Thanks,
Joe Damato

[1] http://lkml.org/lkml/2008/10/24/459

---

Joe Damato (2):
  x86: Add getter/setter static inlines for x86 descriptors
  x86: Replace direct access to descriptor fields with getter/setters

 arch/x86/include/asm/desc.h           |   32 ++++++++++++++++++++++++++------
 arch/x86/include/asm/xen/hypercall.h  |    5 +++--
 arch/x86/kernel/tls.c                 |    6 ++++--
 arch/x86/kernel/vmi_32.c              |    3 ++-
 arch/x86/lguest/boot.c                |    2 +-
 arch/x86/math-emu/fpu_system.h        |   20 ++++++++++----------
 drivers/lguest/interrupts_and_traps.c |   25 +++++++++++++------------
 drivers/lguest/segments.c             |   20 ++++++++++++--------
 drivers/pnp/pnpbios/bioscalls.c       |    4 ++--
 9 files changed, 73 insertions(+), 44 deletions(-)

[PATCH 1/2] x86: Add getter/setter static inlines for x86 descriptors

[Joe Damato]

Static inline getters/setters have been provided to encourage consumers not to touch the internals of 32bit x86 descriptors directly.

Signed-off-by: Joe Damato <ice799@gmail.com>
---
 arch/x86/include/asm/desc.h |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index dc27705..c62cf93 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -44,6 +44,26 @@ static inline struct desc_struct *get_cpu_gdt_table(unsigned int cpu)
 	return per_cpu(gdt_page, cpu).gdt;
 }
 
+static inline void desc_set_lo(struct desc_struct *d, unsigned int lo)
+{
+	d->a = lo;
+}
+
+static inline void desc_set_hi(struct desc_struct *d, unsigned int hi)
+{
+	d->b = hi;
+}
+
+static inline unsigned int desc_get_lo(const struct desc_struct *d)
+{
+	return d->a;
+}
+
+static inline unsigned int desc_get_hi(const struct desc_struct *d)
+{
+	return d->b;
+}
+
 #ifdef CONFIG_X86_64
 
 static inline void pack_gate(gate_desc *gate, unsigned type, unsigned long func,
-- 
1.6.2

[PATCH 2/2] x86: Replace direct access to descriptor fields with getter/setters

[Joe Damato]

Use the getters/setters for 32bit x86 descriptors instead of touching the internal fields directly.

Signed-off-by: Joe Damato <ice799@gmail.com>
---
 arch/x86/include/asm/desc.h           |   12 ++++++------
 arch/x86/include/asm/xen/hypercall.h  |    5 +++--
 arch/x86/kernel/tls.c                 |    6 ++++--
 arch/x86/kernel/vmi_32.c              |    3 ++-
 arch/x86/lguest/boot.c                |    2 +-
 arch/x86/math-emu/fpu_system.h        |   20 ++++++++++----------
 drivers/lguest/interrupts_and_traps.c |   25 +++++++++++++------------
 drivers/lguest/segments.c             |   20 ++++++++++++--------
 drivers/pnp/pnpbios/bioscalls.c       |    4 ++--
 9 files changed, 53 insertions(+), 44 deletions(-)

diff --git a/arch/x86/include/asm/desc.h b/arch/x86/include/asm/desc.h
index c62cf93..9d3bb3f 100644
--- a/arch/x86/include/asm/desc.h
+++ b/arch/x86/include/asm/desc.h
@@ -86,9 +86,9 @@ static inline void pack_gate(gate_desc *gate, unsigned char type,
 			     unsigned long base, unsigned dpl, unsigned flags,
 			     unsigned short seg)
 {
-	gate->a = (seg << 16) | (base & 0xffff);
-	gate->b = (base & 0xffff0000) |
-		  (((0x80 | type | (dpl << 5)) & 0xff) << 8);
+	desc_set_lo(gate, (seg << 16) | (base & 0xffff));
+	desc_set_hi(gate, (base & 0xffff0000) |
+		  (((0x80 | type | (dpl << 5)) & 0xff) << 8));
 }
 
 #endif
@@ -166,10 +166,10 @@ static inline void pack_descriptor(struct desc_struct *desc, unsigned long base,
 				   unsigned long limit, unsigned char type,
 				   unsigned char flags)
 {
-	desc->a = ((base & 0xffff) << 16) | (limit & 0xffff);
-	desc->b = (base & 0xff000000) | ((base & 0xff0000) >> 16) |
+	desc_set_lo(desc, ((base & 0xffff) << 16) | (limit & 0xffff));
+	desc_set_hi(desc, (base & 0xff000000) | ((base & 0xff0000) >> 16) |
 		(limit & 0x000f0000) | ((type & 0xff) << 8) |
-		((flags & 0xf) << 20);
+		((flags & 0xf) << 20));
 	desc->p = 1;
 }
 
diff --git a/arch/x86/include/asm/xen/hypercall.h b/arch/x86/include/asm/xen/hypercall.h
index 5e79ca6..4401632 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b/arch/x86/include/asm/xen/hypercall.h
@@ -39,6 +39,7 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
+#include <asm/desc.h>
 #include <asm/page.h>
 #include <asm/pgtable.h>
 
@@ -478,8 +479,8 @@ MULTI_update_descriptor(struct multicall_entry *mcl, u64 maddr,
 	} else {
 		mcl->args[0] = maddr;
 		mcl->args[1] = maddr >> 32;
-		mcl->args[2] = desc.a;
-		mcl->args[3] = desc.b;
+		mcl->args[2] = desc_get_lo(&desc);
+		mcl->args[3] = desc_get_hi(&desc);
 	}
 }
 
diff --git a/arch/x86/kernel/tls.c b/arch/x86/kernel/tls.c
index 6bb7b85..4f2ca6d 100644
--- a/arch/x86/kernel/tls.c
+++ b/arch/x86/kernel/tls.c
@@ -41,8 +41,10 @@ static void set_tls_desc(struct task_struct *p, int idx,
 	cpu = get_cpu();
 
 	while (n-- > 0) {
-		if (LDT_empty(info))
-			desc->a = desc->b = 0;
+		if (LDT_empty(info)) {
+			desc_set_lo(desc, 0);
+			desc_set_hi(desc, 0);
+		}
 		else
 			fill_ldt(desc, info);
 		++info;
diff --git a/arch/x86/kernel/vmi_32.c b/arch/x86/kernel/vmi_32.c
index bef58b4..9ef53f0 100644
--- a/arch/x86/kernel/vmi_32.c
+++ b/arch/x86/kernel/vmi_32.c
@@ -189,7 +189,8 @@ static void vmi_cpuid(unsigned int *ax, unsigned int *bx,
 
 static inline void vmi_maybe_load_tls(struct desc_struct *gdt, int nr, struct desc_struct *new)
 {
-	if (gdt[nr].a != new->a || gdt[nr].b != new->b)
+	if (desc_get_lo(gdt + nr) != desc_get_lo(new) ||
+	    desc_get_hi(gdt + nr) != desc_get_hi(new))
 		write_gdt_entry(gdt, nr, new, 0);
 }
 
diff --git a/arch/x86/lguest/boot.c b/arch/x86/lguest/boot.c
index 960a8d9..b5787d5 100644
--- a/arch/x86/lguest/boot.c
+++ b/arch/x86/lguest/boot.c
@@ -236,7 +236,7 @@ static void lguest_load_idt(const struct desc_ptr *desc)
 	struct desc_struct *idt = (void *)desc->address;
 
 	for (i = 0; i < (desc->size+1)/8; i++)
-		hcall(LHCALL_LOAD_IDT_ENTRY, i, idt[i].a, idt[i].b);
+		hcall(LHCALL_LOAD_IDT_ENTRY, i, desc_get_lo(idt + i), desc_get_hi(idt + i));
 }
 
 /*
diff --git a/arch/x86/math-emu/fpu_system.h b/arch/x86/math-emu/fpu_system.h
index 50fa0ec..0fc7512 100644
--- a/arch/x86/math-emu/fpu_system.h
+++ b/arch/x86/math-emu/fpu_system.h
@@ -19,16 +19,16 @@
 /* s is always from a cpu register, and the cpu does bounds checking
  * during register load --> no further bounds checks needed */
 #define LDT_DESCRIPTOR(s)	(((struct desc_struct *)current->mm->context.ldt)[(s) >> 3])
-#define SEG_D_SIZE(x)		((x).b & (3 << 21))
-#define SEG_G_BIT(x)		((x).b & (1 << 23))
-#define SEG_GRANULARITY(x)	(((x).b & (1 << 23)) ? 4096 : 1)
-#define SEG_286_MODE(x)		((x).b & ( 0xff000000 | 0xf0000 | (1 << 23)))
-#define SEG_BASE_ADDR(s)	(((s).b & 0xff000000) \
-				 | (((s).b & 0xff) << 16) | ((s).a >> 16))
-#define SEG_LIMIT(s)		(((s).b & 0xff0000) | ((s).a & 0xffff))
-#define SEG_EXECUTE_ONLY(s)	(((s).b & ((1 << 11) | (1 << 9))) = (1 << 11))
-#define SEG_WRITE_PERM(s)	(((s).b & ((1 << 11) | (1 << 9))) = (1 << 9))
-#define SEG_EXPAND_DOWN(s)	(((s).b & ((1 << 11) | (1 << 10))) \
+#define SEG_D_SIZE(x)		(desc_get_hi(&x) & (3 << 21))
+#define SEG_G_BIT(x)		(desc_get_hi(&x) & (1 << 23))
+#define SEG_GRANULARITY(x)	((desc_get_hi(&x) & (1 << 23)) ? 4096 : 1)
+#define SEG_286_MODE(x)		(desc_get_hi(&x) & (0xff000000 | 0xf0000 | (1 << 23)))
+#define SEG_BASE_ADDR(s)	((desc_get_hi(&s) & 0xff000000) \
+				 | ((desc_get_hi(&s) & 0xff) << 16) | (desc_get_lo(&s) >> 16))
+#define SEG_LIMIT(s)		((desc_get_hi(&s) & 0xff0000) | (desc_get_lo(&s) & 0xffff))
+#define SEG_EXECUTE_ONLY(s)	((desc_get_hi(&s) & ((1 << 11) | (1 << 9))) = (1 << 11))
+#define SEG_WRITE_PERM(s)	((desc_get_hi(&s) & ((1 << 11) | (1 << 9))) = (1 << 9))
+#define SEG_EXPAND_DOWN(s)	((desc_get_hi(&s) & ((1 << 11) | (1 << 10))) \
 				 = (1 << 10))
 
 #define I387			(current->thread.xstate)
diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c
index 415fab0..7394b23 100644
--- a/drivers/lguest/interrupts_and_traps.c
+++ b/drivers/lguest/interrupts_and_traps.c
@@ -178,13 +178,13 @@ void maybe_do_interrupt(struct lg_cpu *cpu)
 	 * over them. */
 	idt = &cpu->arch.idt[FIRST_EXTERNAL_VECTOR+irq];
 	/* If they don't have a handler (yet?), we just ignore it */
-	if (idt_present(idt->a, idt->b)) {
+	if (idt_present(desc_get_lo(idt), desc_get_hi(idt))) {
 		/* OK, mark it no longer pending and deliver it. */
 		clear_bit(irq, cpu->irqs_pending);
 		/* set_guest_interrupt() takes the interrupt descriptor and a
 		 * flag to say whether this interrupt pushes an error code onto
 		 * the stack as well: virtual interrupts never do. */
-		set_guest_interrupt(cpu, idt->a, idt->b, 0);
+		set_guest_interrupt(cpu, desc_get_lo(idt), desc_get_hi(idt), 0);
 	}
 
 	/* Every time we deliver an interrupt, we update the timestamp in the
@@ -259,10 +259,10 @@ int deliver_trap(struct lg_cpu *cpu, unsigned int num)
 
 	/* Early on the Guest hasn't set the IDT entries (or maybe it put a
 	 * bogus one in): if we fail here, the Guest will be killed. */
-	if (!idt_present(cpu->arch.idt[num].a, cpu->arch.idt[num].b))
+	if (!idt_present(desc_get_lo(cpu->arch.idt + num), desc_get_hi(cpu->arch.idt + num)))
 		return 0;
-	set_guest_interrupt(cpu, cpu->arch.idt[num].a,
-			    cpu->arch.idt[num].b, has_err(num));
+	set_guest_interrupt(cpu, desc_get_lo(cpu->arch.idt + num),
+			    desc_get_hi(cpu->arch.idt + num), has_err(num));
 	return 1;
 }
 
@@ -365,7 +365,8 @@ static void set_trap(struct lg_cpu *cpu, struct desc_struct *trap,
 
 	/* We zero-out a not-present entry */
 	if (!idt_present(lo, hi)) {
-		trap->a = trap->b = 0;
+		desc_set_lo(trap, 0);
+		desc_set_hi(trap, 0);
 		return;
 	}
 
@@ -377,8 +378,8 @@ static void set_trap(struct lg_cpu *cpu, struct desc_struct *trap,
 	 * type.  The privilege level controls where the trap can be triggered
 	 * manually with an "int" instruction.  This is usually GUEST_PL,
 	 * except for system calls which userspace can use. */
-	trap->a = ((__KERNEL_CS|GUEST_PL)<<16) | (lo&0x0000FFFF);
-	trap->b = (hi&0xFFFFEF00);
+	desc_set_lo(trap, ((__KERNEL_CS|GUEST_PL)<<16) | (lo&0x0000FFFF));
+	desc_set_hi(trap, (hi&0xFFFFEF00));
 }
 
 /*H:230 While we're here, dealing with delivering traps and interrupts to the
@@ -424,11 +425,11 @@ static void default_idt_entry(struct desc_struct *idt,
 	else if (base)
 		/* Copy priv. level from what Guest asked for.  This allows
 		 * debug (int 3) traps from Guest userspace, for example. */
-		flags |= (base->b & 0x6000);
+		flags |= (desc_get_hi(base) & 0x6000);
 
 	/* Now pack it into the IDT entry in its weird format. */
-	idt->a = (LGUEST_CS<<16) | (handler&0x0000FFFF);
-	idt->b = (handler&0xFFFF0000) | flags;
+	desc_set_lo(idt, (LGUEST_CS<<16) | (handler&0x0000FFFF));
+	desc_set_hi(idt, (handler&0xFFFF0000) | flags);
 }
 
 /* When the Guest first starts, we put default entries into the IDT. */
@@ -466,7 +467,7 @@ void copy_traps(const struct lg_cpu *cpu, struct desc_struct *idt,
 		 * If it can't go direct, we still need to copy the priv. level:
 		 * they might want to give userspace access to a software
 		 * interrupt. */
-		if (idt_type(gidt->a, gidt->b) = 0xF)
+		if (idt_type(desc_get_lo(gidt), desc_get_hi(gidt)) = 0xF)
 			idt[i] = *gidt;
 		else
 			default_idt_entry(&idt[i], i, def[i], gidt);
diff --git a/drivers/lguest/segments.c b/drivers/lguest/segments.c
index ec6aa3f..9646195 100644
--- a/drivers/lguest/segments.c
+++ b/drivers/lguest/segments.c
@@ -61,6 +61,7 @@ static int ignored_gdt(unsigned int num)
 static void fixup_gdt_table(struct lg_cpu *cpu, unsigned start, unsigned end)
 {
 	unsigned int i;
+	struct desc_struct *tmp = NULL;
 
 	for (i = start; i < end; i++) {
 		/* We never copy these ones to real GDT, so we don't care what
@@ -71,14 +72,15 @@ static void fixup_gdt_table(struct lg_cpu *cpu, unsigned start, unsigned end)
 		/* Segment descriptors contain a privilege level: the Guest is
 		 * sometimes careless and leaves this as 0, even though it's
 		 * running at privilege level 1.  If so, we fix it here. */
-		if ((cpu->arch.gdt[i].b & 0x00006000) = 0)
-			cpu->arch.gdt[i].b |= (GUEST_PL << 13);
+		tmp = &cpu->arch.gdt[i];
+		if ((desc_get_hi(tmp) & 0x00006000) = 0)
+			desc_set_hi(tmp, desc_get_hi(tmp) | (GUEST_PL << 13));
 
 		/* Each descriptor has an "accessed" bit.  If we don't set it
 		 * now, the CPU will try to set it when the Guest first loads
 		 * that entry into a segment register.  But the GDT isn't
 		 * writable by the Guest, so bad things can happen. */
-		cpu->arch.gdt[i].b |= 0x00000100;
+		desc_set_hi(tmp,  desc_get_hi(tmp) | 0x00000100);
 	}
 }
 
@@ -102,9 +104,9 @@ void setup_default_gdt_entries(struct lguest_ro_state *state)
 	 * Forgive the magic flags: the 0x8900 means the entry is Present, it's
 	 * privilege level 0 Available 386 TSS system segment, and the 0x67
 	 * means Saturn is eclipsed by Mercury in the twelfth house. */
-	gdt[GDT_ENTRY_TSS].a = 0x00000067 | (tss << 16);
-	gdt[GDT_ENTRY_TSS].b = 0x00008900 | (tss & 0xFF000000)
-		| ((tss >> 16) & 0x000000FF);
+	desc_set_lo(gdt + GDT_ENTRY_TSS, 0x00000067 | (tss << 16));
+	desc_set_hi(gdt + GDT_ENTRY_TSS, 0x00008900 | (tss & 0xFF000000)
+		| ((tss >> 16) & 0x000000FF));
 }
 
 /* This routine sets up the initial Guest GDT for booting.  All entries start
@@ -116,8 +118,10 @@ void setup_guest_gdt(struct lg_cpu *cpu)
 	cpu->arch.gdt[GDT_ENTRY_KERNEL_DS] = FULL_SEGMENT;
 	/* ...except the Guest is allowed to use them, so set the privilege
 	 * level appropriately in the flags. */
-	cpu->arch.gdt[GDT_ENTRY_KERNEL_CS].b |= (GUEST_PL << 13);
-	cpu->arch.gdt[GDT_ENTRY_KERNEL_DS].b |= (GUEST_PL << 13);
+	desc_set_hi(cpu->arch.gdt + GDT_ENTRY_KERNEL_CS,
+		desc_get_hi(cpu->arch.gdt + GDT_ENTRY_KERNEL_CS) | (GUEST_PL << 13));
+	desc_set_hi(cpu->arch.gdt + GDT_ENTRY_KERNEL_DS,
+		desc_get_hi(cpu->arch.gdt + GDT_ENTRY_KERNEL_DS) | (GUEST_PL << 13));
 }
 
 /*H:650 An optimization of copy_gdt(), for just the three "thead-local storage"
diff --git a/drivers/pnp/pnpbios/bioscalls.c b/drivers/pnp/pnpbios/bioscalls.c
index 7e6b5a3..d6a5368 100644
--- a/drivers/pnp/pnpbios/bioscalls.c
+++ b/drivers/pnp/pnpbios/bioscalls.c
@@ -476,8 +476,8 @@ void pnpbios_calls_init(union pnp_bios_install_struct *header)
 	pnp_bios_callpoint.offset = header->fields.pm16offset;
 	pnp_bios_callpoint.segment = PNP_CS16;
 
-	bad_bios_desc.a = 0;
-	bad_bios_desc.b = 0x00409200;
+	desc_set_lo(&bad_bios_desc, 0);
+	desc_set_hi(&bad_bios_desc, 0x00409200);
 
 	set_base(bad_bios_desc, __va((unsigned long)0x40 << 4));
 	_set_limit((char *)&bad_bios_desc, 4095 - (0x40 << 4));
-- 
1.6.2

Re: [PATCH 1/2] x86: Add getter/setter static inlines for x86 descriptors

[H. Peter Anvin]

Joe Damato wrote:
> Static inline getters/setters have been provided to encourage consumers not to touch the internals of 32bit x86 descriptors directly.
> 
> Signed-off-by: Joe Damato <ice799@gmail.com>

Okay, what is the motivation for this?

This is a serious question.  The x86 descriptors are so complex that
it's not clear to me that this restriction makes the code any more
clear.  Especially not with things like:

-		cpu->arch.gdt[i].b |= 0x00000100;
+		desc_set_hi(tmp,  desc_get_hi(tmp) | 0x00000100);

This isn't an improvement.  If you're doing to so something like this,
you need to actually implement the *intent* here.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Re: [PATCH 1/2] x86: Add getter/setter static inlines for x86

[Joe Damato]

On Sun, Mar 29, 2009 at 2:28 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> Joe Damato wrote:
>> Static inline getters/setters have been provided to encourage consumers not to touch the internals of 32bit x86 descriptors directly.
>>
>> Signed-off-by: Joe Damato <ice799@gmail.com>
>
> Okay, what is the motivation for this?
>
> This is a serious question.  The x86 descriptors are so complex that
> it's not clear to me that this restriction makes the code any more
> clear.  Especially not with things like:
>
> -               cpu->arch.gdt[i].b |= 0x00000100;
> +               desc_set_hi(tmp,  desc_get_hi(tmp) | 0x00000100);
>
> This isn't an improvement.  If you're doing to so something like this,
> you need to actually implement the *intent* here.

Hi -

In my first patch set several months ago I replaced the structure
desc_struct with bit fields that exposed the fields for IDT/LDT/TSS
entries. This patch set was rejected for several reasons, one of which
was that Linux is trying to move away from bit fields. I also received
other comments such as "take a smaller bite", etc.

So instead of the bit fields, I implemented static inline
getters/setters. I thought that this small change would be a good
first step to re-test the waters of submitting patches to the kernel
before I started to clean out more pieces of the x86 architecture
specific code.

I agree that this isn't very clear, but from comments I received on my
first set, I assumed that this was closer to what people wanted to see
in the kernel. I am happy to iterate and submit something better, but
I am clearly misunderstanding what people would like to see.

Any suggestions on how this first-timer can help are greatly appreciated.

Joe
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] x86: Add getter/setter static inlines for x86 descriptors

[H. Peter Anvin]

Joe Damato wrote:
>>
>> This is a serious question.  The x86 descriptors are so complex that
>> it's not clear to me that this restriction makes the code any more
>> clear.  Especially not with things like:
>>
>> -               cpu->arch.gdt[i].b |= 0x00000100;
>> +               desc_set_hi(tmp,  desc_get_hi(tmp) | 0x00000100);
>>
>> This isn't an improvement.  If you're doing to so something like this,
>> you need to actually implement the *intent* here.
> 
> Hi -
> 
> In my first patch set several months ago I replaced the structure
> desc_struct with bit fields that exposed the fields for IDT/LDT/TSS
> entries. This patch set was rejected for several reasons, one of which
> was that Linux is trying to move away from bit fields. I also received
> other comments such as "take a smaller bite", etc.
> 
> So instead of the bit fields, I implemented static inline
> getters/setters. I thought that this small change would be a good
> first step to re-test the waters of submitting patches to the kernel
> before I started to clean out more pieces of the x86 architecture
> specific code.
> 
> I agree that this isn't very clear, but from comments I received on my
> first set, I assumed that this was closer to what people wanted to see
> in the kernel. I am happy to iterate and submit something better, but
> I am clearly misunderstanding what people would like to see.
> 
> Any suggestions on how this first-timer can help are greatly appreciated.
> 

If you're going to implement get/set then you need getters and setters
that make sense.  Now, doing something like that might be acceptable as
part of a larger patchset (which is one way to "take smaller bites"),
but the above is just gratuitous obfuscation, since it leaves in place
the ugliest part of it all, which is the magic constant 0x00000100.

Personally, I would be just as happy treating the descriptor as an u64
and explicitly do shifts and masks with well-defined constants.  It's
the magic constants that suck.

	-hpa

-- 
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.