[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 06/11] Add support for S390x system emulation
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH 06/11] Add support for S390x system emulation |
Date: |
Wed, 2 Dec 2009 09:41:18 +0100 |
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 <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().
>>
>
> 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