[06/11] Add support for S390x system emulation

Let's enable the basics for system emulation so we can run virtual machines
with KVM!

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
 target-s390x/exec.h               |    5 +
 target-s390x/helper.c             |   22 +++++
 target-s390x/machine.c            |   30 +++++++
 4 files changed, 208 insertions(+), 2 deletions(-)
 create mode 100644 default-configs/s390x-softmmu.mak
 create mode 100644 target-s390x/machine.c

On Thu, Nov 26, 2009 at 02:23:15PM +0100, Alexander Graf wrote:
> Let's enable the basics for system emulation so we can run virtual machines
> with KVM!

I don't really understand while this whole patch is not merged in patch
number 1. Otherwise, please find the comments below.

> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
>  target-s390x/exec.h               |    5 +
>  target-s390x/helper.c             |   22 +++++
>  target-s390x/machine.c            |   30 +++++++
>  4 files changed, 208 insertions(+), 2 deletions(-)
>  create mode 100644 default-configs/s390x-softmmu.mak
>  create mode 100644 target-s390x/machine.c
> 
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> new file mode 100644
> index 0000000..e69de29
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index f45b00c..a74745c 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -30,8 +30,7 @@
>  
>  #include "softfloat.h"
>  
> -#define NB_MMU_MODES 2 // guess
> -#define MMU_USER_IDX 0 // guess
> +#define NB_MMU_MODES 2
>  
>  typedef union FPReg {
>      struct {
> @@ -77,6 +76,15 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
>  }
>  #endif
>  
> +#define MMU_MODE0_SUFFIX _kernel
> +#define MMU_MODE1_SUFFIX _user
> +#define MMU_USER_IDX 1
> +static inline int cpu_mmu_index (CPUState *env)
> +{
> +    /* XXX: Currently we don't implement virtual memory */
> +    return 0;

Is it correct? It means that memory access will aways be kernel memory
accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
some cases (devices ?).

> +}
> +
>  CPUS390XState *cpu_s390x_init(const char *cpu_model);
>  int cpu_s390x_exec(CPUS390XState *s);
>  void cpu_s390x_close(CPUS390XState *s);
> @@ -92,6 +100,13 @@ int cpu_s390x_handle_mmu_fault (CPUS390XState *env, target_ulong address, int rw
>  
>  #define TARGET_PAGE_BITS 12
>  
> +#ifndef CONFIG_USER_ONLY
> +extern int s390_virtio_hypercall(CPUState *env);
> +extern void kvm_s390_virtio_irq(CPUState *env, int config_change, uint64_t token);
> +extern CPUState *s390_cpu_addr2state(uint16_t cpu_addr);
> +#endif
> +
> +
>  #define cpu_init cpu_s390x_init
>  #define cpu_exec cpu_s390x_exec
>  #define cpu_gen_code cpu_s390x_gen_code
> @@ -116,4 +131,138 @@ static inline void cpu_get_tb_cpu_state(CPUState* env, target_ulong *pc,
>      *cs_base = 0;
>      *flags = env->psw.mask; // guess
>  }
> +
> +/* Program Status Word.  */
> +#define S390_PSWM_REGNUM 0
> +#define S390_PSWA_REGNUM 1
> +/* General Purpose Registers.  */
> +#define S390_R0_REGNUM 2
> +#define S390_R1_REGNUM 3
> +#define S390_R2_REGNUM 4
> +#define S390_R3_REGNUM 5
> +#define S390_R4_REGNUM 6
> +#define S390_R5_REGNUM 7
> +#define S390_R6_REGNUM 8
> +#define S390_R7_REGNUM 9
> +#define S390_R8_REGNUM 10
> +#define S390_R9_REGNUM 11
> +#define S390_R10_REGNUM 12
> +#define S390_R11_REGNUM 13
> +#define S390_R12_REGNUM 14
> +#define S390_R13_REGNUM 15
> +#define S390_R14_REGNUM 16
> +#define S390_R15_REGNUM 17
> +/* Access Registers.  */
> +#define S390_A0_REGNUM 18
> +#define S390_A1_REGNUM 19
> +#define S390_A2_REGNUM 20
> +#define S390_A3_REGNUM 21
> +#define S390_A4_REGNUM 22
> +#define S390_A5_REGNUM 23
> +#define S390_A6_REGNUM 24
> +#define S390_A7_REGNUM 25
> +#define S390_A8_REGNUM 26
> +#define S390_A9_REGNUM 27
> +#define S390_A10_REGNUM 28
> +#define S390_A11_REGNUM 29
> +#define S390_A12_REGNUM 30
> +#define S390_A13_REGNUM 31
> +#define S390_A14_REGNUM 32
> +#define S390_A15_REGNUM 33
> +/* Floating Point Control Word.  */
> +#define S390_FPC_REGNUM 34
> +/* Floating Point Registers.  */
> +#define S390_F0_REGNUM 35
> +#define S390_F1_REGNUM 36
> +#define S390_F2_REGNUM 37
> +#define S390_F3_REGNUM 38
> +#define S390_F4_REGNUM 39
> +#define S390_F5_REGNUM 40
> +#define S390_F6_REGNUM 41
> +#define S390_F7_REGNUM 42
> +#define S390_F8_REGNUM 43
> +#define S390_F9_REGNUM 44
> +#define S390_F10_REGNUM 45
> +#define S390_F11_REGNUM 46
> +#define S390_F12_REGNUM 47
> +#define S390_F13_REGNUM 48
> +#define S390_F14_REGNUM 49
> +#define S390_F15_REGNUM 50
> +/* Total.  */
> +#define S390_NUM_REGS 51
> +
> +/* Pseudo registers -- PC and condition code.  */
> +#define S390_PC_REGNUM S390_NUM_REGS
> +#define S390_CC_REGNUM (S390_NUM_REGS+1)
> +#define S390_NUM_PSEUDO_REGS 2
> +#define S390_NUM_TOTAL_REGS (S390_NUM_REGS+2)
> +
> +
> +
> +/* Program Status Word.  */
> +#define S390_PSWM_REGNUM 0
> +#define S390_PSWA_REGNUM 1
> +/* General Purpose Registers.  */
> +#define S390_R0_REGNUM 2
> +#define S390_R1_REGNUM 3
> +#define S390_R2_REGNUM 4
> +#define S390_R3_REGNUM 5
> +#define S390_R4_REGNUM 6
> +#define S390_R5_REGNUM 7
> +#define S390_R6_REGNUM 8
> +#define S390_R7_REGNUM 9
> +#define S390_R8_REGNUM 10
> +#define S390_R9_REGNUM 11
> +#define S390_R10_REGNUM 12
> +#define S390_R11_REGNUM 13
> +#define S390_R12_REGNUM 14
> +#define S390_R13_REGNUM 15
> +#define S390_R14_REGNUM 16
> +#define S390_R15_REGNUM 17
> +/* Access Registers.  */
> +#define S390_A0_REGNUM 18
> +#define S390_A1_REGNUM 19
> +#define S390_A2_REGNUM 20
> +#define S390_A3_REGNUM 21
> +#define S390_A4_REGNUM 22
> +#define S390_A5_REGNUM 23
> +#define S390_A6_REGNUM 24
> +#define S390_A7_REGNUM 25
> +#define S390_A8_REGNUM 26
> +#define S390_A9_REGNUM 27
> +#define S390_A10_REGNUM 28
> +#define S390_A11_REGNUM 29
> +#define S390_A12_REGNUM 30
> +#define S390_A13_REGNUM 31
> +#define S390_A14_REGNUM 32
> +#define S390_A15_REGNUM 33
> +/* Floating Point Control Word.  */
> +#define S390_FPC_REGNUM 34
> +/* Floating Point Registers.  */
> +#define S390_F0_REGNUM 35
> +#define S390_F1_REGNUM 36
> +#define S390_F2_REGNUM 37
> +#define S390_F3_REGNUM 38
> +#define S390_F4_REGNUM 39
> +#define S390_F5_REGNUM 40
> +#define S390_F6_REGNUM 41
> +#define S390_F7_REGNUM 42
> +#define S390_F8_REGNUM 43
> +#define S390_F9_REGNUM 44
> +#define S390_F10_REGNUM 45
> +#define S390_F11_REGNUM 46
> +#define S390_F12_REGNUM 47
> +#define S390_F13_REGNUM 48
> +#define S390_F14_REGNUM 49
> +#define S390_F15_REGNUM 50
> +/* Total.  */
> +#define S390_NUM_REGS 51
> +
> +/* Pseudo registers -- PC and condition code.  */
> +#define S390_PC_REGNUM S390_NUM_REGS
> +#define S390_CC_REGNUM (S390_NUM_REGS+1)
> +#define S390_NUM_PSEUDO_REGS 2
> +#define S390_NUM_TOTAL_REGS (S390_NUM_REGS+2)
> +
> +
>  #endif
> diff --git a/target-s390x/exec.h b/target-s390x/exec.h
> index 5198359..13dc7dd 100644
> --- a/target-s390x/exec.h
> +++ b/target-s390x/exec.h
> @@ -22,9 +22,14 @@
>  
>  register struct CPUS390XState *env asm(AREG0);
>  
> +#include "config.h"
>  #include "cpu.h"
>  #include "exec-all.h"
>  
> +#if !defined(CONFIG_USER_ONLY)
> +#include "softmmu_exec.h"
> +#endif /* !defined(CONFIG_USER_ONLY) */
> +
>  static inline int cpu_has_work(CPUState *env)
>  {
>      return env->interrupt_request & CPU_INTERRUPT_HARD; // guess
> diff --git a/target-s390x/helper.c b/target-s390x/helper.c
> index 0e222e3..f4c4e04 100644
> --- a/target-s390x/helper.c
> +++ b/target-s390x/helper.c
> @@ -27,6 +27,9 @@
>  #include "gdbstub.h"
>  #include "qemu-common.h"
>  
> +#include <linux/kvm.h>
> +#include "kvm.h"
> +
>  CPUS390XState *cpu_s390x_init(const char *cpu_model)
>  {
>      CPUS390XState *env;
> @@ -60,3 +63,22 @@ void cpu_reset(CPUS390XState *env)
>      /* FIXME: reset vector? */
>      tlb_flush(env, 1);
>  }
> +
> +#ifndef CONFIG_USER_ONLY
> +
> +int cpu_s390x_handle_mmu_fault (CPUState *env, target_ulong address, int rw,
> +                                int mmu_idx, int is_softmmu)
> +{
> +    target_ulong phys;
> +    int prot;
> +
> +    /* XXX: implement mmu */
> +
> +    phys = address;
> +    prot = PAGE_READ | PAGE_WRITE;
> +
> +    return tlb_set_page(env, address & TARGET_PAGE_MASK,
> +                        phys & TARGET_PAGE_MASK, prot,
> +                        mmu_idx, is_softmmu);

Same as previous comment, if softmmu accesses are possible, this
function should be written correctly.

> +}
> +#endif /* CONFIG_USER_ONLY */
> diff --git a/target-s390x/machine.c b/target-s390x/machine.c
> new file mode 100644
> index 0000000..3e79be6
> --- /dev/null
> +++ b/target-s390x/machine.c
> @@ -0,0 +1,30 @@
> +/*
> + * QEMU S390x machine definitions
> + *
> + * Copyright (c) 2009 Alexander Graf <agraf@suse.de>
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library 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
> + * Lesser General Public License for more details.
/> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "hw/hw.h"
> +#include "hw/boards.h"
> +
> +void cpu_save(QEMUFile *f, void *opaque)
> +{
> +}
> +
> +int cpu_load(QEMUFile *f, void *opaque, int version_id)
> +{
> +    return 0;
> +}
> -- 
> 1.6.0.2
> 
> 
> 
>

On 30.11.2009, at 19:18, Aurelien Jarno wrote:

> On Thu, Nov 26, 2009 at 02:23:15PM +0100, Alexander Graf wrote:
>> Let's enable the basics for system emulation so we can run virtual machines
>> with KVM!
> 
> I don't really understand while this whole patch is not merged in patch
> number 1. Otherwise, please find the comments below.

Historical reasons. To keep Uli's stripped down version separate from my code.

> 
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>> target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
>> target-s390x/exec.h               |    5 +
>> target-s390x/helper.c             |   22 +++++
>> target-s390x/machine.c            |   30 +++++++
>> 4 files changed, 208 insertions(+), 2 deletions(-)
>> create mode 100644 default-configs/s390x-softmmu.mak
>> create mode 100644 target-s390x/machine.c
>> 
>> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
>> new file mode 100644
>> index 0000000..e69de29
>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>> index f45b00c..a74745c 100644
>> --- a/target-s390x/cpu.h
>> +++ b/target-s390x/cpu.h
>> @@ -30,8 +30,7 @@
>> 
>> #include "softfloat.h"
>> 
>> -#define NB_MMU_MODES 2 // guess
>> -#define MMU_USER_IDX 0 // guess
>> +#define NB_MMU_MODES 2
>> 
>> typedef union FPReg {
>>     struct {
>> @@ -77,6 +76,15 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
>> }
>> #endif
>> 
>> +#define MMU_MODE0_SUFFIX _kernel
>> +#define MMU_MODE1_SUFFIX _user
>> +#define MMU_USER_IDX 1
>> +static inline int cpu_mmu_index (CPUState *env)
>> +{
>> +    /* XXX: Currently we don't implement virtual memory */
>> +    return 0;
> 
> Is it correct? It means that memory access will aways be kernel memory
> accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
> some cases (devices ?).

I can't imagine any hardware using the CPU's MMU to write to RAM. That's what IOMMUs are for.

The only 2 consumers are:

1) tcg
2) gdb / monitor

With 2) being broken, because we can't resolve virtual addresses to physical addresses. But that won't change until someone implements the softmmu emulation target for real.

