From 3ec5ad40081b14af28496198b4d08dbe13386790 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Mar 2022 16:23:47 +0100 Subject: [PATCH 1/2] target/i386: kvm: do not access uninitialized variable on older kernels KVM support for AMX includes a new system attribute, KVM_X86_XCOMP_GUEST_SUPP. Commit 19db68ca68 ("x86: Grant AMX permission for guest", 2022-03-15) however did not fully consider the behavior on older kernels. First, it warns too aggressively. Second, it invokes the KVM_GET_DEVICE_ATTR ioctl unconditionally and then uses the "bitmask" variable, which remains uninitialized if the ioctl fails. Third, kvm_ioctl returns -errno rather than -1 on errors. While at it, explain why the ioctl is needed and KVM_GET_SUPPORTED_CPUID is not enough. Signed-off-by: Paolo Bonzini --- target/i386/kvm/kvm.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index ef2c68a6f4..06901c2a43 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -411,6 +411,12 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, } } else if (function == 0xd && index == 0 && (reg == R_EAX || reg == R_EDX)) { + /* + * The value returned by KVM_GET_SUPPORTED_CPUID does not include + * features that still have to be enabled with the arch_prctl + * system call. QEMU needs the full value, which is retrieved + * with KVM_GET_DEVICE_ATTR. + */ struct kvm_device_attr attr = { .group = 0, .attr = KVM_X86_XCOMP_GUEST_SUPP, @@ -419,13 +425,16 @@ uint32_t kvm_arch_get_supported_cpuid(KVMState *s, uint32_t function, bool sys_attr = kvm_check_extension(s, KVM_CAP_SYS_ATTRIBUTES); if (!sys_attr) { - warn_report("cannot get sys attribute capabilities %d", sys_attr); + return ret; } int rc = kvm_ioctl(s, KVM_GET_DEVICE_ATTR, &attr); - if (rc == -1 && (errno == ENXIO || errno == EINVAL)) { - warn_report("KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) " - "error: %d", rc); + if (rc < 0) { + if (rc != -ENXIO) { + warn_report("KVM_GET_DEVICE_ATTR(0, KVM_X86_XCOMP_GUEST_SUPP) " + "error: %d", rc); + } + return ret; } ret = (reg == R_EAX) ? bitmask : bitmask >> 32; } else if (function == 0x80000001 && reg == R_ECX) { From 17e6ffa6a5d2674cb2ebfd967d28b1048261d977 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 21 Mar 2022 15:30:18 +0100 Subject: [PATCH 2/2] hw/i386/amd_iommu: Fix maybe-uninitialized error with GCC 12 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Be more explicit that the loop must roll at least once. Avoids the following warning: FAILED: libqemu-x86_64-softmmu.fa.p/hw_i386_amd_iommu.c.o In function 'pte_get_page_mask', inlined from 'amdvi_page_walk' at hw/i386/amd_iommu.c:945:25, inlined from 'amdvi_do_translate' at hw/i386/amd_iommu.c:989:5, inlined from 'amdvi_translate' at hw/i386/amd_iommu.c:1038:5: hw/i386/amd_iommu.c:877:38: error: 'oldlevel' may be used uninitialized [-Werror=maybe-uninitialized] 877 | return ~((1UL << ((oldlevel * 9) + 3)) - 1); | ~~~~~~~~~~~~~~~~^~~~ hw/i386/amd_iommu.c: In function 'amdvi_translate': hw/i386/amd_iommu.c:906:41: note: 'oldlevel' was declared here 906 | unsigned level, present, pte_perms, oldlevel; | ^~~~~~~~ cc1: all warnings being treated as errors Having: $ gcc --version gcc (Debian 12-20220313-1) 12.0.1 20220314 (experimental) Reported-by: Philippe Mathieu-Daudé Signed-off-by: Paolo Bonzini --- hw/i386/amd_iommu.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/hw/i386/amd_iommu.c b/hw/i386/amd_iommu.c index 4d13d8e697..6986ad3b87 100644 --- a/hw/i386/amd_iommu.c +++ b/hw/i386/amd_iommu.c @@ -913,7 +913,7 @@ static void amdvi_page_walk(AMDVIAddressSpace *as, uint64_t *dte, } /* we are at the leaf page table or page table encodes a huge page */ - while (level > 0) { + do { pte_perms = amdvi_get_perms(pte); present = pte & 1; if (!present || perms != (perms & pte_perms)) { @@ -932,10 +932,7 @@ static void amdvi_page_walk(AMDVIAddressSpace *as, uint64_t *dte, } oldlevel = level; level = get_pte_translation_mode(pte); - if (level == 0x7) { - break; - } - } + } while (level > 0 && level < 7); if (level == 0x7) { page_mask = pte_override_page_mask(pte);