Re: [Qemu-devel] [PATCH v6 27/28] migration/qmp: add command migrate-pause

[Peter Xu]

On Wed, Feb 14, 2018 at 06:56:59PM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Tue, Feb 13, 2018 at 08:11:00PM +, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (pet...@redhat.com) wrote:
> > > > It pauses an ongoing migration.  Currently it only supports postcopy.
> > > > Note that this command will work on either side of the migration.
> > > > Basically when we trigger this on one side, it'll interrupt the other
> > > > side as well since the other side will get notified on the disconnect
> > > > event.
> > > > 
> > > > However, it's still possible that the other side is not notified, for
> > > > example, when the network is totally broken, or due to some firewall
> > > > configuration changes.  In that case, we will also need to run the same
> > > > command on the other side so both sides will go into the paused state.
> > > > 
> > > > Signed-off-by: Peter Xu 
> > > > ---
> > > >  migration/migration.c | 27 +++
> > > >  qapi/migration.json   | 16 
> > > >  2 files changed, 43 insertions(+)
> > > > 
> > > > diff --git a/migration/migration.c b/migration/migration.c
> > > > index bb57ed9ade..139abec0c3 100644
> > > > --- a/migration/migration.c
> > > > +++ b/migration/migration.c
> > > > @@ -1448,6 +1448,33 @@ void qmp_migrate_recover(const char *uri, Error 
> > > > **errp)
> > > >  qemu_start_incoming_migration(uri, errp);
> > > >  }
> > > >  
> > > > +void qmp_migrate_pause(Error **errp)
> > > > +{
> > > > +MigrationState *ms = migrate_get_current();
> > > > +MigrationIncomingState *mis = migration_incoming_get_current();
> > > > +int ret;
> > > > +
> > > > +if (ms->state == MIGRATION_STATUS_POSTCOPY_ACTIVE) {
> > > > +/* Source side, during postcopy */
> > > > +ret = qemu_file_shutdown(ms->to_dst_file);
> > > 
> > > This doesn't feel thread safe; although I'm not sure how to make it so.
> > > If the migration finishes just after we check the state but before the
> > > shutdown we end up using a bogus QEMUFile*
> > > Making all the places that close a QEMUFile* set hte pointer Null before
> > > they do the close doesn't help because you still race with that.
> > > 
> > > (The race is small, but still)
> > 
> > IMHO we can fix it by adding a migration lock for management code. If
> > you see my previous migrate cleanup series, it's in my todo. ;)
> > 
> > The basic idea is that we take the lock for critical paths (but not
> > during most of the migration process).  E.g., we may need the lock
> > for:
> > 
> > - very beginning of migration, during setup
> > - reaching the end of migration
> > - every single migration QMP command (since HMP calls them so HMP will
> >   also acquire the lock)
> > - anywhere else I didn't mention that may necessary, e.g., when we
> >   change migrate state, meanwhile we do something else - basically
> >   that should be an "atomic operation", and we need the lock to make
> >   sure of that.
> 
> But then we couldn't take that lock in an OOB command, you'd have to
> audit all of those places that took it to make sure it didn't do any of
> the things OOB commands aren't allowed to do.

Yeah OOB commands will be special - my plan is that they just never
take the lock.  E.g., they only touches FDs, and FDs are naturally
thread safe (like this command).

And some major migration commands (like "migrate" itself) should never
be an OOB command.

> 
> > For the recovery series, I would prefer that we ignore this issue for
> > now - since this problem is there for quite a long time AFAICT in the
> > whole migration code rather than this series only, and we need to
> > solve it once and for all.
> 
> I don't think those problems happen (much) in the existing code, because
> everything is done in the main thread.

But migration is running in its own thread (migration_thread)?

For example: What if we send migration commands during the end of
migration or a failing migration?  Could there be risk in old code
too since both main thread and migration thread may be manipulating
MigrationState object?

> That's one reason why the to_dst_file is closed in migrate_fd_cleanup
> which is normally closed in the back-half run on the main thread.
> 
> One way would be to make the state go to POSTCOPY_PAUSED here;
> note that migrate_set_state already uses an atomic_cmpxchg to do the
> update.

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [Resend][PATCH] qga: unset frozen state if no mount points are frozen

[Chen Hanxiao]

At 2018-02-16 02:41:25, "Michael Roth"  wrote:
>Quoting Chen Hanxiao (2018-02-08 19:35:42)
>> From: Chen Hanxiao 
>> 
>> If we set mountpoints to qmp_guest_fsfreeze_freeze_list,
>> we may got nothing to freeze as all mountpoints are
>> not valid.
>> Call ga_unset_frozen in this senario.
>> 
>> Cc: Michael Roth 
>> Signed-off-by: Chen Hanxiao 
>> ---
>> Rebase on master
>> 
>>  qga/commands-posix.c | 6 ++
>>  1 file changed, 6 insertions(+)
>> 
>> diff --git a/qga/commands-posix.c b/qga/commands-posix.c
>> index e809e382eb..9fd51f1d7a 100644
>> --- a/qga/commands-posix.c
>> +++ b/qga/commands-posix.c
>> @@ -1273,6 +1273,12 @@ int64_t qmp_guest_fsfreeze_freeze_list(bool 
>> has_mountpoints,
>>  }
>> 
>>  free_fs_mount_list();
>> +/* We may not issue any FIFREEZE here when had mountpoints.
>> + * Just unset ga_state here and ready for the next call.
>> + */
>> +if (has_mountpoints && i == 0) {
>> +ga_unset_frozen(ga_state);
>> +}
>
>It seems odd to special-case has_mountpoints. Wouldn't:
>
>  if (i == 0) {
>...
>  }
>
>be more consistent? Then management could infer i==0 leaves qga in
>unfrozen state. Otherwise I'd rather just stick with expecting a
>gratuitous unfreeze() since it requires less special-casing on the
>management/ side.
>
>And if we do change this, we'd probably want to update
>qga/qapi-schema.json to reflect the behavior. I.e.:
>
>##
># @guest-fsfreeze-freeze:
>#
># Sync and freeze all freezable, local guest filesystems. If this
># command succeeded, you may call @guest-fsfreeze-thaw later to
># unfreeze.
>...
># Returns: Number of file systems currently frozen. On error, all filesystems
>-# will be thawed.
>+# will be thawed. If no filesystems are frozen as a result of this call,
>+# then @guest-fsfreeze-status will remain "thawed" and calling
>+# @guest-fsfreeze-thaw is not necessary.
>

Thanks for the review.
Will be fixed in v2.

Regards,
- Chen

Re: [Qemu-devel] [PATCH v6 21/28] migration: setup ramstate for resume

[Peter Xu]

On Wed, Feb 14, 2018 at 06:40:46PM +, Dr. David Alan Gilbert wrote:
> * Peter Xu (pet...@redhat.com) wrote:
> > On Tue, Feb 13, 2018 at 06:17:51PM +, Dr. David Alan Gilbert wrote:
> > > * Peter Xu (pet...@redhat.com) wrote:
> > > > After we updated the dirty bitmaps of ramblocks, we also need to update
> > > > the critical fields in RAMState to make sure it is ready for a resume.
> > > > 
> > > > Signed-off-by: Peter Xu 
> > > > ---
> > > >  migration/ram.c| 40 +++-
> > > >  migration/trace-events |  1 +
> > > >  2 files changed, 40 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/migration/ram.c b/migration/ram.c
> > > > index a2a4b05d5c..d275875f54 100644
> > > > --- a/migration/ram.c
> > > > +++ b/migration/ram.c
> > > > @@ -2250,6 +2250,36 @@ static int ram_init_all(RAMState **rsp)
> > > >  return 0;
> > > >  }
> > > >  
> > > > +static void ram_state_resume_prepare(RAMState *rs, QEMUFile *out)
> > > > +{
> > > > +RAMBlock *block;
> > > > +long pages = 0;
> > > > +
> > > > +/*
> > > > + * Postcopy is not using xbzrle/compression, so no need for that.
> > > > + * Also, since source are already halted, we don't need to care
> > > > + * about dirty page logging as well.
> > > > + */
> > > > +
> > > > +RAMBLOCK_FOREACH(block) {
> > > > +pages += bitmap_count_one(block->bmap,
> > > > +  block->used_length >> 
> > > > TARGET_PAGE_BITS);
> > > > +}
> > > > +
> > > > +/* This may not be aligned with current bitmaps. Recalculate. */
> > > > +rs->migration_dirty_pages = pages;
> > > 
> > > migration_dirty_pages is uint64_t - so we should probably do the cast
> > > above and keep 'pages' as uint64_t.
> > 
> > Sure.
> > 
> > > 
> > > > +rs->last_seen_block = NULL;
> > > > +rs->last_sent_block = NULL;
> > > > +rs->last_page = 0;
> > > > +rs->last_version = ram_list.version;
> > > 
> > > Do you need to explicitly set
> > >rs->ram_bulk_stage = false;
> > > 
> > > if the failure happened just after the start of postcopy and no
> > > requested pages had been sent, I think it might still  be set?
> > 
> > Could you elaborate what would go wrong even if it's still set?
> 
> I think it might start sending all pages rather than just those
> that are dirty/needed;  see migration_bitmap_find_dirty.

Ah yes.  I should turn it off.

-- 
Peter Xu

Re: [Qemu-devel] [Qemu-ppc] [PATCH 0/2] Firmware blob and git submodule for Sam460ex

[Thomas Huth]

On 21.02.2018 19:33, Peter Maydell wrote:
> On 21 February 2018 at 17:06, BALATON Zoltan  wrote:
>> It's not that upstream u-boot has abandoned board support (it only removed
>> support for the PPC440 CPU it once had). The board itself never had support
>> in upstream u-boot, it only exists in vendor's fork which is the reason we
>> need a separate source and cannot use upstream u-boot source we already
>> have.
>>
>> In my opinion we don't aim to take on support for this board in u-boot, we
>> only need to include the firmware binary for the emulation to be useful
>> which then requires us to also include the source for the GPL it's licensed
>> under. I've also found a few bugs in the firmware which I've fixed but apart
>> from such occasional bug fixes when needed I don't expect to take over
>> support for the board from the hardware vendor so this source is only so we
>> can include the firmware binary which is needed for the board emulation.
>> Does this answer your concerns?
> 
> We have lots of boards we don't ship firmware blobs for and
> which we expect the users to provide the guest code for
> if they're going to use them.

... which is also somewhat unfortunate. Have you ever tried to run one
of those boards to see whether you've broken something with your code
changes or not? Hunting the firmware for such a board can be quite
challenging. I'm not saying that we should now try to include way more
firmware blobs in our repository (its size would explode, I guess), but
maybe we should at least start a Wiki page with links to the various
firmware images or so?

 Thomas

[Qemu-devel] [PATCH v3 0/5] keymap: support kbd layouts with multiple mappings for the same key.

[Gerd Hoffmann]

The reverse keymap (-k $map) code has problems dealing with keymaps
where multiple key combinations can create the same keysym, because
it can store a single keycode per keysym only.  This series fixes it
and does some cleanups along the way.

v2: rebase, codestyle fixes.
v3: use GINT_TO_POINTER

Gerd Hoffmann (5):
  keymap: make struct kbd_layout_t private to ui/keymaps.c
  keymap: use glib hash for kbd_layout_t
  keymap: numpad keysyms and keycodes are fixed
  keymap: record multiple keysym -> keycode mappings
  keymap: consider modifier state when picking a mapping

 ui/keymaps.h|  30 +++
 ui/curses.c |   3 +-
 ui/keymaps.c| 163 +---
 ui/sdl.c|   6 ++-
 ui/vnc.c|   9 +++-
 ui/trace-events |   2 +-
 6 files changed, 105 insertions(+), 108 deletions(-)

-- 
2.9.3

Re: [Qemu-devel] [RFC PATCH v6 00/20] replay additions

[Pavel Dovgalyuk]

> From: Pavel Dovgalyuk [mailto:dovga...@ispras.ru]
> > From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
> > On Wed, Feb 21, 2018 at 6:41 AM, Pavel Dovgalyuk  wrote:
> > >> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
> > >> On Tue, Feb 20, 2018 at 9:46 AM, Pavel Dovgalyuk  
> > >> wrote:
> > >> >
> > >> > Updated the branch on github.
> > >> > You may try it.
> > >>
> > >> At 8a482834780a131e7747c1c3c1931379ed0beedc ARM initrd record runs,
> > >> but replay is getting stuck at:
> > >>
> > >> [   12.120424] scsi host0: sym-2.2.3
> 
> Yes, I've got the same.
> But when I use -serial stdio instead of -nographic, replay makes a pause
> at this line and then continues and finishes successfully.

It doesn't stuck, but works much slower with -nographic. 
You just have to wait for 5 minutes or something to notice the progress.

Pavel Dovgalyuk

[Qemu-devel] [PATCH v3 4/5] keymap: record multiple keysym -> keycode mappings

[Gerd Hoffmann]

Sometimes the same keysym can be created using different key
combinations.  Record them all in the reverse keymap, not only
the first one.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.c | 14 +++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index cbdd65c767..1eba6d7057 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -29,7 +29,8 @@
 #include "qemu/error-report.h"
 
 struct keysym2code {
-uint16_t keycode;
+uint32_t count;
+uint16_t keycodes[4];
 };
 
 struct kbd_layout_t {
@@ -62,11 +63,18 @@ static void add_keysym(char *line, int keysym, int keycode, 
kbd_layout_t *k)
 
 keysym2code = g_hash_table_lookup(k->hash, GINT_TO_POINTER(keysym));
 if (keysym2code) {
+if (keysym2code->count < ARRAY_SIZE(keysym2code->keycodes)) {
+keysym2code->keycodes[keysym2code->count++] = keycode;
+} else {
+warn_report("more than %zd keycodes for keysym %d",
+ARRAY_SIZE(keysym2code->keycodes), keysym);
+}
 return;
 }
 
 keysym2code = g_new0(struct keysym2code, 1);
-keysym2code->keycode = keycode;
+keysym2code->keycodes[0] = keycode;
+keysym2code->count = 1;
 g_hash_table_replace(k->hash, GINT_TO_POINTER(keysym), keysym2code);
 trace_keymap_add(keysym, keycode, line);
 }
@@ -185,7 +193,7 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 return 0;
 }
 
-return keysym2code->keycode;
+return keysym2code->keycodes[0];
 }
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
-- 
2.9.3

[Qemu-devel] [PATCH v3 2/5] keymap: use glib hash for kbd_layout_t

[Gerd Hoffmann]

Drop home-grown lookup code, which is a strange mix of a lookup table
and a list.  Use standard glib hash instead.

Signed-off-by: Gerd Hoffmann 
---
 ui/keymaps.c| 73 -
 ui/trace-events |  2 +-
 2 files changed, 32 insertions(+), 43 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index 134958a197..449c3dec22 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,22 +28,18 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
-#define MAX_NORMAL_KEYCODE 512
-#define MAX_EXTRA_COUNT 256
-
 struct key_range {
 int start;
 int end;
 struct key_range *next;
 };
 
+struct keysym2code {
+uint16_t keycode;
+};
+
 struct kbd_layout_t {
-uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
-struct {
-int keysym;
-uint16_t keycode;
-} keysym2keycode_extra[MAX_EXTRA_COUNT];
-int extra_count;
+GHashTable *hash;
 struct key_range *keypad_range;
 struct key_range *numlock_range;
 };
@@ -91,23 +87,19 @@ static void add_to_key_range(struct key_range **krp, int 
code) {
 }
 }
 
