Re: [PATCH] xen/x86: Make XEN_DOMCTL_get_vcpu_msrs more configurable

[Tamas K Lengyel <tamas.k.lengyel@xxxxxxxxx>]

On Wed, Oct 26, 2022, 7:48 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:

On 26.10.2022 13:34, Tamas K Lengyel wrote:
> On Wed, Oct 26, 2022, 7:06 AM Andrew Cooper <Andrew.Cooper3@xxxxxxxxxx>
> wrote:
>
>> On 24/10/2022 17:58, Tamas K Lengyel wrote:
>>> Currently the XEN_DOMCTL_get_vcpu_msrs is only capable of gathering a
>> handful
>>> of predetermined vcpu MSRs. In our use-case gathering the vPMU MSRs by an
>>> external privileged tool is necessary, thus we extend the domctl to
>> allow for
>>> querying for any guest MSRs. To remain compatible with the existing
>> setup if
>>> no specific MSR is requested via the domctl the default list is returned.
>>>
>>> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
>>
>> Naming aside, XEN_DOMCTL_{get,set}_vcpu_msrs is supposed to be "get me
>> all MSRs needed to migrate a vCPU".  (I do intend to retire the
>> hypercall as part of fixing the Xen side of migration, but that's ages
>> away)
>>
>> It seems like what you want is something more like
>> XEN_DOMCTL_{rd,wr}msr_list  (convenient timing, given the recent ISE
>> update).  I think those would be better as a separate pair of
>> hypercalls, rather than trying to repurpose an existing hypercall.
>>
>>
>> As for actually getting the values, please fix up guest_{rd,wr}msr() to
>> access the perf MSRs safely.  I know the vpmu MSR handling is in a
>> tragic state, but this new get_msr subop is making the problem even more
>> tangled.
>>
>
> Adding a separate hypercall is fine.

Which will then also avoid altering the behavior of the existing hypercall:
You can't just assume e.g. input fields to be zeroed (or otherwise
suitably set) by existing callers, when those were previously output only.

I did add a memset to zero it on the single caller I could find.

Tamas