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

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, Oct 25, 2022 at 01:48:36PM -0400, Tamas K Lengyel wrote:
> On Tue, Oct 25, 2022 at 4:13 AM Roger Pau Monné <roger.pau@citrix.com>
> wrote:
> >
> > On Mon, Oct 24, 2022 at 12:58:54PM -0400, 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.
> >
> > I'm afraid I would benefit from some extra description about why you
> > need to introduce a separate hook instead of using the existing
> > do_rdmsr hook in arch_vpmu_ops (which is already hooked into
> > guest_rdmsr()).
> >
> > Are the MSRs you are trying to fetch not accessible for the guest
> > itself to read?
> 
> No, the reason we need this different hook is because do_rdmsr assumes the
> guest is reading the MSRs that are currently loaded. For external tools
> where v != current the vpmu context needs to be saved by pausing the vcpu
> first and then the MSR content returned from the saved context.

Hm, I see.

We need to dump the CPU MSR contents into the structure so they can be
read from a different pCPU differently than the currently running one.

It would be nice if this could all be somehow wired into
guest_rdmsr(), but the function executing a vcpu_pause() as part of
it's operations would be quite weird, also it having a vcpu parameter
is kind of misleading, as under some circumstances it will perform a
rdmsr and that's likely only correct when v == current.

I guess I will ask for others opinion, but having that specific vPMU
function call in XEN_DOMCTL_get_vcpu_msrs on the side of guest_rdmsr()
seems like a layering violation, as it should all be contained in
guest_rdmsr().

Thanks, Roger.