Aurelien Jarno wrote:
>> +static inline int cpu_mmu_index (CPUState *env)
>> +{
>> +    /* XXX: Currently we don't implement virtual memory */
>> +    return 0;
> 
> Is it correct? It means that memory access will aways be kernel memory
> accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
> some cases (devices ?).
On s390, we don't have memory mapped I/O. KVM is virtio only, and even 
if we would add channel devices we would not require softmmu emulation 
here. S390 does'nt have pci, usb or alike.

On Mon, Nov 30, 2009 at 11:19:06PM +0100, Alexander Graf wrote:
> 
> On 30.11.2009, at 19:18, Aurelien Jarno wrote:
> 
> > On Thu, Nov 26, 2009 at 02:23:15PM +0100, Alexander Graf wrote:
> >> Let's enable the basics for system emulation so we can run virtual machines
> >> with KVM!
> > 
> > I don't really understand while this whole patch is not merged in patch
> > number 1. Otherwise, please find the comments below.
> 
> Historical reasons. To keep Uli's stripped down version separate from my code.
> 
> > 
> >> Signed-off-by: Alexander Graf <agraf@suse.de>
> >> ---
> >> target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
> >> target-s390x/exec.h               |    5 +
> >> target-s390x/helper.c             |   22 +++++
> >> target-s390x/machine.c            |   30 +++++++
> >> 4 files changed, 208 insertions(+), 2 deletions(-)
> >> create mode 100644 default-configs/s390x-softmmu.mak
> >> create mode 100644 target-s390x/machine.c
> >> 
> >> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> >> new file mode 100644
> >> index 0000000..e69de29
> >> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> >> index f45b00c..a74745c 100644
> >> --- a/target-s390x/cpu.h
> >> +++ b/target-s390x/cpu.h
> >> @@ -30,8 +30,7 @@
> >> 
> >> #include "softfloat.h"
> >> 
> >> -#define NB_MMU_MODES 2 // guess
> >> -#define MMU_USER_IDX 0 // guess
> >> +#define NB_MMU_MODES 2
> >> 
> >> typedef union FPReg {
> >>     struct {
> >> @@ -77,6 +76,15 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
> >> }
> >> #endif
> >> 
> >> +#define MMU_MODE0_SUFFIX _kernel
> >> +#define MMU_MODE1_SUFFIX _user
> >> +#define MMU_USER_IDX 1
> >> +static inline int cpu_mmu_index (CPUState *env)
> >> +{
> >> +    /* XXX: Currently we don't implement virtual memory */
> >> +    return 0;
> > 
> > Is it correct? It means that memory access will aways be kernel memory
> > accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
> > some cases (devices ?).
> 
> I can't imagine any hardware using the CPU's MMU to write to RAM. That's what IOMMUs are for.
> 
> The only 2 consumers are:
> 
> 1) tcg
> 2) gdb / monitor
> 
> With 2) being broken, because we can't resolve virtual addresses to physical addresses. But that won't change until someone implements the softmmu emulation target for real.

