Re: [Qemu-devel] [PATCH] spapr-rtas: reset top 4 bits in parameters address

[Alexey Kardashevskiy]

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.


-- 
Alexey