xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Stefano Stabellini <sstabellini@kernel.org>
To: Peng Fan <van.freenix@gmail.com>
Cc: jgross@suse.com, Peng Fan <peng.fan@nxp.com>,
	sstabellini@kernel.org,
	George Dunlap <george.dunlap@eu.citrix.com>,
	andrew.cooper3@citrix.com, dario.faggioli@citrix.com,
	xen-devel@lists.xen.org, Julien Grall <julien.grall@arm.com>,
	jbeulich@suse.com
Subject: Re: [RFC 0/5] xen/arm: support big.little SoC
Date: Mon, 19 Sep 2016 13:56:52 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.10.1609191341130.26423@sstabellini-ThinkPad-X260> (raw)
In-Reply-To: <20160919131553.GB7407@linux-u7w5.ap.freescale.net>

On Mon, 19 Sep 2016, Peng Fan wrote:
> On Mon, Sep 19, 2016 at 11:59:05AM +0200, Julien Grall wrote:
> >
> >
> >On 19/09/2016 11:38, Peng Fan wrote:
> >>On Mon, Sep 19, 2016 at 10:53:56AM +0200, Julien Grall wrote:
> >>>Hello,
> >>>
> >>>On 19/09/2016 10:36, Peng Fan wrote:
> >>>>On Mon, Sep 19, 2016 at 10:09:06AM +0200, Julien Grall wrote:
> >>>>>Hello Peng,
> >>>>>
> >>>>>On 19/09/2016 04:08, van.freenix@gmail.com wrote:
> >>>>>>From: Peng Fan <peng.fan@nxp.com>
> >>>>>>
> >>>>>>This patchset is to support XEN run on big.little SoC.
> >>>>>>The idea of the patch is from
> >>>>>>"https://lists.xenproject.org/archives/html/xen-devel/2016-05/msg00465.html"
> >>>>>>
> >>>>>>There are some changes to cpupool and add x86 stub functions to avoid build
> >>>>>>break. Sending The RFC patchset out is to request for comments to see whether
> >>>>>>this implementation is acceptable or not. Patchset have been tested based on
> >>>>>>xen-4.8 unstable on NXP i.MX8.
> >>>>>>
> >>>>>>I use Big/Little CPU and cpupool to explain the idea.
> >>>>>>A pool contains Big CPUs is called Big Pool.
> >>>>>>A pool contains Little CPUs is called Little Pool.
> >>>>>>If a pool does not contains any physical cpus, Little CPUs or Big CPUs
> >>>>>>can be added to the cpupool. But the cpupool can not contain both Little
> >>>>>>and Big CPUs. The CPUs in a cpupool must have the same cpu type(midr value for ARM).
> >>>>>>CPUs can not be added to the cpupool which contains cpus that have different cpu type.
> >>>>>>Little CPUs can not be moved to Big Pool if there are Big CPUs in Big Pool,
> >>>>>>and versa. Domain in Big Pool can not be migrated to Little Pool, and versa.
> >>>>>>When XEN tries to bringup all the CPUs, only add CPUs with the same cpu type(same midr value)
> >>>>>>into cpupool0.
> >>>>>
> >>>>>As mentioned in the mail you pointed above, this series is not enough to make
> >>>>>big.LITTLE working on then. Xen is always using the boot CPU to detect the
> >>>>>list of features. With big.LITTLE features may not be the same.
> >>>>>
> >>>>>And I would prefer to see Xen supporting big.LITTLE correctly before
> >>>>>beginning to think to expose big.LITTLE to the userspace (via cpupool)
> >>>>>automatically.
> >>>>
> >>>>Do you mean vcpus be scheduled between big and little cpus freely?
> >>>
> >>>By supporting big.LITTLE correctly I meant Xen thinks that all the cores has
> >>>the same set of features. So the feature detection is only done the boot CPU.
> >>>See processor_setup for instance...
> >>>
> >>>Moving vCPUs between big and little cores would be a hard task (cache line
> >>>issue, and possibly feature) and I don't expect to ever cross this in Xen.
> >>>However, I am expecting to see big.LITTLE exposed to the guest (i.e having
> >>>big and little vCPUs).
> >>
> >>big vCPUs scheduled on big Physical CPUs and little vCPUs scheduled on little
> >>physical cpus, right?
> >>If it is, is there is a need to let Xen think all the cores has the same set
> >>of features?
> >
> >I think you missed my point. The feature registers on big and little cores
> >may be different. Currently, Xen is reading the feature registers of the CPU
> >boot and wrongly assumes that those features will exists on all CPUs. This is
> >not the case and should be fixed before we are getting in trouble.
> >
> >>
> >>Developing big.little guest support, I am not sure how much efforts needed.
> >>Is this really needed?
> >
> >This is not necessary at the moment, although I have seen some interest about
> >it. Running a guest only on a little core is a nice beginning, but a guest
> >may want to take advantage of big.LITTLE (running hungry app on big one and
> >little on small one).
> >
> >>
> >>>
> >>>>
> >>>>This patchset is to use cpupool to block the vcpu be scheduled between big and
> >>>>little cpus.
> >>>>
> >>>>>
> >>>>>See for instance v->arch.actlr = READ_SYSREG32(ACTLR_EL1).
> >>>>
> >>>>Thanks for this. I only expose cpuid to guest, missed actlr. I'll check
> >>>>the A53 and A72 TRM about AArch64 implementationd defined registers.
> >>>>This actlr can be added to the cpupool_arch_info as midr.
> >>>>
> >>>>Reading "vcpu_initialise", seems only MIDR and ACTLR needs to be handled.
> >>>>Please advise if I missed anything else.
> >>>
> >>>Have you check the register emulation?
> >>
> >>Checked midr. Have not checked others.
> >>I think I missed some registers in ctxt_switch_to.
> >>
> >>>
> >>>>
> >>>>>
> >>>>>>
> >>>>>>Thinking an SoC with 4 A53(cpu[0-3]) + 2 A72(cpu[4-5]), cpu0 is the first one
> >>>>>>that boots up. When XEN tries to bringup secondary CPUs, add cpu[0-3] to
> >>>>>>cpupool0 and leave cpu[4-5] not in any cpupool. Then when Dom0 boots up,
> >>>>>>`xl cpupool-list -c` will show cpu[0-3] in Pool-0.
> >>>>>>
> >>>>>>Then use the following script to create a new cpupool and add cpu[4-5] to
> >>>>>>the cpupool.
> >>>>>>#xl cpupool-create name=\"Pool-A72\" sched=\"credit2\"
> >>>>>>#xl cpupool-cpu-add Pool-A72 4
> >>>>>>#xl cpupool-cpu-add Pool-A72 5
> >>>>>>#xl create -d /root/xen/domu-test pool=\"Pool-A72\"
> >>>>>
> >>>>>I am a bit confused with these runes. It means that only the first kind of
> >>>>>CPUs have pool assigned. Why don't you directly create all the pools at boot
> >>>>>time?
> >>>>
> >>>>If need to create all the pools, need to decided how many pools need to be created.
> >>>>I thought about this, but I do not come out a good idea.
> >>>>
> >>>>The cpupool0 is defined in xen/common/cpupool.c, if need to create many pools,
> >>>>need to alloc cpupools dynamically when booting. I would not like to change a
> >>>>lot to common code.
> >>>
> >>>Why? We should avoid to choose a specific design just because the common code
> >>>does not allow you to do it without heavy change.
> >>>
> >>>We never came across the big.LITTLE problem on x86, so it is normal to modify
> >>>the code.
> >>>
> >>>>The implementation in this patchset I think is an easy way to let Big and Little
> >>>>CPUs all run.
> >>>
> >>>I care about having a design allowing an easy use of big.LITTLE on Xen. Your
> >>>solution requires the administrator to know the underlying platform and
> >>>create the pool.
> >>
> >>I suppose big.little is mainly used in embedded SoC :). So the user(developer?)
> >>needs to know the hardware platform.
> >
> >The user will always be happy if Xen can save him a bit of time to create
> >cpupool. ;)
> >
> >>
> >>>
> >>>In the solution I suggested, the pools would be created by Xen (and the info
> >>>exposed to the userspace for the admin).
> >>
> >>I think the reason to create cpupools to support big.little SoC is to
> >>avoid vcpus scheduled between big and little physical cpus.
> >>
> >>If need to support big.little guest, I think no need to create more
> >>cpupools expect cpupoo0. Need to make sure vcpus not be scheduled between
> >>big and little physical cpus. All the cpus needs to be in one cpupool.
> >>
> >>>
> >>>>
> >>>>>
> >>>>>Also, in which pool a domain will be created if none is specified?
> >>>>>
> >>>>>>Now `xl cpupool-list -c` shows:
> >>>>>>Name            CPU list
> >>>>>>Pool-0          0,1,2,3
> >>>>>>Pool-A72        4,5
> >>>>>>
> >>>>>>`xl cpupool-list` shows:
> >>>>>>Name               CPUs   Sched     Active   Domain count
> >>>>>>Pool-0               4    credit       y          1
> >>>>>>Pool-A72             2   credit2       y          1
> >>>>>>
> >>>>>>`xl cpupool-cpu-remove Pool-A72 4`, then `xl cpupool-cpu-add Pool-0 4`
> >>>>>>not success, because Pool-0 contains A53 CPUs, but CPU4 is an A72 CPU.
> >>>>>>
> >>>>>>`xl cpupool-migrate DomU Pool-0` will also fail, because DomU is created
> >>>>>>in Pool-A72 with A72 vcpu, while Pool-0 have A53 physical cpus.
> >>>>>>
> >>>>>>Patch 1/5:
> >>>>>>use "cpumask_weight(cpupool0->cpu_valid);" to replace "num_online_cpus()",
> >>>>>>because num_online_cpus() counts all the online CPUs, but now we only
> >>>>>>need Big or Little CPUs.
> >>>>>
> >>>>>So if I understand correctly, if the boot CPU is a little CPU, DOM0 will
> >>>>>always be able to only use little ones. Is that right?
> >>>>
> >>>>Yeah. Dom0 only use the little ones.
> >>>
> >>>This is really bad, dom0 on normal case will have all the backends. It may
> >>>not be possible to select the boot CPU, and therefore always get a little
> >>>CPU.
> >>
> >>Dom0 runs in cpupool0. cpupool0 only contains the cpu[0-3] in my case.
> >
> >So the performance of dom0 will be impacted because it will only use little
> >cores.
> >
> >>
> >>>
> >>>Creating the pool at boot time would have avoid a such issue because, unless
> >>>we expose big.LITTLE to dom0 (I would need the input of George and Dario for
> >>>this bits), we could have a parameter to specify which set of CPUs (e.g pool)
> >>>to allocate dom0 vCPUs.
> >>
> >>dom0 is control domain, I think no need to expose big.little for dom0.
> >>Pin VCPU to specific physical cpus, this may help support big.little guest.
> >>
> >>>
> >>>Note, that I am not asking you to implement everything. But I think we need a
> >>>coherent view of big.LITTLE support in Xen today to go forward.
> >>
> >>Yeah. Then you prefer supporting big.little guest?
> >
> >I have seen some interest on it.
> 
> I have no idea about the use cases. Anyway big.little guest do have benefits.
> 
> >
> >>Please advise if you have any plan/ideas or what I can do on this.
> >
> >I already gave some ideas on what could be done for big.LITTLE support. But,
> >I admit I haven't yet much think about it, so I may miss some part.
> 
> This comes to me a question about Dom0. Do we need to let Dom0 be big.little? or
> add a bootargs to indicate big.little, big or little for Dom0?

