[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters addr
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address |
Date: |
Thu, 5 Sep 2013 15:08:10 +0200 |
On 05.09.2013, at 14:49, Alexey Kardashevskiy wrote:
> On 09/05/2013 10:16 PM, Alexander Graf wrote:
>>
>> On 05.09.2013, at 14:04, Alexey Kardashevskiy wrote:
>>
>>> On 09/05/2013 08:21 PM, Alexander Graf wrote:
>>>>
>>>> On 05.09.2013, at 12:17, Alexey Kardashevskiy wrote:
>>>>
>>>>> On 09/05/2013 07:27 PM, Alexander Graf wrote:
>>>>>>
>>>>>> On 05.09.2013, at 09:40, Alexey Kardashevskiy wrote:
>>>>>>
>>>>>>> On 09/05/2013 05:08 PM, Alexander Graf wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> Am 05.09.2013 um 07:58 schrieb Alexey Kardashevskiy <address@hidden>:
>>>>>>>>
>>>>>>>>> On the real hardware, RTAS is called in real mode and therefore
>>>>>>>>> ignores top 4 bits of the address passed in the call.
>>>>>>>>
>>>>>>>> Shouldn't we ignore the upper 4 bits for every memory access in real
>>>>>>>> mode, not just that one parameter?
>>>>>>>
>>>>>>> We probably should but I just do not see any easy way of doing this. Yet
>>>>>>> another "Ignore N bits on the top" memory region type? No idea.
>>>>>>
>>>>>> Well, it already works for code that runs inside of guest context,
>>>>>> because there the softmmu code for real mode strips the upper 4 bits.
>>>>>>
>>>>>> I basically see 2 ways of fixing this "correctly":
>>>>>>
>>>>>
>>>>>> 1) Don't access memory through cpu_physical_memory_rw or ldx_phys but
>>>>>> instead through real mode wrappers that strip the upper 4 bits, similar
>>>>>> to how we handle virtual memory differently from physical memory
>>>>>
>>>>> But there is no a ready wrapper for this, correct? I could not find any. I
>>>>> would rather do this, looks nicer than 2).
>>>>>
>>>>>
>>>>>> 2) Create 15 aliases to system_memory at the upper 4 bits of address
>>>>>> space. That should at the end of the day give you the same effect
>>>>>
>>>>> Wow. Is not that too much?
>>>>> Ooor since I am normally making bad decisions, I should do this :)
>>>>>
>>>>>
>>>>>> The fix as you're proposing it wouldn't work for indirect memory
>>>>>> descriptors. Imagine you have an "address" parameter that gives you a
>>>>>> pointer to a struct in memory that again contains a pointer. You still
>>>>>> want that pointer be interpreted correctly, no?
>>>>>
>>>>> Yes I do. I just think that having non zero bits at the top is a bug and I
>>>>> would not want the guest to continue sending bad addresses to the host. Or
>>>>> at least I want to know if it still happening.
>>>>>
>>>>> Now we know that the only occasion of this misbehaviour is the "stop-self"
>>>>> call and others works just fine. If something new comes up (what is pretty
>>>>> unlikely, otherwise we would have noticed this issue a loong time ago AND
>>>>> Paul already made&posted a patch for the host to fix __pa() so it is not
>>>>> going to happen on new kernels either), ok, we will think of fixing this.
>>>>>
>>>>> Doing in QEMU what the hardware does is a good thing but here I would
>>>>> think
>>>>> twice.
>>>>
>>>> Well, the idea behind RTAS is that everything RTAS does is usually run in
>>>> IR=0 DR=0 inside of guest context, so that's the view of the world we
>>>> should expose.
>>>>
>>>> Which makes me think.
>>>>
>>>
>>>> Couldn't we just set IR=0 DR=0 when getting an RTAS call and use the
>>>> virtual memory access functions? Those will already strip the upper 4
>>>> bits.
>>>
>>> Ok. We reached the border where my ignorance starts :) Never could
>>> understand the concept of the guest virtual memory in QEMU.
>>>
>>> So we clear IR/DR and call what API? This is not address_space_rw() and
>>> company, right?
>>
>> Nono, we basically route things through the same accesses that instructions
>> inside of guest context would call. Something like
>>
>> cpu_ldl_data()
>>
>> for example. IIRC there is also an #ifdef that allows you to just run ldl().
>
> cpu_ldl_data() is defined for CONFIG_USER_ONLY. But ok, it is defined
> simply as ldl_p():
>
> #define cpu_ldl_data(env, addr) ldl_raw(addr)
> #define g2h(x) ((void *)((unsigned long)(target_ulong)(x) + GUEST_BASE))
> #define laddr(x) g2h(x)
> #define ldl_raw(p) ldl_p(laddr((p)))
>
> static inline int ldl_p(const void *ptr)
> {
> int32_t r;
> memcpy(&r, ptr, sizeof(r));
> return r;
> }
>
> So it tries accessing memory @ptr (which is the guest physical) and -
> crashes :) So I need an address converter which is not there.
>
> What do I miss? Thanks.
It should be defined through a bunch of macros and incomprehensible #include's
and glue()'s for softmmu too. Just try and see if it works for you.
>
>
>> It automatically uses the current virtual layout the same way that the
>> instruction emulator would do it - which is pretty much what we want.
>>
>
>> IIRC you also have to enter RTAS calls with DR=0, so we wouldn't even
>> need to flip any MSR bits when emulating RTAS calls, right?
>
> Probably. Right now cpu->env.msr==0x0 in rtas handler but not sure that I
> see the real value.
Make sure you run cpu_synchronize_state() before you look at cpu->env.msr.
Alex
- [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexander Graf, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexander Graf, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexander Graf, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexander Graf, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address,
Alexander Graf <=
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/05
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/06
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexander Graf, 2013/09/06
- Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address, Alexey Kardashevskiy, 2013/09/06