Re: [libvirt] [PATCH 0/5] introduce support for CPU dies in host/guest topology
A few nits and minor comments but LGTM. All patches: Reviewed-by: Daniel Henrique Barboza On 1/7/20 11:50 AM, Daniel P. Berrangé wrote: Ping On Thu, Dec 19, 2019 at 12:42:03PM +, Daniel P. Berrangé wrote: Latest generation CPUs (CascadeLake-AP) support a new topology level known as a 'die', sitting between a socket and a core. QEMU supports this with -smp arg since 4.1.0 Linux can report this via /sys/devices/system/cpu/cpuNNN/topology via 'die_id' and 'die_cpus' and 'die_cpus_list' files since 5.2 This series adds support for in guest XML to have a new 'dies' parameter, passed to QEMU, which defaults to '1' if omitted. It extends host capabilities so that NUMA topology reports a new 'die_id' attribute We can't expand 'virNodeInfoPtr' struct to have a die field, so this will remain forever reporting 'cores' as being 'dies * cores'. The in host capabilities XML is an interesting question. If we are strict with our API semantics we would *not* add a 'dies' parameter with any value other than '1' to in the host capabilities. If we reported a value other than 1, then any existing apps which multiple sockets*cores*threads will get the wrong total CPU count. We already know is broken by design for asymetric hardware, so we could simply document that it will forever be broken wrt to CPU dies too. In this case we might be better to not even report 'dies=1', just leave out the attr entirely. Interestingly though, is already more broken than it should be. For a VM with -smp 12,sockets=2,dies=3,cores=2,threads=1 it is reporting . It should at least do . I suspect the presence of dies is confusing the really incredibly horrible logic in virHostCPUGetInfoLinux. This will also impact virNodeInfoPtr data. So even if we don't report dies, I think we still have a bug that needs fixing here for the coarse node topology. I'm also confused about what I see with EPYC. IIUC, it was supposed to use the 'dies' concept, but in machines I've tested, Linux never reports die count other than 1. Perhaps only certain EPYC CPU models or generations use 'dies', or perhaps Linux isn't reporting correctly for EPYC, or perhaps I'm mislead into believeing it uses dies. Anyway, the upshot is I've not found any real hardware to test this series on. I've tested it only inside a QEMU guest with the suitable -smp arg to fake dies. Daniel P. Berrangé (5): conf: add support for specifying CPU "dies" parameter conf: remove unused virCapabilitiesSetHostCPU method qemu: add support for specifying CPU "dies" topology parameter hostcpu: add support for reporting die_id in NUMA topology tests: add host CPU data files for validating die_id docs/formatcaps.html.in | 2 +- docs/formatdomain.html.in | 14 +- docs/schemas/capability.rng | 3 + docs/schemas/cputypes.rng | 5 + src/bhyve/bhyve_command.c | 5 + src/conf/capabilities.c | 26 +- src/conf/capabilities.h | 7 +- src/conf/cpu_conf.c | 18 +- src/conf/cpu_conf.h | 1 + src/conf/domain_conf.c| 3 +- src/cpu/cpu.c | 1 + src/libvirt_linux.syms| 1 + src/libvirt_private.syms | 1 - src/libxl/libxl_capabilities.c| 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 12 +- src/util/virhostcpu.c | 16 + src/util/virhostcpu.h | 1 + src/vmx/vmx.c | 7 + .../x86_64-host+guest,model486-result.xml | 2 +- .../x86_64-host+guest,models-result.xml | 2 +- .../cputestdata/x86_64-host+guest-result.xml | 2 +- tests/cputestdata/x86_64-host+guest.xml | 2 +- .../x86_64-host+host-model-nofallback.xml | 2 +- ...t-Haswell-noTSX+Haswell,haswell-result.xml | 2 +- ...ell-noTSX+Haswell-noTSX,haswell-result.xml | 2 +- ...ost-Haswell-noTSX+Haswell-noTSX-result.xml | 2 +- .../x86_64-host-worse+guest-result.xml| 2 +- .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../ppc64-modern-bulk-result-conf.xml | 2 +- .../ppc64-modern-bulk-result-live.xml | 2 +- .../ppc64-modern-individual-result-conf.xml | 2 +- .../ppc64-modern-individual-result-live.xml | 2 +- .../x86-modern-bulk-result-conf.xml | 2 +- .../x86-modern-bulk-result-live.xml | 2 +-
Re: [libvirt] [PATCH 0/5] introduce support for CPU dies in host/guest topology
Ping On Thu, Dec 19, 2019 at 12:42:03PM +, Daniel P. Berrangé wrote: > Latest generation CPUs (CascadeLake-AP) support a new topology level > known as a 'die', sitting between a socket and a core. > > QEMU supports this with -smp arg since 4.1.0 > > Linux can report this via /sys/devices/system/cpu/cpuNNN/topology > via 'die_id' and 'die_cpus' and 'die_cpus_list' files since 5.2 > > This series adds support for in guest XML to have a new > 'dies' parameter, passed to QEMU, which defaults to '1' if omitted. > > It extends host capabilities so that NUMA topology reports a new > 'die_id' attribute > > We can't expand 'virNodeInfoPtr' struct to have a die field, so > this will remain forever reporting 'cores' as being 'dies * cores'. > > The in host capabilities XML is an interesting question. > > If we are strict with our API semantics we would *not* add a 'dies' > parameter with any value other than '1' to in the host > capabilities. If we reported a value other than 1, then any existing > apps which multiple sockets*cores*threads will get the wrong total > CPU count. > > We already know is broken by design for asymetric > hardware, so we could simply document that it will forever be broken > wrt to CPU dies too. In this case we might be better to not even > report 'dies=1', just leave out the attr entirely. > > Interestingly though, is already more broken than it > should be. For a VM with -smp 12,sockets=2,dies=3,cores=2,threads=1 > it is reporting . > It should at least do . > > I suspect the presence of dies is confusing the really incredibly > horrible logic in virHostCPUGetInfoLinux. This will also impact > virNodeInfoPtr data. > > So even if we don't report dies, I think we still have a bug that > needs fixing here for the coarse node topology. > > I'm also confused about what I see with EPYC. IIUC, it was supposed > to use the 'dies' concept, but in machines I've tested, Linux never > reports die count other than 1. Perhaps only certain EPYC CPU > models or generations use 'dies', or perhaps Linux isn't reporting > correctly for EPYC, or perhaps I'm mislead into believeing it uses > dies. > > Anyway, the upshot is I've not found any real hardware to test this > series on. I've tested it only inside a QEMU guest with the suitable > -smp arg to fake dies. > > Daniel P. Berrangé (5): > conf: add support for specifying CPU "dies" parameter > conf: remove unused virCapabilitiesSetHostCPU method > qemu: add support for specifying CPU "dies" topology parameter > hostcpu: add support for reporting die_id in NUMA topology > tests: add host CPU data files for validating die_id > > docs/formatcaps.html.in | 2 +- > docs/formatdomain.html.in | 14 +- > docs/schemas/capability.rng | 3 + > docs/schemas/cputypes.rng | 5 + > src/bhyve/bhyve_command.c | 5 + > src/conf/capabilities.c | 26 +- > src/conf/capabilities.h | 7 +- > src/conf/cpu_conf.c | 18 +- > src/conf/cpu_conf.h | 1 + > src/conf/domain_conf.c| 3 +- > src/cpu/cpu.c | 1 + > src/libvirt_linux.syms| 1 + > src/libvirt_private.syms | 1 - > src/libxl/libxl_capabilities.c| 1 + > src/qemu/qemu_capabilities.c | 2 + > src/qemu/qemu_capabilities.h | 1 + > src/qemu/qemu_command.c | 12 +- > src/util/virhostcpu.c | 16 + > src/util/virhostcpu.h | 1 + > src/vmx/vmx.c | 7 + > .../x86_64-host+guest,model486-result.xml | 2 +- > .../x86_64-host+guest,models-result.xml | 2 +- > .../cputestdata/x86_64-host+guest-result.xml | 2 +- > tests/cputestdata/x86_64-host+guest.xml | 2 +- > .../x86_64-host+host-model-nofallback.xml | 2 +- > ...t-Haswell-noTSX+Haswell,haswell-result.xml | 2 +- > ...ell-noTSX+Haswell-noTSX,haswell-result.xml | 2 +- > ...ost-Haswell-noTSX+Haswell-noTSX-result.xml | 2 +- > .../x86_64-host-worse+guest-result.xml| 2 +- > .../caps_4.1.0.x86_64.xml | 1 + > .../caps_4.2.0.aarch64.xml| 1 + > .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + > .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + > .../caps_4.2.0.x86_64.xml | 1 + > .../ppc64-modern-bulk-result-conf.xml | 2 +- > .../ppc64-modern-bulk-result-live.xml | 2 +- > .../ppc64-modern-individual-result-conf.xml | 2 +- > .../ppc64-modern-individual-result-live.xml | 2 +- > .../x86-modern-bulk-result-conf.xml | 2 +- > .../x86-modern-bulk-result-live.xml | 2 +- >
[libvirt] [PATCH 0/5] introduce support for CPU dies in host/guest topology
Latest generation CPUs (CascadeLake-AP) support a new topology level known as a 'die', sitting between a socket and a core. QEMU supports this with -smp arg since 4.1.0 Linux can report this via /sys/devices/system/cpu/cpuNNN/topology via 'die_id' and 'die_cpus' and 'die_cpus_list' files since 5.2 This series adds support for in guest XML to have a new 'dies' parameter, passed to QEMU, which defaults to '1' if omitted. It extends host capabilities so that NUMA topology reports a new 'die_id' attribute We can't expand 'virNodeInfoPtr' struct to have a die field, so this will remain forever reporting 'cores' as being 'dies * cores'. The in host capabilities XML is an interesting question. If we are strict with our API semantics we would *not* add a 'dies' parameter with any value other than '1' to in the host capabilities. If we reported a value other than 1, then any existing apps which multiple sockets*cores*threads will get the wrong total CPU count. We already know is broken by design for asymetric hardware, so we could simply document that it will forever be broken wrt to CPU dies too. In this case we might be better to not even report 'dies=1', just leave out the attr entirely. Interestingly though, is already more broken than it should be. For a VM with -smp 12,sockets=2,dies=3,cores=2,threads=1 it is reporting . It should at least do . I suspect the presence of dies is confusing the really incredibly horrible logic in virHostCPUGetInfoLinux. This will also impact virNodeInfoPtr data. So even if we don't report dies, I think we still have a bug that needs fixing here for the coarse node topology. I'm also confused about what I see with EPYC. IIUC, it was supposed to use the 'dies' concept, but in machines I've tested, Linux never reports die count other than 1. Perhaps only certain EPYC CPU models or generations use 'dies', or perhaps Linux isn't reporting correctly for EPYC, or perhaps I'm mislead into believeing it uses dies. Anyway, the upshot is I've not found any real hardware to test this series on. I've tested it only inside a QEMU guest with the suitable -smp arg to fake dies. Daniel P. Berrangé (5): conf: add support for specifying CPU "dies" parameter conf: remove unused virCapabilitiesSetHostCPU method qemu: add support for specifying CPU "dies" topology parameter hostcpu: add support for reporting die_id in NUMA topology tests: add host CPU data files for validating die_id docs/formatcaps.html.in | 2 +- docs/formatdomain.html.in | 14 +- docs/schemas/capability.rng | 3 + docs/schemas/cputypes.rng | 5 + src/bhyve/bhyve_command.c | 5 + src/conf/capabilities.c | 26 +- src/conf/capabilities.h | 7 +- src/conf/cpu_conf.c | 18 +- src/conf/cpu_conf.h | 1 + src/conf/domain_conf.c| 3 +- src/cpu/cpu.c | 1 + src/libvirt_linux.syms| 1 + src/libvirt_private.syms | 1 - src/libxl/libxl_capabilities.c| 1 + src/qemu/qemu_capabilities.c | 2 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 12 +- src/util/virhostcpu.c | 16 + src/util/virhostcpu.h | 1 + src/vmx/vmx.c | 7 + .../x86_64-host+guest,model486-result.xml | 2 +- .../x86_64-host+guest,models-result.xml | 2 +- .../cputestdata/x86_64-host+guest-result.xml | 2 +- tests/cputestdata/x86_64-host+guest.xml | 2 +- .../x86_64-host+host-model-nofallback.xml | 2 +- ...t-Haswell-noTSX+Haswell,haswell-result.xml | 2 +- ...ell-noTSX+Haswell-noTSX,haswell-result.xml | 2 +- ...ost-Haswell-noTSX+Haswell-noTSX-result.xml | 2 +- .../x86_64-host-worse+guest-result.xml| 2 +- .../caps_4.1.0.x86_64.xml | 1 + .../caps_4.2.0.aarch64.xml| 1 + .../qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 1 + .../qemucapabilitiesdata/caps_4.2.0.s390x.xml | 1 + .../caps_4.2.0.x86_64.xml | 1 + .../ppc64-modern-bulk-result-conf.xml | 2 +- .../ppc64-modern-bulk-result-live.xml | 2 +- .../ppc64-modern-individual-result-conf.xml | 2 +- .../ppc64-modern-individual-result-live.xml | 2 +- .../x86-modern-bulk-result-conf.xml | 2 +- .../x86-modern-bulk-result-live.xml | 2 +- .../x86-modern-individual-add-result-conf.xml | 2 +- .../x86-modern-individual-add-result-live.xml | 2 +- .../x86-old-bulk-result-conf.xml | 2 +- .../x86-old-bulk-result-live.xml | 2 +- .../cpu-hotplug-granularity.xml | 2 +-