-static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k) {
-if (keysym < MAX_NORMAL_KEYCODE) {
-trace_keymap_add("normal", keysym, keycode, line);
-k->keysym2keycode[keysym] = keycode;
-} else {
-if (k->extra_count >= MAX_EXTRA_COUNT) {
-warn_report("Could not assign keysym %s (0x%x)"
-" because of memory constraints.", line, keysym);
-} else {
-trace_keymap_add("extra", keysym, keycode, line);
-k->keysym2keycode_extra[k->extra_count].
-keysym = keysym;
-k->keysym2keycode_extra[k->extra_count].
-keycode = keycode;
-k->extra_count++;
-}
+static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
+{
+struct keysym2code *keysym2code;
+
+keysym2code = g_hash_table_lookup(k->hash, GINT_TO_POINTER(keysym));
+if (keysym2code) {
+return;
 }
+
+keysym2code = g_new0(struct keysym2code, 1);
+keysym2code->keycode = keycode;
+g_hash_table_replace(k->hash, GINT_TO_POINTER(keysym), keysym2code);
+trace_keymap_add(keysym, keycode, line);
 }
 
 static kbd_layout_t *parse_keyboard_layout(const name2keysym_t *table,
@@ -131,6 +123,7 @@ static kbd_layout_t *parse_keyboard_layout(const 
name2keysym_t *table,
 
 if (!k) {
 k = g_new0(kbd_layout_t, 1);
+k->hash = g_hash_table_new(NULL, NULL);
 }
 
 for(;;) {
@@ -215,26 +208,22 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t 
*table,
 
 int keysym2scancode(kbd_layout_t *k, int keysym)
 {
-if (keysym < MAX_NORMAL_KEYCODE) {
-if (k->keysym2keycode[keysym] == 0) {
-trace_keymap_unmapped(keysym);
-warn_report("no scancode found for keysym %d", keysym);
-}
-return k->keysym2keycode[keysym];
-} else {
-int i;
+struct keysym2code *keysym2code;
+
 #ifdef XK_ISO_Left_Tab
-if (keysym == XK_ISO_Left_Tab) {
-keysym = XK_Tab;
-}
+if (keysym == XK_ISO_Left_Tab) {
+keysym = XK_Tab;
+}
 #endif
-for (i = 0; i < k->extra_count; i++) {
-if (k->keysym2keycode_extra[i].keysym == keysym) {
-return k->keysym2keycode_extra[i].keycode;
-}
-}
+
+keysym2code = g_hash_table_lookup(k->hash, GINT_TO_POINTER(keysym));
+if (!keysym2code) {
+trace_keymap_unmapped(keysym);
+warn_report("no scancode found for keysym %d", keysym);
+return 0;
 }
-return 0;
+
+return keysym2code->keycode;
 }
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
diff --git a/ui/trace-events b/ui/trace-events
index 34229e6747..861b68a305 100644
--- a/ui/trace-events
+++ b/ui/trace-events
@@ -78,7 +78,7 @@ qemu_spice_create_update(uint32_t left, uint32_t right, 
uint32_t top, uint32_t b
 
 # ui/keymaps.c
 keymap_parse(const char *file) "file %s"
-keymap_add(const char *type, int sym, int code, const char *line) "%-6s 
sym=0x%04x code=0x%04x (line: %s)"
+keymap_add(int sym, int code, const char *line) "sym=0x%04x code=0x%04x (line: 
%s)"
 keymap_unmapped(int sym) "sym=0x%04x"
 
 # ui/x_keymap.c
-- 
2.9.3

[Qemu-devel] [PATCH v3 1/5] keymap: make struct kbd_layout_t private to ui/keymaps.c

[Gerd Hoffmann]

Also use kbd_layout_t pointers instead of void pointers.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.h | 29 ++---
 ui/keymaps.c | 32 +---
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index 8757465529..17ec03387a 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -32,25 +32,6 @@ typedef struct {
int keysym;
 } name2keysym_t;
 
-struct key_range {
-int start;
-int end;
-struct key_range *next;
-};
-
-#define MAX_NORMAL_KEYCODE 512
-#define MAX_EXTRA_COUNT 256
-typedef struct {
-uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
-struct {
-   int keysym;
-   uint16_t keycode;
-} keysym2keycode_extra[MAX_EXTRA_COUNT];
-int extra_count;
-struct key_range *keypad_range;
-struct key_range *numlock_range;
-} kbd_layout_t;
-
 /* scancode without modifiers */
 #define SCANCODE_KEYMASK 0xff
 /* scancode without grey or up bit */
@@ -69,10 +50,12 @@ typedef struct {
 #define SCANCODE_ALT0x400
 #define SCANCODE_ALTGR  0x800
 
+typedef struct kbd_layout_t kbd_layout_t;
 
-void *init_keyboard_layout(const name2keysym_t *table, const char *language);
-int keysym2scancode(void *kbd_layout, int keysym);
-int keycode_is_keypad(void *kbd_layout, int keycode);
-int keysym_is_numlock(void *kbd_layout, int keysym);
+kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
+   const char *language);
+int keysym2scancode(kbd_layout_t *k, int keysym);
+int keycode_is_keypad(kbd_layout_t *k, int keycode);
+int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
 #endif /* QEMU_KEYMAPS_H */
diff --git a/ui/keymaps.c b/ui/keymaps.c
index f9762d1497..134958a197 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,6 +28,26 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
+#define MAX_NORMAL_KEYCODE 512
+#define MAX_EXTRA_COUNT 256
+
+struct key_range {
+int start;
+int end;
+struct key_range *next;
+};
+
+struct kbd_layout_t {
+uint16_t keysym2keycode[MAX_NORMAL_KEYCODE];
+struct {
+int keysym;
+uint16_t keycode;
+} keysym2keycode_extra[MAX_EXTRA_COUNT];
+int extra_count;
+struct key_range *keypad_range;
+struct key_range *numlock_range;
+};
+
 static int get_keysym(const name2keysym_t *table,
   const char *name)
 {
@@ -186,15 +206,15 @@ static kbd_layout_t *parse_keyboard_layout(const 
name2keysym_t *table,
 }
 
 
-void *init_keyboard_layout(const name2keysym_t *table, const char *language)
+kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
+   const char *language)
 {
 return parse_keyboard_layout(table, language, NULL);
 }
 
 
-int keysym2scancode(void *kbd_layout, int keysym)
+int keysym2scancode(kbd_layout_t *k, int keysym)
 {
-kbd_layout_t *k = kbd_layout;
 if (keysym < MAX_NORMAL_KEYCODE) {
 if (k->keysym2keycode[keysym] == 0) {
 trace_keymap_unmapped(keysym);
@@ -217,9 +237,8 @@ int keysym2scancode(void *kbd_layout, int keysym)
 return 0;
 }
 
-int keycode_is_keypad(void *kbd_layout, int keycode)
+int keycode_is_keypad(kbd_layout_t *k, int keycode)
 {
-kbd_layout_t *k = kbd_layout;
 struct key_range *kr;
 
 for (kr = k->keypad_range; kr; kr = kr->next) {
@@ -230,9 +249,8 @@ int keycode_is_keypad(void *kbd_layout, int keycode)
 return 0;
 }
 
-int keysym_is_numlock(void *kbd_layout, int keysym)
+int keysym_is_numlock(kbd_layout_t *k, int keysym)
 {
-kbd_layout_t *k = kbd_layout;
 struct key_range *kr;
 
 for (kr = k->numlock_range; kr; kr = kr->next) {
-- 
2.9.3

Re: [Qemu-devel] [RFC PATCH v6 00/20] replay additions

[Pavel Dovgalyuk]

> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
> On Wed, Feb 21, 2018 at 6:41 AM, Pavel Dovgalyuk  wrote:
> >> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
> >> On Tue, Feb 20, 2018 at 9:46 AM, Pavel Dovgalyuk  
> >> wrote:
> >> >
> >> > Updated the branch on github.
> >> > You may try it.
> >>
> >> At 8a482834780a131e7747c1c3c1931379ed0beedc ARM initrd record runs,
> >> but replay is getting stuck at:
> >>
> >> [   12.120424] scsi host0: sym-2.2.3

Yes, I've got the same.
But when I use -serial stdio instead of -nographic, replay makes a pause
at this line and then continues and finishes successfully.

> >>
> >> Neighboring lines on record:
> >>
> >> [   11.346357] sym53c8xx :00:0c.0: enabling device (0100 -> 0103)
> >> [   11.536683] sym0: <895a> rev 0x0 at pci :00:0c.0 irq 66
> >> [   11.731679] sym0: No NVRAM, ID 7, Fast-40, LVD, parity checking
> >> [   11.930599] sym0: SCSI BUS has been reset.
> >> [   12.120424] scsi host0: sym-2.2.3
> >> [   15.451809] scsi 0:0:2:0: CD-ROMQEMU QEMU CD-ROM
> >>   2.5+ PQ: 0 ANSI: 5
> >> [   15.847227] scsi target0:0:2: tagged command queuing enabled,
> >> command queue depth 16.
> >> [   16.256585] scsi target0:0:2: Beginning Domain Validation
> >> [   16.482189] scsi target0:0:2: Domain Validation skipping write tests
> >> [   16.699445] scsi target0:0:2: Ending Domain Validation
> >>
> >> My QEMU command:
> >>
> >> time ./buildroot/output.arm~/host/usr/bin/qemu-system-arm -M
> >> versatilepb -append 'root=/dev/sda nokaslr norandmaps
> >> printk.devkmsg=on printk.time=y - lkmc_eval="/rand_check.out;wget -S
> >> google.com;/poweroff.out;"'
> >>  -kernel ./buildroot/output.arm~/images/zImage -dtb
> >> ./buildroot/output.arm~/images/versatile-pb.dtb -nographic -initrd
> >> ./buildroot/output.arm~/images/rootfs.cpio -netdev user,id=net1
> >> -device rtl8139,netdev=net1
> >> -object filter-replay,id=replay,netdev=net1
> >>
> >> What is your full QEMU command?



Pavel Dovgalyuk

[Qemu-devel] [PATCH v3 3/5] keymap: numpad keysyms and keycodes are fixed

[Gerd Hoffmann]

No need to figure them at runtime from the keymap.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.c | 61 +---
 1 file changed, 9 insertions(+), 52 deletions(-)

diff --git a/ui/keymaps.c b/ui/keymaps.c
index 449c3dec22..cbdd65c767 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -28,20 +28,12 @@
 #include "trace.h"
 #include "qemu/error-report.h"
 
-struct key_range {
-int start;
-int end;
-struct key_range *next;
-};
-
 struct keysym2code {
 uint16_t keycode;
 };
 
 struct kbd_layout_t {
 GHashTable *hash;
-struct key_range *keypad_range;
-struct key_range *numlock_range;
 };
 
 static int get_keysym(const name2keysym_t *table,
@@ -64,29 +56,6 @@ static int get_keysym(const name2keysym_t *table,
 }
 
 
-static void add_to_key_range(struct key_range **krp, int code) {
-struct key_range *kr;
-for (kr = *krp; kr; kr = kr->next) {
-if (code >= kr->start && code <= kr->end) {
-break;
-}
-if (code == kr->start - 1) {
-kr->start--;
-break;
-}
-if (code == kr->end + 1) {
-kr->end++;
-break;
-}
-}
-if (kr == NULL) {
-kr = g_malloc0(sizeof(*kr));
-kr->start = kr->end = code;
-kr->next = *krp;
-*krp = kr;
-}
-}
-
 static void add_keysym(char *line, int keysym, int keycode, kbd_layout_t *k)
 {
 struct keysym2code *keysym2code;
@@ -160,13 +129,6 @@ static kbd_layout_t *parse_keyboard_layout(const 
name2keysym_t *table,
 const char *rest = line + offset + 1;
 int keycode = strtol(rest, NULL, 0);
 
-if (strstr(rest, "numlock")) {
-add_to_key_range(>keypad_range, keycode);
-add_to_key_range(>numlock_range, keysym);
-/* fprintf(stderr, "keypad keysym %04x keycode %d\n",
-   keysym, keycode); */
-}
-
 if (strstr(rest, "shift")) {
 keycode |= SCANCODE_SHIFT;
 }
@@ -228,24 +190,19 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 
 int keycode_is_keypad(kbd_layout_t *k, int keycode)
 {
-struct key_range *kr;
-
-for (kr = k->keypad_range; kr; kr = kr->next) {
-if (keycode >= kr->start && keycode <= kr->end) {
-return 1;
-}
+if (keycode >= 0x47 && keycode <= 0x53) {
+return true;
 }
-return 0;
+return false;
 }
 
 int keysym_is_numlock(kbd_layout_t *k, int keysym)
 {
-struct key_range *kr;
-
-for (kr = k->numlock_range; kr; kr = kr->next) {
-if (keysym >= kr->start && keysym <= kr->end) {
-return 1;
-}
+switch (keysym) {
+case 0xffb0 ... 0xffb9:  /* KP_0 .. KP_9 */
+case 0xffac: /* KP_Separator */
+case 0xffae: /* KP_Decimal   */
+return true;
 }
-return 0;
+return false;
 }
-- 
2.9.3

[Qemu-devel] [PATCH v3 5/5] keymap: consider modifier state when picking a mapping

[Gerd Hoffmann]

Pass the modifier state to the keymap lookup function.  In case multiple
keysym -> keycode mappings exist look at the modifier state and prefer
the mapping where the modifier state matches.

Signed-off-by: Gerd Hoffmann 
Reviewed-by: Daniel P. Berrangé 
---
 ui/keymaps.h |  3 ++-
 ui/curses.c  |  3 ++-
 ui/keymaps.c | 33 -
 ui/sdl.c |  6 +-
 ui/vnc.c |  9 +++--
 5 files changed, 48 insertions(+), 6 deletions(-)

diff --git a/ui/keymaps.h b/ui/keymaps.h
index 17ec03387a..0693588225 100644
--- a/ui/keymaps.h
+++ b/ui/keymaps.h
@@ -54,7 +54,8 @@ typedef struct kbd_layout_t kbd_layout_t;
 
 kbd_layout_t *init_keyboard_layout(const name2keysym_t *table,
const char *language);
-int keysym2scancode(kbd_layout_t *k, int keysym);
+int keysym2scancode(kbd_layout_t *k, int keysym,
+bool shift, bool altgr, bool ctrl);
 int keycode_is_keypad(kbd_layout_t *k, int keycode);
 int keysym_is_numlock(kbd_layout_t *k, int keysym);
 
diff --git a/ui/curses.c b/ui/curses.c
index 479b77bd03..597e47fd4a 100644
--- a/ui/curses.c
+++ b/ui/curses.c
@@ -271,7 +271,8 @@ static void curses_refresh(DisplayChangeListener *dcl)
 keysym = chr;
 }
 
-keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK);
+keycode = keysym2scancode(kbd_layout, keysym & KEYSYM_MASK,
+  false, false, false);
 if (keycode == 0)
 continue;
 
diff --git a/ui/keymaps.c b/ui/keymaps.c
index 1eba6d7057..43fe604724 100644
--- a/ui/keymaps.c
+++ b/ui/keymaps.c
@@ -176,8 +176,12 @@ kbd_layout_t *init_keyboard_layout(const name2keysym_t 
*table,
 }
 
 
-int keysym2scancode(kbd_layout_t *k, int keysym)
+int keysym2scancode(kbd_layout_t *k, int keysym,
+bool shift, bool altgr, bool ctrl)
 {
+static const uint32_t mask =
+SCANCODE_SHIFT | SCANCODE_ALTGR | SCANCODE_CTRL;
+uint32_t mods, i;
 struct keysym2code *keysym2code;
 
 #ifdef XK_ISO_Left_Tab
@@ -193,6 +197,33 @@ int keysym2scancode(kbd_layout_t *k, int keysym)
 return 0;
 }
 
+if (keysym2code->count == 1) {
+return keysym2code->keycodes[0];
+}
+
+/*
+ * We have multiple keysym -> keycode mappings.
+ *
+ * Check whenever we find one mapping where the modifier state of
+ * the mapping matches the current user interface modifier state.
+ * If so, prefer that one.
+ */
+mods = 0;
+if (shift) {
+mods |= SCANCODE_SHIFT;
+}
+if (altgr) {
+mods |= SCANCODE_ALTGR;
+}
+if (ctrl) {
+mods |= SCANCODE_CTRL;
+}
+
+for (i = 0; i < keysym2code->count; i++) {
+if ((keysym2code->keycodes[i] & mask) == mods) {
+return keysym2code->keycodes[i];
+}
+}
 return keysym2code->keycodes[0];
 }
 
diff --git a/ui/sdl.c b/ui/sdl.c
index 963cdf77a7..c4ae7ab05d 100644
--- a/ui/sdl.c
+++ b/ui/sdl.c
@@ -201,6 +201,9 @@ static kbd_layout_t *kbd_layout = NULL;
 
 static uint8_t sdl_keyevent_to_keycode_generic(const SDL_KeyboardEvent *ev)
 {
+bool shift = modifiers_state[0x2a] || modifiers_state[0x36];
+bool altgr = modifiers_state[0xb8];
+bool ctrl  = modifiers_state[0x1d] || modifiers_state[0x9d];
 int keysym;
 /* workaround for X11+SDL bug with AltGR */
 keysym = ev->keysym.sym;
@@ -210,7 +213,8 @@ static uint8_t sdl_keyevent_to_keycode_generic(const 
SDL_KeyboardEvent *ev)
 if (keysym == 92 && ev->keysym.scancode == 133) {
 keysym = 0xa5;
 }
-return keysym2scancode(kbd_layout, keysym) & SCANCODE_KEYMASK;
+return keysym2scancode(kbd_layout, keysym,
+   shift, altgr, ctrl) & SCANCODE_KEYMASK;
 }
 
 
diff --git a/ui/vnc.c b/ui/vnc.c
index a77b568b57..d19f86c7f4 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1734,7 +1734,8 @@ static void reset_keys(VncState *vs)
 
 static void press_key(VncState *vs, int keysym)
 {
-int keycode = keysym2scancode(vs->vd->kbd_layout, keysym) & 
SCANCODE_KEYMASK;
+int keycode = keysym2scancode(vs->vd->kbd_layout, keysym,
+  false, false, false) & SCANCODE_KEYMASK;
 qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, true);
 qemu_input_event_send_key_delay(vs->vd->key_delay_ms);
 qemu_input_event_send_key_number(vs->vd->dcl.con, keycode, false);
@@ -1993,6 +1994,9 @@ static const char *code2name(int keycode)
 
 static void key_event(VncState *vs, int down, uint32_t sym)
 {
+bool shift = vs->modifiers_state[0x2a] || vs->modifiers_state[0x36];
+bool altgr = vs->modifiers_state[0xb8];
+bool ctrl  = vs->modifiers_state[0x1d] || vs->modifiers_state[0x9d];
 int keycode;
 int lsym = sym;
 
@@ -2000,7 +2004,8 @@ static void key_event(VncState *vs, int down, uint32_t 
sym)
 lsym = lsym - 'A' + 'a';
 }
 
-keycode = 

Re: [Qemu-devel] [PATCH v4 0/7] vfio: add display support

[Zhenyu Wang]

On 2018.02.20 18:04:17 +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Sneak preview at https://www.kraxel.org/cgit/qemu/log/?h=work/intel-vgpu
> > Note: branch is a moving target ;)
> > 
> > State:
> >   spice: Partly working (no mouse ptr yet).
> 
> Working now, in case anyone wants play.
> 
> Must turn on opengl and use a unix socket, i.e. this:
> 
>   qemu -spice disable-ticketing,gl=on,unix,addr=/tmp/spice-vgpu
> 
> and this:
> 
>   remote-viewer spice+unix:tmp/spice-vgpu
> 
> Host needs a 4.16-rc kernel.
> 
> Guest can be older, most of the time I'm testing with the Fedora 27 live
> iso (4.13) which works ok.
> 

Nice! Seems to be the last missing gap for local spice with cursor
dmabuf support, we'll do more testing on that for sure. Btw, another
method might be to add direct cursor dmabuf passing for spice as gl
output, is that correct way?

And really like to see dmabuf support could be in upstream soon. Do
you have any predict for which qemu version?

Thanks!

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


signature.asc
Description: PGP signature

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi

[Thomas Huth]

On 21.02.2018 20:35, Collin L. Walling wrote:
> Interactive boot menu for scsi. This follows a similar procedure
> as the interactive menu for eckd dasd. An example follows:
> 
> s390x Enumerated Boot Menu.
> 
> 3 entries detected. Select from index 0 to 2.
> 
> Signed-off-by: Collin L. Walling 
> Reviewed-by: Thomas Huth 
> ---
>  hw/s390x/ipl.c  |  1 +
>  pc-bios/s390-ccw/bootmap.c  |  4 
>  pc-bios/s390-ccw/main.c |  1 +
>  pc-bios/s390-ccw/menu.c | 14 ++
>  pc-bios/s390-ccw/s390-ccw.h |  1 +
>  5 files changed, 21 insertions(+)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index a0f4f40..566248e 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -238,6 +238,7 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
>  *flags |= QIPL_FLAG_BM_OPTS_ZIPL;
>  return;
>  }
> +case S390_IPL_TYPE_QEMU_SCSI:
>  break;

It's not a real bug, but I'm pretty sure this will cause Coverity to
report an issue of unintended switch-case fall through. Could you please
add a break before the new case label?
(Since all other patches look fine to me, this could maybe also be fixed
when the patches are picked up, so no need to respin the whole patch
series just because of this)

 Thomas

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present

[Thomas Huth]

On 21.02.2018 20:35, Collin L. Walling wrote:
> If no boot menu options are present, then flag the boot menu to
> use the zipl options that were set in the zipl configuration file
> (and stored on disk by zipl). These options are found at some
> offset prior to the start of the zipl boot menu banner. The zipl
> timeout value is limited to a 16-bit unsigned integer and stored
> as seconds, so we take care to convert it to milliseconds in order
> to conform to the rest of the boot menu functionality. This is
> limited to CCW devices.
> 
> For reference, the zipl configuration file uses the following
> fields in the menu section:
> 
>   prompt=1  enable the boot menu
>   timeout=X set the timeout to X seconds
> 
> To explicitly disregard any boot menu options, then menu=off or
>  must be specified.
> 
> Signed-off-by: Collin L. Walling 
> ---
>  hw/s390x/ipl.c  |  5 +
>  hw/s390x/ipl.h  |  1 +
>  pc-bios/s390-ccw/iplb.h |  1 +
>  pc-bios/s390-ccw/main.c |  3 ++-
>  pc-bios/s390-ccw/menu.c | 14 ++
>  5 files changed, 23 insertions(+), 1 deletion(-)

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs

[Thomas Huth]

On 21.02.2018 20:35, Collin L. Walling wrote:
> It is possible while waiting for multiple types of external
> interrupts that we might have pending irqs remaining between
> irq consumption and irq-type disabling. Those interrupts
> could potentially propagate to the guest after IPL completes
> and cause unwanted behavior.
> 
> As it is today, the SCLP will only recognize write events that
> are enabled by the control program's send and receive masks. To
> limit the window for, and prevent further irqs from, ASCII
> console events (specifically keystrokes), we should only enable
> the control program's receive mask when we need it.
> 
> While we're at it, remove assignment of the (non control program)
> send and receive masks, as those are actually set by the SCLP.
> 
> Signed-off-by: Collin L. Walling 
> ---
>  pc-bios/s390-ccw/menu.c |  5 +
>  pc-bios/s390-ccw/s390-ccw.h |  1 +
>  pc-bios/s390-ccw/sclp.c | 10 --
>  3 files changed, 10 insertions(+), 6 deletions(-)

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [RFC PATCH v0 1/2] pc-dimm: Make pc_dimm_built_list() global

[Bharata B Rao]

On Tue, Feb 20, 2018 at 03:35:10PM +0100, Igor Mammedov wrote:
> On Mon, 19 Feb 2018 12:12:53 +0530
> Bharata B Rao  wrote:
> 
> > Making pc_dimm_built_list() global allows other parts of QEMU code
> > to build and walk through the DIMM list in address-sorted order.
> > 
> > This is needed in the next patch for sPAPR code to create
> > ibm,dynamic-memory-v2 device tree property that will have entries
> > for populated DIMMs as well as available hotpluggable areas.
> > 
> > CHECK: List of DIMMs is already available via qmp_pc_dimm_device_list(),
> maybe make it sorted first and use it?
> 
> (i.e. use pc_dimm_built_list in qmp_pc_dimm_device_list) and hide
> recursive callback ugliness from external users.
> 
> MemoryDeviceInfoList *qmp_pc_dimm_device_list(void) {
> object_child_foreach(qdev_get_machine(), pc_dimm_built_list, );
> ...
> }

Thanks will attempt this in my next version.

Regards,
Bharata.

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 06/13] s390-ccw: parse and set boot menu options

[Thomas Huth]

On 21.02.2018 20:35, Collin L. Walling wrote:
> Set boot menu options for an s390 guest and store them in
> the iplb. These options are set via the QEMU command line
> option:
> 
> -boot menu=on|off[,splash-time=X]
> 
> or via the libvirt domain xml:
> 
> 
>   
> 
> 
> Where X represents some positive integer representing
> milliseconds.
> 
> Any value set for loadparm will override all boot menu options.
> If loadparm=PROMPT, then the menu will be enabled without a
> timeout.
> 
> Signed-off-by: Collin L. Walling 
> Reviewed-by: Janosch Frank 
> ---
>  hw/s390x/ipl.c  | 44 
>  hw/s390x/ipl.h  |  9 +++--
>  pc-bios/s390-ccw/iplb.h |  9 +++--
>  3 files changed, 58 insertions(+), 4 deletions(-)

Reviewed-by: Thomas Huth 

[Qemu-devel] [PATCH qemu repost] qmp: Add qom-list-properties to list QOM object properties

[Alexey Kardashevskiy]

There is already 'device-list-properties' which does most of the job,
however it does not handle everything returned by qom-list-types such
as machines as they inherit directly from TYPE_OBJECT and not TYPE_DEVICE.

This adds a new qom-list-properties command which prints properties
of a specific class and its instance. It is pretty much a simplified copy
of the device-list-properties handler.

Since it creates an object instance, device properties should appear
in the output as they are copied to QOM properties at the instance_init
hook.

Signed-off-by: Alexey Kardashevskiy 
---

This is a simple rebase on top of the current upstream.


---
 qapi-schema.json | 29 +
 qmp.c| 52 
 2 files changed, 81 insertions(+)

diff --git a/qapi-schema.json b/qapi-schema.json
index 0262b9f..fa5f189 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -1455,6 +1455,35 @@
   'returns': [ 'DevicePropertyInfo' ] }
 
 ##
+# @QOMPropertyInfo:
+#
+# Information about object properties.
+#
+# @name: the name of the property
+# @type: the typename of the property
+# @description: if specified, the description of the property.
+#
+# Since: 2.12
+##
+{ 'struct': 'QOMPropertyInfo',
+  'data': { 'name': 'str', 'type': 'str', '*description': 'str' } }
+
+##
+# @qom-list-properties:
+#
+# List properties associated with a QOM object.
+#
+# @typename: the type name of an object
+#
+# Returns: a list of QOMPropertyInfo describing object properties
+#
+# Since: 2.12
+##
+{ 'command': 'qom-list-properties',
+  'data': { 'typename': 'str'},
+  'returns': [ 'QOMPropertyInfo' ] }
+
+##
 # @xen-set-global-dirty-log:
 #
 # Enable or disable the global dirty log mode.
diff --git a/qmp.c b/qmp.c
index 793f6f3..f2d4781 100644
--- a/qmp.c
+++ b/qmp.c
@@ -576,6 +576,58 @@ DevicePropertyInfoList *qmp_device_list_properties(const 
char *typename,
 return prop_list;
 }
 
+QOMPropertyInfoList *qmp_qom_list_properties(const char *typename,
+ Error **errp)
+{
+ObjectClass *klass;
+Object *obj;
+ObjectProperty *prop;
+ObjectPropertyIterator iter;
+QOMPropertyInfoList *prop_list = NULL;
+
+klass = object_class_by_name(typename);
+if (klass == NULL) {
+error_set(errp, ERROR_CLASS_DEVICE_NOT_FOUND,
+  "Class '%s' not found", typename);
+return NULL;
+}
+
+klass = object_class_dynamic_cast(klass, TYPE_OBJECT);
+if (klass == NULL) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename", 
TYPE_OBJECT);
+return NULL;
+}
+
+if (object_class_is_abstract(klass)) {
+error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "typename",
+   "non-abstract class");
+return NULL;
+}
+
+obj = object_new(typename);
+
+object_property_iter_init(, obj);
+while ((prop = object_property_iter_next())) {
+QOMPropertyInfo *info;
+QOMPropertyInfoList *entry;
+
+info = g_malloc0(sizeof(*info));
+info->name = g_strdup(prop->name);
+info->type = g_strdup(prop->type);
+info->has_description = !!prop->description;
+info->description = g_strdup(prop->description);
+
+entry = g_malloc0(sizeof(*entry));
+entry->value = info;
+entry->next = prop_list;
+prop_list = entry;
+}
+
+object_unref(obj);
+
+return prop_list;
+}
+
 CpuDefinitionInfoList *qmp_query_cpu_definitions(Error **errp)
 {
 return arch_query_cpu_definitions(errp);
-- 
2.11.0

[Qemu-devel] [PATCH V4 0/3] tests: Add migration test for aarch64

[Wei Huang]

This patchset adds a migration test for aarch64. It leverages
Dave Gilbert's recent patch "tests/migration: Add source to PC boot block"
to create a new test case for aarch64.

V3->V4:
 * Rename .s to .S, allowing assembly to include C-style header file
 * Move test defines into a new migration-test.h file
 * Use different cpu & gic settings for kvm and tcg modes on aarch64
 * Clean up aarch64-a-b-kernel.S based on Andrew Jones' comments
 
V2->V3:
 * Convert build script to Makefile
 * Add cross-compilation support
 * Fix CPU type for "tcg" machine type
 * Revise asm code and the compilation process from asm to header file

V1->V2:
 * Similar to Dave Gilbert's recent changes to migration-test, we
   provide the test source and a build script in V2.
 * aarch64 kernel blob is defined as "unsigned char" because the source
   is now provided in V2.
 * Add "-machine none" to test_deprecated() because aarch64 doesn't have
   a default machine type.

RFC->V1:
 * aarch64 kernel blob is defined as an uint32_t array
 * The test code is re-written to address a data caching issue under KVM.
   Tests passed under both x86 and aarch64.
 * Re-use init_bootfile_x86() for both x86 and aarch64
 * Other minor fixes

Thanks,
-Wei

Wei Huang (3):
  tests/migration: Convert the boot block compilation script into
Makefile
  tests/migration: Add migration-test header file
  tests: Add migration test for aarch64

 tests/Makefile.include |  1 +
 tests/migration-test.c | 52 +---
 tests/migration/Makefile   | 46 ++
 tests/migration/aarch64-a-b-kernel.S   | 71 ++
 tests/migration/aarch64-a-b-kernel.h   | 19 ++
 tests/migration/migration-test.h   | 24 
 tests/migration/rebuild-x86-bootblock.sh   | 33 --
 .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}   | 12 ++--
 tests/migration/x86-a-b-bootblock.h|  4 +-
 9 files changed, 213 insertions(+), 49 deletions(-)
 create mode 100644 tests/migration/Makefile
 create mode 100644 tests/migration/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64-a-b-kernel.h
 create mode 100644 tests/migration/migration-test.h
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh
 rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (88%)

-- 
2.14.3

[Qemu-devel] [PATCH V4 3/3] tests: Add migration test for aarch64

[Wei Huang]

This patch adds migration test support for aarch64. The test code, which
implements the same functionality as x86, is booted as a kernel in qemu.
Here are the design choices we make for aarch64:

 * We choose this -kernel approach because aarch64 QEMU doesn't provide a
   built-in fw like x86 does. So instead of relying on a boot loader, we
   use -kernel approach for aarch64.
 * The serial output is sent to PL011 directly.
 * The physical memory base for mach-virt machine is 0x4000. We change
   the start_address and end_address for aarch64.

In addition to providing the binary, this patch also includes the source
code and the build script in tests/migration/. So users can change the
source and/or re-compile the binary as they wish.

Signed-off-by: Wei Huang 
---
 tests/Makefile.include   |  1 +
 tests/migration-test.c   | 47 +---
 tests/migration/Makefile | 12 +-
 tests/migration/aarch64-a-b-kernel.S | 71 
 tests/migration/aarch64-a-b-kernel.h | 19 ++
 tests/migration/migration-test.h |  5 +++
 6 files changed, 147 insertions(+), 8 deletions(-)
 create mode 100644 tests/migration/aarch64-a-b-kernel.S
 create mode 100644 tests/migration/aarch64-a-b-kernel.h

diff --git a/tests/Makefile.include b/tests/Makefile.include
index a1bcbffe12..df9f64438f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -372,6 +372,7 @@ check-qtest-arm-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y = tests/numa-test$(EXESUF)
 check-qtest-aarch64-y += tests/sdhci-test$(EXESUF)
 check-qtest-aarch64-y += tests/boot-serial-test$(EXESUF)
+check-qtest-aarch64-y += tests/migration-test$(EXESUF)
 
 check-qtest-microblazeel-y = $(check-qtest-microblaze-y)
 
diff --git a/tests/migration-test.c b/tests/migration-test.c
index e2e06ed337..a4f6732a59 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -11,6 +11,7 @@
  */
 
 #include "qemu/osdep.h"
+#include 
 
 #include "libqtest.h"
 #include "qapi/qmp/qdict.h"
@@ -23,8 +24,8 @@
 
 #include "migration/migration-test.h"
 
-const unsigned start_address = TEST_MEM_START;
-const unsigned end_address = TEST_MEM_END;
+unsigned start_address = TEST_MEM_START;
+unsigned end_address = TEST_MEM_END;
 bool got_stop;
 
 #if defined(__linux__)
@@ -81,12 +82,13 @@ static const char *tmpfs;
  * outputting a 'B' every so often if it's still running.
  */
 #include "tests/migration/x86-a-b-bootblock.h"
+#include "tests/migration/aarch64-a-b-kernel.h"
 
-static void init_bootfile_x86(const char *bootpath)
+static void init_bootfile(const char *bootpath, void *content)
 {
 FILE *bootfile = fopen(bootpath, "wb");
 
-g_assert_cmpint(fwrite(x86_bootsect, 512, 1, bootfile), ==, 1);
+g_assert_cmpint(fwrite(content, 512, 1, bootfile), ==, 1);
 fclose(bootfile);
 }
 
@@ -393,7 +395,7 @@ static void test_migrate_start(QTestState **from, 
QTestState **to,
 got_stop = false;
 
 if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
-init_bootfile_x86(bootpath);
+init_bootfile(bootpath, x86_bootsect);
 cmd_src = g_strdup_printf("-machine accel=%s -m 150M"
   " -name source,debug-threads=on"
   " -serial file:%s/src_serial"
@@ -422,6 +424,39 @@ static void test_migrate_start(QTestState **from, 
QTestState **to,
   " -serial file:%s/dest_serial"
   " -incoming %s",
   accel, tmpfs, uri);
+} else if (strcmp(arch, "aarch64") == 0) {
+const char *cpu;
+const char *gic_ver;
+struct utsname utsname;
+
+/* kvm and tcg need different cpu and gic-version configs */
+if (access("/dev/kvm", F_OK) == 0 && uname() == 0 &&
+strcmp(utsname.machine, "aarch64") == 0) {
+accel = "kvm";
+cpu = "host";
+gic_ver = "host";
+} else {
+accel = "tcg";
+cpu = "cortex-a57";
+gic_ver = "2";
+}
+
+init_bootfile(bootpath, aarch64_kernel);
+cmd_src = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
+  "-name vmsource,debug-threads=on -cpu %s "
+  "-m 150M -serial file:%s/src_serial "
+  "-kernel %s ",
+  accel, gic_ver, cpu, tmpfs, bootpath);
+cmd_dst = g_strdup_printf("-machine virt,accel=%s,gic-version=%s "
+  "-name vmdest,debug-threads=on -cpu %s "
+  "-m 150M -serial file:%s/dest_serial "
+  "-kernel %s "
+  "-incoming %s ",
+  accel, gic_ver, cpu, tmpfs, bootpath, uri);
+
+/* aarch64 virt machine physical 

[Qemu-devel] [PATCH V4 1/3] tests/migration: Convert the boot block compilation script into Makefile

[Wei Huang]

The x86 boot block header currently is generated with a shell script.
To better support other CPUs (e.g. aarch64), we convert the script
into Makefile. This allows us to 1) support cross-compilation easily,
and 2) avoid creating a script file for every architecture.

Signed-off-by: Wei Huang 
---
 tests/migration/Makefile | 38 
 tests/migration/rebuild-x86-bootblock.sh | 33 ---
 tests/migration/x86-a-b-bootblock.h  |  2 +-
 tests/migration/x86-a-b-bootblock.s  |  5 ++---
 4 files changed, 41 insertions(+), 37 deletions(-)
 create mode 100644 tests/migration/Makefile
 delete mode 100755 tests/migration/rebuild-x86-bootblock.sh

diff --git a/tests/migration/Makefile b/tests/migration/Makefile
new file mode 100644
index 00..1c07dd7be9
--- /dev/null
+++ b/tests/migration/Makefile
@@ -0,0 +1,38 @@
+#
+# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
+#
+# Authors:
+#   Dave Gilbert 
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+path := $(subst :, ,$(PATH))
+system := $(shell uname -s | tr "A-Z" "a-z")
+
+cross-ld = $(firstword $(wildcard $(patsubst %,%/$(1)-*$(system)*-ld,$(path
+cross-gcc = $(firstword $(wildcard $(patsubst %ld,%gcc,$(call cross-ld,$(1)
+find-cross-prefix = $(subst gcc,,$(notdir $(call cross-gcc,$(1
+
+x86_64_cross_prefix := $(call find-cross-prefix,x86_64)
+
+export __note
+override define __note
+/* This file is automatically generated from
+ * tests/migration/$<, edit that and then run
+ * "make $@" inside tests/migration to update,
+ * and then remember to send both in your patch submission.
+ */
+endef
+
+all: x86-a-b-bootblock.h
+
+x86-a-b-bootblock.h: x86-a-b-bootblock.s
+   $(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
+   $(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
+   dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
+   echo "$$__note" > $@
+   xxd -i x86.bootsect | sed -e 's/.*int.*//' >> $@
+
+clean:
+   rm -f *.bootsect *.boot *.o
diff --git a/tests/migration/rebuild-x86-bootblock.sh 
b/tests/migration/rebuild-x86-bootblock.sh
deleted file mode 100755
index 86cec5d284..00
--- a/tests/migration/rebuild-x86-bootblock.sh
+++ /dev/null
@@ -1,33 +0,0 @@
-#!/bin/sh
-# Copyright (c) 2016-2018 Red Hat, Inc. and/or its affiliates
-# This work is licensed under the terms of the GNU GPL, version 2 or later.
-# See the COPYING file in the top-level directory.
-#
-# Author: dgilb...@redhat.com
-
-ASMFILE=$PWD/tests/migration/x86-a-b-bootblock.s
-HEADER=$PWD/tests/migration/x86-a-b-bootblock.h
-
-if [ ! -e "$ASMFILE" ]
-then
-  echo "Couldn't find $ASMFILE" >&2
-  exit 1
-fi
-
-ASM_WORK_DIR=$(mktemp -d --tmpdir X86BB.XX)
-cd "$ASM_WORK_DIR" &&
-as --32 -march=i486 "$ASMFILE" -o x86.o &&
-objcopy -O binary x86.o x86.boot &&
-dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124 &&
-xxd -i x86.bootsect |
-sed -e 's/.*int.*//' > x86.hex &&
-cat - x86.hex < "$HEADER"
-/* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
- * tests/migration/rebuild-x86-bootblock.sh to update,
- * and then remember to send both in your patch submission.
- */
-HERE
-
-rm x86.hex x86.bootsect x86.boot x86.o
-cd .. && rmdir "$ASM_WORK_DIR"
diff --git a/tests/migration/x86-a-b-bootblock.h 
b/tests/migration/x86-a-b-bootblock.h
index 78a151fe2a..9e8e2e028b 100644
--- a/tests/migration/x86-a-b-bootblock.h
+++ b/tests/migration/x86-a-b-bootblock.h
@@ -1,6 +1,6 @@
 /* This file is automatically generated from
  * tests/migration/x86-a-b-bootblock.s, edit that and then run
- * tests/migration/rebuild-x86-bootblock.sh to update,
+ * "make x86-a-b-bootblock.h" inside tests/migration to update,
  * and then remember to send both in your patch submission.
  */
 unsigned char x86_bootsect[] = {
diff --git a/tests/migration/x86-a-b-bootblock.s 
b/tests/migration/x86-a-b-bootblock.s
index b1642641a7..98dbfab084 100644
--- a/tests/migration/x86-a-b-bootblock.s
+++ b/tests/migration/x86-a-b-bootblock.s
@@ -3,9 +3,8 @@
 #  range.
 #  Outputs an initial 'A' on serial followed by repeated 'B's
 #
-# run   tests/migration/rebuild-x86-bootblock.sh
-#   to regenerate the hex, and remember to include both the .h and .s
-#   in any patches.
+#  In tests/migration dir, run 'make x86-a-b-bootblock.h' to regenerate
+#  the hex, and remember to include both the .h and .s in any patches.
 #
 # Copyright (c) 2016 Red Hat, Inc. and/or its affiliates
 # This work is licensed under the terms of the GNU GPL, version 2 or later.
-- 
2.14.3

[Qemu-devel] [PATCH V4 2/3] tests/migration: Add migration-test header file

[Wei Huang]

This patch moves the settings related migration-test from the
migration-test.c file to a seperate header file. It also renames the
x86-a-b-bootblock.s file extension from .s to .S, allowing gcc
pre-processor to include the C-style header file correctly.

Signed-off-by: Wei Huang 
---
 tests/migration-test.c|  9 +
 tests/migration/Makefile  |  4 ++--
 tests/migration/migration-test.h  | 19 +++
 .../{x86-a-b-bootblock.s => x86-a-b-bootblock.S}  |  7 ---
 tests/migration/x86-a-b-bootblock.h   |  2 +-
 5 files changed, 31 insertions(+), 10 deletions(-)
 create mode 100644 tests/migration/migration-test.h
 rename tests/migration/{x86-a-b-bootblock.s => x86-a-b-bootblock.S} (94%)

diff --git a/tests/migration-test.c b/tests/migration-test.c
index 74f9361bdd..e2e06ed337 100644
--- a/tests/migration-test.c
+++ b/tests/migration-test.c
@@ -21,10 +21,10 @@
 #include "sysemu/sysemu.h"
 #include "hw/nvram/chrp_nvram.h"
 
-#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
+#include "migration/migration-test.h"
 
-const unsigned start_address = 1024 * 1024;
-const unsigned end_address = 100 * 1024 * 1024;
+const unsigned start_address = TEST_MEM_START;
+const unsigned end_address = TEST_MEM_END;
 bool got_stop;
 
 #if defined(__linux__)
@@ -278,7 +278,8 @@ static void check_guests_ram(QTestState *who)
 qtest_memread(who, start_address, _byte, 1);
 last_byte = first_byte;
 
-for (address = start_address + 4096; address < end_address; address += 
4096)
+for (address = start_address + TEST_MEM_PAGE_SIZE; address < end_address;
+ address += TEST_MEM_PAGE_SIZE)
 {
 uint8_t b;
 qtest_memread(who, address, , 1);
diff --git a/tests/migration/Makefile b/tests/migration/Makefile
index 1c07dd7be9..b768d0729d 100644
--- a/tests/migration/Makefile
+++ b/tests/migration/Makefile
@@ -27,8 +27,8 @@ endef
 
 all: x86-a-b-bootblock.h
 
-x86-a-b-bootblock.h: x86-a-b-bootblock.s
-   $(x86_64_cross_prefix)as --32 -march=i486 $< -o x86.o
+x86-a-b-bootblock.h: x86-a-b-bootblock.S
+   $(x86_64_cross_prefix)gcc -m32 -march=i486 -c $< -o x86.o
$(x86_64_cross_prefix)objcopy -O binary x86.o x86.boot
dd if=x86.boot of=x86.bootsect bs=256 count=2 skip=124
echo "$$__note" > $@
diff --git a/tests/migration/migration-test.h b/tests/migration/migration-test.h
new file mode 100644
index 00..a618fe069e
--- /dev/null
+++ b/tests/migration/migration-test.h
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2018 Red Hat, Inc. and/or its affiliates
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef _TEST_MIGRATION_H_
+#define _TEST_MIGRATION_H_
+
+/* Common */
+#define TEST_MEM_START  (1 * 1024 * 1024)
+#define TEST_MEM_END(100 * 1024 * 1024)
+#define TEST_MEM_PAGE_SIZE  4096
+
+/* PPC */
+#define MIN_NVRAM_SIZE 8192 /* from spapr_nvram.c */
+
+#endif /* _TEST_MIGRATION_H_ */
+
diff --git a/tests/migration/x86-a-b-bootblock.s 
b/tests/migration/x86-a-b-bootblock.S
similarity index 94%
rename from tests/migration/x86-a-b-bootblock.s
rename to tests/migration/x86-a-b-bootblock.S
index 98dbfab084..08b51f9e7f 100644
--- a/tests/migration/x86-a-b-bootblock.s
+++ b/tests/migration/x86-a-b-bootblock.S
@@ -12,6 +12,7 @@
 #
 # Author: dgilb...@redhat.com
 
+#include "migration-test.h"
 
 .code16
 .org 0x7c00
@@ -45,11 +46,11 @@ start: # at 0x7c00 ?
 mov $0, %bl
 mainloop:
 # Start from 1MB
-mov $(1024*1024),%eax
+mov $TEST_MEM_START,%eax
 innerloop:
 incb (%eax)
-add $4096,%eax
-cmp $(100*1024*1024),%eax
+add $TEST_MEM_PAGE_SIZE,%eax
+cmp $TEST_MEM_END,%eax
 jl innerloop
 
 inc %bl
diff --git a/tests/migration/x86-a-b-bootblock.h 
b/tests/migration/x86-a-b-bootblock.h
index 9e8e2e028b..44e4b99506 100644
--- a/tests/migration/x86-a-b-bootblock.h
+++ b/tests/migration/x86-a-b-bootblock.h
@@ -1,5 +1,5 @@
 /* This file is automatically generated from
- * tests/migration/x86-a-b-bootblock.s, edit that and then run
+ * tests/migration/x86-a-b-bootblock.S, edit that and then run
  * "make x86-a-b-bootblock.h" inside tests/migration to update,
  * and then remember to send both in your patch submission.
  */
-- 
2.14.3

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location

[Thomas Huth]

On 21.02.2018 20:35, Collin L. Walling wrote:
> The s390-ccw firmware needs some information in support of the
> boot process which is not available on the native machine.
> Examples are the netboot firmware load address and now the
> boot menu parameters.
> 
> While storing that data in unused fields of the IPL parameter block
> works, that approach could create problems if the parameter block
> definition should change in the future. Because then a guest could
> overwrite these fields using the set IPLB diagnose.
> 
> In fact the data in question is of more global nature and not really
> tied to an IPL device, so separating it is rather logical.
> 
> This commit introduces a new structure to hold firmware relevant
> IPL parameters set by QEMU. The data is stored at location 204 (dec)
> and can contain up to 7 32-bit words. This area is available to
> programming in the z/Architecture Principles of Operation and
> can thus safely be used by the firmware until the IPL has completed.
> 
> Signed-off-by: Viktor Mihajlovski 
> Signed-off-by: Collin L. Walling 
> ---
>  hw/s390x/ipl.c  | 18 +-
>  hw/s390x/ipl.h  | 25 +++--
>  pc-bios/s390-ccw/iplb.h | 18 --
>  pc-bios/s390-ccw/main.c |  6 +-
>  4 files changed, 61 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
> index 0d06fc1..79f5a58 100644
> --- a/hw/s390x/ipl.c
> +++ b/hw/s390x/ipl.c
> @@ -399,6 +399,21 @@ void s390_reipl_request(void)
>  qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>  }
>  
> +static void s390_ipl_prepare_qipl(S390CPU *cpu)
> +{
> +S390IPLState *ipl = get_ipl_device();
> +uint8_t *addr;
> +uint64_t len = 4096;
> +
> +addr = cpu_physical_memory_map(cpu->env.psa, , 1);
> +if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
> +error_report("Cannot set QEMU IPL parameters");
> +return;
> +}
> +memcpy(addr + QIPL_ADDRESS, >qipl, sizeof(QemuIplParameters));
> +cpu_physical_memory_unmap(addr, len, 1, len);
> +}
> +
>  void s390_ipl_prepare_cpu(S390CPU *cpu)
>  {
>  S390IPLState *ipl = get_ipl_device();
> @@ -418,8 +433,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
>  error_report_err(err);
>  vm_stop(RUN_STATE_INTERNAL_ERROR);
>  }
> -ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
> +ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
>  }
> +s390_ipl_prepare_qipl(cpu);
>  }
>  
>  static void s390_ipl_reset(DeviceState *dev)
> diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
> index 8a705e0..08926a3 100644
> --- a/hw/s390x/ipl.h
> +++ b/hw/s390x/ipl.h
> @@ -16,8 +16,7 @@
>  #include "cpu.h"
>  
>  struct IplBlockCcw {
> -uint64_t netboot_start_addr;
> -uint8_t  reserved0[77];
> +uint8_t  reserved0[85];
>  uint8_t  ssid;
>  uint16_t devno;
>  uint8_t  vm_flags;
> @@ -90,6 +89,27 @@ void s390_ipl_prepare_cpu(S390CPU *cpu);
>  IplParameterBlock *s390_ipl_get_iplb(void);
>  void s390_reipl_request(void);
>  
> +#define QIPL_ADDRESS  0xcc
> +
> +/*
> + * The QEMU IPL Parameters will be stored at absolute address
> + * 204 (0xcc) which means it is 32-bit word aligned but not
> + * double-word aligned.
> + * Placement of data fields in this area must account for
> + * their alignment needs. E.g., netboot_start_address must
> + * have an offset of n * 8 bytes within the struct in order
> + * to keep it double-word aligned.

Should that rather be "4 + n * 8" instead of "n * 8" ?

Apart from that, patch looks good to me now, so once you've fixed the
comment (if necessary):

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH] intel-iommu: Accept 64-bit writes to FEADDR

[Peter Xu]

On Sat, Feb 17, 2018 at 12:26:19PM +0100, Jan Kiszka wrote:
> From: Jan Kiszka 
> 
> Xen is doing this [1] and currently triggers an abort.
> 
> [1] 
> http://xenbits.xenproject.org/gitweb/?p=xen.git;a=blob;f=xen/drivers/passthrough/vtd/iommu.c;h=daaed0abbdd06b6ba3d948ea103aadf02651e83c;hb=refs/heads/master#l1108
> 
> Reported-by: Luis Lloret 
> Signed-off-by: Jan Kiszka 
> ---
>  hw/i386/intel_iommu.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 2e841cde27..b61d0da270 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -2129,7 +2129,12 @@ static void vtd_mem_write(void *opaque, hwaddr addr,
>  
>  /* Fault Event Address Register, 32-bit */
>  case DMAR_FEADDR_REG:
> -assert(size == 4);
> +/*
> + * While the register is 32-bit only, some guests (Xen...) write to 
> it
> + * with 64-bit. Ignore the upper part, that's likely what the 
> hardware
> + * does as well (plus the upper part is not used by our model 
> anyway).
> + */
> +assert(size >= 4);
>  vtd_set_long(s, addr, val);
>  break;

(Sorry for the late response due to Chinese Spring Festival)

I agree with the problem there, but do we still better provide a
conditional vtd_set_quad()?  Since from the spec 10.4.13 the upper 32
bits may still be used when x2apic (Extended Interrupt Mode) is
enabled?

Thanks,

-- 
Peter Xu

Re: [Qemu-devel] [qemu-s390x] [PATCH v8 04/13] s390-ccw: update libc

[Thomas Huth]

On 21.02.2018 20:35, Collin L. Walling wrote:
> Moved:
>   memcmp from bootmap.h to libc.h (renamed from _memcmp)
>   strlen from sclp.c to libc.h (renamed from _strlen)
> 
> Added C standard functions:
>   isdigit
> 
> Added non C-standard function:
>   uitoa
>   atoui
> 
> Signed-off-by: Collin L. Walling 
> Acked-by: Christian Borntraeger 
> Reviewed-by: Janosch Frank 
> ---
>  pc-bios/s390-ccw/Makefile  |  2 +-
>  pc-bios/s390-ccw/bootmap.c |  4 +--
>  pc-bios/s390-ccw/bootmap.h | 16 +
>  pc-bios/s390-ccw/libc.c| 88 
> ++
>  pc-bios/s390-ccw/libc.h| 37 +--
>  pc-bios/s390-ccw/main.c| 17 +
>  pc-bios/s390-ccw/sclp.c| 10 +-
>  7 files changed, 129 insertions(+), 45 deletions(-)
>  create mode 100644 pc-bios/s390-ccw/libc.c

Reviewed-by: Thomas Huth 

Re: [Qemu-devel] [PATCH 0/2] Firmware blob and git submodule for Sam460ex

[David Gibson]

On Wed, Feb 21, 2018 at 06:33:42PM +, Peter Maydell wrote:
> On 21 February 2018 at 17:06, BALATON Zoltan  wrote:
> > It's not that upstream u-boot has abandoned board support (it only removed
> > support for the PPC440 CPU it once had). The board itself never had support
> > in upstream u-boot, it only exists in vendor's fork which is the reason we
> > need a separate source and cannot use upstream u-boot source we already
> > have.
> >
> > In my opinion we don't aim to take on support for this board in u-boot, we
> > only need to include the firmware binary for the emulation to be useful
> > which then requires us to also include the source for the GPL it's licensed
> > under. I've also found a few bugs in the firmware which I've fixed but apart
> > from such occasional bug fixes when needed I don't expect to take over
> > support for the board from the hardware vendor so this source is only so we
> > can include the firmware binary which is needed for the board emulation.
> > Does this answer your concerns?
> 
> We have lots of boards we don't ship firmware blobs for and
> which we expect the users to provide the guest code for
> if they're going to use them. If we had a git submodule
> for every random dev board model that needs some hardware
> vendor's BSP and bootloader we'd probably have 50 submodules...
> 
> Which isn't to say I'm definitely against this -- I'm just
> trying to figure out where we should draw the line of
> "these bits of guest code we build for you and ship with
> QEMU" versus "we provide the model of the hardware for you
> to run whatever guest code you happen to have".

So, I encouraged Zoltan to attempt this because I thought boards
without suitable firmware blobs were the exception.  If they're common
it's probably fine as is.

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 09/11] mac_newworld: use object link to pass OpenPIC object to macio

[David Gibson]

On Mon, Feb 19, 2018 at 06:19:20PM +, Mark Cave-Ayland wrote:
> Also switch macio_newworld_realize() over to use it rather than using the 
> pic_mem
> memory region directly.
> 
> Now that both Old World and New World macio devices no longer make use of the
> pic_mem memory region directly, we can remove it.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: David Gibson 

> ---
>  hw/misc/macio/macio.c | 14 +-
>  hw/ppc/mac_newworld.c | 20 +++-
>  include/hw/misc/macio/macio.h |  4 +++-
>  3 files changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index d4c1d190c4..e5288f1084 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -279,10 +279,10 @@ static void macio_newworld_realize(PCIDevice *d, Error 
> **errp)
>  sysbus_connect_irq(sysbus_dev, 0, ns->irqs[cur_irq++]);
>  sysbus_connect_irq(sysbus_dev, 1, ns->irqs[cur_irq++]);
>  
> -if (s->pic_mem) {
> -/* OpenPIC */
> -memory_region_add_subregion(>bar, 0x4, s->pic_mem);
> -}
> +/* OpenPIC */
> +sysbus_dev = SYS_BUS_DEVICE(ns->pic);
> +memory_region_add_subregion(>bar, 0x4,
> +sysbus_mmio_get_region(sysbus_dev, 0));
>  
>  /* IDE buses */
>  for (i = 0; i < ARRAY_SIZE(ns->ide); i++) {
> @@ -311,6 +311,11 @@ static void macio_newworld_init(Object *obj)
>  
>  qdev_init_gpio_out(DEVICE(obj), ns->irqs, ARRAY_SIZE(ns->irqs));
>  
> +object_property_add_link(obj, "pic", TYPE_OPENPIC,
> + (Object **) >pic,
> + qdev_prop_allow_set_link_before_realize,
> + 0, NULL);
> +
>  for (i = 0; i < 2; i++) {
>  macio_init_ide(s, >ide[i], sizeof(ns->ide[i]), i);
>  }
> @@ -441,7 +446,6 @@ void macio_init(PCIDevice *d,
>  {
>  MacIOState *macio_state = MACIO(d);
>  
> -macio_state->pic_mem = pic_mem;
>  /* Note: this code is strongly inspirated from the corresponding code
> in PearPC */
>  qdev_prop_set_uint64(DEVICE(_state->cuda), "timebase-frequency",
> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
> index 396216954e..c7960ab67a 100644
> --- a/hw/ppc/mac_newworld.c
> +++ b/hw/ppc/mac_newworld.c
> @@ -154,7 +154,7 @@ static void ppc_core99_init(MachineState *machine)
>  hwaddr kernel_base, initrd_base, cmdline_base = 0;
>  long kernel_size, initrd_size;
>  PCIBus *pci_bus;
> -PCIDevice *macio;
> +NewWorldMacIOState *macio;
>  MACIOIDEState *macio_ide;
>  BusState *adb_bus;
>  MacIONVRAMState *nvr;
> @@ -166,7 +166,7 @@ static void ppc_core99_init(MachineState *machine)
>  void *fw_cfg;
>  int machine_arch;
>  SysBusDevice *s;
> -DeviceState *dev;
> +DeviceState *dev, *pic_dev;
>  int *token = g_new(int, 1);
>  hwaddr nvram_addr = 0xFFF04000;
>  uint64_t tbfreq;
> @@ -333,10 +333,10 @@ static void ppc_core99_init(MachineState *machine)
>  
>  pic = g_new0(qemu_irq, 64);
>  
> -dev = qdev_create(NULL, TYPE_OPENPIC);
> -qdev_prop_set_uint32(dev, "model", OPENPIC_MODEL_KEYLARGO);
> -qdev_init_nofail(dev);
> -s = SYS_BUS_DEVICE(dev);
> +pic_dev = qdev_create(NULL, TYPE_OPENPIC);
> +qdev_prop_set_uint32(pic_dev, "model", OPENPIC_MODEL_KEYLARGO);
> +qdev_init_nofail(pic_dev);
> +s = SYS_BUS_DEVICE(pic_dev);
>  pic_mem = s->mmio[0].memory;
>  k = 0;
>  for (i = 0; i < smp_cpus; i++) {
> @@ -346,7 +346,7 @@ static void ppc_core99_init(MachineState *machine)
>  }
>  
>  for (i = 0; i < 64; i++) {
> -pic[i] = qdev_get_gpio_in(dev, i);
> +pic[i] = qdev_get_gpio_in(pic_dev, i);
>  }
>  
>  if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
> @@ -369,7 +369,7 @@ static void ppc_core99_init(MachineState *machine)
>  }
>  
>  /* MacIO */
> -macio = pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO);
> +macio = NEWWORLD_MACIO(pci_create(pci_bus, -1, TYPE_NEWWORLD_MACIO));
>  dev = DEVICE(macio);
>  qdev_connect_gpio_out(dev, 0, pic[0x19]); /* CUDA */
>  qdev_connect_gpio_out(dev, 1, pic[0x24]); /* ESCC-B */
> @@ -379,7 +379,9 @@ static void ppc_core99_init(MachineState *machine)
>  qdev_connect_gpio_out(dev, 5, pic[0x0e]); /* IDE */
>  qdev_connect_gpio_out(dev, 6, pic[0x03]); /* IDE DMA */
>  qdev_prop_set_uint64(dev, "frequency", tbfreq);
> -macio_init(macio, pic_mem);
> +object_property_set_link(OBJECT(macio), OBJECT(pic_dev), "pic",
> + _abort);
> +macio_init(PCI_DEVICE(macio), pic_mem);
>  
>  /* We only emulate 2 out of 3 IDE controllers for now */
>  ide_drive_get(hd, ARRAY_SIZE(hd));
> diff --git a/include/hw/misc/macio/macio.h b/include/hw/misc/macio/macio.h
> index 843c114c07..4528282b36 100644
> --- a/include/hw/misc/macio/macio.h
> +++ 

Re: [Qemu-devel] [PATCH 08/11] openpic: move OpenPIC state and related definitions to openpic.h

[David Gibson]

On Mon, Feb 19, 2018 at 06:19:19PM +, Mark Cave-Ayland wrote:
> This is to faciliate access to OpenPICState when wiring up the PIC to the 
> macio
> controller.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: David Gibson 

> ---
>  hw/intc/openpic.c| 157 --
>  include/hw/ppc/openpic.h | 160 
> ++-
>  2 files changed, 159 insertions(+), 158 deletions(-)
> 
> diff --git a/hw/intc/openpic.c b/hw/intc/openpic.c
> index 9159a06f07..811cee9b26 100644
> --- a/hw/intc/openpic.c
> +++ b/hw/intc/openpic.c
> @@ -63,10 +63,6 @@ static int get_current_cpu(void);
>  } \
>  } while (0)
>  
> -#define MAX_CPU 32
> -#define MAX_MSI 8
> -#define VID 0x03 /* MPIC version ID */
> -
>  /* OpenPIC capability flags */
>  #define OPENPIC_FLAG_IDR_CRIT (1 << 0)
>  #define OPENPIC_FLAG_ILR  (2 << 0)
> @@ -85,35 +81,6 @@ static int get_current_cpu(void);
>  #define OPENPIC_CPU_REG_START0x2
>  #define OPENPIC_CPU_REG_SIZE 0x100 + ((MAX_CPU - 1) * 0x1000)
>  
> -/* Raven */
> -#define RAVEN_MAX_CPU  2
> -#define RAVEN_MAX_EXT 48
> -#define RAVEN_MAX_IRQ 64
> -#define RAVEN_MAX_TMR  OPENPIC_MAX_TMR
> -#define RAVEN_MAX_IPI  OPENPIC_MAX_IPI
> -
> -/* KeyLargo */
> -#define KEYLARGO_MAX_CPU  4
> -#define KEYLARGO_MAX_EXT  64
> -#define KEYLARGO_MAX_IPI  4
> -#define KEYLARGO_MAX_IRQ  (64 + KEYLARGO_MAX_IPI)
> -#define KEYLARGO_MAX_TMR  0
> -#define KEYLARGO_IPI_IRQ  (KEYLARGO_MAX_EXT) /* First IPI IRQ */
> -/* Timers don't exist but this makes the code happy... */
> -#define KEYLARGO_TMR_IRQ  (KEYLARGO_IPI_IRQ + KEYLARGO_MAX_IPI)
> -
> -/* Interrupt definitions */
> -#define RAVEN_FE_IRQ (RAVEN_MAX_EXT) /* Internal functional IRQ */
> -#define RAVEN_ERR_IRQ(RAVEN_MAX_EXT + 1) /* Error IRQ */
> -#define RAVEN_TMR_IRQ(RAVEN_MAX_EXT + 2) /* First timer IRQ */
> -#define RAVEN_IPI_IRQ(RAVEN_TMR_IRQ + RAVEN_MAX_TMR) /* First IPI IRQ */
> -/* First doorbell IRQ */
> -#define RAVEN_DBL_IRQ(RAVEN_IPI_IRQ + (RAVEN_MAX_CPU * RAVEN_MAX_IPI))
> -
> -typedef struct FslMpicInfo {
> -int max_ext;
> -} FslMpicInfo;
> -
>  static FslMpicInfo fsl_mpic_20 = {
>  .max_ext = 12,
>  };
> @@ -211,55 +178,6 @@ static void openpic_cpu_write_internal(void *opaque, 
> hwaddr addr,
> uint32_t val, int idx);
>  static void openpic_reset(DeviceState *d);
>  
> -typedef enum IRQType {
> -IRQ_TYPE_NORMAL = 0,
> -IRQ_TYPE_FSLINT,/* FSL internal interrupt -- level only */
> -IRQ_TYPE_FSLSPECIAL,/* FSL timer/IPI interrupt, edge, no polarity */
> -} IRQType;
> -
> -/* Round up to the nearest 64 IRQs so that the queue length
> - * won't change when moving between 32 and 64 bit hosts.
> - */
> -#define IRQQUEUE_SIZE_BITS ((OPENPIC_MAX_IRQ + 63) & ~63)
> -
> -typedef struct IRQQueue {
> -unsigned long *queue;
> -int32_t queue_size; /* Only used for VMSTATE_BITMAP */
> -int next;
> -int priority;
> -} IRQQueue;
> -
> -typedef struct IRQSource {
> -uint32_t ivpr;  /* IRQ vector/priority register */
> -uint32_t idr;   /* IRQ destination register */
> -uint32_t destmask; /* bitmap of CPU destinations */
> -int last_cpu;
> -int output; /* IRQ level, e.g. OPENPIC_OUTPUT_INT */
> -int pending;/* TRUE if IRQ is pending */
> -IRQType type;
> -bool level:1;   /* level-triggered */
> -bool nomask:1;  /* critical interrupts ignore mask on some FSL MPICs */
> -} IRQSource;
> -
> -#define IVPR_MASK_SHIFT   31
> -#define IVPR_MASK_MASK(1U << IVPR_MASK_SHIFT)
> -#define IVPR_ACTIVITY_SHIFT   30
> -#define IVPR_ACTIVITY_MASK(1U << IVPR_ACTIVITY_SHIFT)
> -#define IVPR_MODE_SHIFT   29
> -#define IVPR_MODE_MASK(1U << IVPR_MODE_SHIFT)
> -#define IVPR_POLARITY_SHIFT   23
> -#define IVPR_POLARITY_MASK(1U << IVPR_POLARITY_SHIFT)
> -#define IVPR_SENSE_SHIFT  22
> -#define IVPR_SENSE_MASK   (1U << IVPR_SENSE_SHIFT)
> -
> -#define IVPR_PRIORITY_MASK (0xFU << 16)
> -#define IVPR_PRIORITY(_ivprr_) ((int)(((_ivprr_) & IVPR_PRIORITY_MASK) >> 
> 16))
> -#define IVPR_VECTOR(opp, _ivprr_) ((_ivprr_) & (opp)->vector_mask)
> -
> -/* IDR[EP/CI] are only for FSL MPIC prior to v4.0 */
> -#define IDR_EP  0x8000  /* external pin */
> -#define IDR_CI  0x4000  /* critical interrupt */
> -
>  /* Convert between openpic clock ticks and nanosecs.  In the hardware the 
> clock
> frequency is driven by board inputs to the PIC which the PIC would then
> divide by 4 or 8.  For now hard code to 25MZ.
> @@ -275,81 +193,6 @@ static inline uint64_t ticks_to_ns(uint64_t ticks)
>  return ticks * OPENPIC_TIMER_NS_PER_TICK;
>  }
>  
> -typedef struct OpenPICTimer {
> -uint32_t tccr;  /* Global timer current count register */
> -uint32_t tbcr;  /* Global timer 

Re: [Qemu-devel] [PATCH 10/11] macio: move setting of CUDA timebase frequency to macio_common_realize()

[David Gibson]

On Mon, Feb 19, 2018 at 06:19:21PM +, Mark Cave-Ayland wrote:
> This removes the last of the functionality from macio_init() in preparation
> for its subsequent removal.
> 
> Signed-off-by: Mark Cave-Ayland 

Reviewed-by: David Gibson 

> ---
>  hw/misc/macio/macio.c | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/misc/macio/macio.c b/hw/misc/macio/macio.c
> index e5288f1084..f71ed61819 100644
> --- a/hw/misc/macio/macio.c
> +++ b/hw/misc/macio/macio.c
> @@ -101,6 +101,8 @@ static void macio_common_realize(PCIDevice *d, Error 
> **errp)
>  memory_region_add_subregion(>bar, 0x08000,
>  sysbus_mmio_get_region(sysbus_dev, 0));
>  
> +qdev_prop_set_uint64(DEVICE(>cuda), "timebase-frequency",
> + s->frequency);
>  object_property_set_bool(OBJECT(>cuda), true, "realized", );
>  if (err) {
>  error_propagate(errp, err);
> @@ -444,12 +446,7 @@ type_init(macio_register_types)
>  void macio_init(PCIDevice *d,
>  MemoryRegion *pic_mem)
>  {
> -MacIOState *macio_state = MACIO(d);
> -
>  /* Note: this code is strongly inspirated from the corresponding code
> in PearPC */
> -qdev_prop_set_uint64(DEVICE(_state->cuda), "timebase-frequency",
> - macio_state->frequency);
> -
>  qdev_init_nofail(DEVICE(d));
>  }

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH] hw/acpi-build: build SRAT memory affinity structures for NVDIMM

[Haozhong Zhang]

On 02/21/18 14:55 +0100, Igor Mammedov wrote:
> On Tue, 20 Feb 2018 17:17:58 -0800
> Dan Williams  wrote:
> 
> > On Tue, Feb 20, 2018 at 6:10 AM, Igor Mammedov  wrote:
> > > On Sat, 17 Feb 2018 14:31:35 +0800
> > > Haozhong Zhang  wrote:
> > >  
> > >> ACPI 6.2A Table 5-129 "SPA Range Structure" requires the proximity
> > >> domain of a NVDIMM SPA range must match with corresponding entry in
> > >> SRAT table.
> > >>
> > >> The address ranges of vNVDIMM in QEMU are allocated from the
> > >> hot-pluggable address space, which is entirely covered by one SRAT
> > >> memory affinity structure. However, users can set the vNVDIMM
> > >> proximity domain in NFIT SPA range structure by the 'node' property of
> > >> '-device nvdimm' to a value different than the one in the above SRAT
> > >> memory affinity structure.
> > >>
> > >> In order to solve such proximity domain mismatch, this patch build one
> > >> SRAT memory affinity structure for each NVDIMM device with the
> > >> proximity domain used in NFIT. The remaining hot-pluggable address
> > >> space is covered by one or multiple SRAT memory affinity structures
> > >> with the proximity domain of the last node as before.
> > >>
> > >> Signed-off-by: Haozhong Zhang   
> > > If we consider hotpluggable system, correctly implemented OS should
> > > be able pull proximity from Device::_PXM and override any value from SRAT.
> > > Do we really have a problem here (anything that breaks if we would use 
> > > _PXM)?
> > > Maybe we should add _PXM object to nvdimm device nodes instead of 
> > > massaging SRAT?  
> > 
> > Unfortunately _PXM is an awkward fit. Currently the proximity domain
> > is attached to the SPA range structure. The SPA range may be
> > associated with multiple DIMM devices and those individual NVDIMMs may
> > have conflicting _PXM properties.
> There shouldn't be any conflict here as  NVDIMM device's _PXM method,
> should override in runtime any proximity specified by parent scope.
> (as parent scope I'd also count boot time NFIT/SRAT tables).
> 
> To make it more clear we could clear valid proximity domain flag in SPA
> like this:
> 
> diff --git a/hw/acpi/nvdimm.c b/hw/acpi/nvdimm.c
> index 59d6e42..131bca5 100644
> --- a/hw/acpi/nvdimm.c
> +++ b/hw/acpi/nvdimm.c
> @@ -260,9 +260,7 @@ nvdimm_build_structure_spa(GArray *structures, 
> DeviceState *dev)
>   */
>  nfit_spa->flags = cpu_to_le16(1 /* Control region is strictly for
> management during hot add/online
> -   operation */ |
> -  2 /* Data in Proximity Domain field is
> -   valid*/);
> +   operation */);
>  
>  /* NUMA node. */
>  nfit_spa->proximity_domain = cpu_to_le32(node);
> 
> > Even if that was unified across
> > DIMMs it is ambiguous whether a DIMM-device _PXM would relate to the
> > device's control interface, or the assembled persistent memory SPA
> > range.
> I'm not sure what you mean under 'device's control interface',
> could you clarify where the ambiguity comes from?
> 
> I read spec as: _PXM applies to address range covered by NVDIMM
> device it belongs to.
> 
> As for assembled SPA, I'd assume that it applies to interleaved set
> and all NVDIMMs with it should be on the same node. It's somewhat
> irrelevant question though as QEMU so far implements only
>   1:1:1/SPA:Region Mapping:NVDIMM Device/
> mapping.
> 
> My main concern with using static configuration tables for proximity
> mapping, we'd miss on hotplug side of equation. However if we start
> from dynamic side first, we could later complement it with static
> tables if there really were need for it.

This patch affects only the static tables and static-plugged NVDIMM.
For hot-plugged NVDIMMs, guest OSPM still needs to evaluate _FIT to
get the information of the new NVDIMMs including their proximity
domains.

One intention of this patch is to simulate the bare metal as much as
possible. I have been using this patch to develop and test NVDIMM
enabling work on Xen, and think it might be useful for developers of
other OS and hypervisors.


Haozhong

Re: [Qemu-devel] [PATCH v4 00/22] re-factor softfloat and add fp16 functions

[Fam Zheng]

On Mon, 02/19 13:56, Peter Maydell wrote:
> On 17 February 2018 at 13:23, Alex Bennée  wrote:
> > Peter Maydell  writes:
> >> If you persuade git to use the --minimal, --patience or --histogram
> >> git diff option when generating these patches you'll find that it
> >> doesn't produce unreadable patches that provoke all the checkpatch
> >> warnings.
> >
> > I think this is patchew getting confused
> 
> Oh yes, sorry. Fam, can we update patchew's git config to use
>   git config --global diff.algorithm histogram
> (equivalently, algorithm = histogram in the .gitconfig) -- that
> way the patches it runs checkpatch on are more likely to be
> formatted so that they minimise spurious checkpatch output, I think.

Updated. (With s/--global/--local/ along with other diff options, in the testing
command list:

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

)

Fam

[Qemu-devel] [PATCH v2 2/3] qcow2: Don't allow overflow during cluster allocation

[Eric Blake]

Our code was already checking that we did not attempt to
allocate more clusters than what would fit in an INT64 (the
physical maximimum if we can access a full off_t's worth of
data).  But this does not catch smaller limits enforced by
various spots in the qcow2 image description: L1 and normal
clusters of L2 are documented as having bits 63-56 reserved
for other purposes, capping our maximum offset at 64PB (bit
55 is the maximum bit set).  And for compressed images with
2M clusters, the cap drops the maximum offset to bit 48, or
a maximum offset of 512TB.  If we overflow that offset, we
would write compressed data into one place, but try to
decompress from another, which won't work.

I don't have 512TB handy to prove whether things break if we
compress so much data that we overflow that limit, and don't
think that iotests can (quickly) test it either.  Test 138
comes close (it corrupts an image into thinking something lives
at 32PB, which is half the maximum for L1 sizing - although
it relies on 512-byte clusters).  But that test points out
that we will generally hit other limits first (such as running
out of memory for the refcount table, or exceeding file system
limits like 16TB on ext4, etc), so this is more a theoretical
safety valve than something likely to be hit.

Signed-off-by: Eric Blake 
---
 block/qcow2.h  |  6 ++
 block/qcow2-refcount.c | 20 +---
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/block/qcow2.h b/block/qcow2.h
index 883802241fb..560008c331d 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -41,6 +41,12 @@
 #define QCOW_MAX_CRYPT_CLUSTERS 32
 #define QCOW_MAX_SNAPSHOTS 65536

+/* Field widths in qcow2 mean normal cluster offsets cannot reach
+ * 64PB; depending on cluster size, compressed clusters can have a
+ * smaller limit (64PB for up to 16k clusters, then ramps down to
+ * 512TB for 2M clusters).  */
+#define QCOW_MAX_CLUSTER_OFFSET ((1ULL << 56) - 1)
+
 /* 8 MB refcount table is enough for 2 PB images at 64k cluster size
  * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
 #define QCOW_MAX_REFTABLE_SIZE 0x80
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 28afbb1b5ea..58c19789a22 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -31,7 +31,8 @@
 #include "qemu/bswap.h"
 #include "qemu/cutils.h"

-static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
+static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size,
+uint64_t max);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
 int64_t offset, int64_t length, uint64_t addend,
 bool decrease, enum qcow2_discard_type type);
@@ -362,7 +363,8 @@ static int alloc_refcount_block(BlockDriverState *bs,
 }

 /* Allocate the refcount block itself and mark it as used */
-int64_t new_block = alloc_clusters_noref(bs, s->cluster_size);
+int64_t new_block = alloc_clusters_noref(bs, s->cluster_size,
+ QCOW_MAX_CLUSTER_OFFSET);
 if (new_block < 0) {
 return new_block;
 }
@@ -947,7 +949,8 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs,


 /* return < 0 if error */
-static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size)
+static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size,
+uint64_t max)
 {
 BDRVQcow2State *s = bs->opaque;
 uint64_t i, nb_clusters, refcount;
@@ -972,9 +975,9 @@ retry:
 }

 /* Make sure that all offsets in the "allocated" range are representable
- * in an int64_t */
+ * in the requested max */
 if (s->free_cluster_index > 0 &&
-s->free_cluster_index - 1 > (INT64_MAX >> s->cluster_bits))
+s->free_cluster_index - 1 > (max >> s->cluster_bits))
 {
 return -EFBIG;
 }
