From nobody Fri May 17 08:24:58 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1553013699354196.6332213658551; Tue, 19 Mar 2019 09:41:39 -0700 (PDT) Received: from localhost ([127.0.0.1]:60101 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h6Hnw-0007ZL-CS for importer@patchew.org; Tue, 19 Mar 2019 12:41:32 -0400 Received: from eggs.gnu.org ([209.51.188.92]:37978) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h6Hie-0003wg-Sc for qemu-devel@nongnu.org; Tue, 19 Mar 2019 12:36:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h6Hic-0006ZU-TN for qemu-devel@nongnu.org; Tue, 19 Mar 2019 12:36:04 -0400 Received: from mx1.redhat.com ([209.132.183.28]:60328) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h6HiY-0006SL-1R; Tue, 19 Mar 2019 12:35:58 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 447D5308622F; Tue, 19 Mar 2019 16:35:57 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-132.ams2.redhat.com [10.36.116.132]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 85A8128543; Tue, 19 Mar 2019 16:35:53 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id F3A0311385FA; Tue, 19 Mar 2019 17:35:51 +0100 (CET) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Tue, 19 Mar 2019 17:35:50 +0100 Message-Id: <20190319163551.32499-2-armbru@redhat.com> In-Reply-To: <20190319163551.32499-1-armbru@redhat.com> References: <20190319163551.32499-1-armbru@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Tue, 19 Mar 2019 16:35:57 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v8 1/2] pflash: Require backend size to match device, improve errors X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-block@nongnu.org, alex.bennee@linaro.org, philmd@redhat.com, mreitz@redhat.com, lersek@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" We reject undersized backends with a rather enigmatic "failed to read the initial flash content" error. For instance: $ qemu-system-ppc64 -S -display none -M sam460ex -drive if=3Dpflash,for= mat=3Draw,file=3Deins.img qemu-system-ppc64: Initialization of device cfi.pflash02 failed: failed= to read the initial flash content We happily accept oversized images, ignoring their tail. Throwing away parts of firmware that way is pretty much certain to end in an even more enigmatic failure to boot. Require the backend's size to match the device's size exactly. Report mismatch like this: qemu-system-ppc64: Initialization of device cfi.pflash01 failed: device= requires 1048576 bytes, block backend provides 512 bytes Improve the error for actual read failures to "can't read block backend". To avoid duplicating even more code between the two pflash device models, do all that in new helper blk_check_size_and_read_all(). The error reporting can still be confusing. For instance: qemu-system-ppc64 -S -display none -M taihu -drive if=3Dpflash,format= =3Draw,file=3Deins.img -drive if=3Dpflash,unit=3D1,format=3Draw,file=3Dzwe= i.img qemu-system-ppc64: Initialization of device cfi.pflash02 failed: device= requires 2097152 bytes, block backend provides 512 bytes Leaves the user guessing which of the two -drive is wrong. Mention the issue in a TODO comment. Suggested-by: Alex Benn=C3=A9e Signed-off-by: Markus Armbruster Reviewed-by: Alex Benn=C3=A9e Reviewed-by: Laszlo Ersek Reviewed-by: Philippe Mathieu-Daud=C3=A9 --- hw/block/block.c | 48 +++++++++++++++++++++++++++++++++++++++- hw/block/pflash_cfi01.c | 8 +++---- hw/block/pflash_cfi02.c | 7 +++--- include/hw/block/block.h | 7 +++++- 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/hw/block/block.c b/hw/block/block.c index cf0eb826f1..bf56c7612b 100644 --- a/hw/block/block.c +++ b/hw/block/block.c @@ -13,7 +13,53 @@ #include "hw/block/block.h" #include "qapi/error.h" #include "qapi/qapi-types-block.h" -#include "qemu/error-report.h" + +/* + * Read the entire contents of @blk into @buf. + * @blk's contents must be @size bytes, and @size must be at most + * BDRV_REQUEST_MAX_BYTES. + * On success, return true. + * On failure, store an error through @errp and return false. + * Note that the error messages do not identify the block backend. + * TODO Since callers don't either, this can result in confusing + * errors. + * This function not intended for actual block devices, which read on + * demand. It's for things like memory devices that (ab)use a block + * backend to provide persistence. + */ +bool blk_check_size_and_read_all(BlockBackend *blk, void *buf, hwaddr size, + Error **errp) +{ + int64_t blk_len; + int ret; + + blk_len =3D blk_getlength(blk); + if (blk_len < 0) { + error_setg_errno(errp, -blk_len, + "can't get size of block backend"); + return false; + } + if (blk_len !=3D size) { + error_setg(errp, "device requires %" HWADDR_PRIu " bytes, " + "block backend provides %" PRIu64 " bytes", + size, blk_len); + return false; + } + + /* + * We could loop for @size > BDRV_REQUEST_MAX_BYTES, but if we + * ever get to the point we want to read *gigabytes* here, we + * should probably rework the device to be more like an actual + * block device and read only on demand. + */ + assert(size <=3D BDRV_REQUEST_MAX_BYTES); + ret =3D blk_pread(blk, 0, buf, size); + if (ret < 0) { + error_setg_errno(errp, -ret, "can't read block backend"); + return false; + } + return true; +} =20 void blkconf_blocksizes(BlockConf *conf) { diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 125f70b8e4..9937739a82 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -38,6 +38,7 @@ =20 #include "qemu/osdep.h" #include "hw/hw.h" +#include "hw/block/block.h" #include "hw/block/flash.h" #include "sysemu/block-backend.h" #include "qapi/error.h" @@ -763,12 +764,9 @@ static void pflash_cfi01_realize(DeviceState *dev, Err= or **errp) } =20 if (pfl->blk) { - /* read the initial flash content */ - ret =3D blk_pread(pfl->blk, 0, pfl->storage, total_len); - - if (ret < 0) { + if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, total_len, + errp)) { vmstate_unregister_ram(&pfl->mem, DEVICE(pfl)); - error_setg(errp, "failed to read the initial flash content"); return; } } diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index c9db430611..9b934305fa 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -37,6 +37,7 @@ =20 #include "qemu/osdep.h" #include "hw/hw.h" +#include "hw/block/block.h" #include "hw/block/flash.h" #include "qapi/error.h" #include "qemu/timer.h" @@ -581,11 +582,9 @@ static void pflash_cfi02_realize(DeviceState *dev, Err= or **errp) } =20 if (pfl->blk) { - /* read the initial flash content */ - ret =3D blk_pread(pfl->blk, 0, pfl->storage, chip_len); - if (ret < 0) { + if (!blk_check_size_and_read_all(pfl->blk, pfl->storage, chip_len, + errp)) { vmstate_unregister_ram(&pfl->orig_mem, DEVICE(pfl)); - error_setg(errp, "failed to read the initial flash content"); return; } } diff --git a/include/hw/block/block.h b/include/hw/block/block.h index e9f9e2223f..d06f25aa0f 100644 --- a/include/hw/block/block.h +++ b/include/hw/block/block.h @@ -11,7 +11,7 @@ #ifndef HW_BLOCK_H #define HW_BLOCK_H =20 -#include "qemu-common.h" +#include "exec/hwaddr.h" #include "qapi/qapi-types-block-core.h" =20 /* Configuration */ @@ -70,6 +70,11 @@ static inline unsigned int get_physical_block_exp(BlockC= onf *conf) DEFINE_PROP_BLOCKDEV_ON_ERROR("werror", _state, _conf.werror, \ BLOCKDEV_ON_ERROR_AUTO) =20 +/* Backend access helpers */ + +bool blk_check_size_and_read_all(BlockBackend *blk, void *buf, hwaddr size, + Error **errp); + /* Configuration helpers */ =20 bool blkconf_geometry(BlockConf *conf, int *trans, --=20 2.17.2 From nobody Fri May 17 08:24:58 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1553013591161809.4546219060915; Tue, 19 Mar 2019 09:39:51 -0700 (PDT) Received: from localhost ([127.0.0.1]:60055 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h6HmH-0006DU-UW for importer@patchew.org; Tue, 19 Mar 2019 12:39:50 -0400 Received: from eggs.gnu.org ([209.51.188.92]:37947) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1h6Hic-0003vY-Hg for qemu-devel@nongnu.org; Tue, 19 Mar 2019 12:36:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1h6Hib-0006XQ-6v for qemu-devel@nongnu.org; Tue, 19 Mar 2019 12:36:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34080) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1h6HiX-0006Ro-VT; Tue, 19 Mar 2019 12:35:58 -0400 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1D6978762E; Tue, 19 Mar 2019 16:35:57 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-116-132.ams2.redhat.com [10.36.116.132]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 88D0D28545; Tue, 19 Mar 2019 16:35:53 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 0888B113861C; Tue, 19 Mar 2019 17:35:52 +0100 (CET) From: Markus Armbruster To: qemu-devel@nongnu.org Date: Tue, 19 Mar 2019 17:35:51 +0100 Message-Id: <20190319163551.32499-3-armbru@redhat.com> In-Reply-To: <20190319163551.32499-1-armbru@redhat.com> References: <20190319163551.32499-1-armbru@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 19 Mar 2019 16:35:57 +0000 (UTC) Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH v8 2/2] pflash: Bury disabled code to limit device sizes X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, qemu-block@nongnu.org, alex.bennee@linaro.org, philmd@redhat.com, mreitz@redhat.com, lersek@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" Content-Type: text/plain; charset="utf-8" From: Alex Benn=C3=A9e We disabled code to limit device sizes to 8, 16, 32 or 64MiB more than a decade ago in commit 95d1f3edd5e and c8b153d7949, v0.9.1. Bury. Signed-off-by: Alex Benn=C3=A9e Reviewed-by: Laszlo Ersek [Extracted from a larger patch, extended to pflash_cfi02.c] Signed-off-by: Markus Armbruster --- hw/block/pflash_cfi01.c | 7 ------- hw/block/pflash_cfi02.c | 6 ------ 2 files changed, 13 deletions(-) diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c index 9937739a82..16dfae14b8 100644 --- a/hw/block/pflash_cfi01.c +++ b/hw/block/pflash_cfi01.c @@ -731,13 +731,6 @@ static void pflash_cfi01_realize(DeviceState *dev, Err= or **errp) } device_len =3D sector_len_per_device * blocks_per_device; =20 - /* XXX: to be fixed */ -#if 0 - if (total_len !=3D (8 * 1024 * 1024) && total_len !=3D (16 * 1024 * 10= 24) && - total_len !=3D (32 * 1024 * 1024) && total_len !=3D (64 * 1024 * 1= 024)) - return NULL; -#endif - memory_region_init_rom_device( &pfl->mem, OBJECT(dev), &pflash_cfi01_ops, diff --git a/hw/block/pflash_cfi02.c b/hw/block/pflash_cfi02.c index 9b934305fa..f2c6201f81 100644 --- a/hw/block/pflash_cfi02.c +++ b/hw/block/pflash_cfi02.c @@ -551,12 +551,6 @@ static void pflash_cfi02_realize(DeviceState *dev, Err= or **errp) } =20 chip_len =3D pfl->sector_len * pfl->nb_blocs; - /* XXX: to be fixed */ -#if 0 - if (total_len !=3D (8 * 1024 * 1024) && total_len !=3D (16 * 1024 * 10= 24) && - total_len !=3D (32 * 1024 * 1024) && total_len !=3D (64 * 1024 * 1= 024)) - return NULL; -#endif =20 memory_region_init_rom_device(&pfl->orig_mem, OBJECT(pfl), pfl->be ? &pflash_cfi02_ops_be : &pflash_cfi02_ops= _le, --=20 2.17.2