OK, I believe I have a patch that unifies the code for both AMD and Intel.
Do I post it as a separate review-board item? On Thu, Jan 22, 2015 at 11:32 AM, mike upton <[email protected]> wrote: > This is an automatically generated e-mail. To reply, visit: > http://reviews.gem5.org/r/2557/ > > On January 21st, 2015, 9:22 p.m. UTC, *mike upton* wrote: > > src/arch/x86/process.cc > <http://reviews.gem5.org/r/2557/diff/2/?file=42948#file42948line218> (Diff > revision 2) > > X86_64LiveProcess::initState() > > 217 > > SegDescriptor csLowPLDesc = initDesc; > > 218 > > csLowPLDesc.type.codeOrData = 1; > > 219 > > csLowPLDesc.dpl = 0; > > 220 > > uint64_t csLowPLDescVal = csLowPLDesc; > > 221 > > physProxy.writeBlob(GDTPhysAddr + numGDTEntries * 8, > > 222 > > (uint8_t *)(&csLowPLDescVal), 8); > > 223 > > 224 > > numGDTEntries++; > > 225 > > 226 > > SegSelector csLowPL = 0; > > 227 > > csLowPL.si = numGDTEntries - 1; > > 228 > > csLowPL.rpl = 0; > > 229 > > 230 > > //64 bit data segment > > 231 > > SegDescriptor dsLowPLDesc = initDesc; > > 232 > > dsLowPLDesc.type.codeOrData = 0; > > 233 > > dsLowPLDesc.dpl = 0; > > 234 > > uint64_t dsLowPLDescVal = dsLowPLDesc; > > 235 > > physProxy.writeBlob(GDTPhysAddr + numGDTEntries * 8, > > 236 > > (uint8_t *)(&dsLowPLDescVal), 8); > > For AMD systems, the sys descriptors need to come first. On intel systems > they need to come second. > > I do not know how to resolve... > > On January 21st, 2015, 9:23 p.m. UTC, *mike upton* wrote: > > I have been debugging why patch rb2557 breaks AMD KVM functionality. > > > > > I was hoping to get to code that would work on both intel and AMD platforms, > but am not there yet. > > > > > This patch is to be applied on top of rb2557.patch. > > > > > There are 2 main issues, neither of which I understand well enough to take > much further. > > > > > The first issue is that the order that the segment descriptors get > instantiated in the GDT table seems to matter between AMD and Intel, and they > seem to be mutually incompatible. > > > > > AMD wants: > > csSys > > dsSys > > ds > > cs > > > > > Intel wants: > > ds > > cs > > dsSys > > csSys > > > > > I am not sure the relative ordering of ds and cs within a class matters, only > that AMD wants the Sys ones first, and Intel wants them second. > > > > > There is also an issue with how 'star' gets defined. > > I can not make the Intel code work for AMD. > > > > > Both issues are addressed in this patch. > > > > > The patch makes the AMD system work, but breaks Intel functionality. > > > > > I am also not sure how to upload this into review board. Do I create a > separate patch from TOT, or can I somehow attach this to rb2557. > > > > > Hopefully Gabe or Alexandru can weigh in. I am happy to help, but I am at my > 'Peter Principal Limit' as far as my understanding goes. > > > > > I think it would be really ugly to have a machine-type test to version the > code... > > On January 21st, 2015, 9:48 p.m. UTC, *Gabe Black* wrote: > > You should grab a copy of the architecture manual. From there: > > > STAR—The STAR register has the following fields (unless otherwise noted, all > bits are > read/write): > - SYSRET CS and SS Selectors—Bits 63:48. This field is used to specify both > the CS and SS > selectors loaded into CS and SS during SYSRET. If SYSRET is returning to > 32-bit mode > (either legacy or compatibility), this field is copied directly into the CS > selector field. If > SYSRET is returning to 64-bit mode, the CS selector is set to this field + > 16. SS.Sel is set to > this field + 8, regardless of the target mode. Because SYSRET always returns > to CPL 3, the > RPL bits 49:48 should be initialized to 11b. > - SYSCALL CS and SS Selectors—Bits 47:32. This field is used to specify both > the CS and SS > selectors loaded into CS and SS during SYSCALL. This field is copied directly > into CS.Sel. > SS.Sel is set to this field + 8. Because SYSCALL always switches to CPL 0, > the RPL bits > 33:32 should be initialized to 00b. > > > That's why the order matters and is what it is. > > On January 21st, 2015, 11:15 p.m. UTC, *mike upton* wrote: > > AMD and Intel use different solutions, right? > > AMD: Syscall, sysret > Intel: Sysenter, sysexit > > Do we need independent code streams for each? > > The original code worked for AMD, but not intel. > The current 2557 patch works for Intel, but not AMD. > > On January 21st, 2015, 11:35 p.m. UTC, *Gabe Black* wrote: > > Yeah, there are some differences between the two. I think both support both > pairs of instructions, but I think one or the other only works in 32 bit mode > on for one of the vendors, or something along those lines. At one point I > could have told you exactly what the difference was, but now I'd have to > check the manuals. My expectation/hope is that a single GDT layout would work > for both. I doubt the kernel, for instance, specializes its layout based on > who's CPU it's running on. > > OK, it seems like the feedback is that we do need to runtime test the CPU we > are running on and do CPU specific code. > I certainly can code this up. > > Any pointers to what state is available in the simulator to test? > Or should I just add a cpuHostType() routine that will return Intel, AMD, or > UNKNOWN? > > > > - mike > > On December 10th, 2014, 10:11 a.m. UTC, Gabe Black wrote: > Review request for Default. > By Gabe Black. > > *Updated Dec. 10, 2014, 10:11 a.m.* > *Repository: * gem5 > Description > > Changeset 10606:aa3eb7453246 > --------------------------- > x86: kvm: Fix the KVM CPU in SE and FS on Intel CPUs. > > There were a number of problems with how things were initialized which prevent > VMX from running the simulation as a guest. > > Diffs > > - src/arch/x86/process.cc (8fc6e7a835d1d313e139c9095251105f904ac1b4) > - src/arch/x86/regs/misc.hh (8fc6e7a835d1d313e139c9095251105f904ac1b4) > - src/arch/x86/system.hh (8fc6e7a835d1d313e139c9095251105f904ac1b4) > - src/arch/x86/system.cc (8fc6e7a835d1d313e139c9095251105f904ac1b4) > - src/arch/x86/utility.hh (8fc6e7a835d1d313e139c9095251105f904ac1b4) > - src/arch/x86/utility.cc (8fc6e7a835d1d313e139c9095251105f904ac1b4) > - src/cpu/kvm/x86_cpu.cc (8fc6e7a835d1d313e139c9095251105f904ac1b4) > > View Diff <http://reviews.gem5.org/r/2557/diff/> > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
