On 17.05.2012, at 00:29, Scott Wood <scottwood@xxxxxxxxxxxxx> wrote: > On 05/16/2012 04:13 AM, Alexander Graf wrote: >> On 05/16/2012 11:02 AM, Sethi Varun-B16395 wrote: >>> >>>> -----Original Message----- >>>> From: kvm-ppc-owner@xxxxxxxxxxxxxxx [mailto:kvm-ppc- >>>> owner@xxxxxxxxxxxxxxx] On Behalf Of Bhushan Bharat-R65777 >>>> Sent: Wednesday, May 16, 2012 12:58 PM >>>> To: Alexander Graf >>>> Cc: Wood Scott-B07421; Yoder Stuart-B08248; kvm-ppc@xxxxxxxxxxxxxxx >>>> Subject: Not emulated registers on BOOKE_HV (GS-mode) >>>> >>>> Hi Alex, >>>> >>>> There is below comment in arch/powerpc/kvm/booke_emulate.c >>>> >>>> /* >>>> * NOTE: some of these registers are not emulated on BOOKE_HV >>>> (GS-mode). >>>> * Their backing store is in real registers, and these functions >>>> * will return the wrong result if called for them in another context >>>> * (such as debugging). >>>> */ >>>> >>>> "some of these registers are not emulated on BOOKE_HV (GS-mode)" >>>> 1) Is not that mtspr()/mfspr() for "not emulated" registers should >>>> follow EMULATE_FAIL path? So should be ifdef out for BOOKE_HV? Otherwise >>>> the emulation code execute. >>>> 2) Or These are not emulated because the GS mode have direct access to >>>> these registers, Right? So no trap? >>>> >>> For BOOKE_HV mtspr/mfspr would get mapped to hardware maintained guest >>> shadow copies. >>> For example guest access to dear would get mapped to gdear and >>> wouldn't trap >>> >>>> "and these functions will return the wrong result if called for them in >>>> another context (such as debugging)." >>>> 1) So do you mean that guest is not supposed to access these registers >>>> in normal scenario but the debugger (some command on gdb in guest) can >>>> access these register? then does it make sense to treat mtspr() as nop >>>> and mfspr returns 0/undefined? >>>> >>> Actually, in a normal scenario (For BOOKE_HV) guest (MSR[GS] = 1) >>> would be able to directly access these registers >>> without hypervisor intervention. However I am not sure under what >>> condition would the guest access generate a trap, all >>> guest accesses would be with MSR[GS]=1. >> >> Yup. E500mc would just never reach that code - and even if it did, it >> simply wouldn't do anything useful. >> >> What did you guys need this change for? I'm sure there's some incentive >> behind it, right? The only one I can see right now would be reduced code >> size. > > This was something Stuart pulled out of my local working tree from a > commit where the entire commit message was "crit ints and misc emulation > stuff to be sorted through". :-P > > Looking through previous versions of the history, the ifdefs were how > Varun originally added critical interrupt support. I probably changed > it to the "NOTE:" version without ifdefs, and the "misc emulation stuff" > patch was just waiting for me to integrate that change properly with > things like crit ints that weren't part of the e500mc patchset I posted. > > That said, if this code ever does get used for debug or similar > non-emulation access, it would be nicer to fail than to return garbage > -- but what isn't nice is increasing the points where this stuff is > decided at compile time. Yeah. I have a brach on my tree caed emul-rework. If you like, feel free to check it out. It's what I envisioned a cleaner API to the whole instruction emulation. Unfortunately, it turned out to be significantly slower (measurable) with more memory overhead (allocated and text) than what we have today. So I figured I won't go with it for now... Long-term, we should try to come up with better solutions to the whole thing though. I just haven't found the ideal one yet ;). Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