@@ -994,7 +997,7 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t 
size)

 BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC);
 do {
-offset = alloc_clusters_noref(bs, size);
+offset = alloc_clusters_noref(bs, size, QCOW_MAX_CLUSTER_OFFSET);
 if (offset < 0) {
 return offset;
 }
@@ -1076,7 +1079,10 @@ int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size)
 free_in_cluster = s->cluster_size - offset_into_cluster(s, offset);
 do {
 if (!offset || free_in_cluster < size) {
-int64_t new_cluster = alloc_clusters_noref(bs, s->cluster_size);
+int64_t new_cluster;
+
+new_cluster = alloc_clusters_noref(bs, s->cluster_size,
+   (1ULL << s->csize_shift) - 1);
 if (new_cluster < 0) {
 return new_cluster;
 }
-- 
2.14.3

[Qemu-devel] [PATCH v2 1/3] qcow2: Prefer byte-based calls into bs->file

[Eric Blake]

We had only three sector-based stragglers left; convert them to use
our preferred byte-based accesses.

Signed-off-by: Eric Blake 
Reviewed-by: Alberto Garcia 

---
v2: indentation fix
---
 block/qcow2-cluster.c  | 5 ++---
 block/qcow2-refcount.c | 6 +++---
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index e406b0f3b9e..85be7d5e340 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1615,13 +1615,12 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
uint64_t cluster_offset)
 }

 BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
-ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
-nb_csectors);
+ret = bdrv_pread(bs->file, coffset, s->cluster_data, csize);
 if (ret < 0) {
 return ret;
 }
 if (decompress_buffer(s->cluster_cache, s->cluster_size,
-  s->cluster_data + sector_offset, csize) < 0) {
+  s->cluster_data, csize) < 0) {
 return -EIO;
 }
 s->cluster_cache_offset = coffset;
diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index d46b69d7f34..28afbb1b5ea 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -2310,8 +2310,8 @@ write_refblocks:
 on_disk_refblock = (void *)((char *) *refcount_table +
 refblock_index * s->cluster_size);

-ret = bdrv_write(bs->file, refblock_offset / BDRV_SECTOR_SIZE,
- on_disk_refblock, s->cluster_sectors);
+ret = bdrv_pwrite(bs->file, refblock_offset, on_disk_refblock,
+  s->cluster_size);
 if (ret < 0) {
 fprintf(stderr, "ERROR writing refblock: %s\n", strerror(-ret));
 goto fail;
@@ -2533,7 +2533,7 @@ fail:
  * - 0 if writing to this offset will not affect the mentioned metadata
  * - a positive QCow2MetadataOverlap value indicating one overlapping section
  * - a negative value (-errno) indicating an error while performing a check,
- *   e.g. when bdrv_read failed on QCOW2_OL_INACTIVE_L2
+ *   e.g. when bdrv_pread failed on QCOW2_OL_INACTIVE_L2
  */
 int qcow2_check_metadata_overlap(BlockDriverState *bs, int ign, int64_t offset,
  int64_t size)
-- 
2.14.3

[Qemu-devel] [PATCH v2 3/3] qcow2: Avoid memory over-allocation on compressed images

[Eric Blake]

When reading a compressed image, we were allocating s->cluster_data
to 32*cluster_size + 512 (possibly over 64 megabytes, for an image
with 2M clusters).  Let's check out the history:

Back when qcow2 was first written, we used s->cluster_data for
everything, including copy_sectors() and encryption, where we want
to operate on more than one cluster at once.  Obviously, at that
point, the buffer had to be aligned for other users, even though
compression itself doesn't require any alignment (the fact that
the compressed data generally starts mid-sector means that aligning
our buffer buys us nothing - either the protocol already supports
byte-based access into whatever offset we want, or we are already
using a bounce buffer to read a full sector, and copying into
our destination no longer requires alignment).

But commit 1b9f1491 (v1.1!) changed things to allocate parallel
buffers on demand rather than sharing a single buffer, for encryption
and COW, leaving compression as the final client of s->cluster_data.
That use was still preserved, because if a single compressed cluster
is read more than once, we reuse the cache instead of decompressing
it a second time (someday, we may come up with better caching to
avoid wasting repeated decompressions while still being more parallel,
but that is a task for another patch; the XXX comment in
qcow2_co_preadv for QCOW2_CLUSTER_COMPRESSED is telling).

Much later, in commit de82815d (v2.2), we noticed that a 64M
allocation is prone to failure, so we switched over to a graceful
memory allocation error message.  Elsewhere in the code, we do
g_malloc(2 * cluster_size) without ever checking for failure, but
even 4M starts to be large enough that trying to be nice is worth
the effort, so we want to keep that aspect.

Then even later, in 3e4c7052 (2.11), we realized that allocating
a large buffer up front for every qcow2 image is expensive, and
switched to lazy allocation only for images that actually had
compressed clusters.  But in the process, we never even bothered
to check whether what we were allocating still made sense in its
new context!

So, it's time to cut back on the waste.  A compressed cluster
written by qemu will NEVER occupy more than an uncompressed
cluster, but based on mid-sector alignment, we may still need
to read 1 cluster + 1 sector in order to recover enough bytes
for the decompression.  But third-party producers of qcow2 may
not be as smart, and gzip DOES document that because the
compression stream adds metadata, and because of the pigeonhole
principle, there are worst case scenarios where attempts to
compress will actually inflate an image, by up to 0.015% (or 62
sectors larger for an unfortunate 2M compression).  In fact,
the qcow2 spec permits up to 2 full clusters of sectors beyond
the initial offset; and the way decompression works, it really
doesn't matter if we read too much (gzip ignores slop, once it
has decoded a full cluster), so it's feasible to encounter a
third-party image that reports the maximum 'nb_csectors'
possible, even if it no longer has any bearing to the actual
compressed size.  So it's easier to just allocate cluster_data
to be as large as we can ever possibly see; even if it still
wastes up to 2M on any image created by qcow2, that's still an
improvment of 60M less waste than pre-patch.

Signed-off-by: Eric Blake 

---
v2: actually check allocation failure (previous version meant
to use g_malloc, but ended up posted with g_try_malloc without
checking); add assertions outside of conditional, improve
commit message to better match reality now that qcow2 spec bug
has been fixed
---
 block/qcow2-cluster.c | 27 ++-
 block/qcow2.c |  2 +-
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 85be7d5e340..7d5276b5f6b 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -1598,20 +1598,29 @@ int qcow2_decompress_cluster(BlockDriverState *bs, 
uint64_t cluster_offset)
 sector_offset = coffset & 511;
 csize = nb_csectors * 512 - sector_offset;

