Re: [Xen-devel] [PATCH v4] arch/x86: Add registers to vm_event

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.10.18 at 11:11, <aisaila@xxxxxxxxxxxxxxx> wrote:
> @@ -157,6 +157,21 @@
>  #define VM_EVENT_X86_CR4    2
>  #define VM_EVENT_X86_XCR0   3
>  
> +/*
> + * The limit field is right-shifted by 12 bits if .ar.g is set.
> + */

This is supposed to be a single line comment.

> @@ -191,9 +206,19 @@ struct vm_event_regs_x86 {
>      uint64_t msr_efer;
>      uint64_t msr_star;
>      uint64_t msr_lstar;
> -    uint64_t fs_base;
> -    uint64_t gs_base;
> -    uint32_t cs_arbytes;
> +    struct x86_selector_reg_32 cs;
> +    struct x86_selector_reg_32 ss;
> +    struct x86_selector_reg_32 ds;
> +    struct x86_selector_reg_32 es;
> +    struct x86_selector_reg_64 fs;

Since you say space is scarce on the ring, you may want to consider
filling the (I think) 4-byte gaps here and ...

> +    struct x86_selector_reg_64 gs;

... here, e.g. by putting two selector values there. But of course
this requires either splitting struct x86_selector_reg_{32,64} or
somehow (in a gcc extension free way) reducing the alignment of
the latter. My recommendation would be to pack only limit and
access rights into a structure, and provided bases as standalone
fields just like you already do for selectors. With that you'd then
also not need to put some of the selector values in the middle (to
fill padding holes).

But this is just a suggestion - I'm not the maintainer of any of the
code you change.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel