On Wed, Mar 31, 2021 at 05:24:24PM +0100, Andrew Cooper wrote:
> On 31/03/2021 17:02, Jan Beulich wrote:
> > On 31.03.2021 12:32, Roger Pau Monne wrote:
> >> EOIs are always executed in guest vCPU context, so there's no reason to
> >> pass a vCPU parameter around as can be fetched from current.
> >>
> >> While there make the vector parameter of both callbacks unsigned int.
> >>
> >> No functional change intended.
> >>
> >> Suggested-by: Paul Durrant <pdurr...@amazon.com>
> >> Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> >> Reviewed-by: Paul Durrant <p...@xen.org>
> >> ---
> >> Changes since v1:
> >> - New in this version.
> > I'm afraid the situation with viridian_synic_wrmsr() hasn't changed
> > since v2, and hence my previous comment still applies.
>
> Only just spotted that line of reasoning.
>
> Longterm, I do want to remove all the Viridian special cases, and handle
> all of the state via architectural mechanisms (cpu policy for static
> settings, and the existing MSR records for dynamic content).
>
> A consequence of this cleanup is that guest_{rd,wr}msr() will be
> eventually be used to save/restore dynamic state in the migrate stream,
> which is why I've been engineering guest_{rd,wr}msr() to work for MSRs
> in "current || !scheduled" context.
>
> As such, it is important to retain a vcpu pointer because it will not be
> current on the save/restore hypercalls, which execute in control domain
> context.
But you are never going to restore an HV_X64_MSR_EOI MSR, as such
should never be part of the migrate stream in the first place. It
doesn't have a value itself - it's only used as an alternative way to
EOI an interrupt.
I still think the EOIs will always be performed on the affected vCPU
context, but I don't want this discussion the block the current
series, so I'm just going to drop this patch.
Roger.
