> -----Original Message-----
> From: Jan Beulich [mailto:[email protected]]
> Sent: 30 March 2016 12:23
> To: Paul Durrant
> Cc: Andrew Cooper; [email protected]; Keir (Xen.org)
> Subject: Re: [PATCH] x86/hvm/viridian: zero and check vcpu context __pad
> field
>
> >>> On 30.03.16 at 12:32, <[email protected]> wrote:
> > --- a/xen/arch/x86/hvm/viridian.c
> > +++ b/xen/arch/x86/hvm/viridian.c
> > @@ -824,6 +824,8 @@ static int viridian_save_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> > for_each_vcpu( d, v ) {
> > struct hvm_viridian_vcpu_context ctxt;
> >
> > + memset(&ctxt, 0, sizeof(ctxt));
>
> How about just adding an empty initializer to the declaration?
>
I think having a 'zero the entire struct' call at the start is better as it
will cover any additions made to the struct in future. It's what I had
mistakenly assumed was already there. In fact I think adding a similar call
into the domain context save function would probably be worthwhile.
> > @@ -834,6 +836,15 @@ static int viridian_save_vcpu_ctxt(struct domain
> *d, hvm_domain_context_t *h)
> > return 0;
> > }
> >
> > +static bool_t is_zero(void *p, size_t size)
>
> At the very least this wants to be a pointer to const.
>
> > +{
> > + while ( size-- )
> > + if ( *(uint8_t *)p++ != 0 )
> > + return 0;
> > +
> > + return 1;
> > +}
> > +
> > static int viridian_load_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> > {
> > int vcpuid;
> > @@ -851,6 +862,9 @@ static int viridian_load_vcpu_ctxt(struct domain *d,
> hvm_domain_context_t *h)
> > if ( hvm_load_entry_zeroextend(VIRIDIAN_VCPU, h, &ctxt) != 0 )
> > return -EINVAL;
> >
> > + if ( !is_zero(&ctxt._pad, sizeof(ctxt._pad)) )
> > + return -EINVAL;
>
> "memcmp(&ctx._pad, zero_page, sizeof(ctxt._pad))" would be an
> alternative not requiring any new helper function.
>
Ah, I didn't know about the zero_page definition. I'll use that and drop the
helper.
Paul
> Jan
_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xen.org/xen-devel
