On Tue, May 05, 2020 at 06:32:50PM +0100, Andrew Cooper wrote:
> Rework the vmcbcleanbits_t definitons to use bool, drop 'fields' from the
> namespace, position the comments in an unambiguous position, and include the
> bit position.
>
> In svm_vmexit_handler(), don't bother conditionally writing ~0 or 0 based on
> hardware support. The field was entirely unused and ignored on older
> hardware (and we're already setting reserved cleanbits anyway).
>
> In nsvm_vmcb_prepare4vmrun(), simplify the logic massivly by dropping the
^e
> vcleanbit_set() macro using a vmcbcleanbits_t local variable which only gets
> filled in the case that clean bits were valid previously. Fix up the style on
> impacted lines.
>
> No practical change in behaviour.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: Roger Pau Monné <roger....@citrix.com>
> diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
> index 5950e4d52b..aeebeaf873 100644
> --- a/xen/arch/x86/hvm/svm/svm.c
> +++ b/xen/arch/x86/hvm/svm/svm.c
> @@ -345,7 +345,7 @@ static int svm_vmcb_restore(struct vcpu *v, struct
> hvm_hw_cpu *c)
> else
> vmcb->event_inj.raw = 0;
>
> - vmcb->cleanbits.bytes = 0;
> + vmcb->cleanbits.raw = 0;
> paging_update_paging_modes(v);
>
> return 0;
> @@ -693,12 +693,12 @@ static void svm_set_segment_register(struct vcpu *v,
> enum x86_segment seg,
> case x86_seg_ds:
> case x86_seg_es:
> case x86_seg_ss: /* cpl */
> - vmcb->cleanbits.fields.seg = 0;
> + vmcb->cleanbits.seg = 0;
> break;
>
> case x86_seg_gdtr:
> case x86_seg_idtr:
> - vmcb->cleanbits.fields.dt = 0;
> + vmcb->cleanbits.dt = 0;
Nit: using false here (and above) would be better, since the fields
are now booleans.
Thanks, Roger.
