On 10/20/2017 03:22 AM, Lorenzo Pieralisi wrote: > On Thu, Oct 19, 2017 at 11:54:22AM -0500, Jeremy Linton wrote: > > [...] > >>>> + cpu_topology[cpu].core_id = topology_id; >>>> + topology_id = setup_acpi_cpu_topology(cpu, 2); >>>> + cpu_topology[cpu].cluster_id = topology_id; >>>> + topology_id = setup_acpi_cpu_topology(cpu, max_topo); >>> >>> If you want a package id (that's just a package tag to group cores), you >>> should not use a large level because you know how setup_acpi_cpu_topology()works, you should add an API that allows you to retrieve the package id >>> (so that you can use th ACPI_PPTT_PHYSICAL_PACKAGE flag consistenly, >>> whatever it represents). >> >> I don't think the spec requires the use of PHYSICAL_PACKAGE... Am I >> misreading it? Which means we need to "pick" a node level to >> represent the physical package if one doesn't exist... > > The specs define a means to detect if a given PPTT node corresponds to a > package (I am refraining from stating again that to me that's not clean > cut what a package is _architecturally_, I think you know my POV by now) > and that's what you need to use to retrieve a packageid for a given cpu, > if I understand the aim of the physical package flag. > > Either that or that flag is completely useless. > > Lorenzo > > ACPI 6.2 - Table 5-151 (page 248) > Physical package > ----------------- > Set to 1 if this node of the processor topology represents the boundary > of a physical package, whether socketed or surface mounted. Set to 0 if > this instance of the processor topology does not represent the boundary > of a physical package. > I've been following the discussion and I'm not sure I understand what the confusion is around having a physical package ID. Since I'm the one that insisted it be in the spec, I'd be glad to clarify anything. My apologies for not saying anything sooner but things IRL have been very complicated of late. What was intended was a simple flag that was meant to tell me if a CPU ID (this could be a CPU, a cluster, a processor container -- I don't really care which) is *also* an actual physical device on a motherboard. That is the only intent; there was no architectural meaning intended at all -- that is what the PPTT structures are for, in conjunction with any DSDT information uncovered later in the boot process. However, in the broader server ecosystem, this can be incredibly useful. There are a significant number of software products sold or supported that base their fees on the number of physical sockets in use. There have been in the past (and may be in the near future) machines where the cost of the lease on the machine is determined by how many physical sockets (or even CPUs) are in use, even if the machine has many more available. Some vendors also include FRU (Field Replaceable Unit) location info in their ACPI tables. So, for example, one or more CPUs or caches might fail in one physical package, which is then reported to a maintenance system of some sort that tells some human which of the physical sockets on what motherboard needs a replacement device, or it's simply noted and shut off until it's time to replace the entire server, or perhaps it's logged and used in an algorithm to predict when the server might fail completely. So, that's why the flag exists in the spec. It seems to make sense to me to have a package ID as part of struct cpu_topology -- it might even be really handy for CPU hotplug. If you don't, it seems to me a whole separate struct would be needed with more cpumasks to show who belongs to what physical package; that might be okay but seems unnecessarily complicated to me. You can also tell me that I have completely missed the point of the discussion so far :-). But if you do, you have to tell me what I missed. Hope this helps clarify... -- ciao, al ----------------------------------- Al Stone Software Engineer Red Hat, Inc. ahs3@xxxxxxxxxx ----------------------------------- -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
