[PATCH V3 0/1] target/loongarch: Fixed tlb huge page loading issue

[PATCH V3 0/1] target/loongarch: Fixed tlb huge page loading issue

[Xianglai Li]

When we use qemu tcg simulation, the page size of bios is 4KB.
When using the level 2 super large page (page size is 1G) to create the page table,
it is found that the content of the corresponding address space is abnormal,
resulting in the bios can not start the operating system and graphical interface normally.

The lddir and ldpte instruction emulation has
a problem with the use of super large page processing above level 2.
The page size is not correctly calculated,
resulting in the wrong page size of the table entry found by tlb.

Changes log:
V2->V3:
Delete the intermediate variable LDDIR_PS, and implement lddir and ldpte
huge pages by referring to the latest architecture reference manual.

V1->V2:
Modified the patch title format and Enrich the commit mesg description

Cc: maobibo@loongson.cn
Cc: Song Gao <gaosong@loongson.cn>
Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: zhaotianrui@loongson.cn

Xianglai Li (1):
  target/loongarch: Fixed tlb huge page loading issue

 target/loongarch/internals.h      |  8 +++
 target/loongarch/tcg/tlb_helper.c | 92 +++++++++++++++++++++++--------
 2 files changed, 76 insertions(+), 24 deletions(-)

-- 
2.39.1

[PATCH V3 1/1] target/loongarch: Fixed tlb huge page loading issue

[Xianglai Li]

When we use qemu tcg simulation, the page size of bios is 4KB.
When using the level 2 super large page (page size is 1G) to create the page table,
it is found that the content of the corresponding address space is abnormal,
resulting in the bios can not start the operating system and graphical interface normally.

The lddir and ldpte instruction emulation has
a problem with the use of super large page processing above level 2.
The page size is not correctly calculated,
resulting in the wrong page size of the table entry found by tlb.

Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
Cc: maobibo@loongson.cn
Cc: Song Gao <gaosong@loongson.cn>
Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
Cc: zhaotianrui@loongson.cn
---
 target/loongarch/internals.h      |  8 +++
 target/loongarch/tcg/tlb_helper.c | 92 +++++++++++++++++++++++--------
 2 files changed, 76 insertions(+), 24 deletions(-)

diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
index a2fc54c8a7..55ceb4c079 100644
--- a/target/loongarch/internals.h
+++ b/target/loongarch/internals.h
@@ -16,6 +16,14 @@
 #define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
 #define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
 
+/*
+ * The [13..14]bits of the entry base address of the lddir/ldpte
+ * directive are used to represent the level of the large page
+ * when processing the huge page entry
+ */
+#define HUGE_PAGE_LEVEL_SHIFT   13
+#define HUGE_PAGE_LEVEL_MASK MAKE_64BIT_MASK(HUGE_PAGE_LEVEL_SHIFT, 2)
+
 /* Global bit used for lddir/ldpte */
 #define LOONGARCH_PAGE_HUGE_SHIFT   6
 /* Global bit for huge page */
diff --git a/target/loongarch/tcg/tlb_helper.c b/target/loongarch/tcg/tlb_helper.c
index a08c08b05a..2db77b48c5 100644
--- a/target/loongarch/tcg/tlb_helper.c
+++ b/target/loongarch/tcg/tlb_helper.c
@@ -17,6 +17,9 @@
 #include "exec/log.h"
 #include "cpu-csr.h"
 
