Re: [Qemu-devel] [PATCH v4 4/4] sockets: Handle race condition between binds to the same port

[Knut Omang]

On Mon, 2017-06-26 at 11:34 +0100, Daniel P. Berrange wrote:
> On Fri, Jun 23, 2017 at 12:31:08PM +0200, Knut Omang wrote:
> > 
> > If an offset of ports is specified to the inet_listen_saddr function(),
> > and two or more processes tries to bind from these ports at the same time,
> > occasionally more than one process may be able to bind to the same
> > port. The condition is detected by listen() but too late to avoid a failure.
> > 
> > This function is called by socket_listen() and used
> > by all socket listening code in QEMU, so all cases where any form of dynamic
> > port selection is used should be subject to this issue.
> > 
> > Add code to close and re-establish the socket when this
> > condition is observed, hiding the race condition from the user.
> > 
> > This has been developed and tested by means of the
> > test-listen unit test in the previous commit.
> > Enable the test for make check now that it passes.
> > 
> > Signed-off-by: Knut Omang 
> > Reviewed-by: Bhavesh Davda 
> > Reviewed-by: Yuval Shaia 
> > Reviewed-by: Girish Moodalbail 
> > ---
> >  tests/Makefile.include |  2 +-
> >  util/qemu-sockets.c| 68 ---
> >  2 files changed, 53 insertions(+), 17 deletions(-)
> > 
> > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > index 22bb97e..c38f94e 100644
> > --- a/tests/Makefile.include
> > +++ b/tests/Makefile.include
> > @@ -127,7 +127,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF)
> >  gcov-files-check-bufferiszero-y = util/bufferiszero.c
> >  check-unit-y += tests/test-uuid$(EXESUF)
> >  check-unit-y += tests/ptimer-test$(EXESUF)
> > -#check-unit-y += tests/test-listen$(EXESUF)
> > +check-unit-y += tests/test-listen$(EXESUF)
> >  gcov-files-ptimer-test-y = hw/core/ptimer.c
> >  check-unit-y += tests/test-qapi-util$(EXESUF)
> >  gcov-files-test-qapi-util-y = qapi/qapi-util.c
> > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > index 48b9319..7b118b4 100644
> > --- a/util/qemu-sockets.c
> > +++ b/util/qemu-sockets.c
> > @@ -201,6 +201,42 @@ static int try_bind(int socket, InetSocketAddress
> > *saddr, struct addrinfo *e)
> >  #endif
> >  }
> >  
> > +static int try_bind_listen(int *socket, InetSocketAddress *saddr,
> > +   struct addrinfo *e, int port, Error **errp)
> > +{
> > +int s = *socket;
> > +int ret;
> > +
> > +inet_setport(e, port);
> > +ret = try_bind(s, saddr, e);
> > +if (ret) {
> > +if (errno != EADDRINUSE) {
> > +error_setg_errno(errp, errno, "Failed to bind socket");
> > +}
> > +return errno;
> > +}
> > +if (listen(s, 1) == 0) {
> > +return 0;
> > +}
> > +if (errno == EADDRINUSE) {
> > +/* We got to bind the socket to a port but someone else managed
> > + * to bind to the same port and beat us to listen on it!
> > + * Recreate the socket and return EADDRINUSE to preserve the
> > + * expected state by the caller:
> > + */
> > +closesocket(s);
> > +s = create_fast_reuse_socket(e, errp);
> 
> This usage scenario for create_fast_reuse_socket() makes its error
> reporting behaviour even more wrong. Recall that create_fast_reuse_socket
> is reporting an error if e->ai_next is NULL, which is a way of determining
> this is the last call to create_fast_reuse_socket in the loop. That
> assumption is violated though now that we're calling the method from
> inside the inner loop. Even when e->ai_next is NULL, we may be calling
> create_fast_reuse_socket many many times due to the port  'to' range.

I agree that the error reporting should go out of create_fast_reuse_socket().
Note however that this code will only be called when the race condition occurs,
which I think is very unlikely to happen more than once for each call to
inet_listen_saddr (except in my test of course..)

> 
> > 
> > +if (s < 0) {
> > +return errno;
> > +}
> > +*socket = s;
> > +errno = EADDRINUSE;
> > +return errno;
> > +}
> > +error_setg_errno(errp, errno, "Failed to listen on socket");
> > +return errno;
> > +}
> 
> This method is both preserving the global errno, and returning the
> global errno. The caller expects global errno to be preserved, so
> I think we can just return '-1' from this method.

will do,

Thanks,
Knut

> 
> > 
> > +
> >  static int inet_listen_saddr(InetSocketAddress *saddr,
> >   int port_offset,
> >   bool update_addr,
> > @@ -210,7 +246,9 @@ static int inet_listen_saddr(InetSocketAddress *saddr,
> >  char port[33];
> >  char uaddr[INET6_ADDRSTRLEN+1];
> >  char uport[33];
> > -int slisten, rc, port_min, port_max, p;
> > +int rc, port_min, port_max, p;
> > +int slisten = 0;
> > +int saved_errno = 0;
> >  Error *err = NULL;
> >  
> >  memset(&ai,0, sizeof(ai));
> > @@ -276,28 +314,26 @@ static int inet

Re: [Qemu-devel] [PATCH v4 4/4] sockets: Handle race condition between binds to the same port

[Knut Omang]

On Mon, 2017-06-26 at 13:49 +0100, Daniel P. Berrange wrote:
> On Mon, Jun 26, 2017 at 02:32:48PM +0200, Knut Omang wrote:
> > 
> > On Mon, 2017-06-26 at 11:22 +0100, Daniel P. Berrange wrote:
> > > 
> > > On Fri, Jun 23, 2017 at 12:31:08PM +0200, Knut Omang wrote:
> > > > 
> > > > If an offset of ports is specified to the inet_listen_saddr function(),
> > > > and two or more processes tries to bind from these ports at the same
> > > > time,
> > > > occasionally more than one process may be able to bind to the same
> > > > port. The condition is detected by listen() but too late to avoid a
> > > > failure.
> > > >  
> > > > This function is called by socket_listen() and used
> > > > by all socket listening code in QEMU, so all cases where any form of
> > > > dynamic
> > > > port selection is used should be subject to this issue.
> > > >  
> > > > Add code to close and re-establish the socket when this
> > > > condition is observed, hiding the race condition from the user.
> > > >  
> > > > This has been developed and tested by means of the
> > > > test-listen unit test in the previous commit.
> > > > Enable the test for make check now that it passes.
> > > >  
> > > > Signed-off-by: Knut Omang 
> > > > Reviewed-by: Bhavesh Davda 
> > > > Reviewed-by: Yuval Shaia 
> > > > Reviewed-by: Girish Moodalbail 
> > > > ---
> > > >   tests/Makefile.include |  2 +-
> > > >   util/qemu-sockets.c| 68 
> > > > ---
> > > >   2 files changed, 53 insertions(+), 17 deletions(-)
> > > >  
> > > > diff --git a/tests/Makefile.include b/tests/Makefile.include
> > > > index 22bb97e..c38f94e 100644
> > > > --- a/tests/Makefile.include
> > > > +++ b/tests/Makefile.include
> > > > @@ -127,7 +127,7 @@ check-unit-y += tests/test-bufferiszero$(EXESUF)
> > > >   gcov-files-check-bufferiszero-y = util/bufferiszero.c
> > > >   check-unit-y += tests/test-uuid$(EXESUF)
> > > >   check-unit-y += tests/ptimer-test$(EXESUF)
> > > > -#check-unit-y += tests/test-listen$(EXESUF)
> > > > +check-unit-y += tests/test-listen$(EXESUF)
> > > >   gcov-files-ptimer-test-y = hw/core/ptimer.c
> > > >   check-unit-y += tests/test-qapi-util$(EXESUF)
> > > >   gcov-files-test-qapi-util-y = qapi/qapi-util.c
> > > > diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
> > > > index 48b9319..7b118b4 100644
> > > > --- a/util/qemu-sockets.c
> > > > +++ b/util/qemu-sockets.c
> > > > @@ -201,6 +201,42 @@ static int try_bind(int socket, InetSocketAddress
> > > > *saddr, struct
> > > addrinfo *e)
> > > > 
> > > >   #endif
> > > >   }
> > > >   
> > > > +static int try_bind_listen(int *socket, InetSocketAddress *saddr,
> > > > +   struct addrinfo *e, int port, Error **errp)
> > > > +{
> > > > +int s = *socket;
> > > > +int ret;
> > > > +
> > > > +inet_setport(e, port);
> > > > +ret = try_bind(s, saddr, e);
> > > > +if (ret) {
> > > > +if (errno != EADDRINUSE) {
> > > > +error_setg_errno(errp, errno, "Failed to bind socket");
> > > > +}
> > > > +return errno;
> > > > +}
> > > > +if (listen(s, 1) == 0) {
> > > > +return 0;
> > > > +}
> > > > +if (errno == EADDRINUSE) {
> > > > +/* We got to bind the socket to a port but someone else managed
> > > > + * to bind to the same port and beat us to listen on it!
> > > > + * Recreate the socket and return EADDRINUSE to preserve the
> > > > + * expected state by the caller:
> > > > + */
> > > > +closesocket(s);
> > > > +s = create_fast_reuse_socket(e, errp);
> > > > +if (s < 0) {
> > > > +return errno;
> > > > +}
> > > > +*socket = s;
> > > 
> > > I don't really like this at all - if we need to close + recreate the
> > > socket, IMHO that should remain the job of the caller, since it owns
> > > the socket FD ultimately.
> > 
> > Normally I would agree, but this is a very unlikely situation. I considered
> > moving the
> > complexity out to the caller, even to recreate for every call, but found
> > those solutions
> > to be inferior as they do not in any way confine the problem, and cause the
> > handling of
> > the common cases to be much less readable. It's going to be some trade-offs
> > here.
> > 
> > As long as the caller is aware of (by the reference call) that the socket in
> > use may
> > change, this is in my view a clean (as clean as possible) abstraction that
> > simplifies the
> > logic at the next level. My intention is to make the common, good case as
> > readable as
> > possible and hide some of the complexity of these 
> > unlikely error scenarios inside the new functions - divide and conquer..
> > 
> > > 
> > > 
> > > > 
> > > > +errno = EADDRINUSE;
> > > > +return errno;
> > > > +}
> > > > +error_setg_errno(errp, errno, "Failed to listen on socket");
> > > > +return errno;
> > > > +}
> > > > +
> > > >   static int inet_listen_saddr(InetSocket

[Qemu-devel] [PATCH 1/2] block: add clock_type field to ThrottleGroup

[Manos Pitsidianakis]

Clock type in throttling is currently inferred by the ThrottleTimer's
clock type even though it is a per-ThrottleGroup property; it doesn't
make sense to have different clock types in the same group. Moving this
to a field in ThrottleGroup can simplify some of the throttle functions.

Signed-off-by: Manos Pitsidianakis 
---
 block/throttle-groups.c | 20 ++--
 fsdev/qemu-fsdev-throttle.c |  2 +-
 include/qemu/throttle.h |  1 +
 tests/test-throttle.c   |  4 ++--
 util/throttle.c |  4 +++-
 5 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index da2b490c38..71af3f72a1 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -61,6 +61,7 @@ typedef struct ThrottleGroup {
 QLIST_HEAD(, BlockBackendPublic) head;
 BlockBackend *tokens[2];
 bool any_timer_armed[2];
+QEMUClockType clock_type;
 
 /* These two are protected by the global throttle_groups_lock */
 unsigned refcount;
@@ -98,6 +99,12 @@ ThrottleState *throttle_group_incref(const char *name)
 if (!tg) {
 tg = g_new0(ThrottleGroup, 1);
 tg->name = g_strdup(name);
+tg->clock_type = QEMU_CLOCK_REALTIME;
+
+if (qtest_enabled()) {
+/* For testing block IO throttling only */
+tg->clock_type = QEMU_CLOCK_VIRTUAL;
+}
 qemu_mutex_init(&tg->lock);
 throttle_init(&tg->ts);
 QLIST_INIT(&tg->head);
@@ -310,7 +317,7 @@ static void schedule_next_request(BlockBackend *blk, bool 
is_write)
 token = blk;
 } else {
 ThrottleTimers *tt = &blk_get_public(token)->throttle_timers;
-int64_t now = qemu_clock_get_ns(tt->clock_type);
+int64_t now = qemu_clock_get_ns(tg->clock_type);
 timer_mod(tt->timers[is_write], now);
 tg->any_timer_armed[is_write] = true;
 }
@@ -430,7 +437,7 @@ void throttle_group_config(BlockBackend *blk, 
ThrottleConfig *cfg)
 if (timer_pending(tt->timers[1])) {
 tg->any_timer_armed[1] = false;
 }
-throttle_config(ts, tt, cfg);
+throttle_config(ts, tg->clock_type, tt, cfg);
 qemu_mutex_unlock(&tg->lock);
 
 throttle_group_restart_blk(blk);
@@ -497,13 +504,6 @@ void throttle_group_register_blk(BlockBackend *blk, const 
char *groupname)
 BlockBackendPublic *blkp = blk_get_public(blk);
 ThrottleState *ts = throttle_group_incref(groupname);
 ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
-int clock_type = QEMU_CLOCK_REALTIME;
-
-if (qtest_enabled()) {
-/* For testing block IO throttling only */
-clock_type = QEMU_CLOCK_VIRTUAL;
-}
-
 blkp->throttle_state = ts;
 
 qemu_mutex_lock(&tg->lock);
@@ -518,7 +518,7 @@ void throttle_group_register_blk(BlockBackend *blk, const 
char *groupname)
 
 throttle_timers_init(&blkp->throttle_timers,
  blk_get_aio_context(blk),
- clock_type,
+ tg->clock_type,
  read_timer_cb,
  write_timer_cb,
  blk);
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 7ae4e86646..453fb1efde 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -86,7 +86,7 @@ void fsdev_throttle_init(FsThrottle *fst)
  fsdev_throttle_read_timer_cb,
  fsdev_throttle_write_timer_cb,
  fst);
-throttle_config(&fst->ts, &fst->tt, &fst->cfg);
+throttle_config(&fst->ts, QEMU_CLOCK_REALTIME, &fst->tt, &fst->cfg);
 qemu_co_queue_init(&fst->throttled_reqs[0]);
 qemu_co_queue_init(&fst->throttled_reqs[1]);
 }
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 9109657609..8d2fd77d2b 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -139,6 +139,7 @@ bool throttle_enabled(ThrottleConfig *cfg);
 bool throttle_is_valid(ThrottleConfig *cfg, Error **errp);
 
 void throttle_config(ThrottleState *ts,
+ QEMUClockType clock_type,
  ThrottleTimers *tt,
  ThrottleConfig *cfg);
 
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index a9201b1fea..2d9cd4647c 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -228,7 +228,7 @@ static void test_config_functions(void)
  read_timer_cb, write_timer_cb, &ts);
 /* structure reset by throttle_init previous_leak should be null */
 g_assert(!ts.previous_leak);
-throttle_config(&ts, &tt, &orig_cfg);
+throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &tt, &orig_cfg);
 
 /* has previous leak been initialized by throttle_config ? */
 g_assert(ts.previous_leak);
@@ -486,7 +486,7 @@ static bool do_test_accounting(bool is_ops, /* are we 
testing bps or ops */
 throttle_init(&ts);
  

[Qemu-devel] [PATCH 2/2] block: remove timer canceling in throttle_config()

[Manos Pitsidianakis]

throttle_config() cancels the timers of the calling BlockBackend. This
doesn't make sense because other BlockBackends in the group remain
untouched. There's no need to cancel the timers in the one specific
BlockBackend so let's not do that. Throttled requests will run as
scheduled and future requests will follow the new configuration. This
also allows a throttle group's configuration to be changed even when it
has no members.

Signed-off-by: Manos Pitsidianakis 
---
 block/throttle-groups.c | 10 +-
 fsdev/qemu-fsdev-throttle.c |  2 +-
 include/qemu/throttle.h |  1 -
 tests/test-throttle.c   |  4 ++--
 util/throttle.c | 14 --
 5 files changed, 4 insertions(+), 27 deletions(-)

diff --git a/block/throttle-groups.c b/block/throttle-groups.c
index 71af3f72a1..890bfded3f 100644
--- a/block/throttle-groups.c
+++ b/block/throttle-groups.c
@@ -426,18 +426,10 @@ void throttle_group_restart_blk(BlockBackend *blk)
 void throttle_group_config(BlockBackend *blk, ThrottleConfig *cfg)
 {
 BlockBackendPublic *blkp = blk_get_public(blk);
-ThrottleTimers *tt = &blkp->throttle_timers;
 ThrottleState *ts = blkp->throttle_state;
 ThrottleGroup *tg = container_of(ts, ThrottleGroup, ts);
 qemu_mutex_lock(&tg->lock);
-/* throttle_config() cancels the timers */
-if (timer_pending(tt->timers[0])) {
-tg->any_timer_armed[0] = false;
-}
-if (timer_pending(tt->timers[1])) {
-tg->any_timer_armed[1] = false;
-}
-throttle_config(ts, tg->clock_type, tt, cfg);
+throttle_config(ts, tg->clock_type, cfg);
 qemu_mutex_unlock(&tg->lock);
 
 throttle_group_restart_blk(blk);
diff --git a/fsdev/qemu-fsdev-throttle.c b/fsdev/qemu-fsdev-throttle.c
index 453fb1efde..49eebb5412 100644
--- a/fsdev/qemu-fsdev-throttle.c
+++ b/fsdev/qemu-fsdev-throttle.c
@@ -86,7 +86,7 @@ void fsdev_throttle_init(FsThrottle *fst)
  fsdev_throttle_read_timer_cb,
  fsdev_throttle_write_timer_cb,
  fst);
-throttle_config(&fst->ts, QEMU_CLOCK_REALTIME, &fst->tt, &fst->cfg);
+throttle_config(&fst->ts, QEMU_CLOCK_REALTIME, &fst->cfg);
 qemu_co_queue_init(&fst->throttled_reqs[0]);
 qemu_co_queue_init(&fst->throttled_reqs[1]);
 }
diff --git a/include/qemu/throttle.h b/include/qemu/throttle.h
index 8d2fd77d2b..d056008c18 100644
--- a/include/qemu/throttle.h
+++ b/include/qemu/throttle.h
@@ -140,7 +140,6 @@ bool throttle_is_valid(ThrottleConfig *cfg, Error **errp);
 
 void throttle_config(ThrottleState *ts,
  QEMUClockType clock_type,
- ThrottleTimers *tt,
  ThrottleConfig *cfg);
 
 void throttle_get_config(ThrottleState *ts, ThrottleConfig *cfg);
diff --git a/tests/test-throttle.c b/tests/test-throttle.c
index 2d9cd4647c..768f11dfed 100644
--- a/tests/test-throttle.c
+++ b/tests/test-throttle.c
@@ -228,7 +228,7 @@ static void test_config_functions(void)
  read_timer_cb, write_timer_cb, &ts);
 /* structure reset by throttle_init previous_leak should be null */
 g_assert(!ts.previous_leak);
-throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &tt, &orig_cfg);
+throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &orig_cfg);
 
 /* has previous leak been initialized by throttle_config ? */
 g_assert(ts.previous_leak);
@@ -486,7 +486,7 @@ static bool do_test_accounting(bool is_ops, /* are we 
testing bps or ops */
 throttle_init(&ts);
 throttle_timers_init(&tt, ctx, QEMU_CLOCK_VIRTUAL,
  read_timer_cb, write_timer_cb, &ts);
-throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &tt, &cfg);
+throttle_config(&ts, QEMU_CLOCK_VIRTUAL, &cfg);
 
 /* account a read */
 throttle_account(&ts, false, size);