-/* Allocate buffers on first decompress operation, most images are
- * uncompressed and the memory overhead can be avoided.  The buffers
- * are freed in .bdrv_close().
+/* Allocate buffers on the first decompress operation; most
+ * images are uncompressed and the memory overhead can be
+ * avoided.  The buffers are freed in .bdrv_close().  qemu
+ * never writes an inflated cluster, and gzip itself never
+ * inflates a problematic cluster by more than 0.015%, but the
+ * qcow2 format allows up to 2 full clusters beyond the sector
+ * containing offset, and gzip ignores trailing slop, so it's
+ * easier to just allocate that much up front than to reject
+ * third-party images with overlarge csize.
  */
+assert(!!s->cluster_data == 

[Qemu-devel] [PATCH v2 0/3] qcow2: minor compression improvements

[Eric Blake]

Updates to v1:
- fix whitespace [Berto]
- fix g_try_malloc usage [Berto, Kevin]
- improve comments [Berto, Kevin]
- add a patch to avoid overflow on 512TB images with 2M clusters

Eric Blake (3):
  qcow2: Prefer byte-based calls into bs->file
  qcow2: Don't allow overflow during cluster allocation
  qcow2: Avoid memory over-allocation on compressed images

 block/qcow2.h  |  6 ++
 block/qcow2-cluster.c  | 32 
 block/qcow2-refcount.c | 26 --
 block/qcow2.c  |  2 +-
 4 files changed, 43 insertions(+), 23 deletions(-)

-- 
2.14.3

Re: [Qemu-devel] [RFC PATCH v6 00/20] replay additions

[Ciro Santilli]

On Wed, Feb 21, 2018 at 6:41 AM, Pavel Dovgalyuk  wrote:
>> From: Ciro Santilli [mailto:ciro.santi...@gmail.com]
>> On Tue, Feb 20, 2018 at 9:46 AM, Pavel Dovgalyuk  wrote:
>> >
>> > Updated the branch on github.
>> > You may try it.
>>
>> At 8a482834780a131e7747c1c3c1931379ed0beedc ARM initrd record runs,
>> but replay is getting stuck at:
>>
>> [   12.120424] scsi host0: sym-2.2.3
>>
>> Neighboring lines on record:
>>
>> [   11.346357] sym53c8xx :00:0c.0: enabling device (0100 -> 0103)
>> [   11.536683] sym0: <895a> rev 0x0 at pci :00:0c.0 irq 66
>> [   11.731679] sym0: No NVRAM, ID 7, Fast-40, LVD, parity checking
>> [   11.930599] sym0: SCSI BUS has been reset.
>> [   12.120424] scsi host0: sym-2.2.3
>> [   15.451809] scsi 0:0:2:0: CD-ROMQEMU QEMU CD-ROM
>>   2.5+ PQ: 0 ANSI: 5
>> [   15.847227] scsi target0:0:2: tagged command queuing enabled,
>> command queue depth 16.
>> [   16.256585] scsi target0:0:2: Beginning Domain Validation
>> [   16.482189] scsi target0:0:2: Domain Validation skipping write tests
>> [   16.699445] scsi target0:0:2: Ending Domain Validation
>>
>> My QEMU command:
>>
>> time ./buildroot/output.arm~/host/usr/bin/qemu-system-arm -M
>> versatilepb -append 'root=/dev/sda nokaslr norandmaps
>> printk.devkmsg=on printk.time=y - lkmc_eval="/rand_check.out;wget -S
>> google.com;/poweroff.out;"'
>>  -kernel ./buildroot/output.arm~/images/zImage -dtb
>> ./buildroot/output.arm~/images/versatile-pb.dtb -nographic -initrd
>> ./buildroot/output.arm~/images/rootfs.cpio -netdev user,id=net1
>> -device rtl8139,netdev=net1
>> -object filter-replay,id=replay,netdev=net1
>>
>> What is your full QEMU command?
>
> I used your previous command and encountered kernel panic in guest.
> What is rootfs.cpio file? Is it the renamed rootfs.ext2 from your images.zip?
>

rootfs.cpio is the cpio version of rootfs.ext2, both should represent
the same filesystem in different serialized formats.

I've added it to this updated zip:
https://github.com/cirosantilli/linux-kernel-module-cheat/releases/download/test-replay-arm/images2.zip

The previous .zip didn't have the .cpio, then I later made Buildroot
generate it as well to try out the -initrd method.

I'm using this branch for my testing:
https://github.com/cirosantilli/linux-kernel-module-cheat/tree/rr with
"./build -a arm && ./recarm"

>> Also I think the patch to fix qmeu-img was not included in the branch.
>
> Forgot about it. Now it should be ok.
>
>
> Pavel Dovgalyuk
>

Re: [Qemu-devel] [PATCHv3 1/2] translator: merge max_insns into DisasContextBase

[Emilio G. Cota]

On Wed, Feb 21, 2018 at 13:05:45 -0800, Richard Henderson wrote:
> On 02/21/2018 12:55 PM, Emilio G. Cota wrote:
> > While at it, use int for both num_insns and max_insns to make
> > sure we have same-type comparisons.
> > 
> > Signed-off-by: Emilio G. Cota 
> > ---
> >  accel/tcg/translator.c | 21 ++---
> >  include/exec/translator.h  |  8 
> >  target/alpha/translate.c   |  6 ++
> >  target/arm/translate-a64.c |  8 +++-
> >  target/arm/translate.c |  9 +++--
> >  target/hppa/translate.c|  7 ++-
> >  target/i386/translate.c|  5 +
> >  target/ppc/translate.c |  5 ++---
> >  8 files changed, 27 insertions(+), 42 deletions(-)
> 
> Reviewed-by: Richard Henderson 

Thanks.

To avoid merge conflicts, I'll send v2's of the other conversions
once this patch goes in.

Emilio

Re: [Qemu-devel] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

[Eric Blake]

On 02/21/2018 08:08 AM, Alberto Garcia wrote:

This patch fixes several mistakes in the documentation of the
compressed cluster descriptor:



More things to consider, as followup patches:

Note that both the L1 table, and the standard L2 descriptors, have a cap 
on bit 55 as the maximum host offset (< 64PB).  But for compressed 
clusters, our maximum depends on cluster_bits, as follows:


512 byte cluster: bit 61 forms 'number clusters', leaving bits 60-0 for 
computing the host offset.  But even though this looks on the surface 
like you could allocate 2EB of compressed clusters, it does not play 
well with the rest of the L1/L2 system, so we should probably explicitly 
document that bits 60-56 MUST be 0, if they are assigned to the 'host 
offset field', making the maximum compressed cluster offset at 64PB.


2M cluster: bits 61-50 form 'number clusters', leaving bit 49 as the 
maximum bit in the host offset (< 512 TB).  But we never validate that 
we don't overflow this value!  I'm working on a patch.


Meanwhile, the refcount table currently allows all the way up to bit 63 
to form an offset to a refcount block, although capping that at 55 the 
way L1/L2 are capped would be reasonable (it gets weird to state that 
your metadata must live below 64PB but that your data pointed to by the 
metadata can live beyond that point).  So it may also be worth 
considering a spec patch that points out the 64PB maximum useful size, 
and maybe even a comment that the maximum size may be further 
constrained by the protocol layer (for example, ext4 has a 16TB cap on 
individual file size).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH v8 11/13] s390-ccw: set cp_receive mask only when needed and consume pending service irqs

[Collin L. Walling]

It is possible while waiting for multiple types of external
interrupts that we might have pending irqs remaining between
irq consumption and irq-type disabling. Those interrupts
could potentially propagate to the guest after IPL completes
and cause unwanted behavior.

As it is today, the SCLP will only recognize write events that
are enabled by the control program's send and receive masks. To
limit the window for, and prevent further irqs from, ASCII
console events (specifically keystrokes), we should only enable
the control program's receive mask when we need it.

While we're at it, remove assignment of the (non control program)
send and receive masks, as those are actually set by the SCLP.

Signed-off-by: Collin L. Walling 
---
 pc-bios/s390-ccw/menu.c |  5 +
 pc-bios/s390-ccw/s390-ccw.h |  1 +
 pc-bios/s390-ccw/sclp.c | 10 --
 3 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 01b120d..4f84bfe 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -11,6 +11,7 @@
 
 #include "libc.h"
 #include "s390-ccw.h"
+#include "sclp.h"
 
 #define KEYCODE_NO_INP '\0'
 #define KEYCODE_ESCAPE '\033'
@@ -116,8 +117,12 @@ static int get_index(void)
 
 memset(buf, 0, sizeof(buf));
 
+sclp_set_write_mask(SCLP_EVENT_MASK_MSG_ASCII, SCLP_EVENT_MASK_MSG_ASCII);
+
 len = read_prompt(buf, sizeof(buf) - 1);
 
+sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII);
+
 /* If no input, boot default */
 if (len == 0) {
 return 0;
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index f7c12bf..56b0c2a 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -69,6 +69,7 @@ unsigned int get_loadparm_index(void);
 
 /* sclp.c */
 void sclp_print(const char *string);
+void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask);
 void sclp_setup(void);
 void sclp_get_loadparm_ascii(char *loadparm);
 int sclp_read(char *str, size_t count);
diff --git a/pc-bios/s390-ccw/sclp.c b/pc-bios/s390-ccw/sclp.c
index a2f25eb..3836cb4 100644
--- a/pc-bios/s390-ccw/sclp.c
+++ b/pc-bios/s390-ccw/sclp.c
@@ -46,23 +46,21 @@ static int sclp_service_call(unsigned int command, void 
*sccb)
 return 0;
 }
 
-static void sclp_set_write_mask(void)
+void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask)
 {
 WriteEventMask *sccb = (void *)_sccb;
 
 sccb->h.length = sizeof(WriteEventMask);
 sccb->mask_length = sizeof(unsigned int);
-sccb->receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
-sccb->cp_receive_mask = SCLP_EVENT_MASK_MSG_ASCII;
-sccb->send_mask = SCLP_EVENT_MASK_MSG_ASCII;
-sccb->cp_send_mask = SCLP_EVENT_MASK_MSG_ASCII;
+sccb->cp_receive_mask = receive_mask;
+sccb->cp_send_mask = send_mask;
 
 sclp_service_call(SCLP_CMD_WRITE_EVENT_MASK, sccb);
 }
 
 void sclp_setup(void)
 {
-sclp_set_write_mask();
+sclp_set_write_mask(0, SCLP_EVENT_MASK_MSG_ASCII);
 }
 
 long write(int fd, const void *str, size_t len)
-- 
2.7.4

Re: [Qemu-devel] [PATCHv2 1/2] translator: merge max_insns into DisasContextBase

[Emilio G. Cota]

On Wed, Feb 21, 2018 at 10:19:05 -0800, Richard Henderson wrote:
> On 02/19/2018 06:26 PM, Emilio G. Cota wrote:
> > @@ -68,6 +69,7 @@ typedef struct DisasContextBase {
> >  target_ulong pc_next;
> >  DisasJumpType is_jmp;
> >  unsigned int num_insns;
> > +int max_insns;
> >  bool singlestep_enabled;
> >  } DisasContextBase;
> 
> We really should pick the same type for max_insns and num_insns, which ever
> type we settle on.  I can't see how we can go wrong with unsigned...

I was just trying to avoid warnings with -Wsign-compare in case
we ever enabled it.

Should I bother converting the "bound" variables we use for
MIN(max_insns, bound) to unsigned as well? Or just leave them
alone and forget about -Wsign-compare?

Thanks,

Emilio

Re: [Qemu-devel] [PATCHv3 1/2] translator: merge max_insns into DisasContextBase

[Richard Henderson]

On 02/21/2018 12:55 PM, Emilio G. Cota wrote:
> While at it, use int for both num_insns and max_insns to make
> sure we have same-type comparisons.
> 
> Signed-off-by: Emilio G. Cota 
> ---
>  accel/tcg/translator.c | 21 ++---
>  include/exec/translator.h  |  8 
>  target/alpha/translate.c   |  6 ++
>  target/arm/translate-a64.c |  8 +++-
>  target/arm/translate.c |  9 +++--
>  target/hppa/translate.c|  7 ++-
>  target/i386/translate.c|  5 +
>  target/ppc/translate.c |  5 ++---
>  8 files changed, 27 insertions(+), 42 deletions(-)

Reviewed-by: Richard Henderson 


r~

Re: [Qemu-devel] [PATCH v2 10/36] test-qemu-opts: Test qemu_opts_to_qdict_filtered()

[Eric Blake]

On 02/21/2018 07:53 AM, Kevin Wolf wrote:

Signed-off-by: Kevin Wolf 
---
  tests/test-qemu-opts.c | 125 +
  1 file changed, 125 insertions(+)

diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
index 6c3183390b..2c422abcd4 100644
--- a/tests/test-qemu-opts.c
+++ b/tests/test-qemu-opts.c
@@ -10,6 +10,7 @@
  #include "qemu/osdep.h"
  #include "qemu/cutils.h"
  #include "qemu/option.h"
+#include "qemu/option_int.h"
  #include "qapi/error.h"
  #include "qapi/qmp/qdict.h"
  #include "qapi/qmp/qstring.h"
@@ -868,6 +869,127 @@ static void test_opts_append(void)
  qemu_opts_free(merged);
  }
  
+static void test_opts_to_qdict_basic(void)

+{
+QemuOpts *opts;
+QDict *dict;
+
+opts = qemu_opts_parse(_list_01, "str1=foo,str2=,str3=bar,number1=42",
+   false, _abort);


Worth any additional craziness in regards to our QemuOpts parsing, like 
str1=foo,,bar,str2... for an option containing commas, or 
str2=,str1=foo, for supplying options in a different order than the 
list?  But what you have is a good addition even if you don't tweak it.


Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCH] tcg/i386: Support INDEX_op_dup2_vec for -m32

[Richard Henderson]

Unknown why -m32 was passing with gcc but not clang; it should have
failed for both.  This would be used for tcg_gen_dup_i64_vec, and
visible with the right TB and an aarch64 guest.

Reported-by: Max Reitz 
Signed-off-by: Richard Henderson 
---
 tcg/i386/tcg-target.inc.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/tcg/i386/tcg-target.inc.c b/tcg/i386/tcg-target.inc.c
