>>> On 28.06.18 at 11:25, <[email protected]> wrote:
> +static int hvm_save_cpu_ctxt_one(struct vcpu *v, hvm_domain_context_t *h)
> +{
> + struct segment_register seg;
> + struct hvm_hw_cpu ctxt;
> +
> + memset(&ctxt, 0, sizeof(ctxt));
> +
> + /*
> + * We don't need to save state for a vcpu that is down; the restore
> + * code will leave it down if there is nothing saved.
> + */
> + if ( v->pause_flags & VPF_down )
> + return CONTINUE;
Note how the original code had if() and memset() the other way around.
> static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
> {
> struct vcpu *v;
> - struct hvm_hw_cpu ctxt;
> - struct segment_register seg;
> + int rc = 0;
>
> for_each_vcpu ( d, v )
> {
> - /* We don't need to save state for a vcpu that is down; the restore
> - * code will leave it down if there is nothing saved. */
> - if ( v->pause_flags & VPF_down )
> + rc = hvm_save_cpu_ctxt_one(v, h);
> + if (rc == CONTINUE)
Style. I'm pretty sure you were asked before to go through and
check your additions for style.
> --- a/xen/include/asm-x86/hvm/support.h
> +++ b/xen/include/asm-x86/hvm/support.h
> @@ -52,6 +52,8 @@ extern unsigned int opt_hvm_debug_level;
> #define HVM_DBG_LOG(level, _f, _a...) do {} while (0)
> #endif
>
> +#define CONTINUE 2
This is way too generic an identifier name. And it's not helpful at
all without other possible values also enumerated. And please
take "enumerated" as a hint ... Otoh, looking at its use - this is
an agreement between hvm_save_cpu_ctxt() and
hvm_save_cpu_ctxt_one() only. Why does such need a globally
visible #define?
Jan
_______________________________________________
Xen-devel mailing list
[email protected]
https://lists.xenproject.org/mailman/listinfo/xen-devel
