- 18 Apr, 2019 13 commits
-
-
Dave Martin authored
The current error code documentation for KVM_GET_ONE_REG and KVM_SET_ONE_REG could be read as implying that all architectures implement these error codes, or that KVM guarantees which error code is returned in a particular situation. Because this is not really the case, this patch waters down the documentation explicitly to remove such guarantees. EPERM is marked as arm64-specific, since for now arm64 really is the only architecture that yields this error code for the finalization-required case. Keeping this as a distinct error code is useful however for debugging due to the statefulness of the API in this instance. No functional change. Suggested-by:
Andrew Jones <drjones@redhat.com> Fixes: 395f562f ("KVM: Document errors for KVM_GET_ONE_REG and KVM_SET_ONE_REG") Fixes: 50036ad0 ("KVM: arm64/sve: Document KVM API extensions for SVE") Signed-off-by:
Dave Martin <Dave.Martin@arm.com> Reviewed-by:
Marc Zyngier <marc.zyngier@arm.com>
-
Dave Martin authored
Userspace is only supposed to use KVM_ARM_VCPU_FINALIZE when there is some vcpu feature that can actually be finalized. This means that documenting KVM_ARM_VCPU_FINALIZE as available or not depending on the capabilities present is not helpful. This patch amends the documentation to describe availability in terms of which capability is required for each finalizable feature instead. In any case, userspace sees the same error (EINVAL) regardless of whether the given feature is not present or KVM_ARM_VCPU_FINALIZE is not implemented at all. No functional change. Suggested-by:
-
Dave Martin authored
Currently, the internal vcpu finalization functions use a different name ("what") for the feature parameter than the name ("feature") used in the documentation. To avoid future confusion, this patch converts everything to use the name "feature" consistently. No functional change. Suggested-by: -
Dave Martin authored
Correct virtualization of SVE relies for correctness on code in set_sve_vls() that verifies consistency between the set of vector lengths requested by userspace and the set of vector lengths available on the host. However, the purpose of this code is not obvious, and not likely to be apparent at all to people who do not have detailed knowledge of the SVE system-level architecture. This patch adds a suitable comment to explain what these checks are for. No functional change. Suggested-by:
-
Dave Martin authored
A complicated DIV_ROUND_UP() expression is currently written out explicitly in multiple places in order to specify the size of the bitmap exchanged with userspace to represent the value of the KVM_REG_ARM64_SVE_VLS pseudo-register. Userspace currently has no direct way to work this out either: for documentation purposes, the size is just quoted as 8 u64s. To make this more intuitive, this patch replaces these with a single define, which is also exported to userspace as KVM_ARM64_SVE_VLS_WORDS. Since the number of words in a bitmap is just the index of the last word used + 1, this patch expresses the bound that way instead. This should make it clearer what is being expressed. For userspace convenience, the minimum and maximum possible vector lengths relevant to the KVM ABI are exposed to UAPI as KVM_ARM64_SVE_VQ_MIN, KVM_ARM64_SVE_VQ_MAX. Since the only direct use for these at present is manipulation of KVM_REG_ARM64_SVE_VLS, no corresponding _VL_ macros are defined. They could be added later if a need arises. Since use of DIV_ROUND_UP() was the only reason for including <linux/kernel.h> in guest.c, this patch also removes that #include. Suggested-by:
-
Dave Martin authored
sve_reg_to_region() currently passes the result of vcpu_sve_state_size() to array_index_nospec(), effectively leading to a divide / modulo operation. Currently the code bails out and returns -EINVAL if vcpu_sve_state_size() turns out to be zero, in order to avoid going ahead and attempting to divide by zero. This is reasonable, but it should only happen if the kernel contains some other bug that allowed this code to be reached without the vcpu having been properly initialised. To make it clear that this is a defence against bugs rather than something that the user should be able to trigger, this patch marks the check with WARN_ON(). Suggested-by:
-
Dave Martin authored
Currently, the way error codes are generated when processing the SVE register access ioctls in a bit haphazard. This patch refactors the code so that the behaviour is more consistent: now, -EINVAL should be returned only for unrecognised register IDs or when some other runtime error occurs. -ENOENT is returned for register IDs that are recognised, but whose corresponding register (or slice) does not exist for the vcpu. To this end, in {get,set}_sve_reg() we now delegate the vcpu_has_sve() check down into {get,set}_sve_vls() and sve_reg_to_region(). The KVM_REG_ARM64_SVE_VLS special case is picked off first, then sve_reg_to_region() plays the role of exhaustively validating or rejecting the register ID and (where accepted) computing the applicable register region as before. sve_reg_to_region() is rearranged so that -ENOENT or -EPERM is not returned prematurely, before checking whether reg->id is in a recognised range. -EPERM is now only returned when an attempt is made to access an actually existing register slice on an unfinalized vcpu. Fixes: e1c9c983 ("KVM: arm64/sve: Add SVE support to register access ioctl interface") Fixes: 9033bba4 ("KVM: arm64/sve: Add pseudo-register for the guest's vector lengths") Suggested-by: -
Dave Martin authored
* Remove a few redundant blank lines that are stylistically inconsistent with code already in guest.c and are just taking up space. * Delete a couple of pointless empty default cases from switch statements whose behaviour is otherwise obvious anyway. * Fix some typos and consolidate some redundantly duplicated comments. * Respell the slice index check in sve_reg_to_region() as "> 0" to be more consistent with what is logically being checked here (i.e., "is the slice index too large"), even though we don't try to cope with multiple slices yet. No functional change. Suggested-by:
-
Dave Martin authored
Currently, the SVE register ID macros are not all defined in the same way, and advertise the fact that FFR maps onto the nonexistent predicate register P16. This is really just for kernel convenience, and may lead userspace into bad habits. Instead, this patch masks the ID macro arguments so that architecturally invalid register numbers will not be passed through any more, and uses a literal KVM_REG_ARM64_SVE_FFR_BASE macro to define KVM_REG_ARM64_SVE_FFR(), similarly to the way the _ZREG() and _PREG() macros are defined. Rather than plugging in magic numbers for the number of Z- and P- registers and the maximum possible number of register slices, this patch provides definitions for those too. Userspace is going to need them in any case, and it makes sense for them to come from <uapi/asm/kvm.h>. sve_reg_to_region() uses convenience constants that are defined in a different way, and also makes use of the fact that the FFR IDs are really contiguous with the P15 IDs, so this patch retains the existing convenience constants in guest.c, supplemented with a couple of sanity checks to check for consistency with the UAPI header. Fixes: e1c9c983 ("KVM: arm64/sve: Add SVE support to register access ioctl interface") Suggested-by:
-
Dave Martin authored
Because of the logic in kvm_arm_sys_reg_{get,set}_reg() and sve_id_visibility(), we should never call {get,set}_id_aa64zfr0_el1() for a vcpu where !vcpu_has_sve(vcpu). To avoid the code giving the impression that it is valid for these functions to be called in this situation, and to help the compiler make the right optimisation decisions, this patch adds WARN_ON() for these cases. Given the way the logic is spread out, this seems preferable to dropping the checks altogether. Suggested-by: -
Dave Martin authored
The vcpu finalization stubs kvm_arm_vcpu_finalize() and kvm_arm_vcpu_is_finalized() are currently #defines for ARM, which limits the type-checking that the compiler can do at runtime. The only reason for them to be #defines was to avoid reliance on the definition of struct kvm_vcpu, which is not available here due to circular #include problems. However, because these are stubs containing no code, they don't need the definition of struct kvm_vcpu after all; only a declaration is needed (which is available already). So in the interests of cleanliness, this patch converts them to inline functions. No functional change. Suggested-by:
-
Dave Martin authored
The introduction of kvm_arm_init_arch_resources() looks like premature factoring, since nothing else uses this hook yet and it is not clear what will use it in the future. For now, let's not pretend that this is a general thing: This patch simply renames the function to kvm_arm_init_sve(), retaining the arm stub version under the new name. Suggested-by:
-
Dave Martin authored
Currently the meanings of sve_vq_map and the ancillary helpers __bit_to_vq() and __vq_to_bit() are not clearly explained. This patch makes the explanatory comment clearer, and removes the duplicate comment from fpsimd.h. The WARN_ON() currently present in __bit_to_vq() confuses the intended use of this helper. Since these are low-level helpers not intended for general-purpose use anyway, it is better not to make guesses about how these functions will be used: rather, this patch removes the WARN_ON() and relies on callers to use the helpers sensibly. Suggested-by:
-
- 02 Apr, 2019 1 commit
-
-
Marc Zyngier authored
The introduction of the SVE registers to userspace started with a refactoring of the way we expose any register via the ONE_REG interface. Unfortunately, this change doesn't exactly behave as expected if the number of registers is non-zero and consider everything to be an error. The visible result is that QEMU barfs very early when creating vcpus. Make sure we only exit early in case there is an actual error, rather than a positive number of registers... Fixes: be25bbb3 ("KVM: arm64: Factor out core register ID enumeration") Signed-off-by:
-
- 29 Mar, 2019 26 commits
-
-
Dave Martin authored
This patch adds sections to the KVM API documentation describing the extensions for supporting the Scalable Vector Extension (SVE) in guests. Signed-off-by:
-
Dave Martin authored
KVM_GET_ONE_REG and KVM_SET_ONE_REG return some error codes that are not documented (but hopefully not surprising either). To give an indication of what these may mean, this patch adds brief documentation. Signed-off-by:
-
Dave Martin authored
To provide a uniform way to check for KVM SVE support amongst other features, this patch adds a suitable capability KVM_CAP_ARM_SVE, and reports it as present when SVE is available. Signed-off-by:
Julien Thierry <julien.thierry@arm.com> Tested-by:
zhang.lei <zhang.lei@jp.fujitsu.com> Signed-off-by:
-
Dave Martin authored
Now that all the pieces are in place, this patch offers a new flag KVM_ARM_VCPU_SVE that userspace can pass to KVM_ARM_VCPU_INIT to turn on SVE for the guest, on a per-vcpu basis. As part of this, support for initialisation and reset of the SVE vector length set and registers is added in the appropriate places, as well as finally setting the KVM_ARM64_GUEST_HAS_SVE vcpu flag, to turn on the SVE support code. Allocation of the SVE register storage in vcpu->arch.sve_state is deferred until the SVE configuration is finalized, by which time the size of the registers is known. Setting the vector lengths supported by the vcpu is considered configuration of the emulated hardware rather than runtime configuration, so no support is offered for changing the vector lengths available to an existing vcpu across reset. Signed-off-by:
-
Dave Martin authored
This patch adds a new pseudo-register KVM_REG_ARM64_SVE_VLS to allow userspace to set and query the set of vector lengths visible to the guest. In the future, multiple register slices per SVE register may be visible through the ioctl interface. Once the set of slices has been determined we would not be able to allow the vector length set to be changed any more, in order to avoid userspace seeing inconsistent sets of registers. For this reason, this patch adds support for explicit finalization of the SVE configuration via the KVM_ARM_VCPU_FINALIZE ioctl. Finalization is the proper place to allocate the SVE register state storage in vcpu->arch.sve_state, so this patch adds that as appropriate. The data is freed via kvm_arch_vcpu_uninit(), which was previously a no-op on arm64. To simplify the logic for determining what vector lengths can be supported, some code is added to KVM init to work this out, in the kvm_arm_init_arch_resources() hook. The KVM_REG_ARM64_SVE_VLS pseudo-register is not exposed yet. Subsequent patches will allow SVE to be turned on for guest vcpus, making it visible. Signed-off-by:
-
Dave Martin authored
Some aspects of vcpu configuration may be too complex to be completed inside KVM_ARM_VCPU_INIT. Thus, there may be a requirement for userspace to do some additional configuration before various other ioctls will work in a consistent way. In particular this will be the case for SVE, where userspace will need to negotiate the set of vector lengths to be made available to the guest before the vcpu becomes fully usable. In order to provide an explicit way for userspace to confirm that it has finished setting up a particular vcpu feature, this patch adds a new ioctl KVM_ARM_VCPU_FINALIZE. When userspace has opted into a feature that requires finalization, typically by means of a feature flag passed to KVM_ARM_VCPU_INIT, a matching call to KVM_ARM_VCPU_FINALIZE is now required before KVM_RUN or KVM_GET_REG_LIST is allowed. Individual features may impose additional restrictions where appropriate. No existing vcpu features are affected by this, so current userspace implementations will continue to work exactly as before, with no need to issue KVM_ARM_VCPU_FINALIZE. As implemented in this patch, KVM_ARM_VCPU_FINALIZE is currently a placeholder: no finalizable features exist yet, so ioctl is not required and will always yield EINVAL. Subsequent patches will add the finalization logic to make use of this ioctl for SVE. No functional change for existing userspace. Signed-off-by:
-
Dave Martin authored
This patch adds a kvm_arm_init_arch_resources() hook to perform subarch-specific initialisation when starting up KVM. This will be used in a subsequent patch for global SVE-related setup on arm64. No functional change. Signed-off-by:
-
Dave Martin authored
KVM will need to interrogate the set of SVE vector lengths available on the system. This patch exposes the relevant bits to the kernel, along with a sve_vq_available() helper to check whether a particular vector length is supported. __vq_to_bit() and __bit_to_vq() are not intended for use outside these functions: now that these are exposed outside fpsimd.c, they are prefixed with __ in order to provide an extra hint that they are not intended for general-purpose use. Signed-off-by:
Alex Bennée <alex.bennee@linaro.org> Tested-by:
-
Dave Martin authored
This patch includes the SVE register IDs in the list returned by KVM_GET_REG_LIST, as appropriate. On a non-SVE-enabled vcpu, no new IDs are added. On an SVE-enabled vcpu, IDs for the FPSIMD V-registers are removed from the list, since userspace is required to access the Z- registers instead in order to access the V-register content. For the variably-sized SVE registers, the appropriate set of slice IDs are enumerated, depending on the maximum vector length for the vcpu. As it currently stands, the SVE architecture never requires more than one slice to exist per register, so this patch adds no explicit support for enumerating multiple slices. The code can be extended straightforwardly to support this in the future, if needed. Signed-off-by:
-
Dave Martin authored
This patch adds the following registers for access via the KVM_{GET,SET}_ONE_REG interface: * KVM_REG_ARM64_SVE_ZREG(n, i) (n = 0..31) (in 2048-bit slices) * KVM_REG_ARM64_SVE_PREG(n, i) (n = 0..15) (in 256-bit slices) * KVM_REG_ARM64_SVE_FFR(i) (in 256-bit slices) In order to adapt gracefully to future architectural extensions, the registers are logically divided up into slices as noted above: the i parameter denotes the slice index. This allows us to reserve space in the ABI for future expansion of these registers. However, as of today the architecture does not permit registers to be larger than a single slice, so no code is needed in the kernel to expose additional slices, for now. The code can be extended later as needed to expose them up to a maximum of 32 slices (as carved out in the architecture itself) if they really exist someday. The registers are only visible for vcpus that have SVE enabled. They are not enumerated by KVM_GET_REG_LIST on vcpus that do not have SVE. Accesses to the FPSIMD registers via KVM_REG_ARM_CORE is not allowed for SVE-enabled vcpus: SVE-aware userspace can use the KVM_REG_ARM64_SVE_ZREG() interface instead to access the same register state. This avoids some complex and pointless emulation in the kernel to convert between the two views of these aliased registers. Signed-off-by: -
Dave Martin authored
In order to avoid the pointless complexity of maintaining two ioctl register access views of the same data, this patch blocks ioctl access to the FPSIMD V-registers on vcpus that support SVE. This will make it more straightforward to add SVE register access support. Since SVE is an opt-in feature for userspace, this will not affect existing users. Signed-off-by:
-
Dave Martin authored
In preparation for adding logic to filter out some KVM_REG_ARM_CORE registers from the KVM_GET_REG_LIST output, this patch factors out the core register enumeration into a separate function and rebuilds num_core_regs() on top of it. This may be a little more expensive (depending on how good a job the compiler does of specialising the code), but KVM_GET_REG_LIST is not a hot path. This will make it easier to consolidate ID filtering code in one place. No functional change. Signed-off-by:
-
Dave Martin authored
arch/arm64/kvm/guest.c uses the string functions, but the corresponding header is not included. We seem to get away with this for now, but for completeness this patch adds the #include, in preparation for adding yet more memset() calls. Signed-off-by:
-
Dave Martin authored
The Arm SVE architecture defines registers that are up to 2048 bits in size (with some possibility of further future expansion). In order to avoid the need for an excessively large number of ioctls when saving and restoring a vcpu's registers, this patch adds a #define to make support for individual 2048-bit registers through the KVM_{GET,SET}_ONE_REG ioctl interface official. This will allow each SVE register to be accessed in a single call. There are sufficient spare bits in the register id size field for this change, so there is no ABI impact, providing that KVM_GET_REG_LIST does not enumerate any 2048-bit register unless userspace explicitly opts in to the relevant architecture-specific features. Signed-off-by: -
Dave Martin authored
In order to give each vcpu its own view of the SVE registers, this patch adds context storage via a new sve_state pointer in struct vcpu_arch. An additional member sve_max_vl is also added for each vcpu, to determine the maximum vector length visible to the guest and thus the value to be configured in ZCR_EL2.LEN while the vcpu is active. This also determines the layout and size of the storage in sve_state, which is read and written by the same backend functions that are used for context-switching the SVE state for host tasks. On SVE-enabled vcpus, SVE access traps are now handled by switching in the vcpu's SVE context and disabling the trap before returning to the guest. On other vcpus, the trap is not handled and an exit back to the host occurs, where the handle_sve() fallback path reflects an undefined instruction exception back to the guest, consistently with the behaviour of non-SVE-capable hardware (as was done unconditionally prior to this patch). No SVE handling is added on non-VHE-only paths, since VHE is an architectural and Kconfig prerequisite of SVE. Signed-off-by:
-
Dave Martin authored
This patch adds the necessary support for context switching ZCR_EL1 for each vcpu. ZCR_EL1 is trapped alongside the FPSIMD/SVE registers, so it makes sense for it to be handled as part of the guest FPSIMD/SVE context for context switch purposes instead of handling it as a general system register. This means that it can be switched in lazily at the appropriate time. No effort is made to track host context for this register, since SVE requires VHE: thus the hosts's value for this register lives permanently in ZCR_EL2 and does not alias the guest's value at any time. The Hyp switch and fpsimd context handling code is extended appropriately. Accessors are added in sys_regs.c to expose the SVE system registers and ID register fields. Because these need to be conditionally visible based on the guest configuration, they are implemented separately for now rather than by use of the generic system register helpers. This may be abstracted better later on when/if there are more features requiring this model. ID_AA64ZFR0_EL1 is RO-RAZ for MRS/MSR when SVE is disabled for the guest, but for compatibility with non-SVE aware KVM implementations the register should not be enumerated at all for KVM_GET_REG_LIST in this case. For consistency we also reject ioctl access to the register. This ensures that a non-SVE-enabled guest looks the same to userspace, irrespective of whether the kernel KVM implementation supports SVE. Signed-off-by:
-
Dave Martin authored
Some optional features of the Arm architecture add new system registers that are not present in the base architecture. Where these features are optional for the guest, the visibility of these registers may need to depend on some runtime configuration, such as a flag passed to KVM_ARM_VCPU_INIT. For example, ZCR_EL1 and ID_AA64ZFR0_EL1 need to be hidden if SVE is not enabled for the guest, even though these registers may be present in the hardware and visible to the host at EL2. Adding special-case checks all over the place for individual registers is going to get messy as the number of conditionally- visible registers grows. In order to help solve this problem, this patch adds a new sysreg method visibility() that can be used to hook in any needed runtime visibility checks. This method can currently return REG_HIDDEN_USER to inhibit enumeration and ioctl access to the register for userspace, and REG_HIDDEN_GUEST to inhibit runtime access by the guest using MSR/MRS. Wrappers are added to allow these flags to be conveniently queried. This approach allows a conditionally modified view of individual system registers such as the CPU ID registers, in addition to completely hiding register where appropriate. Signed-off-by:
-
Dave Martin authored
Architecture features that are conditionally visible to the guest will require run-time checks in the ID register accessor functions. In particular, read_id_reg() will need to perform checks in order to generate the correct emulated value for certain ID register fields such as ID_AA64PFR0_EL1.SVE for example. This patch propagates vcpu into read_id_reg() so that future patches can add run-time checks on the guest configuration here. For now, there is no functional change. Signed-off-by:
-
Dave Martin authored
Since SVE will be enabled or disabled on a per-vcpu basis, a flag is needed in order to track which vcpus have it enabled. This patch adds a suitable flag and a helper for checking it. Signed-off-by:
-
Dave Martin authored
The current FPSIMD/SVE context handling support for non-task (i.e., KVM vcpu) contexts does not take SVE into account. This means that only task contexts can safely use SVE at present. In preparation for enabling KVM guests to use SVE, it is necessary to keep track of SVE state for non-task contexts too. This patch adds the necessary support, removing assumptions from the context switch code about the location of the SVE context storage. When binding a vcpu context, its vector length is arbitrarily specified as SVE_VL_MIN for now. In any case, because TIF_SVE is presently cleared at vcpu context bind time, the specified vector length will not be used for anything yet. In later patches TIF_SVE will be set here as appropriate, and the appropriate maximum vector length for the vcpu will be passed when binding. Signed-off-by:
Julien Grall <julien.grall@arm.com> Tested-by:
-
Dave Martin authored
Due to the way the effective SVE vector length is controlled and trapped at different exception levels, certain mismatches in the sets of vector lengths supported by different physical CPUs in the system may prevent straightforward virtualisation of SVE at parity with the host. This patch analyses the extent to which SVE can be virtualised safely without interfering with migration of vcpus between physical CPUs, and rejects late secondary CPUs that would erode the situation further. It is left up to KVM to decide what to do with this information. Signed-off-by:
-
Dave Martin authored
The roles of sve_init_vq_map(), sve_update_vq_map() and sve_verify_vq_map() are highly non-obvious to anyone who has not dug through cpufeatures.c in detail. Since the way these functions interact with each other is more important here than a full understanding of the cpufeatures code, this patch adds comments to make the functions' roles clearer. No functional change. Signed-off-by:
-
Dave Martin authored
kvm_host.h uses some declarations from other headers that are currently included by accident, without an explicit #include. This patch adds a few #includes that are clearly missing. Although the header builds without them today, this should help to avoid future surprises. Signed-off-by:
Mark Rutland <mark.rutland@arm.com> Tested-by:
-
Dave Martin authored
kvm_arm_num_regs() adds together various partial register counts in a freeform sum expression, which makes it harder than necessary to read diffs that add, modify or remove a single term in the sum (which is expected to the common case under maintenance). This patch refactors the code to add the term one per line, for maximum readability. Signed-off-by:
-
Dave Martin authored
__fpsimd_enabled() no longer exists, but a dangling declaration has survived in kvm_hyp.h. This patch gets rid of it. Signed-off-by:
-
Dave Martin authored
This patch updates fpsimd_flush_task_state() to mirror the new semantics of fpsimd_flush_cpu_state() introduced by commit d8ad71fa ("arm64: fpsimd: Fix TIF_FOREIGN_FPSTATE after invalidating cpu regs"). Both functions now implicitly set TIF_FOREIGN_FPSTATE to indicate that the task's FPSIMD state is not loaded into the cpu. As a side-effect, fpsimd_flush_task_state() now sets TIF_FOREIGN_FPSTATE even for non-running tasks. In the case of non-running tasks this is not useful but also harmless, because the flag is live only while the corresponding task is running. This function is not called from fast paths, so special-casing this for the task == current case is not really worth it. Compiler barriers previously present in restore_sve_fpsimd_context() are pulled into fpsimd_flush_task_state() so that it can be safely called with preemption enabled if necessary. Explicit calls to set TIF_FOREIGN_FPSTATE that accompany fpsimd_flush_task_state() calls and are now redundant are removed as appropriate. fpsimd_flush_task_state() is used to get exclusive access to the representation of the task's state via task_struct, for the purpose of replacing the state. Thus, the call to this function should happen before manipulating fpsimd_state or sve_state etc. in task_struct. Anomalous cases are reordered appropriately in order to make the code more consistent, although there should be no functional difference since these cases are protected by local_bh_disable() anyway. Signed-off-by:
-
