Re: [Xen-devel] [PATCH Altp2m cleanup v5 1/3] altp2m cleanup work.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.09.16 at 18:46, <paul.c.lai@xxxxxxxxx> wrote:
> Indent goto labels by one space.
> Inline (header) altp2m functions.
> In do_altp2m_op(), during the sanity check of the passed command,
> return -ENONSYS if not a valid command.
> In do_altp2m_op(), when evaluating a command, ASSERT_UNREACHABLE()
> if the command is not recognizable.  The sanity check above should
> have triggered the return of -ENOSYS.
> Make altp2m_vcpu_emulate_ve() return actual bool_t (rather than return
> void()).
> 
> Signed-off-by: Paul Lai <paul.c.lai@xxxxxxxxx>
> ---

It is very disappointing to find that there is still no information here
on what changed from the previous version.

> @@ -5349,6 +5362,8 @@ static int do_altp2m_op(
>              rc = p2m_change_altp2m_gfn(d, a.u.change_gfn.view,
>                      _gfn(a.u.change_gfn.old_gfn),
>                      _gfn(a.u.change_gfn.new_gfn));
> +    default:
> +        ASSERT_UNREACHABLE();
>      }

And it is even worse that the bug pointed out here is still present.

>  /* emulates #VE */
> -bool_t altp2m_vcpu_emulate_ve(struct vcpu *v);
> +static inline bool_t altp2m_vcpu_emulate_ve(struct vcpu *v)

Nor did you switch to plain bool here, as was requested during both
v3 and v4 review.

Please do not resubmit this series until you have taken care of
_all_ review comments you've received so far. As with the
previous version I'm not going to spend time looking at the other
two patches due to this fundamental requirement, despite
having been pointed out before, not being met.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel