>>> On 23.11.16 at 16:38, <andrew.coop...@citrix.com> wrote:
> + case x86_seg_tr:
> + ASSERT(reg->attr.fields.p); /* Usable. */
> + ASSERT(!reg->attr.fields.s); /* System segment. */
> + ASSERT(!(reg->sel & 0x4)); /* !TI. */
> + ASSERT(reg->attr.fields.type == SYS_DESC_tss16_busy ||
> + reg->attr.fields.type == SYS_DESC_tss_busy);
> + ASSERT(is_canonical_address(reg->base));
> + break;
I can't help thinking that the slightly more strict
+ if ( reg->attr.fields.type == SYS_DESC_tss_busy )
+ ASSERT(is_canonical_address(reg->base));
+ else if ( reg->attr.fields.type == SYS_DESC_tss16_busy )
+ ASSERT(!(reg->base >> 32));
+ else
+ ASSERT_UNREACHABLE();
would be better, even if that's going to make TR checking look a
little different than the others (but we should leverage the
information we have).
> + case x86_seg_ldtr:
> + if ( reg->attr.fields.p )
> + {
> + ASSERT(!reg->attr.fields.s); /* System segment. */
> + ASSERT(!(reg->sel & 0x4)); /* !TI. */
> + ASSERT(reg->attr.fields.type == SYS_DESC_ldt);
> + ASSERT(is_canonical_address(reg->base));
> + }
> + break;
> +
> + case x86_seg_gdtr:
> + case x86_seg_idtr:
> + ASSERT(is_canonical_address(reg->base));
> + ASSERT((reg->limit >> 16) == 0); /* Upper bits clear. */
> + break;
> +
> + default:
> + ASSERT_UNREACHABLE();
> + break;
> + }
Didn't you agree to change this last "break" to "return"?
> --- a/xen/include/asm-x86/desc.h
> +++ b/xen/include/asm-x86/desc.h
> @@ -89,7 +89,13 @@
> #ifndef __ASSEMBLY__
>
> /* System Descriptor types for GDT and IDT entries. */
> +#define SYS_DESC_tss16_avail 1
> #define SYS_DESC_ldt 2
> +#define SYS_DESC_tss16_busy 3
> +#define SYS_DESC_call_gate16 4
> +#define SYS_DESC_task_gate 5
> +#define SYS_DESC_irq_gate16 6
> +#define SYS_DESC_trap_gate16 7
> #define SYS_DESC_tss_avail 9
> #define SYS_DESC_tss_busy 11
> #define SYS_DESC_call_gate 12
Thanks for completing the set.
Regardless of how you decide on the two earlier remarks,
Reviewed-by: Jan Beulich <jbeul...@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
