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

Re: [Xen-devel] [PATCH v2] x86/altp2m: Add xc_altp2m_get_vcpu_p2m_idx



>>> On 07.06.19 at 11:37, <aisaila@xxxxxxxxxxxxxxx> wrote:
> @@ -4735,6 +4736,29 @@ static int do_altp2m_op(
>                      _gfn(a.u.change_gfn.old_gfn),
>                      _gfn(a.u.change_gfn.new_gfn));
>          break;
> +
> +    case HVMOP_altp2m_get_p2m_idx:
> +    {
> +        struct vcpu *v;
> +
> +        if ( a.u.get_vcpu_p2m_idx.vcpu_id >= d->max_vcpus )

I'm sorry if it wasn't obvious from both my earlier reply and you
looking at the function, but this is redundant with ...

> +        {
> +            rc = -EINVAL;
> +            break;
> +        }
> +
> +        if ( !altp2m_active(d) )
> +        {
> +            rc = -EOPNOTSUPP;
> +            break;
> +        }
> +
> +        v = domain_vcpu(d, a.u.get_vcpu_p2m_idx.vcpu_id);

... this function call. The function specifically exists to abstract
away that bounds check in a correct / safe way. Obviously (I
hope) you need to check the return value of the function.

Also a note on the title: Naming a libxc function that gets introduced
anew doesn't seem optimal to me; in particular this doesn't clarify
that there's a new hypercall sub-op being introduced. I'd suggest to
either use a purely textual title here, or point out the hypercall op
that you introduce (and that the libxc function is just wrapping).

Jan



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

 


Rackspace

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