Re: [Xen-devel] [PATCH 2/4] SVM: infer type in VMCB_ACCESSORS()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 31/05/17 08:21, Jan Beulich wrote:
> Prevent accidental mistakes by not requiring explicit types to be
> specified in the macro invocations.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

I am not a fan of these accessors being macro-generated; I've lost count
of the number of times I've tried greping for one of them, just to
finally remember that they can't be searched for. 

OTOH, this change doesn't make that problem worse, and does fix one
issue in the current setup.  One comment however...

>  /* Updates are all via hvm_set_segment_register(). */
> -/* VMCB_ACCESSORS(svm_segment_register_t, gdtr, dt) */
> -/* VMCB_ACCESSORS(svm_segment_register_t, idtr, dt) */
> -/* VMCB_ACCESSORS(svm_segment_register_t, cs, seg) */
> -/* VMCB_ACCESSORS(svm_segment_register_t, ds, seg) */
> -/* VMCB_ACCESSORS(svm_segment_register_t, es, seg) */
> -/* VMCB_ACCESSORS(svm_segment_register_t, ss, seg) */
> -VMCB_ACCESSORS(u8, cpl, seg)
> -VMCB_ACCESSORS(u64, cr2, cr2)
> -VMCB_ACCESSORS(u64, debugctlmsr, lbr)
> -VMCB_ACCESSORS(u64, lastbranchfromip, lbr)
> -VMCB_ACCESSORS(u64, lastbranchtoip, lbr)
> -VMCB_ACCESSORS(u64, lastintfromip, lbr)
> -VMCB_ACCESSORS(u64, lastinttoip, lbr)
> +/* VMCB_ACCESSORS(gdtr, dt) */
> +/* VMCB_ACCESSORS(idtr, dt) */
> +/* VMCB_ACCESSORS(cs, seg) */
> +/* VMCB_ACCESSORS(ds, seg) */
> +/* VMCB_ACCESSORS(es, seg) */
> +/* VMCB_ACCESSORS(ss, seg) */

I'd just drop these entirely.  I can't see any need for them to be
introduced, but even if a need does arise, its not like they are hard to
introduce from first principles.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel