From b1fbee456c8dcb5d53dd0324bde1e17ffc6bc5de Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 19 Jun 2024 13:45:49 +0200 Subject: [PATCH 01/23] configure: detect --cpu=mipsisa64r6 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Treat it as a MIPS64 machine. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Thomas Huth Signed-off-by: Paolo Bonzini --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index 5ad1674ca5..8b6a2f16ce 100755 --- a/configure +++ b/configure @@ -450,7 +450,7 @@ case "$cpu" in linux_arch=loongarch ;; - mips64*) + mips64*|mipsisa64*) cpu=mips64 host_arch=mips linux_arch=mips From fe721c1948ef459caab106190276717bec252c88 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 18 Jun 2024 17:34:32 +0200 Subject: [PATCH 02/23] Revert "host/i386: assume presence of POPCNT" This reverts commit 45ccdbcb24baf99667997fac5cf60318e5e7db51. The x86-64 instruction set can now be tuned down to x86-64 v1 or i386 Pentium Pro. Signed-off-by: Paolo Bonzini --- host/include/i386/host/cpuinfo.h | 1 + tcg/i386/tcg-target.h | 5 +++-- util/cpuinfo-i386.c | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h index c1e94d75ce..72f6fad61e 100644 --- a/host/include/i386/host/cpuinfo.h +++ b/host/include/i386/host/cpuinfo.h @@ -11,6 +11,7 @@ #define CPUINFO_ALWAYS (1u << 0) /* so cpuinfo is nonzero */ #define CPUINFO_MOVBE (1u << 2) #define CPUINFO_LZCNT (1u << 3) +#define CPUINFO_POPCNT (1u << 4) #define CPUINFO_BMI1 (1u << 5) #define CPUINFO_BMI2 (1u << 6) #define CPUINFO_AVX1 (1u << 9) diff --git a/tcg/i386/tcg-target.h b/tcg/i386/tcg-target.h index ecc6982728..2f67a97e05 100644 --- a/tcg/i386/tcg-target.h +++ b/tcg/i386/tcg-target.h @@ -111,6 +111,7 @@ typedef enum { #endif #define have_bmi1 (cpuinfo & CPUINFO_BMI1) +#define have_popcnt (cpuinfo & CPUINFO_POPCNT) #define have_avx1 (cpuinfo & CPUINFO_AVX1) #define have_avx2 (cpuinfo & CPUINFO_AVX2) #define have_movbe (cpuinfo & CPUINFO_MOVBE) @@ -142,7 +143,7 @@ typedef enum { #define TCG_TARGET_HAS_nor_i32 0 #define TCG_TARGET_HAS_clz_i32 1 #define TCG_TARGET_HAS_ctz_i32 1 -#define TCG_TARGET_HAS_ctpop_i32 1 +#define TCG_TARGET_HAS_ctpop_i32 have_popcnt #define TCG_TARGET_HAS_deposit_i32 1 #define TCG_TARGET_HAS_extract_i32 1 #define TCG_TARGET_HAS_sextract_i32 1 @@ -177,7 +178,7 @@ typedef enum { #define TCG_TARGET_HAS_nor_i64 0 #define TCG_TARGET_HAS_clz_i64 1 #define TCG_TARGET_HAS_ctz_i64 1 -#define TCG_TARGET_HAS_ctpop_i64 1 +#define TCG_TARGET_HAS_ctpop_i64 have_popcnt #define TCG_TARGET_HAS_deposit_i64 1 #define TCG_TARGET_HAS_extract_i64 1 #define TCG_TARGET_HAS_sextract_i64 0 diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c index 8f2694d88f..6d474a6259 100644 --- a/util/cpuinfo-i386.c +++ b/util/cpuinfo-i386.c @@ -35,6 +35,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void) __cpuid(1, a, b, c, d); info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0); + info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0); info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0); /* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */ From 39a367a42a3e77f56e9cc01d098298167df3fcc3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 18 Jun 2024 17:34:45 +0200 Subject: [PATCH 03/23] Revert "host/i386: assume presence of SSSE3" This reverts commit 433cd6d94a8256af70a5200f236dc8047c3c1468. The x86-64 instruction set can now be tuned down to x86-64 v1 or i386 Pentium Pro. Signed-off-by: Paolo Bonzini --- util/cpuinfo-i386.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c index 6d474a6259..ca74ef04f5 100644 --- a/util/cpuinfo-i386.c +++ b/util/cpuinfo-i386.c @@ -38,8 +38,8 @@ unsigned __attribute__((constructor)) cpuinfo_init(void) info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0); info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0); - /* NOTE: our AES support requires SSSE3 (PSHUFB) as well. */ - info |= (c & bit_AES) ? CPUINFO_AES : 0; + /* Our AES support requires PSHUFB as well. */ + info |= ((c & bit_AES) && (c & bit_SSSE3) ? CPUINFO_AES : 0); /* For AVX features, we must check available and usable. */ if ((c & bit_AVX) && (c & bit_OSXSAVE)) { From 87b8bde55dc1700f212b2249b9c150714df67369 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 18 Jun 2024 17:34:48 +0200 Subject: [PATCH 04/23] Revert "host/i386: assume presence of SSE2" This reverts commit b18236897ca15c3db1506d8edb9a191dfe51429c. The x86-64 instruction set can now be tuned down to x86-64 v1 or i386 Pentium Pro. Signed-off-by: Paolo Bonzini --- host/include/i386/host/bufferiszero.c.inc | 5 +++-- host/include/i386/host/cpuinfo.h | 1 + util/cpuinfo-i386.c | 1 + 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/host/include/i386/host/bufferiszero.c.inc b/host/include/i386/host/bufferiszero.c.inc index 3b9605d806..74ae98580f 100644 --- a/host/include/i386/host/bufferiszero.c.inc +++ b/host/include/i386/host/bufferiszero.c.inc @@ -110,13 +110,14 @@ static biz_accel_fn const accel_table[] = { static unsigned best_accel(void) { -#ifdef CONFIG_AVX2_OPT unsigned info = cpuinfo_init(); + +#ifdef CONFIG_AVX2_OPT if (info & CPUINFO_AVX2) { return 2; } #endif - return 1; + return info & CPUINFO_SSE2 ? 1 : 0; } #else diff --git a/host/include/i386/host/cpuinfo.h b/host/include/i386/host/cpuinfo.h index 72f6fad61e..81771733ea 100644 --- a/host/include/i386/host/cpuinfo.h +++ b/host/include/i386/host/cpuinfo.h @@ -14,6 +14,7 @@ #define CPUINFO_POPCNT (1u << 4) #define CPUINFO_BMI1 (1u << 5) #define CPUINFO_BMI2 (1u << 6) +#define CPUINFO_SSE2 (1u << 7) #define CPUINFO_AVX1 (1u << 9) #define CPUINFO_AVX2 (1u << 10) #define CPUINFO_AVX512F (1u << 11) diff --git a/util/cpuinfo-i386.c b/util/cpuinfo-i386.c index ca74ef04f5..90f92a42dc 100644 --- a/util/cpuinfo-i386.c +++ b/util/cpuinfo-i386.c @@ -34,6 +34,7 @@ unsigned __attribute__((constructor)) cpuinfo_init(void) if (max >= 1) { __cpuid(1, a, b, c, d); + info |= (d & bit_SSE2 ? CPUINFO_SSE2 : 0); info |= (c & bit_MOVBE ? CPUINFO_MOVBE : 0); info |= (c & bit_POPCNT ? CPUINFO_POPCNT : 0); info |= (c & bit_PCLMUL ? CPUINFO_PCLMUL : 0); From ef7d1adfa8589bb7d6cb06463bf554877e086beb Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 18 Jun 2024 17:32:52 +0200 Subject: [PATCH 05/23] meson: allow configuring the x86-64 baseline Add a Meson option to configure which x86-64 instruction set to use. QEMU will now default to x86-64-v1 + cmpxchg16b for 64-bit builds (that corresponds to a Pentium 4 for 32-bit builds). The baseline can be tuned down to Pentium Pro for 32-bit builds (with -Dx86_version=0), or up as desired. Acked-by: Richard Henderson Signed-off-by: Paolo Bonzini --- meson.build | 41 ++++++++++++++++++++++++++++------- meson_options.txt | 3 +++ scripts/meson-buildoptions.sh | 3 +++ 3 files changed, 39 insertions(+), 8 deletions(-) diff --git a/meson.build b/meson.build index 97e00d6f59..6e694ecd9f 100644 --- a/meson.build +++ b/meson.build @@ -336,15 +336,40 @@ if host_arch == 'i386' and not cc.links(''' qemu_common_flags = ['-march=i486'] + qemu_common_flags endif -# Assume x86-64-v2 (minus CMPXCHG16B for 32-bit code) -if host_arch == 'i386' - qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags -endif +# Pick x86-64 baseline version if host_arch in ['i386', 'x86_64'] - qemu_common_flags = ['-mpopcnt', '-msse4.2'] + qemu_common_flags -endif -if host_arch == 'x86_64' - qemu_common_flags = ['-mcx16'] + qemu_common_flags + if get_option('x86_version') == '0' and host_arch == 'x86_64' + error('x86_64-v1 required for x86-64 hosts') + endif + + # add flags for individual instruction set extensions + if get_option('x86_version') >= '1' + if host_arch == 'i386' + qemu_common_flags = ['-mfpmath=sse'] + qemu_common_flags + else + # present on basically all processors but technically not part of + # x86-64-v1, so only include -mneeded for x86-64 version 2 and above + qemu_common_flags = ['-mcx16'] + qemu_common_flags + endif + endif + if get_option('x86_version') >= '2' + qemu_common_flags = ['-mpopcnt'] + qemu_common_flags + qemu_common_flags = cc.get_supported_arguments('-mneeded') + qemu_common_flags + endif + if get_option('x86_version') >= '3' + qemu_common_flags = ['-mmovbe', '-mabm', '-mbmi1', '-mbmi2', '-mfma', '-mf16c'] + qemu_common_flags + endif + + # add required vector instruction set (each level implies those below) + if get_option('x86_version') == '1' + qemu_common_flags = ['-msse2'] + qemu_common_flags + elif get_option('x86_version') == '2' + qemu_common_flags = ['-msse4.2'] + qemu_common_flags + elif get_option('x86_version') == '3' + qemu_common_flags = ['-mavx2'] + qemu_common_flags + elif get_option('x86_version') == '4' + qemu_common_flags = ['-mavx512f', '-mavx512bw', '-mavx512cd', '-mavx512dq', '-mavx512vl'] + qemu_common_flags + endif endif if get_option('prefer_static') diff --git a/meson_options.txt b/meson_options.txt index 7a79dd8970..6065ed2d35 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -370,3 +370,6 @@ option('qemu_ga_version', type: 'string', value: '', option('hexagon_idef_parser', type : 'boolean', value : true, description: 'use idef-parser to automatically generate TCG code for the Hexagon frontend') + +option('x86_version', type : 'combo', choices : ['0', '1', '2', '3', '4'], value: '1', + description: 'tweak required x86_64 architecture version beyond compiler default') diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 58d49a447d..62842d47e8 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -82,6 +82,8 @@ meson_options_help() { printf "%s\n" ' --with-suffix=VALUE Suffix for QEMU data/modules/config directories' printf "%s\n" ' (can be empty) [qemu]' printf "%s\n" ' --with-trace-file=VALUE Trace file prefix for simple backend [trace]' + printf "%s\n" ' --x86-version=CHOICE tweak required x86_64 architecture version beyond' + printf "%s\n" ' compiler default [1] (choices: 0/1/2/3)' printf "%s\n" '' printf "%s\n" 'Optional features, enabled with --enable-FEATURE and' printf "%s\n" 'disabled with --disable-FEATURE, default is enabled if available' @@ -552,6 +554,7 @@ _meson_option_parse() { --disable-werror) printf "%s" -Dwerror=false ;; --enable-whpx) printf "%s" -Dwhpx=enabled ;; --disable-whpx) printf "%s" -Dwhpx=disabled ;; + --x86-version=*) quote_sh "-Dx86_version=$2" ;; --enable-xen) printf "%s" -Dxen=enabled ;; --disable-xen) printf "%s" -Dxen=disabled ;; --enable-xen-pci-passthrough) printf "%s" -Dxen_pci_passthrough=enabled ;; From b3f1ce8a472e7239b196bcb3aaa38738012f23b5 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Jun 2024 14:57:30 +0200 Subject: [PATCH 06/23] meson: remove dead optimization option Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- meson.build | 13 ------------- meson_options.txt | 2 -- scripts/meson-buildoptions.sh | 3 --- 3 files changed, 18 deletions(-) diff --git a/meson.build b/meson.build index 6e694ecd9f..54e6b09f4f 100644 --- a/meson.build +++ b/meson.build @@ -2874,18 +2874,6 @@ config_host_data.set('CONFIG_AVX2_OPT', get_option('avx2') \ int main(int argc, char *argv[]) { return bar(argv[argc - 1]); } '''), error_message: 'AVX2 not available').allowed()) -config_host_data.set('CONFIG_AVX512F_OPT', get_option('avx512f') \ - .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512F') \ - .require(cc.links(''' - #include - #include - static int __attribute__((target("avx512f"))) bar(void *a) { - __m512i x = *(__m512i *)a; - return _mm512_test_epi64_mask(x, x); - } - int main(int argc, char *argv[]) { return bar(argv[argc - 1]); } - '''), error_message: 'AVX512F not available').allowed()) - config_host_data.set('CONFIG_AVX512BW_OPT', get_option('avx512bw') \ .require(have_cpuid_h, error_message: 'cpuid.h not available, cannot enable AVX512BW') \ .require(cc.links(''' @@ -4283,7 +4271,6 @@ summary_info += {'mutex debugging': get_option('debug_mutex')} summary_info += {'memory allocator': get_option('malloc')} summary_info += {'avx2 optimization': config_host_data.get('CONFIG_AVX2_OPT')} summary_info += {'avx512bw optimization': config_host_data.get('CONFIG_AVX512BW_OPT')} -summary_info += {'avx512f optimization': config_host_data.get('CONFIG_AVX512F_OPT')} summary_info += {'gcov': get_option('b_coverage')} summary_info += {'thread sanitizer': get_option('tsan')} summary_info += {'CFI support': get_option('cfi')} diff --git a/meson_options.txt b/meson_options.txt index 6065ed2d35..0269fa0f16 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -119,8 +119,6 @@ option('membarrier', type: 'feature', value: 'disabled', option('avx2', type: 'feature', value: 'auto', description: 'AVX2 optimizations') -option('avx512f', type: 'feature', value: 'disabled', - description: 'AVX512F optimizations') option('avx512bw', type: 'feature', value: 'auto', description: 'AVX512BW optimizations') option('keyring', type: 'feature', value: 'auto', diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 62842d47e8..cfadb5ea86 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -95,7 +95,6 @@ meson_options_help() { printf "%s\n" ' auth-pam PAM access control' printf "%s\n" ' avx2 AVX2 optimizations' printf "%s\n" ' avx512bw AVX512BW optimizations' - printf "%s\n" ' avx512f AVX512F optimizations' printf "%s\n" ' blkio libblkio block device driver' printf "%s\n" ' bochs bochs image format support' printf "%s\n" ' bpf eBPF support' @@ -240,8 +239,6 @@ _meson_option_parse() { --disable-avx2) printf "%s" -Davx2=disabled ;; --enable-avx512bw) printf "%s" -Davx512bw=enabled ;; --disable-avx512bw) printf "%s" -Davx512bw=disabled ;; - --enable-avx512f) printf "%s" -Davx512f=enabled ;; - --disable-avx512f) printf "%s" -Davx512f=disabled ;; --enable-gcov) printf "%s" -Db_coverage=true ;; --disable-gcov) printf "%s" -Db_coverage=false ;; --enable-lto) printf "%s" -Db_lto=true ;; From ae8b45d29317be0bcc60d40e5d627f978b27ccd0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 24 Nov 2022 16:29:06 +0100 Subject: [PATCH 07/23] block: make assertion more generic .bdrv_needs_filename is only set for drivers that also set bdrv_file_open, i.e. protocol drivers. So we can make the assertion always, it will always pass for those drivers that use bdrv_open. Signed-off-by: Paolo Bonzini --- block.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/block.c b/block.c index 468cf5e67d..69a2905178 100644 --- a/block.c +++ b/block.c @@ -1655,8 +1655,8 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name, bs->drv = drv; bs->opaque = g_malloc0(drv->instance_size); + assert(!drv->bdrv_needs_filename || bs->filename[0]); if (drv->bdrv_file_open) { - assert(!drv->bdrv_needs_filename || bs->filename[0]); ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); } else if (drv->bdrv_open) { ret = drv->bdrv_open(bs, options, open_flags, &local_err); From 41770f6e6ff41ca98e130c79b566f05ec68912fe Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 24 Nov 2022 16:21:18 +0100 Subject: [PATCH 08/23] block: do not check bdrv_file_open The set of BlockDrivers that have .bdrv_file_open coincides with those that have .protocol_name and guess what---checking drv->bdrv_file_open is done to see if the driver is a protocol. So check drv->protocol_name instead. Signed-off-by: Paolo Bonzini --- block.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/block.c b/block.c index 69a2905178..dd14ba85fc 100644 --- a/block.c +++ b/block.c @@ -926,7 +926,6 @@ BlockDriver *bdrv_find_protocol(const char *filename, int i; GLOBAL_STATE_CODE(); - /* TODO Drivers without bdrv_file_open must be specified explicitly */ /* * XXX(hch): we really should not let host device detection @@ -1983,7 +1982,7 @@ static int bdrv_open_common(BlockDriverState *bs, BlockBackend *file, open_flags = bdrv_open_flags(bs, bs->open_flags); node_name = qemu_opt_get(opts, "node-name"); - assert(!drv->bdrv_file_open || file == NULL); + assert(!drv->protocol_name || file == NULL); ret = bdrv_open_driver(bs, drv, node_name, options, open_flags, errp); if (ret < 0) { goto fail_opts; @@ -2084,7 +2083,7 @@ static int bdrv_fill_options(QDict **options, const char *filename, } /* If the user has explicitly specified the driver, this choice should * override the BDRV_O_PROTOCOL flag */ - protocol = drv->bdrv_file_open; + protocol = drv->protocol_name; } if (protocol) { @@ -4123,7 +4122,7 @@ bdrv_open_inherit(const char *filename, const char *reference, QDict *options, } /* BDRV_O_PROTOCOL must be set iff a protocol BDS is about to be created */ - assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->bdrv_file_open); + assert(!!(flags & BDRV_O_PROTOCOL) == !!drv->protocol_name); /* file must be NULL if a protocol BDS is about to be created * (the inverse results in an error message from bdrv_open_common()) */ assert(!(flags & BDRV_O_PROTOCOL) || !file); @@ -5971,7 +5970,7 @@ int64_t coroutine_fn bdrv_co_get_allocated_file_size(BlockDriverState *bs) return drv->bdrv_co_get_allocated_file_size(bs); } - if (drv->bdrv_file_open) { + if (drv->protocol_name) { /* * Protocol drivers default to -ENOTSUP (most of their data is * not stored in any of their children (if they even have any), @@ -8030,7 +8029,7 @@ void bdrv_refresh_filename(BlockDriverState *bs) * Both of these conditions are represented by generate_json_filename. */ if (primary_child_bs->exact_filename[0] && - primary_child_bs->drv->bdrv_file_open && + primary_child_bs->drv->protocol_name && !drv->is_filter && !generate_json_filename) { strcpy(bs->exact_filename, primary_child_bs->exact_filename); From 44b424dc4a3e2d47fa20676f00645fb950d8d76a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 24 Nov 2022 16:22:22 +0100 Subject: [PATCH 09/23] block: remove separate bdrv_file_open callback bdrv_file_open and bdrv_open are completely equivalent, they are never checked except to see which one to invoke. So merge them into a single one. Signed-off-by: Paolo Bonzini --- block.c | 4 +--- block/blkdebug.c | 2 +- block/blkio.c | 2 +- block/blkverify.c | 2 +- block/curl.c | 8 ++++---- block/file-posix.c | 8 ++++---- block/file-win32.c | 4 ++-- block/gluster.c | 6 +++--- block/iscsi.c | 4 ++-- block/nbd.c | 6 +++--- block/nfs.c | 2 +- block/null.c | 4 ++-- block/nvme.c | 2 +- block/rbd.c | 3 ++- block/ssh.c | 2 +- block/vvfat.c | 2 +- include/block/block_int-common.h | 3 --- 17 files changed, 30 insertions(+), 34 deletions(-) diff --git a/block.c b/block.c index dd14ba85fc..c1cc313d21 100644 --- a/block.c +++ b/block.c @@ -1655,9 +1655,7 @@ bdrv_open_driver(BlockDriverState *bs, BlockDriver *drv, const char *node_name, bs->opaque = g_malloc0(drv->instance_size); assert(!drv->bdrv_needs_filename || bs->filename[0]); - if (drv->bdrv_file_open) { - ret = drv->bdrv_file_open(bs, options, open_flags, &local_err); - } else if (drv->bdrv_open) { + if (drv->bdrv_open) { ret = drv->bdrv_open(bs, options, open_flags, &local_err); } else { ret = 0; diff --git a/block/blkdebug.c b/block/blkdebug.c index 9da8c9eddc..c95c818c38 100644 --- a/block/blkdebug.c +++ b/block/blkdebug.c @@ -1073,7 +1073,7 @@ static BlockDriver bdrv_blkdebug = { .is_filter = true, .bdrv_parse_filename = blkdebug_parse_filename, - .bdrv_file_open = blkdebug_open, + .bdrv_open = blkdebug_open, .bdrv_close = blkdebug_close, .bdrv_reopen_prepare = blkdebug_reopen_prepare, .bdrv_child_perm = blkdebug_child_perm, diff --git a/block/blkio.c b/block/blkio.c index 882e1c297b..1a38064ce7 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp) */ #define BLKIO_DRIVER_COMMON \ .instance_size = sizeof(BDRVBlkioState), \ - .bdrv_file_open = blkio_file_open, \ + .bdrv_open = blkio_file_open, \ .bdrv_close = blkio_close, \ .bdrv_co_getlength = blkio_co_getlength, \ .bdrv_co_truncate = blkio_truncate, \ diff --git a/block/blkverify.c b/block/blkverify.c index ec45d8335e..5a9bf674d9 100644 --- a/block/blkverify.c +++ b/block/blkverify.c @@ -321,7 +321,7 @@ static BlockDriver bdrv_blkverify = { .instance_size = sizeof(BDRVBlkverifyState), .bdrv_parse_filename = blkverify_parse_filename, - .bdrv_file_open = blkverify_open, + .bdrv_open = blkverify_open, .bdrv_close = blkverify_close, .bdrv_child_perm = bdrv_default_perms, .bdrv_co_getlength = blkverify_co_getlength, diff --git a/block/curl.c b/block/curl.c index 419f7c89ef..ef5252d00b 100644 --- a/block/curl.c +++ b/block/curl.c @@ -1034,7 +1034,7 @@ static BlockDriver bdrv_http = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, @@ -1053,7 +1053,7 @@ static BlockDriver bdrv_https = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, @@ -1072,7 +1072,7 @@ static BlockDriver bdrv_ftp = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, @@ -1091,7 +1091,7 @@ static BlockDriver bdrv_ftps = { .instance_size = sizeof(BDRVCURLState), .bdrv_parse_filename = curl_parse_filename, - .bdrv_file_open = curl_open, + .bdrv_open = curl_open, .bdrv_close = curl_close, .bdrv_co_getlength = curl_co_getlength, diff --git a/block/file-posix.c b/block/file-posix.c index be25e35ff6..f3bd946afa 100644 --- a/block/file-posix.c +++ b/block/file-posix.c @@ -3886,7 +3886,7 @@ BlockDriver bdrv_file = { .bdrv_needs_filename = true, .bdrv_probe = NULL, /* no probe for protocols */ .bdrv_parse_filename = raw_parse_filename, - .bdrv_file_open = raw_open, + .bdrv_open = raw_open, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, .bdrv_reopen_abort = raw_reopen_abort, @@ -4257,7 +4257,7 @@ static BlockDriver bdrv_host_device = { .bdrv_needs_filename = true, .bdrv_probe_device = hdev_probe_device, .bdrv_parse_filename = hdev_parse_filename, - .bdrv_file_open = hdev_open, + .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, @@ -4396,7 +4396,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_needs_filename = true, .bdrv_probe_device = cdrom_probe_device, .bdrv_parse_filename = cdrom_parse_filename, - .bdrv_file_open = cdrom_open, + .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, @@ -4522,7 +4522,7 @@ static BlockDriver bdrv_host_cdrom = { .bdrv_needs_filename = true, .bdrv_probe_device = cdrom_probe_device, .bdrv_parse_filename = cdrom_parse_filename, - .bdrv_file_open = cdrom_open, + .bdrv_open = cdrom_open, .bdrv_close = raw_close, .bdrv_reopen_prepare = raw_reopen_prepare, .bdrv_reopen_commit = raw_reopen_commit, diff --git a/block/file-win32.c b/block/file-win32.c index 48b790d917..7e1baa1ece 100644 --- a/block/file-win32.c +++ b/block/file-win32.c @@ -746,7 +746,7 @@ BlockDriver bdrv_file = { .instance_size = sizeof(BDRVRawState), .bdrv_needs_filename = true, .bdrv_parse_filename = raw_parse_filename, - .bdrv_file_open = raw_open, + .bdrv_open = raw_open, .bdrv_refresh_limits = raw_probe_alignment, .bdrv_close = raw_close, .bdrv_co_create_opts = raw_co_create_opts, @@ -920,7 +920,7 @@ static BlockDriver bdrv_host_device = { .bdrv_needs_filename = true, .bdrv_parse_filename = hdev_parse_filename, .bdrv_probe_device = hdev_probe_device, - .bdrv_file_open = hdev_open, + .bdrv_open = hdev_open, .bdrv_close = raw_close, .bdrv_refresh_limits = hdev_refresh_limits, diff --git a/block/gluster.c b/block/gluster.c index d0999903df..f8b415f381 100644 --- a/block/gluster.c +++ b/block/gluster.c @@ -1551,7 +1551,7 @@ static BlockDriver bdrv_gluster = { .format_name = "gluster", .protocol_name = "gluster", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1580,7 +1580,7 @@ static BlockDriver bdrv_gluster_tcp = { .format_name = "gluster", .protocol_name = "gluster+tcp", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, @@ -1609,7 +1609,7 @@ static BlockDriver bdrv_gluster_unix = { .format_name = "gluster", .protocol_name = "gluster+unix", .instance_size = sizeof(BDRVGlusterState), - .bdrv_file_open = qemu_gluster_open, + .bdrv_open = qemu_gluster_open, .bdrv_reopen_prepare = qemu_gluster_reopen_prepare, .bdrv_reopen_commit = qemu_gluster_reopen_commit, .bdrv_reopen_abort = qemu_gluster_reopen_abort, diff --git a/block/iscsi.c b/block/iscsi.c index 2ff14b7472..979bf90cb7 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -2429,7 +2429,7 @@ static BlockDriver bdrv_iscsi = { .instance_size = sizeof(IscsiLun), .bdrv_parse_filename = iscsi_parse_filename, - .bdrv_file_open = iscsi_open, + .bdrv_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, @@ -2468,7 +2468,7 @@ static BlockDriver bdrv_iser = { .instance_size = sizeof(IscsiLun), .bdrv_parse_filename = iscsi_parse_filename, - .bdrv_file_open = iscsi_open, + .bdrv_open = iscsi_open, .bdrv_close = iscsi_close, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, diff --git a/block/nbd.c b/block/nbd.c index 589d28af83..d464315766 100644 --- a/block/nbd.c +++ b/block/nbd.c @@ -2146,7 +2146,7 @@ static BlockDriver bdrv_nbd = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, @@ -2174,7 +2174,7 @@ static BlockDriver bdrv_nbd_tcp = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, @@ -2202,7 +2202,7 @@ static BlockDriver bdrv_nbd_unix = { .bdrv_parse_filename = nbd_parse_filename, .bdrv_co_create_opts = bdrv_co_create_opts_simple, .create_opts = &bdrv_create_opts_simple, - .bdrv_file_open = nbd_open, + .bdrv_open = nbd_open, .bdrv_reopen_prepare = nbd_client_reopen_prepare, .bdrv_co_preadv = nbd_client_co_preadv, .bdrv_co_pwritev = nbd_client_co_pwritev, diff --git a/block/nfs.c b/block/nfs.c index 60240a8733..0500f60c08 100644 --- a/block/nfs.c +++ b/block/nfs.c @@ -888,7 +888,7 @@ static BlockDriver bdrv_nfs = { #endif .bdrv_co_truncate = nfs_file_co_truncate, - .bdrv_file_open = nfs_file_open, + .bdrv_open = nfs_file_open, .bdrv_close = nfs_file_close, .bdrv_co_create = nfs_file_co_create, .bdrv_co_create_opts = nfs_file_co_create_opts, diff --git a/block/null.c b/block/null.c index 4808704ffd..6fa64d20d8 100644 --- a/block/null.c +++ b/block/null.c @@ -283,7 +283,7 @@ static BlockDriver bdrv_null_co = { .protocol_name = "null-co", .instance_size = sizeof(BDRVNullState), - .bdrv_file_open = null_file_open, + .bdrv_open = null_file_open, .bdrv_parse_filename = null_co_parse_filename, .bdrv_co_getlength = null_co_getlength, .bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size, @@ -304,7 +304,7 @@ static BlockDriver bdrv_null_aio = { .protocol_name = "null-aio", .instance_size = sizeof(BDRVNullState), - .bdrv_file_open = null_file_open, + .bdrv_open = null_file_open, .bdrv_parse_filename = null_aio_parse_filename, .bdrv_co_getlength = null_co_getlength, .bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size, diff --git a/block/nvme.c b/block/nvme.c index 3a3c6da73d..c84914af6d 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -1630,7 +1630,7 @@ static BlockDriver bdrv_nvme = { .create_opts = &bdrv_create_opts_simple, .bdrv_parse_filename = nvme_parse_filename, - .bdrv_file_open = nvme_file_open, + .bdrv_open = nvme_file_open, .bdrv_close = nvme_close, .bdrv_co_getlength = nvme_co_getlength, .bdrv_probe_blocksizes = nvme_probe_blocksizes, diff --git a/block/rbd.c b/block/rbd.c index 84bb2fa5d7..9c0fd0cb3f 100644 --- a/block/rbd.c +++ b/block/rbd.c @@ -1815,8 +1815,9 @@ static const char *const qemu_rbd_strong_runtime_opts[] = { static BlockDriver bdrv_rbd = { .format_name = "rbd", .instance_size = sizeof(BDRVRBDState), + .bdrv_parse_filename = qemu_rbd_parse_filename, - .bdrv_file_open = qemu_rbd_open, + .bdrv_open = qemu_rbd_open, .bdrv_close = qemu_rbd_close, .bdrv_reopen_prepare = qemu_rbd_reopen_prepare, .bdrv_co_create = qemu_rbd_co_create, diff --git a/block/ssh.c b/block/ssh.c index a88171d4b5..1344822ed8 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -1362,7 +1362,7 @@ static BlockDriver bdrv_ssh = { .protocol_name = "ssh", .instance_size = sizeof(BDRVSSHState), .bdrv_parse_filename = ssh_parse_filename, - .bdrv_file_open = ssh_file_open, + .bdrv_open = ssh_file_open, .bdrv_co_create = ssh_co_create, .bdrv_co_create_opts = ssh_co_create_opts, .bdrv_close = ssh_close, diff --git a/block/vvfat.c b/block/vvfat.c index 9d050ba3ae..086fedf474 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -3258,7 +3258,7 @@ static BlockDriver bdrv_vvfat = { .instance_size = sizeof(BDRVVVFATState), .bdrv_parse_filename = vvfat_parse_filename, - .bdrv_file_open = vvfat_open, + .bdrv_open = vvfat_open, .bdrv_refresh_limits = vvfat_refresh_limits, .bdrv_close = vvfat_close, .bdrv_child_perm = vvfat_child_perm, diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h index 761276127e..ebb4e56a50 100644 --- a/include/block/block_int-common.h +++ b/include/block/block_int-common.h @@ -248,9 +248,6 @@ struct BlockDriver { int GRAPH_UNLOCKED_PTR (*bdrv_open)( BlockDriverState *bs, QDict *options, int flags, Error **errp); - /* Protocol drivers should implement this instead of bdrv_open */ - int GRAPH_UNLOCKED_PTR (*bdrv_file_open)( - BlockDriverState *bs, QDict *options, int flags, Error **errp); void (*bdrv_close)(BlockDriverState *bs); int coroutine_fn GRAPH_UNLOCKED_PTR (*bdrv_co_create)( From d656aaa1369adcd20baea485d4a96a4bfd6b1c86 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 4 Sep 2023 12:07:19 +0200 Subject: [PATCH 10/23] block: rename former bdrv_file_open callbacks Since there is no bdrv_file_open callback anymore, rename the implementations so that they end with "_open" instead of "_file_open". NFS is the exception because all the functions are named nfs_file_*. Suggested-by: Kevin Wolf Signed-off-by: Paolo Bonzini --- block/blkio.c | 8 ++++---- block/null.c | 8 ++++---- block/nvme.c | 8 ++++---- block/ssh.c | 6 +++--- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/block/blkio.c b/block/blkio.c index 1a38064ce7..3d9a2e764c 100644 --- a/block/blkio.c +++ b/block/blkio.c @@ -713,7 +713,7 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options, * for example will fail. * * In order to open the device read-only, we are using the `read-only` - * property of the libblkio driver in blkio_file_open(). + * property of the libblkio driver in blkio_open(). */ fd = qemu_open(path, O_RDWR, NULL); if (fd < 0) { @@ -791,8 +791,8 @@ static int blkio_virtio_blk_connect(BlockDriverState *bs, QDict *options, return 0; } -static int blkio_file_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) +static int blkio_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) { const char *blkio_driver = bs->drv->protocol_name; BDRVBlkioState *s = bs->opaque; @@ -1088,7 +1088,7 @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp) */ #define BLKIO_DRIVER_COMMON \ .instance_size = sizeof(BDRVBlkioState), \ - .bdrv_open = blkio_file_open, \ + .bdrv_open = blkio_open, \ .bdrv_close = blkio_close, \ .bdrv_co_getlength = blkio_co_getlength, \ .bdrv_co_truncate = blkio_truncate, \ diff --git a/block/null.c b/block/null.c index 6fa64d20d8..4730acc1eb 100644 --- a/block/null.c +++ b/block/null.c @@ -77,8 +77,8 @@ static void null_aio_parse_filename(const char *filename, QDict *options, } } -static int null_file_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) +static int null_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) { QemuOpts *opts; BDRVNullState *s = bs->opaque; @@ -283,7 +283,7 @@ static BlockDriver bdrv_null_co = { .protocol_name = "null-co", .instance_size = sizeof(BDRVNullState), - .bdrv_open = null_file_open, + .bdrv_open = null_open, .bdrv_parse_filename = null_co_parse_filename, .bdrv_co_getlength = null_co_getlength, .bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size, @@ -304,7 +304,7 @@ static BlockDriver bdrv_null_aio = { .protocol_name = "null-aio", .instance_size = sizeof(BDRVNullState), - .bdrv_open = null_file_open, + .bdrv_open = null_open, .bdrv_parse_filename = null_aio_parse_filename, .bdrv_co_getlength = null_co_getlength, .bdrv_co_get_allocated_file_size = null_co_get_allocated_file_size, diff --git a/block/nvme.c b/block/nvme.c index c84914af6d..3b588b139f 100644 --- a/block/nvme.c +++ b/block/nvme.c @@ -889,7 +889,7 @@ out: qemu_vfio_pci_unmap_bar(s->vfio, 0, (void *)regs, 0, sizeof(NvmeBar)); } - /* Cleaning up is done in nvme_file_open() upon error. */ + /* Cleaning up is done in nvme_open() upon error. */ return ret; } @@ -967,8 +967,8 @@ static void nvme_close(BlockDriverState *bs) g_free(s->device); } -static int nvme_file_open(BlockDriverState *bs, QDict *options, int flags, - Error **errp) +static int nvme_open(BlockDriverState *bs, QDict *options, int flags, + Error **errp) { const char *device; QemuOpts *opts; @@ -1630,7 +1630,7 @@ static BlockDriver bdrv_nvme = { .create_opts = &bdrv_create_opts_simple, .bdrv_parse_filename = nvme_parse_filename, - .bdrv_open = nvme_file_open, + .bdrv_open = nvme_open, .bdrv_close = nvme_close, .bdrv_co_getlength = nvme_co_getlength, .bdrv_probe_blocksizes = nvme_probe_blocksizes, diff --git a/block/ssh.c b/block/ssh.c index 1344822ed8..27d582e0e3 100644 --- a/block/ssh.c +++ b/block/ssh.c @@ -837,8 +837,8 @@ static int connect_to_ssh(BDRVSSHState *s, BlockdevOptionsSsh *opts, return ret; } -static int ssh_file_open(BlockDriverState *bs, QDict *options, int bdrv_flags, - Error **errp) +static int ssh_open(BlockDriverState *bs, QDict *options, int bdrv_flags, + Error **errp) { BDRVSSHState *s = bs->opaque; BlockdevOptionsSsh *opts; @@ -1362,7 +1362,7 @@ static BlockDriver bdrv_ssh = { .protocol_name = "ssh", .instance_size = sizeof(BDRVSSHState), .bdrv_parse_filename = ssh_parse_filename, - .bdrv_open = ssh_file_open, + .bdrv_open = ssh_open, .bdrv_co_create = ssh_co_create, .bdrv_co_create_opts = ssh_co_create_opts, .bdrv_close = ssh_close, From 17c7df806b39d512271749ffdfc0376473744d63 Mon Sep 17 00:00:00 2001 From: Roman Kiryanov Date: Tue, 18 Jun 2024 15:45:53 -0700 Subject: [PATCH 11/23] exec: avoid using C++ keywords in function parameters to use the QEMU headers with a C++ compiler. Signed-off-by: Roman Kiryanov Link: https://lore.kernel.org/r/20240618224553.878869-1-rkir@google.com Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 0903513d13..154626f9ad 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -925,7 +925,7 @@ struct MemoryListener { * the current transaction. */ void (*log_start)(MemoryListener *listener, MemoryRegionSection *section, - int old, int new); + int old_val, int new_val); /** * @log_stop: @@ -944,7 +944,7 @@ struct MemoryListener { * the current transaction. */ void (*log_stop)(MemoryListener *listener, MemoryRegionSection *section, - int old, int new); + int old_val, int new_val); /** * @log_sync: From 7246c4cc470409bc77ae607463d1fcd026149d6a Mon Sep 17 00:00:00 2001 From: Roman Kiryanov Date: Thu, 20 Jun 2024 13:16:54 -0700 Subject: [PATCH 12/23] exec: don't use void* in pointer arithmetic in headers void* pointer arithmetic is a GCC extentension which could not be available in other build tools (e.g. C++). This changes removes this assumption. Signed-off-by: Roman Kiryanov Suggested-by: Paolo Bonzini Link: https://lore.kernel.org/r/20240620201654.598024-1-rkir@google.com Signed-off-by: Paolo Bonzini --- include/exec/memory.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 154626f9ad..c26ede33d2 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -2764,7 +2764,7 @@ MemTxResult address_space_write_rom(AddressSpace *as, hwaddr addr, #include "exec/memory_ldst_phys.h.inc" struct MemoryRegionCache { - void *ptr; + uint8_t *ptr; hwaddr xlat; hwaddr len; FlatView *fv; From eb350d1d01d9b9df0ce174e2e1681699b071bab3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 25 Jun 2024 13:12:20 +0200 Subject: [PATCH 13/23] include: move typeof_strip_qual to compiler.h, use it in QAPI_LIST_LENGTH() The typeof_strip_qual() is most useful for the atomic fetch-and-modify operations in atomic.h, but it can be used elsewhere as well. For example, QAPI_LIST_LENGTH() assumes that the argument is not const, which is not a requirement. Move the macro to compiler.h and, while at it, move it under #ifndef __cplusplus to emphasize that it uses C-only constructs. A C++ version of typeof_strip_qual() using type traits is possible[1], but beyond the scope of this patch because the little C++ code that is in QEMU does not use QAPI. The patch was tested by changing the declaration of strv_from_str_list() in qapi/qapi-type-helpers.c to: char **strv_from_str_list(const strList *const list) This is valid C code, and it fails to compile without this change. [1] https://lore.kernel.org/qemu-devel/20240624205647.112034-1-flwu@google.com/ Reviewed-by: Richard Henderson Reviewed-by: Manos Pitsidianakis Tested-by: Manos Pitsidianakis Signed-off-by: Paolo Bonzini --- include/qapi/util.h | 2 +- include/qemu/atomic.h | 42 ------------------------------------- include/qemu/compiler.h | 46 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/include/qapi/util.h b/include/qapi/util.h index 20dfea8a54..b8254247b8 100644 --- a/include/qapi/util.h +++ b/include/qapi/util.h @@ -62,7 +62,7 @@ int parse_qapi_name(const char *name, bool complete); #define QAPI_LIST_LENGTH(list) \ ({ \ size_t _len = 0; \ - typeof(list) _tail; \ + typeof_strip_qual(list) _tail; \ for (_tail = list; _tail != NULL; _tail = _tail->next) { \ _len++; \ } \ diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h index 99110abefb..dc4118ddd9 100644 --- a/include/qemu/atomic.h +++ b/include/qemu/atomic.h @@ -20,48 +20,6 @@ /* Compiler barrier */ #define barrier() ({ asm volatile("" ::: "memory"); (void)0; }) -/* The variable that receives the old value of an atomically-accessed - * variable must be non-qualified, because atomic builtins return values - * through a pointer-type argument as in __atomic_load(&var, &old, MODEL). - * - * This macro has to handle types smaller than int manually, because of - * implicit promotion. int and larger types, as well as pointers, can be - * converted to a non-qualified type just by applying a binary operator. - */ -#define typeof_strip_qual(expr) \ - typeof( \ - __builtin_choose_expr( \ - __builtin_types_compatible_p(typeof(expr), bool) || \ - __builtin_types_compatible_p(typeof(expr), const bool) || \ - __builtin_types_compatible_p(typeof(expr), volatile bool) || \ - __builtin_types_compatible_p(typeof(expr), const volatile bool), \ - (bool)1, \ - __builtin_choose_expr( \ - __builtin_types_compatible_p(typeof(expr), signed char) || \ - __builtin_types_compatible_p(typeof(expr), const signed char) || \ - __builtin_types_compatible_p(typeof(expr), volatile signed char) || \ - __builtin_types_compatible_p(typeof(expr), const volatile signed char), \ - (signed char)1, \ - __builtin_choose_expr( \ - __builtin_types_compatible_p(typeof(expr), unsigned char) || \ - __builtin_types_compatible_p(typeof(expr), const unsigned char) || \ - __builtin_types_compatible_p(typeof(expr), volatile unsigned char) || \ - __builtin_types_compatible_p(typeof(expr), const volatile unsigned char), \ - (unsigned char)1, \ - __builtin_choose_expr( \ - __builtin_types_compatible_p(typeof(expr), signed short) || \ - __builtin_types_compatible_p(typeof(expr), const signed short) || \ - __builtin_types_compatible_p(typeof(expr), volatile signed short) || \ - __builtin_types_compatible_p(typeof(expr), const volatile signed short), \ - (signed short)1, \ - __builtin_choose_expr( \ - __builtin_types_compatible_p(typeof(expr), unsigned short) || \ - __builtin_types_compatible_p(typeof(expr), const unsigned short) || \ - __builtin_types_compatible_p(typeof(expr), volatile unsigned short) || \ - __builtin_types_compatible_p(typeof(expr), const volatile unsigned short), \ - (unsigned short)1, \ - (expr)+0)))))) - #ifndef __ATOMIC_RELAXED #error "Expecting C11 atomic ops" #endif diff --git a/include/qemu/compiler.h b/include/qemu/compiler.h index c797f0d457..554c5ce7df 100644 --- a/include/qemu/compiler.h +++ b/include/qemu/compiler.h @@ -227,4 +227,50 @@ #define SECOND_ARG(first, second, ...) second #define IS_EMPTY_(junk_maybecomma) SECOND_ARG(junk_maybecomma 1, 0) +#ifndef __cplusplus +/* + * Useful in macros that need to declare temporary variables. For example, + * the variable that receives the old value of an atomically-accessed + * variable must be non-qualified, because atomic builtins return values + * through a pointer-type argument as in __atomic_load(&var, &old, MODEL). + * + * This macro has to handle types smaller than int manually, because of + * implicit promotion. int and larger types, as well as pointers, can be + * converted to a non-qualified type just by applying a binary operator. + */ +#define typeof_strip_qual(expr) \ + typeof( \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), bool) || \ + __builtin_types_compatible_p(typeof(expr), const bool) || \ + __builtin_types_compatible_p(typeof(expr), volatile bool) || \ + __builtin_types_compatible_p(typeof(expr), const volatile bool), \ + (bool)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), signed char) || \ + __builtin_types_compatible_p(typeof(expr), const signed char) || \ + __builtin_types_compatible_p(typeof(expr), volatile signed char) || \ + __builtin_types_compatible_p(typeof(expr), const volatile signed char), \ + (signed char)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), unsigned char) || \ + __builtin_types_compatible_p(typeof(expr), const unsigned char) || \ + __builtin_types_compatible_p(typeof(expr), volatile unsigned char) || \ + __builtin_types_compatible_p(typeof(expr), const volatile unsigned char), \ + (unsigned char)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), signed short) || \ + __builtin_types_compatible_p(typeof(expr), const signed short) || \ + __builtin_types_compatible_p(typeof(expr), volatile signed short) || \ + __builtin_types_compatible_p(typeof(expr), const volatile signed short), \ + (signed short)1, \ + __builtin_choose_expr( \ + __builtin_types_compatible_p(typeof(expr), unsigned short) || \ + __builtin_types_compatible_p(typeof(expr), const unsigned short) || \ + __builtin_types_compatible_p(typeof(expr), volatile unsigned short) || \ + __builtin_types_compatible_p(typeof(expr), const volatile unsigned short), \ + (unsigned short)1, \ + (expr)+0)))))) +#endif + #endif /* COMPILER_H */ From e36b976da4f6f4c0d434e6bb811f60b7b445e8ea Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Jun 2024 19:46:07 +0200 Subject: [PATCH 14/23] target/i386: fix CC_OP dump POPCNT was missing, and the entries were all out of order after ADCX/ADOX/ADCOX were moved close to EFLAGS. Just use designated initializers. Fixes: 4885c3c4953 ("target-i386: Use ctpop helper", 2017-01-10) Fixes: cc155f19717 ("target/i386: rewrite flags writeback for ADCX/ADOX", 2024-06-11) Signed-off-by: Paolo Bonzini --- target/i386/cpu-dump.c | 101 +++++++++++++++++++++-------------------- 1 file changed, 51 insertions(+), 50 deletions(-) diff --git a/target/i386/cpu-dump.c b/target/i386/cpu-dump.c index 40697064d9..3bb8e44091 100644 --- a/target/i386/cpu-dump.c +++ b/target/i386/cpu-dump.c @@ -28,69 +28,70 @@ /* x86 debug */ static const char *cc_op_str[CC_OP_NB] = { - "DYNAMIC", - "EFLAGS", + [CC_OP_DYNAMIC] = "DYNAMIC", - "MULB", - "MULW", - "MULL", - "MULQ", + [CC_OP_EFLAGS] = "EFLAGS", + [CC_OP_ADCX] = "ADCX", + [CC_OP_ADOX] = "ADOX", + [CC_OP_ADCOX] = "ADCOX", - "ADDB", - "ADDW", - "ADDL", - "ADDQ", + [CC_OP_MULB] = "MULB", + [CC_OP_MULW] = "MULW", + [CC_OP_MULL] = "MULL", + [CC_OP_MULQ] = "MULQ", - "ADCB", - "ADCW", - "ADCL", - "ADCQ", + [CC_OP_ADDB] = "ADDB", + [CC_OP_ADDW] = "ADDW", + [CC_OP_ADDL] = "ADDL", + [CC_OP_ADDQ] = "ADDQ", - "SUBB", - "SUBW", - "SUBL", - "SUBQ", + [CC_OP_ADCB] = "ADCB", + [CC_OP_ADCW] = "ADCW", + [CC_OP_ADCL] = "ADCL", + [CC_OP_ADCQ] = "ADCQ", - "SBBB", - "SBBW", - "SBBL", - "SBBQ", + [CC_OP_SUBB] = "SUBB", + [CC_OP_SUBW] = "SUBW", + [CC_OP_SUBL] = "SUBL", + [CC_OP_SUBQ] = "SUBQ", - "LOGICB", - "LOGICW", - "LOGICL", - "LOGICQ", + [CC_OP_SBBB] = "SBBB", + [CC_OP_SBBW] = "SBBW", + [CC_OP_SBBL] = "SBBL", + [CC_OP_SBBQ] = "SBBQ", - "INCB", - "INCW", - "INCL", - "INCQ", + [CC_OP_LOGICB] = "LOGICB", + [CC_OP_LOGICW] = "LOGICW", + [CC_OP_LOGICL] = "LOGICL", + [CC_OP_LOGICQ] = "LOGICQ", - "DECB", - "DECW", - "DECL", - "DECQ", + [CC_OP_INCB] = "INCB", + [CC_OP_INCW] = "INCW", + [CC_OP_INCL] = "INCL", + [CC_OP_INCQ] = "INCQ", - "SHLB", - "SHLW", - "SHLL", - "SHLQ", + [CC_OP_DECB] = "DECB", + [CC_OP_DECW] = "DECW", + [CC_OP_DECL] = "DECL", + [CC_OP_DECQ] = "DECQ", - "SARB", - "SARW", - "SARL", - "SARQ", + [CC_OP_SHLB] = "SHLB", + [CC_OP_SHLW] = "SHLW", + [CC_OP_SHLL] = "SHLL", + [CC_OP_SHLQ] = "SHLQ", - "BMILGB", - "BMILGW", - "BMILGL", - "BMILGQ", + [CC_OP_SARB] = "SARB", + [CC_OP_SARW] = "SARW", + [CC_OP_SARL] = "SARL", + [CC_OP_SARQ] = "SARQ", - "ADCX", - "ADOX", - "ADCOX", + [CC_OP_BMILGB] = "BMILGB", + [CC_OP_BMILGW] = "BMILGW", + [CC_OP_BMILGL] = "BMILGL", + [CC_OP_BMILGQ] = "BMILGQ", - "CLR", + [CC_OP_POPCNT] = "POPCNT", + [CC_OP_CLR] = "CLR", }; static void From 944f4001346019a3cd05567695aa48830c904626 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Jun 2024 11:07:39 +0200 Subject: [PATCH 15/23] target/i386: use cpu_cc_dst for CC_OP_POPCNT It is the only CCOp, among those that compute ZF from one of the cc_op_* registers, that uses cpu_cc_src. Do not make it the odd one off, instead use cpu_cc_dst like the others. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/cpu.h | 2 +- target/i386/tcg/cc_helper.c | 2 +- target/i386/tcg/emit.c.inc | 4 ++-- target/i386/tcg/translate.c | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 52571ababe..1b4edbe058 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1332,7 +1332,7 @@ typedef enum { CC_OP_BMILGQ, CC_OP_CLR, /* Z set, all other flags clear. */ - CC_OP_POPCNT, /* Z via CC_SRC, all other flags clear. */ + CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */ CC_OP_NB, } CCOp; diff --git a/target/i386/tcg/cc_helper.c b/target/i386/tcg/cc_helper.c index f76e9cb8cf..301ed95406 100644 --- a/target/i386/tcg/cc_helper.c +++ b/target/i386/tcg/cc_helper.c @@ -107,7 +107,7 @@ target_ulong helper_cc_compute_all(target_ulong dst, target_ulong src1, case CC_OP_CLR: return CC_Z | CC_P; case CC_OP_POPCNT: - return src1 ? 0 : CC_Z; + return dst ? 0 : CC_Z; case CC_OP_MULB: return compute_all_mulb(dst, src1); diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc index 11faa70b5e..fc7477833b 100644 --- a/target/i386/tcg/emit.c.inc +++ b/target/i386/tcg/emit.c.inc @@ -2804,10 +2804,10 @@ static void gen_POPA(DisasContext *s, X86DecodedInsn *decode) static void gen_POPCNT(DisasContext *s, X86DecodedInsn *decode) { - decode->cc_src = tcg_temp_new(); + decode->cc_dst = tcg_temp_new(); decode->cc_op = CC_OP_POPCNT; - tcg_gen_mov_tl(decode->cc_src, s->T0); + tcg_gen_mov_tl(decode->cc_dst, s->T0); tcg_gen_ctpop_tl(s->T0, s->T0); } diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index ad1819815a..eb353dc3c9 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -324,7 +324,7 @@ static const uint8_t cc_op_live[CC_OP_NB] = { [CC_OP_ADOX] = USES_CC_SRC | USES_CC_SRC2, [CC_OP_ADCOX] = USES_CC_DST | USES_CC_SRC | USES_CC_SRC2, [CC_OP_CLR] = 0, - [CC_OP_POPCNT] = USES_CC_SRC, + [CC_OP_POPCNT] = USES_CC_DST, }; static void set_cc_op_1(DisasContext *s, CCOp op, bool dirty) @@ -1020,7 +1020,7 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg) case CC_OP_CLR: return (CCPrepare) { .cond = TCG_COND_ALWAYS }; case CC_OP_POPCNT: - return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_src }; + return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst }; default: { MemOp size = (s->cc_op - CC_OP_ADDB) & 3; From 460231ad369fd9e6608859fd747bbf276850d96b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 20 Jun 2024 10:33:56 +0200 Subject: [PATCH 16/23] target/i386: give CC_OP_POPCNT low bits corresponding to MO_TL Handle it like the other arithmetic cc_ops. This simplifies a bit the implementation of bit test instructions. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/cpu.h | 13 +++++++++++-- target/i386/tcg/translate.c | 3 +-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/target/i386/cpu.h b/target/i386/cpu.h index 1b4edbe058..29daf37048 100644 --- a/target/i386/cpu.h +++ b/target/i386/cpu.h @@ -1275,6 +1275,7 @@ typedef enum { CC_OP_ADCX, /* CC_DST = C, CC_SRC = rest. */ CC_OP_ADOX, /* CC_SRC2 = O, CC_SRC = rest. */ CC_OP_ADCOX, /* CC_DST = C, CC_SRC2 = O, CC_SRC = rest. */ + CC_OP_CLR, /* Z and P set, all other flags clear. */ CC_OP_MULB, /* modify all flags, C, O = (CC_SRC != 0) */ CC_OP_MULW, @@ -1331,8 +1332,16 @@ typedef enum { CC_OP_BMILGL, CC_OP_BMILGQ, - CC_OP_CLR, /* Z set, all other flags clear. */ - CC_OP_POPCNT, /* Z via CC_DST, all other flags clear. */ + /* + * Note that only CC_OP_POPCNT (i.e. the one with MO_TL size) + * is used or implemented, because the translation needs + * to zero-extend CC_DST anyway. + */ + CC_OP_POPCNTB__, /* Z via CC_DST, all other flags clear. */ + CC_OP_POPCNTW__, + CC_OP_POPCNTL__, + CC_OP_POPCNTQ__, + CC_OP_POPCNT = sizeof(target_ulong) == 8 ? CC_OP_POPCNTQ__ : CC_OP_POPCNTL__, CC_OP_NB, } CCOp; diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index eb353dc3c9..934c514e64 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -1019,8 +1019,6 @@ static CCPrepare gen_prepare_eflags_z(DisasContext *s, TCGv reg) .imm = CC_Z }; case CC_OP_CLR: return (CCPrepare) { .cond = TCG_COND_ALWAYS }; - case CC_OP_POPCNT: - return (CCPrepare) { .cond = TCG_COND_EQ, .reg = cpu_cc_dst }; default: { MemOp size = (s->cc_op - CC_OP_ADDB) & 3; @@ -3177,6 +3175,7 @@ static void disas_insn_old(DisasContext *s, CPUState *cpu, int b) case CC_OP_SHLB ... CC_OP_SHLQ: case CC_OP_SARB ... CC_OP_SARQ: case CC_OP_BMILGB ... CC_OP_BMILGQ: + case CC_OP_POPCNT: /* Z was going to be computed from the non-zero status of CC_DST. We can get that same Z value (and the new C value) by leaving CC_DST alone, setting CC_SRC, and using a CC_OP_SAR of the From 74f73c29187f8a464e0cea88e2ae43755b9961c4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 27 Jun 2024 12:52:25 +0200 Subject: [PATCH 17/23] target/i386: remove unused enum Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/tcg/translate.c | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 934c514e64..95bad55bf4 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -282,22 +282,6 @@ enum { JCC_LE, }; -enum { - /* I386 int registers */ - OR_EAX, /* MUST be even numbered */ - OR_ECX, - OR_EDX, - OR_EBX, - OR_ESP, - OR_EBP, - OR_ESI, - OR_EDI, - - OR_TMP0 = 16, /* temporary operand register */ - OR_TMP1, - OR_A0, /* temporary register used when doing address evaluation */ -}; - enum { USES_CC_DST = 1, USES_CC_SRC = 2, From 68c3aa3e97d843b080d5e445ff3a900f6f703471 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 26 Jun 2024 19:03:38 +0200 Subject: [PATCH 18/23] target/i386: SEV: rename sev_snp_guest->id_block Free the "id_block" name for the binary version of the data. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/sev.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index 30b83f1d77..6daa8c264c 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -152,7 +152,7 @@ struct SevSnpGuestState { /* configuration parameters */ char *guest_visible_workarounds; - char *id_block; + char *id_block_base64; char *id_auth; char *host_data; @@ -1296,7 +1296,7 @@ sev_snp_launch_finish(SevCommonState *sev_common) } } - trace_kvm_sev_snp_launch_finish(sev_snp->id_block, sev_snp->id_auth, + trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth, sev_snp->host_data); ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH, finish, &error); @@ -2159,7 +2159,7 @@ sev_snp_guest_get_id_block(Object *obj, Error **errp) { SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj); - return g_strdup(sev_snp_guest->id_block); + return g_strdup(sev_snp_guest->id_block_base64); } static void @@ -2170,14 +2170,14 @@ sev_snp_guest_set_id_block(Object *obj, const char *value, Error **errp) gsize len; finish->id_block_en = 0; - g_free(sev_snp_guest->id_block); + g_free(sev_snp_guest->id_block_base64); g_free((guchar *)finish->id_block_uaddr); /* store the base64 str so we don't need to re-encode in getter */ - sev_snp_guest->id_block = g_strdup(value); + sev_snp_guest->id_block_base64 = g_strdup(value); finish->id_block_uaddr = - (uint64_t)qbase64_decode(sev_snp_guest->id_block, -1, &len, errp); + (uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, &len, errp); if (!finish->id_block_uaddr) { return; From dd1b2fb554fb0ace09319e96b21e3b776eb7f5ba Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 26 Jun 2024 19:05:21 +0200 Subject: [PATCH 19/23] target/i386: SEV: store pointer to decoded id_block in SevSnpGuest Do not rely on finish->id_block_uaddr, so that there are no casts from pointer to uint64_t. They break on 32-bit hosts. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/sev.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index 6daa8c264c..2d4cfd41e8 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -153,6 +153,7 @@ struct SevSnpGuestState { /* configuration parameters */ char *guest_visible_workarounds; char *id_block_base64; + uint8_t *id_block; char *id_auth; char *host_data; @@ -2170,16 +2171,15 @@ sev_snp_guest_set_id_block(Object *obj, const char *value, Error **errp) gsize len; finish->id_block_en = 0; + g_free(sev_snp_guest->id_block); g_free(sev_snp_guest->id_block_base64); - g_free((guchar *)finish->id_block_uaddr); /* store the base64 str so we don't need to re-encode in getter */ sev_snp_guest->id_block_base64 = g_strdup(value); + sev_snp_guest->id_block = + qbase64_decode(sev_snp_guest->id_block_base64, -1, &len, errp); - finish->id_block_uaddr = - (uint64_t)qbase64_decode(sev_snp_guest->id_block_base64, -1, &len, errp); - - if (!finish->id_block_uaddr) { + if (!sev_snp_guest->id_block) { return; } @@ -2190,6 +2190,7 @@ sev_snp_guest_set_id_block(Object *obj, const char *value, Error **errp) } finish->id_block_en = 1; + finish->id_block_uaddr = (uintptr_t)sev_snp_guest->id_block; } static char * From 803b7718e6de87a3ecd5555da7d9fce6435c7db9 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 26 Jun 2024 19:03:38 +0200 Subject: [PATCH 20/23] target/i386: SEV: rename sev_snp_guest->id_auth Free the "id_auth" name for the binary version of the data. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/sev.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index 2d4cfd41e8..a6b063b762 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -154,7 +154,7 @@ struct SevSnpGuestState { char *guest_visible_workarounds; char *id_block_base64; uint8_t *id_block; - char *id_auth; + char *id_auth_base64; char *host_data; struct kvm_sev_snp_launch_start kvm_start_conf; @@ -1297,7 +1297,7 @@ sev_snp_launch_finish(SevCommonState *sev_common) } } - trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth, + trace_kvm_sev_snp_launch_finish(sev_snp->id_block_base64, sev_snp->id_auth_base64, sev_snp->host_data); ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_FINISH, finish, &error); @@ -2198,7 +2198,7 @@ sev_snp_guest_get_id_auth(Object *obj, Error **errp) { SevSnpGuestState *sev_snp_guest = SEV_SNP_GUEST(obj); - return g_strdup(sev_snp_guest->id_auth); + return g_strdup(sev_snp_guest->id_auth_base64); } static void @@ -2208,14 +2208,14 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value, Error **errp) struct kvm_sev_snp_launch_finish *finish = &sev_snp_guest->kvm_finish_conf; gsize len; - g_free(sev_snp_guest->id_auth); + g_free(sev_snp_guest->id_auth_base64); g_free((guchar *)finish->id_auth_uaddr); /* store the base64 str so we don't need to re-encode in getter */ - sev_snp_guest->id_auth = g_strdup(value); + sev_snp_guest->id_auth_base64 = g_strdup(value); finish->id_auth_uaddr = - (uint64_t)qbase64_decode(sev_snp_guest->id_auth, -1, &len, errp); + (uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, &len, errp); if (!finish->id_auth_uaddr) { return; From 1ab620bf3601a3c9e264031d80c7023690ddc2b4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 26 Jun 2024 19:05:21 +0200 Subject: [PATCH 21/23] target/i386: SEV: store pointer to decoded id_auth in SevSnpGuest Do not rely on finish->id_auth_uaddr, so that there are no casts from pointer to uint64_t. They break on 32-bit hosts. Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- target/i386/sev.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index a6b063b762..28d6bd3adf 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -155,6 +155,7 @@ struct SevSnpGuestState { char *id_block_base64; uint8_t *id_block; char *id_auth_base64; + uint8_t *id_auth; char *host_data; struct kvm_sev_snp_launch_start kvm_start_conf; @@ -2208,16 +2209,16 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value, Error **errp) struct kvm_sev_snp_launch_finish *finish = &sev_snp_guest->kvm_finish_conf; gsize len; + finish->id_auth_uaddr = 0; + g_free(sev_snp_guest->id_auth); g_free(sev_snp_guest->id_auth_base64); - g_free((guchar *)finish->id_auth_uaddr); /* store the base64 str so we don't need to re-encode in getter */ sev_snp_guest->id_auth_base64 = g_strdup(value); + sev_snp_guest->id_auth = + qbase64_decode(sev_snp_guest->id_auth_base64, -1, &len, errp); - finish->id_auth_uaddr = - (uint64_t)qbase64_decode(sev_snp_guest->id_auth_base64, -1, &len, errp); - - if (!finish->id_auth_uaddr) { + if (!sev_snp_guest->id_auth) { return; } @@ -2226,6 +2227,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value, Error **errp) len, KVM_SEV_SNP_ID_AUTH_SIZE); return; } + + finish->id_auth_uaddr = (uintptr_t)sev_snp_guest->id_auth; } static bool From cb61b174620abc41badf12b4caedb85c1747605c Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 26 Jun 2024 12:49:49 -0700 Subject: [PATCH 22/23] target/i386/sev: Use size_t for object sizes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This code was using both uint32_t and uint64_t for len. Consistently use size_t instead. Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Link: https://lore.kernel.org/r/20240626194950.1725800-3-richard.henderson@linaro.org Signed-off-by: Paolo Bonzini --- target/i386/sev.c | 16 ++++++++-------- target/i386/trace-events | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index 28d6bd3adf..0ffdf8952c 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -121,7 +121,7 @@ struct SevCommonStateClass { Error **errp); int (*launch_start)(SevCommonState *sev_common); void (*launch_finish)(SevCommonState *sev_common); - int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t *ptr, uint64_t len); + int (*launch_update_data)(SevCommonState *sev_common, hwaddr gpa, uint8_t *ptr, size_t len); int (*kvm_init)(ConfidentialGuestSupport *cgs, Error **errp); }; @@ -173,7 +173,7 @@ typedef struct SevLaunchUpdateData { QTAILQ_ENTRY(SevLaunchUpdateData) next; hwaddr gpa; void *hva; - uint64_t len; + size_t len; int type; } SevLaunchUpdateData; @@ -886,7 +886,7 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest, if (!data->hva || !data->len) { error_report("SNP_LAUNCH_UPDATE called with invalid address" - "/ length: %p / %lx", + "/ length: %p / %zx", data->hva, data->len); return 1; } @@ -945,7 +945,8 @@ out: } static int -sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa, uint8_t *addr, uint64_t len) +sev_launch_update_data(SevCommonState *sev_common, hwaddr gpa, + uint8_t *addr, size_t len) { int ret, fw_error; struct kvm_sev_launch_update_data update; @@ -1090,8 +1091,7 @@ sev_launch_finish(SevCommonState *sev_common) } static int -snp_launch_update_data(uint64_t gpa, void *hva, - uint32_t len, int type) +snp_launch_update_data(uint64_t gpa, void *hva, size_t len, int type) { SevLaunchUpdateData *data; @@ -1108,7 +1108,7 @@ snp_launch_update_data(uint64_t gpa, void *hva, static int sev_snp_launch_update_data(SevCommonState *sev_common, hwaddr gpa, - uint8_t *ptr, uint64_t len) + uint8_t *ptr, size_t len) { int ret = snp_launch_update_data(gpa, ptr, len, KVM_SEV_SNP_PAGE_TYPE_NORMAL); @@ -1165,7 +1165,7 @@ sev_snp_cpuid_info_fill(SnpCpuidInfo *snp_cpuid_info, } static int -snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, uint32_t cpuid_len) +snp_launch_update_cpuid(uint32_t cpuid_addr, void *hva, size_t cpuid_len) { KvmCpuidInfo kvm_cpuid_info = {0}; SnpCpuidInfo snp_cpuid_info; diff --git a/target/i386/trace-events b/target/i386/trace-events index 06b44ead2e..51301673f0 100644 --- a/target/i386/trace-events +++ b/target/i386/trace-events @@ -6,7 +6,7 @@ kvm_memcrypt_register_region(void *addr, size_t len) "addr %p len 0x%zx" kvm_memcrypt_unregister_region(void *addr, size_t len) "addr %p len 0x%zx" kvm_sev_change_state(const char *old, const char *new) "%s -> %s" kvm_sev_launch_start(int policy, void *session, void *pdh) "policy 0x%x session %p pdh %p" -kvm_sev_launch_update_data(void *addr, uint64_t len) "addr %p len 0x%" PRIx64 +kvm_sev_launch_update_data(void *addr, size_t len) "addr %p len 0x%zx" kvm_sev_launch_measurement(const char *value) "data %s" kvm_sev_launch_finish(void) "" kvm_sev_launch_secret(uint64_t hpa, uint64_t hva, uint64_t secret, int len) "hpa 0x%" PRIx64 " hva 0x%" PRIx64 " data 0x%" PRIx64 " len %d" From b31d386781cf85c193f3b1355dd0604cd6a59943 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 26 Jun 2024 12:49:50 -0700 Subject: [PATCH 23/23] target/i386/sev: Fix printf formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit hwaddr uses HWADDR_PRIx, sizeof yields size_t so uses %zu, and gsize uses G_GSIZE_FORMAT. Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Link: https://lore.kernel.org/r/20240626194950.1725800-4-richard.henderson@linaro.org Signed-off-by: Paolo Bonzini --- target/i386/sev.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/target/i386/sev.c b/target/i386/sev.c index 0ffdf8952c..3ab8b3c28b 100644 --- a/target/i386/sev.c +++ b/target/i386/sev.c @@ -934,8 +934,9 @@ sev_snp_launch_update(SevSnpGuestState *sev_snp_guest, out: if (!ret && update.gfn_start << TARGET_PAGE_BITS != data->gpa + data->len) { - error_report("SEV-SNP: expected update of GPA range %lx-%lx," - "got GPA range %lx-%llx", + error_report("SEV-SNP: expected update of GPA range %" + HWADDR_PRIx "-%" HWADDR_PRIx "," + "got GPA range %" HWADDR_PRIx "-%llx", data->gpa, data->gpa + data->len, data->gpa, update.gfn_start << TARGET_PAGE_BITS); ret = -EIO; @@ -2148,7 +2149,8 @@ sev_snp_guest_set_guest_visible_workarounds(Object *obj, const char *value, } if (len != sizeof(start->gosvw)) { - error_setg(errp, "parameter length of %lu exceeds max of %lu", + error_setg(errp, "parameter length of %" G_GSIZE_FORMAT + " exceeds max of %zu", len, sizeof(start->gosvw)); return; } @@ -2185,7 +2187,8 @@ sev_snp_guest_set_id_block(Object *obj, const char *value, Error **errp) } if (len != KVM_SEV_SNP_ID_BLOCK_SIZE) { - error_setg(errp, "parameter length of %lu not equal to %u", + error_setg(errp, "parameter length of %" G_GSIZE_FORMAT + " not equal to %u", len, KVM_SEV_SNP_ID_BLOCK_SIZE); return; } @@ -2223,7 +2226,8 @@ sev_snp_guest_set_id_auth(Object *obj, const char *value, Error **errp) } if (len > KVM_SEV_SNP_ID_AUTH_SIZE) { - error_setg(errp, "parameter length:ID_AUTH %lu exceeds max of %u", + error_setg(errp, "parameter length:ID_AUTH %" G_GSIZE_FORMAT + " exceeds max of %u", len, KVM_SEV_SNP_ID_AUTH_SIZE); return; } @@ -2291,7 +2295,8 @@ sev_snp_guest_set_host_data(Object *obj, const char *value, Error **errp) } if (len != sizeof(finish->host_data)) { - error_setg(errp, "parameter length of %lu not equal to %lu", + error_setg(errp, "parameter length of %" G_GSIZE_FORMAT + " not equal to %zu", len, sizeof(finish->host_data)); return; }