On Wed, 5 Jul 2023 10:12:40 +0200 Claudio Fontana <cfont...@suse.de> wrote:
> Hi all, partially resurrecting an old thread. > > I've seen how for Epyc something special was done in the past in terms of > apicid assignments based on topology, which was then reverted apparently, > but I wonder if something more general would be useful to all? > > The QEMU apicid assignments first of all do not seem to match what is > happening on real hardware. QEMU typically does generate valid APIC IDs it however doesn't do a good job when using odd number of cores and/or NUMA enabled cases. (That is what Babu have attempted to fix, but eventually that have been dropped for reasons described in quoted cover letter) > Functionally things are ok, but then when trying to investigate issues, > specifically in the guest kernel KVM PV code (arch/x86/kernel/kvm.c), > in some cases the actual apicid values in relationship to the topology do > matter, Care to point out specific places you are referring to? KVM is not the only place where it might matter, it affects topo/numa code on guest side as well. > and currently there is no way (I know of), of supplying our own apicid > assignment, more closely matching what happens on hardware. > > This has been an issue when debugging guest images in the cloud, where being > able to reproduce issues locally would be very beneficial as opposed to using > cloud images as the feedback loop, > but unfortunately QEMU cannot currently create the right apicid values to > associate to the cpus. Indeed EPYC APIC encoding mess increases support cases load downstream, but as long as one has access to similar host hw, one should be able to reproduce the issue locally. However I would expect end result on such support end with an advice to change topo/use another CPU model. (what we lack is a documentation what works and what doesn't, perhaps writing guidelines would be sufficient to steer users to the usable EPYC configurations) > Do I understand the issue correctly, comments, ideas? > How receptive the project would be for changes aimed at providing a custom > assignment of apicids to cpus, regardless of Intel or AMD? It's not that simple to just set custom APIC ID in register and be done with it, you'll likely break (from the top of my head: some CPUID leaves might depend on it, ACPI tables, NUMA mapping, KVM's vcpu_id). Current topo code aims to work on information based on '-smp'/'-numa', all through out QEMU codebase. If however we were let user set APIC ID (which is somehow correct), we would need to take reverse steps to decode that (in vendor specific way) and incorporate resulting topo into other code that uses topology info. That makes it quite messy, not to mention it's x86(AMD specific) and doesn't fit well with generalizing topo handling. So I don't really like this route. (x86 cpus have apic_id property, so theoretically you can set it and with some minimal hacking lunch a guest, but then expect guest to be unhappy when ACPI ID goes out of sync with everything else. I would do that only for the sake of an experiment and wouldn't try to upstream that) What I wouldn't mind is taking the 2nd stab at what Babu had tried do. Provided it manages to encode APIC ID for EPYC correctly and won't complicate code much (and still using -smp/-numa as the root source for topo configuration). > Thanks, > > Claudio > > > > On 9/1/20 17:57, Babu Moger wrote: > > To support some of the complex topology, we introduced EPYC mode apicid > > decode. > > But, EPYC mode decode is running into problems. Also it can become quite a > > maintenance problem in the future. So, it was decided to remove that code > > and > > use the generic decode which works for majority of the topology. Most of the > > SPECed configuration would work just fine. With some non-SPECed user inputs, > > it will create some sub-optimal configuration. > > > > Here is the discussion thread. > > https://lore.kernel.org/qemu-devel/c0bcc1a6-1d84-a6e7-e468-d5b437c1b...@amd.com/ > > https://lore.kernel.org/qemu-devel/20200826143849.59f69...@redhat.com/ > > > > This series removes all the EPYC mode specific apicid changes and use the > > generic > > apicid decode. > > --- > > v7: > > Eduardo has already queued 1-8 from the v6. Sending rest of the patches. > > Fixed CPUID 800000ld based on Igor's comment and few text changes. > > > > v6: > > > > https://lore.kernel.org/qemu-devel/159889924378.21294.16494070903874534542.st...@naples-babu.amd.com/ > > Found out that numa configuration is not mandatory for all the EPYC model > > topology. > > We can use the generic decode which works pretty well. Also noticed that > > cpuid does not changes when the numa nodes change(NPS- Nodes per socket). > > Took care of couple comments from Igor and Eduardo. > > Thank you Igor, Daniel, David, Eduardo for your feedback. > > > > v5: > > > > https://lore.kernel.org/qemu-devel/159804762216.39954.15502128500494116468.st...@naples-babu.amd.com/ > > Revert EPYC specific decode. > > Simplify CPUID_8000_001E > > > > v4: > > > > https://lore.kernel.org/qemu-devel/159744083536.39197.13827776633866601278.st...@naples-babu.amd.com/ > > Not much of a change. Just added few text changes. > > Error out configuration instead of warning if dies are not configured in > > EPYC. > > Few other text changes to clarify the removal of node_id, nr_nodes and > > nodes_per_pkg. > > > > v3: > > > > https://lore.kernel.org/qemu-devel/159681772267.9679.1334429994189974662.st...@naples-babu.amd.com/#r > > Added a new check to pass the dies for EPYC numa configuration. > > Added Simplify CPUID_8000_001E patch with some changes suggested by Igor. > > Dropped the patch to build the topology from CpuInstanceProperties. > > TODO: Not sure if we still need the Autonuma changes Igor mentioned. > > Needs more clarity on that. > > > > v2: > > > > https://lore.kernel.org/qemu-devel/159362436285.36204.986406297373871949.st...@naples-babu.amd.com/ > > Used the numa information from CpuInstanceProperties for building > > the apic_id suggested by Igor. > > Also did some minor code re-aarangement to take care of changes. > > Dropped the patch "Simplify CPUID_8000_001E" from v1. Will send > > it later. > > > > v1: > > > > https://lore.kernel.org/qemu-devel/159164739269.20543.3074052993891532749.st...@naples-babu.amd.com > > > > Babu Moger (2): > > i386: Simplify CPUID_8000_001d for AMD > > i386: Simplify CPUID_8000_001E for AMD > > > > > > target/i386/cpu.c | 226 > > ++++++++++++++--------------------------------------- > > 1 file changed, 61 insertions(+), 165 deletions(-) > > > > -- > > >
