[PATCH v2 0/8] various fixes for 11.2 (gitlab, tests, char-fe, fdc, mailmap, deprecations) pre-PR
This is a rather random selection of fixes that didn't quite make it to 11.1 although we have some qemu-stable material in here. The CFLAGS tweak allow another target to be built and tested on MacOS. There are also riscv64 compilers which could be enabled via brew but I don't have time to look at those. The CFLAGS changes are also subsumed into Pierrick's check-tcg meson updates which might hit first. The char-fe clean-up removes a code smell of device models reaching into qemu_chr_fe_get_driver which is poor practice. The fdc fix resolves one of the many security reports for those that still care about floppy controllers in VMs. In my brief excursion into our confidential bugs I did look at fixing virtio-crypto but found it needs some serious love. Therefor I'm taking the other option of deprecating the module. Finally I'm experimenting with using b4's patch series handling so forgive any finger trouble on this series. I'll roll the PR on Wednesday. Alex. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- Changes in v2: - added mailmap patch - added fcd test case at review request (AI-used-for trailer) - tweaked virtio-crypto deprecation and commit message - Link to v1: https://lore.kernel.org/qemu-devel/20260819-fixes-for-11-2-v1-0-b7981c2cc371... To: qemu-devel@nongnu.org Cc: Philippe Mathieu-Daudé <philmd@mailo.com> Cc: Alex Bennée <alex.bennee@linaro.org> Cc: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Cc: Peter Maydell <peter.maydell@linaro.org> Cc: qemu-arm@nongnu.org Cc: "Michael S. Tsirkin" <mst@redhat.com> Cc: Stefano Garzarella <sgarzare@redhat.com> Cc: "Gonglei (Arei)" <arei.gonglei@huawei.com> Cc: zhenwei pi <zhenwei.pi@linux.dev> Cc: Laurent Vivier <lvivier@redhat.com> Cc: Amit Shah <amit@kernel.org> Cc: Marc-André Lureau <marcandre.lureau@redhat.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: John Snow <jsnow@redhat.com> Cc: Kevin Wolf <kwolf@redhat.com> Cc: Hanna Reitz <hreitz@redhat.com> Cc: qemu-block@nongnu.org Cc: Fabiano Rosas <farosas@suse.de> Cc: devel@lists.libvirt.org --- Alex Bennée (7): gitlab: use .base_meson_job_template for macOS jobs tests/arm: add explicit CFLAGS for system build gitlab: add arm-softmmu tests to the macOS build char-fe: implement qemu_chr_fe_backend_name hw/block: validate the fdc sector position fits within bounds tests/qtest: add test case for fdc sector overflow docs: deprecate virtio-crypto Denis V. Lunev (1): mailmap: map the last From-munged author back to a real identity docs/about/deprecated.rst | 18 +++ hw/block/fdc-internal.h | 1 + include/chardev/char-fe.h | 13 +- backends/cryptodev-vhost-user.c | 8 +- backends/rng-egd.c | 8 +- backends/vhost-user.c | 8 +- hw/block/fdc.c | 30 ++++- hw/char/imx_serial.c | 4 +- tests/qtest/fdc-test.c | 246 +++++++++++++++++++++++++++++++++- .gitlab-ci.d/macos.yml | 7 +- .mailmap | 1 + tests/tcg/arm/Makefile.softmmu-target | 2 +- 12 files changed, 307 insertions(+), 39 deletions(-) --- base-commit: 3375621e78660838b862d411c43f90e905d0b985 change-id: 20260819-fixes-for-11-2-f98744b82c12 Best regards, -- Alex Bennée <alex.bennee@linaro.org>
This way we'll get more environmental information which will help with debugging. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-2-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .gitlab-ci.d/macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.d/macos.yml b/.gitlab-ci.d/macos.yml index 641a48b3832..bdb3eae18f0 100644 --- a/.gitlab-ci.d/macos.yml +++ b/.gitlab-ci.d/macos.yml @@ -1,5 +1,5 @@ .macos_job_template: - extends: .base_job_template + extends: .base_meson_job_template stage: build tags: - saas-macos-large-m2pro -- 2.47.3
The brew toolchain on macOS (arm-none-eabi-gcc) is a little different from the default on Debian (arm-linux-gnueabihf-gcc) as a result it defaults to an older architectural baseline (without isb's), assumes softfloat and will get confused by including stdint.h without being told it is compiling freestanding code. Make these all explicit so we can build arm-softmmu tests on macOS. The test-armv6m-undef is unaffected as it has it's very own build stanza. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-3-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/tcg/arm/Makefile.softmmu-target | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/arm/Makefile.softmmu-target b/tests/tcg/arm/Makefile.softmmu-target index b66074b0b43..3e8a9b4073e 100644 --- a/tests/tcg/arm/Makefile.softmmu-target +++ b/tests/tcg/arm/Makefile.softmmu-target @@ -29,7 +29,7 @@ ARM_TESTS+=$(patsubst $(ARM_SRC)/%.c, %, $(ARM_TEST_SRCS)) CRT_PATH=$(ARM_SRC) LINK_SCRIPT=$(ARM_SRC)/kernel.ld LDFLAGS=-Wl,-T$(LINK_SCRIPT) -CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC) +CFLAGS+=-march=armv7-a+fp -ffreestanding -nostdlib -ggdb -O0 $(MINILIB_INC) LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc # building head blobs -- 2.47.3
Now we have fixed the compiler issues lets have at it. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-4-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .gitlab-ci.d/macos.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.d/macos.yml b/.gitlab-ci.d/macos.yml index bdb3eae18f0..40b9b637b3b 100644 --- a/.gitlab-ci.d/macos.yml +++ b/.gitlab-ci.d/macos.yml @@ -22,7 +22,7 @@ - export PKG_CONFIG_PATH="$PKG_CONFIG_PATH" - brew update - brew install $PKGS - - brew install aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc + - brew install arm-none-eabi-gcc aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc - if test -n "$PYPI_PKGS" ; then PYLIB=$($PYTHON -c 'import sysconfig; print(sysconfig.get_path("stdlib"))'); rm -f $PYLIB/EXTERNALLY-MANAGED; $PIP3 install --break-system-packages $PYPI_PKGS ; fi script: - mkdir build @@ -40,8 +40,9 @@ aarch64-macos-15-build: PATH_EXTRA: /opt/homebrew/gettext/bin PKG_CONFIG_PATH: /opt/homebrew/curl/lib/pkgconfig:/opt/homebrew/ncurses/lib/pkgconfig:/opt/homebrew/readline/lib/pkgconfig CONFIGURE_ARGS: - --target-list=aarch64-softmmu,i386-softmmu,x86_64-softmmu + --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,x86_64-softmmu --cross-prefix-aarch64=aarch64-elf- + --cross-prefix-arm=arm-none-eabi- --cross-prefix-i386=i686-elf- --cross-prefix-x86_64=x86_64-elf- --disable-plugins -- 2.47.3
On 24/8/26 17:24, Alex Bennée wrote:
Now we have fixed the compiler issues lets have at it.
The rational isn't clear about the benefit of adding this target. 32-bit is already covered by i686, ARM by aarch64.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-4-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .gitlab-ci.d/macos.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/.gitlab-ci.d/macos.yml b/.gitlab-ci.d/macos.yml index bdb3eae18f0..40b9b637b3b 100644 --- a/.gitlab-ci.d/macos.yml +++ b/.gitlab-ci.d/macos.yml @@ -22,7 +22,7 @@ - export PKG_CONFIG_PATH="$PKG_CONFIG_PATH" - brew update - brew install $PKGS - - brew install aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc + - brew install arm-none-eabi-gcc aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc - if test -n "$PYPI_PKGS" ; then PYLIB=$($PYTHON -c 'import sysconfig; print(sysconfig.get_path("stdlib"))'); rm -f $PYLIB/EXTERNALLY-MANAGED; $PIP3 install --break-system-packages $PYPI_PKGS ; fi script: - mkdir build @@ -40,8 +40,9 @@ aarch64-macos-15-build: PATH_EXTRA: /opt/homebrew/gettext/bin PKG_CONFIG_PATH: /opt/homebrew/curl/lib/pkgconfig:/opt/homebrew/ncurses/lib/pkgconfig:/opt/homebrew/readline/lib/pkgconfig CONFIGURE_ARGS: - --target-list=aarch64-softmmu,i386-softmmu,x86_64-softmmu + --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,x86_64-softmmu --cross-prefix-aarch64=aarch64-elf- + --cross-prefix-arm=arm-none-eabi- --cross-prefix-i386=i686-elf- --cross-prefix-x86_64=x86_64-elf- --disable-plugins
Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> writes:
On 24/8/26 17:24, Alex Bennée wrote:
Now we have fixed the compiler issues lets have at it.
The rational isn't clear about the benefit of adding this target. 32-bit is already covered by i686, ARM by aarch64.
The MacOS coverage is pretty woeful for all guests given the paucity of compilers available. I haven't done any in-depth measurement of how much extra coverage this gets but it will be some.
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-4-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .gitlab-ci.d/macos.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.d/macos.yml b/.gitlab-ci.d/macos.yml index bdb3eae18f0..40b9b637b3b 100644 --- a/.gitlab-ci.d/macos.yml +++ b/.gitlab-ci.d/macos.yml @@ -22,7 +22,7 @@ - export PKG_CONFIG_PATH="$PKG_CONFIG_PATH" - brew update - brew install $PKGS - - brew install aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc + - brew install arm-none-eabi-gcc aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc - if test -n "$PYPI_PKGS" ; then PYLIB=$($PYTHON -c 'import sysconfig; print(sysconfig.get_path("stdlib"))'); rm -f $PYLIB/EXTERNALLY-MANAGED; $PIP3 install --break-system-packages $PYPI_PKGS ; fi script: - mkdir build @@ -40,8 +40,9 @@ aarch64-macos-15-build: PATH_EXTRA: /opt/homebrew/gettext/bin PKG_CONFIG_PATH: /opt/homebrew/curl/lib/pkgconfig:/opt/homebrew/ncurses/lib/pkgconfig:/opt/homebrew/readline/lib/pkgconfig CONFIGURE_ARGS: - --target-list=aarch64-softmmu,i386-softmmu,x86_64-softmmu + --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,x86_64-softmmu --cross-prefix-aarch64=aarch64-elf- + --cross-prefix-arm=arm-none-eabi- --cross-prefix-i386=i686-elf- --cross-prefix-x86_64=x86_64-elf- --disable-plugins
-- Alex Bennée Virtualisation Tech Lead @ Linaro
A number of chardev users where calling qemu_chr_fe_get_driver() which is discouraged as it blocks backend hotswap. As the pattern is common and need for the string is transitory add a helper and keep qemu_chr_fe_get_driver() for those that really need the funkiness of poking around the backend setup. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-ID: <20260810153503.2652324-1-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - checkpatch fixes - r-b --- include/chardev/char-fe.h | 13 ++++++++++++- backends/cryptodev-vhost-user.c | 8 +------- backends/rng-egd.c | 8 +------- backends/vhost-user.c | 8 +------- hw/char/imx_serial.c | 4 ++-- 5 files changed, 17 insertions(+), 24 deletions(-) diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index 5f8a6df17dc..a82894e1d51 100644 --- a/include/chardev/char-fe.h +++ b/include/chardev/char-fe.h @@ -52,7 +52,8 @@ void qemu_chr_fe_deinit(CharFrontend *c, bool del); * associated Chardev. * Note: avoid this function as the driver should never be accessed directly, * especially by the frontends that support chardevice hotswap. - * Consider qemu_chr_fe_backend_connected() to check for driver existence + * Consider qemu_chr_fe_backend_connected() to check for driver + * existence or qemu_chr_fe_backend_name() if you need the name. */ Chardev *qemu_chr_fe_get_driver(CharFrontend *c); @@ -70,6 +71,16 @@ bool qemu_chr_fe_backend_connected(CharFrontend *c); */ bool qemu_chr_fe_backend_open(CharFrontend *c); +/** + * qemu_chr_fe_backend_name: + * + * Returns: caller freeable string or NULL + */ +static inline char *qemu_chr_fe_backend_name(CharFrontend *c) +{ + return (c->chr && c->chr->label) ? g_strdup(c->chr->label) : NULL; +} + /** * qemu_chr_fe_set_handlers_full: * @c: a CharFrontend diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c index cc478d9902d..3334e280a27 100644 --- a/backends/cryptodev-vhost-user.c +++ b/backends/cryptodev-vhost-user.c @@ -373,13 +373,7 @@ cryptodev_vhost_user_get_chardev(Object *obj, Error **errp) { CryptoDevBackendVhostUser *s = CRYPTODEV_BACKEND_VHOST_USER(obj); - Chardev *chr = qemu_chr_fe_get_driver(&s->chr); - - if (chr && chr->label) { - return g_strdup(chr->label); - } - - return NULL; + return qemu_chr_fe_backend_name(&s->chr); } static void cryptodev_vhost_user_finalize(Object *obj) diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 1d92bd71cf7..c2207f40c5c 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -126,13 +126,7 @@ static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp) static char *rng_egd_get_chardev(Object *obj, Error **errp) { RngEgd *s = RNG_EGD(obj); - Chardev *chr = qemu_chr_fe_get_driver(&s->chr); - - if (chr && chr->label) { - return g_strdup(chr->label); - } - - return NULL; + return qemu_chr_fe_backend_name(&s->chr); } static void rng_egd_finalize(Object *obj) diff --git a/backends/vhost-user.c b/backends/vhost-user.c index 380d8250233..46dadb74800 100644 --- a/backends/vhost-user.c +++ b/backends/vhost-user.c @@ -152,13 +152,7 @@ static void set_chardev(Object *obj, const char *value, Error **errp) static char *get_chardev(Object *obj, Error **errp) { VhostUserBackend *b = VHOST_USER_BACKEND(obj); - Chardev *chr = qemu_chr_fe_get_driver(&b->chr); - - if (chr && chr->label) { - return g_strdup(chr->label); - } - - return NULL; + return qemu_chr_fe_backend_name(&b->chr); } static void vhost_user_backend_class_init(ObjectClass *oc, const void *data) diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c index fb41ee2ac50..8ac4efbb5e9 100644 --- a/hw/char/imx_serial.c +++ b/hw/char/imx_serial.c @@ -278,10 +278,10 @@ static void imx_serial_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { IMXSerialState *s = (IMXSerialState *)opaque; - Chardev *chr = qemu_chr_fe_get_driver(&s->chr); + g_autofree char *label = qemu_chr_fe_backend_name(&s->chr); unsigned char ch; - trace_imx_serial_write(chr ? chr->label : "NODEV", offset, value); + trace_imx_serial_write(label ? label : "NODEV", offset, value); switch (offset >> 2) { case 0x10: /* UTXD */ -- 2.47.3
Previously the guest controlled the value of last_sec and wasn't particularly careful to check it didn't exceed the media size. We can't re-use drv->last_sec as that changes as we do operations so we set media_last_sect when the geometry is probed. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3800 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - checkpatch - move check in fdctrl_handle_format_track before we set data_state --- hw/block/fdc-internal.h | 1 + hw/block/fdc.c | 30 ++++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/hw/block/fdc-internal.h b/hw/block/fdc-internal.h index e219623dc7a..3a39ac903c7 100644 --- a/hw/block/fdc-internal.h +++ b/hw/block/fdc-internal.h @@ -83,6 +83,7 @@ typedef struct FDrive { uint8_t max_track; /* Nb of tracks */ uint16_t bps; /* Bytes per sector */ uint8_t ro; /* Is read-only */ + uint8_t media_last_sect; /* Probed sector per track*/ uint8_t media_changed; /* Is media changed */ uint8_t media_rate; /* Data rate of medium */ diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 1178b959a64..d20f749c1ab 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -192,6 +192,7 @@ static void fd_init(FDrive *drv) drv->max_track = 0; drv->ro = true; drv->media_changed = 1; + drv->media_last_sect = 0; } #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) @@ -373,6 +374,7 @@ static int pick_geometry(FDrive *drv) } drv->max_track = parse->max_track; drv->last_sect = parse->last_sect; + drv->media_last_sect = parse->last_sect; drv->disk = parse->drive; drv->media_rate = parse->rate; return 0; @@ -1905,6 +1907,17 @@ static void fdctrl_handle_partid(FDCtrl *fdctrl, int direction) fdctrl_to_result_phase(fdctrl, 1); } +static bool fd_validate_last_sect(FDrive *drv, uint8_t new_last_sect_val) +{ + if (drv->media_validated && new_last_sect_val > drv->media_last_sect) { + qemu_log_mask(LOG_GUEST_ERROR, + "FDC: Guest attempted to set last_sect to %u, exceeding valid media max of %u\n", + new_last_sect_val, drv->media_last_sect); + return false; + } + return true; +} + static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction) { FDrive *cur_drv = get_cur_drv(fdctrl); @@ -1919,6 +1932,10 @@ static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction) /* timers */ fdctrl->timer0 = fdctrl->fifo[7]; fdctrl->timer1 = fdctrl->fifo[8]; + if (!fd_validate_last_sect(cur_drv, fdctrl->fifo[9])) { + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00); + return; + } cur_drv->last_sect = fdctrl->fifo[9]; fdctrl->lock = fdctrl->fifo[10] >> 7; cur_drv->perpendicular = (fdctrl->fifo[10] >> 2) & 0xF; @@ -1976,6 +1993,12 @@ static void fdctrl_handle_format_track(FDCtrl *fdctrl, int direction) SET_CUR_DRV(fdctrl, fdctrl->fifo[1] & FD_DOR_SELMASK); cur_drv = get_cur_drv(fdctrl); + + if (!fd_validate_last_sect(cur_drv, fdctrl->fifo[3])) { + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00); + return; + } + fdctrl->data_state |= FD_STATE_FORMAT; if (fdctrl->fifo[0] & 0x80) fdctrl->data_state |= FD_STATE_MULTI; @@ -1983,13 +2006,8 @@ static void fdctrl_handle_format_track(FDCtrl *fdctrl, int direction) fdctrl->data_state &= ~FD_STATE_MULTI; cur_drv->bps = fdctrl->fifo[2] > 7 ? 16384 : 128 << fdctrl->fifo[2]; -#if 0 - cur_drv->last_sect = - cur_drv->flags & FDISK_DBL_SIDES ? fdctrl->fifo[3] : - fdctrl->fifo[3] / 2; -#else + cur_drv->last_sect = fdctrl->fifo[3]; -#endif /* TODO: implement format using DMA expected by the Bochs BIOS * and Linux fdformat (read 3 bytes per sector via DMA and fill * the sector with the specified fill byte -- 2.47.3
Add a test case for #3800 where we check that the last sector is properly bounded by the physical media that is inserted. Link: https://gitlab.com/qemu-project/qemu/-/issues/3800 AI-used-for: initial draft of test Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/qtest/fdc-test.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 241 insertions(+), 5 deletions(-) diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 1e1dd8659d0..3f2642fe671 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -31,7 +31,8 @@ #define DRIVE_FLOPPY_BLANK \ "-drive if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k" -#define TEST_IMAGE_SIZE 1440 * 1024 +#define TEST_IMAGE_1440KB (1440 * 1024) +#define TEST_IMAGE_720KB (720 * 1024) #define FLOPPY_BASE 0x3f0 #define FLOPPY_IRQ 6 @@ -49,8 +50,11 @@ enum { enum { CMD_SENSE_INT = 0x08, CMD_READ_ID = 0x0a, + CMD_FORMAT_TRACK = 0x4d, CMD_SEEK = 0x0f, CMD_VERIFY = 0x16, + CMD_SAVE = 0x2e, + CMD_RESTORE = 0x4e, CMD_READ = 0xe6, CMD_RELATIVE_SEEK_OUT = 0x8f, CMD_RELATIVE_SEEK_IN = 0xcf, @@ -69,9 +73,11 @@ enum { ST0_IC_ABNTERM = 0x40, /* abnormal termination */ ST1_MA = 0x01, /* missing address mark */ + ST1_EC = 0x80, /* end of cylinder / sector past last_sect */ }; static char *test_image; +static char *test_image_720k; #define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask)) #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0) @@ -276,12 +282,17 @@ static void test_cmos(void) g_assert(cmos == 0x40 || cmos == 0x50); } -static void media_insert(void) +static void media_insert_path(const char *path) { qtest_qmp_assert_success(global_qtest, "{'execute':'blockdev-change-medium', 'arguments':{" " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", - test_image); + path); +} + +static void media_insert(void) +{ + media_insert_path(test_image); } static void media_eject(void) @@ -586,6 +597,222 @@ static void test_verify(void) g_assert(ret == 0); } +/* + * Query cur_drv->last_sect using the SAVE command (CMD_SAVE, 0x2e). + * Byte 8 of the 15 result bytes returned by CMD_SAVE holds last_sect. + */ +static uint8_t get_lastsect(void) +{ + uint8_t res[15]; + int i; + + floppy_send(CMD_SAVE); + for (i = 0; i < 15; i++) { + res[i] = floppy_recv(); + } + return res[8]; +} + +/* + * Attempt to set cur_drv->last_sect directly using the RESTORE command + * (CMD_RESTORE, 0x4e). + * While the 82078 datasheet describes RESTORE for restoring a previously + * saved state, a guest can issue raw RESTORE commands with arbitrary + * parameters without having issued SAVE. Parameter byte 9 is used by the + * controller to restore cur_drv->last_sect. + */ +static void fake_lastsect(uint8_t last_sect) +{ + floppy_send(CMD_RESTORE); + floppy_send(0); /* fifo[1] */ + floppy_send(0); /* fifo[2] */ + floppy_send(0); /* fifo[3]: drv0 track */ + floppy_send(0); /* fifo[4]: drv1 track */ + floppy_send(0); /* fifo[5]: drv2 track */ + floppy_send(0); /* fifo[6]: drv3 track */ + floppy_send(0); /* fifo[7]: timer0 */ + floppy_send(0); /* fifo[8]: timer1 */ + floppy_send(last_sect); /* fifo[9]: last_sect */ + floppy_send(0); /* fifo[10]: lock/perpendicular */ + floppy_send(0); /* fifo[11]: config */ + floppy_send(0); /* fifo[12]: precomp_trk */ + floppy_send(0); /* fifo[13]: pwrd */ + floppy_send(0); /* fifo[14] */ + floppy_send(0); /* fifo[15] */ + floppy_send(0); /* fifo[16] */ + floppy_send(0); /* fifo[17] */ +} + +static void send_format_track(uint8_t drive, uint8_t head, uint8_t last_sect, + uint8_t *st0_out, uint8_t *st1_out) +{ + uint8_t st0, st1; + + floppy_send(CMD_FORMAT_TRACK); + floppy_send((head << 2) | drive); + floppy_send(2); /* 512 bytes per sector */ + floppy_send(last_sect); /* sectors per track */ + floppy_send(0x1b); /* GAP length */ + floppy_send(0x00); /* filler byte */ + + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* st2 */ + floppy_recv(); /* track */ + floppy_recv(); /* head */ + floppy_recv(); /* sect */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sz */ + g_assert(!get_irq(FLOPPY_IRQ)); + + if (st0_out) { + *st0_out = st0; + } + if (st1_out) { + *st1_out = st1; + } +} + +/* + * Test that guest cannot set last_sect beyond the probed media size + * via RESTORE or FORMAT TRACK commands (gitlab issue #3800). + */ +static void test_last_sect_bounds(void) +{ + uint8_t st0, st1; + + /* Start with 1.44 MB media inserted (last_sect = 18) */ + media_insert(); + send_seek(1); + send_seek(0); + + /* Valid last_sect values (<= 18) should succeed */ + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + fake_lastsect(9); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 9); + + /* Restoring to the default 18 */ + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + /* Invalid last_sect value (> 18) must fail */ + fake_lastsect(19); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* st2 */ + floppy_recv(); /* track */ + floppy_recv(); /* head */ + floppy_recv(); /* sect */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sz */ + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Verify last_sect was not changed to 19 */ + g_assert_cmpint(get_lastsect(), ==, 18); + + /* FORMAT TRACK with valid last_sect (18) should succeed */ + send_format_track(0, 0, 18, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, 0); + g_assert_cmpint(st1, ==, 0); + + /* FORMAT TRACK with invalid last_sect (19) must fail */ + send_format_track(0, 0, 19, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Change media to 720 kB floppy (last_sect = 9) */ + media_eject(); + media_insert_path(test_image_720k); + send_seek(1); + send_seek(0); + + /* Probed geometry now has last_sect = 9 */ + fake_lastsect(9); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 9); + + /* Values exceeding 9 (e.g. 10 or 18) must now fail */ + fake_lastsect(10); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + fake_lastsect(18); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* FORMAT TRACK on 720 kB floppy */ + send_format_track(0, 0, 9, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, 0); + g_assert_cmpint(st1, ==, 0); + + send_format_track(0, 0, 10, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + send_format_track(0, 0, 18, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* + * Change back to 1.44 MB floppy and verify last_sect = 18 is allowed + * again. + */ + media_eject(); + media_insert(); + send_seek(1); + send_seek(0); + + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + fake_lastsect(19); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Leave drive empty */ + media_eject(); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -661,10 +888,16 @@ int main(int argc, char **argv) int fd; int ret; - /* Create a temporary raw image */ + /* Create temporary raw images */ fd = g_file_open_tmp("qtest.XXXXXX", &test_image, NULL); g_assert(fd >= 0); - ret = ftruncate(fd, TEST_IMAGE_SIZE); + ret = ftruncate(fd, TEST_IMAGE_1440KB); + g_assert(ret == 0); + close(fd); + + fd = g_file_open_tmp("qtest720.XXXXXX", &test_image_720k, NULL); + g_assert(fd >= 0); + ret = ftruncate(fd, TEST_IMAGE_720KB); g_assert(ret == 0); close(fd); @@ -686,6 +919,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/read_no_dma_1", test_read_no_dma_1); qtest_add_func("/fdc/read_no_dma_18", test_read_no_dma_18); qtest_add_func("/fdc/read_no_dma_19", test_read_no_dma_19); + qtest_add_func("/fdc/last_sect_bounds", test_last_sect_bounds); qtest_add_func("/fdc/fuzz-registers", fuzz_registers); qtest_add_func("/fdc/fuzz/cve_2021_20196", test_cve_2021_20196); qtest_add_func("/fdc/fuzz/cve_2021_3507", test_cve_2021_3507); @@ -696,6 +930,8 @@ int main(int argc, char **argv) qtest_end(); unlink(test_image); g_free(test_image); + unlink(test_image_720k); + g_free(test_image_720k); return ret; } -- 2.47.3
On Mon, Aug 24, 2026 at 04:24:53PM +0100, Alex Bennée wrote:
Add a test case for #3800 where we check that the last sector is properly bounded by the physical media that is inserted.
Link: https://gitlab.com/qemu-project/qemu/-/issues/3800 AI-used-for: initial draft of test
Wasn't the change in AI policy still pending a new patch proposal from Paolo ?
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/qtest/fdc-test.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 241 insertions(+), 5 deletions(-)
diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 1e1dd8659d0..3f2642fe671 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -31,7 +31,8 @@ #define DRIVE_FLOPPY_BLANK \ "-drive if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k"
-#define TEST_IMAGE_SIZE 1440 * 1024 +#define TEST_IMAGE_1440KB (1440 * 1024) +#define TEST_IMAGE_720KB (720 * 1024)
#define FLOPPY_BASE 0x3f0 #define FLOPPY_IRQ 6 @@ -49,8 +50,11 @@ enum { enum { CMD_SENSE_INT = 0x08, CMD_READ_ID = 0x0a, + CMD_FORMAT_TRACK = 0x4d, CMD_SEEK = 0x0f, CMD_VERIFY = 0x16, + CMD_SAVE = 0x2e, + CMD_RESTORE = 0x4e, CMD_READ = 0xe6, CMD_RELATIVE_SEEK_OUT = 0x8f, CMD_RELATIVE_SEEK_IN = 0xcf, @@ -69,9 +73,11 @@ enum { ST0_IC_ABNTERM = 0x40, /* abnormal termination */
ST1_MA = 0x01, /* missing address mark */ + ST1_EC = 0x80, /* end of cylinder / sector past last_sect */ };
static char *test_image; +static char *test_image_720k;
#define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask)) #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0) @@ -276,12 +282,17 @@ static void test_cmos(void) g_assert(cmos == 0x40 || cmos == 0x50); }
-static void media_insert(void) +static void media_insert_path(const char *path) { qtest_qmp_assert_success(global_qtest, "{'execute':'blockdev-change-medium', 'arguments':{" " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", - test_image); + path); +} + +static void media_insert(void) +{ + media_insert_path(test_image); }
static void media_eject(void) @@ -586,6 +597,222 @@ static void test_verify(void) g_assert(ret == 0); }
+/* + * Query cur_drv->last_sect using the SAVE command (CMD_SAVE, 0x2e). + * Byte 8 of the 15 result bytes returned by CMD_SAVE holds last_sect. + */ +static uint8_t get_lastsect(void) +{ + uint8_t res[15]; + int i; + + floppy_send(CMD_SAVE); + for (i = 0; i < 15; i++) { + res[i] = floppy_recv(); + } + return res[8]; +} + +/* + * Attempt to set cur_drv->last_sect directly using the RESTORE command + * (CMD_RESTORE, 0x4e). + * While the 82078 datasheet describes RESTORE for restoring a previously + * saved state, a guest can issue raw RESTORE commands with arbitrary + * parameters without having issued SAVE. Parameter byte 9 is used by the + * controller to restore cur_drv->last_sect. + */ +static void fake_lastsect(uint8_t last_sect) +{ + floppy_send(CMD_RESTORE); + floppy_send(0); /* fifo[1] */ + floppy_send(0); /* fifo[2] */ + floppy_send(0); /* fifo[3]: drv0 track */ + floppy_send(0); /* fifo[4]: drv1 track */ + floppy_send(0); /* fifo[5]: drv2 track */ + floppy_send(0); /* fifo[6]: drv3 track */ + floppy_send(0); /* fifo[7]: timer0 */ + floppy_send(0); /* fifo[8]: timer1 */ + floppy_send(last_sect); /* fifo[9]: last_sect */ + floppy_send(0); /* fifo[10]: lock/perpendicular */ + floppy_send(0); /* fifo[11]: config */ + floppy_send(0); /* fifo[12]: precomp_trk */ + floppy_send(0); /* fifo[13]: pwrd */ + floppy_send(0); /* fifo[14] */ + floppy_send(0); /* fifo[15] */ + floppy_send(0); /* fifo[16] */ + floppy_send(0); /* fifo[17] */ +} + +static void send_format_track(uint8_t drive, uint8_t head, uint8_t last_sect, + uint8_t *st0_out, uint8_t *st1_out) +{ + uint8_t st0, st1; + + floppy_send(CMD_FORMAT_TRACK); + floppy_send((head << 2) | drive); + floppy_send(2); /* 512 bytes per sector */ + floppy_send(last_sect); /* sectors per track */ + floppy_send(0x1b); /* GAP length */ + floppy_send(0x00); /* filler byte */ + + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* st2 */ + floppy_recv(); /* track */ + floppy_recv(); /* head */ + floppy_recv(); /* sect */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sz */ + g_assert(!get_irq(FLOPPY_IRQ)); + + if (st0_out) { + *st0_out = st0; + } + if (st1_out) { + *st1_out = st1; + } +} + +/* + * Test that guest cannot set last_sect beyond the probed media size + * via RESTORE or FORMAT TRACK commands (gitlab issue #3800). + */ +static void test_last_sect_bounds(void) +{ + uint8_t st0, st1; + + /* Start with 1.44 MB media inserted (last_sect = 18) */ + media_insert(); + send_seek(1); + send_seek(0); + + /* Valid last_sect values (<= 18) should succeed */ + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + fake_lastsect(9); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 9); + + /* Restoring to the default 18 */ + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + /* Invalid last_sect value (> 18) must fail */ + fake_lastsect(19); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* st2 */ + floppy_recv(); /* track */ + floppy_recv(); /* head */ + floppy_recv(); /* sect */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sz */ + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Verify last_sect was not changed to 19 */ + g_assert_cmpint(get_lastsect(), ==, 18); + + /* FORMAT TRACK with valid last_sect (18) should succeed */ + send_format_track(0, 0, 18, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, 0); + g_assert_cmpint(st1, ==, 0); + + /* FORMAT TRACK with invalid last_sect (19) must fail */ + send_format_track(0, 0, 19, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Change media to 720 kB floppy (last_sect = 9) */ + media_eject(); + media_insert_path(test_image_720k); + send_seek(1); + send_seek(0); + + /* Probed geometry now has last_sect = 9 */ + fake_lastsect(9); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 9); + + /* Values exceeding 9 (e.g. 10 or 18) must now fail */ + fake_lastsect(10); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + fake_lastsect(18); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* FORMAT TRACK on 720 kB floppy */ + send_format_track(0, 0, 9, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, 0); + g_assert_cmpint(st1, ==, 0); + + send_format_track(0, 0, 10, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + send_format_track(0, 0, 18, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* + * Change back to 1.44 MB floppy and verify last_sect = 18 is allowed + * again. + */ + media_eject(); + media_insert(); + send_seek(1); + send_seek(0); + + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + fake_lastsect(19); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Leave drive empty */ + media_eject(); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -661,10 +888,16 @@ int main(int argc, char **argv) int fd; int ret;
- /* Create a temporary raw image */ + /* Create temporary raw images */ fd = g_file_open_tmp("qtest.XXXXXX", &test_image, NULL); g_assert(fd >= 0); - ret = ftruncate(fd, TEST_IMAGE_SIZE); + ret = ftruncate(fd, TEST_IMAGE_1440KB); + g_assert(ret == 0); + close(fd); + + fd = g_file_open_tmp("qtest720.XXXXXX", &test_image_720k, NULL); + g_assert(fd >= 0); + ret = ftruncate(fd, TEST_IMAGE_720KB); g_assert(ret == 0); close(fd);
@@ -686,6 +919,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/read_no_dma_1", test_read_no_dma_1); qtest_add_func("/fdc/read_no_dma_18", test_read_no_dma_18); qtest_add_func("/fdc/read_no_dma_19", test_read_no_dma_19); + qtest_add_func("/fdc/last_sect_bounds", test_last_sect_bounds); qtest_add_func("/fdc/fuzz-registers", fuzz_registers); qtest_add_func("/fdc/fuzz/cve_2021_20196", test_cve_2021_20196); qtest_add_func("/fdc/fuzz/cve_2021_3507", test_cve_2021_3507); @@ -696,6 +930,8 @@ int main(int argc, char **argv) qtest_end(); unlink(test_image); g_free(test_image); + unlink(test_image_720k); + g_free(test_image_720k);
return ret; }
-- 2.47.3
With regards, Daniel -- |: https://berrange.com ~~ https://hachyderm.io/@berrange :| |: https://libvirt.org ~~ https://entangle-photo.org :| |: https://pixelfed.art/berrange ~~ https://fstop138.berrange.com :|
Daniel P. Berrangé <berrange@redhat.com> writes:
On Mon, Aug 24, 2026 at 04:24:53PM +0100, Alex Bennée wrote:
Add a test case for #3800 where we check that the last sector is properly bounded by the physical media that is inserted.
Link: https://gitlab.com/qemu-project/qemu/-/issues/3800 AI-used-for: initial draft of test
Wasn't the change in AI policy still pending a new patch proposal from Paolo ?
Yes it is, however given I was rushed and am not super familiar with the fdc qtest code I took a shortcut to get something posted. FWIW it's mostly as the original prompt created apart from: - I added the TEST_IMAGE_xxx changes - I suggested fake_lastsect/fake_getsect naming over the original suggestion - I asked it to expand the comments on using RESTORE/SAVE after consulting the datasheet myself when reviewing the code Think of it as a road test for the proposal...
<snip>
-#define TEST_IMAGE_SIZE 1440 * 1024 +#define TEST_IMAGE_1440KB (1440 * 1024) +#define TEST_IMAGE_720KB (720 * 1024)
<snip>
+/* + * Query cur_drv->last_sect using the SAVE command (CMD_SAVE, 0x2e). + * Byte 8 of the 15 result bytes returned by CMD_SAVE holds last_sect. + */ +static uint8_t get_lastsect(void) +{ + uint8_t res[15]; + int i; + + floppy_send(CMD_SAVE); + for (i = 0; i < 15; i++) { + res[i] = floppy_recv(); + } + return res[8]; +} + +/* + * Attempt to set cur_drv->last_sect directly using the RESTORE command + * (CMD_RESTORE, 0x4e). + * While the 82078 datasheet describes RESTORE for restoring a previously + * saved state, a guest can issue raw RESTORE commands with arbitrary + * parameters without having issued SAVE. Parameter byte 9 is used by the + * controller to restore cur_drv->last_sect. + */ +static void fake_lastsect(uint8_t last_sect) +{ + floppy_send(CMD_RESTORE); + floppy_send(0); /* fifo[1] */ + floppy_send(0); /* fifo[2] */ + floppy_send(0); /* fifo[3]: drv0 track */ + floppy_send(0); /* fifo[4]: drv1 track */ + floppy_send(0); /* fifo[5]: drv2 track */ + floppy_send(0); /* fifo[6]: drv3 track */ + floppy_send(0); /* fifo[7]: timer0 */ + floppy_send(0); /* fifo[8]: timer1 */ + floppy_send(last_sect); /* fifo[9]: last_sect */ + floppy_send(0); /* fifo[10]: lock/perpendicular */ + floppy_send(0); /* fifo[11]: config */ + floppy_send(0); /* fifo[12]: precomp_trk */ + floppy_send(0); /* fifo[13]: pwrd */ + floppy_send(0); /* fifo[14] */ + floppy_send(0); /* fifo[15] */ + floppy_send(0); /* fifo[16] */ + floppy_send(0); /* fifo[17] */ +} +
<snip>
- /* Create a temporary raw image */ + /* Create temporary raw images */ fd = g_file_open_tmp("qtest.XXXXXX", &test_image, NULL); g_assert(fd >= 0); - ret = ftruncate(fd, TEST_IMAGE_SIZE); + ret = ftruncate(fd, TEST_IMAGE_1440KB); + g_assert(ret == 0); + close(fd); + + fd = g_file_open_tmp("qtest720.XXXXXX", &test_image_720k, NULL); + g_assert(fd >= 0); + ret = ftruncate(fd, TEST_IMAGE_720KB); g_assert(ret == 0); close(fd);
<snip>
-- Alex Bennée Virtualisation Tech Lead @ Linaro
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just the deprecate the sub-system with a view to removing it in a couple of cycles. We can also remove the backend code at that point as well. Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - mention the backend bits (only used by virtio-crypto) - use Gonglei's modified version of the text --- docs/about/deprecated.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..05e4ce8cf16 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,24 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919. +Devices +------- + +``virtio-crypto`` and cryptodev backends (since 11.2) +''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable, so it is +unlikely to be used in any serious virtualization setting. Modern +ISAs provide on-CPU cryptography instructions (e.g. AES-NI/VAES, +armv8 crypto extensions), and the Linux kernel deprecated AF_ALG +and dropped its off-CPU accelerator support in Linux 7.2, removing +the primary userspace path for off-CPU crypto acceleration. The +time for this sort of off-load has passed. + +As this is the only device that uses the cryptodev backends these will +be removed at the same time the ``virtio-crypto`` device is. + Device options -------------- -- 2.47.3
On Mon, Aug 24, 2026 at 04:24:54PM +0100, Alex Bennée wrote:
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just the deprecate the sub-system with a view to removing it in a couple of cycles.
We can also remove the backend code at that point as well.
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com> I am assuming it will be merged with the rest of it?
--- v2 - mention the backend bits (only used by virtio-crypto) - use Gonglei's modified version of the text --- docs/about/deprecated.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..05e4ce8cf16 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,24 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919.
+Devices +------- + +``virtio-crypto`` and cryptodev backends (since 11.2) +''''''''''''''''''''''''''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable, so it is +unlikely to be used in any serious virtualization setting. Modern +ISAs provide on-CPU cryptography instructions (e.g. AES-NI/VAES, +armv8 crypto extensions), and the Linux kernel deprecated AF_ALG +and dropped its off-CPU accelerator support in Linux 7.2, removing +the primary userspace path for off-CPU crypto acceleration. The +time for this sort of off-load has passed. + +As this is the only device that uses the cryptodev backends these will +be removed at the same time the ``virtio-crypto`` device is. + Device options --------------
-- 2.47.3
From: "Denis V. Lunev" <den@openvz.org> qemu-block@nongnu.org rewrote the From: header of commit b67e353863 ("block: drop force_dup parameter of raw_reconfigure_getfd()") for DMARC reasons, so it landed in git attributed to the list instead of to its author, whose real address is in the Signed-off-by trailer. Map it like the other munged identities collected in that section. This was the last one left unmapped, so 'git log --format="%aN"' no longer reports any author 'via' a list. Signed-off-by: Denis V. Lunev <den@openvz.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com> Message-ID: <20260812112456.970371-1-den@openvz.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index e5dc644623f..08f0fa9e118 100644 --- a/.mailmap +++ b/.mailmap @@ -68,6 +68,7 @@ Stefan Weil <sw@weilnetz.de> Stefan Weil via <qemu-devel@nongnu.org> Stefan Weil <sw@weilnetz.de> Stefan Weil via <qemu-trivial@nongnu.org> Andrey Drobyshev <andrey.drobyshev@virtuozzo.com> Andrey Drobyshev via <qemu-block@nongnu.org> BALATON Zoltan <balaton@eik.bme.hu> BALATON Zoltan via <qemu-ppc@nongnu.org> +Denis V. Lunev <den@openvz.org> Denis V. Lunev via <qemu-block@nongnu.org> # Next, replace old addresses by a more recent one. Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> <akihiko.odaki@daynix.com> -- 2.47.3
participants (4)
-
Alex Bennée -
Daniel P. Berrangé -
Michael S. Tsirkin -
Philippe Mathieu-Daudé