index 45943e540c..5e8f59dc47 100644
--- a/tcg/i386/tcg-target.inc.c
+++ b/tcg/i386/tcg-target.inc.c
@@ -2696,6 +2696,12 @@ static void tcg_out_vec_op(TCGContext *s, TCGOpcode opc,
 case INDEX_op_x86_packus_vec:
 insn = packus_insn[vece];
 goto gen_simd;
+#if TCG_TARGET_REG_BITS == 32
+case INDEX_op_dup2_vec:
+/* Constraints have already placed both 32-bit inputs in xmm regs.  */
+insn = OPC_PUNPCKLDQ;
+goto gen_simd;
+#endif
 gen_simd:
 tcg_debug_assert(insn != OPC_UD2);
 if (type == TCG_TYPE_V256) {
@@ -3045,6 +3051,9 @@ static const TCGTargetOpDef *tcg_target_op_def(TCGOpcode 
op)
 case INDEX_op_x86_vperm2i128_vec:
 case INDEX_op_x86_punpckl_vec:
 case INDEX_op_x86_punpckh_vec:
+#if TCG_TARGET_REG_BITS == 32
+case INDEX_op_dup2_vec:
+#endif
 return _x_x;
 case INDEX_op_dup_vec:
 case INDEX_op_shli_vec:
-- 
2.14.3

[Qemu-devel] [PATCHv3 2/2] target/sh4: convert to TranslatorOps

[Emilio G. Cota]

This was fairly straightforward since it had already been converted
to DisasContextBase; just had to add TARGET_TOO_MANY to the switch
in tb_stop.

Reviewed-by: Richard Henderson 
Signed-off-by: Emilio G. Cota 
---
 target/sh4/translate.c | 171 +
 1 file changed, 86 insertions(+), 85 deletions(-)

diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 012156b..58bdfeb 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -2258,126 +2258,127 @@ static int decode_gusa(DisasContext *ctx, CPUSH4State 
*env, int *pmax_insns)
 }
 #endif
 
-void gen_intermediate_code(CPUState *cs, struct TranslationBlock *tb)
+static void sh4_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs)
 {
+DisasContext *ctx = container_of(dcbase, DisasContext, base);
 CPUSH4State *env = cs->env_ptr;
-DisasContext ctx;
-target_ulong pc_start;
-int num_insns;
-int max_insns;
-
-pc_start = tb->pc;
-ctx.base.pc_next = pc_start;
-ctx.tbflags = (uint32_t)tb->flags;
-ctx.envflags = tb->flags & TB_FLAG_ENVFLAGS_MASK;
-ctx.base.is_jmp = DISAS_NEXT;
-ctx.memidx = (ctx.tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
+int bound;
+
+ctx->tbflags = (uint32_t)ctx->base.tb->flags;
+ctx->envflags = ctx->base.tb->flags & TB_FLAG_ENVFLAGS_MASK;
+ctx->memidx = (ctx->tbflags & (1u << SR_MD)) == 0 ? 1 : 0;
 /* We don't know if the delayed pc came from a dynamic or static branch,
so assume it is a dynamic branch.  */
-ctx.delayed_pc = -1; /* use delayed pc from env pointer */
-ctx.base.tb = tb;
-ctx.base.singlestep_enabled = cs->singlestep_enabled;
-ctx.features = env->features;
-ctx.has_movcal = (ctx.tbflags & TB_FLAG_PENDING_MOVCA);
-ctx.gbank = ((ctx.tbflags & (1 << SR_MD)) &&
- (ctx.tbflags & (1 << SR_RB))) * 0x10;
-ctx.fbank = ctx.tbflags & FPSCR_FR ? 0x10 : 0;
-
-max_insns = tb_cflags(tb) & CF_COUNT_MASK;
-if (max_insns == 0) {
-max_insns = CF_COUNT_MASK;
-}
-max_insns = MIN(max_insns, TCG_MAX_INSNS);
+ctx->delayed_pc = -1; /* use delayed pc from env pointer */
+ctx->features = env->features;
+ctx->has_movcal = (ctx->tbflags & TB_FLAG_PENDING_MOVCA);
+ctx->gbank = ((ctx->tbflags & (1 << SR_MD)) &&
+  (ctx->tbflags & (1 << SR_RB))) * 0x10;
+ctx->fbank = ctx->tbflags & FPSCR_FR ? 0x10 : 0;
 
 /* Since the ISA is fixed-width, we can bound by the number
of instructions remaining on the page.  */
-num_insns = -(ctx.base.pc_next | TARGET_PAGE_MASK) / 2;
-max_insns = MIN(max_insns, num_insns);
-
-/* Single stepping means just that.  */
-if (ctx.base.singlestep_enabled || singlestep) {
-max_insns = 1;
-}
-
-gen_tb_start(tb);
-num_insns = 0;
+bound = -(ctx->base.pc_next | TARGET_PAGE_MASK) / 2;
+ctx->base.max_insns = MIN(ctx->base.max_insns, bound);
+}
 
+static void sh4_tr_tb_start(DisasContextBase *dcbase, CPUState *cs)
+{
 #ifdef CONFIG_USER_ONLY
-if (ctx.tbflags & GUSA_MASK) {
-num_insns = decode_gusa(, env, _insns);
+DisasContext *ctx = container_of(dcbase, DisasContext, base);
+CPUSH4State *env = cs->env_ptr;
+
+if (ctx->tbflags & GUSA_MASK) {
+ctx->base.num_insns = decode_gusa(ctx, env, >base.max_insns);
 }
 #endif
+}
 
-while (ctx.base.is_jmp == DISAS_NEXT
-   && num_insns < max_insns
-   && !tcg_op_buf_full()) {
-tcg_gen_insn_start(ctx.base.pc_next, ctx.envflags);
-num_insns++;
+static void sh4_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
+{
+DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
-if (unlikely(cpu_breakpoint_test(cs, ctx.base.pc_next, BP_ANY))) {
-/* We have hit a breakpoint - make sure PC is up-to-date */
-gen_save_cpu_state(, true);
-gen_helper_debug(cpu_env);
-ctx.base.is_jmp = DISAS_NORETURN;
-/* The address covered by the breakpoint must be included in
-   [tb->pc, tb->pc + tb->size) in order to for it to be
-   properly cleared -- thus we increment the PC here so that
-   the logic setting tb->size below does the right thing.  */
-ctx.base.pc_next += 2;
-break;
-}
+tcg_gen_insn_start(ctx->base.pc_next, ctx->envflags);
+}
 
-if (num_insns == max_insns && (tb_cflags(tb) & CF_LAST_IO)) {
-gen_io_start();
-}
+static bool sh4_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
+const CPUBreakpoint *bp)
+{
+DisasContext *ctx = container_of(dcbase, DisasContext, base);
 
-ctx.opcode = cpu_lduw_code(env, ctx.base.pc_next);
-   decode_opc();
-ctx.base.pc_next += 2;
-}
-if (tb_cflags(tb) & CF_LAST_IO) {
-gen_io_end();
-}
+/* We have hit a 

Re: [Qemu-devel] [PATCH v2 09/36] test-qemu-opts: Test qemu_opts_append()

[Eric Blake]

On 02/21/2018 07:53 AM, Kevin Wolf wrote:

Basic test for merging two QemuOptsLists.

Signed-off-by: Kevin Wolf 
---
  tests/test-qemu-opts.c | 128 +
  1 file changed, 128 insertions(+)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [PATCHv3 0/2] target/sh4: translator loop conversion

[Emilio G. Cota]

Changes from v2:
- Use int for both num_insns and max_insns, which involves no churn
  compared to converting everything to unsigned.

Thanks,

Emilio

[Qemu-devel] [PATCHv3 1/2] translator: merge max_insns into DisasContextBase

[Emilio G. Cota]

While at it, use int for both num_insns and max_insns to make
sure we have same-type comparisons.

Signed-off-by: Emilio G. Cota 
---
 accel/tcg/translator.c | 21 ++---
 include/exec/translator.h  |  8 
 target/alpha/translate.c   |  6 ++
 target/arm/translate-a64.c |  8 +++-
 target/arm/translate.c |  9 +++--
 target/hppa/translate.c|  7 ++-
 target/i386/translate.c|  5 +
 target/ppc/translate.c |  5 ++---
 8 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index 23c6602..0f9dca9 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -34,8 +34,6 @@ void translator_loop_temp_check(DisasContextBase *db)
 void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
  CPUState *cpu, TranslationBlock *tb)
 {
-int max_insns;
-
 /* Initialize DisasContext */
 db->tb = tb;
 db->pc_first = tb->pc;
@@ -45,18 +43,18 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 db->singlestep_enabled = cpu->singlestep_enabled;
 
 /* Instruction counting */
-max_insns = tb_cflags(db->tb) & CF_COUNT_MASK;
-if (max_insns == 0) {
-max_insns = CF_COUNT_MASK;
+db->max_insns = tb_cflags(db->tb) & CF_COUNT_MASK;
+if (db->max_insns == 0) {
+db->max_insns = CF_COUNT_MASK;
 }
-if (max_insns > TCG_MAX_INSNS) {
-max_insns = TCG_MAX_INSNS;
+if (db->max_insns > TCG_MAX_INSNS) {
+db->max_insns = TCG_MAX_INSNS;
 }
 if (db->singlestep_enabled || singlestep) {
-max_insns = 1;
+db->max_insns = 1;
 }
 
-max_insns = ops->init_disas_context(db, cpu, max_insns);
+ops->init_disas_context(db, cpu);
 tcg_debug_assert(db->is_jmp == DISAS_NEXT);  /* no early exit */
 
 /* Reset the temp count so that we can identify leaks */
@@ -95,7 +93,8 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
update db->pc_next and db->is_jmp to indicate what should be
done next -- either exiting this loop or locate the start of
the next instruction.  */
-if (db->num_insns == max_insns && (tb_cflags(db->tb) & CF_LAST_IO)) {
+if (db->num_insns == db->max_insns
+&& (tb_cflags(db->tb) & CF_LAST_IO)) {
 /* Accept I/O on the last instruction.  */
 gen_io_start();
 ops->translate_insn(db, cpu);
@@ -111,7 +110,7 @@ void translator_loop(const TranslatorOps *ops, 
DisasContextBase *db,
 
 /* Stop translation if the output buffer is full,
or we have executed all of the allowed instructions.  */
-if (tcg_op_buf_full() || db->num_insns >= max_insns) {
+if (tcg_op_buf_full() || db->num_insns >= db->max_insns) {
 db->is_jmp = DISAS_TOO_MANY;
 break;
 }
diff --git a/include/exec/translator.h b/include/exec/translator.h
index e2dc2a0..71e7b2c 100644
--- a/include/exec/translator.h
+++ b/include/exec/translator.h
@@ -58,6 +58,7 @@ typedef enum DisasJumpType {
  *   disassembly).
  * @is_jmp: What instruction to disassemble next.
  * @num_insns: Number of translated instructions (including current).
+ * @max_insns: Maximum number of instructions to be translated in this TB.
  * @singlestep_enabled: "Hardware" single stepping enabled.
  *
  * Architecture-agnostic disassembly context.
@@ -67,7 +68,8 @@ typedef struct DisasContextBase {
 target_ulong pc_first;
 target_ulong pc_next;
 DisasJumpType is_jmp;
-unsigned int num_insns;
+int num_insns;
+int max_insns;
 bool singlestep_enabled;
 } DisasContextBase;
 
@@ -76,7 +78,6 @@ typedef struct DisasContextBase {
  * @init_disas_context:
  *  Initialize the target-specific portions of DisasContext struct.
  *  The generic DisasContextBase has already been initialized.
- *  Return max_insns, modified as necessary by db->tb->flags.
  *
  * @tb_start:
  *  Emit any code required before the start of the main loop,
@@ -106,8 +107,7 @@ typedef struct DisasContextBase {
  *  Print instruction disassembly to log.
  */
 typedef struct TranslatorOps {
-int (*init_disas_context)(DisasContextBase *db, CPUState *cpu,
-  int max_insns);
+void (*init_disas_context)(DisasContextBase *db, CPUState *cpu);
 void (*tb_start)(DisasContextBase *db, CPUState *cpu);
 void (*insn_start)(DisasContextBase *db, CPUState *cpu);
 bool (*breakpoint_check)(DisasContextBase *db, CPUState *cpu,
diff --git a/target/alpha/translate.c b/target/alpha/translate.c
index 73a1b5e..15eca71 100644
--- a/target/alpha/translate.c
+++ b/target/alpha/translate.c
@@ -2919,8 +2919,7 @@ static DisasJumpType translate_one(DisasContext *ctx, 
uint32_t insn)
 return ret;
 }
 
-static int alpha_tr_init_disas_context(DisasContextBase *dcbase,
-   

Re: [Qemu-devel] [PATCH] fpu/softfloat: use hardware sqrt if we can (EXPERIMENT!)

[Alex Bennée]

Alex Bennée  writes:

> This is an attempt to save some of the cost of sqrt by using the
> inbuilt support of the host hardware. The idea is assuming we start
> with a valid input we can use the hardware. If any tininess issues
> occur this will trip and FPU exception where:
>
>   - we turn off cpu->use_host_fpu
>   - mask the FPU exceptions
>   - return to what we were doing
>
> Once we return we should pick up the fact that there was something
> weird about the operation and fall-back to the pure software
> implementation.
>
> You could imagine this being extended for code generation but instead
> of returning to the code we could exit and re-generate the TB but this
> time with pure software helpers rather than any support from the
> hardware.
>
> This is a sort of fix-it-up after the fact approach because reading
> the FP state is an expensive operation for everything so let's only
> worry about exceptions when they trip...
>

> --- a/linux-user/signal.c
> +++ b/linux-user/signal.c
> @@ -20,6 +20,7 @@
>  #include "qemu/bitops.h"
>  #include 
>  #include 
> +#include 
>
>  #include "qemu.h"
>  #include "qemu-common.h"
> @@ -639,6 +640,21 @@ static void host_signal_handler(int host_signum, 
> siginfo_t *info,
>  ucontext_t *uc = puc;
>  struct emulated_sigtable *k;
>
> +/* Catch any FPU exceptions we might get from having tried to use
> + * the host FPU to speed up some calculations
> + */
> +if (host_signum == SIGFPE && cpu->use_host_fpu) {
> +cpu->use_host_fpu = false;
> +/* sadly this gets lost on the context switch when we return */
> +fedisableexcept(FE_INVALID   |
> +FE_OVERFLOW  |
> +FE_UNDERFLOW |
> +FE_INEXACT);
> +/* sigaddset(>uc_sigmask, SIGFPE); */
> +uc->__fpregs_mem.mxcsr |= 0x1f80;

This is a bug, the correct place to reset mxcsr for the return is:

(uc->uc_mcontext.fpregs)->mxcsr |= 0x1f80;

--
Alex Bennée

Re: [Qemu-devel] [PATCH v2 08/36] util: Add qemu_opts_to_qdict_filtered()

[Eric Blake]

On 02/21/2018 07:53 AM, Kevin Wolf wrote:

This allows, given a QemuOpts for a QemuOptsList that was merged from
multiple QemuOptsList, to only consider those options that exist in one
specific list. Block drivers need this to separate format-layer create
options from protocol-level options.

Signed-off-by: Kevin Wolf 
---
  include/qemu/option.h |  2 ++
  util/qemu-option.c| 42 +-
  2 files changed, 39 insertions(+), 5 deletions(-)



Reviewed-by: Eric Blake 

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

[Qemu-devel] [Bug 1750899] [NEW] Mouse cursor sometimes can't pass the invisible border on the right side of the screen

[Michael Akushsky]

Public bug reported:

I'm using qemu 2.11 on Gentoo Linux, with configured GPU passthrough (Radeon 
RX580) to the guest Windows 10.
This configuration is alive for last 4 years, this time I changed a lot qemu, 
linux kernel and windows versions, changed GPU and always all was working as 
expected. I always used standard PS/2 mouse emulation and that was enough for 
me.

Now, I bought two new monitors, instead of old one, and setup them as
one logical monitor, using technology called Eyefinity - it's a part of
standard Radeon software. Now Windows thinks, that I have one monitor
with resolution 2160x1920 (I bought Dell monitors with a thin borders
and use them in portrait mode).

Windows uses it without any problems, but mouse become crazy - sometimes (~3 
times from each 5) I can't move cursor to the right border of the screen, it 
looks like the invisible vertical border. I spent really huge amount of time to 
understand, which component is the root of problem and found, that it's really 
a mouse. I tried all possible variants (standard, tablet, virtio-mouse-pci, 
virtio-tablet-pci), and found, that in both mouse variants bug is reproducing, 
and in both tablet variants - cursor stuck near all real borders and corners, 
so it's not a variant too.
The only working variant becomes passing real USB port to my VM and insert 
second mouse to this port. So, now it's working, but I have two mice on my 
working place, which doesn't seems very useful.

Here is my command line:

QEMU_AUDIO_DRV=pa QEMU_PA_SAMPLES=4096 qemu-system-x86_64 -enable-kvm -M q35 -m 
12168 -cpu host,kvm=off -smp 4,sockets=1,cores=4 \
-bios /usr/share/qemu/bios.bin -rtc base=localtime -vga none -device 
secondary-vga \
-drive 
id=virtiocd,if=none,format=raw,file=/home/akushsky/virtio-win-0.1.141.iso \
-device driver=ide-cd,bus=ide.1,drive=virtiocd \
-device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
-device 
vfio-pci,host=05:00.0,bus=root.1,addr=00.0,multifunction=on,romfile=/opt/kvm/images/Sapphire.RX580.8192.170320_1.bin,x-vga=on
 \
-device virtio-scsi-pci,id=scsi \
-drive 
file=/dev/sdb,id=disk,format=raw,if=none,discard=on,cache=none,aio=native,detect-zeroes=unmap
 -device scsi-hd,drive=disk,id=scsi0 \
-device ich9-intel-hda,bus=pcie.0,addr=1b.0,id=sound0 -device 
hda-duplex,id=sound0-codec0,bus=sound0.0,cad=0 \
-usb -usbdevice host:046d:c52b

All in all, I checked on Windows 7 and Windows 10, and on qemu 2.10 and
2.11 - bug is always reproducible.

** Affects: qemu
 Importance: Undecided
 Status: New

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1750899

Title:
  Mouse cursor sometimes can't pass the invisible border on the right
  side of the screen

Status in QEMU:
  New

Bug description:
  I'm using qemu 2.11 on Gentoo Linux, with configured GPU passthrough (Radeon 
RX580) to the guest Windows 10.
  This configuration is alive for last 4 years, this time I changed a lot qemu, 
linux kernel and windows versions, changed GPU and always all was working as 
expected. I always used standard PS/2 mouse emulation and that was enough for 
me.

  Now, I bought two new monitors, instead of old one, and setup them as
  one logical monitor, using technology called Eyefinity - it's a part
  of standard Radeon software. Now Windows thinks, that I have one
  monitor with resolution 2160x1920 (I bought Dell monitors with a thin
  borders and use them in portrait mode).

  Windows uses it without any problems, but mouse become crazy - sometimes (~3 
times from each 5) I can't move cursor to the right border of the screen, it 
looks like the invisible vertical border. I spent really huge amount of time to 
understand, which component is the root of problem and found, that it's really 
a mouse. I tried all possible variants (standard, tablet, virtio-mouse-pci, 
virtio-tablet-pci), and found, that in both mouse variants bug is reproducing, 
and in both tablet variants - cursor stuck near all real borders and corners, 
so it's not a variant too.
  The only working variant becomes passing real USB port to my VM and insert 
second mouse to this port. So, now it's working, but I have two mice on my 
working place, which doesn't seems very useful.

  Here is my command line:

  QEMU_AUDIO_DRV=pa QEMU_PA_SAMPLES=4096 qemu-system-x86_64 -enable-kvm -M q35 
-m 12168 -cpu host,kvm=off -smp 4,sockets=1,cores=4 \
  -bios /usr/share/qemu/bios.bin -rtc base=localtime -vga none -device 
secondary-vga \
  -drive 
id=virtiocd,if=none,format=raw,file=/home/akushsky/virtio-win-0.1.141.iso \
  -device driver=ide-cd,bus=ide.1,drive=virtiocd \
  -device 
ioh3420,bus=pcie.0,addr=1c.0,multifunction=on,port=1,chassis=1,id=root.1 \
  -device 
vfio-pci,host=05:00.0,bus=root.1,addr=00.0,multifunction=on,romfile=/opt/kvm/images/Sapphire.RX580.8192.170320_1.bin,x-vga=on
 \
  -device virtio-scsi-pci,id=scsi \
  -drive 

Re: [Qemu-devel] [PATCH v5 4/4] linux-user: MIPS set cpu to r6 CPU if binary is R6

[Richard Henderson]

On 02/20/2018 09:33 AM, Laurent Vivier wrote:
> From: YunQiang Su 
> 
> So here we need to detect the version of binaries and set
> cpu_model for it.
> 
> Signed-off-by: YunQiang Su 
> [lv: original patch modified to move code into cpu_get_model()]
> Signed-off-by: Laurent Vivier 

Reviewed-by: Richard Henderson 


r~

[Qemu-devel] [PATCH v8 06/13] s390-ccw: parse and set boot menu options

[Collin L. Walling]

Set boot menu options for an s390 guest and store them in
the iplb. These options are set via the QEMU command line
option:

-boot menu=on|off[,splash-time=X]

or via the libvirt domain xml:


  


Where X represents some positive integer representing
milliseconds.

Any value set for loadparm will override all boot menu options.
If loadparm=PROMPT, then the menu will be enabled without a
timeout.

Signed-off-by: Collin L. Walling 
Reviewed-by: Janosch Frank 
---
 hw/s390x/ipl.c  | 44 
 hw/s390x/ipl.h  |  9 +++--
 pc-bios/s390-ccw/iplb.h |  9 +++--
 3 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 79f5a58..a176c4a 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -23,6 +23,9 @@
 #include "hw/s390x/ebcdic.h"
 #include "ipl.h"
 #include "qemu/error-report.h"
+#include "qemu/config-file.h"
+#include "qemu/cutils.h"
+#include "qemu/option.h"
 
 #define KERN_IMAGE_START0x01UL
 #define KERN_PARM_AREA  0x010480UL
@@ -219,6 +222,46 @@ static Property s390_ipl_properties[] = {
 DEFINE_PROP_END_OF_LIST(),
 };
 
+static void s390_ipl_set_boot_menu(S390IPLState *ipl)
+{
+QemuOptsList *plist = qemu_find_opts("boot-opts");
+QemuOpts *opts = QTAILQ_FIRST(>head);
+uint8_t *flags = >qipl.qipl_flags;
+uint32_t *timeout = >qipl.boot_menu_timeout;
+const char *tmp;
+unsigned long splash_time = 0;
+
+switch (ipl->iplb.pbt) {
+case S390_IPL_TYPE_CCW:
+break;
+default:
+error_report("boot menu is not supported for this device type.");
+return;
+}
+
+if (!boot_menu) {
+return;
+}
+
+*flags |= QIPL_FLAG_BM_OPTS_CMD;
+
+tmp = qemu_opt_get(opts, "splash-time");
+
+if (tmp && qemu_strtoul(tmp, NULL, 10, _time)) {
+error_report("splash-time is invalid, forcing it to 0.");
+*timeout = 0;
+return;
+}
+
+if (splash_time > 0x) {
+error_report("splash-time is too large, forcing it to max value.");
+*timeout = 0x;
+return;
+}
+
+*timeout = cpu_to_be32(splash_time);
+}
+
 static bool s390_gen_initial_iplb(S390IPLState *ipl)
 {
 DeviceState *dev_st;
@@ -435,6 +478,7 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
 }
 ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
 }
+s390_ipl_set_boot_menu(ipl);
 s390_ipl_prepare_qipl(cpu);
 }
 
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 08926a3..3a37924 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -91,6 +91,9 @@ void s390_reipl_request(void);
 
 #define QIPL_ADDRESS  0xcc
 
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD   0x80
+
 /*
  * The QEMU IPL Parameters will be stored at absolute address
  * 204 (0xcc) which means it is 32-bit word aligned but not
@@ -104,9 +107,11 @@ void s390_reipl_request(void);
  * in pc-bios/s390-ccw/iplb.h.
  */
 struct QemuIplParameters {
-uint8_t  reserved1[4];
+uint8_t  qipl_flags;
+uint8_t  reserved1[3];
 uint64_t netboot_start_addr;
-uint8_t  reserved2[16];
+uint32_t boot_menu_timeout;
+uint8_t  reserved2[12];
 } QEMU_PACKED;
 typedef struct QemuIplParameters QemuIplParameters;
 
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 31d2934..832bb94 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -74,14 +74,19 @@ extern IplParameterBlock iplb 
__attribute__((__aligned__(PAGE_SIZE)));
 
 #define QIPL_ADDRESS  0xcc
 
+/* Boot Menu flags */
+#define QIPL_FLAG_BM_OPTS_CMD   0x80
+
 /*
  * This definition must be kept in sync with the defininition
  * in hw/s390x/ipl.h
  */
 struct QemuIplParameters {
-uint8_t  reserved1[4];
+uint8_t  qipl_flags;
+uint8_t  reserved1[3];
 uint64_t netboot_start_addr;
-uint8_t  reserved2[16];
+uint32_t boot_menu_timeout;
+uint8_t  reserved2[12];
 } __attribute__ ((packed));
 typedef struct QemuIplParameters QemuIplParameters;
 
-- 
2.7.4

[Qemu-devel] [PATCH v8 05/13] s390-ccw: move auxiliary IPL data to separate location

[Collin L. Walling]

The s390-ccw firmware needs some information in support of the
boot process which is not available on the native machine.
Examples are the netboot firmware load address and now the
boot menu parameters.

While storing that data in unused fields of the IPL parameter block
works, that approach could create problems if the parameter block
definition should change in the future. Because then a guest could
overwrite these fields using the set IPLB diagnose.

In fact the data in question is of more global nature and not really
tied to an IPL device, so separating it is rather logical.

This commit introduces a new structure to hold firmware relevant
IPL parameters set by QEMU. The data is stored at location 204 (dec)
and can contain up to 7 32-bit words. This area is available to
programming in the z/Architecture Principles of Operation and
can thus safely be used by the firmware until the IPL has completed.

Signed-off-by: Viktor Mihajlovski 
Signed-off-by: Collin L. Walling 
---
 hw/s390x/ipl.c  | 18 +-
 hw/s390x/ipl.h  | 25 +++--
 pc-bios/s390-ccw/iplb.h | 18 --
 pc-bios/s390-ccw/main.c |  6 +-
 4 files changed, 61 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 0d06fc1..79f5a58 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -399,6 +399,21 @@ void s390_reipl_request(void)
 qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
 }
 
+static void s390_ipl_prepare_qipl(S390CPU *cpu)
+{
+S390IPLState *ipl = get_ipl_device();
+uint8_t *addr;
+uint64_t len = 4096;
+
+addr = cpu_physical_memory_map(cpu->env.psa, , 1);
+if (!addr || len < QIPL_ADDRESS + sizeof(QemuIplParameters)) {
+error_report("Cannot set QEMU IPL parameters");
+return;
+}
+memcpy(addr + QIPL_ADDRESS, >qipl, sizeof(QemuIplParameters));
+cpu_physical_memory_unmap(addr, len, 1, len);
+}
+
 void s390_ipl_prepare_cpu(S390CPU *cpu)
 {
 S390IPLState *ipl = get_ipl_device();
@@ -418,8 +433,9 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
 error_report_err(err);
 vm_stop(RUN_STATE_INTERNAL_ERROR);
 }
-ipl->iplb.ccw.netboot_start_addr = cpu_to_be64(ipl->start_addr);
+ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
 }
+s390_ipl_prepare_qipl(cpu);
 }
 
 static void s390_ipl_reset(DeviceState *dev)
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 8a705e0..08926a3 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -16,8 +16,7 @@
 #include "cpu.h"
 
 struct IplBlockCcw {
-uint64_t netboot_start_addr;
-uint8_t  reserved0[77];
+uint8_t  reserved0[85];
 uint8_t  ssid;
 uint16_t devno;
 uint8_t  vm_flags;
@@ -90,6 +89,27 @@ void s390_ipl_prepare_cpu(S390CPU *cpu);
 IplParameterBlock *s390_ipl_get_iplb(void);
 void s390_reipl_request(void);
 
+#define QIPL_ADDRESS  0xcc
+
+/*
+ * The QEMU IPL Parameters will be stored at absolute address
+ * 204 (0xcc) which means it is 32-bit word aligned but not
+ * double-word aligned.
+ * Placement of data fields in this area must account for
+ * their alignment needs. E.g., netboot_start_address must
+ * have an offset of n * 8 bytes within the struct in order
+ * to keep it double-word aligned.
+ * The total size of the struct must never exceed 28 bytes.
+ * This definition must be kept in sync with the defininition
+ * in pc-bios/s390-ccw/iplb.h.
+ */
+struct QemuIplParameters {
+uint8_t  reserved1[4];
+uint64_t netboot_start_addr;
+uint8_t  reserved2[16];
+} QEMU_PACKED;
+typedef struct QemuIplParameters QemuIplParameters;
+
 #define TYPE_S390_IPL "s390-ipl"
 #define S390_IPL(obj) OBJECT_CHECK(S390IPLState, (obj), TYPE_S390_IPL)
 
@@ -105,6 +125,7 @@ struct S390IPLState {
 bool iplb_valid;
 bool reipl_requested;
 bool netboot;
+QemuIplParameters qipl;
 
 /*< public >*/
 char *kernel;
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 890aed9..31d2934 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -13,8 +13,7 @@
 #define IPLB_H
 
 struct IplBlockCcw {
-uint64_t netboot_start_addr;
-uint8_t  reserved0[77];
+uint8_t  reserved0[85];
 uint8_t  ssid;
 uint16_t devno;
 uint8_t  vm_flags;
@@ -73,6 +72,21 @@ typedef struct IplParameterBlock IplParameterBlock;
 
 extern IplParameterBlock iplb __attribute__((__aligned__(PAGE_SIZE)));
 
+#define QIPL_ADDRESS  0xcc
+
+/*
+ * This definition must be kept in sync with the defininition
+ * in hw/s390x/ipl.h
+ */
+struct QemuIplParameters {
+uint8_t  reserved1[4];
+uint64_t netboot_start_addr;
+uint8_t  reserved2[16];
+} __attribute__ ((packed));
+typedef struct QemuIplParameters QemuIplParameters;
+
+extern QemuIplParameters qipl;
+
 #define S390_IPL_TYPE_FCP 0x00
 #define S390_IPL_TYPE_CCW 0x02
 #define S390_IPL_TYPE_QEMU_SCSI 0xff
diff --git a/pc-bios/s390-ccw/main.c 

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

[Kevin Wolf]

Am 21.02.2018 um 19:32 hat Eric Blake geschrieben:
> On 02/21/2018 11:39 AM, Kevin Wolf wrote:
> > > See my commit message comment - we have other spots in the code base that
> > > blindly g_malloc(2 * s->cluster_size).
> > 
> > Though is that a reason to do the same in new code or to phase out such
> > allocations whenever you touch them?
> 
> Touché.
> 
> > 
> > > And I intended (but sent the email without amending my commit) to use
> > > g_malloc().  But as Berto has convinced me that an externally produced
> > > image can convince us to read up to 4M (even though we don't need that
> > > much to decompress), I suppose that the try_ variant plus checking is
> > > reasonable (and care in NULL'ing out if one but not both allocations
> > > succeed).
> > 
> > Sounds good.
> > 
> > Another thought I had is whether we should do per-request allocation for
> > compressed clusters, too, instead of having per-BDS buffers.
> 
> The only benefit of a per-BDS buffer is that we cache things - multiple
> sub-cluster reads in a row all from the same compressed cluster benefit from
> decompressing only once.

Oh, you're right. I missed that this is actually used as a cache.

I guess we want to leave it for now then. Maybe at some point we can
actually implement the data cache that I proposed a few years ago (using
Qcow2Cache for data clusters under some circumstances), then we could
probably make that hold the data instead of having a separate cache.

> The drawbacks of a per-BDS buffer: we can't do things in parallel
> (everything else in qcow2 drops the lock around bdrv_co_pread[v]), so
> the initial read prevents anything else in the qcow2 layer from
> progressing.

Yes, though there are probably other optimisations that could be made
for compression before this becomes relevant, like reading more than one
cluster at a time.

> I also wonder - since we ARE allowing multiple parallel readers in other
> parts of qcow2 (without a patch, decompression is not in this boat, but
> decryption and even bounce buffers due to lower-layer alignment constraints
> are), what sort of mechanisms do we have for using a pool of reusable
> buffers, rather than having each cluster access that requires a buffer
> malloc and free the buffer on a per-access basis?  I don't know how much
> time the malloc/free per-transaction overhead adds, or if it is already much
> smaller than the actual I/O time.

I don't either. A while ago, we used g_slice_alloc() in some places (I
remember qemu_aio_get), but it was actually slower than just using
malloc/free each time.

So if we do want to pool buffers, we probably need to implement that
manually. I don't think we have a generic memory pool in qemu yet.

> But note that while reusable buffers from a pool would cut down on the
> per-I/O malloc/free overhead if we switch decompression away from per-BDS
> buffer, it would still not solve the fact that we only get the caching
> ability where multiple sub-cluster requests from the same compressed cluster
> require only one decompression, since that's only possible on a per-BDS
> caching level.

Yes, as I said above, I didn't notice that it's a real cache. Without
the possibility to use Qcow2Cache instead, we'll want to keep it.

Kevin

[Qemu-devel] [PATCH v8 12/13] s390-ccw: use zipl values when no boot menu options are present

[Collin L. Walling]

If no boot menu options are present, then flag the boot menu to
use the zipl options that were set in the zipl configuration file
(and stored on disk by zipl). These options are found at some
offset prior to the start of the zipl boot menu banner. The zipl
timeout value is limited to a 16-bit unsigned integer and stored
as seconds, so we take care to convert it to milliseconds in order
to conform to the rest of the boot menu functionality. This is
limited to CCW devices.

For reference, the zipl configuration file uses the following
fields in the menu section:

  prompt=1  enable the boot menu
  timeout=X set the timeout to X seconds

To explicitly disregard any boot menu options, then menu=off or
 must be specified.

Signed-off-by: Collin L. Walling 
---
 hw/s390x/ipl.c  |  5 +
 hw/s390x/ipl.h  |  1 +
 pc-bios/s390-ccw/iplb.h |  1 +
 pc-bios/s390-ccw/main.c |  3 ++-
 pc-bios/s390-ccw/menu.c | 14 ++
 5 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index a176c4a..a0f4f40 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -233,6 +233,11 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
 
 switch (ipl->iplb.pbt) {
 case S390_IPL_TYPE_CCW:
+/* In the absence of -boot menu, use zipl parameters */
+if (!qemu_opt_get(opts, "menu")) {
+*flags |= QIPL_FLAG_BM_OPTS_ZIPL;
+return;
+}
 break;
 default:
 error_report("boot menu is not supported for this device type.");
diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 3a37924..1a8adc2 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -93,6 +93,7 @@ void s390_reipl_request(void);
 
 /* Boot Menu flags */
 #define QIPL_FLAG_BM_OPTS_CMD   0x80
+#define QIPL_FLAG_BM_OPTS_ZIPL  0x40
 
 /*
  * The QEMU IPL Parameters will be stored at absolute address
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index 832bb94..7dfce4f 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -76,6 +76,7 @@ extern IplParameterBlock iplb 
__attribute__((__aligned__(PAGE_SIZE)));
 
 /* Boot Menu flags */
 #define QIPL_FLAG_BM_OPTS_CMD   0x80
+#define QIPL_FLAG_BM_OPTS_ZIPL  0x40
 
 /*
  * This definition must be kept in sync with the defininition
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index 32ed70e..a7473b0 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -20,6 +20,7 @@ QemuIplParameters qipl;
 
 #define LOADPARM_PROMPT "PROMPT  "
 #define LOADPARM_EMPTY  ""
+#define BOOT_MENU_FLAG_MASK (QIPL_FLAG_BM_OPTS_CMD | QIPL_FLAG_BM_OPTS_ZIPL)
 
 /*
  * Priniciples of Operations (SA22-7832-09) chapter 17 requires that
@@ -91,7 +92,7 @@ static void menu_setup(void)
 
 switch (iplb.pbt) {
 case S390_IPL_TYPE_CCW:
-menu_set_parms(qipl.qipl_flags & QIPL_FLAG_BM_OPTS_CMD,
+menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
qipl.boot_menu_timeout);
 return;
 }
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 4f84bfe..4fc328d 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -18,6 +18,10 @@
 #define KEYCODE_BACKSP '\177'
 #define KEYCODE_ENTER  '\r'
 
+/* Offsets from zipl fields to zipl banner start */
+#define ZIPL_TIMEOUT_OFFSET 138
+#define ZIPL_FLAG_OFFSET140
+
 #define TOD_CLOCK_MILLISECOND   0x3e8000
 
 #define LOW_CORE_EXTERNAL_INT_ADDR   0x86
@@ -187,6 +191,16 @@ int menu_get_zipl_boot_index(const char *menu_data)
 {
 size_t len;
 int entries;
+uint16_t zipl_flag = *(uint16_t *)(menu_data - ZIPL_FLAG_OFFSET);
+uint16_t zipl_timeout = *(uint16_t *)(menu_data - ZIPL_TIMEOUT_OFFSET);
+
+if (flag == QIPL_FLAG_BM_OPTS_ZIPL) {
+if (!zipl_flag) {
+return 0; /* Boot default */
+}
+/* zipl stores timeout as seconds */
+timeout = zipl_timeout * 1000;
+}
 
 /* Print and count all menu items, including the banner */
 for (entries = 0; *menu_data; entries++) {
-- 
2.7.4

Re: [Qemu-devel] [PULL 19/20] tcg/i386: Add vector operations

[Richard Henderson]

On 02/20/2018 08:44 AM, Max Reitz wrote:
> This patch makes make check with clang -m32 fail for me.  (Interestingly
> enough, though, clang -m64 and gcc -m32 work fine.)

What's really interesting is that gcc -m32 should *not* have worked, but does.
I'm not sure why.  The assert is for the missing constraints for 
INDEX_op_dup2_vec.

Will fix.


r~

[Qemu-devel] [PATCH v8 00/13] Interactive Boot Menu for DASD and SCSI Guests on s390x

[Collin L. Walling]

Due to the introduction of the QemuIplParameter block and taking some time to
revist some areas, a few chunks of code have been reworked to better align with
those changes. I also think the reworkings improve readability. The same 
functionality is still there, the code just looks a little different (and 
hopefully looks better).

--- [v8] ---

The tl;dr:

cleaned up some early patches based on review, threw zipl boot option code
into its own patch, refactored s390_ipl_set_boot_menu to reflect latest
changes.

The "pls explain":

* cleaned up "s390-ccw: refactor boot map table code"

- removed void ptr casting in a couple of spots

* cleaned up "s390-ccw: set cp_receive mask..." patch

- setting the mask consumes a service interrupt, so there is no need 
for 
  a followup sclp_print

* fixed uitoa based on feedback from v7

* fixed alignment concerns in QemuIplParameters and renamed flags field

- reasoning: necessary; better readability

- s/boot_menu_flags/qipl_flags so this field can be (potentially) used 
for 
  other purposes in the future

- when setting the boot menu parms in the bios, we take care to mask out
  the appropriate qipl_flags for boot menu specific flags

- boot menu flags defines are renamed to be prefixed with "QIPL_FLAG_"

- also updated comment above this struct

* cleaned up "s390-ccw: read user input..." patch

- defines for low core external interrupt code addr and 
  clock comparator interrupt code

- take care to make sure buf remains null terminated when passed to 
read_prompt

* [NEW PATCH] "s390-ccw: use zipl values when no boot menu options are 
present"

- reasoning: better readability; further explanation of feature

- *nothing new added to this patch series here*

- zipl options flag setting and parsing *moved to* this patch

- this attempts to better explain how these fields are used and how 
they get
  parsed

- the commit message gives details on how to set these fields in the 
zipl
  configuration file

- the zipl options are only set for CCW type IPL devices (since no 
  other devices actually support it)

* refactored s390_ipl_set_boot_menu

- reasoning: better readability

- the idea is that we should take care to appropriately set the boot 
menu
  flags for each IPL device type from the beginning. We should not set
  certain flags for devices that cannot support certain features (eg 
SCSI 
  does not support zipl menus, so we should never set the use_zipl_opts 
flag
  for SCSI) 

- since there are no longer boot menu fields specific to each IPL type,
  the switch statement is simply used to detect if the IPL device type
  is capable of a boot menu

- since zipl flags are only set for CCW type IPL devices, I reworked 
  the logic and removed some indentation

* s/menu_check_flags/menu_is_enabled

- reasoning: better readability

- no parameters

- "if menu is enabled" reads better than "if these specific flag bits 
are set"

* removed menu.h

- reasoning: file not needed; less to maintain

- introduction of qipl and better understanding of zipl options led to 
  this decision. by the end of these changes, this file ended up 
  housing 4 function declarations and no longer seemed necessary

- all menu related function declarations are in s390-ccw.h

- boot menu flags are defined in iplb.h (which aligns with 
hw/s390x/ipl.h)

--- [Summary] ---

These patches implement a boot menu for ECKD DASD and SCSI guests on s390x. 
The menu will only appear if the disk has been configured for IPL with the 
zIPL tool and with the following QEMU command line options:

-boot menu=on[,splash-time=X] and/or -machine loadparm='prompt'

The following must be specified for the device to be IPL'd from:

-device ...,bootindex=1

or via the following libvirt domain xml:


  


or


  ...
  


Where X is some positive integer representing time in milliseconds.

 must be specified for the device to be IPL'd from

A loadparm other than 'prompt' will disable the menu and just boot 
the specified entry.

If no boot options are specified, we will attempt to use the values
provided by zipl (ECKD DASD only).

Collin L. Walling (13):
  s390-ccw: refactor boot map table code
  s390-ccw: refactor eckd_block_num to use CHS
  s390-ccw: refactor IPL structs
  s390-ccw: update libc
  s390-ccw: move auxiliary IPL data to separate location
  s390-ccw: parse and set boot menu options
  s390-ccw: set up interactive boot menu parameters
  s390-ccw: read stage2 boot loader data to find menu
  s390-ccw: print zipl boot menu
  s390-ccw: read user input for boot index via the SCLP 

Re: [Qemu-devel] [PATCHv2 1/2] translator: merge max_insns into DisasContextBase

[Richard Henderson]

On 02/19/2018 06:26 PM, Emilio G. Cota wrote:
> @@ -68,6 +69,7 @@ typedef struct DisasContextBase {
>  target_ulong pc_next;
>  DisasJumpType is_jmp;
>  unsigned int num_insns;
> +int max_insns;
>  bool singlestep_enabled;
>  } DisasContextBase;

We really should pick the same type for max_insns and num_insns, which ever
type we settle on.  I can't see how we can go wrong with unsigned...


r~

Re: [Qemu-devel] [PATCH] docs: document how to use the l2-cache-entry-size parameter

[Kevin Wolf]

Am 19.02.2018 um 15:54 hat Alberto Garcia geschrieben:
> This patch updates docs/qcow2-cache.txt explaining how to use the new
> l2-cache-entry-size parameter.
> 
> Here's a more detailed technical description of this feature:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-09/msg00635.html
> 
> And here are some performance numbers:
> 
>https://lists.gnu.org/archive/html/qemu-block/2017-12/msg00507.html
> 
> Signed-off-by: Alberto Garcia 

Thanks, applied to the block branch.


While reviewing this, I read the whole document and stumbled across
these paragraphs:

> The reason for this 1/4 ratio is to ensure that both caches cover the
> same amount of disk space. Note however that this is only valid with
> the default value of refcount_bits (16). If you are using a different
> value you might want to calculate both cache sizes yourself since QEMU
> will always use the same 1/4 ratio.

Sounds like we should fix our defaults?

While we're at it, would l2-cache-entry-size = MIN(cluster_size, 64k)
make sense as a default?

> It's also worth mentioning that there's no strict need for both caches
> to cover the same amount of disk space. The refcount cache is used
> much less often than the L2 cache, so it's perfectly reasonable to
> keep it small.

More precisely, it is only used for cluster allocation, not for read or
for rewrites. Usually this means that it's indeed accessed a lot less,
though especially in benchmarks, this isn't necessarily less often.

However, the more important part is that even for allocating writes with
random I/O, the refcount cache is still accessed sequentially and we
don't really take advantage of having more than a single refcount block
in memory. This only stops being true as soon as you add something that
can free clusters (discards, overwriting compressed cluster, deleting
internal snapshots).

We have a minimum refcount block cache size of 4 clusters because of the
possible recursion during refcount table growth, which leaves some room
to hold the refcount block for an occasional discard (and subsequent
reallocation).

So should we default to this minimum on the grounds that for most
people, refcounts blocks are probably only accessed sequentially in
practice? The remaining memory of the total cache size seems to help the
average case more if it's added to the L2 cache instead.

Kevin

Re: [Qemu-devel] [PATCH 1/2] target/openrisc: convert to DisasContextBase

[Richard Henderson]

On 02/17/2018 05:32 PM, Emilio G. Cota wrote:
> Signed-off-by: Emilio G. Cota 
> ---
>  target/openrisc/translate.c | 87 
> ++---
>  1 file changed, 43 insertions(+), 44 deletions(-)
> 
> diff --git a/target/openrisc/translate.c b/target/openrisc/translate.c
> index 2747b24..0450144 100644
> --- a/target/openrisc/translate.c
> +++ b/target/openrisc/translate.c
> @@ -36,7 +36,8 @@
>  #include "exec/log.h"
>  
>  #define LOG_DIS(str, ...) \
> -qemu_log_mask(CPU_LOG_TB_IN_ASM, "%08x: " str, dc->pc, ## __VA_ARGS__)
> +qemu_log_mask(CPU_LOG_TB_IN_ASM, "%08x: " str, dc->base.pc_next,\
> +  ## __VA_ARGS__)
>  
>  /* is_jmp field values */
>  #define DISAS_JUMPDISAS_TARGET_0 /* only pc was modified dynamically */
> @@ -44,13 +45,10 @@
>  #define DISAS_TB_JUMP DISAS_TARGET_2 /* only pc was modified statically */
>  
>  typedef struct DisasContext {
> -TranslationBlock *tb;
> -target_ulong pc;
> -uint32_t is_jmp;
> +DisasContextBase base;
>  uint32_t mem_idx;
>  uint32_t tb_flags;
>  uint32_t delayed_branch;
> -bool singlestep_enabled;
>  } DisasContext;
>  
>  static TCGv cpu_sr;
> @@ -126,9 +124,9 @@ static void gen_exception(DisasContext *dc, unsigned int 
> excp)
>  
>  static void gen_illegal_exception(DisasContext *dc)
>  {
> -tcg_gen_movi_tl(cpu_pc, dc->pc);
> +tcg_gen_movi_tl(cpu_pc, dc->base.pc_next);
>  gen_exception(dc, EXCP_ILLEGAL);
> -dc->is_jmp = DISAS_UPDATE;
> +dc->base.is_jmp = DISAS_UPDATE;

Should be NORETURN.

> @@ -1254,14 +1252,14 @@ static void dec_sys(DisasContext *dc, uint32_t insn)
>  switch (op0) {
>  case 0x000:/* l.sys */
>  LOG_DIS("l.sys %d\n", K16);
> -tcg_gen_movi_tl(cpu_pc, dc->pc);
> +tcg_gen_movi_tl(cpu_pc, dc->base.pc_next);
>  gen_exception(dc, EXCP_SYSCALL);
> -dc->is_jmp = DISAS_UPDATE;
> +dc->base.is_jmp = DISAS_UPDATE;

Likewise.


r~

[Qemu-devel] [PATCH v8 09/13] s390-ccw: print zipl boot menu

[Collin L. Walling]

When the boot menu options are present and the guest's
disk has been configured by the zipl tool, then the user
will be presented with an interactive boot menu with
labeled entries. An example of what the menu might look
like:

zIPL v1.37.1-build-20170714 interactive boot menu.

0. default (linux-4.13.0)

  1. linux-4.13.0
  2. performance
  3. kvm

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/menu.c | 33 -
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index ba886be..8f50e55 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -15,11 +15,42 @@
 static uint8_t flag;
 static uint64_t timeout;
 
-int menu_get_zipl_boot_index(const char *menu_data)
+static int get_boot_index(int entries)
 {
 return 0; /* implemented next patch */
 }
 
+static void zipl_println(const char *data, size_t len)
+{
+char buf[len + 2];
+
+ebcdic_to_ascii(data, buf, len);
+buf[len] = '\n';
+buf[len + 1] = '\0';
+
+sclp_print(buf);
+}
+
+int menu_get_zipl_boot_index(const char *menu_data)
+{
+size_t len;
+int entries;
+
+/* Print and count all menu items, including the banner */
+for (entries = 0; *menu_data; entries++) {
+len = strlen(menu_data);
+zipl_println(menu_data, len);
+menu_data += len + 1;
+
+if (entries < 2) {
+sclp_print("\n");
+}
+}
+
+sclp_print("\n");
+return get_boot_index(entries - 1); /* subtract 1 to exclude banner */
+}
+
 void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
 {
 flag = boot_menu_flag;
-- 
2.7.4

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

[John Snow]

On 02/21/2018 10:59 AM, Eric Blake wrote:
> On 02/21/2018 09:00 AM, Eric Blake wrote:
>> On 02/21/2018 04:04 AM, Alberto Garcia wrote:
>>> On Tue 20 Feb 2018 11:24:59 PM CET, Eric Blake wrote:
>>>
>>> I was also preparing a patch to change this, but you arrived first :-)
>>>
 So, it's time to cut back on the waste.  A compressed cluster
 will NEVER occupy more than an uncompressed cluster
> 
> 
>> And here, csize can only get smaller than the length picked by
>> nb_csectors.  Therefore, csize is GUARANTEED to be <= c->sector_size.
>>
>>>
>>> - Solution a: check that csize < s->cluster_size and return an error if
>>>    it's not. However! although QEMU won't produce an image with a
>>>    compressed cluster that is larger than the uncompressed one, the
>>> qcow2
>>>    on-disk format in principle allows for that, so arguably we should
>>>    accept it.
>>
>> No, the qcow2 on-disk format does not have enough bits reserved for
>> that.  It is impossible to store an inflated cluster, because you
>> don't have enough bits to record it.
> 
> Okay, the spec is WRONG, compared to our code base.
> 
>>
>> That said, we MAY have a bug, more likely to be visible in 512-byte
>> clusters but possible on any size.  While the 'number sectors' field
>> IS sufficient for any compressed cluster starting at offset 0 in the
>> cluster, we run into issues if the starting offset is later in the
>> cluster.  That is, even though the compressed length of a 512-byte
>> cluster is <= 512 (or we wouldn't compress it), if we start at offset
>> 510, we NEED to read the next cluster to get the rest of the
>> compressed stream - but at 512-byte clusters, there are 0 bits
>> reserved for 'number sectors'.  Per the above calculations with the
>> example offset of 510, nb_csectors would be 1 (it can't be anything
>> else for a 512-byte cluster image), and csize would then be 2 bytes,
>> which is insufficient for reading back enough data to reconstruct the
>> cluster.
> 
> In fact, here's a demonstration of a discrepancy, where qemu-img and
> John's qcheck tool [1] disagree about the validity of an image created
> by qemu (although it may just be that qcheck hasn't yet learned about
> compressed clusters):
> 
> [1]https://github.com/jnsnow/qcheck
> 

I wouldn't trust my tool's ability to understand compressed clusters :)

I didn't get very far, though I did run across the fact that there
appeared to be a discrepancy between the spec and the actual
implementation, IIRC.

It looked like you came to the same conclusion when you stepped through
it manually.

> $ f=12345678
> $ f=$f$f$f$f # 32
> $ f=$f$f$f$f # 128
> $ f=$f$f$f$f # 512
> $ f=$f$f$f$f # 2k
> $ f=$f$f$f$f # 8k
> $ f=$f$f$f$f # 32k
> $ f=$f$f$f$f # 128k
> $ printf "$f" > data
> $ qemu-img convert -c -f raw -O qcow2 -o cluster_size=512 \
>   data data.qcow2
> $ qemu-img check data.qcow2
> No errors were found on the image.
> 256/256 = 100.00% allocated, 100.00% fragmented, 100.00% compressed
> clusters
> Image end offset: 18944
> $ ./qcheck data.qcow2
> ...
> == L2 Tables ==
> 
> == L2 cluster l1[0] : 0x0800 ==
> Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
> == L2 cluster l1[1] : 0x0e00 ==
> Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
> == L2 cluster l1[2] : 0x1400 ==
> Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
> == L2 cluster l1[3] : 0x1a00 ==
> Corrupt entries: 63; Non-zero entries: 64; Corrupt:Non-zero ratio: 0.984375
> L2 tables: Could not complete analysis, 257 problems found
> 
> 
> == Reference Count Analysis ==
> 
> Refcount analysis: 00 vacant clusters
> Refcount analysis: 04 leaked clusters
> Refcount analysis: 00 ghost clusters
> Refcount analysis: 04 miscounted clusters
> Refcount analysis: 8 problems found
> 
> 
> == Cluster Counts ==
> 
> Metadata: 0x1000
> Data: 0x800
> Leaked: 0x800
> Vacant: 0x0
> total: 0x2000
> qcheck: 73 problems found
> 
> 
>>
>> Not true.  It is (cluster_bits - 9) or (cluster_size / 512). 
>> Remember, x = 62 - (cluster_bits - 8); for a 512-byte cluster, x =
>> 61.  The 'number sectors' field is then bits x+1 - 61 (but you can't
>> have a bitfield occupying bit 62 upto 61; especially since bit 62 is
>> the bit for compressed cluster).
> 
> So instead of blindly reading the spec, I'm now going to single-stepping
> through the 'qemu-img convert' command line above, to see what REALLY
> happens:
> 
> Line numbers from commit a6e0344fa0
> $ gdb --args ./qemu-img convert -c -f raw -O qcow2 -o cluster_size=512
> data data.qcow2
> ...
> (gdb) b qcow2_alloc_bytes
> Breakpoint 1 at 0x57610: file block/qcow2-refcount.c, line 1052.
> (gdb) r
> Thread 1 "qemu-img" hit Breakpoint 1, qcow2_alloc_bytes (
>     bs=bs@entry=0x55d87f50, size=size@entry=15)
>     at block/qcow2-refcount.c:1052
> 1052    {
> (gdb)
> 
> So we are compressing 512 bytes down to 15 every time, which means that
> after 34 

Re: [Qemu-devel] [PATCH 1/6] build-sys: fix -fsanitize=address check

[Emilio G. Cota]

On Thu, Feb 15, 2018 at 22:25:47 +0100, Marc-André Lureau wrote:
> Since 218bb57dd79d6843e0592c30a82ea8c1fddc74a5, the -fsanitize=address
> check fails with:
> config-temp/qemu-conf.c:3:20: error: integer overflow in expression 
> [-Werror=overflow]
>return INT32_MIN / -1;
> 
> Interestingly, UBSAN check doesn't produce a compile time warning.
> Use a test that doesn't have compile time warnings, and make it
> specific to UBSAN check.
> 
> Signed-off-by: Marc-André Lureau 

Reviewed-by: Emilio G. Cota 

E.

[Qemu-devel] [PATCH v8 01/13] s390-ccw: refactor boot map table code

[Collin L. Walling]

Some ECKD bootmap code was using structs designed for SCSI.
Even though this works, it confuses readability. Add a new
BootMapTable struct to assist with readability in bootmap
entry code. Also:

- replace ScsiMbr in ECKD code with appropriate structs
- fix read_block messages to reflect BootMapTable
- fixup ipl_scsi to use BootMapTable (referred to as Program Table)
- defined value for maximum table entries

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/bootmap.c | 60 +-
 pc-bios/s390-ccw/bootmap.h | 11 -
 2 files changed, 37 insertions(+), 34 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 67a6123..a4eaf24 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -182,24 +182,24 @@ static block_number_t load_eckd_segments(block_number_t 
blk, uint64_t *address)
 return block_nr;
 }
 
-static void run_eckd_boot_script(block_number_t mbr_block_nr)
+static void run_eckd_boot_script(block_number_t bmt_block_nr)
 {
 int i;
 unsigned int loadparm = get_loadparm_index();
 block_number_t block_nr;
 uint64_t address;
-ScsiMbr *bte = (void *)sec; /* Eckd bootmap table entry */
+BootMapTable *bmt = (void *)sec;
 BootMapScript *bms = (void *)sec;
 
 debug_print_int("loadparm", loadparm);
-IPL_assert(loadparm < 31, "loadparm value greater than"
+IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
" maximum number of boot entries allowed");
 
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
-read_block(mbr_block_nr, sec, "Cannot read MBR");
+read_block(bmt_block_nr, sec, "Cannot read Boot Map Table");
 
-block_nr = eckd_block_num((void *)&(bte->blockptr[loadparm]));
-IPL_assert(block_nr != -1, "No Boot Map");
+block_nr = eckd_block_num(>entry[loadparm]);
+IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry");
 
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(block_nr, sec, "Cannot read Boot Map Script");
@@ -223,7 +223,7 @@ static void ipl_eckd_cdl(void)
 XEckdMbr *mbr;
 Ipl2 *ipl2 = (void *)sec;
 IplVolumeLabel *vlbl = (void *)sec;
-block_number_t block_nr;
+block_number_t bmt_block_nr;
 
 /* we have just read the block #0 and recognized it as "IPL1" */
 sclp_print("CDL\n");
@@ -238,8 +238,8 @@ static void ipl_eckd_cdl(void)
 IPL_assert(mbr->dev_type == DEV_TYPE_ECKD,
"Non-ECKD device type in zIPL section of IPL2 record.");
 
-/* save pointer to Boot Script */
-block_nr = eckd_block_num((void *)&(mbr->blockptr));
+/* save pointer to Boot Map Table */
+bmt_block_nr = eckd_block_num(>blockptr);
 
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(2, vlbl, "Cannot read Volume Label at block 2");
@@ -249,7 +249,7 @@ static void ipl_eckd_cdl(void)
"Invalid magic of volser block");
 print_volser(vlbl->f.volser);
 
-run_eckd_boot_script(block_nr);
+run_eckd_boot_script(bmt_block_nr);
 /* no return */
 }
 
@@ -280,7 +280,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode)
 
 static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
 {
-block_number_t block_nr;
+block_number_t bmt_block_nr;
 BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */
 
 if (mode != ECKD_LDL_UNLABELED) {
@@ -299,8 +299,10 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
 }
 verify_boot_info(bip);
 
-block_nr = eckd_block_num((void *)&(bip->bp.ipl.bm_ptr.eckd.bptr));
-run_eckd_boot_script(block_nr);
+/* save pointer to Boot Map Table */
+bmt_block_nr = eckd_block_num((void *)>bp.ipl.bm_ptr.eckd.bptr);
+
+run_eckd_boot_script(bmt_block_nr);
 /* no return */
 }
 
@@ -325,7 +327,7 @@ static void print_eckd_msg(void)
 
 static void ipl_eckd(void)
 {
-ScsiMbr *mbr = (void *)sec;
+XEckdMbr *mbr = (void *)sec;
 LDL_VTOC *vlbl = (void *)sec;
 
 print_eckd_msg();
@@ -449,10 +451,8 @@ static void zipl_run(ScsiBlockPtr *pte)
 static void ipl_scsi(void)
 {
 ScsiMbr *mbr = (void *)sec;
-uint8_t *ns, *ns_end;
 int program_table_entries = 0;
-const int pte_len = sizeof(ScsiBlockPtr);
-ScsiBlockPtr *prog_table_entry = NULL;
+BootMapTable *prog_table = (void *)sec;
 unsigned int loadparm = get_loadparm_index();
 
 /* Grab the MBR */
@@ -467,34 +467,28 @@ static void ipl_scsi(void)
 debug_print_int("MBR Version", mbr->version_id);
 IPL_check(mbr->version_id == 1,
   "Unknown MBR layout version, assuming version 1");
-debug_print_int("program table", mbr->blockptr[0].blockno);
-IPL_assert(mbr->blockptr[0].blockno, "No Program Table");
+debug_print_int("program table", mbr->pt.blockno);
+IPL_assert(mbr->pt.blockno, "No Program Table");
 
 /* Parse the program table */
-

Re: [Qemu-devel] [PATCHv2 1/2] translator: merge max_insns into DisasContextBase

[Richard Henderson]

On 02/21/2018 12:17 PM, Emilio G. Cota wrote:
> On Wed, Feb 21, 2018 at 10:19:05 -0800, Richard Henderson wrote:
>> On 02/19/2018 06:26 PM, Emilio G. Cota wrote:
>>> @@ -68,6 +69,7 @@ typedef struct DisasContextBase {
>>>  target_ulong pc_next;
>>>  DisasJumpType is_jmp;
>>>  unsigned int num_insns;
>>> +int max_insns;
>>>  bool singlestep_enabled;
>>>  } DisasContextBase;
>>
>> We really should pick the same type for max_insns and num_insns, which ever
>> type we settle on.  I can't see how we can go wrong with unsigned...
> 
> I was just trying to avoid warnings with -Wsign-compare in case
> we ever enabled it.

Well, right.  That's my point.  We compare num_insns < max_insns.

> Should I bother converting the "bound" variables we use for
> MIN(max_insns, bound) to unsigned as well? Or just leave them
> alone and forget about -Wsign-compare?

You could change num_insns to "int" as well.


r~

[Qemu-devel] [PATCH v8 13/13] s390-ccw: interactive boot menu for scsi

[Collin L. Walling]

Interactive boot menu for scsi. This follows a similar procedure
as the interactive menu for eckd dasd. An example follows:

s390x Enumerated Boot Menu.

3 entries detected. Select from index 0 to 2.

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 hw/s390x/ipl.c  |  1 +
 pc-bios/s390-ccw/bootmap.c  |  4 
 pc-bios/s390-ccw/main.c |  1 +
 pc-bios/s390-ccw/menu.c | 14 ++
 pc-bios/s390-ccw/s390-ccw.h |  1 +
 5 files changed, 21 insertions(+)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index a0f4f40..566248e 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -238,6 +238,7 @@ static void s390_ipl_set_boot_menu(S390IPLState *ipl)
 *flags |= QIPL_FLAG_BM_OPTS_ZIPL;
 return;
 }
+case S390_IPL_TYPE_QEMU_SCSI:
 break;
 default:
 error_report("boot menu is not supported for this device type.");
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 2701ae6..81dbd4a 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -568,6 +568,10 @@ static void ipl_scsi(void)
 debug_print_int("program table entries", program_table_entries);
 IPL_assert(program_table_entries != 0, "Empty Program Table");
 
+if (menu_is_enabled()) {
+loadparm = menu_get_enum_boot_index(program_table_entries);
+}
+
 debug_print_int("loadparm", loadparm);
 IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
" maximum number of boot entries allowed");
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index a7473b0..9d9f8cf 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -92,6 +92,7 @@ static void menu_setup(void)
 
 switch (iplb.pbt) {
 case S390_IPL_TYPE_CCW:
+case S390_IPL_TYPE_QEMU_SCSI:
 menu_set_parms(qipl.qipl_flags & BOOT_MENU_FLAG_MASK,
qipl.boot_menu_timeout);
 return;
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 4fc328d..16b5310 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -217,6 +217,20 @@ int menu_get_zipl_boot_index(const char *menu_data)
 return get_boot_index(entries - 1); /* subtract 1 to exclude banner */
 }
 
+int menu_get_enum_boot_index(int entries)
+{
+char tmp[4];
+
+sclp_print("s390x Enumerated Boot Menu.\n\n");
+
+sclp_print(uitoa(entries, tmp, sizeof(tmp)));
+sclp_print(" entries detected. Select from boot index 0 to ");
+sclp_print(uitoa(entries - 1, tmp, sizeof(tmp)));
+sclp_print(".\n\n");
+
+return get_boot_index(entries);
+}
+
 void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
 {
 flag = boot_menu_flag;
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 56b0c2a..cfcaf3c 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -91,6 +91,7 @@ void zipl_load(void);
 void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
 bool menu_is_enabled(void);
 int menu_get_zipl_boot_index(const char *menu_data);
+int menu_get_enum_boot_index(int entries);
 
 static inline void fill_hex(char *out, unsigned char val)
 {
-- 
2.7.4

[Qemu-devel] [PATCH v8 08/13] s390-ccw: read stage2 boot loader data to find menu

[Collin L. Walling]

Read the stage2 boot loader data block-by-block. We scan the
current block for the string "zIPL" to detect the start of the
boot menu banner. We then load the adjacent blocks (previous
block and next block) to account for the possibility of menu
data spanning multiple blocks.

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/bootmap.c  | 94 ++---
 pc-bios/s390-ccw/bootmap.h  | 23 ++-
 pc-bios/s390-ccw/menu.c |  5 +++
 pc-bios/s390-ccw/s390-ccw.h |  1 +
 4 files changed, 116 insertions(+), 7 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 092fb35..2701ae6 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -83,6 +83,10 @@ static void jump_to_IPL_code(uint64_t address)
 
 static unsigned char _bprs[8*1024]; /* guessed "max" ECKD sector size */
 static const int max_bprs_entries = sizeof(_bprs) / sizeof(ExtEckdBlockPtr);
+static uint8_t _s2[MAX_SECTOR_SIZE * 3] 
__attribute__((__aligned__(PAGE_SIZE)));
+static void *s2_prev_blk = _s2;
+static void *s2_cur_blk = _s2 + MAX_SECTOR_SIZE;
+static void *s2_next_blk = _s2 + MAX_SECTOR_SIZE * 2;
 
 static inline void verify_boot_info(BootInfo *bip)
 {
@@ -182,7 +186,77 @@ static block_number_t load_eckd_segments(block_number_t 
blk, uint64_t *address)
 return block_nr;
 }
 
-static void run_eckd_boot_script(block_number_t bmt_block_nr)
+static bool find_zipl_boot_menu_banner(int *offset)
+{
+int i;
+
+/* Menu banner starts with "zIPL" */
+for (i = 0; i < virtio_get_block_size() - 4; i++) {
+if (magic_match(s2_cur_blk + i, ZIPL_MAGIC_EBCDIC)) {
+*offset = i;
+return true;
+}
+}
+
+return false;
+}
+
+static int eckd_get_boot_menu_index(block_number_t s1b_block_nr)
+{
+block_number_t cur_block_nr;
+block_number_t prev_block_nr = 0;
+block_number_t next_block_nr = 0;
+EckdStage1b *s1b = (void *)sec;
+int banner_offset;
+int i;
+
+/* Get Stage1b data */
+memset(sec, FREE_SPACE_FILLER, sizeof(sec));
+read_block(s1b_block_nr, s1b, "Cannot read stage1b boot loader");
+
+memset(_s2, FREE_SPACE_FILLER, sizeof(_s2));
+
+/* Get Stage2 data */
+for (i = 0; i < STAGE2_BLK_CNT_MAX; i++) {
+cur_block_nr = eckd_block_num(>seek[i].chs);
+
+if (!cur_block_nr) {
+break;
+}
+
+read_block(cur_block_nr, s2_cur_blk, "Cannot read stage2 boot loader");
+
+if (find_zipl_boot_menu_banner(_offset)) {
+/*
+ * Load the adjacent blocks to account for the
+ * possibility of menu data spanning multiple blocks.
+ */
+if (prev_block_nr) {
+read_block(prev_block_nr, s2_prev_blk,
+   "Cannot read stage2 boot loader");
+}
+
+if (i + 1 < STAGE2_BLK_CNT_MAX) {
+next_block_nr = eckd_block_num(>seek[i + 1].chs);
+}
+
+if (next_block_nr) {
+read_block(next_block_nr, s2_next_blk,
+   "Cannot read stage2 boot loader");
+}
+
+return menu_get_zipl_boot_index(s2_cur_blk + banner_offset);
+}
+
+prev_block_nr = cur_block_nr;
+}
+
+sclp_print("No zipl boot menu data found. Booting default entry.");
+return 0;
+}
+
+static void run_eckd_boot_script(block_number_t bmt_block_nr,
+ block_number_t s1b_block_nr)
 {
 int i;
 unsigned int loadparm = get_loadparm_index();
@@ -191,6 +265,10 @@ static void run_eckd_boot_script(block_number_t 
bmt_block_nr)
 BootMapTable *bmt = (void *)sec;
 BootMapScript *bms = (void *)sec;
 
+if (menu_is_enabled()) {
+loadparm = eckd_get_boot_menu_index(s1b_block_nr);
+}
+
 debug_print_int("loadparm", loadparm);
 IPL_assert(loadparm <= MAX_TABLE_ENTRIES, "loadparm value greater than"
" maximum number of boot entries allowed");
@@ -223,7 +301,7 @@ static void ipl_eckd_cdl(void)
 XEckdMbr *mbr;
 EckdCdlIpl2 *ipl2 = (void *)sec;
 IplVolumeLabel *vlbl = (void *)sec;
-block_number_t bmt_block_nr;
+block_number_t bmt_block_nr, s1b_block_nr;
 
 /* we have just read the block #0 and recognized it as "IPL1" */
 sclp_print("CDL\n");
@@ -241,6 +319,9 @@ static void ipl_eckd_cdl(void)
 /* save pointer to Boot Map Table */
 bmt_block_nr = eckd_block_num(>blockptr.xeckd.bptr.chs);
 
+/* save pointer to Stage1b Data */
+s1b_block_nr = eckd_block_num(>stage1.seek[0].chs);
+
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(2, vlbl, "Cannot read Volume Label at block 2");
 IPL_assert(magic_match(vlbl->key, VOL1_MAGIC),
@@ -249,7 +330,7 @@ static void ipl_eckd_cdl(void)
"Invalid magic of volser block");
 print_volser(vlbl->f.volser);
 
- 

[Qemu-devel] [PATCH v8 07/13] s390-ccw: set up interactive boot menu parameters

[Collin L. Walling]

Reads boot menu flag and timeout values from the iplb and
sets the respective fields for the menu.

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/Makefile   |  2 +-
 pc-bios/s390-ccw/main.c | 24 
 pc-bios/s390-ccw/menu.c | 27 +++
 pc-bios/s390-ccw/s390-ccw.h |  4 
 4 files changed, 56 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/s390-ccw/menu.c

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 9f7904f..1712c2d 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 
 .PHONY : all clean build-all
 
-OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o 
virtio-blkdev.o libc.o
+OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o 
virtio-blkdev.o libc.o menu.o
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
 QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
 QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index e41b264..32ed70e 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -18,6 +18,9 @@ IplParameterBlock iplb 
__attribute__((__aligned__(PAGE_SIZE)));
 static char loadparm[8] = { 0, 0, 0, 0, 0, 0, 0, 0 };
 QemuIplParameters qipl;
 
+#define LOADPARM_PROMPT "PROMPT  "
+#define LOADPARM_EMPTY  ""
+
 /*
  * Priniciples of Operations (SA22-7832-09) chapter 17 requires that
  * a subsystem-identification is at 184-187 and bytes 188-191 are zero
@@ -74,6 +77,26 @@ static bool find_dev(Schib *schib, int dev_no)
 return false;
 }
 
+static void menu_setup(void)
+{
+if (memcmp(loadparm, LOADPARM_PROMPT, 8) == 0) {
+menu_set_parms(QIPL_FLAG_BM_OPTS_CMD, 0);
+return;
+}
+
+/* If loadparm was set to any other value, then do not enable menu */
+if (memcmp(loadparm, LOADPARM_EMPTY, 8) != 0) {
+return;
+}
+
+switch (iplb.pbt) {
+case S390_IPL_TYPE_CCW:
+menu_set_parms(qipl.qipl_flags & QIPL_FLAG_BM_OPTS_CMD,
+   qipl.boot_menu_timeout);
+return;
+}
+}
+
 static void virtio_setup(void)
 {
 Schib schib;
@@ -117,6 +140,7 @@ static void virtio_setup(void)
 default:
 panic("List-directed IPL not supported yet!\n");
 }
+menu_setup();
 } else {
 for (ssid = 0; ssid < 0x3; ssid++) {
 blk_schid.ssid = ssid;
diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
new file mode 100644
index 000..03d91ea
--- /dev/null
+++ b/pc-bios/s390-ccw/menu.c
@@ -0,0 +1,27 @@
+/*
+ * QEMU S390 Interactive Boot Menu
+ *
+ * Copyright 2018 IBM Corp.
+ * Author: Collin L. Walling 
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+
+static uint8_t flag;
+static uint64_t timeout;
+
+void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout)
+{
+flag = boot_menu_flag;
+timeout = boot_menu_timeout;
+}
+
+bool menu_is_enabled(void)
+{
+return flag;
+}
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 25d4d21..09cc840 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -84,6 +84,10 @@ ulong get_second(void);
 /* bootmap.c */
 void zipl_load(void);
 
+/* menu.c */
+void menu_set_parms(uint8_t boot_menu_flag, uint32_t boot_menu_timeout);
+bool menu_is_enabled(void);
+
 static inline void fill_hex(char *out, unsigned char val)
 {
 const char hex[] = "0123456789abcdef";
-- 
2.7.4

[Qemu-devel] [PATCH v8 03/13] s390-ccw: refactor IPL structs

[Collin L. Walling]

ECKD DASDs have different IPL structures for CDL and LDL
formats. The current Ipl1 and Ipl2 structs follow the CDL
format, so we prepend "EckdCdl" to them. Boot info for LDL
has been moved to a new struct: EckdLdlIpl1.

Signed-off-by: Collin L. Walling 
Acked-by: Janosch Frank 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/bootmap.c | 12 ++--
 pc-bios/s390-ccw/bootmap.h | 37 +
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index 9534f56..a94638d 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -221,7 +221,7 @@ static void run_eckd_boot_script(block_number_t 
bmt_block_nr)
 static void ipl_eckd_cdl(void)
 {
 XEckdMbr *mbr;
-Ipl2 *ipl2 = (void *)sec;
+EckdCdlIpl2 *ipl2 = (void *)sec;
 IplVolumeLabel *vlbl = (void *)sec;
 block_number_t bmt_block_nr;
 
@@ -231,7 +231,7 @@ static void ipl_eckd_cdl(void)
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(1, ipl2, "Cannot read IPL2 record at block 1");
 
-mbr = >u.x.mbr;
+mbr = >mbr;
 IPL_assert(magic_match(mbr, ZIPL_MAGIC), "No zIPL section in IPL2 
record.");
 IPL_assert(block_size_ok(mbr->blockptr.xeckd.bptr.size),
"Bad block size in zIPL section of IPL2 record.");
@@ -281,7 +281,7 @@ static void print_eckd_ldl_msg(ECKD_IPL_mode_t mode)
 static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
 {
 block_number_t bmt_block_nr;
-BootInfo *bip = (void *)(sec + 0x70); /* BootInfo is MBR for LDL */
+EckdLdlIpl1 *ipl1 = (void *)sec;
 
 if (mode != ECKD_LDL_UNLABELED) {
 print_eckd_ldl_msg(mode);
@@ -292,15 +292,15 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(0, sec, "Cannot read block 0 to grab boot info.");
 if (mode == ECKD_LDL_UNLABELED) {
-if (!magic_match(bip->magic, ZIPL_MAGIC)) {
+if (!magic_match(ipl1->bip.magic, ZIPL_MAGIC)) {
 return; /* not applicable layout */
 }
 sclp_print("unlabeled LDL.\n");
 }
-verify_boot_info(bip);
+verify_boot_info(>bip);
 
 /* save pointer to Boot Map Table */
-bmt_block_nr = eckd_block_num(>bp.ipl.bm_ptr.eckd.bptr.chs);
+bmt_block_nr = eckd_block_num(>bip.bp.ipl.bm_ptr.eckd.bptr.chs);
 
 run_eckd_boot_script(bmt_block_nr);
 /* no return */
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index b361084..4bd95cd 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -239,22 +239,27 @@ typedef struct BootInfo {  /* @ 0x70, record #0   
 */
 } bp;
 } __attribute__ ((packed)) BootInfo; /* see also XEckdMbr   */
 
-typedef struct Ipl1 {
-unsigned char key[4]; /* == "IPL1" */
-unsigned char data[24];
-} __attribute__((packed)) Ipl1;
-
-typedef struct Ipl2 {
-unsigned char key[4]; /* == "IPL2" */
-union {
-unsigned char data[144];
-struct {
-unsigned char reserved1[92-4];
-XEckdMbr mbr;
-unsigned char reserved2[144-(92-4)-sizeof(XEckdMbr)];
-} x;
-} u;
-} __attribute__((packed)) Ipl2;
+/*
+ * Structs for IPL
+ */
+#define STAGE2_BLK_CNT_MAX  24 /* Stage 1b can load up to 24 blocks */
+
+typedef struct EckdCdlIpl1 {
+uint8_t key[4]; /* == "IPL1" */
+uint8_t data[24];
+} __attribute__((packed)) EckdCdlIpl1;
+
+typedef struct EckdCdlIpl2 {
+uint8_t key[4]; /* == "IPL2" */
+uint8_t reserved0[88];
+XEckdMbr mbr;
+uint8_t reserved[24];
+} __attribute__((packed)) EckdCdlIpl2;
+
+typedef struct EckdLdlIpl1 {
+uint8_t reserved[112];
+BootInfo bip; /* BootInfo is MBR for LDL */
+} __attribute__((packed)) EckdLdlIpl1;
 
 typedef struct IplVolumeLabel {
 unsigned char key[4]; /* == "VOL1" */
-- 
2.7.4

[Qemu-devel] [PATCH v8 04/13] s390-ccw: update libc

[Collin L. Walling]

Moved:
  memcmp from bootmap.h to libc.h (renamed from _memcmp)
  strlen from sclp.c to libc.h (renamed from _strlen)

Added C standard functions:
  isdigit

Added non C-standard function:
  uitoa
  atoui

Signed-off-by: Collin L. Walling 
Acked-by: Christian Borntraeger 
Reviewed-by: Janosch Frank 
---
 pc-bios/s390-ccw/Makefile  |  2 +-
 pc-bios/s390-ccw/bootmap.c |  4 +--
 pc-bios/s390-ccw/bootmap.h | 16 +
 pc-bios/s390-ccw/libc.c| 88 ++
 pc-bios/s390-ccw/libc.h| 37 +--
 pc-bios/s390-ccw/main.c| 17 +
 pc-bios/s390-ccw/sclp.c| 10 +-
 7 files changed, 129 insertions(+), 45 deletions(-)
 create mode 100644 pc-bios/s390-ccw/libc.c

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index 6d0c2ee..9f7904f 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -9,7 +9,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 
 .PHONY : all clean build-all
 
-OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o 
virtio-blkdev.o
+OBJECTS = start.o main.o bootmap.o sclp.o virtio.o virtio-scsi.o 
virtio-blkdev.o libc.o
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
 QEMU_CFLAGS += -ffreestanding -fno-delete-null-pointer-checks -msoft-float
 QEMU_CFLAGS += -march=z900 -fPIE -fno-strict-aliasing
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index a94638d..092fb35 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -506,7 +506,7 @@ static bool is_iso_bc_entry_compatible(IsoBcSection *s)
 "Failed to read image sector 0");
 
 /* Checking bytes 8 - 32 for S390 Linux magic */
-return !_memcmp(magic_sec + 8, linux_s390_magic, 24);
+return !memcmp(magic_sec + 8, linux_s390_magic, 24);
 }
 
 /* Location of the current sector of the directory */
@@ -635,7 +635,7 @@ static uint32_t find_iso_bc(void)
 if (vd->type == VOL_DESC_TYPE_BOOT) {
 IsoVdElTorito *et = >vd.boot;
 
-if (!_memcmp(>el_torito[0], el_torito_magic, 32)) {
+if (!memcmp(>el_torito[0], el_torito_magic, 32)) {
 return bswap32(et->bc_offset);
 }
 }
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 4bd95cd..4cf7e1e 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -328,20 +328,6 @@ static inline bool magic_match(const void *data, const 
void *magic)
 return *((uint32_t *)data) == *((uint32_t *)magic);
 }
 
-static inline int _memcmp(const void *s1, const void *s2, size_t n)
-{
-int i;
-const uint8_t *p1 = s1, *p2 = s2;
-
-for (i = 0; i < n; i++) {
-if (p1[i] != p2[i]) {
-return p1[i] > p2[i] ? 1 : -1;
-}
-}
-
-return 0;
-}
-
 static inline uint32_t iso_733_to_u32(uint64_t x)
 {
 return (uint32_t)x;
@@ -434,7 +420,7 @@ const uint8_t vol_desc_magic[] = "CD001";
 
 static inline bool is_iso_vd_valid(IsoVolDesc *vd)
 {
-return !_memcmp(>ident[0], vol_desc_magic, 5) &&
+return !memcmp(>ident[0], vol_desc_magic, 5) &&
vd->version == 0x1 &&
vd->type <= VOL_DESC_TYPE_PARTITION;
 }
diff --git a/pc-bios/s390-ccw/libc.c b/pc-bios/s390-ccw/libc.c
new file mode 100644
index 000..38ea77d
--- /dev/null
+++ b/pc-bios/s390-ccw/libc.c
@@ -0,0 +1,88 @@
+/*
+ * libc-style definitions and functions
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Collin L. Walling 
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ */
+
+#include "libc.h"
+#include "s390-ccw.h"
+
+/**
+ * atoui:
+ * @str: the string to be converted.
+ *
+ * Given a string @str, convert it to an integer. Leading spaces are
+ * ignored. Any other non-numerical value will terminate the conversion
+ * and return 0. This function only handles numbers between 0 and
+ * UINT64_MAX inclusive.
+ *
+ * Returns: an integer converted from the string @str, or the number 0
+ * if an error occurred.
+ */
+uint64_t atoui(const char *str)
+{
+int val = 0;
+
+if (!str || !str[0]) {
+return 0;
+}
+
+while (*str == ' ') {
+str++;
+}
+
+while (*str) {
+if (!isdigit(*str)) {
+break;
+}
+val = val * 10 + *str - '0';
+str++;
+}
+
+return val;
+}
+
+/**
+ * uitoa:
+ * @num: an integer (base 10) to be converted.
+ * @str: a pointer to a string to store the conversion.
+ * @len: the length of the passed string.
+ *
+ * Given an integer @num, convert it to a string. The string @str must be
+ * allocated beforehand. The resulting string will be null terminated and
+ * returned. This function only handles numbers 

[Qemu-devel] [PATCH v8 02/13] s390-ccw: refactor eckd_block_num to use CHS

[Collin L. Walling]

Add new cylinder/head/sector struct. Use it to calculate
eckd block numbers instead of a BootMapPointer (which used
eckd chs anyway).

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/bootmap.c | 28 ++--
 pc-bios/s390-ccw/bootmap.h |  8 ++--
 2 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index a4eaf24..9534f56 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -95,32 +95,32 @@ static inline void verify_boot_info(BootInfo *bip)
"Bad block size in zIPL section of the 1st record.");
 }
 
-static block_number_t eckd_block_num(BootMapPointer *p)
+static block_number_t eckd_block_num(EckdCHS *chs)
 {
 const uint64_t sectors = virtio_get_sectors();
 const uint64_t heads = virtio_get_heads();
-const uint64_t cylinder = p->eckd.cylinder
-+ ((p->eckd.head & 0xfff0) << 12);
-const uint64_t head = p->eckd.head & 0x000f;
+const uint64_t cylinder = chs->cylinder
++ ((chs->head & 0xfff0) << 12);
+const uint64_t head = chs->head & 0x000f;
 const block_number_t block = sectors * heads * cylinder
+ sectors * head
-   + p->eckd.sector
+   + chs->sector
- 1; /* block nr starts with zero */
 return block;
 }
 
 static bool eckd_valid_address(BootMapPointer *p)
 {
-const uint64_t head = p->eckd.head & 0x000f;
+const uint64_t head = p->eckd.chs.head & 0x000f;
 
 if (head >= virtio_get_heads()
-||  p->eckd.sector > virtio_get_sectors()
-||  p->eckd.sector <= 0) {
+||  p->eckd.chs.sector > virtio_get_sectors()
+||  p->eckd.chs.sector <= 0) {
 return false;
 }
 
 if (!virtio_guessed_disk_nature() &&
-eckd_block_num(p) >= virtio_get_blocks()) {
+eckd_block_num(>eckd.chs) >= virtio_get_blocks()) {
 return false;
 }
 
@@ -140,7 +140,7 @@ static block_number_t load_eckd_segments(block_number_t 
blk, uint64_t *address)
 do {
 more_data = false;
 for (j = 0;; j++) {
-block_nr = eckd_block_num((void *)&(bprs[j].xeckd));
+block_nr = eckd_block_num([j].xeckd.bptr.chs);
 if (is_null_block_number(block_nr)) { /* end of chunk */
 break;
 }
@@ -198,7 +198,7 @@ static void run_eckd_boot_script(block_number_t 
bmt_block_nr)
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(bmt_block_nr, sec, "Cannot read Boot Map Table");
 
-block_nr = eckd_block_num(>entry[loadparm]);
+block_nr = eckd_block_num(>entry[loadparm].xeckd.bptr.chs);
 IPL_assert(block_nr != -1, "Cannot find Boot Map Table Entry");
 
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
@@ -206,7 +206,7 @@ static void run_eckd_boot_script(block_number_t 
bmt_block_nr)
 
 for (i = 0; bms->entry[i].type == BOOT_SCRIPT_LOAD; i++) {
 address = bms->entry[i].address.load_address;
-block_nr = eckd_block_num(&(bms->entry[i].blkptr));
+block_nr = eckd_block_num(>entry[i].blkptr.xeckd.bptr.chs);
 
 do {
 block_nr = load_eckd_segments(block_nr, );
@@ -239,7 +239,7 @@ static void ipl_eckd_cdl(void)
"Non-ECKD device type in zIPL section of IPL2 record.");
 
 /* save pointer to Boot Map Table */
-bmt_block_nr = eckd_block_num(>blockptr);
+bmt_block_nr = eckd_block_num(>blockptr.xeckd.bptr.chs);
 
 memset(sec, FREE_SPACE_FILLER, sizeof(sec));
 read_block(2, vlbl, "Cannot read Volume Label at block 2");
@@ -300,7 +300,7 @@ static void ipl_eckd_ldl(ECKD_IPL_mode_t mode)
 verify_boot_info(bip);
 
 /* save pointer to Boot Map Table */
-bmt_block_nr = eckd_block_num((void *)>bp.ipl.bm_ptr.eckd.bptr);
+bmt_block_nr = eckd_block_num(>bp.ipl.bm_ptr.eckd.bptr.chs);
 
 run_eckd_boot_script(bmt_block_nr);
 /* no return */
diff --git a/pc-bios/s390-ccw/bootmap.h b/pc-bios/s390-ccw/bootmap.h
index 486c0f3..b361084 100644
--- a/pc-bios/s390-ccw/bootmap.h
+++ b/pc-bios/s390-ccw/bootmap.h
@@ -32,10 +32,14 @@ typedef struct FbaBlockPtr {
 uint16_t blockct;
 } __attribute__ ((packed)) FbaBlockPtr;
 
-typedef struct EckdBlockPtr {
-uint16_t cylinder; /* cylinder/head/sector is an address of the block */
+typedef struct EckdCHS {
+uint16_t cylinder;
 uint16_t head;
 uint8_t sector;
+} __attribute__ ((packed)) EckdCHS;
+
+typedef struct EckdBlockPtr {
+EckdCHS chs; /* cylinder/head/sector is an address of the block */
 uint16_t size;
 uint8_t count; /* (size_in_blocks-1);
 * it's 0 for TablePtr, ScriptPtr, and SectionPtr */
-- 
2.7.4

[Qemu-devel] [PATCH v8 10/13] s390-ccw: read user input for boot index via the SCLP console

[Collin L. Walling]

Implements an sclp_read function to capture input from the
console and a wrapper function that handles parsing certain
characters and adding input to a buffer. The input is checked
for any erroneous values and is handled appropriately.

A prompt will persist until input is entered or the timeout
expires (if one was set). Example:

  Please choose (default will boot in 10 seconds):

Correct input will boot the respective boot index. If the
user's input is empty, 0, or if the timeout expires, then
the default zipl entry will be chosen. If the input is
within the range of available boot entries, then the
selection will be booted. Any erroneous input will cancel
the timeout and re-prompt the user.

Signed-off-by: Collin L. Walling 
Reviewed-by: Thomas Huth 
---
 pc-bios/s390-ccw/menu.c | 149 +++-
 pc-bios/s390-ccw/s390-ccw.h |   2 +
 pc-bios/s390-ccw/sclp.c |  19 ++
 pc-bios/s390-ccw/virtio.c   |   2 +-
 4 files changed, 170 insertions(+), 2 deletions(-)

diff --git a/pc-bios/s390-ccw/menu.c b/pc-bios/s390-ccw/menu.c
index 8f50e55..01b120d 100644
--- a/pc-bios/s390-ccw/menu.c
+++ b/pc-bios/s390-ccw/menu.c
@@ -12,12 +12,159 @@
 #include "libc.h"
 #include "s390-ccw.h"
 
+#define KEYCODE_NO_INP '\0'
+#define KEYCODE_ESCAPE '\033'
+#define KEYCODE_BACKSP '\177'
+#define KEYCODE_ENTER  '\r'
+
+#define TOD_CLOCK_MILLISECOND   0x3e8000
+
+#define LOW_CORE_EXTERNAL_INT_ADDR   0x86
+#define CLOCK_COMPARATOR_INT 0X1004
+
 static uint8_t flag;
 static uint64_t timeout;
 
+static inline void enable_clock_int(void)
+{
+uint64_t tmp = 0;
+
+asm volatile(
+"stctg  0,0,%0\n"
+"oi 6+%0, 0x8\n"
+"lctlg  0,0,%0"
+: : "Q" (tmp) : "memory"
+);
+}
+
+static inline void disable_clock_int(void)
+{
+uint64_t tmp = 0;
+
+asm volatile(
+"stctg  0,0,%0\n"
+"ni 6+%0, 0xf7\n"
+"lctlg  0,0,%0"
+: : "Q" (tmp) : "memory"
+);
+}
+
+static inline void set_clock_comparator(uint64_t time)
+{
+asm volatile("sckc %0" : : "Q" (time));
+}
+
+static inline bool check_clock_int(void)
+{
+uint16_t *code = (uint16_t *)LOW_CORE_EXTERNAL_INT_ADDR;
+
+consume_sclp_int();
+
+return *code == CLOCK_COMPARATOR_INT;
+}
+
+static int read_prompt(char *buf, size_t len)
+{
+char inp[2] = {};
+uint8_t idx = 0;
+uint64_t time;
+
+if (timeout) {
+time = get_clock() + timeout * TOD_CLOCK_MILLISECOND;
+set_clock_comparator(time);
+enable_clock_int();
+timeout = 0;
+}
+
+while (!check_clock_int()) {
+
+sclp_read(inp, 1); /* Process only one character at a time */
+
+switch (inp[0]) {
+case KEYCODE_NO_INP:
+case KEYCODE_ESCAPE:
+continue;
+case KEYCODE_BACKSP:
+if (idx > 0) {
+buf[--idx] = 0;
+sclp_print("\b \b");
+}
+continue;
+case KEYCODE_ENTER:
+disable_clock_int();
+return idx;
+default:
+/* Echo input and add to buffer */
+if (idx < len) {
+buf[idx++] = inp[0];
+sclp_print(inp);
+}
+}
+}
+
+disable_clock_int();
+*buf = 0;
+
+return 0;
+}
+
+static int get_index(void)
+{
+char buf[11];
+int len;
+int i;
+
+memset(buf, 0, sizeof(buf));
+
+len = read_prompt(buf, sizeof(buf) - 1);
+
+/* If no input, boot default */
+if (len == 0) {
+return 0;
+}
+
+/* Check for erroneous input */
+for (i = 0; i < len; i++) {
+if (!isdigit(buf[i])) {
+return -1;
+}
+}
+
+return atoui(buf);
+}
+
+static void boot_menu_prompt(bool retry)
+{
+char tmp[11];
+
+if (retry) {
+sclp_print("\nError: undefined configuration"
+   "\nPlease choose:\n");
+} else if (timeout > 0) {
+sclp_print("Please choose (default will boot in ");
+sclp_print(uitoa(timeout / 1000, tmp, sizeof(tmp)));
+sclp_print(" seconds):\n");
+} else {
+sclp_print("Please choose:\n");
+}
+}
+
 static int get_boot_index(int entries)
 {
-return 0; /* implemented next patch */
+int boot_index;
+bool retry = false;
+char tmp[5];
+
+do {
+boot_menu_prompt(retry);
+boot_index = get_index();
+retry = true;
+} while (boot_index < 0 || boot_index >= entries);
+
+sclp_print("\nBooting entry #");
+sclp_print(uitoa(boot_index, tmp, sizeof(tmp)));
+
+return boot_index;
 }
 
 static void zipl_println(const char *data, size_t len)
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index 24b48b0..f7c12bf 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -71,6 +71,7 @@ unsigned int get_loadparm_index(void);
 void sclp_print(const char 

Re: [Qemu-devel] [PATCH v2 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

[Cornelia Huck]

On Wed, 21 Feb 2018 18:29:19 +0100
David Hildenbrand  wrote:

> On 21.02.2018 17:56, Halil Pasic wrote:

s/390x/s390x/

> > The 'bit' field of the 'S390FeatDef' structure is not applicable to all
> > its instances. Currently this field is not applicable, and remains
> > unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value 0
> > specified for multiple such feature definition  was a little confusing,  
> 
> s/  / /

also s/definition/definitions/

> 
> > as it's a perfectly legit bit value, and as the value of the bit
> > field is usually ought to be unique for each feature of a given
> > feature type.
> > 
> > Let us introduce a specialized macro for defining features of type
> > S390_FEAT_TYPE_MISC so, that one does not have to specify neither bit nor
> > type (as the later is implied).  
> 
> s/later is implied/latter is implicit/

I kept 'implied'.

> 
> > 
> > Signed-off-by: Halil Pasic 
> > ---
> > 
> > v1 -> v2
> > * Specialized feature initializer macro for type MISC that does not
> >   require a bit value instead of defining a 'not a bit number' (that
> >   is extremal) bit number.
> > ---
> >  target/s390x/cpu_features.c | 8 ++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)

(...)

> Reviewed-by: David Hildenbrand 

Thanks, applied.

Re: [Qemu-devel] [PATCH] configure: fix sanitizers' test program to mend ASan detection

[Emilio G. Cota]

On Wed, Feb 21, 2018 at 10:12:28 +0100, Marc-André Lureau wrote:
> I sent a patch a few days ago:
> "[PATCH 1/6] build-sys: fix -fsanitize=address check"

Ouch, I missed that. Please disregard my patch.

Thanks,

Emilio

Re: [Qemu-devel] [PATCH 2/2] target/openrisc: convert to TranslatorOps

[Richard Henderson]

On 02/17/2018 05:32 PM, Emilio G. Cota wrote:
> +dc->next_page_start = (dc->base.pc_first & TARGET_PAGE_MASK) +
> +TARGET_PAGE_SIZE;
> +return max_insns;

OpenRISC is standard 4-byte risc insn; should use the same bound mechanism as
ppc and sparc.


r~

Re: [Qemu-devel] [PATCH 0/2] Firmware blob and git submodule for Sam460ex

[Peter Maydell]

On 21 February 2018 at 17:06, BALATON Zoltan  wrote:
> It's not that upstream u-boot has abandoned board support (it only removed
> support for the PPC440 CPU it once had). The board itself never had support
> in upstream u-boot, it only exists in vendor's fork which is the reason we
> need a separate source and cannot use upstream u-boot source we already
> have.
>
> In my opinion we don't aim to take on support for this board in u-boot, we
> only need to include the firmware binary for the emulation to be useful
> which then requires us to also include the source for the GPL it's licensed
> under. I've also found a few bugs in the firmware which I've fixed but apart
> from such occasional bug fixes when needed I don't expect to take over
> support for the board from the hardware vendor so this source is only so we
> can include the firmware binary which is needed for the board emulation.
> Does this answer your concerns?

We have lots of boards we don't ship firmware blobs for and
which we expect the users to provide the guest code for
if they're going to use them. If we had a git submodule
for every random dev board model that needs some hardware
vendor's BSP and bootloader we'd probably have 50 submodules...

Which isn't to say I'm definitely against this -- I'm just
trying to figure out where we should draw the line of
"these bits of guest code we build for you and ship with
QEMU" versus "we provide the model of the hardware for you
to run whatever guest code you happen to have".

thanks
-- PMM

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

[Eric Blake]

On 02/21/2018 11:39 AM, Kevin Wolf wrote:

See my commit message comment - we have other spots in the code base that
blindly g_malloc(2 * s->cluster_size).


Though is that a reason to do the same in new code or to phase out such
allocations whenever you touch them?


Touché.




And I intended (but sent the email without amending my commit) to use
g_malloc().  But as Berto has convinced me that an externally produced
image can convince us to read up to 4M (even though we don't need that
much to decompress), I suppose that the try_ variant plus checking is
reasonable (and care in NULL'ing out if one but not both allocations
succeed).


Sounds good.

Another thought I had is whether we should do per-request allocation for
compressed clusters, too, instead of having per-BDS buffers.


The only benefit of a per-BDS buffer is that we cache things - multiple 
sub-cluster reads in a row all from the same compressed cluster benefit 
from decompressing only once.  The drawbacks of a per-BDS buffer: we 
can't do things in parallel (everything else in qcow2 drops the lock 
around bdrv_co_pread[v]), so the initial read prevents anything else in 
the qcow2 layer from progressing.


I also wonder - since we ARE allowing multiple parallel readers in other 
parts of qcow2 (without a patch, decompression is not in this boat, but 
decryption and even bounce buffers due to lower-layer alignment 
constraints are), what sort of mechanisms do we have for using a pool of 
reusable buffers, rather than having each cluster access that requires a 
buffer malloc and free the buffer on a per-access basis?  I don't know 
how much time the malloc/free per-transaction overhead adds, or if it is 
already much smaller than the actual I/O time.


But note that while reusable buffers from a pool would cut down on the 
per-I/O malloc/free overhead if we switch decompression away from 
per-BDS buffer, it would still not solve the fact that we only get the 
caching ability where multiple sub-cluster requests from the same 
compressed cluster require only one decompression, since that's only 
possible on a per-BDS caching level.


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

Re: [Qemu-devel] [PATCH v2 03/32] target/arm/cpu64: allow fp16 to be disabled

[Richard Henderson]

On 02/21/2018 08:35 AM, Alex Bennée wrote:
>> Good news everybody -- this is an opportunity for a naming bikeshed
>> discussion!
...
>>  * use the hwcaps names that Linux defines and prints in /proc/cpuinfo
>>(in this case that would be a combination of "fphp" and "asimdhp",
>>since hwcaps reflects the ID register setup that allows a CPU
>>to report support for one and not the other)
> 
> In naming I favour the Arm ARM over whatever Linux-ism /proc came up
> with.

Likewise.


r~

Re: [Qemu-devel] [PATCH v7 00/23] QMP: out-of-band (OOB) execution support

[Stefan Hajnoczi]

On Wed, Jan 24, 2018 at 01:39:34PM +0800, Peter Xu wrote:
> This version should have addressed all comments in previous one, also
> fixed another race condition after I addressed all the comments (a new
> race condition introduced by addressing the comments...).  For some
> more details of the race condition, please see the last entry of
> change log, and please refer to patch 9 for the code change.
> 
> I removed RFC tag from this version.  Please review.  Thanks.

I have posted comments on a few patches.

Stefan


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 19/23] qmp: isolate responses into io thread

[Stefan Hajnoczi]

On Wed, Jan 24, 2018 at 01:39:53PM +0800, Peter Xu wrote:
> For those monitors who have enabled IO thread, we'll offload the
> responding procedure into IO thread.  The main reason is that chardev is
> not thread safe, and we need to do all the read/write IOs in the same
> thread.  For use_io_thr=true monitors, that thread is the IO thread.
> 
> We do this isolation in similar pattern as what we have done to the
> request queue: we first create one response queue for each monitor, then
> instead of replying directly in the main thread, we queue the responses
> and kick the IO thread to do the rest of the job for us.
> 
> A funny thing after doing this is that, when the QMP clients send "quit"
> to QEMU, it's possible that we close the IOThread even earlier than
> replying to that "quit".  So another thing we need to do before cleaning
> up the monitors is that we need to flush the response queue (we don't
> need to do that for command queue; after all we are quitting) to make
> sure replies for handled commands are always flushed back to clients.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 96 
> ++-
>  1 file changed, 95 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

[Kevin Wolf]

Am 21.02.2018 um 17:59 hat Eric Blake geschrieben:
> On 02/21/2018 10:51 AM, Kevin Wolf wrote:
> > Am 20.02.2018 um 23:24 hat Eric Blake geschrieben:
> > > When reading a compressed image, we were allocating s->cluster_data
> > > to 32*cluster_size + 512 (possibly over 64 megabytes, for an image
> > > with 2M clusters).  Let's check out the history:
> > > 
> 
> > > Much later, in commit de82815d (v2.2), we noticed that a 64M
> > > allocation is prone to failure, so we switched over to a graceful
> > > memory allocation error message.  But note that elsewhere in the
> > > code, we do g_malloc(2 * cluster_size) without ever checking for
> > > failure.
> > > 
> 
> > > -}
> > > -if (!s->cluster_cache) {
> > > -s->cluster_cache = g_malloc(s->cluster_size);
> > > +assert(!s->cluster_cache);
> > 
> > Wouldn't it be better to assert (!!s->cluster_cache ==
> > !!s->cluster_data) unconditionally?
> > 
> 
> Sure.
> 
> > > +s->cluster_data = g_try_malloc(s->cluster_size);
> > 
> > Why are you going from qemu_try_blockalign() to simple malloc here? This
> > buffer is used with bdrv_read() (or bdrv_pread() after patch 1), so we
> > should avoid unnecessary use of a bounce buffer.
> 
> But does bdrv_pread() actually need to use a bounce buffer if we don't have
> an aligned buffer to read into?  Either the underlying protocol already
> supports byte-aligned reads (direct into our buffer, regardless of
> alignment, no bouncing required), or it already has do to a full sector read
> into a bounce buffer anyways (and it doesn't matter whether we aligned our
> buffer).  blockalign() made sense when we had multiple clients for the
> buffer, but ever since v1.1, when we have only a single client, and that
> single client is most likely not going to read sector-aligned data in the
> first place, aligning the buffer doesn't buy us anything.

Good point.

To be honest, I don't even analyse each caller, but just consistently use
qemu_try_blockalign() whenever a buffer is used for I/O. It's a simple
rule of thumb that generally makes sense.

So as you say, in this case it's unlikely, but possible that we can
benefit from an aligned buffer. I guess my point is more about
consistency than actual functionality then. But it's okay either way.

> > 
> > > +s->cluster_cache = g_try_malloc(s->cluster_size);
> > 
> > As you already said, either g_malloc() or check the result. I actually
> > think that g_try_malloc() and checking the result is nicer, we still
> > allocate up to 2 MB here.
> 
> See my commit message comment - we have other spots in the code base that
> blindly g_malloc(2 * s->cluster_size).

Though is that a reason to do the same in new code or to phase out such
allocations whenever you touch them?

> And I intended (but sent the email without amending my commit) to use
> g_malloc().  But as Berto has convinced me that an externally produced
> image can convince us to read up to 4M (even though we don't need that
> much to decompress), I suppose that the try_ variant plus checking is
> reasonable (and care in NULL'ing out if one but not both allocations
> succeed).

Sounds good.

Another thought I had is whether we should do per-request allocation for
compressed clusters, too, instead of having per-BDS buffers.

Kevin

Re: [Qemu-devel] [PATCH RFCv2] s390x/sclp: remove memory hotplug support

[Cornelia Huck]

On Tue, 20 Feb 2018 16:05:54 +0100
David Hildenbrand  wrote:

> On 20.02.2018 15:57, Cornelia Huck wrote:
> > On Tue, 20 Feb 2018 13:16:37 +0100
> > David Hildenbrand  wrote:
> >   
> >> On 20.02.2018 13:05, Christian Borntraeger wrote:  
> >>>
> >>>
> >>> On 02/19/2018 06:42 PM, David Hildenbrand wrote:
>  From an architecture point of view, nothing can be mapped into the 
>  address
>  space on s390x. All there is is memory. Therefore there is also not 
>  really
>  an interface to communicate such information to the guest. All we can do 
>  is
>  specify the maximum ram address and guests can probe in that range if
>  memory is available and usable (TPROT).
> >>>
> >>> In fact there is an interface in SCLP that describes the memory sizes 
> >>> (maximum in 
> >>> read scp info) and the details (read_storage_element0_info).  I am asking 
> >>> myself
> >>> if we should re-introduce read_storage_element_info and use that to avoid 
> >>> tprot
> >>
> >> Yes, we could do that (basically V1 of this patch) but have to glue it
> >> to the a compatibility machine then.  
> > 
> > Actually, this makes quite a bit of sense (introduce the interface for
> > everyone in 2.12 and turn it off in compat machines).  
> 
> Jup, either 2.12 or 2.13, no need to hurry.
> 
> > 
> > Does real hardware have configurations where you can get the memory
> > sizes, but not the attach/deattach support? (Hardware with the feature,
> > but no standby memory defined?)  
> 
> I would guess that "0" for standby memory is valid but only people with
> access to documentation can answer that :)

So, should we go with this patch now and re-introduce the read
functions if the above is indeed true?

> 
> >> Interesting, didn't know about that. Will rephrase then to
> >>
> >> "While the hypervisor can deny to online an increment, all increments
> >> have to be predefined and there is no way of telling the guest about a
> >> newly "hotplugged" increment."  
> > 
> > Rephrase which part? :)  
> 
> "And nobody can really hinder it from doing so."

OK.

Re: [Qemu-devel] [PATCH v7 18/23] qmp: support out-of-band (oob) execution

[Stefan Hajnoczi]

On Wed, Jan 24, 2018 at 01:39:52PM +0800, Peter Xu wrote:
> Having "allow-oob" to true for a command does not mean that this command
> will always be run in out-of-band mode.  The out-of-band quick path will
> only be executed if we specify the extra "run-oob" flag when sending the
> QMP request:
> 
> { "execute":   "command-that-allows-oob",
>   "arguments": { ... },
>   "control":   { "run-oob": true } }
> 
> The "control" key is introduced to store this extra flag.  "control"
> field is used to store arguments that are shared by all the commands,
> rather than command specific arguments.  Let "run-oob" be the first.
> 
> Note that in the patch I exported qmp_dispatch_check_obj() to be used to
> check the request earlier, and at the same time allowed "id" field to be
> there since actually we always allow that.
> 
> Signed-off-by: Peter Xu 
> ---
>  include/qapi/qmp/dispatch.h |  2 ++
>  monitor.c   | 84 
> -
>  qapi/qmp-dispatch.c | 32 -
>  trace-events|  2 ++
>  4 files changed, 110 insertions(+), 10 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v1 3/3] s390x/sclp: extend SCLP event masks to 64 bits

[Cornelia Huck]

On Wed, 21 Feb 2018 17:51:19 +0100
Claudio Imbrenda  wrote:

> On Wed, 21 Feb 2018 16:34:59 +0100
> Cornelia Huck  wrote:
> 
> > On Tue, 20 Feb 2018 19:45:02 +0100
> > Claudio Imbrenda  wrote:

> > > +static const VMStateDescription vmstate_event_facility_mask64 = {
> > > +.name = "vmstate-event-facility/mask64",
> > > +.version_id = 0,
> > > +.minimum_version_id = 0,
> > > +.needed = vmstate_event_facility_mask64_needed,
> > > +.pre_load = vmstate_event_facility_mask64_pre_load,
> > > +.fields = (VMStateField[]) {
> > > +VMSTATE_UINT64(receive_mask, SCLPEventFacility),
> > > +VMSTATE_END_OF_LIST()
> > > + }
> > > +};
> > > +
> > 
> > Are there plans for extending this beyond 64 bits? Would it make sense  
> 
> I don't know. I'm not even aware of anything above 32 bits, but since we
> are already using all of the first 32 bits, it's only matter of time I
> guess :)
> 
> > to use the maximum possible size for the mask here, so you don't need
> > to introduce yet another vmstate in the future? (If it's unlikely that  
> 
> That's true, but it requires changing simple scalars into bitmasks.
> Surely doable, but I wanted to touch as little as possible.

OK, that pushes this firmly into the 'overkill' area. Let's just go
with your current approach.

> 
> > the mask will ever move beyond 64 bit, that might be overkill, of
> > course.)

Re: [Qemu-devel] [PATCH v1 2/3] s390x/sclp: clean up sclp masks

[Cornelia Huck]

On Wed, 21 Feb 2018 17:42:57 +0100
Claudio Imbrenda  wrote:

> On Wed, 21 Feb 2018 16:20:05 +0100
> Cornelia Huck  wrote:
> 
> > On Tue, 20 Feb 2018 19:45:01 +0100
> > Claudio Imbrenda  wrote:

> > > diff --git a/include/hw/s390x/event-facility.h
> > > b/include/hw/s390x/event-facility.h index 5119b9b..0a8b47a 100644
> > > --- a/include/hw/s390x/event-facility.h
> > > +++ b/include/hw/s390x/event-facility.h
> > > @@ -28,12 +28,14 @@
> > >  #define SCLP_EVENT_SIGNAL_QUIESCE   0x1d
> > >  
> > >  /* SCLP event masks */
> > > -#define SCLP_EVENT_MASK_SIGNAL_QUIESCE  0x0008
> > > -#define SCLP_EVENT_MASK_MSG_ASCII   0x0040
> > > -#define SCLP_EVENT_MASK_CONFIG_MGT_DATA 0x1000
> > > -#define SCLP_EVENT_MASK_OP_CMD  0x8000
> > > -#define SCLP_EVENT_MASK_MSG 0x4000
> > > -#define SCLP_EVENT_MASK_PMSGCMD 0x0080
> > > +#define SCLPEVMSK(T)  (1ULL << (sizeof(sccb_mask_t) * 8 - (T)))
> > 
> > SCLP_EVMASK() would be a bit more readable, I think.  
> 
> I know, but then it looks ugly when trying to fit everything in 80
> columns. 

I'd rather go slightly over 80 in that case (as long as you don't cross
90).

> 
> > > +
> > > +#define SCLP_EVENT_MASK_OP_CMD
> > > SCLPEVMSK(SCLP_EVENT_OPRTNS_COMMAND) +#define
> > > SCLP_EVENT_MASK_MSG SCLPEVMSK(SCLP_EVENT_MESSAGE)
> > > +#define SCLP_EVENT_MASK_CONFIG_MGT_DATA
> > > SCLPEVMSK(SCLP_EVENT_CONFIG_MGT_DATA) +#define
> > > SCLP_EVENT_MASK_PMSGCMD SCLPEVMSK(SCLP_EVENT_PMSGCMD)
> > > +#define SCLP_EVENT_MASK_MSG_ASCII
> > > SCLPEVMSK(SCLP_EVENT_ASCII_CONSOLE_DATA) +#define
> > > SCLP_EVENT_MASK_SIGNAL_QUIESCE
> > > SCLPEVMSK(SCLP_EVENT_SIGNAL_QUIESCE) #define
> > > SCLP_UNCONDITIONAL_READ 0x00 #define
> > > SCLP_SELECTIVE_READ 0x01
> >   
> 

Re: [Qemu-devel] [PATCH v2 1/1] 390x/cpumodel: document S390FeatDef.bit not applicable

[David Hildenbrand]

On 21.02.2018 17:56, Halil Pasic wrote:
> The 'bit' field of the 'S390FeatDef' structure is not applicable to all
> its instances. Currently this field is not applicable, and remains
> unused, iff the feature is of type S390_FEAT_TYPE_MISC. Having the value 0
> specified for multiple such feature definition  was a little confusing,

s/  / /

> as it's a perfectly legit bit value, and as the value of the bit
> field is usually ought to be unique for each feature of a given
> feature type.
> 
> Let us introduce a specialized macro for defining features of type
> S390_FEAT_TYPE_MISC so, that one does not have to specify neither bit nor
> type (as the later is implied).

s/later is implied/latter is implicit/

> 
> Signed-off-by: Halil Pasic 
> ---
> 
> v1 -> v2
> * Specialized feature initializer macro for type MISC that does not
>   require a bit value instead of defining a 'not a bit number' (that
>   is extremal) bit number.
> ---
>  target/s390x/cpu_features.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index a5619f2893..3b9e2745e9 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -23,6 +23,10 @@
>  .desc = _desc,   \
>  }
>  
> +/* S390FeatDef.bit is not applicable as there is no feature block. */
> +#define FEAT_INIT_MISC(_name, _desc) \
> +FEAT_INIT(_name, S390_FEAT_TYPE_MISC, 0, _desc)
> +
>  /* indexed by feature number for easy lookup */
>  static const S390FeatDef s390_features[] = {
>  FEAT_INIT("esan3", S390_FEAT_TYPE_STFL, 0, "Instructions marked as n3"),
> @@ -123,8 +127,8 @@ static const S390FeatDef s390_features[] = {
>  FEAT_INIT("ib", S390_FEAT_TYPE_SCLP_CPU, 42, "SIE: Intervention bypass 
> facility"),
>  FEAT_INIT("cei", S390_FEAT_TYPE_SCLP_CPU, 43, "SIE: 
> Conditional-external-interception facility"),
>  
> -FEAT_INIT("dateh2", S390_FEAT_TYPE_MISC, 0, "DAT-enhancement facility 
> 2"),
> -FEAT_INIT("cmm", S390_FEAT_TYPE_MISC, 0, 
> "Collaborative-memory-management facility"),
> +FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
> +FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>  
>  FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit 
> in general registers)"),
>  FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 
> bit in parameter list)"),
> 

Reviewed-by: David Hildenbrand 

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v7 16/23] monitor: send event when command queue full

[Stefan Hajnoczi]

On Wed, Jan 24, 2018 at 01:39:50PM +0800, Peter Xu wrote:
> Set maximum QMP command queue length to 8.  If queue full, instead of
> queue the command, we directly return a "command-dropped" event, telling
> client that specific command is dropped.
> 
> Note that this flow control mechanism is only valid if OOB is enabled.
> If it's not, the effective queue length will always be 1, which strictly
> follows original behavior of QMP command handling (which never drop
> messages).
> 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 18 +-
>  1 file changed, 17 insertions(+), 1 deletion(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 15/23] qmp: add new event "command-dropped"

[Stefan Hajnoczi]

On Wed, Jan 24, 2018 at 01:39:49PM +0800, Peter Xu wrote:
> This event will be emitted if one QMP command is dropped.  Along,
> declare an enum for the reasons.
> 
> Reviewed-by: Fam Zheng 
> Signed-off-by: Peter Xu 
> ---
>  qapi-schema.json | 37 +
>  1 file changed, 37 insertions(+)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

Re: [Qemu-devel] [PATCH v7 14/23] monitor: separate QMP parser and dispatcher

[Stefan Hajnoczi]

On Wed, Jan 24, 2018 at 01:39:48PM +0800, Peter Xu wrote:
> Originally QMP goes through these steps:
> 
>   JSON Parser --> QMP Dispatcher --> Respond
>   /|\(2)(3) |
>(1) |   \|/ (4)
>+-  main thread  +
> 
> This patch does this:
> 
>   JSON Parser QMP Dispatcher --> Respond
>   /|\ |   /|\   (4) |
>|  | (2)| (3)|  (5)
>(1) |  +->  |   \|/
>+-  main thread  <---+
> 
> So the parsing job and the dispatching job is isolated now.  It gives us
> a chance in following up patches to totally move the parser outside.
> 
> The isolation is done using one QEMUBH. Only one dispatcher QEMUBH is
> used for all the monitors.
> 
> Signed-off-by: Peter Xu 
> ---
>  monitor.c | 201 
> +++---
>  1 file changed, 178 insertions(+), 23 deletions(-)

Reviewed-by: Stefan Hajnoczi 


signature.asc
Description: PGP signature

[Qemu-devel] [RFC PATCH 1/5] kbd-state: add keyboard state tracker

[Gerd Hoffmann]

Now that most user interfaces are using QKeyCodes it is easier to have
common keyboard code useable by all user interfaces.

This patch adds helper code to track the state of all keyboard keys,
using a bitmap indexed by QKeyCode.  Modifier state is tracked too,
as separate bitmap.  That makes checking modifier state easier.
Likewise we can easily apply special handling for capslock & numlock
(toggles on keypress) and ctrl + shift (we have two keys for that).

Signed-off-by: Gerd Hoffmann 
---
 include/ui/kbd-state.h |  22 +
 ui/kbd-state.c | 119 +
 ui/Makefile.objs   |   2 +-
 3 files changed, 142 insertions(+), 1 deletion(-)
 create mode 100644 include/ui/kbd-state.h
 create mode 100644 ui/kbd-state.c

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
new file mode 100644
index 00..c961da45b2
--- /dev/null
+++ b/include/ui/kbd-state.h
@@ -0,0 +1,22 @@
+typedef enum KbdModifier KbdModifier;
+
+enum KbdModifier {
+KBD_MOD_NONE = 0,
+
+KBD_MOD_SHIFT,
+KBD_MOD_CTRL,
+KBD_MOD_ALT,
+
+KBD_MOD_NUMLOCK,
+KBD_MOD_CAPSLOCK,
+
+KBD_MOD__MAX
+};
+
+typedef struct KbdState KbdState;
+
+bool kbd_state_modifier_get(KbdState *kbd, KbdModifier mod);
+bool kbd_state_key_get(KbdState *kbd, QKeyCode qcode);
+void kbd_state_key_event(KbdState *kbd, QKeyCode qcode, bool down);
+void kbd_state_lift_all_keys(KbdState *kbd);
+KbdState *kbd_state_init(QemuConsole *con);
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
new file mode 100644
index 00..7a9fe268c2
--- /dev/null
+++ b/ui/kbd-state.c
@@ -0,0 +1,119 @@
+#include "qemu/osdep.h"
+#include "qemu/bitmap.h"
+#include "qemu/queue.h"
+#include "qapi-types.h"
+#include "ui/console.h"
+#include "ui/input.h"
+#include "ui/kbd-state.h"
+
+typedef struct KbdHotkey KbdHotkey;
+
+struct KbdHotkey {
+uint32_t id;
+QKeyCode qcode;
+DECLARE_BITMAP(mods, KBD_MOD__MAX);
+QTAILQ_ENTRY(KbdHotkey) next;
+};
+
+struct KbdState {
+QemuConsole *con;
+DECLARE_BITMAP(keys, Q_KEY_CODE__MAX);
+DECLARE_BITMAP(mods, KBD_MOD__MAX);
+QTAILQ_HEAD(,KbdHotkey) hotkeys;
+};
+
+static void kbd_state_modifier_update(KbdState *kbd,
+  QKeyCode qcode1, QKeyCode qcode2,
+  KbdModifier mod)
+{
+if (test_bit(qcode1, kbd->keys) || test_bit(qcode2, kbd->keys)) {
+set_bit(mod, kbd->mods);
+} else {
+clear_bit(mod, kbd->mods);
+}
+}
+
+bool kbd_state_modifier_get(KbdState *kbd, KbdModifier mod)
+{
+return test_bit(mod, kbd->mods);
+}
+
+bool kbd_state_key_get(KbdState *kbd, QKeyCode qcode)
+{
+return test_bit(qcode, kbd->keys);
+}
+
+void kbd_state_key_event(KbdState *kbd, QKeyCode qcode, bool down)
+{
+bool state = test_bit(qcode, kbd->keys);
+
+if (state == down) {
+/*
+ * Filter out events which don't change the keyboard state.
+ *
+ * Most notably this allows to simply send along all key-up
+ * events, and this function will filter out everything where
+ * the corresponding key-down event wasn't send to the guest,
+ * for example due to being a host hotkey.
+ */
+return;
+}
+
+/* update key and modifier state */
+change_bit(qcode, kbd->keys);
+switch (qcode) {
+case Q_KEY_CODE_SHIFT:
+case Q_KEY_CODE_SHIFT_R:
+kbd_state_modifier_update(kbd, Q_KEY_CODE_SHIFT, Q_KEY_CODE_SHIFT_R,
+  KBD_MOD_SHIFT);
+break;
+case Q_KEY_CODE_CTRL:
+case Q_KEY_CODE_CTRL_R:
+kbd_state_modifier_update(kbd, Q_KEY_CODE_CTRL, Q_KEY_CODE_CTRL_R,
+  KBD_MOD_CTRL);
+break;
+case Q_KEY_CODE_ALT:
+kbd_state_modifier_update(kbd, Q_KEY_CODE_ALT, Q_KEY_CODE_ALT,
+  KBD_MOD_ALT);
+break;
+case Q_KEY_CODE_CAPS_LOCK:
+if (down) {
+change_bit(KBD_MOD_CAPSLOCK, kbd->mods);
+}
+break;
+case Q_KEY_CODE_NUM_LOCK:
+if (down) {
+change_bit(KBD_MOD_NUMLOCK, kbd->mods);
+}
+break;
+default:
+/* keep gcc happy */
+break;
+}
+
+/* send to guest */
+if (qemu_console_is_graphic(kbd->con)) {
+qemu_input_event_send_key_qcode(kbd->con, qcode, down);
+}
+}
+
+void kbd_state_lift_all_keys(KbdState *kbd)
+{
+int qcode;
+
+for (qcode = 0; qcode < Q_KEY_CODE__MAX; qcode++) {
+if (test_bit(qcode, kbd->keys)) {
+kbd_state_key_event(kbd, qcode, false);
+}
+}
+}
+
+KbdState *kbd_state_init(QemuConsole *con)
+{
+KbdState *kbd = g_new0(KbdState, 1);
+
+kbd->con = con;
+QTAILQ_INIT(>hotkeys);
+
+return kbd;
+}
diff --git a/ui/Makefile.objs b/ui/Makefile.objs
index ced7d91a63..aa81ce4c47 100644
--- a/ui/Makefile.objs
+++ b/ui/Makefile.objs
@@ -8,7 +8,7 @@ vnc-obj-y += 

[Qemu-devel] [RFC PATCH 2/5] kbd-state: add hotkey registry

[Gerd Hoffmann]

Add support to register hotkeys and to check whenever a given QKeyCode
combined with the current modifier state is a hotkey.  A hotkey can be
any key combined with up to three modifier keys.

Signed-off-by: Gerd Hoffmann 
---
 include/ui/kbd-state.h | 27 +++
 ui/kbd-state.c | 41 +
 2 files changed, 64 insertions(+), 4 deletions(-)

diff --git a/include/ui/kbd-state.h b/include/ui/kbd-state.h
index c961da45b2..3f13649b63 100644
--- a/include/ui/kbd-state.h
+++ b/include/ui/kbd-state.h
@@ -20,3 +20,30 @@ bool kbd_state_key_get(KbdState *kbd, QKeyCode qcode);
 void kbd_state_key_event(KbdState *kbd, QKeyCode qcode, bool down);
 void kbd_state_lift_all_keys(KbdState *kbd);
 KbdState *kbd_state_init(QemuConsole *con);
+
+/* -- */
+
+typedef enum KbdHotkey KbdHotkey;
+
+enum KbdHotkey {
+KBD_HOTKEY_NONE = 0,
+
+KBD_HOTKEY_GRAB,
+KBD_HOTKEY_FULLSCREEN,
+KBD_HOTKEY_REDRAW,
+
+KBD_HOTKEY_CONSOLE_1,
+KBD_HOTKEY_CONSOLE_2,
+KBD_HOTKEY_CONSOLE_3,
+KBD_HOTKEY_CONSOLE_4,
+KBD_HOTKEY_CONSOLE_5,
+KBD_HOTKEY_CONSOLE_6,
+KBD_HOTKEY_CONSOLE_7,
+KBD_HOTKEY_CONSOLE_8,
+KBD_HOTKEY_CONSOLE_9,
+};
+
+void kbd_state_hotkey_register(KbdState *kbd, KbdHotkey, QKeyCode qcode,
+   KbdModifier mod1, KbdModifier mod2,
+   KbdModifier mod3);
+KbdHotkey kbd_state_hotkey_get(KbdState *kbd, QKeyCode qcode);
diff --git a/ui/kbd-state.c b/ui/kbd-state.c
index 7a9fe268c2..812cb368e3 100644
--- a/ui/kbd-state.c
+++ b/ui/kbd-state.c
@@ -6,20 +6,20 @@
 #include "ui/input.h"
 #include "ui/kbd-state.h"
 
-typedef struct KbdHotkey KbdHotkey;
+typedef struct KbdHotkeyEntry KbdHotkeyEntry;
 
-struct KbdHotkey {
+struct KbdHotkeyEntry {
 uint32_t id;
 QKeyCode qcode;
 DECLARE_BITMAP(mods, KBD_MOD__MAX);
-QTAILQ_ENTRY(KbdHotkey) next;
+QTAILQ_ENTRY(KbdHotkeyEntry) next;
 };
 
 struct KbdState {
 QemuConsole *con;
 DECLARE_BITMAP(keys, Q_KEY_CODE__MAX);
 DECLARE_BITMAP(mods, KBD_MOD__MAX);
-QTAILQ_HEAD(,KbdHotkey) hotkeys;
+QTAILQ_HEAD(, KbdHotkeyEntry) hotkeys;
 };
 
 static void kbd_state_modifier_update(KbdState *kbd,
@@ -117,3 +117,36 @@ KbdState *kbd_state_init(QemuConsole *con)
 
 return kbd;
 }
+
+void kbd_state_hotkey_register(KbdState *kbd, KbdHotkey id, QKeyCode qcode,
+   KbdModifier mod1, KbdModifier mod2,
+   KbdModifier mod3)
+{
+KbdHotkeyEntry *hotkey = g_new0(KbdHotkeyEntry, 1);
+
+hotkey->id= id;
+hotkey->qcode = qcode;
+if (mod1 != KBD_MOD_NONE) {
+set_bit(mod1, hotkey->mods);
+}
+if (mod2 != KBD_MOD_NONE) {
+set_bit(mod2, hotkey->mods);
+}
+if (mod3 != KBD_MOD_NONE) {
+set_bit(mod3, hotkey->mods);
+}
+QTAILQ_INSERT_TAIL(>hotkeys, hotkey, next);
+}
+
+KbdHotkey kbd_state_hotkey_get(KbdState *kbd, QKeyCode qcode)
+{
+KbdHotkeyEntry *hotkey;
+
+QTAILQ_FOREACH(hotkey, >hotkeys, next) {
+if (qcode == hotkey->qcode &&
+bitmap_equal(kbd->mods, hotkey->mods, KBD_MOD__MAX)) {
+return hotkey->id;
+}
+}
+return KBD_HOTKEY_NONE;
+}
-- 
2.9.3

Re: [Qemu-devel] [PATCH v3] specs/qcow2: Fix documentation of the compressed cluster descriptor

[Kevin Wolf]

Am 21.02.2018 um 15:08 hat Alberto Garcia geschrieben:
> This patch fixes several mistakes in the documentation of the
> compressed cluster descriptor:
> 
> 1) the documentation claims that the cluster descriptor contains the
>number of sectors used to store the compressed data, but what it
>actually contains is the number of sectors *minus one* or, in other
>words, the number of additional sectors after the first one.
> 
> 2) the width of the fields is incorrectly specified. The number of bits
>used by each field is
> 
>   x = 62 - (cluster_bits - 8)   for the offset field
>   y = (cluster_bits - 8)for the size field
> 
>So the offset field's location is [0, x-1], not [0, x] as stated.
> 
> 3) the size field does not contain the size of the compressed data,
>but rather the number of sectors where that data is stored. The
>compressed data starts at the exact point specified in the offset
>field and ends when there's enough data to produce a cluster of
>decompressed data. Both points can be in the middle of a sector,
>allowing several compressed clusters to be stored next to one
>another, sharing sectors if necessary.
> 
> Signed-off-by: Alberto Garcia 

Thanks, applied to the block branch.

Kevin

[Qemu-devel] [RFC PATCH 5/5] sdl2: use only QKeyCode in sdl2_process_key()

[Gerd Hoffmann]

Small cleanup.  Also drop the special backspace handling,
kbd_put_qcode_console() learned to handle that meanwhile.
And sdl2_process_key is never called with scon == NULL.

Signed-off-by: Gerd Hoffmann 
---
 ui/sdl2-input.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 82ec375ea2..f0847dd745 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -39,23 +39,19 @@ void sdl2_process_key(struct sdl2_console *scon,
   SDL_KeyboardEvent *ev)
 {
 int qcode;
-QemuConsole *con = scon ? scon->dcl.con : NULL;
+QemuConsole *con = scon->dcl.con;
 
 if (ev->keysym.scancode >= qemu_input_map_usb_to_qcode_len) {
 return;
 }
-
 qcode = qemu_input_map_usb_to_qcode[ev->keysym.scancode];
 
 if (!qemu_console_is_graphic(con)) {
 if (ev->type == SDL_KEYDOWN) {
-switch (ev->keysym.scancode) {
-case SDL_SCANCODE_RETURN:
+switch (qcode) {
+case Q_KEY_CODE_RET:
 kbd_put_keysym_console(con, '\n');
 break;
-case SDL_SCANCODE_BACKSPACE:
-kbd_put_keysym_console(con, QEMU_KEY_BACKSPACE);
-break;
 default:
 kbd_put_qcode_console(con, qcode);
 break;
-- 
2.9.3

[Qemu-devel] [RFC PATCH 0/5] ui: add keyboard state and hotkey tracker

[Gerd Hoffmann]

Ok, here is my alternative approach to hotkey configuration.

Downside: It does *not* allow to freely choose any keys for hotkeys.
Hotkeys are restricted to be one normal key combined with up to three
modifiers (no modifiers is fine too).

Upside:  The code is simpler.  We don't have to keep back events because
they might be part of a hotkey.  The code also supports multiple
hotkeys.

While being at it have the code also track all key state, so user
interfaces don't have to do that on their own any more.

The sdl2 ui is used to show this in action.  No command line option
parsing added yet, so hotkeys are not actually configurable for now.
The groundwork to change them at runtime is there though.

Gerd Hoffmann (5):
  kbd-state: add keyboard state tracker
  kbd-state: add hotkey registry
  kbd-state: use state tracker for sdl2
  kbd-state: register sdl2 hotkeys
  sdl2: use only QKeyCode in sdl2_process_key()

 include/ui/kbd-state.h |  49 
 include/ui/sdl2.h  |   2 +
 ui/kbd-state.c | 152 +
 ui/sdl2-input.c|  54 ++
 ui/sdl2.c  | 114 -
 ui/Makefile.objs   |   2 +-
 6 files changed, 270 insertions(+), 103 deletions(-)
 create mode 100644 include/ui/kbd-state.h
 create mode 100644 ui/kbd-state.c

-- 
2.9.3

[Qemu-devel] [RFC PATCH 3/5] kbd-state: use state tracker for sdl2

[Gerd Hoffmann]

Use the new keyboard state tracked for sdl2.  We can drop the modifier
state tracking from sdl2.  Also keyup code is simpler, the state tracker
will take care to not send suspious keyup events to the guest.

Signed-off-by: Gerd Hoffmann 
---
 include/ui/sdl2.h |  2 ++
 ui/sdl2-input.c   | 43 ++-
 ui/sdl2.c | 18 +++---
 3 files changed, 7 insertions(+), 56 deletions(-)

diff --git a/include/ui/sdl2.h b/include/ui/sdl2.h
index 51084e6320..c316e42706 100644
--- a/include/ui/sdl2.h
+++ b/include/ui/sdl2.h
@@ -7,6 +7,7 @@
 #include 
 #include 
 
+#include "ui/kbd-state.h"
 #ifdef CONFIG_OPENGL
 # include "ui/egl-helpers.h"
 #endif
@@ -26,6 +27,7 @@ struct sdl2_console {
 int idle_counter;
 int ignore_hotkeys;
 SDL_GLContext winctx;
+KbdState *kbd;
 #ifdef CONFIG_OPENGL
 QemuGLShader *gls;
 egl_fb guest_fb;
diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index 605d781971..d970655616 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -30,22 +30,9 @@
 #include "ui/sdl2.h"
 #include "sysemu/sysemu.h"
 
-static uint8_t modifiers_state[SDL_NUM_SCANCODES];
-
 void sdl2_reset_keys(struct sdl2_console *scon)
 {
-QemuConsole *con = scon ? scon->dcl.con : NULL;
-int i;
-
-for (i = 0 ;
- i < SDL_NUM_SCANCODES && i < qemu_input_map_usb_to_qcode_len ;
- i++) {
-if (modifiers_state[i]) {
-int qcode = qemu_input_map_usb_to_qcode[i];
-qemu_input_event_send_key_qcode(con, qcode, false);
-modifiers_state[i] = 0;
-}
-}
+kbd_state_lift_all_keys(scon->kbd);
 }
 
 void sdl2_process_key(struct sdl2_console *scon,
@@ -77,31 +64,5 @@ void sdl2_process_key(struct sdl2_console *scon,
 return;
 }
 
-switch (ev->keysym.scancode) {
-#if 0
-case SDL_SCANCODE_NUMLOCKCLEAR:
-case SDL_SCANCODE_CAPSLOCK:
-/* SDL does not send the key up event, so we generate it */
-qemu_input_event_send_key_qcode(con, qcode, true);
-qemu_input_event_send_key_qcode(con, qcode, false);
-return;
-#endif
-case SDL_SCANCODE_LCTRL:
-case SDL_SCANCODE_LSHIFT:
-case SDL_SCANCODE_LALT:
-case SDL_SCANCODE_LGUI:
-case SDL_SCANCODE_RCTRL:
-case SDL_SCANCODE_RSHIFT:
-case SDL_SCANCODE_RALT:
-case SDL_SCANCODE_RGUI:
-if (ev->type == SDL_KEYUP) {
-modifiers_state[ev->keysym.scancode] = 0;
-} else {
-modifiers_state[ev->keysym.scancode] = 1;
-}
-/* fall though */
-default:
-qemu_input_event_send_key_qcode(con, qcode,
-ev->type == SDL_KEYDOWN);
-}
+kbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
 }
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 6e96a4a24c..50f5fca0f6 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -40,7 +40,6 @@ static int gui_grab; /* if true, all keyboard/mouse events 
are grabbed */
 static int gui_saved_grab;
 static int gui_fullscreen;
 static int gui_key_modifier_pressed;
-static int gui_keysym;
 static int gui_grab_code = KMOD_LALT | KMOD_LCTRL;
 static SDL_Cursor *sdl_cursor_normal;
 static SDL_Cursor *sdl_cursor_hidden;
@@ -331,6 +330,7 @@ static void handle_keydown(SDL_Event *ev)
 {
 int win;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
+int gui_keysym = 0;
 
 gui_key_modifier_pressed = get_mod_state();
 
@@ -413,23 +413,10 @@ static void handle_keydown(SDL_Event *ev)
 
 static void handle_keyup(SDL_Event *ev)
 {
-int mod_state;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
 
 scon->ignore_hotkeys = false;
-
-if (!alt_grab) {
-mod_state = (ev->key.keysym.mod & gui_grab_code);
-} else {
-mod_state = (ev->key.keysym.mod & (gui_grab_code | KMOD_LSHIFT));
-}
-if (!mod_state && gui_key_modifier_pressed) {
-gui_key_modifier_pressed = 0;
-gui_keysym = 0;
-}
-if (!gui_keysym) {
-sdl2_process_key(scon, >key);
-}
+sdl2_process_key(scon, >key);
 }
 
 static void handle_textinput(SDL_Event *ev)
@@ -829,6 +816,7 @@ void sdl_display_init(DisplayState *ds, DisplayOptions *o)
 sdl2_console[i].dcl.ops = _2d_ops;
 #endif
 sdl2_console[i].dcl.con = con;
+sdl2_console[i].kbd = kbd_state_init(con);
 register_displaychangelistener(_console[i].dcl);
 
 #if defined(SDL_VIDEO_DRIVER_WINDOWS) || defined(SDL_VIDEO_DRIVER_X11)
-- 
2.9.3

[Qemu-devel] [RFC PATCH 4/5] kbd-state: register sdl2 hotkeys

[Gerd Hoffmann]

Register SDL2 hotkeys in keyboard state tracker, use it to check for
hotkeys.  Drop even more modifier code.

This also changes behavior a bit.  The Ctrl-Alt- hotkey
combinations accept both ctrl keys now.  Likewise the
Ctrl-Alt-Shift- mode you can enable with -alt-grab accepts both
ctrl and shift keys.  The -ctrl-grab mode (use right ctrl key) is not
supported.

Signed-off-by: Gerd Hoffmann 
---
 ui/sdl2-input.c |  1 -
 ui/sdl2.c   | 98 ++---
 2 files changed, 59 insertions(+), 40 deletions(-)

diff --git a/ui/sdl2-input.c b/ui/sdl2-input.c
index d970655616..82ec375ea2 100644
--- a/ui/sdl2-input.c
+++ b/ui/sdl2-input.c
@@ -61,7 +61,6 @@ void sdl2_process_key(struct sdl2_console *scon,
 break;
 }
 }
-return;
 }
 
 kbd_state_key_event(scon->kbd, qcode, ev->type == SDL_KEYDOWN);
diff --git a/ui/sdl2.c b/ui/sdl2.c
index 50f5fca0f6..0f48f5a546 100644
--- a/ui/sdl2.c
+++ b/ui/sdl2.c
@@ -39,8 +39,6 @@ static int gui_grab; /* if true, all keyboard/mouse events 
are grabbed */
 
 static int gui_saved_grab;
 static int gui_fullscreen;
-static int gui_key_modifier_pressed;
-static int gui_grab_code = KMOD_LALT | KMOD_LCTRL;
 static SDL_Cursor *sdl_cursor_normal;
 static SDL_Cursor *sdl_cursor_hidden;
 static int absolute_enabled;
@@ -312,43 +310,35 @@ static void toggle_full_screen(struct sdl2_console *scon)
 sdl2_redraw(scon);
 }
 
-static int get_mod_state(void)
-{
-SDL_Keymod mod = SDL_GetModState();
-
-if (alt_grab) {
-return (mod & (gui_grab_code | KMOD_LSHIFT)) ==
-(gui_grab_code | KMOD_LSHIFT);
-} else if (ctrl_grab) {
-return (mod & KMOD_RCTRL) == KMOD_RCTRL;
-} else {
-return (mod & gui_grab_code) == gui_grab_code;
-}
-}
-
 static void handle_keydown(SDL_Event *ev)
 {
 int win;
 struct sdl2_console *scon = get_scon_from_window(ev->key.windowID);
-int gui_keysym = 0;
+KbdHotkey hotkey;
+QKeyCode qcode;
 
-gui_key_modifier_pressed = get_mod_state();
+if (ev->key.keysym.scancode >= qemu_input_map_usb_to_qcode_len) {
+return;
+}
+qcode = qemu_input_map_usb_to_qcode[ev->key.keysym.scancode];
 
-if (!scon->ignore_hotkeys && gui_key_modifier_pressed && !ev->key.repeat) {
-switch (ev->key.keysym.scancode) {
-case SDL_SCANCODE_2:
-case SDL_SCANCODE_3:
-case SDL_SCANCODE_4:
-case SDL_SCANCODE_5:
-case SDL_SCANCODE_6:
-case SDL_SCANCODE_7:
-case SDL_SCANCODE_8:
-case SDL_SCANCODE_9:
+if (!scon->ignore_hotkeys && !ev->key.repeat) {
+hotkey = kbd_state_hotkey_get(scon->kbd, qcode);
+switch (hotkey) {
+case KBD_HOTKEY_CONSOLE_2:
+case KBD_HOTKEY_CONSOLE_3:
+case KBD_HOTKEY_CONSOLE_4:
+case KBD_HOTKEY_CONSOLE_5:
+case KBD_HOTKEY_CONSOLE_6:
+case KBD_HOTKEY_CONSOLE_7:
+case KBD_HOTKEY_CONSOLE_8:
+case KBD_HOTKEY_CONSOLE_9:
 if (gui_grab) {
 sdl_grab_end(scon);
 }
+sdl2_reset_keys(scon);
 
-win = ev->key.keysym.scancode - SDL_SCANCODE_1;
+win = hotkey - KBD_HOTKEY_CONSOLE_1;
 if (win < sdl2_num_outputs) {
 sdl2_console[win].hidden = !sdl2_console[win].hidden;
 if (sdl2_console[win].real_window) {
@@ -358,29 +348,25 @@ static void handle_keydown(SDL_Event *ev)
 SDL_ShowWindow(sdl2_console[win].real_window);
 }
 }
-gui_keysym = 1;
 }
 break;
-case SDL_SCANCODE_F:
+case KBD_HOTKEY_FULLSCREEN:
 toggle_full_screen(scon);
-gui_keysym = 1;
 break;
-case SDL_SCANCODE_G:
-gui_keysym = 1;
+case KBD_HOTKEY_GRAB:
 if (!gui_grab) {
 sdl_grab_start(scon);
 } else if (!gui_fullscreen) {
 sdl_grab_end(scon);
 }
 break;
-case SDL_SCANCODE_U:
+case KBD_HOTKEY_REDRAW:
 sdl2_window_destroy(scon);
 sdl2_window_create(scon);
 if (!scon->opengl) {
 /* re-create scon->texture */
 sdl2_2d_switch(>dcl, scon->surface);
 }
-gui_keysym = 1;
 break;
 #if 0
 case SDL_SCANCODE_KP_PLUS:
@@ -402,11 +388,14 @@ static void handle_keydown(SDL_Event *ev)
 gui_keysym = 1;
 }
 #endif
+case KBD_HOTKEY_NONE:
+sdl2_process_key(scon, >key);
+break;
 default:
+/* keep gcc happy */
 break;
 }
-}
-if (!gui_keysym) {
+} else {
 sdl2_process_key(scon, >key);
 }
 }
@@ -546,7 +535,7 @@ static void handle_windowevent(SDL_Event *ev)
  * Work around this by 

Re: [Qemu-devel] [PATCH v1 1/3] s390x/sclp: proper support of larger send and receive masks

[Cornelia Huck]

On Wed, 21 Feb 2018 17:28:49 +0100
Claudio Imbrenda  wrote:

> On Wed, 21 Feb 2018 16:12:59 +0100
> Cornelia Huck  wrote:
> 
> > On Tue, 20 Feb 2018 19:45:00 +0100
> > Claudio Imbrenda  wrote:
> >   
> > > The architecture allows the guests to ask for masks up to 1021
> > > bytes in length. Part was fixed in
> > > 67915de9f0383ccf4ab8c42dd02aa18dcd79b411 ("s390x/event-facility:
> > > variable-length event masks"), but some issues were still
> > > remaining, in particular regarding the handling of selective reads.
> > > 
> > > This patch fixes the handling of selective reads, whose size will
> > > now match the length of the event mask, as per architecture.
> > > 
> > > The default behaviour is to be compliant with the architecture, but
> > > when using older machine models the old behaviour is selected, in
> > > order to be able to migrate toward older versions.
> > 
> > I remember trying to do this back when I still had access to the
> > architecture, but somehow never finished this (don't remember why).
> >   
> > > 
> > > Fixes: 67915de9f0383ccf4a ("s390x/event-facility: variable-length
> > > event masks")
> > 
> > Doesn't that rather fix the initial implementation? What am I missing?  
> 
> well that too. but I think this fixes the fix, since the fix was
> incomplete?
> 
> > > Signed-off-by: Claudio Imbrenda 
> > > ---
> > >  hw/s390x/event-facility.c  | 90
> > > +++---
> > > hw/s390x/s390-virtio-ccw.c |  8 - 2 files changed, 84
> > > insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/hw/s390x/event-facility.c b/hw/s390x/event-facility.c
> > > index 155a694..2414614 100644
> > > --- a/hw/s390x/event-facility.c
> > > +++ b/hw/s390x/event-facility.c
> > > @@ -31,6 +31,14 @@ struct SCLPEventFacility {
> > >  SCLPEventsBus sbus;
> > >  /* guest' receive mask */
> > >  unsigned int receive_mask;
> > > +/*
> > > + * when false, we keep the same broken, backwards compatible
> > > behaviour as
> > > + * before; when true, we implement the architecture correctly.
> > > Needed for
> > > + * migration toward older versions.
> > > + */
> > > +bool allow_all_mask_sizes;
> > 
> > The comment does not really tell us what the old behaviour is ;)  
> 
> hmm, I'll fix that
> 
> > So, if this is about extending the mask size, call this
> > "allow_large_masks" or so?  
> 
> no, the old broken behaviour allowed only _exactly_ 4 bytes:
> 
> if (be16_to_cpu(we_mask->mask_length) != 4) {
>  sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_MASK_LENGTH);
>  goto out;
> }

Hm, I can't seem to find this in the code?

> 
> With the 67915de9f0383ccf4a patch mentioned above, we allow any size,
> but we don't keep track of the size itself, only the bits. This is a
> problem for selective reads (see below)

Oh, you meant before that patch...

> 
> > > +/* length of the receive mask */
> > > +uint16_t mask_length;
> > >  };
> > >  
> > >  /* return true if any child has event pending set */
> > > @@ -220,6 +228,17 @@ static uint16_t
> > > handle_sccb_read_events(SCLPEventFacility *ef, SCCB *sccb, return
> > > rc; }
> > >  
> > > +/* copy up to dst_len bytes and fill the rest of dst with zeroes */
> > > +static void copy_mask(uint8_t *dst, uint8_t *src, uint16_t dst_len,
> > > +  uint16_t src_len)
> > > +{
> > > +int i;
> > > +
> > > +for (i = 0; i < dst_len; i++) {
> > > +dst[i] = i < src_len ? src[i] : 0;
> > > +}
> > > +}
> > > +
> > >  static void read_event_data(SCLPEventFacility *ef, SCCB *sccb)
> > >  {
> > >  unsigned int sclp_active_selection_mask;
> > > @@ -240,7 +259,9 @@ static void read_event_data(SCLPEventFacility
> > > *ef, SCCB *sccb) sclp_active_selection_mask = sclp_cp_receive_mask;
> > >  break;
> > >  case SCLP_SELECTIVE_READ:
> > > -sclp_active_selection_mask = be32_to_cpu(red->mask);
> > > +copy_mask((uint8_t *)_active_selection_mask, (uint8_t
> > > *)>mask,
> > > +  sizeof(sclp_active_selection_mask),
> > > ef->mask_length);
> > > +sclp_active_selection_mask =
> > > be32_to_cpu(sclp_active_selection_mask);
> > 
> > Hm, this looks like a real bug fix for the commit referenced above.
> > Split this out? We don't need compat support for that; maybe even
> > cc:stable?  
> 
> I think we do. We can avoid keeping track of the mask size when setting
> the mask size, because we can simply take the bits we need and ignore
> the rest. But for selective reads we need the mask size, so we have to
> keep track of it. (selective reads specify a mask, but not a mask
> length, the mask length is the length of the last mask set)

OK, that's non-obvious without documentation :/

> 
> And now we have additional state that we need to copy around when
> migrating. And we don't want to break older machines! Moreover a

Re: [Qemu-devel] [PATCH 0/2] Firmware blob and git submodule for Sam460ex

[BALATON Zoltan]

On Wed, 21 Feb 2018, Peter Maydell wrote:

On 20 February 2018 at 20:44, Emilio G. Cota  wrote:

On Tue, Feb 20, 2018 at 18:31:17 +, Peter Maydell wrote:

On 20 February 2018 at 18:10, BALATON Zoltan  wrote:

I've created a git repo for the Sam460ex u-boot sources and this adds
that as a submodule and a separate patch to add the binary built from
these sources. Feel free to keep this as two patches, squash them into
one patch or take the git repo and commit the content under the QEMU
repo and use that as a submodule as you see fit (or let me know if any
changes are needed for these patches).

BALATON Zoltan (2):
  roms: Added git submodule for u-boot-sam460 (firmware for sam460ex)
  pc-bios: Added u-boot-sam460 firmware binary


We already have a submodule for u-boot. Is it not possible to
build this bios blob from those upstream u-boot sources?


This is discussed in the following thread:
  Re: [Qemu-ppc] [PATCH v3 2/2] ppc: Add aCube Sam460ex board
  http://lists.gnu.org/archive/html/qemu-ppc/2018-02/msg00268.html


If upstream u-boot have abandoned the board support I'm not very
enthusiastic about our taking it on :-(


It's not that upstream u-boot has abandoned board support (it only removed 
support for the PPC440 CPU it once had). The board itself never had 
support in upstream u-boot, it only exists in vendor's fork which is the 
reason we need a separate source and cannot use upstream u-boot source we 
already have.


In my opinion we don't aim to take on support for this board in u-boot, we 
only need to include the firmware binary for the emulation to be useful 
which then requires us to also include the source for the GPL it's 
licensed under. I've also found a few bugs in the firmware which I've 
fixed but apart from such occasional bug fixes when needed I don't expect 
to take over support for the board from the hardware vendor so this source 
is only so we can include the firmware binary which is needed for the 
board emulation. Does this answer your concerns?


Regards,
BALATON Zoltan

Re: [Qemu-devel] [PATCH 2/2] qcow2: Avoid memory over-allocation on compressed images

[Eric Blake]

On 02/21/2018 10:51 AM, Kevin Wolf wrote:

Am 20.02.2018 um 23:24 hat Eric Blake geschrieben:

When reading a compressed image, we were allocating s->cluster_data
to 32*cluster_size + 512 (possibly over 64 megabytes, for an image
with 2M clusters).  Let's check out the history:




Much later, in commit de82815d (v2.2), we noticed that a 64M
allocation is prone to failure, so we switched over to a graceful
memory allocation error message.  But note that elsewhere in the
code, we do g_malloc(2 * cluster_size) without ever checking for
failure.




-}
-if (!s->cluster_cache) {
-s->cluster_cache = g_malloc(s->cluster_size);
+assert(!s->cluster_cache);


Wouldn't it be better to assert (!!s->cluster_cache ==
!!s->cluster_data) unconditionally?



Sure.


+s->cluster_data = g_try_malloc(s->cluster_size);


Why are you going from qemu_try_blockalign() to simple malloc here? This
buffer is used with bdrv_read() (or bdrv_pread() after patch 1), so we
should avoid unnecessary use of a bounce buffer.


But does bdrv_pread() actually need to use a bounce buffer if we don't 
have an aligned buffer to read into?  Either the underlying protocol 
already supports byte-aligned reads (direct into our buffer, regardless 
of alignment, no bouncing required), or it already has do to a full 
sector read into a bounce buffer anyways (and it doesn't matter whether 
we aligned our buffer).  blockalign() made sense when we had multiple 
clients for the buffer, but ever since v1.1, when we have only a single 
client, and that single client is most likely not going to read 
sector-aligned data in the first place, aligning the buffer doesn't buy 
us anything.





+s->cluster_cache = g_try_malloc(s->cluster_size);


As you already said, either g_malloc() or check the result. I actually
think that g_try_malloc() and checking the result is nicer, we still
allocate up to 2 MB here.


See my commit message comment - we have other spots in the code base 
that blindly g_malloc(2 * s->cluster_size).  And I intended (but sent 
the email without amending my commit) to use g_malloc().  But as Berto 
has convinced me that an externally produced image can convince us to 
read up to 4M (even though we don't need that much to decompress), I 
suppose that the try_ variant plus checking is reasonable (and care in 
NULL'ing out if one but not both allocations succeed).


--
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org