diff --git a/util/throttle.c b/util/throttle.c
index 3e948071dd..b2a52b8b34 100644
--- a/util/throttle.c
+++ b/util/throttle.c
@@ -388,24 +388,14 @@ static void throttle_unfix_bucket(LeakyBucket *bkt)
 }
 }
 
-/* take care of canceling a timer */
-static void throttle_cancel_timer(QEMUTimer *timer)
-{
-assert(timer != NULL);
-
-timer_del(timer);
-}
-
 /* Used to configure the throttle
  *
  * @ts: the throttle state we are working on
  * @clock_type: the group's clock_type
- * @tt: the throttle timers we use in this aio context
  * @cfg: the config to set
  */
 void throttle_config(ThrottleState *ts,
  QEMUClockType clock_type,
- ThrottleTimers *tt,
  ThrottleConfig *cfg)
 {
 int i;
@@ -417,10 +407,6 @@ void throttle_config(ThrottleState *ts,
 }
 
 ts->previous_leak = qemu_clock_get_ns(clock_type);
-
-for (i = 0; i < 2; i++) {
-throttle_cancel_timer(tt->timers[i]);
-}
 }
 
 /* used to get config
-- 
2.11.0

[Qemu-devel] [PATCH 0/2] ThrottleGroup configuration fixes

[Manos Pitsidianakis]

Since in the future ThrottleGroup configuration will be done even if it
has no members, we need to separate the two.

Manos Pitsidianakis (2):
  block: add clock_type field to ThrottleGroup
  block: remove timer canceling in throttle_config()

 block/throttle-groups.c | 27 ++-
 fsdev/qemu-fsdev-throttle.c |  2 +-
 include/qemu/throttle.h |  2 +-
 tests/test-throttle.c   |  4 ++--
 util/throttle.c | 18 +++---
 5 files changed, 17 insertions(+), 36 deletions(-)

-- 
2.11.0

Re: [Qemu-devel] [PATCH v2 2/8] target/s390x: Implement CONVERT UNICODE insns

[Aurelien Jarno]

On 2017-07-01 13:25, Richard Henderson wrote:
> Signed-off-by: Richard Henderson 
> ---
>  target/s390x/helper.h  |   6 +
>  target/s390x/mem_helper.c  | 310 
> +
>  target/s390x/translate.c   |  44 +++
>  target/s390x/insn-data.def |  13 ++
>  4 files changed, 373 insertions(+)
> 

...

> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index e739525..9301daa 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -2122,6 +2122,49 @@ static ExitStatus op_ct(DisasContext *s, DisasOps *o)
>  return NO_EXIT;
>  }
>  
> +static ExitStatus op_cuXX(DisasContext *s, DisasOps *o)
> +{
> +int m3 = get_field(s->fields, m3);
> +TCGv_i32 r1 = tcg_const_i32(get_field(s->fields, r1));
> +TCGv_i32 r2 = tcg_const_i32(get_field(s->fields, r2));
> +TCGv_i32 chk;
> +
> +if (!s390_has_feat(s->insn->fac == S390_FEAT_EXTENDED_TRANSLATION_3
> +   ? S390_FEAT_ETF3_ENH : S390_FEAT_ETF2_ENH)) {
> +m3 = 0;
> +}

This doesn't look correct to me. The well-formedness checking is part of
ETF3_ENH facility, for both convert unicode instructions that are part
of the Z architecture (CU12 and CU21) and for the ones added by the ETF3
facility (CU14 and CU24).

The rest of the patch now looks fine.

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

Re: [Qemu-devel] [PATCH v2 8/8] target/s390x: Fix risbg handling

[Aurelien Jarno]

On 2017-07-01 13:26, Richard Henderson wrote:
> The rotation is to the left, but extract shifts to the right.
> The computation of the extract parameters needs adjusting.
> 
> For the entry condition, simplify
> 
>   64 - rot + len <= 64
>   -rot + len <= 0
>   len <= rot
> 
> Reported-by: David Hildenbrand 
> Suggested-by: Aurelien Jarno 
> Signed-off-by: Richard Henderson 
> ---
>  target/s390x/translate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
> index 1f0c401..89b2ea5 100644
> --- a/target/s390x/translate.c
> +++ b/target/s390x/translate.c
> @@ -3472,8 +3472,8 @@ static ExitStatus op_risbg(DisasContext *s, DisasOps *o)
>  }
>  
>  /* In some cases we can implement this with extract.  */
> -if (imask == 0 && pos == 0 && len > 0 && rot + len <= 64) {
> -tcg_gen_extract_i64(o->out, o->in2, rot, len);
> +if (imask == 0 && pos == 0 && len > 0 && len <= rot) {
> +tcg_gen_extract_i64(o->out, o->in2, 64 - rot, len);
>  return NO_EXIT;
>  }

Reviewed-by: Aurelien Jarno 


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

Re: [Qemu-devel] [PATCH v4 1/2] iotests: Use absolute paths for executables

[Max Reitz]

On 2017-06-30 21:41, Eric Blake wrote:
> On 06/29/2017 09:46 PM, Max Reitz wrote:
> 
 +++ b/tests/qemu-iotests/common.config
 @@ -103,6 +103,12 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
  export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
  fi
  
 +export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
>>>
>>> ...now that you updated per my review to favor 'type' over 'which'?
>>> Otherwise, the R-b stands.
>>
>> Thanks, will fix and apply then...
>>
>> ...and done, applied to my block branch:
>>
>> https://github.com/XanClic/qemu/commits/block
> 
> Sorry for not noticing sooner, but you'll need to replace v4 (commit
> 0f0fec82 on your branch) with a fix, because now your branch does the
> following on all iotests for me:
> 
> 068 3s ... - output mismatch (see 068.out.bad)
> --- /home/eblake/qemu/tests/qemu-iotests/068.out  2017-06-26
> 22:02:56.057734882 -0500
> +++ 068.out.bad   2017-06-30 14:35:28.720241398 -0500
> @@ -1,4 +1,5 @@
>  QA output created by 068
> +realpath: '': No such file or directory

Oops.

> The culprit? $QEMU_VXHS_PROG is empty for me, which means `set_prog_path
> qnio_server` found nothing to use.  You'll have to add in a safety valve
> that only calls 'type' if operating on a non-empty path in the first place.

Sure, will drop the patches and send a v5.

Thanks!

Max



signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v22 19/30] qcow2: add persistent dirty bitmaps support

[Max Reitz]

On 2017-06-30 19:47, Eric Blake wrote:
> On 06/29/2017 09:23 PM, Max Reitz wrote:
>> On 2017-06-30 04:18, Eric Blake wrote:
>>> On 06/28/2017 07:05 AM, Vladimir Sementsov-Ogievskiy wrote:
 Store persistent dirty bitmaps in qcow2 image.

 Signed-off-by: Vladimir Sementsov-Ogievskiy 
 Reviewed-by: Max Reitz 
 ---
> 
>>>
>>> This grabs the size (currently in sectors, although I plan to fix it to
>>> be in bytes)...
>>>
 +const char *bm_name = bdrv_dirty_bitmap_name(bitmap);
 +uint8_t *buf = NULL;
 +BdrvDirtyBitmapIter *dbi;
 +uint64_t *tb;
 +uint64_t tb_size =
 +size_to_clusters(s,
 +bdrv_dirty_bitmap_serialization_size(bitmap, 0, bm_size));
>>>
>>> ...then finds out how many bytes are required to serialize the entire
>>> image, where bm_size should be the same as before...
>>>
 +while ((sector = bdrv_dirty_iter_next(dbi)) != -1) {
 +uint64_t cluster = sector / sbc;
 +uint64_t end, write_size;
 +int64_t off;
 +
 +sector = cluster * sbc;
 +end = MIN(bm_size, sector + sbc);
 +write_size =
 +bdrv_dirty_bitmap_serialization_size(bitmap, sector, end - 
 sector);
>>>
>>> But here, rather than tackling the entire image at once, you are
>>> subdividing your queries along arbitrary lines. But nowhere do I see a
>>> call to bdrv_dirty_bitmap_serialization_align() to make sure your
>>> end-sector value is properly aligned; if it is not aligned, you will
>>> trigger an assertion failure here...
>>
>> It's 4:21 am here, so I cannot claim to be right, but if I remember
>> correctly, it will automatically aligned because sbc is the number of
>> bits (and thus sectors) a bitmap cluster covers.
> 
> Okay, I re-read the spec.  First thing I noticed: we have a potential
> conflict if an image is allowed to be resized:

It isn't, though. (At least currently qemu won't allow it.)

> "All stored bitmaps are related to the virtual disk stored in the same
> image, so each bitmap size is equal to the virtual disk size."
> 
> If you resize an image, does the bitmap size have to be adjusted as
> well?

Sure.

>What if you create one bitmap, then take an internal snapshot,
> then resize?

v3 images store the virtual disk size for each snapshot. So resizing an
image leaves snapshots unaffected.

