Re: [Qemu-devel] [PATCH v2] qemu-doc.texi: Add usb sound card info

2015-08-08 Thread Markus Armbruster
Copying USB maintainer...

Programmingkid programmingk...@gmail.com writes:

 Add info on usb sound card to qemu documentation.

 Signed-off-by: John Arbuckle programmingk...@gmail.com

 ---
 Changed 'thru' to 'through'

  qemu-doc.texi |5 -
  1 files changed, 4 insertions(+), 1 deletions(-)

 diff --git a/qemu-doc.texi b/qemu-doc.texi
 index 0125bc7..9a8f353 100644
 --- a/qemu-doc.texi
 +++ b/qemu-doc.texi
 @@ -1328,7 +1328,8 @@ as necessary to connect multiple USB devices.
  @subsection Connecting USB devices
  
  USB devices can be connected with the @option{-usbdevice} commandline option
 -or the @code{usb_add} monitor command.  Available devices are:
 +or the @code{usb_add} monitor command. Note: some devices may only work if
 +added like this: -usb -device usb device. Available devices are:

I'm afraid may only work is a bit misleading.  All of them work with
-device.  Old ones are also supported by -usbdevice for backward
compatibility.  The whole section should be rewritten to point to
-device instead of legacy -usbdevice, but that's no reason to hold up
your patch.

I'd just drop this hunk for now.  Suboptimal, but I can't think of a
better way short of doing the rewrite.  Perhaps Gerd can.

 +or the @code{usb_add} monitor command. Note: some devices may only work if
 +added like this: -usb -device usb device. Available devices are:

  @table @code
  @item mouse
 @@ -1381,6 +1382,8 @@ usage:
  @example
  qemu-system-i386 [...OPTIONS...] -usbdevice bt:hci,vlan=3 -bt 
 device:keyboard,vlan=3
  @end example
 +@item usb-audio
 +USB sound card. Can only be used if added through the command-line like 
 this: -usb -device usb-audio
  @end table
  
  @node host_usb_devices

Long line.  Could perhaps be fixed on commit.

With the first hunk dropped and the line wrapped:

Reviewed-by: Markus Armbruster arm...@redhat.com



Re: [Qemu-devel] [PATCH RFC v3 11/32] qapi-visit: Convert to QAPISchemaVisitor, fixing bugs