+static int get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
+                              uint64_t *dir_width, target_ulong level);
+
 static void raise_mmu_exception(CPULoongArchState *env, target_ulong address,
                                 MMUAccessType access_type, int tlb_error)
 {
@@ -487,6 +490,16 @@ target_ulong helper_lddir(CPULoongArchState *env, target_ulong base,
     int shift;
     uint64_t dir_base, dir_width;
     bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
+    uint64_t huge_page_level = base & HUGE_PAGE_LEVEL_MASK;
+
+    if (huge) {
+        if (huge_page_level) {
+            return base;
+        } else {
+            huge_page_level = (level & 0x3) << HUGE_PAGE_LEVEL_SHIFT;
+            return base | huge_page_level;
+        }
+    }
 
     badvaddr = env->CSR_TLBRBADV;
     base = base & TARGET_PHYS_MASK;
@@ -495,30 +508,10 @@ target_ulong helper_lddir(CPULoongArchState *env, target_ulong base,
     shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
     shift = (shift + 1) * 3;
 
-    if (huge) {
-        return base;
-    }
-    switch (level) {
-    case 1:
-        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
-        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
-        break;
-    case 2:
-        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
-        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
-        break;
-    case 3:
-        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
-        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
-        break;
-    case 4:
-        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
-        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
-        break;
-    default:
-        do_raise_exception(env, EXCCODE_INE, GETPC());
+    if (get_dir_base_width(env, &dir_base, &dir_width, level) != 0) {
         return 0;
     }
+
     index = (badvaddr >> dir_base) & ((1 << dir_width) - 1);
     phys = base | index << shift;
     ret = ldq_phys(cs->as, phys) & TARGET_PHYS_MASK;
@@ -534,17 +527,38 @@ void helper_ldpte(CPULoongArchState *env, target_ulong base, target_ulong odd,
     bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
     uint64_t ptbase = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
     uint64_t ptwidth = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
+    uint64_t dir_base, dir_width;
+    uint64_t huge_page_level;
 
     base = base & TARGET_PHYS_MASK;
 
     if (huge) {
-        /* Huge Page. base is paddr */
+        /*
+         * Gets the huge page level
+         * Clears the huge page level information in the address
+         * Clears huge page bit
+         * Gets huge page size
+         */
+        huge_page_level = (base & HUGE_PAGE_LEVEL_MASK) >>
+                          HUGE_PAGE_LEVEL_SHIFT;
+
+        base &= ~HUGE_PAGE_LEVEL_MASK;
+
         tmp0 = base ^ (1 << LOONGARCH_PAGE_HUGE_SHIFT);
         /* Move Global bit */
         tmp0 = ((tmp0 & (1 << LOONGARCH_HGLOBAL_SHIFT))  >>
                 LOONGARCH_HGLOBAL_SHIFT) << R_TLBENTRY_G_SHIFT |
                 (tmp0 & (~(1 << LOONGARCH_HGLOBAL_SHIFT)));
-        ps = ptbase + ptwidth - 1;
+
+        huge_page_level++;
+        get_dir_base_width(env, &dir_base, &dir_width, huge_page_level);
+
+        /*
+         * Huge pages are evenly split into parity pages
+         * when loaded into the tlb,
+         * so the tlb page size needs to be divided by 2.
+         */
+        ps = dir_base - 1;
         if (odd) {
             tmp0 += MAKE_64BIT_MASK(ps, 1);
         }
@@ -571,3 +585,33 @@ void helper_ldpte(CPULoongArchState *env, target_ulong base, target_ulong odd,
     }
     env->CSR_TLBREHI = FIELD_DP64(env->CSR_TLBREHI, CSR_TLBREHI, PS, ps);
 }
+
+static int get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
+                              uint64_t *dir_width, target_ulong level)
+{
+    int ret = 0;
+
+    switch (level) {
+    case 1:
+        *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
+        *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
+        break;
+    case 2:
+        *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
+        *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
+        break;
+    case 3:
+        *dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
+        *dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
+        break;
+    case 4:
+        *dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
+        *dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
+        break;
+    default:
+        do_raise_exception(env, EXCCODE_INE, GETPC());
+        ret = -1;
+    }
+
+    return ret;
+}
-- 
2.39.1

Re: [PATCH V3 1/1] target/loongarch: Fixed tlb huge page loading issue

[Richard Henderson]

On 3/6/24 21:37, Xianglai Li wrote:
> When we use qemu tcg simulation, the page size of bios is 4KB.
> When using the level 2 super large page (page size is 1G) to create the page table,
> it is found that the content of the corresponding address space is abnormal,
> resulting in the bios can not start the operating system and graphical interface normally.
> 
> The lddir and ldpte instruction emulation has
> a problem with the use of super large page processing above level 2.
> The page size is not correctly calculated,
> resulting in the wrong page size of the table entry found by tlb.
> 
> Signed-off-by: Xianglai Li <lixianglai@loongson.cn>
> Cc: maobibo@loongson.cn
> Cc: Song Gao <gaosong@loongson.cn>
> Cc: Xiaojuan Yang <yangxiaojuan@loongson.cn>
> Cc: zhaotianrui@loongson.cn
> ---
>   target/loongarch/internals.h      |  8 +++
>   target/loongarch/tcg/tlb_helper.c | 92 +++++++++++++++++++++++--------
>   2 files changed, 76 insertions(+), 24 deletions(-)
> 
> diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h
> index a2fc54c8a7..55ceb4c079 100644
> --- a/target/loongarch/internals.h
> +++ b/target/loongarch/internals.h
> @@ -16,6 +16,14 @@
>   #define TARGET_PHYS_MASK MAKE_64BIT_MASK(0, TARGET_PHYS_ADDR_SPACE_BITS)
>   #define TARGET_VIRT_MASK MAKE_64BIT_MASK(0, TARGET_VIRT_ADDR_SPACE_BITS)
>   
> +/*
> + * The [13..14]bits of the entry base address of the lddir/ldpte
> + * directive are used to represent the level of the large page
> + * when processing the huge page entry
> + */
> +#define HUGE_PAGE_LEVEL_SHIFT   13
> +#define HUGE_PAGE_LEVEL_MASK MAKE_64BIT_MASK(HUGE_PAGE_LEVEL_SHIFT, 2)

This would be cleaner using <hw/registerfields.h>, e.g.

FIELD(LDDIR, HUGE, 6, 1)
FIELD(LDDIR, LEVEL, 13, 2)

> +static int get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
> +                              uint64_t *dir_width, target_ulong level);

Very often you can place the new function just before its first use so that no prior 
declaration is required.

Returning a bool with true for success and false for failure is preferred over 0/-1.

> @@ -487,6 +490,16 @@ target_ulong helper_lddir(CPULoongArchState *env, target_ulong base,
>       int shift;
>       uint64_t dir_base, dir_width;
>       bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
> +    uint64_t huge_page_level = base & HUGE_PAGE_LEVEL_MASK;
> +
> +    if (huge) {

if (FIELD_EX64(base, LDDIR, HUGE))

> +        if (huge_page_level) {

if (FIELD_EX64(base, LDDIR, LEVEL))

> +        } else {
> +            huge_page_level = (level & 0x3) << HUGE_PAGE_LEVEL_SHIFT;
> +            return base | huge_page_level;

return FIELD_DP64(base, LDDIR, LEVEL, level);

I suppose setting bit [6] with level == 4 is a "don't do that" sort of programming error. 
You could log the error here, perhaps:

     if (unlikely(level == 4)) {
         qemu_log_mask(LOG_GUEST_ERROR, "Attempted use of level 4 huge page\n");
     }


> @@ -495,30 +508,10 @@ target_ulong helper_lddir(CPULoongArchState *env, target_ulong base,
>       shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
>       shift = (shift + 1) * 3;
>   
> -    if (huge) {
> -        return base;
> -    }
> -    switch (level) {
> -    case 1:
> -        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
> -        break;
> -    case 2:
> -        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
> -        break;
> -    case 3:
> -        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
> -        break;
> -    case 4:
> -        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
> -        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
> -        break;
> -    default:
> -        do_raise_exception(env, EXCCODE_INE, GETPC());
> +    if (get_dir_base_width(env, &dir_base, &dir_width, level) != 0) {
>           return 0;
>       }

I believe that we should not raise an exception here at all.  This illegal instruction 
exception is based on the LDDIR immediate operand, so we should have diagnosed this error 
and raised an exception in trans_lddir().

Therefore the default label should use only g_assert_not_reached(), and there need not be 
a error return from get_dir_base_width at all.


> @@ -534,17 +527,38 @@ void helper_ldpte(CPULoongArchState *env, target_ulong base, target_ulong odd,
>       bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
>       uint64_t ptbase = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
>       uint64_t ptwidth = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
> +    uint64_t dir_base, dir_width;
> +    uint64_t huge_page_level;
>   
>       base = base & TARGET_PHYS_MASK;
>   
>       if (huge) {
> -        /* Huge Page. base is paddr */
> +        /*
> +         * Gets the huge page level
> +         * Clears the huge page level information in the address
> +         * Clears huge page bit
> +         * Gets huge page size
> +         */
> +        huge_page_level = (base & HUGE_PAGE_LEVEL_MASK) >>
> +                          HUGE_PAGE_LEVEL_SHIFT;
> +
> +        base &= ~HUGE_PAGE_LEVEL_MASK;
> +
>           tmp0 = base ^ (1 << LOONGARCH_PAGE_HUGE_SHIFT);
>           /* Move Global bit */
>           tmp0 = ((tmp0 & (1 << LOONGARCH_HGLOBAL_SHIFT))  >>
>                   LOONGARCH_HGLOBAL_SHIFT) << R_TLBENTRY_G_SHIFT |
>                   (tmp0 & (~(1 << LOONGARCH_HGLOBAL_SHIFT)));
> -        ps = ptbase + ptwidth - 1;
> +
> +        huge_page_level++;

Why are you incrementing the level?

I think you want

     level = MIN(level, 1);

Google translates the documentation for LDPTE as "bits [14:13] ... should be a non-zero 
value".  I don't know if "should" is precisely correct here (english technical documents 
prefer "shall" or "may" to indicate a hard requirement vs optional behaviour).  The 
document does not appear to say what happens if the value is zero.

Since an earlier version of the specification did not have bits [14:13], my suspicion is 
that the the current version of the specification is intended to be compatible, which 
would map [14:13] == 0 to level == 1.

In any case, incrementing the level such that [14:13] == 1 -> level == 2 definitely seems 
wrong.


r~

Re: [PATCH V3 1/1] target/loongarch: Fixed tlb huge page loading issue

[lixianglai]

Hi Richard:
>
>> @@ -495,30 +508,10 @@ target_ulong helper_lddir(CPULoongArchState 
>> *env, target_ulong base,
>>       shift = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTEWIDTH);
>>       shift = (shift + 1) * 3;
>>   -    if (huge) {
>> -        return base;
>> -    }
>> -    switch (level) {
>> -    case 1:
>> -        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
>> -        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
>> -        break;
>> -    case 2:
>> -        dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
>> -        dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
>> -        break;
>> -    case 3:
>> -        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
>> -        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
>> -        break;
>> -    case 4:
>> -        dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
>> -        dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
>> -        break;
>> -    default:
>> -        do_raise_exception(env, EXCCODE_INE, GETPC());
>> +    if (get_dir_base_width(env, &dir_base, &dir_width, level) != 0) {
>>           return 0;
>>       }
>
> I believe that we should not raise an exception here at all.  This 
> illegal instruction exception is based on the LDDIR immediate operand, 
> so we should have diagnosed this error and raised an exception in 
> trans_lddir().

After consulting the hardware technician, when the level value is 
greater than 4,

the hardware does not report an exception, we can check the level in 
helper_lddir,

if the parameter is not valid, we will directly return to base,

and it is not reasonable to check the validity of the immediate number 
in trans_lddir.

The actual action should be implemented in the instruction 
simulation,and the log should be printed and recorded,

like this:

target_ulong helper_lddir( )

{

     if ((level == 0) || (level > 4)) {

        qemu_log_mask(LOG_GUEST_ERROR, "Illegal instruction level 
%lu\n",  level);

         return base;

     }

......

}

>
> Therefore the default label should use only g_assert_not_reached(), 
> and there need not be a error return from get_dir_base_width at all.
>
>
>> @@ -534,17 +527,38 @@ void helper_ldpte(CPULoongArchState *env, 
>> target_ulong base, target_ulong odd,
>>       bool huge = (base >> LOONGARCH_PAGE_HUGE_SHIFT) & 0x1;
>>       uint64_t ptbase = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
>>       uint64_t ptwidth = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
>> +    uint64_t dir_base, dir_width;
>> +    uint64_t huge_page_level;
>>         base = base & TARGET_PHYS_MASK;
>>         if (huge) {
>> -        /* Huge Page. base is paddr */
>> +        /*
>> +         * Gets the huge page level
>> +         * Clears the huge page level information in the address
>> +         * Clears huge page bit
>> +         * Gets huge page size
>> +         */
>> +        huge_page_level = (base & HUGE_PAGE_LEVEL_MASK) >>
>> +                          HUGE_PAGE_LEVEL_SHIFT;
>> +
>> +        base &= ~HUGE_PAGE_LEVEL_MASK;
>> +
>>           tmp0 = base ^ (1 << LOONGARCH_PAGE_HUGE_SHIFT);
>>           /* Move Global bit */
>>           tmp0 = ((tmp0 & (1 << LOONGARCH_HGLOBAL_SHIFT))  >>
>>                   LOONGARCH_HGLOBAL_SHIFT) << R_TLBENTRY_G_SHIFT |
>>                   (tmp0 & (~(1 << LOONGARCH_HGLOBAL_SHIFT)));
>> -        ps = ptbase + ptwidth - 1;
>> +
>> +        huge_page_level++;
>
> Why are you incrementing the level?

level plus 1 is to obtain the dir_base of the upper level,

because I directly use the dir_base of the upper level as the size of 
the huge page when calculating the page size,

this practice is different from the hardware implementation,

the hardware implementation is explained below,

the next version I will refer to the hardware implementation method to 
calculate the size of the huge page.

>
> I think you want
>
>     level = MIN(level, 1);
>
> Google translates the documentation for LDPTE as "bits [14:13] ... 
> should be a non-zero value".  I don't know if "should" is precisely 
> correct here (english technical documents prefer "shall" or "may" to 
> indicate a hard requirement vs optional behaviour). The document does 
> not appear to say what happens if the value is zero.
>

After consulting hardware technicians, LDPTE uses dir_base + dir_width 
corresponding to [14..13]bits as the page size,

and when [14..13]bits is 0, the page size should be PTbase + PTwidth.

So [14..13]bits can be zero and we should revise the manual.

And The get_dir_base_width function plans to add the handling of case 0,

so that get_dir_base_width will not receive illegal level arguments when 
ldpte,

and because of the validity of the level at the entry of the lddir 
function,

the get_dir_base_width function will not receive illegal level arguments.

So you will not receive level == 0 and level > 4:


static void get_dir_base_width(CPULoongArchState *env, uint64_t *dir_base,
                                uint64_t *dir_width, target_ulong level)
{
     switch (level) {
     case 0:
         *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTBASE);
         *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, PTWIDTH);
         break;
     case 1:
         *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_BASE);
         *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR1_WIDTH);
         break;
     case 2:
         *dir_base = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_BASE);
         *dir_width = FIELD_EX64(env->CSR_PWCL, CSR_PWCL, DIR2_WIDTH);
         break;
     case 3:
         *dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_BASE);
         *dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR3_WIDTH);
         break;
     case 4:
         *dir_base = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_BASE);
         *dir_width = FIELD_EX64(env->CSR_PWCH, CSR_PWCH, DIR4_WIDTH);
         break;
     default:
         g_assert_not_reached();
     }
}


> Since an earlier version of the specification did not have bits 
> [14:13], my suspicion is that the the current version of the 
> specification is intended to be compatible, which would map [14:13] == 
> 0 to level == 1.
>
> In any case, incrementing the level such that [14:13] == 1 -> level == 
> 2 definitely seems wrong.
>
>
> r~