These are my two cents: the two cpupools (one big, one LITTLE) need to
be created automatically, either by Xen or by an xl command, such as "xl
cpupool-bigLITTLE-split", as George suggested. If done with an xl
command, it needs to be called automatically at boot somehow, otherwise
we'll leave half of the cpus unused by default. As Xen needs to be able
to recognize big.LITTLE and only start Dom0 on big (or LITTLE) cpus, I
think it might make sense to create both cpupools directly in Xen.

It should be possible to configure to which cpupool Dom0 belongs to, but
I would default to big for performance reasons.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2016-09-19 20:56 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-19  2:08 [RFC 0/5] xen/arm: support big.little SoC van.freenix
2016-09-19  2:08 ` [RFC 1/5] xen/arm: domain_build: setting opt_dom0_max_vcpus according to cpupool0 info van.freenix
2016-09-19  2:08 ` [RFC 2/5] xen: cpupool: introduce cpupool_arch_info van.freenix
2016-09-19  2:08 ` [RFC 3/5] xen: cpupool: add arch cpupool hook van.freenix
2016-09-19  2:08 ` [RFC 4/5] xen/arm: move vpidr from arch_domain to arch_vcpu van.freenix
2016-09-19  2:08 ` [RFC 5/5] xen/arm: cpupool: implement arch_domain_cpupool_compatible van.freenix
2016-09-19  8:09 ` [RFC 0/5] xen/arm: support big.little SoC Julien Grall
2016-09-19  8:36   ` Peng Fan
2016-09-19  8:53     ` Julien Grall
2016-09-19  9:38       ` Peng Fan
2016-09-19  9:59         ` Julien Grall
2016-09-19 13:15           ` Peng Fan
2016-09-19 20:56             ` Stefano Stabellini [this message]
2016-09-19  9:45       ` George Dunlap
2016-09-19 10:06         ` Julien Grall
2016-09-19 10:23           ` Juergen Gross
2016-09-19 17:18             ` Dario Faggioli
2016-09-19 21:03               ` Stefano Stabellini
2016-09-19 22:55                 ` Dario Faggioli
2016-09-20  0:01                   ` Stefano Stabellini
2016-09-20  0:54                     ` Dario Faggioli
2016-09-20 10:03                       ` Peng Fan
2016-09-20 10:27                         ` George Dunlap
2016-09-20 15:34                           ` Julien Grall
2016-09-20 17:24                             ` Dario Faggioli
2016-09-20 19:09                             ` Stefano Stabellini
2016-09-20 19:41                               ` Julien Grall
2016-09-20 20:17                                 ` Stefano Stabellini
2016-09-21  8:38                                   ` Peng Fan
2016-09-21  9:22                                     ` George Dunlap
2016-09-21 12:35                                       ` Peng Fan
2016-09-21 15:00                                       ` Dario Faggioli
2016-09-21 10:15                                     ` Julien Grall
2016-09-21 12:28                                       ` Peng Fan
2016-09-21 15:06                                         ` Dario Faggioli
2016-09-22  9:45                                       ` Peng Fan
2016-09-22 11:21                                         ` Julien Grall
2016-09-23  2:38                                           ` Peng Fan
2016-09-21 10:09                                   ` Julien Grall
2016-09-21 10:22                                     ` George Dunlap
2016-09-21 13:06                                       ` Julien Grall
2016-09-21 15:45                                         ` Dario Faggioli
2016-09-21 19:28                                           ` Julien Grall
2016-09-22  6:16                                             ` Peng Fan
2016-09-22  8:43                                             ` Dario Faggioli
2016-09-22 11:24                                               ` Julien Grall
2016-09-22 16:31                                                 ` Dario Faggioli
2016-09-23 13:56                                                   ` Julien Grall
2016-09-21 18:13                                         ` Stefano Stabellini
2016-09-21 19:11                                           ` Julien Grall
2016-09-21 19:21                                             ` Julien Grall
2016-09-21 23:45                                             ` Stefano Stabellini
2016-09-22  6:49                                             ` Peng Fan
2016-09-22  8:50                                               ` Dario Faggioli
2016-09-22  9:27                                                 ` Peng Fan
2016-09-22  9:51                                                   ` George Dunlap
2016-09-22 10:09                                                     ` Peng Fan
2016-09-22 10:39                                                       ` Dario Faggioli
2016-09-22 10:13                                                     ` Juergen Gross
2016-09-22  9:52                                                   ` Dario Faggioli
2016-09-22 11:29                                                   ` Julien Grall
2016-09-22 17:31                                                     ` Stefano Stabellini
2016-09-22 18:54                                                       ` Julien Grall
2016-09-23  2:14                                                         ` Peng Fan
2016-09-23  9:24                                                           ` Julien Grall
2016-09-23 10:05                                                             ` Peng Fan
2016-09-23 10:15                                                               ` Julien Grall
2016-09-23 13:36                                                                 ` Dario Faggioli
2016-09-24  1:57                                                                   ` Stefano Stabellini
2016-09-23 13:52                                                               ` Dario Faggioli
2016-09-24  1:35                                                         ` Stefano Stabellini
2016-09-23  2:03                                                     ` Peng Fan
2016-09-22 10:05                                                 ` Peng Fan
2016-09-22 16:26                                                   ` Dario Faggioli
2016-09-22 17:33                                                     ` Stefano Stabellini
2016-09-21 12:38                                     ` Peng Fan
2016-09-21  9:45                         ` Dario Faggioli
2016-09-20 10:18                     ` George Dunlap
2016-09-19 20:55             ` Stefano Stabellini
2016-09-19 10:33           ` George Dunlap
2016-09-19 13:33             ` Peng Fan
2016-09-20  0:11               ` Dario Faggioli
2016-09-20  6:18                 ` Peng Fan
2016-09-19 16:43             ` Dario Faggioli
2016-09-19 13:08       ` Peng Fan

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.DEB.2.10.1609191341130.26423@sstabellini-ThinkPad-X260 \
    --to=sstabellini@kernel.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=dario.faggioli@citrix.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=jbeulich@suse.com \
    --cc=jgross@suse.com \
    --cc=julien.grall@arm.com \
    --cc=peng.fan@nxp.com \
    --cc=van.freenix@gmail.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).