Review Board 2.0.15


x86: Move APIC clock divider to Python

Review Request #1709 - Created Feb. 13, 2013 and submitted

Information
Andreas Hansson
gem5
default
Reviewers
Default 
Changeset 9515:408a2b1501a9
---------------------------
x86: Move APIC clock divider to Python

This patch moves the 16x APIC clock divider to the Python code to
avoid the post-instantiation modifications to the clock. The x86 APIC
was the only object setting the clock after creation time and this
required some custom functionality and configuration. With this patch,
the clock multiplier is moved to the Python code and the objects are
instantiated with the appropriate clock.

All regressions passing (excluding t1000 and eio)

Issue Summary

1 1 0 0
Description From Last Updated Status
This seems pretty sketchy. You can always say clock.latency * 9 Nathan Binkert Feb. 20, 2013, 1:16 p.m. Open
Review request changed
Updated (Feb. 18, 2013, 7:10 p.m.)

Status: Closed (submitted)

Nathan Binkert
Posted (Feb. 20, 2013, 1:16 p.m.)



  

   

   
    
    
   


  




   







    

     
     

      

       
        

         
          src/python/m5/params.py
          

           (Diff revision 1)

          
         
        

       
       
        
 


        
 


       
      

     

    

    


     
This seems pretty sketchy.

You can always say clock.latency * 9


     





 

  
  
  
  
  
  Show all issues
 




     




  

   

   
    
    

   
  1. 
    
    
     
    
      
     
    
     
    In a few patches down the line, this will be a DerivedClockDomain with a divider of 16. I would suggest to leave it as it is until then.

Out of curiosity, would you be able to do Parent.clock.latency * 16?
    
    
    
   
    2. 


   
  2. 
    
    
     
    
      
     
    
     
    You mean that you will be removing the __mul__?  I'm basically objecting to allowing math on a Clock. A clock can legitimately be thought of as either a latency or a frequency and you need to specify which way you're thinking before you start doing math.  I don't think that this should be in the tree at all, and if possible it should be folded out if you are going to change it anyway.

You should be able to do Parent.clock.latency.  You can do Self.clock.latency and the code for the two proxies is shared.

There's this way old patch that I don't think ever made it into the tree: http://reviews.gem5.org/r/9/
    
    
    
   
    3. 


   
  3. 
    
    
     
    
      
     
    
     
    The goal here is to allow DVFS and move away from treating clocks as a value (or as you suggested in the referenced patch: translate cycles to time at instantiation time). I agree that there is an ambiguity though with the frequency vs period.

In a not too distant future, the multiplication operator will be removed, and the fact that it is a clock divider will be made explicit by having a derived clock domain that is based on a parent clock domain, with a divider of 16. 

It would be great if we can live with this for now and I hope that the clock domains with dividers is an acceptable solution.
    
    
    
   
    4.