Re: [Xen-devel] [PATCH v8 03/13] x86: maintain COS to CBM mapping for each socket

[Chao Peng <chao.p.peng@xxxxxxxxxxxxxxx>]

On Fri, May 29, 2015 at 04:38:31PM +0800, Chao Peng wrote:
> On Fri, May 29, 2015 at 09:06:46AM +0100, Jan Beulich wrote:
> > >>> On 29.05.15 at 04:43, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> > > On Thu, May 28, 2015 at 02:17:54PM +0100, Jan Beulich wrote:
> > >> >>> On 21.05.15 at 10:41, <chao.p.peng@xxxxxxxxxxxxxxx> wrote:
> > >> > +static int cat_cpu_init(unsigned int cpu)
> > >> > +{
> > >> > +    int rc;
> > >> > +    const struct cpuinfo_x86 *c = cpu_data + cpu;
> > >> > +
> > >> > +    if ( !cpu_has(c, X86_FEATURE_CAT) )
> > >> > +        return 0;
> > >> > +
> > >> > +    if ( test_bit(cpu_to_socket(cpu), cat_socket_enable) )
> > >> > +        return 0;
> > >> > +
> > >> > +    if ( cpu == smp_processor_id() )
> > >> > +        do_cat_cpu_init(&rc);
> > >> > +    else
> > >> > +        on_selected_cpus(cpumask_of(cpu), do_cat_cpu_init, &rc, 1);
> > >> 
> > >> This now being called in the context of CPU_UP_PREPARE, I can't see
> > >> how this works at all: Neither would the CPU's cpu_data[] instance be
> > >> initialized by that time, nor would you be able to IPI that CPU, nor can 
> > >> I
> > >> see how the if() branch could ever get entered. Was this tested at all?
> > > 
> > > Ah, yes! So it sounds really a little difficult to move the memory
> > > allocation from CPU_STARTING to CPU_PREPARA for this case.
> > 
> > Not sure why you talk about memory allocation again. That should
> > be done in CPU_UP_PREPARE. But stuff that needs to happen on
> > the CPU should happen in CPU_STARTING. The memory allocation's
> > size depending on a CPU characteristic of course makes this a little
> > problematic, but (I think I said so before) since we're assuming
> > symmetry in many other places, I don't see anything wrong with
> > you assuming symmetry here too, and hence use e.g. the boot CPU's
> > value to determine the allocation size.
> 
> No problem, then I can just forget the support for asymmetry in XEN.

As this is quite different with our original assumption and what I
described in the design doc, I'd like to have a clear summary here
before submitting the new version.

Basically speaking, the initial design tries to support systems have
different SKUs for each socket. One example would be when plugging one
HSX (Haswell server) and BDX (Broadwell server) processor into each
socket of a Grantley platform.

However, there are difficulties to support this than just to support
systems that always have the same SKUs, AFAICS:

1)  Not able to detect nr_sockets correctly at booting time, especially
when taking cpu hotplug into account. This is also why I added a boot
option for this at the beginning of this patch serial, while I agreed
it's really not a good interface for user.

2)  Unfeasible to allocate memory first and do initialization later in
cpu hotplug notifications. My former approach is performing both the
allocation and initialization in the CPU_STARTING, which is not a good
idea indicated by Jan.

For me, it's better to have it supported. But if that's difficult (just
as described above), I feel comfortable to drop it.

Let's see what changes will be made once that support is dropped:

1)  Current per-socket psr_cat_socket_info will be dropped, instead
psr_cat will be introduced which holds cos_max/cbm_len for all the
sockets, and it will be initialized only once.

2)  cat_socket_enable will be dropped as well, instead check
!!psr_cat, just as CMT did.

3)  No special hotplug consideration, as the CAT hardware information on
booting cpu is applied to others.

4)  We still support per socket cos configuration, so per socket 
'struct psr_cat_cbm *cos_to_cbm' becomes something like
'struct psr_cat_cbm *socket_cbms' which holds all the cos_to_cbm for all
the sockets, and it will be initialized at booting time.

5)  The 'target' in xen_sysctl_psr_cat_op will be removed as now all the
sockets have the same hardware info. Due to this, I'd like to retrofit
xen_sysctl_psr_cmt_op but not introduce a new sub-op for this.
XEN_DOMCTL_psr_cat_op however will still be there.

6)  Related changes in tools side/documentation also should be done.

Though the list looks long but generally it becomes simple (of course).

Suggestions are welcome.

Chao

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