Review Board 2.0.15


x86: kvm: Fix the KVM CPU in SE and FS on Intel CPUs.

Review Request #2557 - Created Dec. 9, 2014 and updated

Information
Gabe Black
gem5
default
Reviewers
Default
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.

   
Review request changed
Updated (Dec. 10, 2014, 2:11 a.m.)

Summary:

-imported patch wip.patch
+x86: kvm: Fix the KVM CPU in SE and FS on Intel CPUs.

Description:

~  

Changeset 10605:edee8f168abd

  ~

Changeset 10606:aa3eb7453246

   
~  

imported patch wip.patch

  ~

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.

Diff:

Revision 2 (+294 -317)

Show changes

Posted (Dec. 10, 2014, 2:12 a.m.)
Ok, now it's for real. Review away.
  1. Thank you Gabe. I am new to gem5. Can you please let me know where can I find the wip.patch and also what will be command for patching gem5 ? 
Posted (Dec. 10, 2014, 2:30 p.m.)
There is some issue with AMD platforms. A test that used to run in 30 sec is not finishing.

  1. hello world passes. SPEC apps hang.
    
  2. Can you identify where it's getting stuck? It could be there's something wrong with how exception handling is set up, and it gets stuck faulting over and over when a page fault happens, for instance. You could have the KVM CPU print what the IP is each time it regains control and see if they cluster somewhere interesting.
  3. Any luck? Unfortunately the time I have to work on gem5 is pretty limited, so I probably won't be able to dig into this for a while.
  4. on the intel platform, with the test input set, all of cpu2006 integer tests pass, except for astar.
    astar dies with a panic and page fault.
    [m5PageFault:build/X86/arch/x86/pseudo_inst.cc, line 85]
    
    on the amd platform, all of the tests I tried fail to complete.
    time continues to advance, but instructions are not completing.
    
    I am not sure what to do, I need the Intel platform code, but we should not break the AMD side.
    
    Are there any AMD folks I can work with to figure out how to get both sides working?
    
    
  5. Sorry about the late reply, I remember replying on a different mailing list thread about this. I have tried this patch on my AMD system with some short running applications and things worked fine. So, I will run cpu2006 and see if there are any issues.
  6. When running SPEC CPU2006 with latest gem5 on a 6 core Bulldozer, Ubuntu 14.04, linux 3.13.0-34, most of them call exit_group syscall except gromacs which fails because of unimplemented clock_gettime syscall. This means they ran until completion, however I have not yet checked if the output is actually correct.
    
    However, after applying this patch things break and there are unexpected exits in the middle of the execution. Looking more into it and coming up with more details.
    
  7. I am new to gem5. Can someone please let me know where can I find the wip.patch and also what will be command for patching gem5 ? 
  8. I am also having trouble running this patch on a different intel x86 system.
    It looks like there are both segmentation changes, and KVM changes merged together.
    
    Can we split it into 2 separate patches?
    
    I think some of the style cleanup in the segmentation stuff changed the behavior.
    
    If no one else does it, I will attempt a separate minimal patch that just fixes the KVM stuff.
  9. @ Mike Upton,
    
    I am not even able to find the patch and I am actively looking for it.
    Did you manually change the below 6 files ? Can you please let me know ? 
    
    src/arch/x86/process.cc: 8 changes 
    src/arch/x86/system.hh: 1 change
    src/arch/x86/system.cc: 3 changes 
    src/arch/x86/utility.hh: 2 changes
    src/arch/x86/utility.cc: 1 change
    src/cpu/kvm/x86_cpu.cc: 2 changes
    
    The changes listed on this link : http://reviews.gem5.org/r/2557/diff/1-2/
    
    
    
    
Posted (Jan. 15, 2015, 4:44 p.m.)



  
src/arch/x86/regs/misc.hh (Diff revision 2)
 
 
really <32, 0> in the new code?
  1. Yeah, I think that should be 31, 0
Posted (Jan. 16, 2015, 3:28 p.m.)



  
src/arch/x86/utility.hh (Diff revision 2)
 
 
Is there a reason the desc.l does not get set for the dataSegDesc()? code sets p,l,d,g,s.
  1. Yes. The long mode bit only means something for code segment descriptors.
Posted (Jan. 16, 2015, 3:56 p.m.)



  
src/arch/x86/process.cc (Diff revision 2)
 
 
 
when limitHigh and limitLow get set by dataSegDesc(), it seems that limitHigh and limitLow get reversed.
  1. This patch reverses them because they started out the wrong way around.
Posted (Jan. 21, 2015, 1:22 p.m.)



  
src/arch/x86/process.cc (Diff revision 2)
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
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...
  1. 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...
  2. 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.
  3. 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.
  4. 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.
  5. 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?
    
    
src/arch/x86/process.cc (Diff revision 2)
 
 
The AMD version only works with the original definition for star...

I could not get the new code to work.
Posted (Jan. 30, 2017, 2:31 p.m.)

Hi folks. I put together this patch a long time ago, there was some discussion about it, and then I lost track of what was going on with it. Have the issues it causes with AMD cpus been tracked down? Glancing through this again, I think there are a couple things which are genuinely wrong which should fix things regardless of the underlying CPU (like reversed bits, things that shouldn't be set but are), but I wouldn't want to push it if it would break some other things which are currently working. Please update me on the current state of things.

  1. I developed another patch on top that had both AMD and intel working. I cant remember the number, and it should certainly be retested before merging.
    I will need to see if I can fins a current AMD box.

    So the answer to your question is: yes there were issues, the issues were tracked down and there is a additional patch that gets both intel and AMD SE virtualization working. The code has not been tested in 2 years though.

  2. I've been using some version of these patches in my local repository for a while now. I think I may have some patches on top of these to fix other issues, too.

    I've added this as a possible project for a Sprint this Sunday (http://gem5.org/Sprint_Ideas#Push_in_fixes_to_x86_KVM). Any chance you're going to be at HPCA, and want to work on this, Mike or Gabe?