Christoffer, Having regs[16] is an internal optimization so when emulating something, lets say a mov from one reg to another, we don't have to figure out where to get/put it every time based on the current mode. With optimization: reg[r?] = xxxx Without optimization if R? < R8 then ... else if R? < R12 & mode=fiq fiq else if R? < R12 usr else if R? = R13 reg13 else if R? = R14 reg14 else reg15 That optimization, I believe, shouldn't be externalized through the kvm_regs struct. I believe adding a regs[16] is additional complexity. For example, did you know that regs[8] through regs[14] in kvm_regs are irrelevant? In set_regs we do not use them because we reload vcpu->regs from the backed registers, overwriting anything that was in there. Does everyone trying to interface with kvm know this? Or would they think that the backed regs (fiq_regs, usr_regs, reg13 and reg14) for the current mode are the ones that are irrelevant. Should the backed regs for the current mode be the same that is in regs? What if they are different, is that an error? Does regs represent the registers for the current mode before or after setting the CPSR? Leaving it as it was raises less questions in my mind. I understand QEMU has the same structure (that is where it was taken from), but we aren't building an interface between QEMU and KVM. We are building an interface that is used by QEMU. Because they have similarities doesn't mean we should complicate things for anyone else that might use the interface. On the same note, how does QEMU "back" their regs? When you do the set_regs are regs and the backed regs consistent? Feel free to move the testTrans.make to the svn repository, it will require some fixup though because it needs arm_translate.c and whatever header files that points to. Finally, the name change doesn't matter to me either way, I was only curious as to the motivation. Thanks, Brian Quoting Christoffer Dall <christofferdall at christofferdall.dk>: > Hi. > > Brian, you're right about the main branch. That was just stupid on my part. > > The changes to kvm_regs is because the role of kvm_regs is mainly to > transfer registers between kvm_vcpu->arch and the QEMU CPUState struct. QEMU > CPUState and kvm_vcpu had almost exact definitions and it was simply > unnecessary complexity for debugging to have a different scheme for an > intermediate step. Do you agree, or am I missing something here? > > I completetly understand if you don't want to change many lines of code, > which is also what I promised to do in earlier e-mails. I modified kvmtest > to work with the modified kvm_regs, but I don't know where to look for more > code dependent on this? > > By the way, when I come to think of this, should we move the special > makefile from the kernel source to the svn source like the kvmtest or is it > easier to keep it in the kernel source? > > Regarding the kvmarm_xxx prefix, maybe you're right. I didn't realize it was > only for interface functions. I just changed the names going over them to > use lower-case more "kernel looking" code, and then it was convenient with > the kvmarm_xxx prefix for code completion in the editor, but I'll change it > back if you think so. > > In general, maybe we should have someone write a "naming" and "code style" > convention page in the wiki, which we can discuss for our KVM code. > > Regards, > Christoffer > > On Sun, Apr 12, 2009 at 8:24 PM, Brian Smith <bls2129 at columbia.edu> wrote: > >> I'm fine with the changes to the makefile and moving the headers to >> /usr/src/android/kernel. >> >> For the commits you made to the master branch, that is unacceptable. I >> have been very careful not to merge to the master branch before letting >> you give the okay, I expect the same be done. I don't care about the >> formatting/name changes so much as the incompatible and redundant >> changes you made to kvm_regs. Please tell me why this is necessary, and >> then I will start thinking about reworking my 50+ kvm_regs scaffolding I >> have created for testing the emulation code. >> >> For the name changes to kvmarm_xxx, did these become interfaces? I was >> under the impression that kvmarm_xxx type stuff should only be for >> interfaces, and if a function is internal to that source file that >> prefix shouldn't be used. I don't care either way, I'm just curious. >> >> Regards, >> Brian >> >> Christoffer Dall wrote: >> > Hi. >> > >> > I changed the kvmtest Makefile to be more general. Thus, it assumes >> > that the "arm-none-linux-gnueabi-*" tools are on the PATH and do not >> > reference these absolutely. This follows standard convention for use >> > of gcc and makes the Makefile portable across our different machines. >> > >> > Unfortunately we have to directly reference a path for our cusom >> > kernel, and we have different locations for this one. Therefore, I >> > though of the best 'convention' to follow and link with headers from >> > /usr/src/android/kernel/... If you don't keep the source there (as I >> > didn't for instance) create a symbolic link to you kernel source. >> > >> > This way we don't have to modify the Makefile on each commit. >> > >> > Also, as can be seen in a recent git commit message, I cleaned up some >> > unused files in the kernel source and consolidated a few header files. >> > I will make sure that this merges with whatever commits we have after >> > this weekend. >> > >> > >> > Best regards, >> > Christoffer >> > ------------------------------------------------------------------------ >> > >> > _______________________________________________ >> > Android-virt mailing list >> > Android-virt at lists.cs.columbia.edu >> > https://lists.cs.columbia.edu/cucslists/listinfo/android-virt >> > >> _______________________________________________ >> Android-virt mailing list >> Android-virt at lists.cs.columbia.edu >> https://lists.cs.columbia.edu/cucslists/listinfo/android-virt >> >