Now bitmaps are not (yet) tied to snapshots. The spec thus says that the
bitmaps always correspond to the active state (it lists "snapshot
switching" as something that should dirty dirty bitmaps).

Therefore, if you then resize (which currently is doubly forbidden,
because qemu won't allow you to resize images with bitmaps or
snapshots), you would have to resize the bitmap, too.

In the future, we might want to allow binding bitmaps to snapshots. Then
the bitmap would not be resized but you couldn't see it anymore unless
you go back to the snapshot (which would bring the image to its original
size).

>   Or do we declare that (at least for now) the presence of a
> bitmap is incompatible with the use of an internal snapshot?

Well, that's not what the spec says. It clearly lists "snapshot
switching" as something that dirty bitmaps track.

> Conversely, we state that:
> 
> 
> "Structure of a bitmap directory entry:
> ...
>  8 - 11:bitmap_table_size
> Number of entries in the bitmap table of the bitmap."
> 
> Since a bitmap therefore tracks its own size, I think the earlier
> statement that all bitmap sizes are equal to the virtual disk size is
> too strict (there seems to be no technical reason why a bitmap can't
> have a different size that the image).

But a logical. qcow2 is not a container format, it's an image format. I
for one am very opposed to storing bitmaps in a qcow2 file which have no
immediate connection to that virtual disk.

A reason I can see myself supporting would be that you could tie bitmaps
to snapshots and then they can have a size that differs from the one of
the active layer.

Max

> But, having read that, you are correct that we are subdividing our
> bitmaps according to what fits in a qcow2 cluster, and the smallest
> qcow2 cluster we can come up with is 512-bytes (or 4k bits of the
> bitmap).  Checking hbitmap.c, we are merely asserting that our alignment
> is always a multiple of 64 << hb->granularity.  But since we are
> partitioning a cluster at a time, our minimum alignment will be 512 <<
> hb->granularity, which is always aligned.
> 
> So all the more I need to do is add an assertion.
> 
>> I'm for fixing it later. I would have announced the series "applied" an
>> hour ago if I hadn't had to bisect iotest 055 breakage...
> 
> I'm working it into my v4 posting of dirty-bitmap cleanups.
> 




signature.asc
Description: OpenPGP digital signature

Re: [Qemu-devel] [PATCH v2] tests: Avoid non-portable 'echo -ARG'

[Max Reitz]

On 2017-06-30 21:58, Eric Blake wrote:
> POSIX says that backslashes in the arguments to 'echo', as well as
> any use of 'echo -n' and 'echo -e', are non-portable; it recommends
> people should favor 'printf' instead.  This is definitely true where
> we do not control which shell is running (such as in makefile snippets
> or in documentation examples).  But even for scripts where we
> require bash (and therefore, where echo does what we want by default),
> it is still possible to use 'shopt -s xpg_echo' to change bash's
> behavior of echo.  And setting a good example never hurts when we are
> not sure if a snippet will be copied from a bash-only script to a
> general shell script (although I don't change the use of non-portable
> \e for ESC when we know the running shell is bash).
> 
> Replace 'echo -n "..."' with 'printf %s "..."', and 'echo -e "..."'
> with 'printf %b "...\n"', with the optimization that the %s/%b
> argument can be omitted if the string being printed contains no
> substitutions that could result in '%' (trivial if there is no
> $ or `, but also possible when using things like $ret that are
> known to be numeric given the assignment to ret just above).

Well, yes, possible, but it's no longer trivial... And just using '%s'
or '%b' in these cases would make reading the code simpler, in my opinion.

Sure, omitting it makes sense for constant format strings, but for
variable the cost outweighs the benefit, in my opinion.

(And since this is a bit supposed to go through qemu-trivial, it should
be trivial, right? :-))

> In the qemu-iotests check script, fix unusual shell quoting
> that would result in word-splitting if 'date' outputs a space.
> 
> In test 051, take an opportunity to shorten the line.
> 
> In test 068, get rid of a pointless second invocation of bash.
> 
> CC: qemu-triv...@nongnu.org
> Signed-off-by: Eric Blake 
> 
> ---
> v2: be robust to potential % in substitutions [Max], rebase to master,
> shorten some long lines and an odd bash -c use
> 
> Add qemu-trivial in cc, although we may decide this is better through
> Max's block tree since it is mostly iotests related
> 
> ---
>  qemu-options.hx |  4 ++--
>  tests/multiboot/run_test.sh | 10 +-
>  tests/qemu-iotests/051  |  7 ---
>  tests/qemu-iotests/068  |  2 +-
>  tests/qemu-iotests/142  | 48 
> ++---
>  tests/qemu-iotests/171  | 14 ++---
>  tests/qemu-iotests/check| 18 -
>  tests/rocker/all| 10 +-
>  tests/tcg/cris/Makefile |  8 
>  9 files changed, 61 insertions(+), 60 deletions(-)

[...]

> diff --git a/tests/qemu-iotests/check b/tests/qemu-iotests/check
> index 4b1c674..0a13df9 100755
> --- a/tests/qemu-iotests/check
> +++ b/tests/qemu-iotests/check

[...]

> @@ -281,9 +281,9 @@ do
>  rm -f $seq.out.bad
>  lasttime=`sed -n -e "/^$seq /s/.* //p" <$TIMESTAMP_FILE`
>  if [ "X$lasttime" != X ]; then
> -echo -n " ${lasttime}s ..."
> +printf " ${lasttime}s ..."

You cannot prove this doesn't contain a %. In fact, I can very easily
put a % into the timestamp file and let printf print it here.

Sure, there shouldn't be one there, but on one hand it's still possible
and on the other, finding out that there shouldn't be a % there is very
much non-trivial.

>  else
> -echo -n ""# prettier output with timestamps.
> +printf ""# prettier output with timestamps.
>  fi
>  rm -f core $seq.notrun
> 

[...]

> diff --git a/tests/tcg/cris/Makefile b/tests/tcg/cris/Makefile
> index 6b3dba4..6888263 100644
> --- a/tests/tcg/cris/Makefile
> +++ b/tests/tcg/cris/Makefile
> @@ -150,17 +150,17 @@ check_addcv17.tst: crtv10.o sysv10.o
>  build: $(CRT) $(SYS) $(TESTCASES)
> 
>  check: $(CRT) $(SYS) $(TESTCASES)
> - @echo -e "\nQEMU simulator."
> + @printf "\nQEMU simulator.\n"
>   for case in $(TESTCASES); do \
> - echo -n "$$case "; \
> + printf "$$case "; \

This is another rather non-trivial case: Checking that this doesn't
contain a % means reading through the whole list defining TESTCASES.

All in all, I'd really prefer just using %s and %b everywhere there is a
variable format string.

Max

>   SIMARGS=; \
>   case $$case in *v17*) SIMARGS="-cpu crisv17";; esac; \
>   $(SIM) $$SIMARGS ./$$case; \
>   done
>  check-g: $(CRT) $(SYS) $(TESTCASES)
> - @echo -e "\nGDB simulator."
> + @printf "\nGDB simulator.\n"
>   @for case in $(TESTCASES); do \
> - echo -n "$$case "; \
> + printf "$$case "; \
>   $(SIMG) $$case; \
>   done
> 




signature.asc
Description: OpenPGP digital signature

[Qemu-devel] [PATCH v5 1/2] iotests: Use absolute paths for executables

[Max Reitz]

A user may specify a relative path for accessing qemu, qemu-img, etc.
through environment variables ($QEMU_PROG and friends) or a symlink.

If a test decides to change its working directory, relative paths will
cease to work, however. Work around this by making all of the paths to
programs that should undergo testing absolute. Besides "realpath", we
also have to use "type -p" to support programs in $PATH.

As a side effect, this fixes specifying these programs as symlinks for
out-of-tree builds: Before, you would have to create two symlinks, one
in the build and one in the source tree (the first one for common.config
to find, the second one for the iotest to use). Now it is sufficient to
create one in the build tree because common.config will resolve it.

Reported-by: Kevin Wolf 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/common.config | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config
index d1b45f5..e0883a0 100644
--- a/tests/qemu-iotests/common.config
+++ b/tests/qemu-iotests/common.config
@@ -103,6 +103,17 @@ if [ -z "$QEMU_VXHS_PROG" ]; then
 export QEMU_VXHS_PROG="`set_prog_path qnio_server`"
 fi
 
+export QEMU_PROG=$(realpath -- "$(type -p "$QEMU_PROG")")
+export QEMU_IMG_PROG=$(realpath -- "$(type -p "$QEMU_IMG_PROG")")
+export QEMU_IO_PROG=$(realpath -- "$(type -p "$QEMU_IO_PROG")")
+export QEMU_NBD_PROG=$(realpath -- "$(type -p "$QEMU_NBD_PROG")")
+
+# This program is not built as part of qemu but (possibly) provided by the
+# system, so it may not be present at all
+if [ -n "$QEMU_VXHS_PROG" ]; then
+export QEMU_VXHS_PROG=$(realpath -- "$(type -p "$QEMU_VXHS_PROG")")
+fi
+
 _qemu_wrapper()
 {
 (
-- 
2.9.4

[Qemu-devel] [PATCH v5 0/2] iotests: Add test for colon handling

[Max Reitz]

v3 of v3 of "iotests: Add test for colon handling"; thus, basically, v5.

This adds an iotest for the original series "block: Fix backing paths
for filenames with colons" and fixes common.config so it works if you
have specified the qemu binaries through relative paths. As a bonus, it
makes symlinked binaries work for out-of-tree builds.


v5: Don't try to resolve QEMU_VXHS_PROG if it is empty (which it may be)
[Eric]


git-backport-diff against v4:

Key:
[] : patches are identical
[] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/2:[0007] [FC] 'iotests: Use absolute paths for executables'
002/2:[] [--] 'iotests: Add test for colon handling'


Max Reitz (2):
  iotests: Use absolute paths for executables
  iotests: Add test for colon handling

 tests/qemu-iotests/126   | 105 +++
 tests/qemu-iotests/126.out   |  23 +
 tests/qemu-iotests/common.config |  11 
 tests/qemu-iotests/group |   1 +
 4 files changed, 140 insertions(+)
 create mode 100755 tests/qemu-iotests/126
 create mode 100644 tests/qemu-iotests/126.out

-- 
2.9.4

[Qemu-devel] [PATCH v5 2/2] iotests: Add test for colon handling

[Max Reitz]

Reviewed-by: Eric Blake 
Signed-off-by: Max Reitz 
---
 tests/qemu-iotests/126 | 105 +
 tests/qemu-iotests/126.out |  23 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 129 insertions(+)
 create mode 100755 tests/qemu-iotests/126
 create mode 100644 tests/qemu-iotests/126.out

diff --git a/tests/qemu-iotests/126 b/tests/qemu-iotests/126
new file mode 100755
index 000..a2d4d6c
--- /dev/null
+++ b/tests/qemu-iotests/126
@@ -0,0 +1,105 @@
+#!/bin/bash
+#
+# Tests handling of colons in filenames (which may be confused with protocol
+# prefixes)
+#
+# Copyright (C) 2017 Red Hat, Inc.
+#
+# This program 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.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see .
+#
+
+# creator
+owner=mre...@redhat.com
+
+seq="$(basename $0)"
+echo "QA output created by $seq"
+
+here="$PWD"
+status=1   # failure is the default!
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# Needs backing file support
+_supported_fmt qcow qcow2 qed vmdk
+# This is the default protocol (and we want to test the difference between
+# colons which separate a protocol prefix from the rest and colons which are
+# just part of the filename, so we cannot test protocols which require a 
prefix)
+_supported_proto file
+_supported_os Linux
+
+echo
+echo '=== Testing plain files ==='
+echo
+
+# A colon after a slash is not a protocol prefix separator
+TEST_IMG="$TEST_DIR/a:b.$IMGFMT" _make_test_img 64M
+_rm_test_img "$TEST_DIR/a:b.$IMGFMT"
+
+# But if you want to be really sure, you can do this
+TEST_IMG="file:$TEST_DIR/a:b.$IMGFMT" _make_test_img 64M
+_rm_test_img "$TEST_DIR/a:b.$IMGFMT"
+
+
+echo
+echo '=== Testing relative backing filename resolution ==='
+echo
+
+BASE_IMG="$TEST_DIR/image:base.$IMGFMT"
+TOP_IMG="$TEST_DIR/image:top.$IMGFMT"
+
+TEST_IMG=$BASE_IMG _make_test_img 64M
+TEST_IMG=$TOP_IMG _make_test_img -b ./image:base.$IMGFMT
+
+# The default cluster size depends on the image format
+TEST_IMG=$TOP_IMG _img_info | grep -v 'cluster_size'
+
+_rm_test_img "$BASE_IMG"
+_rm_test_img "$TOP_IMG"
+
+
+# Do another test where we access both top and base without any slash in them
+echo
+pushd "$TEST_DIR" >/dev/null
+
+BASE_IMG="base.$IMGFMT"
+TOP_IMG="file:image:top.$IMGFMT"
+
+TEST_IMG=$BASE_IMG _make_test_img 64M
+TEST_IMG=$TOP_IMG _make_test_img -b "$BASE_IMG"
+
+TEST_IMG=$TOP_IMG _img_info | grep -v 'cluster_size'
+
+_rm_test_img "$BASE_IMG"
+_rm_test_img "image:top.$IMGFMT"
+
+popd >/dev/null
+
+# Note that we could also do the same test with 
BASE_IMG=file:image:base.$IMGFMT
+# -- but behavior for that case is a bit strange. Protocol-prefixed paths are
+# in a sense always absolute paths, so such paths will never be combined with
+# the path of the overlay. But since "image:base.$IMGFMT" is actually a
+# relative path, it will always be evaluated relative to qemu's CWD (but not
+# relative to the overlay!). While this is more or less intended, it is still
+# pretty strange and thus not something that is tested here.
+# (The root of the issue is the use of a relative path with a protocol prefix.
+#  This may always give you weird results because in one sense, qemu considers
+#  such paths absolute, whereas in another, they are still relative.)
+
+
+# success, all done
+echo '*** done'
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/126.out b/tests/qemu-iotests/126.out
new file mode 100644
index 000..50d7308
--- /dev/null
+++ b/tests/qemu-iotests/126.out
@@ -0,0 +1,23 @@
+QA output created by 126
+
+=== Testing plain files ===
+
+Formatting 'TEST_DIR/a:b.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/a:b.IMGFMT', fmt=IMGFMT size=67108864
+
+=== Testing relative backing filename resolution ===
+
+Formatting 'TEST_DIR/image:base.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'TEST_DIR/image:top.IMGFMT', fmt=IMGFMT size=67108864 
backing_file=./image:base.IMGFMT
+image: TEST_DIR/image:top.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: ./image:base.IMGFMT (actual path: TEST_DIR/./image:base.IMGFMT)
+
+Formatting 'base.IMGFMT', fmt=IMGFMT size=67108864
+Formatting 'file:image:top.IMGFMT', fmt=IMGFMT size=67108864 
backing_file=base.IMGFMT
+image: ./image:top.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+backing file: base.IMGFMT (actual path: ./base.IMGFMT)
+*** done
diff --git a/tests/qemu-iotests/group b/tes

[Qemu-devel] [PATCH 1/2] target/sh4: do not check for PR bit for fabs instruction

[Aurelien Jarno]

The SH4 manual is not fully clear about that, but real hardware do not
check for the PR bit, which allows to select between single or double
precision, for the fabs instruction. This is probably what is meant by
"Same operation is performed regardless of precision."

Remove the check, and at the same time use a TCG instruction instead of
a helper to clear one bit.

LP: https://bugs.launchpad.net/qemu/+bug/1701821
Reported-by: Bruno Haible 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/helper.h|  2 --
 target/sh4/op_helper.c | 10 --
 target/sh4/translate.c | 15 +++
 3 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index dce859caea..f715224822 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -16,8 +16,6 @@ DEF_HELPER_3(macw, void, env, i32, i32)
 
 DEF_HELPER_2(ld_fpscr, void, env, i32)
 
-DEF_HELPER_FLAGS_1(fabs_FT, TCG_CALL_NO_RWG_SE, f32, f32)
-DEF_HELPER_FLAGS_1(fabs_DT, TCG_CALL_NO_RWG_SE, f64, f64)
 DEF_HELPER_FLAGS_3(fadd_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fadd_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(fcnvsd_FT_DT, TCG_CALL_NO_WG, f64, env, f32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 528a40ac1d..5e3a3ba68c 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -252,16 +252,6 @@ static void update_fpscr(CPUSH4State *env, uintptr_t 
retaddr)
 }
 }
 
-float32 helper_fabs_FT(float32 t0)
-{
-return float32_abs(t0);
-}
-
-float64 helper_fabs_DT(float64 t0)
-{
-return float64_abs(t0);
-}
-
 float32 helper_fadd_FT(CPUSH4State *env, float32 t0, float32 t1)
 {
 set_float_exception_flags(0, &env->fp_status);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 4f20537ef8..7c40945908 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1695,19 +1695,10 @@ static void _decode_opc(DisasContext * ctx)
gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
}
return;
-case 0xf05d: /* fabs FRn/DRn */
+case 0xf05d: /* fabs FRn/DRn - FPCSR: Nothing */
CHECK_FPU_ENABLED
-if (ctx->tbflags & FPSCR_PR) {
-   if (ctx->opcode & 0x0100)
-   break; /* illegal instruction */
-   TCGv_i64 fp = tcg_temp_new_i64();
-   gen_load_fpr64(fp, DREG(B11_8));
-   gen_helper_fabs_DT(fp, fp);
-   gen_store_fpr64(fp, DREG(B11_8));
-   tcg_temp_free_i64(fp);
-   } else {
-   gen_helper_fabs_FT(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
-   }
+tcg_gen_andi_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)],
+ 0x7fff);
return;
 case 0xf06d: /* fsqrt FRn */
CHECK_FPU_ENABLED
-- 
2.11.0

[Qemu-devel] [PATCH 2/2] target/sh4: do not use a helper to implement fneg

[Aurelien Jarno]

There is no need to use a helper to flip one bit, just use a TCG xor
instruction instead.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/helper.h| 1 -
 target/sh4/op_helper.c | 5 -
 target/sh4/translate.c | 5 ++---
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index f715224822..d2398922dd 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -32,7 +32,6 @@ DEF_HELPER_FLAGS_2(float_DT, TCG_CALL_NO_WG, f64, env, i32)
 DEF_HELPER_FLAGS_4(fmac_FT, TCG_CALL_NO_WG, f32, env, f32, f32, f32)
 DEF_HELPER_FLAGS_3(fmul_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fmul_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
-DEF_HELPER_FLAGS_1(fneg_T, TCG_CALL_NO_RWG_SE, f32, f32)
 DEF_HELPER_FLAGS_3(fsub_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fsub_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(fsqrt_FT, TCG_CALL_NO_WG, f32, env, f32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 5e3a3ba68c..d561141301 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -396,11 +396,6 @@ float64 helper_fmul_DT(CPUSH4State *env, float64 t0, 
float64 t1)
 return t0;
 }
 
-float32 helper_fneg_T(float32 t0)
-{
-return float32_chs(t0);
-}
-
 float32 helper_fsqrt_FT(CPUSH4State *env, float32 t0)
 {
 set_float_exception_flags(0, &env->fp_status);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 7c40945908..8098228c51 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1691,9 +1691,8 @@ static void _decode_opc(DisasContext * ctx)
return;
 case 0xf04d: /* fneg FRn/DRn - FPSCR: Nothing */
CHECK_FPU_ENABLED
-   {
-   gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
-   }
+tcg_gen_xori_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)],
+ 0x8000);
return;
 case 0xf05d: /* fabs FRn/DRn - FPCSR: Nothing */
CHECK_FPU_ENABLED
-- 
2.11.0

[Qemu-devel] [PATCH 0/2] target/sh4: fix fabs and optimize fneg

[Aurelien Jarno]

This patchset should fix the bug #1701821 reported by Bruno Haible,
which makes the gnulib testsuite to fail for single precision libm
tests.

Aurelien Jarno (2):
  target/sh4: do not check for PR bit for fabs instruction
  target/sh4: do not use a helper to implement fneg

 target/sh4/helper.h|  3 ---
 target/sh4/op_helper.c | 15 ---
 target/sh4/translate.c | 20 +---
 3 files changed, 5 insertions(+), 33 deletions(-)

-- 
2.11.0

Re: [Qemu-devel] [PATCH 0/2] target/sh4: fix fabs and optimize fneg

[Bruno Haible]

Hi Aurelien,

> This patchset should fix the bug #1701821 reported by Bruno Haible,
> which makes the gnulib testsuite to fail for single precision libm
> tests.
> 
> Aurelien Jarno (2):
>   target/sh4: do not check for PR bit for fabs instruction
>   target/sh4: do not use a helper to implement fneg

Thanks. It fixes most of the reported issues indeed. The following tests
still fail, though:

$ ~/inst-qemu/2.9.0/bin/qemu-sh4 test-floor2
../../gltests/test-floor2.c:130: assertion 'correct_result_p (x, reference)' 
failed
qemu: uncaught target signal 6 (Aborted) - core dumped

$ ~/inst-qemu/2.9.0/bin/qemu-sh4 test-round2
reference implementations disagree: round(nan(nan)) = nan(nan) or 0(0x0p+0)?

$ ~/inst-qemu/2.9.0/bin/qemu-sh4 test-roundf2
reference implementations disagree: roundf(nan(nan)) = nan(nan) or 0(0x0p+0)?

But these could also be glibc bugs in the respective functions; I cannot tell.

Bruno

Re: [Qemu-devel] [Qemu devel v5 PATCH 0/5] Add support for Smartfusion2 SoC

[sundeep subbaraya]

Hi Philippe,

On Mon, Jun 26, 2017 at 9:41 PM, sundeep subbaraya 
wrote:

> Hi Philippe,
>
> On Fri, Jun 9, 2017 at 12:51 PM, sundeep subbaraya  > wrote:
>
>> Hi Philippe,
>>
>> On Wed, May 31, 2017 at 11:06 AM, Philippe Mathieu-Daudé > > wrote:
>>
>>> Hi Sundeep,
>>>
>>> On 05/29/2017 02:28 AM, sundeep subbaraya wrote:
>>>
 Hi Philippe,

 Any update on this? I will wait for your comments too
 and send next iteration fixing Alistair comments.

>>>
>>> Sorry I'm supposed to be in holidays ;)
>>>
>>
>> Ohh sorry currently am in vacation :)
>>
>>>
>>>
 Thanks,
 Sundeep

 On Wed, May 17, 2017 at 3:09 PM, sundeep subbaraya
 mailto:sundeep.l...@gmail.com>> wrote:

 Hi Philippe,

 On Wed, May 17, 2017 at 9:57 AM, Philippe Mathieu-Daudé
 mailto:f4...@amsat.org>> wrote:

 Hi Sundeep,

 This patchset is way cleaner!
 I had a fast look and I like it, I'll try to make some time soon
 to review details and test it.


 Thank you




 Is your work interested on U-Boot or more focused in Linux
 kernel?


 I am interested more in kernel. I had to look into u-boot for first
 time for Qemu only.
 I worked only on FPGAs(load kernel with debugger) till now so never
 got a chance to look into u-boot.


 If you compile QEMU with libfdt support you can use the -dtb
 option to pass the blob to the kernel directly, bypassing the
 bootloader.

 Yeah for armv7m I could not find any thing like that in tree.


 If you need a bootloader you may give a look at coreboot which
 supports dts well, see how Vladimir Serbinenko used Linux's dt
 to boot a QEMU Versatile Express board:
 https://mail.coreboot.org/pipermail/coreboot-gerrit/2016-Feb
 ruary/040899.html
 

 Cool. I will look into it.

 Thanks,
 Sundeep


 Regards,

 Phil.


 On 05/16/2017 12:38 PM, Subbaraya Sundeep wrote:

 Hi Qemu-devel,

 I am trying to add Smartfusion2 SoC.
 SoC is from Microsemi and System on Module(SOM)
 board is from Emcraft systems. Smartfusion2 has hardened
 Microcontroller(Cortex-M3)based Sub System and FPGA fabric.
 At the moment only system timer, sysreg and SPI
 controller are modelled.

 Testing:
 ./arm-softmmu/qemu-system-arm -M smartfusion2-som -serial
 mon:stdio \
 -kernel u-boot.bin -display none -drive
 file=spi.bin,if=mtd,format=raw

>>>
>>> I'm not sure the timer is working correctly, U-Boot loops with this
>>> pattern:
>>>
>>> msf2_sysreg_read: addr: 0x0048 data: 0x0220
>>> msf2_sysreg_write: addr: 0x0048 data: 0x0220
>>> msf2_sysreg_read: addr: 0x0048 data: 0x0220
>>> msf2_sysreg_write: addr: 0x0048 data: 0x0020
>>> msf2_sysreg_read: addr: 0x0048 data: 0x0020
>>> msf2_sysreg_write: addr: 0x0048 data: 0x
>>> msf2_sysreg_read: addr: 0x0048 data: 0x
>>> msf2_sysreg_write: addr: 0x0048 data: 0x0020
>>> msf2_sysreg_read: addr: 0x0048 data: 0x0020
>>> msf2_sysreg_write: addr: 0x0048 data: 0x0220
>>>
>>> I checked the images and Linux is booting. But as you mentioned I
> changed u-boot
> for boot delay and have seen this issue. Actually it is taking too long
> for a second.
> Smartfusion2 timer is working fine(Linux) whereas u-boot is using Systick
> for auto-boot
> timer. I did not understand quite correctly about ARM Systick in Qemu. How
> do we
> specify frequency of the Systick timer? How Systick is configured to use
> CPU frequency
> since qemu cpu speed is not constant? How frequency has to be specified
> for
> using external clock as Systick input?
>

I figured out that systick uses cpu clock as clock source and
system_clock_scale
need to be set in msf2-soc.c. There is a bug in u-boot where it uses cpu
clock as
systick input but configures systick in external clock mode. I have tested
the modified
u-boot on real hardware too and it works fine. I am calculating
system_clock_scale
as below:
If CPU clock is X MHz then system_clock_scale = (1 / X) * 1000

Tested with different frequencies and they are yielding same results.

Please correct me if am wrong. I will send next iteration of patches.

Thanks,
Sundeep

>
> Please help me understand this.
>
> Thanks,
> Sundeep
>
>
>>
 Binaries u-boot.bin and spi.bin are at:

>>>
>>> you can compress spi.bin!
>>>
>>> can you share u-boot.elf with debug symbols too?
>>>
>>
>> Sure. I have tes

[Qemu-devel] [PATCH v2 4/5] target/sh4: do not use a helper to implement fneg

[Aurelien Jarno]

There is no need to use a helper to flip one bit, just use a TCG xor
instruction instead.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/helper.h| 1 -
 target/sh4/op_helper.c | 5 -
 target/sh4/translate.c | 5 ++---
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index f715224822..d2398922dd 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -32,7 +32,6 @@ DEF_HELPER_FLAGS_2(float_DT, TCG_CALL_NO_WG, f64, env, i32)
 DEF_HELPER_FLAGS_4(fmac_FT, TCG_CALL_NO_WG, f32, env, f32, f32, f32)
 DEF_HELPER_FLAGS_3(fmul_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fmul_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
-DEF_HELPER_FLAGS_1(fneg_T, TCG_CALL_NO_RWG_SE, f32, f32)
 DEF_HELPER_FLAGS_3(fsub_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fsub_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(fsqrt_FT, TCG_CALL_NO_WG, f32, env, f32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index f2e39c5ca6..64206cf803 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -384,11 +384,6 @@ float64 helper_fmul_DT(CPUSH4State *env, float64 t0, 
float64 t1)
 return t0;
 }
 
-float32 helper_fneg_T(float32 t0)
-{
-return float32_chs(t0);
-}
-
 float32 helper_fsqrt_FT(CPUSH4State *env, float32 t0)
 {
 set_float_exception_flags(0, &env->fp_status);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 7c40945908..8098228c51 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1691,9 +1691,8 @@ static void _decode_opc(DisasContext * ctx)
return;
 case 0xf04d: /* fneg FRn/DRn - FPSCR: Nothing */
CHECK_FPU_ENABLED
-   {
-   gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
-   }
+tcg_gen_xori_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)],
+ 0x8000);
return;
 case 0xf05d: /* fabs FRn/DRn - FPCSR: Nothing */
CHECK_FPU_ENABLED
-- 
2.11.0

[Qemu-devel] [PATCH v2 0/5] target/sh4: misc FPU fixes and optimizations

[Aurelien Jarno]

This patchset should fix the bug#1701821 reported by Bruno Haible,
which makes the gnulib testsuite to fail for single precision libm
tests or for tests relying on unordered comparisons.

It also fixes an inversion of cause and flag bits in the FPSCR register,
which is unrelated with the reported bug. It also improves a bit the fneg
and fcmp instructions.

Aurelien Jarno (5):
  target/sh4: do not check for PR bit for fabs instruction
  target/sh4: fix FPU unorderered compare
  target/sh4: fix FPSCR cause vs flag inversion
  target/sh4: do not use a helper to implement fneg
  target/sh4: return result of fcmp using TCG

 target/sh4/helper.h| 11 +++-
 target/sh4/op_helper.c | 71 --
 target/sh4/translate.c | 30 -
 3 files changed, 37 insertions(+), 75 deletions(-)

-- 
2.11.0

[Qemu-devel] [PATCH v2 3/5] target/sh4: fix FPSCR cause vs flag inversion

[Aurelien Jarno]

The floating-point status/control register contains cause and flag
bits. The cause bits are set to 0 before executing the instruction,
while the flag bits hold the status of the exception generated after
the field was last cleared.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/op_helper.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index f228daf125..f2e39c5ca6 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -219,29 +219,29 @@ static void update_fpscr(CPUSH4State *env, uintptr_t 
retaddr)
 
 xcpt = get_float_exception_flags(&env->fp_status);
 
-/* Clear the flag entries */
-env->fpscr &= ~FPSCR_FLAG_MASK;
+/* Clear the cause entries */
+env->fpscr &= ~FPSCR_CAUSE_MASK;
 
 if (unlikely(xcpt)) {
 if (xcpt & float_flag_invalid) {
-env->fpscr |= FPSCR_FLAG_V;
+env->fpscr |= FPSCR_CAUSE_V;
 }
 if (xcpt & float_flag_divbyzero) {
-env->fpscr |= FPSCR_FLAG_Z;
+env->fpscr |= FPSCR_CAUSE_Z;
 }
 if (xcpt & float_flag_overflow) {
-env->fpscr |= FPSCR_FLAG_O;
+env->fpscr |= FPSCR_CAUSE_O;
 }
 if (xcpt & float_flag_underflow) {
-env->fpscr |= FPSCR_FLAG_U;
+env->fpscr |= FPSCR_CAUSE_U;
 }
 if (xcpt & float_flag_inexact) {
-env->fpscr |= FPSCR_FLAG_I;
+env->fpscr |= FPSCR_CAUSE_I;
 }
 
-/* Accumulate in cause entries */
-env->fpscr |= (env->fpscr & FPSCR_FLAG_MASK)
-  << (FPSCR_CAUSE_SHIFT - FPSCR_FLAG_SHIFT);
+/* Accumulate in flag entries */
+env->fpscr |= (env->fpscr & FPSCR_CAUSE_MASK)
+  >> (FPSCR_CAUSE_SHIFT - FPSCR_FLAG_SHIFT);
 
 /* Generate an exception if enabled */
 cause = (env->fpscr & FPSCR_CAUSE_MASK) >> FPSCR_CAUSE_SHIFT;
-- 
2.11.0

[Qemu-devel] [PATCH v2 5/5] target/sh4: return result of fcmp using TCG

[Aurelien Jarno]

Since that the T bit of the SR register is mapped using a TGC global,
it's better to return the value through TCG than writing it directly. It
allows to declare the helpers with the flag TCG_CALL_NO_WG.

Signed-off-by: Aurelien Jarno 
---
 target/sh4/helper.h|  8 
 target/sh4/op_helper.c | 16 
 target/sh4/translate.c | 10 ++
 3 files changed, 18 insertions(+), 16 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index d2398922dd..767a6d5209 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -21,10 +21,10 @@ DEF_HELPER_FLAGS_3(fadd_DT, TCG_CALL_NO_WG, f64, env, f64, 
f64)
 DEF_HELPER_FLAGS_2(fcnvsd_FT_DT, TCG_CALL_NO_WG, f64, env, f32)
 DEF_HELPER_FLAGS_2(fcnvds_DT_FT, TCG_CALL_NO_WG, f32, env, f64)
 
-DEF_HELPER_3(fcmp_eq_FT, void, env, f32, f32)
-DEF_HELPER_3(fcmp_eq_DT, void, env, f64, f64)
-DEF_HELPER_3(fcmp_gt_FT, void, env, f32, f32)
-DEF_HELPER_3(fcmp_gt_DT, void, env, f64, f64)
+DEF_HELPER_FLAGS_3(fcmp_eq_FT, TCG_CALL_NO_WG, i32, env, f32, f32)
+DEF_HELPER_FLAGS_3(fcmp_eq_DT, TCG_CALL_NO_WG, i32, env, f64, f64)
+DEF_HELPER_FLAGS_3(fcmp_gt_FT, TCG_CALL_NO_WG, i32, env, f32, f32)
+DEF_HELPER_FLAGS_3(fcmp_gt_DT, TCG_CALL_NO_WG, i32, env, f64, f64)
 DEF_HELPER_FLAGS_3(fdiv_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fdiv_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(float_FT, TCG_CALL_NO_WG, f32, env, i32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 64206cf803..c3d19b1f61 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -268,44 +268,44 @@ float64 helper_fadd_DT(CPUSH4State *env, float64 t0, 
float64 t1)
 return t0;
 }
 
-void helper_fcmp_eq_FT(CPUSH4State *env, float32 t0, float32 t1)
+uint32_t helper_fcmp_eq_FT(CPUSH4State *env, float32 t0, float32 t1)
 {
 int relation;
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float32_compare(t0, t1, &env->fp_status);
 update_fpscr(env, GETPC());
-env->sr_t = (relation == float_relation_equal);
+return relation == float_relation_equal;
 }
 
-void helper_fcmp_eq_DT(CPUSH4State *env, float64 t0, float64 t1)
+uint32_t helper_fcmp_eq_DT(CPUSH4State *env, float64 t0, float64 t1)
 {
 int relation;
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float64_compare(t0, t1, &env->fp_status);
 update_fpscr(env, GETPC());
-env->sr_t = (relation == float_relation_equal);
+return relation == float_relation_equal;
 }
 
-void helper_fcmp_gt_FT(CPUSH4State *env, float32 t0, float32 t1)
+uint32_t helper_fcmp_gt_FT(CPUSH4State *env, float32 t0, float32 t1)
 {
 int relation;
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float32_compare(t0, t1, &env->fp_status);
 update_fpscr(env, GETPC());
-env->sr_t = (relation == float_relation_greater);
+return relation == float_relation_greater;
 }
 
-void helper_fcmp_gt_DT(CPUSH4State *env, float64 t0, float64 t1)
+uint32_t helper_fcmp_gt_DT(CPUSH4State *env, float64 t0, float64 t1)
 {
 int relation;
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float64_compare(t0, t1, &env->fp_status);
 update_fpscr(env, GETPC());
-env->sr_t = (relation == float_relation_greater);
+return relation == float_relation_greater;
 }
 
 float64 helper_fcnvsd_FT_DT(CPUSH4State *env, float32 t0)
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 8098228c51..87b04f0d39 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1077,10 +1077,10 @@ static void _decode_opc(DisasContext * ctx)
 gen_helper_fdiv_DT(fp0, cpu_env, fp0, fp1);
 break;
 case 0xf004:   /* fcmp/eq Rm,Rn */
-gen_helper_fcmp_eq_DT(cpu_env, fp0, fp1);
+gen_helper_fcmp_eq_DT(cpu_sr_t, cpu_env, fp0, fp1);
 return;
 case 0xf005:   /* fcmp/gt Rm,Rn */
-gen_helper_fcmp_gt_DT(cpu_env, fp0, fp1);
+gen_helper_fcmp_gt_DT(cpu_sr_t, cpu_env, fp0, fp1);
 return;
 }
gen_store_fpr64(fp0, DREG(B11_8));
@@ -1109,11 +1109,13 @@ static void _decode_opc(DisasContext * ctx)
cpu_fregs[FREG(B7_4)]);
 break;
 case 0xf004:   /* fcmp/eq Rm,Rn */
-gen_helper_fcmp_eq_FT(cpu_env, cpu_fregs[FREG(B11_8)],
+gen_helper_fcmp_eq_FT(cpu_sr_t, cpu_env,
+  cpu_fregs[FREG(B11_8)],
   cpu_fregs[FREG(B7_4)]);
 return;
 case 0xf005:   /* fcmp/gt Rm,Rn */
-gen_helper_fcmp_gt_FT(cpu_env, cpu_fregs[FREG(B11_8)],
+gen_helper_fcmp_gt_FT(cpu_sr_t, cpu_env,
+  cpu_fregs[FREG(

[Qemu-devel] [PATCH v2 2/5] target/sh4: fix FPU unorderered compare

[Aurelien Jarno]

In case of unordered compare, the fcmp instructions should either
trigger and invalid exception (if enabled) or set T=0. The existing code
left it unchanged.

LP: https://bugs.launchpad.net/qemu/+bug/1701821
Reported-by: Bruno Haible 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/op_helper.c | 28 
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 5e3a3ba68c..f228daf125 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -274,11 +274,8 @@ void helper_fcmp_eq_FT(CPUSH4State *env, float32 t0, 
float32 t1)
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float32_compare(t0, t1, &env->fp_status);
-if (unlikely(relation == float_relation_unordered)) {
-update_fpscr(env, GETPC());
-} else {
-env->sr_t = (relation == float_relation_equal);
-}
+update_fpscr(env, GETPC());
+env->sr_t = (relation == float_relation_equal);
 }
 
 void helper_fcmp_eq_DT(CPUSH4State *env, float64 t0, float64 t1)
@@ -287,11 +284,8 @@ void helper_fcmp_eq_DT(CPUSH4State *env, float64 t0, 
float64 t1)
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float64_compare(t0, t1, &env->fp_status);
-if (unlikely(relation == float_relation_unordered)) {
-update_fpscr(env, GETPC());
-} else {
-env->sr_t = (relation == float_relation_equal);
-}
+update_fpscr(env, GETPC());
+env->sr_t = (relation == float_relation_equal);
 }
 
 void helper_fcmp_gt_FT(CPUSH4State *env, float32 t0, float32 t1)
@@ -300,11 +294,8 @@ void helper_fcmp_gt_FT(CPUSH4State *env, float32 t0, 
float32 t1)
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float32_compare(t0, t1, &env->fp_status);
-if (unlikely(relation == float_relation_unordered)) {
-update_fpscr(env, GETPC());
-} else {
-env->sr_t = (relation == float_relation_greater);
-}
+update_fpscr(env, GETPC());
+env->sr_t = (relation == float_relation_greater);
 }
 
 void helper_fcmp_gt_DT(CPUSH4State *env, float64 t0, float64 t1)
@@ -313,11 +304,8 @@ void helper_fcmp_gt_DT(CPUSH4State *env, float64 t0, 
float64 t1)
 
 set_float_exception_flags(0, &env->fp_status);
 relation = float64_compare(t0, t1, &env->fp_status);
-if (unlikely(relation == float_relation_unordered)) {
-update_fpscr(env, GETPC());
-} else {
-env->sr_t = (relation == float_relation_greater);
-}
+update_fpscr(env, GETPC());
+env->sr_t = (relation == float_relation_greater);
 }
 
 float64 helper_fcnvsd_FT_DT(CPUSH4State *env, float32 t0)
-- 
2.11.0

[Qemu-devel] [PATCH v2 1/5] target/sh4: do not check for PR bit for fabs instruction

[Aurelien Jarno]

The SH4 manual is not fully clear about that, but real hardware do not
check for the PR bit, which allows to select between single or double
precision, for the fabs instruction. This is probably what is meant by
"Same operation is performed regardless of precision."

Remove the check, and at the same time use a TCG instruction instead of
a helper to clear one bit.

LP: https://bugs.launchpad.net/qemu/+bug/1701821
Reported-by: Bruno Haible 
Signed-off-by: Aurelien Jarno 
---
 target/sh4/helper.h|  2 --
 target/sh4/op_helper.c | 10 --
 target/sh4/translate.c | 15 +++
 3 files changed, 3 insertions(+), 24 deletions(-)

diff --git a/target/sh4/helper.h b/target/sh4/helper.h
index dce859caea..f715224822 100644
--- a/target/sh4/helper.h
+++ b/target/sh4/helper.h
@@ -16,8 +16,6 @@ DEF_HELPER_3(macw, void, env, i32, i32)
 
 DEF_HELPER_2(ld_fpscr, void, env, i32)
 
-DEF_HELPER_FLAGS_1(fabs_FT, TCG_CALL_NO_RWG_SE, f32, f32)
-DEF_HELPER_FLAGS_1(fabs_DT, TCG_CALL_NO_RWG_SE, f64, f64)
 DEF_HELPER_FLAGS_3(fadd_FT, TCG_CALL_NO_WG, f32, env, f32, f32)
 DEF_HELPER_FLAGS_3(fadd_DT, TCG_CALL_NO_WG, f64, env, f64, f64)
 DEF_HELPER_FLAGS_2(fcnvsd_FT_DT, TCG_CALL_NO_WG, f64, env, f32)
diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c
index 528a40ac1d..5e3a3ba68c 100644
--- a/target/sh4/op_helper.c
+++ b/target/sh4/op_helper.c
@@ -252,16 +252,6 @@ static void update_fpscr(CPUSH4State *env, uintptr_t 
retaddr)
 }
 }
 
-float32 helper_fabs_FT(float32 t0)
-{
-return float32_abs(t0);
-}
-
-float64 helper_fabs_DT(float64 t0)
-{
-return float64_abs(t0);
-}
-
 float32 helper_fadd_FT(CPUSH4State *env, float32 t0, float32 t1)
 {
 set_float_exception_flags(0, &env->fp_status);
diff --git a/target/sh4/translate.c b/target/sh4/translate.c
index 4f20537ef8..7c40945908 100644
--- a/target/sh4/translate.c
+++ b/target/sh4/translate.c
@@ -1695,19 +1695,10 @@ static void _decode_opc(DisasContext * ctx)
gen_helper_fneg_T(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
}
return;
-case 0xf05d: /* fabs FRn/DRn */
+case 0xf05d: /* fabs FRn/DRn - FPCSR: Nothing */
CHECK_FPU_ENABLED
-if (ctx->tbflags & FPSCR_PR) {
-   if (ctx->opcode & 0x0100)
-   break; /* illegal instruction */
-   TCGv_i64 fp = tcg_temp_new_i64();
-   gen_load_fpr64(fp, DREG(B11_8));
-   gen_helper_fabs_DT(fp, fp);
-   gen_store_fpr64(fp, DREG(B11_8));
-   tcg_temp_free_i64(fp);
-   } else {
-   gen_helper_fabs_FT(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)]);
-   }
+tcg_gen_andi_i32(cpu_fregs[FREG(B11_8)], cpu_fregs[FREG(B11_8)],
+ 0x7fff);
return;
 case 0xf06d: /* fsqrt FRn */
CHECK_FPU_ENABLED
-- 
2.11.0

Re: [Qemu-devel] [Qemu devel v5 PATCH 0/5] Add support for Smartfusion2 SoC

[Peter Maydell]

On 2 July 2017 at 18:39, sundeep subbaraya  wrote:
> I figured out that systick uses cpu clock as clock source and
> system_clock_scale
> need to be set in msf2-soc.c. There is a bug in u-boot where it uses cpu
> clock as
> systick input but configures systick in external clock mode. I have tested
> the modified
> u-boot on real hardware too and it works fine. I am calculating
> system_clock_scale
> as below:
> If CPU clock is X MHz then system_clock_scale = (1 / X) * 1000
>
> Tested with different frequencies and they are yielding same results.

If you calculate it like that you'll probably get rounding
errors. Better is
  system_clock_scale = NANOSECONDS_PER_SECOND / freq_in_hz;

(Our systick implementation hardwires the external clock
frequency at 1MHz, but this is not really correct, it
depends on the SoC.)

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2 0/5] target/sh4: misc FPU fixes and optimizations

[Bruno Haible]

Aurelien Jarno wrote:
> This patchset should fix the bug#1701821 reported by Bruno Haible,
> which makes the gnulib testsuite to fail for single precision libm
> tests or for tests relying on unordered comparisons.

It does fix it: All floating-point related tests of gnulib now pass with
qemu-sh4. Thanks!

Bruno

[Qemu-devel] [Bug 1701971] Re: multithreading not working right under qemu user mode for sh4

[Bruno Haible]

** Attachment added: "Compiled test program"
   
https://bugs.launchpad.net/qemu/+bug/1701971/+attachment/4908118/+files/test-tls

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

Title:
  multithreading not working right under qemu user mode for sh4

Status in QEMU:
  New

Bug description:
  In a multithreaded program running under qemu-sh4 (version 2.9.0),
  thread termination and/or pthread_join is not working right.

  The attached program works natively on all kinds of platforms, and
  under qemu user mode emulation for at least alpha, armelhf, aarch64,
  powerpc64le.

  How to reproduce:
  - Compile the program: sh4-linux-gnu-gcc-5 -O -Wall -lpthread -o test-tls 
test-tls.c
  - Set environment variables for running qemu-sh4.
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-tls

  Expected behaviour: After the "Worker x dying" line, the main()
  function prints "OK", and the program terminates.

  Actual behaviour (only on sh4): After the "Worker x dying" line, it 
hangs. Attaching gdb to qemu shows 15 threads with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=128, 
val=val@entry=2, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161181992) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161181992, uaddr2=4134624624, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-160342672, 
  arg6=-161181992, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
  #4  0x5584f86f454a in cpu_loop (env=env@entry=0x5584fb3d04f8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
  #5  0x5584f86f5dd5 in clone_func (arg=0x7fff4d485c20) at 
/build/qemu-2.9.0/linux-user/syscall.c:6234
  #6  0x7f08f05a46ba in start_thread (arg=0x7f08f1368700) at 
pthread_create.c:333
  #7  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

  and 1 thread with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=0, 
val=val@entry=18875, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161180376) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161180376, uaddr2=4135101768, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-159865528, 
  arg6=-161180376, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
  #4  0x5584f86f454a in cpu_loop (env=0x5584fb3b99a8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
  #5  0x5584f86c12d3 in main (argc=, argv=0x7fff4d4878b8, 
envp=)
  at /build/qemu-2.9.0/linux-user/main.c:4860

  and 1 thread with a stack trace like
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  #1  0x5584f876eab5 in qemu_futex_wait (val=, f=) at /build/qemu-2.9.0/include/qemu/futex.h:26
  #2  qemu_event_wait (ev=ev@entry=0x5584faa43d84 ) at 
/build/qemu-2.9.0/util/qemu-thread-posix.c:399
  #3  0x5584f87748ce in call_rcu_thread (opaque=) at 
/build/qemu-2.9.0/util/rcu.c:249
  #4  0x7f08f05a46ba in start_thread (arg=0x7f08eff62700) at 
pthread_create.c:333
  #5  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701971/+subscriptions

[Qemu-devel] [Bug 1701971] [NEW] multithreading not working right under qemu user mode for sh4

[Bruno Haible]

Public bug reported:

In a multithreaded program running under qemu-sh4 (version 2.9.0),
thread termination and/or pthread_join is not working right.

The attached program works natively on all kinds of platforms, and under
qemu user mode emulation for at least alpha, armelhf, aarch64,
powerpc64le.

How to reproduce:
- Compile the program: sh4-linux-gnu-gcc-5 -O -Wall -lpthread -o test-tls 
test-tls.c
- Set environment variables for running qemu-sh4.
- ~/inst-qemu/2.9.0/bin/qemu-sh4 test-tls

Expected behaviour: After the "Worker x dying" line, the main()
function prints "OK", and the program terminates.

Actual behaviour (only on sh4): After the "Worker x dying" line, it hangs. 
Attaching gdb to qemu shows 15 threads with a stack trace like
#0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
#1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=128, 
val=val@entry=2, timeout=, uaddr2=uaddr2@entry=0x0, 
val3=val3@entry=-161181992) at /build/qemu-2.9.0/linux-user/syscall.c:921
#2  0x5584f870353b in do_futex (val3=-161181992, uaddr2=4134624624, 
timeout=0, val=, op=, uaddr=)
at /build/qemu-2.9.0/linux-user/syscall.c:7147
#3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-160342672, 
arg6=-161181992, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
#4  0x5584f86f454a in cpu_loop (env=env@entry=0x5584fb3d04f8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
#5  0x5584f86f5dd5 in clone_func (arg=0x7fff4d485c20) at 
/build/qemu-2.9.0/linux-user/syscall.c:6234
#6  0x7f08f05a46ba in start_thread (arg=0x7f08f1368700) at 
pthread_create.c:333
#7  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

and 1 thread with a stack trace like
#0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
#1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=0, 
val=val@entry=18875, timeout=, uaddr2=uaddr2@entry=0x0, 
val3=val3@entry=-161180376) at /build/qemu-2.9.0/linux-user/syscall.c:921
#2  0x5584f870353b in do_futex (val3=-161180376, uaddr2=4135101768, 
timeout=0, val=, op=, uaddr=)
at /build/qemu-2.9.0/linux-user/syscall.c:7147
#3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-159865528, 
arg6=-161180376, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
#4  0x5584f86f454a in cpu_loop (env=0x5584fb3b99a8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
#5  0x5584f86c12d3 in main (argc=, argv=0x7fff4d4878b8, 
envp=)
at /build/qemu-2.9.0/linux-user/main.c:4860

and 1 thread with a stack trace like
#0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
#1  0x5584f876eab5 in qemu_futex_wait (val=, f=) at /build/qemu-2.9.0/include/qemu/futex.h:26
#2  qemu_event_wait (ev=ev@entry=0x5584faa43d84 ) at 
/build/qemu-2.9.0/util/qemu-thread-posix.c:399
#3  0x5584f87748ce in call_rcu_thread (opaque=) at 
/build/qemu-2.9.0/util/rcu.c:249
#4  0x7f08f05a46ba in start_thread (arg=0x7f08eff62700) at 
pthread_create.c:333
#5  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "source code of test program"
   https://bugs.launchpad.net/bugs/1701971/+attachment/4908117/+files/test-tls.c

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

Title:
  multithreading not working right under qemu user mode for sh4

Status in QEMU:
  New

Bug description:
  In a multithreaded program running under qemu-sh4 (version 2.9.0),
  thread termination and/or pthread_join is not working right.

  The attached program works natively on all kinds of platforms, and
  under qemu user mode emulation for at least alpha, armelhf, aarch64,
  powerpc64le.

  How to reproduce:
  - Compile the program: sh4-linux-gnu-gcc-5 -O -Wall -lpthread -o test-tls 
test-tls.c
  - Set environment variables for running qemu-sh4.
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-tls

  Expected behaviour: After the "Worker x dying" line, the main()
  function prints "OK", and the program terminates.

  Actual behaviour (only on sh4): After the "Worker x dying" line, it 
hangs. Attaching gdb to qemu shows 15 threads with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=128, 
val=val@entry=2, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161181992) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161181992, uaddr2=4134624624, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-160342672, 
  arg6=-161181992, arg7=0, arg8=0) at 
/build/q

[Qemu-devel] [Bug 1701971] Re: multithreading not working right under qemu user mode for sh4

[Bruno Haible]

Another gnulib test (test-lock) fails with an assertion inside glibc:
test-lock: pthread_mutex_lock.c:81: __pthread_mutex_lock: Assertion 
`mutex->__data.__owner == 0' failed.
qemu: uncaught target signal 6 (Aborted) - core dumped

Based on this, I would speculate that the problem is in the qemu
emulation of the Linux system calls that glibc uses for multithreading
or in the implementation of the primitives used by qemu_event_wait().

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

Title:
  multithreading not working right under qemu user mode for sh4

Status in QEMU:
  New

Bug description:
  In a multithreaded program running under qemu-sh4 (version 2.9.0),
  thread termination and/or pthread_join is not working right.

  The attached program works natively on all kinds of platforms, and
  under qemu user mode emulation for at least alpha, armelhf, aarch64,
  powerpc64le.

  How to reproduce:
  - Compile the program: sh4-linux-gnu-gcc-5 -O -Wall -lpthread -o test-tls 
test-tls.c
  - Set environment variables for running qemu-sh4.
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-tls

  Expected behaviour: After the "Worker x dying" line, the main()
  function prints "OK", and the program terminates.

  Actual behaviour (only on sh4): After the "Worker x dying" line, it 
hangs. Attaching gdb to qemu shows 15 threads with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=128, 
val=val@entry=2, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161181992) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161181992, uaddr2=4134624624, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-160342672, 
  arg6=-161181992, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
  #4  0x5584f86f454a in cpu_loop (env=env@entry=0x5584fb3d04f8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
  #5  0x5584f86f5dd5 in clone_func (arg=0x7fff4d485c20) at 
/build/qemu-2.9.0/linux-user/syscall.c:6234
  #6  0x7f08f05a46ba in start_thread (arg=0x7f08f1368700) at 
pthread_create.c:333
  #7  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

  and 1 thread with a stack trace like
  #0  safe_syscall_base () at 
/build/qemu-2.9.0/linux-user/host/x86_64/safe-syscall.inc.S:75
  #1  0x5584f86f4c48 in safe_futex (uaddr=, op=op@entry=0, 
val=val@entry=18875, timeout=, uaddr2=uaddr2@entry=0x0, 
  val3=val3@entry=-161180376) at /build/qemu-2.9.0/linux-user/syscall.c:921
  #2  0x5584f870353b in do_futex (val3=-161180376, uaddr2=4135101768, 
