On 09/05/2013 11:08 PM, Alexander Graf wrote: > > 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 <a...@ozlabs.ru>: >>>>>>>>> >>>>>>>>>> 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.
Hm. I was not clear. I tried. It crashed in ldl_p() and I explained why exactly. I understand what you expected but it should be different set of macros than the one you proposed. >>> 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. Oh, right. -- Alexey