If it is sure it is never used, I would prefer to see an abort().
Otherwise it's fine.

On 02.12.2009, at 09:09, Aurelien Jarno wrote:

> On Mon, Nov 30, 2009 at 11:19:06PM +0100, Alexander Graf wrote:
>> 
>> On 30.11.2009, at 19:18, Aurelien Jarno wrote:
>> 
>>> On Thu, Nov 26, 2009 at 02:23:15PM +0100, Alexander Graf wrote:
>>>> Let's enable the basics for system emulation so we can run virtual machines
>>>> with KVM!
>>> 
>>> I don't really understand while this whole patch is not merged in patch
>>> number 1. Otherwise, please find the comments below.
>> 
>> Historical reasons. To keep Uli's stripped down version separate from my code.
>> 
>>> 
>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>> ---
>>>> target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
>>>> target-s390x/exec.h               |    5 +
>>>> target-s390x/helper.c             |   22 +++++
>>>> target-s390x/machine.c            |   30 +++++++
>>>> 4 files changed, 208 insertions(+), 2 deletions(-)
>>>> create mode 100644 default-configs/s390x-softmmu.mak
>>>> create mode 100644 target-s390x/machine.c
>>>> 
>>>> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
>>>> new file mode 100644
>>>> index 0000000..e69de29
>>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>>> index f45b00c..a74745c 100644
>>>> --- a/target-s390x/cpu.h
>>>> +++ b/target-s390x/cpu.h
>>>> @@ -30,8 +30,7 @@
>>>> 
>>>> #include "softfloat.h"
>>>> 
>>>> -#define NB_MMU_MODES 2 // guess
>>>> -#define MMU_USER_IDX 0 // guess
>>>> +#define NB_MMU_MODES 2
>>>> 
>>>> typedef union FPReg {
>>>>    struct {
>>>> @@ -77,6 +76,15 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
>>>> }
>>>> #endif
>>>> 
>>>> +#define MMU_MODE0_SUFFIX _kernel
>>>> +#define MMU_MODE1_SUFFIX _user
>>>> +#define MMU_USER_IDX 1
>>>> +static inline int cpu_mmu_index (CPUState *env)
>>>> +{
>>>> +    /* XXX: Currently we don't implement virtual memory */
>>>> +    return 0;
>>> 
>>> Is it correct? It means that memory access will aways be kernel memory
>>> accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
>>> some cases (devices ?).
>> 
>> I can't imagine any hardware using the CPU's MMU to write to RAM. That's what IOMMUs are for.
>> 
>> The only 2 consumers are:
>> 
>> 1) tcg
>> 2) gdb / monitor
>> 
>> With 2) being broken, because we can't resolve virtual addresses to physical addresses. But that won't change until someone implements the softmmu emulation target for real.
> 
> If it is sure it is never used, I would prefer to see an abort().
> Otherwise it's fine.