timeout=0, val=, op=, uaddr=)
  at /build/qemu-2.9.0/linux-user/syscall.c:7147
  #3  do_syscall (cpu_env=, num=240, arg1=, 
arg2=, arg3=, arg4=0, arg5=-159865528, 
  arg6=-161180376, arg7=0, arg8=0) at 
/build/qemu-2.9.0/linux-user/syscall.c:11692
  #4  0x5584f86f454a in cpu_loop (env=0x5584fb3b99a8) at 
/build/qemu-2.9.0/linux-user/main.c:2676
  #5  0x5584f86c12d3 in main (argc=, argv=0x7fff4d4878b8, 
envp=)
  at /build/qemu-2.9.0/linux-user/main.c:4860

  and 1 thread with a stack trace like
  #0  syscall () at ../sysdeps/unix/sysv/linux/x86_64/syscall.S:38
  #1  0x5584f876eab5 in qemu_futex_wait (val=, f=) at /build/qemu-2.9.0/include/qemu/futex.h:26
  #2  qemu_event_wait (ev=ev@entry=0x5584faa43d84 ) at 
/build/qemu-2.9.0/util/qemu-thread-posix.c:399
  #3  0x5584f87748ce in call_rcu_thread (opaque=) at 
/build/qemu-2.9.0/util/rcu.c:249
  #4  0x7f08f05a46ba in start_thread (arg=0x7f08eff62700) at 
