From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:39655)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cornelia.huck@de.ibm.com>) id 1apX9e-0004nd-AS
	for qemu-devel@nongnu.org; Mon, 11 Apr 2016 04:25:07 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <cornelia.huck@de.ibm.com>) id 1apX9a-0003iG-7e
	for qemu-devel@nongnu.org; Mon, 11 Apr 2016 04:25:06 -0400
Received: from e06smtp13.uk.ibm.com ([195.75.94.109]:47324)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <cornelia.huck@de.ibm.com>) id 1apX9Z-0003ha-VC
	for qemu-devel@nongnu.org; Mon, 11 Apr 2016 04:25:02 -0400
Received: from localhost
	by e06smtp13.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use
	Only! Violators will be prosecuted
	for <qemu-devel@nongnu.org> from <cornelia.huck@de.ibm.com>;
	Mon, 11 Apr 2016 09:24:59 +0100
Received: from b06cxnps3074.portsmouth.uk.ibm.com
	(d06relay09.portsmouth.uk.ibm.com [9.149.109.194])
	by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id C6393219005E
	for <qemu-devel@nongnu.org>; Mon, 11 Apr 2016 09:24:36 +0100 (BST)
Received: from d06av09.portsmouth.uk.ibm.com (d06av09.portsmouth.uk.ibm.com
	[9.149.37.250])
	by b06cxnps3074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with
	ESMTP id u3B8OvY41114486
	for <qemu-devel@nongnu.org>; Mon, 11 Apr 2016 08:24:57 GMT
Received: from d06av09.portsmouth.uk.ibm.com (localhost [127.0.0.1])
	by d06av09.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with
	ESMTP id u3B8Ov3S009924
	for <qemu-devel@nongnu.org>; Mon, 11 Apr 2016 02:24:57 -0600
Date: Mon, 11 Apr 2016 10:24:54 +0200
From: Cornelia Huck <cornelia.huck@de.ibm.com>
Message-ID: <20160411102454.55152b66.cornelia.huck@de.ibm.com>
In-Reply-To: <1460147350-7601-42-git-send-email-pbonzini@redhat.com>
References: <1460147350-7601-1-git-send-email-pbonzini@redhat.com>
	<1460147350-7601-42-git-send-email-pbonzini@redhat.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH 41/50] s390x: move stuff out of cpu.h
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel/>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: qemu-devel@nongnu.org

On Fri,  8 Apr 2016 22:29:01 +0200
Paolo Bonzini <pbonzini@redhat.com> wrote:

> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/s390x/css.c                              |   4 +-
>  hw/s390x/s390-skeys.c                       |   1 +
>  hw/s390x/s390-virtio-ccw.c                  |   4 +-
>  hw/s390x/virtio-ccw.c                       |   5 +-
>  hw/s390x/virtio-ccw.h                       |   3 +-
>  {hw => include/hw}/s390x/css.h              |  31 +++++++-
>  {target-s390x => include/hw/s390x}/ioinst.h |  16 +---
>  target-s390x/cpu.h                          | 117 ++++++----------------------
>  target-s390x/helper.c                       |   1 +
>  target-s390x/interrupt.c                    |  64 +++++++++++++++
>  target-s390x/ioinst.c                       |   2 +-
>  11 files changed, 131 insertions(+), 117 deletions(-)
>  rename {hw => include/hw}/s390x/css.h (76%)
>  rename {target-s390x => include/hw/s390x}/ioinst.h (87%)
> 

<did not review, but noticed the below:>

> diff --git a/target-s390x/interrupt.c b/target-s390x/interrupt.c
> index bad60a7..a13fd26 100644
> --- a/target-s390x/interrupt.c
> +++ b/target-s390x/interrupt.c
> @@ -10,6 +10,7 @@
>  #include "qemu/osdep.h"
>  #include "cpu.h"
>  #include "sysemu/kvm.h"
> +#include "hw/s390x/ioinst.h"
> 
>  /*
>   * All of the following interrupts are floating, i.e. not per-vcpu.

This used to be true before...

> @@ -17,6 +18,69 @@
>   * non-kvm case.
>   */
>  #if !defined(CONFIG_USER_ONLY)
> +void cpu_inject_ext(S390CPU *cpu, uint32_t code, uint32_t param,
> +                    uint64_t param64)

...but this one can be both floating and per-vcpu. Need to adust the
comment above?

> +{
> +    CPUS390XState *env = &cpu->env;
> +
> +    if (env->ext_index == MAX_EXT_QUEUE - 1) {
> +        /* ugh - can't queue anymore. Let's drop. */
> +        return;
> +    }
> +
> +    env->ext_index++;
> +    assert(env->ext_index < MAX_EXT_QUEUE);
> +
> +    env->ext_queue[env->ext_index].code = code;
> +    env->ext_queue[env->ext_index].param = param;
> +    env->ext_queue[env->ext_index].param64 = param64;
> +
> +    env->pending_int |= INTERRUPT_EXT;
> +    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> +}
> +
> +void cpu_inject_io(S390CPU *cpu, uint16_t subchannel_id,
> +                   uint16_t subchannel_number,
> +                   uint32_t io_int_parm, uint32_t io_int_word)
> +{

This one...

> +    CPUS390XState *env = &cpu->env;
> +    int isc = IO_INT_WORD_ISC(io_int_word);
> +
> +    if (env->io_index[isc] == MAX_IO_QUEUE - 1) {
> +        /* ugh - can't queue anymore. Let's drop. */
> +        return;
> +    }
> +
> +    env->io_index[isc]++;
> +    assert(env->io_index[isc] < MAX_IO_QUEUE);
> +
> +    env->io_queue[env->io_index[isc]][isc].id = subchannel_id;
> +    env->io_queue[env->io_index[isc]][isc].nr = subchannel_number;
> +    env->io_queue[env->io_index[isc]][isc].parm = io_int_parm;
> +    env->io_queue[env->io_index[isc]][isc].word = io_int_word;
> +
> +    env->pending_int |= INTERRUPT_IO;
> +    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> +}
> +
> +void cpu_inject_crw_mchk(S390CPU *cpu)
> +{

...and this one are always floating and therefore should only be called
from this file anyway (so they can be made static).

> +    CPUS390XState *env = &cpu->env;
> +
> +    if (env->mchk_index == MAX_MCHK_QUEUE - 1) {
> +        /* ugh - can't queue anymore. Let's drop. */
> +        return;
> +    }
> +
> +    env->mchk_index++;
> +    assert(env->mchk_index < MAX_MCHK_QUEUE);
> +
> +    env->mchk_queue[env->mchk_index].type = 1;
> +
> +    env->pending_int |= INTERRUPT_MCHK;
> +    cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> +}
> +
>  void s390_sclp_extint(uint32_t parm)
>  {
>      if (kvm_enabled()) {