aarch64:fdt: correct vcpu topology related value #5893
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rbradford
reviewed
Oct 31, 2023
arch/src/aarch64/fdt.rs
Outdated
| let threads_per_core = vcpu_topology.unwrap_or_default().0; | ||
| let cores_per_package = vcpu_topology.unwrap_or_default().1; | ||
| let packages = vcpu_topology.unwrap_or_default().2; | ||
| let (threads_per_core, cores_per_package, packages) = if vcpu_topology.is_none() { |
There was a problem hiding this comment.
This is better: let vcpu_topology = vcpu_topology.unwrap_or((1,1,1));
A better commit message would be "aarch64: fdt: Use more appropriate default value for topology" |
|
Thanks @rbradford -, Fixed. |
arch/src/aarch64/fdt.rs
Outdated
Comment on lines
294
to
299
| if threads_per_core == 0 || cores_per_package == 0 || packages == 0 { | ||
| error!( | ||
| "Invalid vcpu topology: threads_per_core: {}, cores_per_package: {}, packages: {}", | ||
| threads_per_core, cores_per_package, packages | ||
| ); | ||
| } |
There was a problem hiding this comment.
The zeros check is not needed, because it's already done here: https://github.com/cloud-hypervisor/cloud-hypervisor/blob/main/vmm/src/config.rs#L2004C23-L2004C23
If vcpu_topology is not none, you should be free from the zero-value risk.
There was a problem hiding this comment.
Thanks @michael2012z -, fixed.
Now, default values for vcpu topology are 0s, that is not correct and may lead to bug. Fix it by setting default value to 1s. Also add check in case one or more of these values are zero. Fixes: cloud-hypervisor#5892 Signed-off-by: Jianyong Wu <jianyong.wu@arm.com>
michael2012z
approved these changes
Nov 1, 2023
likebreath
added
bug
russell-islam
pushed a commit
to microsoft/cloud-hypervisor
that referenced
this pull request
Apr 12, 2024
v36.0 This release has been tracked in our [roadmap project](https://github.com/orgs/cloud-hypervisor/projects/6) as iteration v36.0. The following user visible changes have been made: Command Line Changes -------------------- We switched back to use the `clap` crate to create our command line, since the `argh` crate is barely maintained. There were several syntax changes: * All `--option value` commands now are `--option=value`. * The `--disk DISK1 --disk DISK2` command now is `--disk DISK1 DISK2`. * The `-v -v -v`command now is `-vvv`. Note: the released binary size increased around 0.3M due to this change. Enabled Features Reported via API Endpoint and CLI -------------------------------------------------- Now the enabled (Cargo) features of the running Cloud Hypervisor instance can be queried via API endpoint (`/vmm.ping`) and CLI (`--version -v`). NUMA Support for PCI segments ----------------------------- The `--numa` command is augmented with a new option `pci_segment=`, so that users can define the relationship between PCI segments and NUMA nodes. Examples can be found from the [memory documentation](docs/memory.md) CPU Topology Support on AMD Platforms ------------------------------------- Now the CPU topology on x86_64 platforms supports multiple vendors. Unix Socket Backend for Serial Port ----------------------------------- The `--serial` command is augmented with a new option `socket=`, allowing users to access the serial port using a Unix socket. AIO Backend for Block Devices ----------------------------- An AIO backend is added for `virtio-block` devices to improve block device performance when the `io_uring` feature is not supported by the host Operating System. Documentation Improvements -------------------------- * New [documentation](docs/coverage.md) for collecting coverage data * Various typo fixes Notable Bug Fixes ----------------- * Fix a deadlock when TDX is enabled (cloud-hypervisor#5845) * Only advertise AMX feature bits to guest when the AMX cpu feature is enabled (cloud-hypervisor#5834) * Correct default value for vCPU topology on AArch64 (cloud-hypervisor#5893) Contributors ------------ Many thanks to everyone who has contributed to our release: * Anatol Belski <anbelski@linux.microsoft.com> * Bo Chen <chen.bo@intel.com> * Dario Nieuwenhuis <dirbaio@dirbaio.net> * Jianyong Wu <jianyong.wu@arm.com> * Jinank Jain <jinankjain@microsoft.com> * Muminul Islam <muislam@microsoft.com> * Praveen K Paladugu <prapal@linux.microsoft.com> * Ravi kumar Veeramally <ravikumar.veeramally@intel.com> * Rob Bradford <rbradford@rivosinc.com> * Thomas Barrett <tbarrett@crusoeenergy.com> * Wei Liu <liuwe@microsoft.com> * Yi Wang <foxywang@tencent.com> * dom.song <dom.song@amperecomputing.com> * tag 'v36.0': (432 commits) build: Release v36.0 main: Add the '--serial socket=' option help information build: Bump zerocopy from 0.7.8 to 0.7.21 build: Bump serde_json from 1.0.107 to 1.0.108 in /fuzz Jenkinsfile: Skip 'test_vfio' and 'test_vfio_user' on AMD workers scripts: Simplify the script for running bare-metal VFIO tests tests: Remove "test_vfio" from the bare-metal worker aarch64: fdt: Use more appropriate default value for topology build: Bump toml_edit from 0.19.8 to 0.19.15 build: Bump MSRV to 1.66 build: Bump winapi-util from 0.1.5 to 0.1.6 scripts: Clear the toolchain environment variables for virtiofsd build: Bump futures-core from 0.3.28 to 0.3.29 in /fuzz virtio-devices, vmm: Update seccomp list build: Bump seccompiler from 0.3.0 to 0.4.0 build: Bump zerocopy from 0.7.11 to 0.7.20 in /fuzz block: add copyright text to raw_async_aio.rs hypervisor: Add support for MMIO write emulation hypervisor: Add support MMIO read VMGEXIT hypervisor: Add support for legacy I/O port emulation ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Once vcpu_topology is None, values, like cores_per_package, will be set to zero.
Correct it by setting them to 1s when vcpu_topology is none.
Also add check in case one or more of these values are zero.
Fixes: #5892