I don't think I understand where you want to put the abort().

Alex

On Wed, Dec 02, 2009 at 09:27:21AM +0100, Alexander Graf wrote:
> 
> On 02.12.2009, at 09:09, Aurelien Jarno wrote:
> 
> > On Mon, Nov 30, 2009 at 11:19:06PM +0100, Alexander Graf wrote:
> >> 
> >> On 30.11.2009, at 19:18, Aurelien Jarno wrote:
> >> 
> >>> On Thu, Nov 26, 2009 at 02:23:15PM +0100, Alexander Graf wrote:
> >>>> Let's enable the basics for system emulation so we can run virtual machines
> >>>> with KVM!
> >>> 
> >>> I don't really understand while this whole patch is not merged in patch
> >>> number 1. Otherwise, please find the comments below.
> >> 
> >> Historical reasons. To keep Uli's stripped down version separate from my code.
> >> 
> >>> 
> >>>> Signed-off-by: Alexander Graf <agraf@suse.de>
> >>>> ---
> >>>> target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
> >>>> target-s390x/exec.h               |    5 +
> >>>> target-s390x/helper.c             |   22 +++++
> >>>> target-s390x/machine.c            |   30 +++++++
> >>>> 4 files changed, 208 insertions(+), 2 deletions(-)
> >>>> create mode 100644 default-configs/s390x-softmmu.mak
> >>>> create mode 100644 target-s390x/machine.c
> >>>> 
> >>>> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> >>>> new file mode 100644
> >>>> index 0000000..e69de29
> >>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> >>>> index f45b00c..a74745c 100644
> >>>> --- a/target-s390x/cpu.h
> >>>> +++ b/target-s390x/cpu.h
> >>>> @@ -30,8 +30,7 @@
> >>>> 
> >>>> #include "softfloat.h"
> >>>> 
> >>>> -#define NB_MMU_MODES 2 // guess
> >>>> -#define MMU_USER_IDX 0 // guess
> >>>> +#define NB_MMU_MODES 2
> >>>> 
> >>>> typedef union FPReg {
> >>>>    struct {
> >>>> @@ -77,6 +76,15 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
> >>>> }
> >>>> #endif
> >>>> 
> >>>> +#define MMU_MODE0_SUFFIX _kernel
> >>>> +#define MMU_MODE1_SUFFIX _user
> >>>> +#define MMU_USER_IDX 1
> >>>> +static inline int cpu_mmu_index (CPUState *env)
> >>>> +{
> >>>> +    /* XXX: Currently we don't implement virtual memory */
> >>>> +    return 0;
> >>> 
> >>> Is it correct? It means that memory access will aways be kernel memory
> >>> accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
> >>> some cases (devices ?).
> >> 
> >> I can't imagine any hardware using the CPU's MMU to write to RAM. That's what IOMMUs are for.
> >> 
> >> The only 2 consumers are:
> >> 
> >> 1) tcg
> >> 2) gdb / monitor
> >> 
> >> With 2) being broken, because we can't resolve virtual addresses to physical addresses. But that won't change until someone implements the softmmu emulation target for real.
> > 
> > If it is sure it is never used, I would prefer to see an abort().
> > Otherwise it's fine.
> 
> I don't think I understand where you want to put the abort().
> 

