11961 add DDI UFM support to the nvme driver

Review Request #2450 — Created Nov. 11, 2019 and submitted

Information
rejohnst
illumos-gate
11961
Reviewers
general 
This change adds DDI UFM support to the nvme driver
Detailed testing notes are available in the ticket:


https://www.illumos.org/issues/11961
  • 0
  • 0
  • 2
  • 0
  • 2
Description From Last Updated
andy_js
andy_js 
  1. 
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/common/io/nvme/nvme.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    This check seems redundant.
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      The logic this bit of code is trying to express is that all firmware slots are writeable, except for first firmware slot, which is only writeable if id_frmw.fw_readonly is zero.  I suppose if we wanted to make it more concise, I could change lines 4696-4699 to something like:
      

      if (slotno != 0 || nvme->n_idctl->id_frmw.fw_readonly == 0)
      
        attr |= DDI_UFM_ATTR_WRITEABLE;
      
 
      

      2. 



    2. 
 
 
      
 
      
  
 
      
 
      
  
      That seems like an improvement over the current code, which appears to always set DDI_UFM_ATTR_WRITEABLE.
      
 
      

      3. 



    3. 
 
 
      
 
      
  
 
      
 
      
  
      Ack! - Yep. Fixed.
      
 
      

      4. 


 
    


    

 
    

    3. 




  3. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    4. 




  

 
















 





 

  

   




pwinder




   

   







  

 



 

  


   

   

    
 
pwinder


    
   


  

  

  



    




  1. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 

















  2. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/common/io/nvme/nvme.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    




    
    A log is void *, the cast is strictly necessary
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 






  3. 
 
    
  
  
    
  
 
    
  
    
   
    
    
     
      usr/src/uts/common/io/nvme/nvme.c
      


       (Diff revision 1)


      
     
    
    
       
   
    
     
    

    
     
    

       
  
    
 
    

   
    
    
    



    
 
    
  
    
   
   
    
    
     
     
     
     
     
    
    Show all issues
   
  
    
 
    

    



    
    Is this correct?
    
  
    
 
    

 
    
  


    
 
    
 
      

  


    1. 
 
 
      
 
      
  
 
      
 
      
  
      Andy flagged this same bit of code - see comment to him above and let me know if that makes sense.
      
 
      

      2. 


 
    


    

 
    

    4. 




  4. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    5. 




  

 
















 



 

  

   




rejohnst




   

   

  

 



 















 





 

  

   




pwinder




   

   







  

 



 

  


   

   

    
 
pwinder


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 





 

  

   




andy_js




   

   







  

 



 

  


   

   

    
 
andy_js


    
   


  

  

  



    




  1. 
 
    
  
    
   
    Ship It!
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    2. 















  2. 
 
    
  
    
   
    
  
    
 
    

 
    
  


    
 
    
 
      

  

 
    


    

 
    

    3. 




  

 
















 



 

  

   




rejohnst




   

   

  

 



 

  


   

   

    
 Review request changed

    
   


  

  

  






 

  Status:
  

  Closed (submitted)

  
 







 


 










  

 









     

    

   

  

  

   
   Loading...