Re: [PATCH v3 4/6] xen/cpupool: Create different cpupools at boot time

[Julien Grall <julien@xxxxxxx>]

Hi,

On 23/03/2022 13:58, Luca Fancellu wrote:
> > On 22 Mar 2022, at 14:01, Julien Grall <julien@xxxxxxx> wrote:
> >
> > Hi,
> >
> > On 22/03/2022 09:52, Luca Fancellu wrote:
> > > > > > Can you document why this is necessary on x86 but not on other architectures?
> > > > > Hi Julien,
> > > > > I received the warning by Juergen here: 
> > > > > https://patchwork.kernel.org/comment/24740762/ that at least on x86 there could 
> > > > > be
> > > > > some problems if cpu0 is not in cpupool0, I tested it on arm and it was working fine 
> > > > > and I didn’t find any restriction.
> > > > What exactly did you test on Arm?
> > > I have tested start/stop of some guest, moving cpus between cpupools, 
> > > create/destroy cpupools, shutdown of Dom0
> > > [ from your last mail ]
> > > > > > If dom0 must run on core0 and core0 is Little then you cannot build a system 
> > > > > > where dom0 is running on big cores.
> > > > > > If the limitation is not there, you can build such a configuration without any 
> > > > > > dependency to the boot core type.
> > > > > This might not be completely clear so let me rephrase:
> > > > > In the current system:
> > > > > - dom0 must run on cpupool-0
> > > > I don't think we need this restriction. In fact, with this series it will 
> > > > become more a problem because the cpupool ID will based on how we parse the 
> > > > Device-Tree.
> > > >
> > > > So for dom0, we need to specify explicitely the cpupool to be used.
> > > >
> > > > > - cpupool-0 must contain the boot core
> > > > > - consequence: dom0 must run on the boot core
> > > > > If boot core is little, you cannot build as system where dom0 runs only on the 
> > > > > big cores.
> > > > > Removing the second limitation (which is not required on arm) is making it 
> > > > > possible.
> > > > IMHO removing the second restriction is a lot more risky than removing the 
> > > > first one.
> > > I see your point, my concern about moving Dom0 on another cpupool, different 
> > > from cpupool0, is that we give the
> > > opportunity to destroy the cpupool0 (we can’t let that happen), or remove every 
> > > cpu from cpupool0.
> >  From my understanding a cpupool can only be destroyed when there are no more 
> > CPUs in the pool. Given that cpu0 has to be in pool0 then this should prevent 
> > the pool to be destroyed.
> >
> > Now, it is quite possible that we don't have a check to prevent CPU0 to be 
> > removed from cpupool0. If so, then I would argue we should add the check 
> > otherwise it is pointless to prevent cpu0 to be initially added in another pool 
> > than pool0 but can be moved afterwards.
> Hi Julien,
>
> I’ve done a test on fvp, first finding is that cpu0 can be removed from Pool-0, 
> there is no check.
> Afterwards I’ve created another pool and I’ve assigned a cpu to it, I’ve called 
> xl cpupool-destroy and the tool removes every cpu from the pool before destroying.
>
> Do you think the check that prevents CPU0 to be removed from Pool-0 should be 
> done in the tools or in Xen?
I think we want a check at least in Xen (so we don't trust the tools to 
do the right thing).
We could also add one in the tools to provide better diagnostics to the 
user (this tends to be a request from Andrew).
Cheers,

--
Julien Grall