In inline cpu_mmu_index(), just before the return, to make sure this
function is never called, as it is clearly wrong.

On 02.12.2009, at 09:37, Aurelien Jarno wrote:

> On Wed, Dec 02, 2009 at 09:27:21AM +0100, Alexander Graf wrote:
>> 
>> On 02.12.2009, at 09:09, Aurelien Jarno wrote:
>> 
>>> On Mon, Nov 30, 2009 at 11:19:06PM +0100, Alexander Graf wrote:
>>>> 
>>>> On 30.11.2009, at 19:18, Aurelien Jarno wrote:
>>>> 
>>>>> On Thu, Nov 26, 2009 at 02:23:15PM +0100, Alexander Graf wrote:
>>>>>> Let's enable the basics for system emulation so we can run virtual machines
>>>>>> with KVM!
>>>>> 
>>>>> I don't really understand while this whole patch is not merged in patch
>>>>> number 1. Otherwise, please find the comments below.
>>>> 
>>>> Historical reasons. To keep Uli's stripped down version separate from my code.
>>>> 
>>>>> 
>>>>>> Signed-off-by: Alexander Graf <agraf@suse.de>
>>>>>> ---
>>>>>> target-s390x/cpu.h                |  153 ++++++++++++++++++++++++++++++++++++-
>>>>>> target-s390x/exec.h               |    5 +
>>>>>> target-s390x/helper.c             |   22 +++++
>>>>>> target-s390x/machine.c            |   30 +++++++
>>>>>> 4 files changed, 208 insertions(+), 2 deletions(-)
>>>>>> create mode 100644 default-configs/s390x-softmmu.mak
>>>>>> create mode 100644 target-s390x/machine.c
>>>>>> 
>>>>>> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
>>>>>> new file mode 100644
>>>>>> index 0000000..e69de29
>>>>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>>>>> index f45b00c..a74745c 100644
>>>>>> --- a/target-s390x/cpu.h
>>>>>> +++ b/target-s390x/cpu.h
>>>>>> @@ -30,8 +30,7 @@
>>>>>> 
>>>>>> #include "softfloat.h"
>>>>>> 
>>>>>> -#define NB_MMU_MODES 2 // guess
>>>>>> -#define MMU_USER_IDX 0 // guess
>>>>>> +#define NB_MMU_MODES 2
>>>>>> 
>>>>>> typedef union FPReg {
>>>>>>   struct {
>>>>>> @@ -77,6 +76,15 @@ static inline void cpu_clone_regs(CPUState *env, target_ulong newsp)
>>>>>> }
>>>>>> #endif
>>>>>> 
>>>>>> +#define MMU_MODE0_SUFFIX _kernel
>>>>>> +#define MMU_MODE1_SUFFIX _user
>>>>>> +#define MMU_USER_IDX 1
>>>>>> +static inline int cpu_mmu_index (CPUState *env)
>>>>>> +{
>>>>>> +    /* XXX: Currently we don't implement virtual memory */
>>>>>> +    return 0;
>>>>> 
>>>>> Is it correct? It means that memory access will aways be kernel memory
>>>>> accesses. IIRC, even with KVM enabled, softmmu accesses are possible in
>>>>> some cases (devices ?).
>>>> 
>>>> I can't imagine any hardware using the CPU's MMU to write to RAM. That's what IOMMUs are for.
>>>> 
>>>> The only 2 consumers are:
>>>> 
>>>> 1) tcg
>>>> 2) gdb / monitor
>>>> 
>>>> With 2) being broken, because we can't resolve virtual addresses to physical addresses. But that won't change until someone implements the softmmu emulation target for real.
>>> 
>>> If it is sure it is never used, I would prefer to see an abort().
>>> Otherwise it's fine.
>> 
>> I don't think I understand where you want to put the abort().
>> 
> 
> In inline cpu_mmu_index(), just before the return, to make sure this
> function is never called, as it is clearly wrong.

It's just always saying we're in kernel mode. I don't see where that's wrong. There's no logic to implement modes, so that's the only reasonable thing to do.

Also, it does get called. That's what I mean with the target 2). When you're in the monitor and do x /i $pc, you end up calling that function. While it's not great to only have linear mapped memory here, it's a lot better than having no reply or, even worse, killing the VM.

Alex