[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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 */


> +    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

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


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.