Re: [Xen-devel] [PATCH] x86: move vgc_flags to struct pv_vcpu

[Julien Grall <julien@xxxxxxx>]

Hi,

On 03/01/2020 11:05, Jan Beulich wrote:
> On 03.01.2020 11:56, Julien Grall wrote:
> > Hi,
> >
> > On 27/12/2019 12:14, Jan Beulich wrote:
> > > On 27.12.2019 12:27, Julien Grall wrote:
> > > > Hi Jan,
> > > >
> > > > On Fri, 27 Dec 2019, 09:22 Jan Beulich, <JBeulich@xxxxxxxx> wrote:
> > > >
> > > > > On 23.12.2019 18:33, Julien Grall wrote:
> > > > > > Hi Jan,
> > > > > >
> > > > > > On 20/12/2019 14:55, Jan Beulich wrote:
> > > > > > > There's been effectively no use of the field for HVM.
> > > > > > >
> > > > > > > Also shrink the field to unsigned int, even if this doesn't immediately
> > > > > > > yield any space benefit for the structure itself. The resulting 32-bit
> > > > > > > padding slot can eventually be used for some other field. The change in
> > > > > > > size makes accesses slightly more efficient though, as no REX.W prefix
> > > > > > > is going to be needed anymore on the respective insns.
> > > > > > >
> > > > > > > Mirror the HVM side change here (dropping of setting the field to
> > > > > > > VGCF_online) also to Arm, on the assumption that it was cloned like
> > > > > > > this originally. VGCF_online really should simply and consistently be
> > > > > > > the guest view of the inverse of VPF_down, and hence needs representing
> > > > > > > only in the get/set vCPU context interfaces.
> > > > > > But vPSCI is just a wrapper to get/set vCPU context interfaces. Your
> > > > > > changes below will clearly break bring-up of secondary vCPUs on Arm.
> > > > > >
> > > > > > This is because arch_set_guest_info() will rely on this flag to
> > > > > > clear/set VPF_down in the pause flags.
> > > > > >
> > > > > > So I think the line in arm/vpsci.c should be left alone.
> > > > > Oh, I see - I didn't notice this (ab)use despite ...
> > > > Out of Interest, why do you think it is an abuse here and not in the
> > > > toolstack?
> > > >
> > > > How do you suggest to improve it? I can add it in my improvement list for
> > > > Arm.
> > > Oh, "abuse" was about the arch_set_guest_info() use, not the use of
> > > the flag by the tool stack.
> > I may have read incorrectly your e-mail, although I think my questions
> > about why this is an abuse and how do you suggest to improve are still
> > relevant.
> arch_set_info_guest() is intended to be used for exactly one purpose
> - vCPU context initialization via hypercall. With this, and _without_
> me knowing anything about PSCI, it _looks_ to me to be an abuse. 
PSCI (Power State Coordination Interface) is a generic way to manage the 
power on your plarform (e.g CPU bring-up).
The CPU_ON call will actually initialize the vCPU context and then start 
the vCPU.
Whilst, this is not a Xen specific interface, they are still hypercall 
based.
> I'd
> expect there to be something in x86 that could be used for
> comparison, and whatever that is - it doesn't need a similar extra
> use of arch_set_info_guest().
How do you manage secondary CPUs on HVM/PVH guest?

Cheers,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel