On Thu, Mar 12, 2020 at 11:28:47AM -0500, Babu Moger wrote: > Eduardo, Can you please queue the series if there are no concerns. > Thanks
I had queued it for today's pull request, but it looks like it breaks "make check". See https://travis-ci.org/github/ehabkost/qemu/jobs/663529282 PASS 4 bios-tables-test /x86_64/acpi/piix4/ipmi Could not access KVM kernel module: No such file or directory qemu-system-x86_64: -accel kvm: failed to initialize kvm: No such file or directory qemu-system-x86_64: falling back to tcg qemu-system-x86_64: Invalid CPU [socket: 0, die: 0, core: 1, thread: 0] with APIC ID 1, valid index range 0:5 Broken pipe /home/travis/build/ehabkost/qemu/tests/qtest/libqtest.c:166: kill_qemu() tried to terminate QEMU process but encountered exit status 1 (expected 0) Aborted (core dumped) ERROR - too few tests run (expected 17, got 4) /home/travis/build/ehabkost/qemu/tests/Makefile.include:633: recipe for target 'check-qtest-x86_64' failed make: *** [check-qtest-x86_64] Error 1 > > On 3/11/20 5:52 PM, Babu Moger wrote: > > This series fixes APIC ID encoding problem reported on AMD EPYC cpu models. > > https://bugzilla.redhat.com/show_bug.cgi?id=1728166 > > > > Currently, the APIC ID is decoded based on the sequence > > sockets->dies->cores->threads. This works for most standard AMD and other > > vendors' configurations, but this decoding sequence does not follow that of > > AMD's APIC ID enumeration strictly. In some cases this can cause CPU > > topology > > inconsistency. When booting a guest VM, the kernel tries to validate the > > topology, and finds it inconsistent with the enumeration of EPYC cpu models. > > > > To fix the problem we need to build the topology as per the Processor > > Programming Reference (PPR) for AMD Family 17h Model 01h, Revision B1 > > Processors. The documentation is available from the bugzilla Link below. > > > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=206537 > > > > Here is the text from the PPR. > > Operating systems are expected to use Core::X86::Cpuid::SizeId[ApicIdSize], > > the > > number of least significant bits in the Initial APIC ID that indicate core > > ID > > within a processor, in constructing per-core CPUID masks. > > Core::X86::Cpuid::SizeId[ApicIdSize] determines the maximum number of cores > > (MNC) that the processor could theoretically support, not the actual number > > of > > cores that are actually implemented or enabled on the processor, as > > indicated > > by Core::X86::Cpuid::SizeId[NC]. > > Each Core::X86::Apic::ApicId[ApicId] register is preset as follows: > > • ApicId[6] = Socket ID. > > • ApicId[5:4] = Node ID. > > • ApicId[3] = Logical CCX L3 complex ID > > • ApicId[2:0]= (SMT) ? {LogicalCoreID[1:0],ThreadId} : > > {1'b0,LogicalCoreID[1:0]} > > > > v7: > > Generated the patches on top of git://github.com/ehabkost/qemu.git > > (x86-next). > > Changes from v6. > > 1. Added new function x86_set_epyc_topo_handlers to override the apic id > > encoding handlers. > > 2. Separated the code to set use_epyc_apic_id_encoding and added as a new > > patch > > as it looked more logical. > > 3. Fixed minor typos. > > > > v6: > > > > https://lore.kernel.org/qemu-devel/158389385028.22020.7608244627303132902.st...@naples-babu.amd.com/ > > Generated the patches on top of git://github.com/ehabkost/qemu.git > > (x86-next). > > Changes from v5. > > 1. Eduardo has already queued couple of patches, submitting the rest here. > > 2. Major change is how the EPYC mode apic id encoding handlers are loaded. > > Added a boolean variable use_epyc_apic_id_encoding in X86CPUDefinition. > > The variable is will be used to tell if we need to use EPYC mode > > encoding. > > 3. Eduardo reported bysectability problem with x86 unit test code. > > Quashed the patches in 1 and 2 to resolve it. Problem was change in > > calling > > conventions of topology related functions. > > 4. Also set the use_epyc_apic_id_encoding for EPYC-Rome. This model is > > added recently to the cpu table. > > > > v5: > > > > https://lore.kernel.org/qemu-devel/158326531474.40452.11433722850425537745.st...@naples-babu.amd.com/ > > Generated the patches on top of git://github.com/ehabkost/qemu.git > > (x86-next). > > Changes from v4. > > 1. Re-arranged the patches 2 and 4 as suggested by Igor. > > 2. Kept the apicid handler functions inside X86MachineState as discussed. > > These handlers are loaded from X86CPUDefinitions. > > 3. Removed unnecessary X86CPUstate initialization from x86_cpu_new. > > Suggested > > by Igor. > > 4. And other minor changes related to patch format. > > > > v4: > > > > https://lore.kernel.org/qemu-devel/158161767653.48948.10578064482878399556.st...@naples-babu.amd.com/ > > Changes from v3. > > 1. Moved the arch_id calculation inside the function x86_cpus_init. With > > this change, > > we dont need to change common numa code.(suggested by Igor) > > 2. Introduced the model specific handlers inside X86CPUDefinitions. > > These handlers are loaded into X86MachineState during the init. > > 3. Removed llc_id from x86CPU. > > 4. Removed init_apicid_fn hanlder from MachineClass. Kept all the code > > changes > > inside the x86. > > 5. Added new handler function apicid_pkg_offset for pkg_offset calculation. > > 6. And some Other minor changes. > > > > v3: > > > > https://lore.kernel.org/qemu-devel/157541968844.46157.17994918142533791313.st...@naples-babu.amd.com/ > > > > 1. Consolidated the topology information in structure X86CPUTopoInfo. > > 2. Changed the ccx_id to llc_id as commented by upstream. > > 3. Generalized the apic id decoding. It is mostly similar to current apic > > id > > except that it adds new field llc_id when numa configured. Removes all > > the > > hardcoded values. > > 4. Removed the earlier parse_numa split. And moved the numa node > > initialization > > inside the numa_complete_configuration. This is bit cleaner as > > commented by > > Eduardo. > > 5. Added new function init_apicid_fn inside machine_class structure. This > > will be used to update the apic id handler specific to cpu model. > > 6. Updated the cpuid unit tests. > > 7. TODO : Need to figure out how to dynamically update the handlers using > > cpu models. > > I might some guidance on that. > > > > v2: > > > > https://lore.kernel.org/qemu-devel/156779689013.21957.1631551572950676212.stgit@localhost.localdomain/ > > 1. Introduced the new property epyc to enable new epyc mode. > > 2. Separated the epyc mode and non epyc mode function. > > 3. Introduced function pointers in PCMachineState to handle the > > differences. > > 4. Mildly tested different combinations to make things are working as > > expected. > > 5. TODO : Setting the epyc feature bit needs to be worked out. This > > feature is > > supported only on AMD EPYC models. I may need some guidance on that. > > > > v1: > > > > https://lore.kernel.org/qemu-devel/20190731232032.51786-1-babu.mo...@amd.com/ > > --- > > > > Babu Moger (13): > > hw/i386: Introduce X86CPUTopoInfo to contain topology info > > hw/i386: Consolidate topology functions > > machine: Add SMP Sockets in CpuTopology > > hw/i386: Remove unnecessary initialization in x86_cpu_new > > hw/i386: Update structures to save the number of nodes per package > > hw/i386: Rename apicid_from_topo_ids to x86_apicid_from_topo_ids > > hw/386: Add EPYC mode topology decoding functions > > target/i386: Cleanup and use the EPYC mode topology functions > > hw/i386: Introduce apicid functions inside X86MachineState > > i386: Introduce use_epyc_apic_id_encoding in X86CPUDefinition > > hw/i386: Move arch_id decode inside x86_cpus_init > > target/i386: Enable new apic id encoding for EPYC based cpus models > > i386: Fix pkg_id offset for EPYC cpu models > > > > > > hw/core/machine.c | 1 > > hw/i386/pc.c | 15 ++- > > hw/i386/x86.c | 73 ++++++++++++---- > > include/hw/boards.h | 2 > > include/hw/i386/topology.h | 195 > > ++++++++++++++++++++++++++++++------------ > > include/hw/i386/x86.h | 12 +++ > > softmmu/vl.c | 1 > > target/i386/cpu.c | 203 > > ++++++++++++++------------------------------ > > target/i386/cpu.h | 3 + > > tests/test-x86-cpuid.c | 116 +++++++++++++++---------- > > 10 files changed, 358 insertions(+), 263 deletions(-) > > > > -- > > Signature > > > -- Eduardo