Re: [PATCH v2 1/5] x86/HVM: introduce hvm_point_entry()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/11/2023 1:46 pm, Jan Beulich wrote:
> ... to accompany hvm_read_entry() when actual copying isn't desirable.
> This allows to remove open-coded stream accesses from hpet_load(),
> along with using the helper in hvm_load() itself.
>
> Since arch_hvm_load()'s declaration would need changing, and since the
> function is not used from elsewhere, purge the declaration. With that it
> makes little sense to keep arch_hvm_save()'s around; convert that
> function to static then at the same time.
>
> In hpet_load() simplify the specific case of error return that's in
> context anyway: There's no need to hold the lock when only updating a
> local variable.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

I really do hate all of this logic and most of it wants to live in
userspace, but this an improvement.

The only thing I'm a little concerned with is the name. 
hvm_read_entry() clearly consumes an entry, while hvm_point_entry()
does, but doesn't obviously convey this at a glance.

Ideally I'd say that hvm_{get,copy}_entry() would be better nomeclature,
as both have at least a vague implication of unmarshalling and one
clearly is making a separate copy.

Thoughts?

~Andrew