Re: [Xen-devel] [PATCH v10 1/6] x86: detect and initialize Cache QoS Monitoring feature

["Xu, Dongxiao" <dongxiao.xu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Monday, March 31, 2014 11:42 PM
> To: Xu, Dongxiao
> Cc: andrew.cooper3@xxxxxxxxxx; Ian.Campbell@xxxxxxxxxx;
> Ian.Jackson@xxxxxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx;
> xen-devel@xxxxxxxxxxxxx; konrad.wilk@xxxxxxxxxx; dgdegra@xxxxxxxxxxxxx;
> keir@xxxxxxx
> Subject: RE: [PATCH v10 1/6] x86: detect and initialize Cache QoS Monitoring
> feature
> 
> >>> On 31.03.14 at 17:33, <dongxiao.xu@xxxxxxxxx> wrote:
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> >>> On 26.03.14 at 07:35, <dongxiao.xu@xxxxxxxxx> wrote:
> >> > +static int cqm_add_socket(int socket)
> >>
> >> unsigned int.
> >
> > This function returns negative return values in error case.
> 
> Oh, sorry, this wasn't precise enough - I meant this for the parameter.
> 
> >> > +    /* According to Intel SDM, the possible maximum rmid number is 2^10
> = 1024,
> >> > +     * thus one page is enough to hold cqm->rmid_to_dom structure */
> >> > +    cqm->rmid_to_dom = alloc_xenheap_page();
> >>
> >> But please validate that this condition is being met (not necessarily here,
> >> perhaps rather where rmid_max gets determined).
> >
> > Okay, will add one ASSERT() to validate this condition.
> 
> An ASSERT() is the wrong thing here - non-debug builds would then
> happily continue. Just check the value to be in range, and stay away
> from enabling cache QoS if it's not.
> 
> >> > +
> >> > +    /* Allocate the buffer that holds MFNs of per-socket CQM LLC */
> >> > +    order = get_order_from_bytes(sizeof(unsigned long) * NR_CPUS);
> >> > +    cqm->socket_l3c_mfn = alloc_xenheap_pages(order, 0);
> >>
> >> You index this buffer by socket ID, yet socket ID values aren't
> >> necessarily linear/contiguous, and hence I don't think there are
> >> really any guarantees that the ID would be less than NR_CPUS. For
> >> the n-th time: I don't think you'll get around doing this properly, and
> >> I still think sufficient data should be retrievable from ACPI to
> >> determine a proper upper bound here.
> >
> > I think we need to assume socket ID is less than NR_CPUS, otherwise many of
> > current Xen's code is wrong, say credit scheduler 2.
> > xen/common/sched_credit2.c: init_pcpu()
> >     rqi = cpu_to_socket(cpu);
> >     rqd=prv->rqd + rqi;
> > Here the priv->rqd is defined as:
> >     struct csched_runqueue_data rqd[NR_CPUS];
> 
> Bad precedents are never a reason to introduce more problems.
> 
> > For getting socket info from ACPI, do you mean ACPI MADT table?
> > It can enumerate the existing APIC structures, which based on the fact that
> > the CPU is already plugged in the socket.
> > Per my understanding, I don't think it can enumerate empty CPU sockets.
> >
> > Can you help to elaborate more about how to get all socket number from ACPI?
> 
> Iirc disabled CPUs (i.e. hot-pluggable ones) are still listed in MADT,
> including their LAPIC ID (which the socket number is being calculated
> from).

I dumped one MADT table from the system, and following is one piece from it:
If the CPU is there, then APIC ID is shown as a valid value, while CPU is not 
there, then APIC ID is shown as 0xFF.
I know we can get socket info if CPU is there (by certain calculation), but if 
CPU is not there, could you help to explain how can we get the socket 
information from it?

...

[1E4h 0484  1]                Subtable Type : 00 <Processor Local APIC>
[1E5h 0485  1]                       Length : 08
[1E6h 0486  1]                 Processor ID : 3D
[1E7h 0487  1]                Local Apic ID : 3D
[1E8h 0488  4]        Flags (decoded below) : 00000001
                          Processor Enabled : 1

...

[1ECh 0492  1]                Subtable Type : 00 <Processor Local APIC>
[1EDh 0493  1]                       Length : 08
[1EEh 0494  1]                 Processor ID : FF
[1EFh 0495  1]                Local Apic ID : FF
[1F0h 0496  4]        Flags (decoded below) : 00000000
                          Processor Enabled : 0
...

Thanks,
Dongxiao

> 
> Jan


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