On 21.07.2025 11:49, Bernhard Kaindl wrote:
> --- a/xen/include/public/vcpu.h
> +++ b/xen/include/public/vcpu.h
> @@ -79,8 +79,18 @@ struct vcpu_runstate_info {
> uint64_t time[4];
> };
> typedef struct vcpu_runstate_info vcpu_runstate_info_t;
> +/* vcpu_runstate_info_t is in the Guest shared memory area (frozen ABI) */
> DEFINE_XEN_GUEST_HANDLE(vcpu_runstate_info_t);
Personally I'm opposed to this kind of comment.
> +/*
> + * Extra information returned from vcpu_runstate_get that is not part
> + * of the Guest shared memory area (not part of the frozen Guest ABI)
> + */
> +struct vcpu_runstate_extra {
> + uint64_t nonaffine_time; /* Time running outside soft_affinity mask */
> +};
> +typedef struct vcpu_runstate_extra vcpu_runstate_extra_t;
And the next time we need some further piece of statistics, we'll have
to add yet another new sub-hypercall? Or wait, there is no new sub-
hypercall here. How's this piece of data going to make it out to guest
space then?
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -1110,8 +1110,8 @@ int vcpu_set_hard_affinity(struct vcpu *v, const
> cpumask_t *affinity);
> int vcpu_affinity_domctl(struct domain *d, uint32_t cmd,
> struct xen_domctl_vcpuaffinity *vcpuaff);
>
> -void vcpu_runstate_get(const struct vcpu *v,
> - struct vcpu_runstate_info *runstate);
> +vcpu_runstate_extra_t vcpu_runstate_get(const struct vcpu *v,
> + struct vcpu_runstate_info *runstate);
As long as it's only a single 64-bit field in the struct, returning this
by value may be okay(ish). Yet I'd still recommend against doing so.
Also, having reached the end of the patch: Where's the caller making use
of this new return value?
Jan