pthread_create.c:333
  #5  0x7f08f02da3dd in clone () at 
../sysdeps/unix/sysv/linux/x86_64/clone.S:109

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701971/+subscriptions

[Qemu-devel] [Bug 1701973] [NEW] pread does not work right under qemu-sh4

[Bruno Haible]

Public bug reported:

The pread system call returns a wrong value in some case, in a program
running under qemu-sh4 (version 2.9.0).

How to reproduce:
- Compile the program:
  sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pread test-pread.c
- Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
- ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pread

Expected output:
ret=1 errno=0

Actual output:
ret=0 errno=2
test-pread.c:44: assertion 'ret == 1' failed
qemu: uncaught target signal 6 (Aborted) - core dumped

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "source code of test program"
   
https://bugs.launchpad.net/bugs/1701973/+attachment/4908129/+files/test-pread.c

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

Title:
  pread does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pread system call returns a wrong value in some case, in a program
  running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pread test-pread.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pread

  Expected output:
  ret=1 errno=0

  Actual output:
  ret=0 errno=2
  test-pread.c:44: assertion 'ret == 1' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701973/+subscriptions

[Qemu-devel] [Bug 1701973] Re: pread does not work right under qemu-sh4

[Bruno Haible]

** Attachment added: "Statically linked compiled test program"
   
https://bugs.launchpad.net/qemu/+bug/1701973/+attachment/4908130/+files/test-pread

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

Title:
  pread does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pread system call returns a wrong value in some case, in a program
  running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pread test-pread.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pread

  Expected output:
  ret=1 errno=0

  Actual output:
  ret=0 errno=2
  test-pread.c:44: assertion 'ret == 1' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701973/+subscriptions

[Qemu-devel] [Bug 1701974] Re: pwrite does not work right under qemu-sh4

[Bruno Haible]

In case it matters: My host platform is Linux/x86_64.

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

Title:
  pwrite does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pwrite system call has no effect when writing to a non-zero file
  position, in a program running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pwrite test-pwrite.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pwrite

  Expected output:
  buf = 01W3456789

  Actual output:
  buf = 0123456789
  test-pwrite.c:56: assertion 'strcmp ("01W3456789",buf) == 0' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701974/+subscriptions

[Qemu-devel] [Bug 1701974] Re: pwrite does not work right under qemu-sh4

[Bruno Haible]

** Attachment added: "Statically linked compiled test program"
   
https://bugs.launchpad.net/qemu/+bug/1701974/+attachment/4908132/+files/test-pwrite

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

Title:
  pwrite does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pwrite system call has no effect when writing to a non-zero file
  position, in a program running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pwrite test-pwrite.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pwrite

  Expected output:
  buf = 01W3456789

  Actual output:
  buf = 0123456789
  test-pwrite.c:56: assertion 'strcmp ("01W3456789",buf) == 0' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701974/+subscriptions

[Qemu-devel] [Bug 1701973] Re: pread does not work right under qemu-sh4

[Bruno Haible]

In case it matters: My host platform is Linux/x86_64.

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

Title:
  pread does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pread system call returns a wrong value in some case, in a program
  running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pread test-pread.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pread

  Expected output:
  ret=1 errno=0

  Actual output:
  ret=0 errno=2
  test-pread.c:44: assertion 'ret == 1' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701973/+subscriptions

[Qemu-devel] [Bug 1701974] [NEW] pwrite does not work right under qemu-sh4

[Bruno Haible]

Public bug reported:

The pwrite system call has no effect when writing to a non-zero file
position, in a program running under qemu-sh4 (version 2.9.0).

How to reproduce:
- Compile the program:
  sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pwrite test-pwrite.c
- Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
- ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pwrite

Expected output:
buf = 01W3456789

Actual output:
buf = 0123456789
test-pwrite.c:56: assertion 'strcmp ("01W3456789",buf) == 0' failed
qemu: uncaught target signal 6 (Aborted) - core dumped

** Affects: qemu
 Importance: Undecided
 Status: New

** Attachment added: "source code of test program"
   
https://bugs.launchpad.net/bugs/1701974/+attachment/4908131/+files/test-pwrite.c

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

Title:
  pwrite does not work right under qemu-sh4

Status in QEMU:
  New

Bug description:
  The pwrite system call has no effect when writing to a non-zero file
  position, in a program running under qemu-sh4 (version 2.9.0).

  How to reproduce:
  - Compile the program:
sh4-linux-gnu-gcc-5 -O -Wall -static -o test-pwrite test-pwrite.c
  - Set environment variable for using qemu-sh4 (actually not needed, since the 
program is statically linked here).
  - ~/inst-qemu/2.9.0/bin/qemu-sh4 test-pwrite

  Expected output:
  buf = 01W3456789

  Actual output:
  buf = 0123456789
  test-pwrite.c:56: assertion 'strcmp ("01W3456789",buf) == 0' failed
  qemu: uncaught target signal 6 (Aborted) - core dumped

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1701974/+subscriptions

Re: [Qemu-devel] [PULL 04/14] migration: let MigrationState be a qdev

[Peter Xu]