2015-08-08 Thread Markus Armbruster
Eric Blake ebl...@redhat.com writes:

 On 08/04/2015 09:57 AM, Markus Armbruster wrote:
 Fixes flat unions to visit the base's base members (the previous
 commit merely added them to the struct).  Same test case.
 
 Patch's effect on visit_type_UserDefFlatUnion():
 
  static void visit_type_UserDefFlatUnion_fields(Visitor *m, 
 UserDefFlatUnion **obj, Error **errp)
  {
  Error *err = NULL;
 
 +visit_type_int(m, (*obj)-integer, integer, err);
 +if (err) {
 +goto out;
 +}
  visit_type_str(m, (*obj)-string, string, err);
  if (err) {
  goto out;
 
 Test cases updated for the bug fix.

 Not quite right.

 
 Fixes alternates to generate a visitor for their implicit enumeration
 type.  None of them are currently used, obviously.  Example:
 block-core.json's BlockdevRef now generates
 visit_type_BlockdevRefKind().
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 ---
  scripts/qapi-visit.py   | 260 
 +---
  tests/qapi-schema/qapi-schema-test.json |   3 -
  tests/test-qmp-input-strict.c   |   2 +-
  tests/test-qmp-input-visitor.c  |   4 +-
  4 files changed, 106 insertions(+), 163 deletions(-)

 +++ b/tests/test-qmp-input-strict.c
 @@ -167,9 +167,9 @@ static void 
 test_validate_union_flat(TestInputVisitorData *data,
  
  v = validate_test_init(data,
 { 'enum1': 'value1', 
 +   'integer': 41, 
 'string': 'str', 
 'boolean': true });
 -/* TODO when generator bug is fixed, add 'integer': 41 */
  
  visit_type_UserDefFlatUnion(v, tmp, NULL, err);
  g_assert(!err);

 Incomplete fix; you need to squash this in to test that cleanup of a
 partial struct is still correct (otherwise, the test sets err for the
 wrong reason of missing 'integer' rather than the intended reason of
 missing 'enum1'):

 diff --git i/tests/test-qmp-input-strict.c w/tests/test-qmp-input-strict.c
 index bfd9d04..4c18096 100644
 --- i/tests/test-qmp-input-strict.c
 +++ w/tests/test-qmp-input-strict.c
 @@ -299,7 +299,7 @@ static void
 test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
  Visitor *v;

  /* test situation where discriminator field ('enum1' here) is
 missing */
 -v = validate_test_init(data, { 'string': 'c', 'string1': 'd',
 'string2': 'e' });
 +v = validate_test_init(data, { 'integer': 42, 'string': 'c',
 'string1': 'd', 'string2': 'e' });

  visit_type_UserDefFlatUnion2(v, tmp, NULL, err);
  g_assert(err);

You're right.  I'll squash it in, thanks!



Re: [Qemu-devel] [PATCH RFC v3 26/32] qapi: Introduce a first class 'any' type

2015-08-08 Thread Markus Armbruster
Eric Blake ebl...@redhat.com writes:

 On 08/04/2015 09:58 AM, Markus Armbruster wrote:
 It's first class, because unlike '**', it actually works, i.e. doesn't
 require 'gen': false.
 
 '**' will go away next.
 
 Signed-off-by: Markus Armbruster arm...@redhat.com
 Reviewed-by: Eric Blake ebl...@redhat.com
 ---

 @@ -1039,8 +1040,7 @@ class QAPISchema(object):
  
  def _def_builtin_type(self, name, json_type, c_type, c_null):
  self._def_entity(QAPISchemaBuiltinType(name, json_type, c_type, 
 c_null))
 -if name != '**':
 -self._make_array_type(name) # TODO really needed?
 +self._make_array_type(name) # TODO really needed?

 Do we really want to allow ['any'] in schemata?  That would imply the
 possibility of a 2D array.

It's 2D-by-trickery.  Unlike the existing trick of wrapping the inner
array in a struct, this one isn't visible on the wire, though.  It
requires giving up some type checking.

From a backend point of view, array of any type should just work.  We're
already debating whether we want them under PATCH 02, so let's continue
there.

 +++ b/tests/qapi-schema/qapi-schema-test.json
 @@ -83,6 +83,8 @@
'returns': 'UserDefTwo' }
  { 'command': 'user_def_cmd3', 'data': {'a': 'int', '*b': 'int' },
'returns': 'int' }
 +# note: command name 'guest-sync' chosen to avoid cannot use built-in 
 error
 +{ 'command': 'guest-sync', 'data': { 'arg': 'any' }, 'returns': 'any' }
  

 In particular, if we DO want to allow it, this file should be enhanced
 to include ['any'] in the UserDefNativeListUnion.

Yes, to keep the test case complete.

 As it is, JSON allows mixed-type arrays, but all our uses of QList are
 fixed-type (all elements share the same type); allowing an array of any
 element may prove to be problematic.

anyList is fixed-type, too: the fixed type is QObject * ;)

['any'] isn't ABI until we actually use it in an external interface.



Re: [Qemu-devel] [RFC v4 5/9] configure: Enable/disable new qemu_{ld, st} excl insns

2015-08-08 Thread Aurelien Jarno
On 2015-08-07 19:03, Alvise Rigo wrote:
 Introduce the new --enable-tcg-ldst-excl configure option to enable the
 LL/SC operations only for those backends that support them.
 
 Suggested-by: Jani Kokkonen jani.kokko...@huawei.com
 Suggested-by: Claudio Fontana claudio.font...@huawei.com
 Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
 ---
  configure | 21 +
  1 file changed, 21 insertions(+)

We have seen that for this kind of patch, it's better to add support in
all backends, otherwise it takes ages to get all the backends converted.
I think you should involve the backend maintainers. I can try to provide
the corresponding patches for mips and ia64.

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net



Re: [Qemu-devel] [PATCH v2] qemu-doc.texi: Add usb sound card info

2015-08-08 Thread Programmingkid

On Aug 8, 2015, at 2:04 AM, Markus Armbruster wrote:

 Copying USB maintainer...
 
 Programmingkid programmingk...@gmail.com writes:
 
 Add info on usb sound card to qemu documentation.
 
 Signed-off-by: John Arbuckle programmingk...@gmail.com
 
 ---
 Changed 'thru' to 'through'
 
 qemu-doc.texi |5 -
 1 files changed, 4 insertions(+), 1 deletions(-)
 
 diff --git a/qemu-doc.texi b/qemu-doc.texi
 index 0125bc7..9a8f353 100644
 --- a/qemu-doc.texi
 +++ b/qemu-doc.texi
 @@ -1328,7 +1328,8 @@ as necessary to connect multiple USB devices.
 @subsection Connecting USB devices
 
 USB devices can be connected with the @option{-usbdevice} commandline option
 -or the @code{usb_add} monitor command.  Available devices are:
 +or the @code{usb_add} monitor command. Note: some devices may only work if
 +added like this: -usb -device usb device. Available devices are:
 
 I'm afraid may only work is a bit misleading.  All of them work with
 -device.  Old ones are also supported by -usbdevice for backward
 compatibility.  The whole section should be rewritten to point to
 -device instead of legacy -usbdevice, but that's no reason to hold up
 your patch.

I did not know -usbdevice was considered legacy. If that is the case, then it 
should probably
be removed from the documentation in favor for -usb -device device name. 

A patch should be made that would allow all USB devices to be added using the 
usb_add
monitor command. 

 I'd just drop this hunk for now.  Suboptimal, but I can't think of a
 better way short of doing the rewrite.  Perhaps Gerd can.
 
 +or the @code{usb_add} monitor command. Note: some devices may only work if
 +added like this: -usb -device usb device. Available devices are:
 
 @table @code
 @item mouse
 @@ -1381,6 +1382,8 @@ usage:
 @example
 qemu-system-i386 [...OPTIONS...] -usbdevice bt:hci,vlan=3 -bt 
 device:keyboard,vlan=3
 @end example
 +@item usb-audio
 +USB sound card. Can only be used if added through the command-line like 
 this: -usb -device usb-audio
 @end table
 
 @node host_usb_devices
 
 Long line.  Could perhaps be fixed on commit.
 
 With the first hunk dropped and the line wrapped:
 
 Reviewed-by: Markus Armbruster arm...@redhat.com




Re: [Qemu-devel] [RFC v4 6/9] tcg-i386: Implement excl variants of qemu_{ld, st}

2015-08-08 Thread Aurelien Jarno
On 2015-08-07 19:03, Alvise Rigo wrote:
 Implement exclusive variants of qemu_{ld,st}_{i32,i64} for tcg-i386.
 The lookup for the proper memory helper has been rewritten to take
 into account the new exclusive helpers.
 
 Suggested-by: Jani Kokkonen jani.kokko...@huawei.com
 Suggested-by: Claudio Fontana claudio.font...@huawei.com
 Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
 ---
  tcg/i386/tcg-target.c | 148 
 +++---
  1 file changed, 129 insertions(+), 19 deletions(-)
 
 diff --git a/tcg/i386/tcg-target.c b/tcg/i386/tcg-target.c
 index ff4d9cf..011907c 100644
 --- a/tcg/i386/tcg-target.c
 +++ b/tcg/i386/tcg-target.c
 @@ -1137,6 +1137,28 @@ static void * const qemu_ld_helpers[16] = {
  [MO_BEQ]  = helper_be_ldq_mmu,
  };
  
 +/* LoadLink helpers, only unsigned. Use the macro below to access them. */
 +static void * const qemu_ldex_helpers[16] = {
 +[MO_UB]   = helper_ret_ldlinkub_mmu,
 +
 +[MO_LEUW] = helper_le_ldlinkuw_mmu,
 +[MO_LEUL] = helper_le_ldlinkul_mmu,
 +[MO_LEQ]  = helper_le_ldlinkq_mmu,
 +
 +[MO_BEUW] = helper_be_ldlinkuw_mmu,
 +[MO_BEUL] = helper_be_ldlinkul_mmu,
 +[MO_BEQ]  = helper_be_ldlinkq_mmu,
 +};
 +
 +static inline tcg_insn_unit *ld_helper(TCGMemOp opc)
 +{
 +if (opc  MO_EXCL) {
 +return qemu_ldex_helpers[((int)opc - MO_EXCL)  (MO_BSWAP | 
 MO_SSIZE)];
 +}
 +
 +return qemu_ld_helpers[opc  ~MO_SIGN];
 +}
 +
  /* helper signature: helper_ret_st_mmu(CPUState *env, target_ulong addr,
   * uintxx_t val, int mmu_idx, uintptr_t 
 ra)
   */
 @@ -1150,6 +1172,26 @@ static void * const qemu_st_helpers[16] = {
  [MO_BEQ]  = helper_be_stq_mmu,
  };
  
 +/* StoreConditional helpers. Use the macro below to access them. */
 +static void * const qemu_stex_helpers[16] = {
 +[MO_UB]   = helper_ret_stcondb_mmu,
 +[MO_LEUW] = helper_le_stcondw_mmu,
 +[MO_LEUL] = helper_le_stcondl_mmu,
 +[MO_LEQ]  = helper_le_stcondq_mmu,
 +[MO_BEUW] = helper_be_stcondw_mmu,
 +[MO_BEUL] = helper_be_stcondl_mmu,
 +[MO_BEQ]  = helper_be_stcondq_mmu,
 +};
 +
 +static inline tcg_insn_unit *st_helper(TCGMemOp opc)
 +{
 +if (opc  MO_EXCL) {
 +return qemu_stex_helpers[((int)opc - MO_EXCL)  (MO_BSWAP | 
 MO_SSIZE)];
 +}
 +
 +return qemu_st_helpers[opc];
 +}
 +
  /* Perform the TLB load and compare.
  
 Inputs:
 @@ -1245,6 +1287,7 @@ static inline void tcg_out_tlb_load(TCGContext *s, 
 TCGReg addrlo, TCGReg addrhi,
   * for a load or store, so that we can later generate the correct helper code
   */
  static void add_qemu_ldst_label(TCGContext *s, bool is_ld, TCGMemOpIdx oi,
 +TCGReg llsc_success,
  TCGReg datalo, TCGReg datahi,
  TCGReg addrlo, TCGReg addrhi,
  tcg_insn_unit *raddr,
 @@ -1253,6 +1296,7 @@ static void add_qemu_ldst_label(TCGContext *s, bool 
 is_ld, TCGMemOpIdx oi,
  TCGLabelQemuLdst *label = new_ldst_label(s);
  
  label-is_ld = is_ld;
 +label-llsc_success = llsc_success;
  label-oi = oi;
  label-datalo_reg = datalo;
  label-datahi_reg = datahi;
 @@ -1307,7 +1351,7 @@ static void tcg_out_qemu_ld_slow_path(TCGContext *s, 
 TCGLabelQemuLdst *l)
   (uintptr_t)l-raddr);
  }
  
 -tcg_out_call(s, qemu_ld_helpers[opc  (MO_BSWAP | MO_SIZE)]);
 +tcg_out_call(s, ld_helper(opc));
  
  data_reg = l-datalo_reg;
  switch (opc  MO_SSIZE) {
 @@ -1411,9 +1455,16 @@ static void tcg_out_qemu_st_slow_path(TCGContext *s, 
 TCGLabelQemuLdst *l)
  }
  }
  
 -/* Tail call to the helper, with the return address back inline.  */
 -tcg_out_push(s, retaddr);
 -tcg_out_jmp(s, qemu_st_helpers[opc  (MO_BSWAP | MO_SIZE)]);
 +if (opc  MO_EXCL) {
 +tcg_out_call(s, st_helper(opc));
 +/* Save the output of the StoreConditional */
 +tcg_out_mov(s, TCG_TYPE_I32, l-llsc_success, TCG_REG_EAX);
 +tcg_out_jmp(s, l-raddr);
 +} else {
 +/* Tail call to the helper, with the return address back inline.  
 */
 +tcg_out_push(s, retaddr);
 +tcg_out_jmp(s, st_helper(opc));
 +}
  }

