[Qemu-devel] [PATCH v2 02/11] pflash_cfi01: Do not exit() on guest aborting "write to buffer"

Markus Armbruster posted 11 patches 6 years, 8 months ago
Maintainers: Aleksandar Markovic <amarkovic@wavecomp.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Andrzej Zaborowski <balrogg@gmail.com>, Kevin Wolf <kwolf@redhat.com>, Jan Kiszka <jan.kiszka@web.de>, Aleksandar Rikalo <arikalo@wavecomp.com>, Peter Maydell <peter.maydell@linaro.org>, Max Filippov <jcmvbkbc@gmail.com>, Richard Henderson <rth@twiddle.net>, Alistair Francis <alistair@alistair23.me>, BALATON Zoltan <balaton@eik.bme.hu>, Paolo Bonzini <pbonzini@redhat.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Michael Walle <michael@walle.cc>, Max Reitz <mreitz@redhat.com>, Magnus Damm <magnus.damm@gmail.com>, Aurelien Jarno <aurelien@aurel32.net>, David Gibson <david@gibson.dropbear.id.au>, Antony Pavlov <antonynpavlov@gmail.com>, "Michael S. Tsirkin" <mst@redhat.com>, Eduardo Habkost <ehabkost@redhat.com>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
There is a newer version of this series
[Qemu-devel] [PATCH v2 02/11] pflash_cfi01: Do not exit() on guest aborting "write to buffer"
Posted by Markus Armbruster 6 years, 8 months ago
When a guest tries to abort "write to buffer" (command 0xE8), we print
"PFLASH: Possible BUG - Write block confirm", then exit(1).  Letting
the guest terminate QEMU is not a good idea.  Instead, LOG_UNIMP we
screwed up, then reset the device.

Macro PFLASH_BUG() is now unused; delete it.

Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 hw/block/pflash_cfi01.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
index a51ac9f399..e6d933a06d 100644
--- a/hw/block/pflash_cfi01.c
+++ b/hw/block/pflash_cfi01.c
@@ -49,12 +49,6 @@
 #include "sysemu/sysemu.h"
 #include "trace.h"
 
-#define PFLASH_BUG(fmt, ...) \
-do { \
-    fprintf(stderr, "PFLASH: Possible BUG - " fmt, ## __VA_ARGS__); \
-    exit(1); \
-} while(0)
-
 /* #define PFLASH_DEBUG */
 #ifdef PFLASH_DEBUG
 #define DPRINTF(fmt, ...)                                   \
@@ -623,8 +617,11 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
                 pfl->wcycle = 0;
                 pfl->status |= 0x80;
             } else {
-                DPRINTF("%s: unknown command for \"write block\"\n", __func__);
-                PFLASH_BUG("Write block confirm");
+                qemu_log_mask(LOG_UNIMP,
+                    "%s: Aborting write to buffer not implemented,"
+                    " the data is already written to storage!\n"
+                    "Flash device reset into READ mode.\n",
+                    __func__);
                 goto reset_flash;
             }
             break;
-- 
2.17.2


Re: [Qemu-devel] [PATCH v2 02/11] pflash_cfi01: Do not exit() on guest aborting "write to buffer"
Posted by Philippe Mathieu-Daudé 6 years, 8 months ago
On 2/26/19 8:33 PM, Markus Armbruster wrote:
> When a guest tries to abort "write to buffer" (command 0xE8), we print
> "PFLASH: Possible BUG - Write block confirm", then exit(1).  Letting
> the guest terminate QEMU is not a good idea.  Instead, LOG_UNIMP we
> screwed up, then reset the device.
> 
> Macro PFLASH_BUG() is now unused; delete it.
> 
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  hw/block/pflash_cfi01.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index a51ac9f399..e6d933a06d 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -49,12 +49,6 @@
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
>  
> -#define PFLASH_BUG(fmt, ...) \
> -do { \
> -    fprintf(stderr, "PFLASH: Possible BUG - " fmt, ## __VA_ARGS__); \
> -    exit(1); \
> -} while(0)
> -
>  /* #define PFLASH_DEBUG */
>  #ifdef PFLASH_DEBUG
>  #define DPRINTF(fmt, ...)                                   \
> @@ -623,8 +617,11 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  pfl->wcycle = 0;
>                  pfl->status |= 0x80;
>              } else {
> -                DPRINTF("%s: unknown command for \"write block\"\n", __func__);
> -                PFLASH_BUG("Write block confirm");
> +                qemu_log_mask(LOG_UNIMP,
> +                    "%s: Aborting write to buffer not implemented,"
> +                    " the data is already written to storage!\n"
> +                    "Flash device reset into READ mode.\n",
> +                    __func__);
>                  goto reset_flash;
>              }
>              break;
> 

Re: [Qemu-devel] [PATCH v2 02/11] pflash_cfi01: Do not exit() on guest aborting "write to buffer"
Posted by Alex Bennée 6 years, 8 months ago
Markus Armbruster <armbru@redhat.com> writes:

> When a guest tries to abort "write to buffer" (command 0xE8), we print
> "PFLASH: Possible BUG - Write block confirm", then exit(1).  Letting
> the guest terminate QEMU is not a good idea.  Instead, LOG_UNIMP we
> screwed up, then reset the device.
>
> Macro PFLASH_BUG() is now unused; delete it.
>
> Suggested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  hw/block/pflash_cfi01.c | 13 +++++--------
>  1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/hw/block/pflash_cfi01.c b/hw/block/pflash_cfi01.c
> index a51ac9f399..e6d933a06d 100644
> --- a/hw/block/pflash_cfi01.c
> +++ b/hw/block/pflash_cfi01.c
> @@ -49,12 +49,6 @@
>  #include "sysemu/sysemu.h"
>  #include "trace.h"
>
> -#define PFLASH_BUG(fmt, ...) \
> -do { \
> -    fprintf(stderr, "PFLASH: Possible BUG - " fmt, ## __VA_ARGS__); \
> -    exit(1); \
> -} while(0)
> -
>  /* #define PFLASH_DEBUG */
>  #ifdef PFLASH_DEBUG
>  #define DPRINTF(fmt, ...)                                   \
> @@ -623,8 +617,11 @@ static void pflash_write(PFlashCFI01 *pfl, hwaddr offset,
>                  pfl->wcycle = 0;
>                  pfl->status |= 0x80;
>              } else {
> -                DPRINTF("%s: unknown command for \"write block\"\n", __func__);
> -                PFLASH_BUG("Write block confirm");
> +                qemu_log_mask(LOG_UNIMP,
> +                    "%s: Aborting write to buffer not implemented,"
> +                    " the data is already written to storage!\n"
> +                    "Flash device reset into READ mode.\n",
> +                    __func__);
>                  goto reset_flash;
>              }
>              break;


--
Alex Bennée