On Fri, Jun 30, 2017 at 04:27:46PM -0500, Eric Blake wrote:
> On 06/30/2017 04:18 PM, Philippe Mathieu-Daudé wrote:
> > Hi Peter, Juan,
> > 
> > On 06/28/2017 08:30 AM, Juan Quintela wrote:
> >> From: Peter Xu 
> >>
> >> Let the old man "MigrationState" join the object family. Direct benefit
> >> is that we can start to use all the property features derived from
> >> current QDev, like: HW_COMPAT_* bits, command line setup for migration
> >> parameters (so will never need to set them up each time using HMP/QMP,
> >> this is really, really attractive for test writters), etc.
> >>
> >> I see no reason to disallow this happen yet. So let's start from this
> >> one, to see whether it would be anything good.
> >>
> >> Now we init the MigrationState struct statically in main() to make sure
> >> it's initialized after global properties are applied, since we'll use
> >> them during creation of the object.
> >>
> >> No functional change at all.
> >>
> 
> > qemu-system-arm: migration/migration.c:127: migrate_get_current:
> > Assertion `current_migration' failed.
> > 
> > I'v bisected to this commit using the following script:
> 
> Known issue;
> 
> https://lists.gnu.org/archive/html/qemu-devel/2017-06/msg06958.html

Yeah, I'll post the fix today. Really sorry for the inconvenience!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v6 04/10] migration: let MigrationState be a qdev

[Peter Xu]

On Fri, Jun 30, 2017 at 03:57:23PM +0200, Max Reitz wrote:
> On 2017-06-30 15:05, Eric Blake wrote:
> > On 06/30/2017 07:33 AM, Max Reitz wrote:
> > 
> >>> The assertion is caused by migrate_add_blocker() called before
> >>> initialization of migration object. I'll fix it.
> >>
> >> Thanks!

Should be my thanks to you for reporting this. :)

> >>
> >>> But even with a fix (so I can pass 055 now), I still cannot pass some
> >>> of the other tests. Errors I got:
> >>>
> >>>   https://pastebin.com/ACqbXAYd
> >>>
> >>> I am not familiar with iotests. Is above usual? Looks like it still
> >>> includes 3 failures, and some output mismatch.
> >>
> >> Well, not usual. But 068 just is broken on master currently (Stefan has
> >> sent "virtio: use ioeventfd in TCG and qtest mode" to fix it, and it's
> >> part of his latest pull request). The failure in test 087 is because you
> >> don't have aio=native enabled in your build, as the message says. :-)
> > 
> > We could obviously patch 087 to gracefully skip instead of fail when the
> > build doesn't support running it.
> > 
> >>
> >> I'm not sure about 118. Maybe the os.chmod() doesn't work as intended on
> >> your machine...? Because it tries to open a read-only image as
> >> read/write and wants to see it fail (which it doesn't in your case).
> > 
> > Maybe a run-as-root issue, where root can write to the file in spite of
> > permissions?
> 
> That's what I had in mind, too.
> 
> >   Ideally, I'm reluctant to run testsuites as root without
> > good reason (or at least a good sandbox), for fear that a bug in the
> > testsuite will hose my system.
> 
> I never do. :-)

Good reason. I'll definitely switch to non-root even on my dev machine
next time, and also I think I'll also switch to use xfs for my rootfs
(I won't let you know that I'm using btrfs :).

> 
> There is one test which requires it, but well, that just never gets run
> on my machine (and I don't feel very sorry about it).

So looks like I didn't break the rest of the failed tests, good.

Then I'll send the fix soon. Thanks!

-- 
Peter Xu

[Qemu-devel] [PATCH 4/4] doc: update TYPE_MIGRATION documents

[Peter Xu]

[Peter collected Eduardo's patch comment and formatted into patch]

Suggested-by: Eduardo Habkost 
Signed-off-by: Peter Xu 
---
 migration/migration.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/migration/migration.c b/migration/migration.c
index dbdc121..6c64b99 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -2052,12 +2052,15 @@ static void migration_instance_init(Object *obj)
 static const TypeInfo migration_type = {
 .name = TYPE_MIGRATION,
 /*
- * NOTE: "migration" itself is not really a device. We used
- * TYPE_DEVICE here only to leverage some existing QDev features
- * like "-global" properties, and HW_COMPAT_* fields (which are
- * finally applied as global properties as well). If one day the
- * global property feature can be migrated from QDev to QObject in
- * general, then we can switch to QObject as well.
+ * NOTE:
+ *
+ * TYPE_MIGRATION is not really a device, as the object is not
+ * created using qdev_create(), it is not attached to the qdev
+ * device tree, and it is never realized.
+ *
+ * If one day we allow non-device QOM objects to use the global
+ * property system, we can switch this from TYPE_DEVICE to
+ * TYPE_OBJECT.
  */
 .parent = TYPE_DEVICE,
 .class_init = migration_class_init,
-- 
2.7.4

[Qemu-devel] [PATCH 0/4] migration: fix iotest 055, only-migratable break

[Peter Xu]

Two breakage introduced during the migration objectify series: one for
--only-migratable, another one for iotest 055.

First two patches fixes the breakages. Latter two are documentation
updates suggested by Eduardo. Please review. Thanks.

Peter Xu (4):
  migration: fix handling for --only-migratable
  vl: move global property, migrate init earlier
  doc: add item for "-M enforce-config-section"
  doc: update TYPE_MIGRATION documents

 include/migration/misc.h |  1 -
 migration/migration.c| 20 +---
 qemu-options.hx  |  8 
 vl.c | 26 +-
 4 files changed, 30 insertions(+), 25 deletions(-)

-- 
2.7.4

[Qemu-devel] [PATCH 1/4] migration: fix handling for --only-migratable

[Peter Xu]

MigrateState object is not ready at that time, so we'll get an
assertion. Use qemu_global_option() instead.

Reported-by: Eduardo Habkost 
Suggested-by: Eduardo Habkost 
Fixes: 3df663e ("migration: move only_migratable to MigrationState")
Signed-off-by: Peter Xu 
---
 include/migration/misc.h | 1 -
 migration/migration.c| 5 -
 vl.c | 2 +-
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/migration/misc.h b/include/migration/misc.h
index 2255121..c079b77 100644
--- a/include/migration/misc.h
+++ b/include/migration/misc.h
@@ -53,7 +53,6 @@ bool migration_has_finished(MigrationState *);
 bool migration_has_failed(MigrationState *);
 /* ...and after the device transmission */
 bool migration_in_postcopy_after_devices(MigrationState *);
-void migration_only_migratable_set(void);
 void migration_global_dump(Monitor *mon);
 
 #endif
diff --git a/migration/migration.c b/migration/migration.c
index 51ccd1a..dbdc121 100644
--- a/migration/migration.c
+++ b/migration/migration.c
@@ -128,11 +128,6 @@ MigrationState *migrate_get_current(void)
 return current_migration;
 }
 
-void migration_only_migratable_set(void)
-{
-migrate_get_current()->only_migratable = true;
-}
-
 MigrationIncomingState *migration_incoming_get_current(void)
 {
 static bool once;
diff --git a/vl.c b/vl.c
index 36ff3f4..0c497a3 100644
--- a/vl.c
+++ b/vl.c
@@ -3958,7 +3958,7 @@ int main(int argc, char **argv, char **envp)
  *
  * "-global migration.only-migratable=true"
  */
-migration_only_migratable_set();
+qemu_global_option("migration.only-migratable=true");
 break;
 case QEMU_OPTION_nodefaults:
 has_defaults = 0;
-- 
2.7.4

[Qemu-devel] [PATCH 3/4] doc: add item for "-M enforce-config-section"

[Peter Xu]

It's never documented, and now we have one more parameter for it (which
means this one can be obsolete in the future). Document it properly.

Although now when enforce-config-section is set, it'll override the
other "-global" parameter, that is not necessarily a rule. Forbid that
usage in the document.

Suggested-by: Eduardo Habkost 
Signed-off-by: Peter Xu 
---
 qemu-options.hx | 8 
 1 file changed, 8 insertions(+)

diff --git a/qemu-options.hx b/qemu-options.hx
index 297bd8a..927c51f 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -85,6 +85,14 @@ Enables or disables NVDIMM support. The default is off.
 @item s390-squash-mcss=on|off
 Enables or disables squashing subchannels into the default css.
 The default is off.
+@item enforce-config-section=on|off
+Decides whether we will send the configuration section when doing
+migration. By default, it is turned on. We can set this to off to
+explicitly disable it. Note: this parameter will be obsolete soon,
+please use "-global migration.send-configuration=on|off" instead.
+"enforce-config-section" cannot be used together with "-global
+migration.send-configuration". If it happens, the behavior is
+undefined.
 @end table
 ETEXI
 
-- 
2.7.4

[Qemu-devel] [PATCH 2/4] vl: move global property, migrate init earlier

[Peter Xu]

Currently drive_init_func() may call migrate_get_current() while the
migrate object is still not ready yet at that time. Move the migration
object init earlier, along with the global properties, right after
acceleration init.

This fixes a breakage for iotest 055, which caused an assertion failure.

Reported-by: Max Reitz 
Reported-by: Philippe Mathieu-Daudé 
Fixes: 3df663 ("migration: move only_migratable to MigrationState")
Signed-off-by: Peter Xu 
---
 vl.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/vl.c b/vl.c
index 0c497a3..2ae4313 100644
--- a/vl.c
+++ b/vl.c
@@ -4414,6 +4414,18 @@ int main(int argc, char **argv, char **envp)
 
 configure_accelerator(current_machine);
 
+/*
+ * Register all the global properties, including accel properties,
+ * machine properties, and user-specified ones.
+ */
+register_global_properties(current_machine);
+
+/*
+ * Migration object can only be created after global properties
+ * are applied correctly.
+ */
+migration_object_init();
+
 if (qtest_chrdev) {
 qtest_init(qtest_chrdev, qtest_log, &error_fatal);
 }
@@ -4595,18 +4607,6 @@ int main(int argc, char **argv, char **envp)
 exit (i == 1 ? 1 : 0);
 }
 
-/*
- * Register all the global properties, including accel properties,
- * machine properties, and user-specified ones.
- */
-register_global_properties(current_machine);
-
-/*
- * Migration object can only be created after global properties
- * are applied correctly.
- */
-migration_object_init();
-
 /* This checkpoint is required by replay to separate prior clock
reading from the other reads, because timer polling functions query
clock values from the log. */
-- 
2.7.4

Re: [Qemu-devel] [PATCH] target/ppc: Only set PCR in kvm if actually in a compat mode

[Suraj Jitindar Singh]

On Fri, 2017-06-30 at 14:03 +1000, David Gibson wrote:
> On Thu, Jun 29, 2017 at 02:59:39PM +1000, Suraj Jitindar Singh wrote:
> > The Processor Compatibility Register (PCR) I used to set the
> > compatibility mode of the processor using the SET_ONE_REG ioctl on
> > KVM_REG_PPC_ARCH_COMPAT. Previously this was only called when a
> > compat
> > mode was actually in use, however a recent patch made it
> > unconditional.
> > Calling this in KVM_PR fails as there is no handler for that call
> > and it
> > is thus impossible to start a machine with KVM_PR.
> > 
> > Change ppc_set_compat() so that the ioctl is only actually called
> > if a
> > compat mode is in use. This means that a KVM_PR guest can boot.
> > Additionally the current behaviour for KVM_HV is preserved where a
> > compat
> > mode of 0 set pcr and arch_compat in the vcore struct to zero, both
> > of
> > which are initialised to zero anyway.
> > 
> > Fixes: 37f516defa2e ("pseries: Reset CPU compatibility mode")
> > 
> > Signed-off-by: Suraj Jitindar Singh 
> 
> This doesn't seem quite right.  With this change, how would we ever
> turn compatibility mode _off_ (which could happen on reset if nothing

Oh yeah, didn't really think about that.

> else).  Really we should add this pseudo-register to KVM PR, although
> I'm fine with also having a qemu workaround to let it work with older
> PR versions.

How do you feel about having a check and only calling the ioctl if the
KVM in use is HV?

> 
> > 
> > ---
> > 
> > Based on: dwg/ppc-for-2.10
> > 
> >  target/ppc/compat.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/compat.c b/target/ppc/compat.c
> > index f1b67fa..4482206 100644
> > --- a/target/ppc/compat.c
> > +++ b/target/ppc/compat.c
> > @@ -143,7 +143,7 @@ void ppc_set_compat(PowerPCCPU *cpu, uint32_t
> > compat_pvr, Error **errp)
> >  cpu->compat_pvr = compat_pvr;
> >  env->spr[SPR_PCR] = pcr & pcc->pcr_mask;
> >  
> > -if (kvm_enabled()) {
> > +if (kvm_enabled() && compat_pvr) {
> >  int ret = kvmppc_set_compat(cpu, cpu->compat_pvr);
> >  if (ret < 0) {
> >  error_setg_errno(errp, -ret,
> 
> 

[Qemu-devel] [Qemu devel v6 PATCH 1/5] msf2: Add Smartfusion2 System timer

[Subbaraya Sundeep]

Modelled System Timer in Microsemi's Smartfusion2 Soc.
Timer has two 32bit down counters and two interrupts.

Signed-off-by: Subbaraya Sundeep 
---
 hw/timer/Makefile.objs   |   1 +
 hw/timer/mss-timer.c | 261 +++
 include/hw/timer/mss-timer.h |  67 +++
 3 files changed, 329 insertions(+)
 create mode 100644 hw/timer/mss-timer.c
 create mode 100644 include/hw/timer/mss-timer.h

diff --git a/hw/timer/Makefile.objs b/hw/timer/Makefile.objs
index dd6f27e..fc4d2da 100644
--- a/hw/timer/Makefile.objs
+++ b/hw/timer/Makefile.objs
@@ -41,3 +41,4 @@ common-obj-$(CONFIG_STM32F2XX_TIMER) += stm32f2xx_timer.o
 common-obj-$(CONFIG_ASPEED_SOC) += aspeed_timer.o
 
 common-obj-$(CONFIG_SUN4V_RTC) += sun4v-rtc.o
+common-obj-$(CONFIG_MSF2) += mss-timer.o
diff --git a/hw/timer/mss-timer.c b/hw/timer/mss-timer.c
new file mode 100644
index 000..e46d118
--- /dev/null
+++ b/hw/timer/mss-timer.c
@@ -0,0 +1,261 @@
+/*
+ * Block model of System timer present in
+ * Microsemi's SmartFusion2 and SmartFusion SoCs.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep .
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/timer/mss-timer.h"
+
+#ifndef MSS_TIMER_ERR_DEBUG
+#define MSS_TIMER_ERR_DEBUG  0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+if (MSS_TIMER_ERR_DEBUG >= lvl) { \
+qemu_log("%s: " fmt "\n", __func__, ## args); \
+} \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+#define R_TIM_VAL 0
+#define R_TIM_LOADVAL 1
+#define R_TIM_BGLOADVAL   2
+#define R_TIM_CTRL3
+#define R_TIM_RIS 4
+#define R_TIM_MIS 5
+
+#define TIMER_CTRL_ENBL (1 << 0)
+#define TIMER_CTRL_ONESHOT  (1 << 1)
+#define TIMER_CTRL_INTR (1 << 2)
+#define TIMER_RIS_ACK   (1 << 0)
+#define TIMER_RST_CLR   (1 << 6)
+#define TIMER_MODE  (1 << 0)
+
+static void timer_update_irq(struct Msf2Timer *st)
+{
+bool isr, ier;
+
+isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
+ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
+qemu_set_irq(st->irq, (ier && isr));
+}
+
+static void timer_update(struct Msf2Timer *st)
+{
+uint64_t count;
+
+if (!(st->regs[R_TIM_CTRL] & TIMER_CTRL_ENBL)) {
+ptimer_stop(st->ptimer);
+return;
+}
+
+count = st->regs[R_TIM_LOADVAL];
+ptimer_set_limit(st->ptimer, count, 1);
+ptimer_run(st->ptimer, 1);
+}
+
+static uint64_t
+timer_read(void *opaque, hwaddr offset, unsigned int size)
+{
+MSSTimerState *t = opaque;
+hwaddr addr;
+struct Msf2Timer *st;
+uint32_t ret = 0;
+int timer = 0;
+int isr;
+int ier;
+
+addr = offset >> 2;
+/*
+ * Two independent timers has same base address.
+ * Based on address passed figure out which timer is being used.
+ */
+if ((addr >= R_TIM1_MAX) && (addr < NUM_TIMERS * R_TIM1_MAX)) {
+timer = 1;
+addr -= R_TIM1_MAX;
+}
+
+st = &t->timers[timer];
+
+switch (addr) {
+case R_TIM_VAL:
+ret = ptimer_get_count(st->ptimer);
+break;
+
+case R_TIM_MIS:
+isr = !!(st->regs[R_TIM_RIS] & TIMER_RIS_ACK);
+ier = !!(st->regs[R_TIM_CTRL] & TIMER_CTRL_INTR);
+ret = ier & isr;
+break;
+
+default:
+if (addr < NUM_TIMERS * R_TIM1_MAX) {
+ret = st->regs[addr];
+} else {
+qemu_log_mask(LOG_GUEST_ERROR,
+TYPE_MSS_TIMER": 64-bit mode not supported\n");
+}
+break;
+}
+
+DB_PRINT("timer=%d 0x%" HWADDR_PRIx "=0x%" PRIx32, timer, offset,
+ret);
+return ret;
+}
+
+static void
+timer_write(void *opaque, hwaddr offset,
+uint64_t val64, unsigned int size)
+{
+MSSTimerState *t = opaque;
+hwaddr addr;
+struct Msf2Timer *st;
+int timer = 0;
+uint32_t value = val64;
+
+addr = offset >> 2;
+/*
+ * Two indep

[Qemu-devel] [Qemu devel v6 PATCH 2/5] msf2: Microsemi Smartfusion2 System Register block.

[Subbaraya Sundeep]

Added Sytem register block of Smartfusion2.
This block has PLL registers which are accessed by guest.

Signed-off-by: Subbaraya Sundeep 
---
 hw/misc/Makefile.objs |   1 +
 hw/misc/msf2-sysreg.c | 200 ++
 include/hw/misc/msf2-sysreg.h |  82 +
 3 files changed, 283 insertions(+)
 create mode 100644 hw/misc/msf2-sysreg.c
 create mode 100644 include/hw/misc/msf2-sysreg.h

diff --git a/hw/misc/Makefile.objs b/hw/misc/Makefile.objs
index c8b4893..0f52354 100644
--- a/hw/misc/Makefile.objs
+++ b/hw/misc/Makefile.objs
@@ -56,3 +56,4 @@ obj-$(CONFIG_EDU) += edu.o
 obj-$(CONFIG_HYPERV_TESTDEV) += hyperv_testdev.o
 obj-$(CONFIG_AUX) += auxbus.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_scu.o aspeed_sdmc.o
+obj-$(CONFIG_MSF2) += msf2-sysreg.o
diff --git a/hw/misc/msf2-sysreg.c b/hw/misc/msf2-sysreg.c
new file mode 100644
index 000..64ee141
--- /dev/null
+++ b/hw/misc/msf2-sysreg.c
@@ -0,0 +1,200 @@
+/*
+ * System Register block model of Microsemi SmartFusion2.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep 
+ *
+ * This program 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.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, see .
+ */
+
+#include "hw/misc/msf2-sysreg.h"
+
+#ifndef MSF2_SYSREG_ERR_DEBUG
+#define MSF2_SYSREG_ERR_DEBUG  0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+if (MSF2_SYSREG_ERR_DEBUG >= lvl) { \
+qemu_log("%s: " fmt "\n", __func__, ## args); \
+} \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+static inline int msf2_divbits(uint32_t div)
+{
+int ret = 0;
+
+switch (div) {
+case 1:
+ret = 0;
+break;
+case 2:
+ret = 1;
+break;
+case 4:
+ret = 2;
+break;
+case 8:
+ret = 4;
+break;
+case 16:
+ret = 5;
+break;
+case 32:
+ret = 6;
+break;
+default:
+break;
+}
+
+return ret;
+}
+
+static void msf2_sysreg_reset(DeviceState *d)
+{
+MSF2SysregState *s = MSF2_SYSREG(d);
+
+DB_PRINT("RESET");
+
+s->regs[MSSDDR_PLL_STATUS_LOW_CR] = 0x021A2358;
+s->regs[MSSDDR_PLL_STATUS] = 0x3;
+s->regs[MSSDDR_FACC1_CR] = msf2_divbits(s->apb0div) << 5 |
+   msf2_divbits(s->apb1div) << 2;
+}
+
+static uint64_t msf2_sysreg_read(void *opaque, hwaddr offset,
+unsigned size)
+{
+MSF2SysregState *s = opaque;
+offset /= 4;
+uint32_t ret = 0;
+
+if (offset < ARRAY_SIZE(s->regs)) {
+ret = s->regs[offset];
+DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx32,
+offset * 4, ret);
+} else {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
+offset * 4);
+}
+
+return ret;
+}
+
+static void msf2_sysreg_write(void *opaque, hwaddr offset,
+  uint64_t val, unsigned size)
+{
+MSF2SysregState *s = (MSF2SysregState *)opaque;
+uint32_t newval = val;
+uint32_t oldval;
+
+DB_PRINT("addr: 0x%08" HWADDR_PRIx " data: 0x%08" PRIx64,
+offset, val);
+
+offset /= 4;
+
+switch (offset) {
+case MSSDDR_PLL_STATUS:
+break;
+
+case ESRAM_CR:
+oldval = s->regs[ESRAM_CR];
+if (oldval ^ newval) {
+qemu_log_mask(LOG_GUEST_ERROR,
+   TYPE_MSF2_SYSREG": eSRAM remapping not supported\n");
+abort();
+}
+break;
+
+case DDR_CR:
+oldval = s->regs[DDR_CR];
+if (oldval ^ newval) {
+qemu_log_mask(LOG_GUEST_ERROR,
+   TYPE_MSF2_SYSREG": DDR remapping not supported\n");
+abort();
+}
+break;
+
+case ENVM_REMAP_BASE_CR:
+oldval = s->regs[ENVM_REMAP_BASE_CR];
+if (oldval ^ newval) {
+qemu_log_mask(LOG_GUEST_ERROR,
+   TYPE_MSF2_SYSREG": eNVM remapping not supported\n");
+abort();
+}
+break;
+
+default:
+if (offset < ARRAY_SIZE(s->regs)) {
+s->regs[offset] = val;
+} else {
+qemu_log_mask(LOG_GUEST_ERROR,
+"%s: Bad offset 0x%08" HWADDR_PRIx "\n", __func__,
+offset * 4);
+}
+break;
+}
+}
+
+static const MemoryRegionOps sysreg_ops = {
+.read = msf2_sysreg_read,
+.write = msf2_sysreg_write,
+.endianness = DEVICE_NATIVE_ENDIAN,
+};
+
+static void msf2_sysreg_init(Object *obj)
+{
+MSF2SysregState *s = MSF2_SYSREG(obj);
+
+memory_region_init_io(&s->iomem, obj, &sysreg_ops, s, TYPE_MSF2_SYSREG,
+ 

[Qemu-devel] [Qemu devel v6 PATCH 0/5] Add support for Smartfusion2 SoC

[Subbaraya Sundeep]

Hi Qemu-devel,

I am trying to add Smartfusion2 SoC.
SoC is from Microsemi and System on Module(SOM)
board is from Emcraft systems. Smartfusion2 has hardened
Microcontroller(Cortex-M3)based Sub System and FPGA fabric.
At the moment only system timer, sysreg and SPI
controller are modelled.

Testing:
./arm-softmmu/qemu-system-arm -M smartfusion2-som -serial mon:stdio \
-kernel u-boot.bin -display none -drive file=spi.bin,if=mtd,format=raw

Binaries u-boot.bin and spi.bin are at:
https://github.com/Subbaraya-Sundeep/qemu-test-binaries.git

U-boot is from Emcraft with modified
- SPI driver not to use PDMA.
- ugly hack to pass dtb to kernel in r1.
@
https://github.com/Subbaraya-Sundeep/emcraft-uboot-sf2.git

Linux is 4.5 linux with Smartfusion2 SoC dts and clocksource
driver added by myself @
https://github.com/Subbaraya-Sundeep/linux.git

v6:
Moved some defines from header files to source files
Added properties m3clk, apb0div, apb0div1 properties
to soc.
Added properties apb0divisor, apb1divisor to sysreg
Update system_clock_source in msf2-soc.c
Changed machine name smartfusion2-som->emcraft-sf2

v5
As per Philippe comments:
Added abort in Sysreg if guest tries to remap memory
other than default mapping.
Use of CONFIG_MSF2 in Makefile for soc.c
Fixed incorrect logic in timer model.
Renamed msf2-timer.c -> mss-timer.c
msf2-spi.c -> mss-spi.c also type names
Renamed function msf2_init->emcraft_sf2_init in msf2-som.c
Added part-name,eNVM-size,eSRAM-size,pclk0 and pclk1
properties to soc.
Pass soc part-name,memory size and clock rate properties from som.
v4:
Fixed build failure by using PRIx macros.
v3:
Added SoC file and board file as per Alistair comments.
v2:
Added SPI controller so that u-boot loads kernel from spi flash.
v1:
Initial patch set with timer and sysreg

Thanks,
Sundeep

Subbaraya Sundeep (5):
  msf2: Add Smartfusion2 System timer
  msf2: Microsemi Smartfusion2 System Register block.
  msf2: Add Smartfusion2 SPI controller
  msf2: Add Smartfusion2 SoC.
  msf2: Add Emcraft's Smartfusion2 SOM kit.

 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   2 +
 hw/arm/msf2-soc.c   | 216 +
 hw/arm/msf2-som.c   |  94 +
 hw/misc/Makefile.objs   |   1 +
 hw/misc/msf2-sysreg.c   | 200 +++
 hw/ssi/Makefile.objs|   1 +
 hw/ssi/mss-spi.c| 415 
 hw/timer/Makefile.objs  |   1 +
 hw/timer/mss-timer.c| 261 +
 include/hw/arm/msf2-soc.h   |  67 +++
 include/hw/misc/msf2-sysreg.h   |  82 
 include/hw/ssi/mss-spi.h|  62 ++
 include/hw/timer/mss-timer.h|  67 +++
 14 files changed, 1470 insertions(+)
 create mode 100644 hw/arm/msf2-soc.c
 create mode 100644 hw/arm/msf2-som.c
 create mode 100644 hw/misc/msf2-sysreg.c
 create mode 100644 hw/ssi/mss-spi.c
 create mode 100644 hw/timer/mss-timer.c
 create mode 100644 include/hw/arm/msf2-soc.h
 create mode 100644 include/hw/misc/msf2-sysreg.h
 create mode 100644 include/hw/ssi/mss-spi.h
 create mode 100644 include/hw/timer/mss-timer.h

-- 
2.5.0

[Qemu-devel] [Qemu devel v6 PATCH 4/5] msf2: Add Smartfusion2 SoC.

[Subbaraya Sundeep]

Smartfusion2 SoC has hardened Microcontroller subsystem
and flash based FPGA fabric. This patch adds support for
Microcontroller subsystem in the SoC.

Signed-off-by: Subbaraya Sundeep 
---
 default-configs/arm-softmmu.mak |   1 +
 hw/arm/Makefile.objs|   1 +
 hw/arm/msf2-soc.c   | 216 
 include/hw/arm/msf2-soc.h   |  67 +
 4 files changed, 285 insertions(+)
 create mode 100644 hw/arm/msf2-soc.c
 create mode 100644 include/hw/arm/msf2-soc.h

diff --git a/default-configs/arm-softmmu.mak b/default-configs/arm-softmmu.mak
index 78d7af0..7062512 100644
--- a/default-configs/arm-softmmu.mak
+++ b/default-configs/arm-softmmu.mak
@@ -122,3 +122,4 @@ CONFIG_ACPI=y
 CONFIG_SMBIOS=y
 CONFIG_ASPEED_SOC=y
 CONFIG_GPIO_KEY=y
+CONFIG_MSF2=y
diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index 4c5c4ee..c828061 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -18,3 +18,4 @@ obj-$(CONFIG_FSL_IMX25) += fsl-imx25.o imx25_pdk.o
 obj-$(CONFIG_FSL_IMX31) += fsl-imx31.o kzm.o
 obj-$(CONFIG_FSL_IMX6) += fsl-imx6.o sabrelite.o
 obj-$(CONFIG_ASPEED_SOC) += aspeed_soc.o aspeed.o
+obj-$(CONFIG_MSF2) += msf2-soc.o
diff --git a/hw/arm/msf2-soc.c b/hw/arm/msf2-soc.c
new file mode 100644
index 000..d45827f
--- /dev/null
+++ b/hw/arm/msf2-soc.c
@@ -0,0 +1,216 @@
+/*
+ * SmartFusion2 SoC emulation.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu-common.h"
+#include "hw/arm/arm.h"
+#include "exec/address-spaces.h"
+#include "hw/char/serial.h"
+#include "hw/boards.h"
+#include "sysemu/block-backend.h"
+#include "hw/arm/msf2-soc.h"
+
+#define MSF2_TIMER_BASE 0x40004000
+#define MSF2_SYSREG_BASE0x40038000
+
+#define ENVM_BASE_ADDRESS 0x6000
+
+#define SRAM_BASE_ADDRESS 0x2000
+
+#define MSF2_ENVM_SIZE(512 * K_BYTE)
+#define MSF2_ESRAM_SIZE   (64 * K_BYTE)
+
+static const uint32_t spi_addr[MSF2_NUM_SPIS] = { 0x40001000 , 0x40011000 };
+static const uint32_t uart_addr[MSF2_NUM_UARTS] = { 0x4000 , 0x4001 };
+
+static const int spi_irq[MSF2_NUM_SPIS] = { 2, 3 };
+static const int uart_irq[MSF2_NUM_UARTS] = { 10, 11 };
+static const int timer_irq[MSF2_NUM_TIMERS] = { 14, 15 };
+
+static void m2sxxx_soc_initfn(Object *obj)
+{
+MSF2State *s = MSF2_SOC(obj);
+int i;
+
+object_initialize(&s->armv7m, sizeof(s->armv7m), TYPE_ARMV7M);
+qdev_set_parent_bus(DEVICE(&s->armv7m), sysbus_get_default());
+
+object_initialize(&s->sysreg, sizeof(s->sysreg), TYPE_MSF2_SYSREG);
+qdev_set_parent_bus(DEVICE(&s->sysreg), sysbus_get_default());
+
+object_initialize(&s->timer, sizeof(s->timer), TYPE_MSS_TIMER);
+qdev_set_parent_bus(DEVICE(&s->timer), sysbus_get_default());
+
+for (i = 0; i < MSF2_NUM_SPIS; i++) {
+object_initialize(&s->spi[i], sizeof(s->spi[i]),
+  TYPE_MSS_SPI);
+qdev_set_parent_bus(DEVICE(&s->spi[i]), sysbus_get_default());
+}
+}
+
+static void m2sxxx_soc_realize(DeviceState *dev_soc, Error **errp)
+{
+MSF2State *s = MSF2_SOC(dev_soc);
+DeviceState *dev, *armv7m;
+SysBusDevice *busdev;
+Error *err = NULL;
+int i;
+
+MemoryRegion *system_memory = get_system_memory();
+MemoryRegion *nvm = g_new(MemoryRegion, 1);
+MemoryRegion *nvm_alias = g_new(MemoryRegion, 1);
+MemoryRegion *sram = g_new(MemoryRegion, 1);
+
+memory_region_init_ram(nvm, NULL, "MSF2.eNVM", s->envm_size,
+   &error_fatal);
+
+/*
+ * On power-on, the eNVM region 0x6000 is automatically
+ * remapped to the Cortex-M3 processor executable region
+ * start address (0x0). We do not support remapping other eNVM,
+ * eSRAM and DDR regions by guest(via Sysreg) currently.
+ */
+memory_region_init_alias(nvm_alias, NULL, "MSF2.eNVM.al

[Qemu-devel] [Qemu devel v6 PATCH 3/5] msf2: Add Smartfusion2 SPI controller

[Subbaraya Sundeep]

Modelled Microsemi's Smartfusion2 SPI controller.

Signed-off-by: Subbaraya Sundeep 
---
 hw/ssi/Makefile.objs |   1 +
 hw/ssi/mss-spi.c | 414 +++
 include/hw/ssi/mss-spi.h |  62 +++
 3 files changed, 477 insertions(+)
 create mode 100644 hw/ssi/mss-spi.c
 create mode 100644 include/hw/ssi/mss-spi.h

diff --git a/hw/ssi/Makefile.objs b/hw/ssi/Makefile.objs
index 487add2..f5bcc65 100644
--- a/hw/ssi/Makefile.objs
+++ b/hw/ssi/Makefile.objs
@@ -4,6 +4,7 @@ common-obj-$(CONFIG_XILINX_SPI) += xilinx_spi.o
 common-obj-$(CONFIG_XILINX_SPIPS) += xilinx_spips.o
 common-obj-$(CONFIG_ASPEED_SOC) += aspeed_smc.o
 common-obj-$(CONFIG_STM32F2XX_SPI) += stm32f2xx_spi.o
+common-obj-$(CONFIG_MSF2) += mss-spi.o
 
 obj-$(CONFIG_OMAP) += omap_spi.o
 obj-$(CONFIG_IMX) += imx_spi.o
diff --git a/hw/ssi/mss-spi.c b/hw/ssi/mss-spi.c
new file mode 100644
index 000..a572abc
--- /dev/null
+++ b/hw/ssi/mss-spi.c
@@ -0,0 +1,414 @@
+/*
+ * Block model of SPI controller present in
+ * Microsemi's SmartFusion2 and SmartFusion SoCs.
+ *
+ * Copyright (C) 2017 Subbaraya Sundeep 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "hw/ssi/mss-spi.h"
+
+#ifndef MSS_SPI_ERR_DEBUG
+#define MSS_SPI_ERR_DEBUG   0
+#endif
+
+#define DB_PRINT_L(lvl, fmt, args...) do { \
+if (MSS_SPI_ERR_DEBUG >= lvl) { \
+qemu_log("%s: " fmt "\n", __func__, ## args); \
+} \
+} while (0);
+
+#define DB_PRINT(fmt, args...) DB_PRINT_L(1, fmt, ## args)
+
+#define FIFO_CAPACITY 32
+#define FIFO_CAPACITY 32
+
+#define R_SPI_CONTROL 0
+#define R_SPI_DFSIZE  1
+#define R_SPI_STATUS  2
+#define R_SPI_INTCLR  3
+#define R_SPI_RX  4
+#define R_SPI_TX  5
+#define R_SPI_CLKGEN  6
+#define R_SPI_SS  7
+#define R_SPI_MIS 8
+#define R_SPI_RIS 9
+
+#define S_TXDONE (1 << 0)
+#define S_RXRDY  (1 << 1)
+#define S_RXCHOVRF   (1 << 2)
+#define S_RXFIFOFUL  (1 << 4)
+#define S_RXFIFOFULNXT   (1 << 5)
+#define S_RXFIFOEMP  (1 << 6)
+#define S_RXFIFOEMPNXT   (1 << 7)
+#define S_TXFIFOFUL  (1 << 8)
+#define S_TXFIFOFULNXT   (1 << 9)
+#define S_TXFIFOEMP  (1 << 10)
+#define S_TXFIFOEMPNXT   (1 << 11)
+#define S_FRAMESTART (1 << 12)
+#define S_SSEL   (1 << 13)
+#define S_ACTIVE (1 << 14)
+
+#define C_ENABLE (1 << 0)
+#define C_MODE   (1 << 1)
+#define C_INTRXDATA  (1 << 4)
+#define C_INTTXDATA  (1 << 5)
+#define C_INTRXOVRFLO(1 << 6)
+#define C_SPS(1 << 26)
+#define C_BIGFIFO(1 << 29)
+#define C_RESET  (1 << 31)
+
+#define FRAMESZ_MASK 0x1F
+#define FMCOUNT_MASK 0x0000
+#define FMCOUNT_SHIFT8
+
+static void txfifo_reset(MSSSpiState *s)
+{
+fifo32_reset(&s->tx_fifo);
+
+s->regs[R_SPI_STATUS] &= ~S_TXFIFOFUL;
+s->regs[R_SPI_STATUS] |= S_TXFIFOEMP;
+}
+
+static void rxfifo_reset(MSSSpiState *s)
+{
+fifo32_reset(&s->rx_fifo);
+
+s->regs[R_SPI_STATUS] &= ~S_RXFIFOFUL;
+s->regs[R_SPI_STATUS] |= S_RXFIFOEMP;
+}
+
+static void set_fifodepth(MSSSpiState *s)
+{
+unsigned int size = s->regs[R_SPI_DFSIZE] & FRAMESZ_MASK;
+
+if (size <= 8) {
+s->fifo_depth = 32;
+} else if (size <= 16) {
+s->fifo_depth = 16;
+} else if (size <= 32) {
+s->fifo_depth = 8;
+} else {
+s->fifo_depth = 4;
+}
+}
+
+static void mss_spi_do_reset(MSSSpiState *s)
+{
+memset(s->regs, 0, sizeof s->regs);
+s->regs[R_SPI_CONTROL] = 0x8102;
+s->regs[R_SPI_DFSIZE] = 0x4;
+s->regs[R_SPI_STATUS] = S_SSEL | S_TXFIFOEMP | S_RXFIFOEMP;
+s->regs[R_SPI_CLKGEN] = 0x7;
+s->regs[R_SPI_RIS] = 0x0;
+
+s->fifo_depth = 4;
+s->frame_count = 1;
+s->enabled = false;
+
+rxfifo_

[Qemu-devel] [Qemu devel v6 PATCH 5/5] msf2: Add Emcraft's Smartfusion2 SOM kit.

[Subbaraya Sundeep]

Emulated Emcraft's Smartfusion2 System On Module starter
kit.

Signed-off-by: Subbaraya Sundeep 
---
 hw/arm/Makefile.objs |  1 +
 hw/arm/msf2-som.c| 94 
 2 files changed, 95 insertions(+)
 create mode 100644 hw/arm/msf2-som.c

diff --git a/hw/arm/Makefile.objs b/hw/arm/Makefile.objs
index c828061..2073934 100644
--- a/hw/arm/Makefile.objs
+++ b/hw/arm/Makefile.objs
@@ -5,6 +5,7 @@ obj-y += omap_sx1.o palm.o realview.o spitz.o stellaris.o
 obj-y += tosa.o versatilepb.o vexpress.o virt.o xilinx_zynq.o z2.o
 obj-$(CONFIG_ACPI) += virt-acpi-build.o
 obj-y += netduino2.o
+obj-$(CONFIG_MSF2) += msf2-som.o
 obj-y += sysbus-fdt.o
 
 obj-y += armv7m.o exynos4210.o pxa2xx.o pxa2xx_gpio.o pxa2xx_pic.o
diff --git a/hw/arm/msf2-som.c b/hw/arm/msf2-som.c
new file mode 100644
index 000..ea945b1
--- /dev/null
+++ b/hw/arm/msf2-som.c
@@ -0,0 +1,94 @@
+/*
+ * SmartFusion2 SOM starter kit(from Emcraft) emulation.
+ *
+ * Copyright (c) 2017 Subbaraya Sundeep 
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to 
deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING 
FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/boards.h"
+#include "hw/arm/msf2-soc.h"
+#include "hw/arm/arm.h"
+#include "exec/address-spaces.h"
+
+#define DDR_BASE_ADDRESS  0xA000
+#define DDR_SIZE  (64 * M_BYTE)
+
+#define M2S010_ENVM_SIZE  (256 * K_BYTE)
+#define M2S010_ESRAM_SIZE (64 * K_BYTE)
+
+static void emcraft_sf2_init(MachineState *machine)
+{
+DeviceState *dev;
+DeviceState *spi_flash;
+MSF2State *soc;
+DriveInfo *dinfo = drive_get_next(IF_MTD);
+qemu_irq cs_line;
+SSIBus *spi_bus;
+MemoryRegion *sysmem = get_system_memory();
+MemoryRegion *ddr = g_new(MemoryRegion, 1);
+
+memory_region_init_ram(ddr, NULL, "ddr-ram", DDR_SIZE,
+   &error_fatal);
+vmstate_register_ram_global(ddr);
+memory_region_add_subregion(sysmem, DDR_BASE_ADDRESS, ddr);
+
+dev = qdev_create(NULL, TYPE_MSF2_SOC);
+qdev_prop_set_string(dev, "part-name", "M2S010");
+qdev_prop_set_uint64(dev, "eNVM-size", M2S010_ENVM_SIZE);
+qdev_prop_set_uint64(dev, "eSRAM-size", M2S010_ESRAM_SIZE);
+
+/*
+ * CPU clock and peripheral clocks(APB0, APB1)are configurable
+ * in Libero. CPU clock is divided by APB0 and APB1 divisors for
+ * peripherals. Emcraft's SoM kit comes with these settings by default.
+ */
+qdev_prop_set_uint32(dev, "m3clk", 142 * 100);
+qdev_prop_set_uint32(dev, "apb0div", 2);
+qdev_prop_set_uint32(dev, "apb1div", 2);
+
+object_property_set_bool(OBJECT(dev), true, "realized", &error_fatal);
+
+soc = MSF2_SOC(dev);
+
+/* Attach SPI flash to SPI0 controller */
+spi_bus = (SSIBus *)qdev_get_child_bus(dev, "spi0");
+spi_flash = ssi_create_slave_no_init(spi_bus, "s25sl12801");
+qdev_prop_set_uint8(spi_flash, "spansion-cr2nv", 1);
+if (dinfo) {
+qdev_prop_set_drive(spi_flash, "drive", blk_by_legacy_dinfo(dinfo),
+&error_fatal);
+}
+qdev_init_nofail(spi_flash);
+cs_line = qdev_get_gpio_in_named(spi_flash, SSI_GPIO_CS, 0);
+sysbus_connect_irq(SYS_BUS_DEVICE(&soc->spi[0]), 1, cs_line);
+
+armv7m_load_kernel(ARM_CPU(first_cpu), machine->kernel_filename,
+   soc->envm_size);
+}
+
+static void emcraft_sf2_machine_init(MachineClass *mc)
+{
+mc->desc = "SmartFusion2 SOM kit from Emcraft";
+mc->init = emcraft_sf2_init;
+}
+
+DEFINE_MACHINE("emcraft-sf2", emcraft_sf2_machine_init)
-- 
2.5.0

Re: [Qemu-devel] [Qemu devel v5 PATCH 0/5] Add support for Smartfusion2 SoC

[sundeep subbaraya]

Hi Peter,

On Mon, Jul 3, 2017 at 2:30 AM, Peter Maydell 
wrote:

> On 2 July 2017 at 18:39, sundeep subbaraya  wrote:
> > I figured out that systick uses cpu clock as clock source and
> > system_clock_scale
> > need to be set in msf2-soc.c. There is a bug in u-boot where it uses cpu
> > clock as
> > systick input but configures systick in external clock mode. I have
> tested
> > the modified
> > u-boot on real hardware too and it works fine. I am calculating
> > system_clock_scale
> > as below:
> > If CPU clock is X MHz then system_clock_scale = (1 / X) * 1000
> >
> > Tested with different frequencies and they are yielding same results.
>
> If you calculate it like that you'll probably get rounding
> errors. Better is
>   system_clock_scale = NANOSECONDS_PER_SECOND / freq_in_hz;
>
> (Our systick implementation hardwires the external clock
> frequency at 1MHz, but this is not really correct, it
> depends on the SoC.)
>

Ok. Modified as per your comment.

Thanks,
Sundeep

>
> thanks
> -- PMM
>

Re: [Qemu-devel] [Qemu-block] [PULL 11/61] virtio-pci: use ioeventfd even when KVM is disabled

[QingFeng Hao]

在 2017/6/28 18:22, Kevin Wolf 写道:

Am 28.06.2017 um 12:11 hat QingFeng Hao geschrieben:

在 2017/6/24 0:21, Kevin Wolf 写道:

From: Stefan Hajnoczi 

Old kvm.ko versions only supported a tiny number of ioeventfds so
virtio-pci avoids ioeventfds when kvm_has_many_ioeventfds() returns 0.

Do not check kvm_has_many_ioeventfds() when KVM is disabled since it
always returns 0.  Since commit 8c56c1a592b5092d91da8d8943c1d6462a6f
("memory: emulate ioeventfd") it has been possible to use ioeventfds in
qtest or TCG mode.

This patch makes -device virtio-blk-pci,iothread=iothread0 work even
when KVM is disabled.

I have tested that virtio-blk-pci works under TCG both with and without
iothread.

Cc: Michael S. Tsirkin 
Signed-off-by: Stefan Hajnoczi 
Reviewed-by: Michael S. Tsirkin 
Signed-off-by: Kevin Wolf 
---
  hw/virtio/virtio-pci.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 20d6a08..301920e 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1740,7 +1740,7 @@ static void virtio_pci_realize(PCIDevice *pci_dev, Error 
**errp)
  bool pcie_port = pci_bus_is_express(pci_dev->bus) &&
   !pci_bus_is_root(pci_dev->bus);

-if (!kvm_has_many_ioeventfds()) {
+if (kvm_enabled() && !kvm_has_many_ioeventfds()) {
  proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
  }

This response is actually for mail thread "Re: [Qemu-devel] [PATCH
1/5] virtio-pci: use ioeventfd even when KVM is disabled"
which I didn't receive, sorry.
I also saw the failed case of 068 as Fam due to the same cause on
s390x and x86.
With this patch applied, no failure found. Further investigation
shows that the error is in
virtio_scsi_dataplane_setup:
  if (!virtio_device_ioeventfd_enabled(vdev)) {
 error_setg(errp, "ioeventfd is required for iothread");
 return;
  }
call flow is:
virtio_device_ioeventfd_enabled-->virtio_bus_ioeventfd_enabled
-->k->ioeventfd_enabled-->virtio_pci_ioeventfd_enabled
virtio_pci_ioeventfd_enabled checks flag
VIRTIO_PCI_FLAG_USE_IOEVENTFD which was
cleared in virtio_pci_realize if this patch isn't applied.

Yes, we know all of this. However, this patch is not correct and causes
'make check' failures on some platforms. The open question is where that
failure comes from. Before this is solved, the patch can't be applied.
Sorry that I found case 068 of the latest master still fails on s390x 
(but passed
on x86) and the cause is that s390x uses "-device virtio-scsi-ccw" 
instead of
"-device virtio-scsi-pci", so the change in virtio_ccw_device_realize is 
also needed:

diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 90d37cb..35896eb 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -711,7 +711,7 @@ static void 
virtio_ccw_device_realize(VirtioCcwDevice *dev, Error

 sch->cssid, sch->ssid, sch->schid, sch->devno,
 ccw_dev->devno.valid ? "user-configured" : "auto-configured");

-if (!kvm_eventfds_enabled()) {
+if (kvm_enabled() && !kvm_eventfds_enabled()) {
 dev->flags &= ~VIRTIO_CCW_FLAG_USE_IOEVENTFD;
 }

I'll send out a patch for that. Thanks!

Kevin



--
Regards
QingFeng Hao

Re: [Qemu-devel] [PATCH v7 3/3] migration: add bitmap for received page

[Peter Xu]

On Fri, Jun 30, 2017 at 06:18:30AM -0400, Alexey Perevalov wrote:
> This patch adds ability to track down already received
> pages, it's necessary for calculation vCPU block time in
> postcopy migration feature, maybe for restore after
> postcopy migration failure.
> Also it's necessary to solve shared memory issue in
> postcopy livemigration. Information about received pages
> will be transferred to the software virtual bridge
> (e.g. OVS-VSWITCHD), to avoid fallocate (unmap) for
> already received pages. fallocate syscall is required for
> remmaped shared memory, due to remmaping itself blocks
> ioctl(UFFDIO_COPY, ioctl in this case will end with EEXIT
> error (struct page is exists after remmap).
> 
> Bitmap is placed into RAMBlock as another postcopy/precopy
> related bitmaps.
> 
> Signed-off-by: Alexey Perevalov 
> ---
>  include/exec/ram_addr.h  | 10 ++
>  migration/postcopy-ram.c | 16 +++-
>  migration/ram.c  | 43 ---
>  migration/ram.h  |  6 ++
>  4 files changed, 67 insertions(+), 8 deletions(-)
> 
> diff --git a/include/exec/ram_addr.h b/include/exec/ram_addr.h
> index 73d1bea..af5bf26 100644
> --- a/include/exec/ram_addr.h
> +++ b/include/exec/ram_addr.h
> @@ -47,6 +47,8 @@ struct RAMBlock {
>   * of the postcopy phase
>   */
>  unsigned long *unsentmap;
> +/* bitmap of already received pages in postcopy */
> +unsigned long *receivedmap;
>  };
>  
>  static inline bool offset_in_ramblock(RAMBlock *b, ram_addr_t offset)
> @@ -60,6 +62,14 @@ static inline void *ramblock_ptr(RAMBlock *block, 
> ram_addr_t offset)
>  return (char *)block->host + offset;
>  }
>  
> +static inline unsigned long int ramblock_recv_bitmap_offset(void *host_addr,
> +RAMBlock *rb)
> +{
> +uint64_t host_addr_offset =
> +(uint64_t)(uintptr_t)(host_addr - (void *)rb->host);
> +return host_addr_offset >> TARGET_PAGE_BITS;
> +}
> +
>  long qemu_getrampagesize(void);
>  unsigned long last_ram_page(void);
>  RAMBlock *qemu_ram_alloc_from_file(ram_addr_t size, MemoryRegion *mr,
> diff --git a/migration/postcopy-ram.c b/migration/postcopy-ram.c
> index be497bb..276ce12 100644
> --- a/migration/postcopy-ram.c
> +++ b/migration/postcopy-ram.c
> @@ -560,22 +560,27 @@ int postcopy_ram_enable_notify(MigrationIncomingState 
> *mis)
>  }
>  
>  static int qemu_ufd_copy_ioctl(int userfault_fd, void *host_addr,
> -void *from_addr, uint64_t pagesize)
> +   void *from_addr, uint64_t pagesize, RAMBlock 
> *rb)
>  {
> +int ret;
>  if (from_addr) {
>  struct uffdio_copy copy_struct;
>  copy_struct.dst = (uint64_t)(uintptr_t)host_addr;
>  copy_struct.src = (uint64_t)(uintptr_t)from_addr;
>  copy_struct.len = pagesize;
>  copy_struct.mode = 0;
> -return ioctl(userfault_fd, UFFDIO_COPY, ©_struct);
> +ret = ioctl(userfault_fd, UFFDIO_COPY, ©_struct);
>  } else {
>  struct uffdio_zeropage zero_struct;
>  zero_struct.range.start = (uint64_t)(uintptr_t)host_addr;
>  zero_struct.range.len = pagesize;
>  zero_struct.mode = 0;
> -return ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> +ret = ioctl(userfault_fd, UFFDIO_ZEROPAGE, &zero_struct);
> +}
> +if (!ret) {
> +ramblock_recv_bitmap_set(host_addr, rb);
>  }
> +return ret;
>  }
>  
>  /*
> @@ -592,7 +597,7 @@ int postcopy_place_page(MigrationIncomingState *mis, void 
> *host, void *from,
>   * which would be slightly cheaper, but we'd have to be careful
>   * of the order of updating our page state.
>   */
> -if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize)) {
> +if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, from, pagesize, rb)) {
>  int e = errno;
>  error_report("%s: %s copy host: %p from: %p (size: %zd)",
>   __func__, strerror(e), host, from, pagesize);
> @@ -614,7 +619,8 @@ int postcopy_place_page_zero(MigrationIncomingState *mis, 
> void *host,
>  trace_postcopy_place_page_zero(host);
>  
>  if (qemu_ram_pagesize(rb) == getpagesize()) {
> -if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, 
> getpagesize())) {
> +if (qemu_ufd_copy_ioctl(mis->userfault_fd, host, NULL, getpagesize(),
> +rb)) {
>  int e = errno;
>  error_report("%s: %s zero host: %p",
>   __func__, strerror(e), host);
> diff --git a/migration/ram.c b/migration/ram.c
> index 9cc1b17..dfbb36b 100644
> --- a/migration/ram.c
> +++ b/migration/ram.c
> @@ -147,6 +147,32 @@ out:
>  return ret;
>  }
>  
> +void ramblock_recv_map_init(void)
> +{
> +RAMBlock *rb;
> +
> +RAMBLOCK_FOREACH(rb) {
> +assert(!rb->receivedmap);
> +rb->receivedmap = bitmap_new(rb->max_length >> TARGET_P

[Qemu-devel] [Qemu-PPC] [PATCH 1/3] target/ppc: Refactor tcg radix mmu code

[Suraj Jitindar Singh]

The mmu-radix64.c file implements functions to enable the radix mmu
emulation in tcg mode. There is a function ppc_radix64_walk_tree() which
performs the radix tree walk and also implicitly checks the pte
protection.

Move the protection checking of the pte from the ppc_radix64_walk_tree()
function into the caller. This means the ppc_radix64_walk_tree() function
can be used without protection checking which is useful for debugging.

ppc_radix64_walk_tree() no longer needs to take the rwx and prot variables.

Signed-off-by: Suraj Jitindar Singh 
---
 target/ppc/mmu-radix64.c | 22 --
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 69fde65..1a650fd 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -147,11 +147,10 @@ static void ppc_radix64_set_rc(PowerPCCPU *cpu, int rwx, 
uint64_t pte,
 }
 }
 
-static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, int rwx, vaddr eaddr,
+static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, vaddr eaddr,
   uint64_t base_addr, uint64_t nls,
   hwaddr *raddr, int *psize,
-  int *fault_cause, int *prot,
-  hwaddr *pte_addr)
+  int *fault_cause, hwaddr *pte_addr)
 {
 CPUState *cs = CPU(cpu);
 uint64_t index, pde;
@@ -177,10 +176,6 @@ static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, int 
rwx, vaddr eaddr,
 uint64_t rpn = pde & R_PTE_RPN;
 uint64_t mask = (1UL << *psize) - 1;
 
-if (ppc_radix64_check_prot(cpu, rwx, pde, fault_cause, prot)) {
-return 0; /* Protection Denied Access */
-}
-
 /* Or high bits of rpn and low bits to ea to form whole real addr */
 *raddr = (rpn & ~mask) | (eaddr & mask);
 *pte_addr = base_addr + (index * sizeof(pde));
@@ -188,9 +183,8 @@ static uint64_t ppc_radix64_walk_tree(PowerPCCPU *cpu, int 
rwx, vaddr eaddr,
 }
 
 /* Next Level of Radix Tree */
-return ppc_radix64_walk_tree(cpu, rwx, eaddr, pde & R_PDE_NLB,
- pde & R_PDE_NLS, raddr, psize,
- fault_cause, prot, pte_addr);
+return ppc_radix64_walk_tree(cpu, eaddr, pde & R_PDE_NLB, pde & R_PDE_NLS,
+ raddr, psize, fault_cause, pte_addr);
 }
 
 int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
@@ -241,11 +235,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
eaddr, int rwx,
 
 /* Walk Radix Tree from Process Table Entry to Convert EA to RA */
 page_size = PRTBE_R_GET_RTS(prtbe0);
-pte = ppc_radix64_walk_tree(cpu, rwx, eaddr & R_EADDR_MASK,
+pte = ppc_radix64_walk_tree(cpu, eaddr & R_EADDR_MASK,
 prtbe0 & PRTBE_R_RPDB, prtbe0 & PRTBE_R_RPDS,
-&raddr, &page_size, &fault_cause, &prot,
-&pte_addr);
-if (!pte) {
+&raddr, &page_size, &fault_cause, &pte_addr);
+if (!pte || ppc_radix64_check_prot(cpu, rwx, pte, &fault_cause, &prot)) {
+/* Couldn't get pte or access denied due to protection */
 ppc_radix64_raise_si(cpu, rwx, eaddr, fault_cause);
 return 1;
 }
-- 
2.9.4

[Qemu-devel] [Qemu-PPC] [PATCH 3/3] target/ppc: Add debug function to dump radix mmu translations

[Suraj Jitindar Singh]

In target/ppc/mmu-hash64.c there already exists the function
dump_slb() to dump the hash translation entries (for effective to
virtual translation at least).

Implement the function ppc_radix64_dump() to allow all the kernel
effective to real address mappings and corresponding ptes to be dumped.
This is called when "info tlb" is invoked in the qemu console. Previously
this command had no output when invoked with a radix guest.

Signed-off-by: Suraj Jitindar Singh 
---
 target/ppc/mmu-radix64.c | 49 
 target/ppc/mmu-radix64.h |  1 +
 target/ppc/mmu_helper.c  |  2 +-
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index bbd37e3..f7eeead 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -296,3 +296,52 @@ hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, 
target_ulong eaddr)
 
 return raddr & TARGET_PAGE_MASK;
 }
+
+static void ppc_radix64_dump_level(FILE *f, fprintf_function cpu_fprintf,
+   PowerPCCPU *cpu, uint64_t eaddr,
+   uint64_t base_addr, uint64_t nls,
+   uint64_t psize, int *num)
+{
+CPUState *cs = CPU(cpu);
+uint64_t i, pte;
+
+for (i = 0; i < (1 << nls); i++) {
+eaddr &= ~((1ULL << psize) - 1); /* Clear the low bits */
+eaddr |= (i << (psize - nls));
+
+pte = ldq_phys(cs->as, base_addr + (i * sizeof(pte)));
+if (!(pte & R_PTE_VALID)) { /* Invalid Entry */
+continue;
+}
+
+if (pte & R_PTE_LEAF) {
+uint64_t mask = (1ULL << (psize - nls)) - 1;
+cpu_fprintf(f, "%d\t0x%.16" PRIx64 " -> 0x%.16" PRIx64
+   " pte: 0x%.16" PRIx64 "\n", (*num)++,
+   eaddr, pte & R_PTE_RPN & ~mask, pte);
+} else {
+ppc_radix64_dump_level(f, cpu_fprintf, cpu, eaddr, pte & R_PDE_NLB,
+   pte & R_PDE_NLS, psize - nls, num);
+}
+}
+}
+
+void ppc_radix64_dump(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu)
+{
+CPUState *cs = CPU(cpu);
+PPCVirtualHypervisorClass *vhc =
+PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+uint64_t patbe, prtbe0;
+int num = 0;
+
+/* Get Process Table */
+patbe = vhc->get_patbe(cpu->vhyp);
+
+/* Load the first entry -> Guest kernel mappings (all we dump for now) */
+prtbe0 = ldq_phys(cs->as, patbe & PATBE1_R_PRTB);
+
+cpu_fprintf(f, "\tEADDR\t\t  RADDR\n");
+ppc_radix64_dump_level(f, cpu_fprintf, cpu, 0ULL, prtbe0 & PRTBE_R_RPDB,
+   prtbe0 & PRTBE_R_RPDS, PRTBE_R_GET_RTS(prtbe0),
+   &num);
+}
diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
index 0ecf063..c6c22bd 100644
--- a/target/ppc/mmu-radix64.h
+++ b/target/ppc/mmu-radix64.h
@@ -47,6 +47,7 @@
 int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
  int mmu_idx);
 hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
+void ppc_radix64_dump(FILE *f, fprintf_function cpu_fprintf, PowerPCCPU *cpu);
 
 static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
 {
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index b7b9088..9587b07 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1287,7 +1287,7 @@ void dump_mmu(FILE *f, fprintf_function cpu_fprintf, 
CPUPPCState *env)
 break;
 case POWERPC_MMU_VER_3_00:
 if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
-/* TODO - Unsupported */
+ppc_radix64_dump(f, cpu_fprintf, ppc_env_get_cpu(env));
 } else {
 dump_slb(f, cpu_fprintf, ppc_env_get_cpu(env));
 break;
-- 
2.9.4

[Qemu-devel] [Qemu-PPC] [PATCH 0/3] target/ppc: Implement radix mmu debug functions

[Suraj Jitindar Singh]

When the radix mmu emulation code was implemented the translation
debug functions were left as a todo.

Implement the functions ppc_radix64_get_phys_page_debug() and 
ppc_radix64_dump_level() to translate a single address and dump all the 
address translations respectively.

This functionality is already implemented for the hash mmu.

A slight refactor of the radix mmu emulation code was necessary to enable
the debug code to call existing functions.

Suraj Jitindar Singh (3):
  target/ppc: Refactor tcg radix mmu code
  target/ppc: Add debug function for radix mmu translation
  target/ppc: Add debug function to dump radix mmu translations

 target/ppc/mmu-radix64.c | 116 +--
 target/ppc/mmu-radix64.h |   2 +
 target/ppc/mmu_helper.c  |   5 +-
 3 files changed, 107 insertions(+), 16 deletions(-)

-- 
2.9.4

[Qemu-devel] [Qemu-PPC] [PATCH 2/3] target/ppc: Add debug function for radix mmu translation

[Suraj Jitindar Singh]

In target/ppc/mmu-hash64.c there already exists the function
ppc_hash64_get_phys_page_debug() to get the physical (real) address for
a given effective address in hash mode.

Implement the function ppc_radix64_get_phys_page_debug() to allow a real
address to be obtained for a given effective address in radix mode.
This is used when a debugger is attached to qemu.

Previously we just had a comment saying this is unimplemented which then
fell through to the default case and caused an abort due to
unrecognised mmu model as the default had no case for the V3 mmu, which
was misleading at best.

We reuse ppc_radix64_walk_tree() which is used by the radix fault
handler since the process of walking the radix tree is identical.

Reported-by: Balbir Singh 
Signed-off-by: Suraj Jitindar Singh 
---
 target/ppc/mmu-radix64.c | 45 +
 target/ppc/mmu-radix64.h |  1 +
 target/ppc/mmu_helper.c  |  3 ++-
 3 files changed, 48 insertions(+), 1 deletion(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 1a650fd..bbd37e3 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -251,3 +251,48 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr 
eaddr, int rwx,
  prot, mmu_idx, 1UL << page_size);
 return 0;
 }
+
+hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
+{
+CPUState *cs = CPU(cpu);
+CPUPPCState *env = &cpu->env;
+PPCVirtualHypervisorClass *vhc =
+PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+hwaddr raddr, pte_addr;
+uint64_t lpid = 0, pid = 0, offset, size, patbe, prtbe0, pte;
+int page_size, fault_cause = 0;
+
+/* Handle Real Mode */
+if (msr_dr == 0) {
+/* In real mode top 4 effective addr bits (mostly) ignored */
+return eaddr & 0x0FFFULL;
+}
+
+/* Virtual Mode Access - get the fully qualified address */
+if (!ppc_radix64_get_fully_qualified_addr(env, eaddr, &lpid, &pid)) {
+return -1;
+}
+
+/* Get Process Table */
+patbe = vhc->get_patbe(cpu->vhyp);
+
+/* Index Process Table by PID to Find Corresponding Process Table Entry */
+offset = pid * sizeof(struct prtb_entry);
+size = 1ULL << ((patbe & PATBE1_R_PRTS) + 12);
+if (offset >= size) {
+/* offset exceeds size of the process table */
+return -1;
+}
+prtbe0 = ldq_phys(cs->as, (patbe & PATBE1_R_PRTB) + offset);
+
+/* Walk Radix Tree from Process Table Entry to Convert EA to RA */
+page_size = PRTBE_R_GET_RTS(prtbe0);
+pte = ppc_radix64_walk_tree(cpu, eaddr & R_EADDR_MASK,
+prtbe0 & PRTBE_R_RPDB, prtbe0 & PRTBE_R_RPDS,
+&raddr, &page_size, &fault_cause, &pte_addr);
+if (!pte) {
+return -1;
+}
+
+return raddr & TARGET_PAGE_MASK;
+}
diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
index 1d5c7cf..0ecf063 100644
--- a/target/ppc/mmu-radix64.h
+++ b/target/ppc/mmu-radix64.h
@@ -46,6 +46,7 @@
 
 int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
  int mmu_idx);
+hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
 
 static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
 {
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 65d1c86..b7b9088 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -30,6 +30,7 @@
 #include "helper_regs.h"
 #include "qemu/error-report.h"
 #include "mmu-book3s-v3.h"
+#include "mmu-radix64.h"
 
 //#define DEBUG_MMU
 //#define DEBUG_BATS
@@ -1432,7 +1433,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr 
addr)
 return ppc_hash64_get_phys_page_debug(cpu, addr);
 case POWERPC_MMU_VER_3_00:
 if (ppc64_radix_guest(ppc_env_get_cpu(env))) {
-/* TODO - Unsupported */
+return ppc_radix64_get_phys_page_debug(cpu, addr);
 } else {
 return ppc_hash64_get_phys_page_debug(cpu, addr);
 }
-- 
2.9.4

Re: [Qemu-devel] [PATCH 0/5] spapr: DRC cleanups (part V)

[David Gibson]

On Tue, Jun 20, 2017 at 09:53:27AM +0800, David Gibson wrote:
> This fifth set of cleanups to the DRC code mostly deals with removing
> unnecessary differences between different cases on the various hot
> plug and unplug paths.

With the assorted R-bs accumulated, I've merged this batch into
ppc-for-2.10.

-- 
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 5/5] spapr: Use unplug_request for PCI hot unplug

[David Gibson]

On Wed, Jun 21, 2017 at 11:50:08AM +0200, Greg Kurz wrote:
> On Tue, 20 Jun 2017 09:53:32 +0800
> David Gibson  wrote:
> 
> > AIUI, ->unplug_request in the HotplugHandler is used for "soft"
> > unplug, where acknowledgement from the guest is required before
> > completing the unplug, whereas ->unplug is used for "hard" unplug
> > where qemu unilaterally removes the device, and the guest just has to
> > cope with its sudden absence.  For spapr we (correctly) use
> > ->unplug_request for CPU and memory hot unplug but we use ->unplug for  
> > PCI.
> > 
> > While I think it might be possible to support "hard" PCI unplug within
> > the PAPR model, that's not how it actually works now.  Although it's
> > called from ->unplug, the PCI unplug path will usually just mark the
> > device for removal, with completion of the unplug delayed until
> > userspace responds to the unplug notification. If the guest doesn't
> > respond as expected, that could delay the unplug completiong
> > arbitrarily long.
> > 
> > To reflect that, change the PCI unplug path to be called from
> > ->unplug_request.  
> > 
> > Signed-off-by: David Gibson 
> > ---
> >  hw/ppc/spapr_pci.c | 7 ---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> > 
> > diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> > index f2543ef..bda8938 100644
> > --- a/hw/ppc/spapr_pci.c
> > +++ b/hw/ppc/spapr_pci.c
> > @@ -1469,8 +1469,8 @@ out:
> >  }
> >  }
> >  
> > -static void spapr_phb_hot_unplug_child(HotplugHandler *plug_handler,
> > -   DeviceState *plugged_dev, Error 
> > **errp)
> > +static void spapr_pci_unplug_request(HotplugHandler *plug_handler,
> > + DeviceState *plugged_dev, Error 
> > **errp)
> >  {
> >  sPAPRPHBState *phb = SPAPR_PCI_HOST_BRIDGE(DEVICE(plug_handler));
> >  PCIDevice *pdev = PCI_DEVICE(plugged_dev);
> > @@ -1485,6 +1485,7 @@ static void spapr_phb_hot_unplug_child(HotplugHandler 
> > *plug_handler,
> >  }
> >  
> >  g_assert(drc);
> > +g_assert(drc->dev == plugged_dev);
> >  
> >  drck = SPAPR_DR_CONNECTOR_GET_CLASS(drc);
> >  if (!drck->release_pending(drc)) {
> > @@ -1973,7 +1974,7 @@ static void spapr_phb_class_init(ObjectClass *klass, 
> > void *data)
> >  dc->user_creatable = true;
> >  set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
> >  hp->plug = spapr_phb_hot_plug_child;
> 
> Maybe you can rename spapr_phb_hot_plug_child() to spapr_pci_plug() for
> consistency ?

Good idea.  I've made that change.

> 
> Anyway,
> 
> Reviewed-by: Greg Kurz 
> 
> > -hp->unplug = spapr_phb_hot_unplug_child;
> > +hp->unplug_request = spapr_pci_unplug_request;
> >  }
> >  
> >  static const TypeInfo spapr_phb_info = {
> 



-- 
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