Re: [Xen-devel] [PATCH V5 2/4] x86/altp2m: Add hypercall to set a range of sve bits

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.12.2019 10:42, Alexandru Stefan ISAILA wrote:
> --- a/xen/include/public/hvm/hvm_op.h
> +++ b/xen/include/public/hvm/hvm_op.h
> @@ -46,6 +46,16 @@ struct xen_hvm_altp2m_suppress_ve {
>      uint64_t gfn;
>  };
>  
> +struct xen_hvm_altp2m_suppress_ve_multi {
> +    uint16_t view;
> +    uint8_t suppress_ve; /* Boolean type. */
> +    uint8_t pad1;
> +    int32_t first_error_code; /* Must be set to 0 . */
> +    uint64_t first_gfn; /* Value will be updated */

s/will/may/

> +    uint64_t last_gfn;
> +    uint64_t first_error; /* Gfn of the first error. Must be set to 0. */

There's no real "must" here. Please at most say "should", but I'd
even consider dropping that part of the comment altogether. The
consumer logic needs to key off of the error code anyway. Even
for the error code field I'd suggest saying just "should": You
don't check it (because you can't), and the caller only shoots
itself in the foot if it doesn't do so.

Also looking at this yet again - I think field names would better
be "first_error" for the error code and "first_error_gfn" for the
GFN.

Anyway, preferably with the suggested adjustments, applicable
hypervisor parts
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>


Jan

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