Re: [Qemu-devel] [PATCH 06/11] Add support for S390x system emulation

[Alexander Graf]

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 <address@hidden>
>>>> ---
>>>> 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