I am not sure it's a good idea to try to use the existing code. For
LL/SC ops, we don't have the slow path and the fast path, as we always
call the helpers. It's probably better to use dedicated code for calling
the helper.

Also given that TCG is already able to handle call helpers, and that the
LL/SC ops basically always call an helper, I do wonder if we can handle
LL/SC ops just like we do for some arithmetic ops, see in tcg-runtime.c
and tcg/tcg-runtime.h. That way the op will be implemented automatically
for all backends. Of course we might still want a wrapper so that the
tcg_gen_* functions transparently call the right helper.
 

-- 
Aurelien Jarno  GPG: 4096R/1DDD8C9B

[Qemu-devel] [PATCH 2/3] vga: remove unused macros

2015-08-08 Thread Paolo Bonzini
Signed-off-by: Paolo Bonzini bonz...@gnu.org
---
 hw/display/vga.c | 14 --
 1 file changed, 14 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 5965ab2..19dcb6b 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -101,12 +101,6 @@ const uint8_t gr_mask[16] = {
 #endif
 
 #ifdef HOST_WORDS_BIGENDIAN
-#define BIG 1
-#else
-#define BIG 0
-#endif
-
-#ifdef HOST_WORDS_BIGENDIAN
 #define GET_PLANE(data, p) (((data)  (24 - (p) * 8))  0xff)
 #else
 #define GET_PLANE(data, p) (((data)  ((p) * 8))  0xff)
@@ -131,14 +125,6 @@ static const uint32_t mask16[16] = {
 PAT(0x),
 };
 
-#undef PAT
-
-#ifdef HOST_WORDS_BIGENDIAN
-#define PAT(x) (x)
-#else
-#define PAT(x) cbswap_32(x)
-#endif
-
 static uint32_t expand4[256];
 static uint16_t expand2[256];
 static uint8_t expand4to8[16];
-- 
2.4.3





[Qemu-devel] [PATCH 3/3] vga: fix CGA 640x200 mode

2015-08-08 Thread Paolo Bonzini
SeaBIOS programs the CGA 2-color 640x200 mode with 0xC1 in the maximum
scan line register.  Ordinarily, this would mean 100 vertical lines,
but the CGA modes ignore bits 4:0 (DOSBox's BIOS also uses 0xC1).
Unfortunately, the test used to catch CGA modes worked for 4-color
320x200 graphics, but not for the 2-color 640x200 mode.

Signed-off-by: Paolo Bonzini bonz...@gnu.org
---
 hw/display/vga.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/display/vga.c b/hw/display/vga.c
index 19dcb6b..7d4e1b0 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -1442,12 +1442,11 @@ static void vga_draw_graphic(VGACommonState *s, int 
full_update)
 
 shift_control = (s-gr[VGA_GFX_MODE]  5)  3;
 double_scan = (s-cr[VGA_CRTC_MAX_SCAN]  7);
-if (shift_control != 1) {
+if (s-cr[VGA_CRTC_MODE]  1) {
 multi_scan = (((s-cr[VGA_CRTC_MAX_SCAN]  0x1f) + 1)  double_scan)
 - 1;
 } else {
 /* in CGA modes, multi_scan is ignored */
-/* XXX: is it correct ? */
 multi_scan = double_scan;
 }
 multi_run = multi_scan;
-- 
2.4.3




[Qemu-devel] [PATCH 0/3] vga: first round of hardware emulation fixes

2015-08-08 Thread Paolo Bonzini
During the last Christmas holidays I spent some time fixing VGA emulation
with old games and old software in general.  In particular I fixed
Commander Keen 4, Jazz Jackrabbit and GW Basic.  I never had the time to
post the patches, however.  Here is a start, it's enough to fix GW Basic
SCREEN 2 and to make Keen 4 playable while a bit jerky.

Paolo

Paolo Bonzini (3):
  vga: mask addresses in non-VESA modes to 256k
  vga: remove unused macros
  vga: fix CGA 640x200 mode

 hw/display/vga-helpers.h | 52 ++--
 hw/display/vga.c | 24 ++
 2 files changed, 39 insertions(+), 37 deletions(-)

-- 
2.4.3




[Qemu-devel] [PATCH 1/3] vga: mask addresses in non-VESA modes to 256k

2015-08-08 Thread Paolo Bonzini
This allows setting the start address to a high value, and reading the
bottom of the screen from the beginning of VRAM.  Commander Keen 4
(Goodbye, Galaxy!) relies on this behavior.

Signed-off-by: Paolo Bonzini bonz...@gnu.org
---
 hw/display/vga-helpers.h | 52 ++--
 hw/display/vga.c |  7 +--
 2 files changed, 38 insertions(+), 21 deletions(-)

diff --git a/hw/display/vga-helpers.h b/hw/display/vga-helpers.h
index 94f6de2..ae61890 100644
--- a/hw/display/vga-helpers.h
+++ b/hw/display/vga-helpers.h
@@ -99,7 +99,7 @@ static void vga_draw_glyph9(uint8_t *d, int linesize,
  * 4 color mode
  */
 static void vga_draw_line2(VGACommonState *s1, uint8_t *d,
-   const uint8_t *s, int width)
+   unsigned int addr, int width)
 {
 uint32_t plane_mask, *palette, data, v;
 int x;
@@ -108,6 +108,7 @@ static void vga_draw_line2(VGACommonState *s1, uint8_t *d,
 plane_mask = mask16[s1-ar[VGA_ATC_PLANE_ENABLE]  0xf];
 width = 3;
 for(x = 0; x  width; x++) {
+const uint8_t *s = s1-vram_ptr + (addr  (VGA_VRAM_SIZE - 1));
 data = ((uint32_t *)s)[0];
 data = plane_mask;
 v = expand2[GET_PLANE(data, 0)];
@@ -124,7 +125,7 @@ static void vga_draw_line2(VGACommonState *s1, uint8_t *d,
 ((uint32_t *)d)[6] = palette[(v  4)  0xf];
 ((uint32_t *)d)[7] = palette[(v  0)  0xf];
 d += 32;
-s += 4;
+addr += 4;
 }
 }
 
@@ -135,7 +136,7 @@ static void vga_draw_line2(VGACommonState *s1, uint8_t *d,
  * 4 color mode, dup2 horizontal
  */
 static void vga_draw_line2d2(VGACommonState *s1, uint8_t *d,
- const uint8_t *s, int width)
+ unsigned int addr, int width)
 {
 uint32_t plane_mask, *palette, data, v;
 int x;
@@ -144,6 +145,7 @@ static void vga_draw_line2d2(VGACommonState *s1, uint8_t *d,
 plane_mask = mask16[s1-ar[VGA_ATC_PLANE_ENABLE]  0xf];
 width = 3;
 for(x = 0; x  width; x++) {
+const uint8_t *s = s1-vram_ptr + (addr  (VGA_VRAM_SIZE - 1));
 data = ((uint32_t *)s)[0];
 data = plane_mask;
 v = expand2[GET_PLANE(data, 0)];
@@ -160,7 +162,7 @@ static void vga_draw_line2d2(VGACommonState *s1, uint8_t *d,
 PUT_PIXEL2(d, 6, palette[(v  4)  0xf]);
 PUT_PIXEL2(d, 7, palette[(v  0)  0xf]);
 d += 64;
-s += 4;
+addr += 4;
 }
 }
 
@@ -168,7 +170,7 @@ static void vga_draw_line2d2(VGACommonState *s1, uint8_t *d,
  * 16 color mode
  */
 static void vga_draw_line4(VGACommonState *s1, uint8_t *d,
-   const uint8_t *s, int width)
+   unsigned int addr, int width)
 {
 uint32_t plane_mask, data, v, *palette;
 int x;
@@ -177,6 +179,7 @@ static void vga_draw_line4(VGACommonState *s1, uint8_t *d,
 plane_mask = mask16[s1-ar[VGA_ATC_PLANE_ENABLE]  0xf];
 width = 3;
 for(x = 0; x  width; x++) {
+const uint8_t *s = s1-vram_ptr + (addr  (VGA_VRAM_SIZE - 1));
 data = ((uint32_t *)s)[0];
 data = plane_mask;
 v = expand4[GET_PLANE(data, 0)];
@@ -192,7 +195,7 @@ static void vga_draw_line4(VGACommonState *s1, uint8_t *d,
 ((uint32_t *)d)[6] = palette[(v  4)  0xf];
 ((uint32_t *)d)[7] = palette[(v  0)  0xf];
 d += 32;
-s += 4;
+addr += 4;
 }
 }
 
@@ -200,7 +203,7 @@ static void vga_draw_line4(VGACommonState *s1, uint8_t *d,
  * 16 color mode, dup2 horizontal
  */
 static void vga_draw_line4d2(VGACommonState *s1, uint8_t *d,
- const uint8_t *s, int width)
+ unsigned int addr, int width)
 {
 uint32_t plane_mask, data, v, *palette;
 int x;
@@ -209,6 +212,7 @@ static void vga_draw_line4d2(VGACommonState *s1, uint8_t *d,
 plane_mask = mask16[s1-ar[VGA_ATC_PLANE_ENABLE]  0xf];
 width = 3;
 for(x = 0; x  width; x++) {
+const uint8_t *s = s1-vram_ptr + (addr  (VGA_VRAM_SIZE - 1));
 data = ((uint32_t *)s)[0];
 data = plane_mask;
 v = expand4[GET_PLANE(data, 0)];
@@ -224,7 +228,7 @@ static void vga_draw_line4d2(VGACommonState *s1, uint8_t *d,
 PUT_PIXEL2(d, 6, palette[(v  4)  0xf]);
 PUT_PIXEL2(d, 7, palette[(v  0)  0xf]);
 d += 64;
-s += 4;
+addr += 4;
 }
 }
 
@@ -234,7 +238,7 @@ static void vga_draw_line4d2(VGACommonState *s1, uint8_t *d,
  * XXX: add plane_mask support (never used in standard VGA modes)
  */
 static void vga_draw_line8d2(VGACommonState *s1, uint8_t *d,
- const uint8_t *s, int width)
+ unsigned int addr, int width)
 {
 uint32_t *palette;
 int x;
@@ -242,12 +246,13 @@ static void vga_draw_line8d2(VGACommonState *s1, uint8_t 
*d,
 palette = s1-last_palette;
 width = 3;
 for(x = 0; x  width; x++) {
+const uint8_t *s = s1-vram_ptr + (addr  

Re: [Qemu-devel] [RFC v4 5/9] configure: Enable/disable new qemu_{ld, st} excl insns

2015-08-08 Thread Peter Maydell
On 8 August 2015 at 13:44, Aurelien Jarno aurel...@aurel32.net wrote:
 On 2015-08-07 19:03, Alvise Rigo wrote:
 Introduce the new --enable-tcg-ldst-excl configure option to enable the
 LL/SC operations only for those backends that support them.

 Suggested-by: Jani Kokkonen jani.kokko...@huawei.com
 Suggested-by: Claudio Fontana claudio.font...@huawei.com
 Signed-off-by: Alvise Rigo a.r...@virtualopensystems.com
 ---
  configure | 21 +
  1 file changed, 21 insertions(+)

 We have seen that for this kind of patch, it's better to add support in
 all backends, otherwise it takes ages to get all the backends converted.
 I think you should involve the backend maintainers. I can try to provide
 the corresponding patches for mips and ia64.

...and if we do need to do it one backend at a time we should do
this automatically, not by requiring users to give configure
arguments...

-- PMM



Re: [Qemu-devel] [Consult] tilegx: About floating point instructions

2015-08-08 Thread Chen Gang
Hello all:

Below is my current idea for all floating point insns. For me, it is not
the precise implementation, even not completely implement -- assume pack
insns can only for packing (u)int32_t when they are used individually:

  fsingle_add1; return calc flags, save calc result to env.

  fsingle_sub1; return calc flags, save calc result to env.

  fsingle_addsub2 ; set has result flag.

  fsingle_mul1; skip return value, save calc result to env.
set has result flag.

  fsingle_mul2; skipped.


  fsingle_pack1   ; skipped.

  fsingle_pack1   ; if has result
reset has result flag.
return calc result from env.
else
pack srca 
reference from tilegx.md: float(uns)sisf2.
get (u)int32_t a, then (u)int32_to_float32.

  fdouble_unpack_max: ; skipped.

  fdouble_unpack_min: ; skipped.

  fdouble_add_flags:  ; return calc flags, save calc result to env.

  fdouble_sub_flags:  ; return calc flags, save calc result to env.

  fdouble_addsub: ; set has result flag.

  fdouble_mul_flags:  ; skip return flags, save calc result to env.
set has result flag.

  fdouble_pack1:  ; if has result 
reset has result flag.
return calc result from env.
else
pack srca and srcb.
reference from tilegx.md: float(uns)sidf2.
get (u)int32_t a, then (u)int32_to_float64.

  fdouble_pack2:  ; skipped.


  (fsingle_add1/sub1, fdouble_add/sub_flags can be used individually,
   e.g gcc testsuit for complex number).


Next, I shall implement the floating point insns, welcome any related
ideas, suggestions, and completions.

Thanks.


On 8/5/15 22:16, Chen Gang wrote:
 On 8/4/15 23:04, Richard Henderson wrote:
 On 08/04/2015 06:56 AM, Chen Gang wrote:

 On 8/4/15 04:47, Chen Gang wrote:
 On 8/4/15 00:40, Richard Henderson wrote:
 On 08/01/2015 02:47 AM, Chen Gang wrote:
 I am just adding floating point instructions (e.g. fsingle_add1),
 but for me, I can not find any details about them (the ISA
 documents only give a summary description, but not details), e.g.

 The tilegx splits the four/six cycle arithmetic into multiple
 black-box instructions.  You need only really implement one of the
 four, with the rest of them being implemented as nops or moves.

 Looking at what gcc produces gives the hints:

 fdouble_unpack_minmin, srca, srcb fdouble_unpack_max  max, 
 srca,
 srcb fdouble_add_flagsflg, srca, srcb fdouble_addsub  max, 
 min, flg 
 fdouble_pack1 dst, max, flg fdouble_pack2 dst, 
 max, zero

 The unpack, addsub, and pack2 insns can be ignored, the add_flags
 insn can perform the whole operation, the pack1 insn performs a move
 from flg to dst.

 Similarly for the single-precision:

 fsingle_add1  tmp, srca, srcb fsingle_addsub2 tmp, 
 srca, srcb 
 fsingle_pack1 flg, tmp fsingle_pack2  dst, tmp, flg

 The add1 insn performs the whole operation, the addsub2 and pack1
 insns are ignored, and the pack2 insn is a move from tmp to dst.


 After check the tilegx.md completely, for me, we still need implement
 each of them precisely, or we can not emulate all cases (e.g. muldf3).

 No, you can still implement all of muldf3 in fdouble_mul_flags.
 Again, the fdouble_pack1 copies from the flag input to the output.

 Yes, there is a 64-bit multiply in there, but the tcg optimizer
 should be able to delete all of that as unused.  Especially if you have the
 fdouble_unpack* insns store zero into their destinations.

 
 For me, I am not quite sure. But I guess, what you said should be OK (at
 least, what you said is very useful for the implementation).
 
 
 Don't get me wrong -- more accurate implementation of the actual
 insns would be nice, especially for debugging.  But if the insns
 aren't accurately documented I don't see what choice we have.

 
 For me, I guess, we can still try to implement the details.
 
  - The document has all floating point instructions' summary, so we can
think of, or guess its implementation entirely.
 
  - gcc uses them all and completely, so it is our good sample and good
reference (but we should not assume gcc must be correct, since we
just use qemu for gcc testsuite).
 
  - Tilegx floating point format should be standard (at least, reference
to the standard format), so we can reference the related information
from google/baidu.
 
 
 On the good side, implementing the entire operation as part of the flags 
 step
 probably results in faster emulation.

 
 I guess so, too.
 
 
 I shall try to finish the simple implementation, firstly. Then try to
 implement the floating point instructions 

Re: [Qemu-devel] [Consult] tilegx: About floating point instructions

2015-08-08 Thread Chen Gang

On 8/9/15 01:23, Chen Gang wrote:
 Hello all:
 
 Below is my current idea for all floating point insns. For me, it is not
 the precise implementation, even not completely implement -- assume pack
 insns can only for packing (u)int32_t when they are used individually:
 
   fsingle_add1; return calc flags, save calc result to env.
 
   fsingle_sub1; return calc flags, save calc result to env.
 
   fsingle_addsub2 ; set has result flag.
 
   fsingle_mul1; skip return value, save calc result to env.
 set has result flag.
 
   fsingle_mul2; skipped.
 
 
   fsingle_pack1   ; skipped.
 
   fsingle_pack1   ; if has result
 reset has result flag.
 return calc result from env.
 else
 pack srca 
 reference from tilegx.md: float(uns)sisf2.
 get (u)int32_t a, then (u)int32_to_float32.

For pack srca and srcb, the related demo like below (srca and srcb
are uint64_t):

switch (srca  0x3ff) {

/* treat it as uint32_t */
case 0x9e:
return uint32_to_float32(srca  32, FP_STATUS);

/* treat it as int32_t, must be negative number */
case 0x29e:
return int32_to_float32(srca  32 | 0x8000, FP_STATUS);

default:
unimplemented (gen_exception).
}

 
   fdouble_unpack_max: ; skipped.
 
   fdouble_unpack_min: ; skipped.
 
   fdouble_add_flags:  ; return calc flags, save calc result to env.
 
   fdouble_sub_flags:  ; return calc flags, save calc result to env.
 
   fdouble_addsub: ; set has result flag.
 
   fdouble_mul_flags:  ; skip return flags, save calc result to env.
 set has result flag.
 
   fdouble_pack1:  ; if has result 
 reset has result flag.
 return calc result from env.
 else
 pack srca and srcb.
 reference from tilegx.md: float(uns)sidf2.
 get (u)int32_t a, then (u)int32_to_float64.

 
For pack srca and srcb, the related demo like below (srca and srcb
are uint64_t):

switch (srcb  0x) {

/* treat it as uint32_t */
case 0x21b00:
return uint32_to_float64(srca  4, FP_STATUS);

/* treat it as int32_t, must be negative number */
case 0xa1b00:
return int32_to_float64(srca  4 | 0x8000, FP_STATUS);

default:
unimplemented (gen_exception).
}

   fdouble_pack2:  ; skipped.
 
 
   (fsingle_add1/sub1, fdouble_add/sub_flags can be used individually,
e.g gcc testsuit for complex number).
 
 
 Next, I shall implement the floating point insns, welcome any related
 ideas, suggestions, and completions.
 
 Thanks.
 
 
 On 8/5/15 22:16, Chen Gang wrote:
 On 8/4/15 23:04, Richard Henderson wrote:
 On 08/04/2015 06:56 AM, Chen Gang wrote:

 On 8/4/15 04:47, Chen Gang wrote:
 On 8/4/15 00:40, Richard Henderson wrote:
 On 08/01/2015 02:47 AM, Chen Gang wrote:
 I am just adding floating point instructions (e.g. fsingle_add1),
 but for me, I can not find any details about them (the ISA
 documents only give a summary description, but not details), e.g.

 The tilegx splits the four/six cycle arithmetic into multiple
 black-box instructions.  You need only really implement one of the
 four, with the rest of them being implemented as nops or moves.

 Looking at what gcc produces gives the hints:

 fdouble_unpack_min   min, srca, srcb fdouble_unpack_max  max, 
 srca,
 srcb fdouble_add_flags   flg, srca, srcb fdouble_addsub  max, 
 min, flg 
 fdouble_pack1dst, max, flg fdouble_pack2 dst, 
 max, zero

 The unpack, addsub, and pack2 insns can be ignored, the add_flags
 insn can perform the whole operation, the pack1 insn performs a move
 from flg to dst.

 Similarly for the single-precision:

 fsingle_add1 tmp, srca, srcb fsingle_addsub2 tmp, 
 srca, srcb 
 fsingle_pack1flg, tmp fsingle_pack2  dst, tmp, flg

 The add1 insn performs the whole operation, the addsub2 and pack1
 insns are ignored, and the pack2 insn is a move from tmp to dst.


 After check the tilegx.md completely, for me, we still need implement
 each of them precisely, or we can not emulate all cases (e.g. muldf3).

 No, you can still implement all of muldf3 in fdouble_mul_flags.
 Again, the fdouble_pack1 copies from the flag input to the output.

 Yes, there is a 64-bit multiply in there, but the tcg optimizer
 should be able to delete all of that as unused.  Especially if you have the
 fdouble_unpack* insns store zero into their destinations.


 For me, I am not quite sure. But I guess, what you said should be OK (at
 least, what you said is very useful for the implementation).


 Don't get me wrong -- more accurate implementation of the actual
 insns would be 

Re: [Qemu-devel] [Consult] tilegx: About floating point instructions

2015-08-08 Thread Chen Gang
On 8/9/15 09:10, Chen Gang wrote:
 
 On 8/9/15 01:23, Chen Gang wrote:
 Hello all:

 Below is my current idea for all floating point insns. For me, it is not
 the precise implementation, even not completely implement -- assume pack
 insns can only for packing (u)int32_t when they are used individually:

   fsingle_add1; return calc flags, save calc result to env.

   fsingle_sub1; return calc flags, save calc result to env.

   fsingle_addsub2 ; set has result flag.

   fsingle_mul1; skip return value, save calc result to env.
 set has result flag.

   fsingle_mul2; skipped.


   fsingle_pack1   ; skipped.

   fsingle_pack1   ; if has result
 reset has result flag.
 return calc result from env.
 else
 pack srca 
 reference from tilegx.md: float(uns)sisf2.
 get (u)int32_t a, then (u)int32_to_float32.
 
 For pack srca and srcb, the related demo like below (srca and srcb
 are uint64_t):
 

Oh, sorry, for pack srca (not for pack srca and srcb)

 switch (srca  0x3ff) {
 
 /* treat it as uint32_t */
 case 0x9e:
 return uint32_to_float32(srca  32, FP_STATUS);
 
 /* treat it as int32_t, must be negative number */
 case 0x29e:
 return int32_to_float32(srca  32 | 0x8000, FP_STATUS);
 
 default:
 unimplemented (gen_exception).
 }
 

   fdouble_unpack_max: ; skipped.

   fdouble_unpack_min: ; skipped.

   fdouble_add_flags:  ; return calc flags, save calc result to env.

   fdouble_sub_flags:  ; return calc flags, save calc result to env.

   fdouble_addsub: ; set has result flag.

   fdouble_mul_flags:  ; skip return flags, save calc result to env.
 set has result flag.

   fdouble_pack1:  ; if has result 
 reset has result flag.
 return calc result from env.
 else
 pack srca and srcb.
 reference from tilegx.md: float(uns)sidf2.
 get (u)int32_t a, then (u)int32_to_float64.

  
 For pack srca and srcb, the related demo like below (srca and srcb
 are uint64_t):
 
 switch (srcb  0x) {
 

Oh, sorry, should use 0xf instead of 0x.

 /* treat it as uint32_t */
 case 0x21b00:
 return uint32_to_float64(srca  4, FP_STATUS);
 
 /* treat it as int32_t, must be negative number */
 case 0xa1b00:
 return int32_to_float64(srca  4 | 0x8000, FP_STATUS);
 
 default:
 unimplemented (gen_exception).
 }
 
   fdouble_pack2:  ; skipped.


   (fsingle_add1/sub1, fdouble_add/sub_flags can be used individually,
e.g gcc testsuit for complex number).


 Next, I shall implement the floating point insns, welcome any related
 ideas, suggestions, and completions.

 Thanks.


 On 8/5/15 22:16, Chen Gang wrote:
 On 8/4/15 23:04, Richard Henderson wrote:
 On 08/04/2015 06:56 AM, Chen Gang wrote:

 On 8/4/15 04:47, Chen Gang wrote:
 On 8/4/15 00:40, Richard Henderson wrote:
 On 08/01/2015 02:47 AM, Chen Gang wrote:
 I am just adding floating point instructions (e.g. fsingle_add1),
 but for me, I can not find any details about them (the ISA
 documents only give a summary description, but not details), e.g.

 The tilegx splits the four/six cycle arithmetic into multiple
 black-box instructions.  You need only really implement one of the
 four, with the rest of them being implemented as nops or moves.

 Looking at what gcc produces gives the hints:

 fdouble_unpack_min  min, srca, srcb fdouble_unpack_max  max, 
 srca,
 srcb fdouble_add_flags  flg, srca, srcb fdouble_addsub  max, 
 min, flg 
 fdouble_pack1   dst, max, flg fdouble_pack2 dst, 
 max, zero

 The unpack, addsub, and pack2 insns can be ignored, the add_flags
 insn can perform the whole operation, the pack1 insn performs a move
 from flg to dst.

 Similarly for the single-precision:

 fsingle_add1tmp, srca, srcb fsingle_addsub2 tmp, 
 srca, srcb 
 fsingle_pack1   flg, tmp fsingle_pack2  dst, tmp, flg

 The add1 insn performs the whole operation, the addsub2 and pack1
 insns are ignored, and the pack2 insn is a move from tmp to dst.


 After check the tilegx.md completely, for me, we still need implement
 each of them precisely, or we can not emulate all cases (e.g. muldf3).

 No, you can still implement all of muldf3 in fdouble_mul_flags.
 Again, the fdouble_pack1 copies from the flag input to the output.

 Yes, there is a 64-bit multiply in there, but the tcg optimizer
 should be able to delete all of that as unused.  Especially if you have the
 fdouble_unpack* insns store zero into their destinations.


 For me, I am not quite sure. But I guess, what you said should be OK