Re: [PATCH HVM v2 1/1] hvm: refactor set param

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.02.2021 21:46, Norbert Manthey wrote:
> I agree with the symmetry for get and set. This is what I'd aim for:
> 
>  1. hvmop_set_param and hvmop_get_param (static) both check for the
> index, and afterwards use the is_hvm_domain(d) function with its barrier
>  2. hvm_set_param (static) and hvm_get_param both call their allow
> helper function, evaluate the return code, and afterwards block speculation.
>  2.1. hvm_get_param is declared in a public header, and cannot be turned
> into a static function, hence needs the index check

But both further call sites are in bounded loops, with the bounds not
guest controlled. It can rely on the callers just as much as ...

>  2.2. hvm_set_param is only called from hvmop_set_param, and index is
> already checked there, hence, do not add check

... this.

>  3. hvm_allow_set_param (static) and hvm_allow_get_param (static) do not
> validate the index parameter
>  3.1. hvm_allow_set_param blocks speculative execution with a barrier
> after domain permissions have been evaluated, before accessing the
> parameters of the domain. hvm_allow_get_param does not access the params
> member of the domain, and hence does not require additional protection.
> 
> To simplify the code, I propose to furthermore make the hvmop_set_param
> function static as well.

Yes - this not being so already is likely simply an oversight,
supported by the fact that there's no declaration in any header.

Jan