Review of AHB verification Agent: mainly on the inter class communication of REQ, RES between Driver and Sequence: Randomization for Burst mode: and Scoreboard comparison using associative storage

[vinayp1209]

Firstly, apologies for lengthy question. As I’m a beginner in UVM and SystemVerilog any suggestion and constructive criticism is welcome!!

class ahb_sequence extends uvm_sequence#(type ahb_p_req = ahb_packet, type ahb_p_rsp = ahb_p_req);
	`uvm_sequence_utils(ahb_sequence,ahb_sequencer) 
	
	req = ahb_p_req::type_id::create("req");
	int inc_undef_burst_hwrite;
	
	function new (string name ="ahb_sequence"); 
		super.new(name);
	endfunction : new
	
	virtual task body();
		uvm_report_info("seq","before uvm_do",UVM_NONE);
				
		/////////////
		///  vvv  ///
		/////////////
		
		start_item(req);
		if(!req.randomize())
		begin
			`uvm_error("seq", "randomization failure for req")
		end
		int inc_undef_burst_hwrite = req.hwrite;
		if (req.hburst != 0 && req.trans=='d2)			// Entering BURST mode write/read transaction
		begin		
			casez(req.hburst)							// case for one of the burst operation, in our DUT please ignore the wrapping burst 
				3'b01?:		
				begin		
					repeat(3)							// 4-beat incrementing burst (INCR4), we use repeat 3, as first transfer has already been initiated
					begin		
						finish_item(req);		
						get_response(rsp); 				// complete of first transfer of Burst and from next cycle address will be incrementing by one
						start_item(req);
						if (req.htrans==2)
							req.trans='d3;
						if (req.hwrite==1)
							req.randomize(hwdata);		// radomize only H-WRITE-DATA if its a write Burst operation
					end
				end
				3'b10?:
				begin
					repeat(7)							// 8-beat incrementing burst (INCR8), we use repeat 7, as first transfer has already been initiated
					begin
						finish_item(req);
						get_response(rsp); 				// complete of first transfer of Burst and from next cycle address will be incrementing by one
						start_item(req);
						if (req.htrans==2)
							req.trans='d3;
						if (req.hwrite==1)
							req.randomize(hwdata);		// radomize only H-WRITE-DATA if its a write Burst operation
					end
				end
				3'b11?:
				begin
					repeat(15)							// 16-beat incrementing burst (INCR16), we use repeat 15, as first transfer has already been initiated
					begin	
						finish_item(req);
						get_response(rsp); 				// complete of first transfer of Burst and from next cycle address will be incrementing by one
						start_item(req);
						if (req.htrans==2)
							req.trans='d3;
						if (req.hwrite==1)
							req.randomize(hwdata);		// radomize only H-WRITE-DATA if its a write Burst operation
					end
				end
				3'b001:
				begin
					do
					begin
						finish_item(req);
						get_response(rsp); 				// complete of first transfer of Burst and from next cycle address will be incrementing by one
						start_item(req);
						if (req.htrans==2)
							req.trans='d3;
						req.randomize(hwdata);
						req.randomize(hwrite);
					end
					while(inc_undef_burst_hwrite == req.hwrite)
				end
			endcase
		end
		
		finish_item(req);
		get_response(rsp); // type of rsp = my_sequence_item2

	endtask : body
endclass : ahb_sequence

class ahb_driver extends uvm_driver#(type ahb_p_req = ahb_packet, type ahb_p_rsp = ahb_p_req);
	`uvm_component_utils(ahb_driver)
	//Interface and packet declaration
	virtual ahb_if vif; 
	// ahb_packet pkt; vvv
	
	function new(string name, uvm_component parent); 
		super.new(name, parent);
	endfunction: new
	
	function void build_phase(uvm_phase phase); 
		super.build_phase(phase);
		void'(uvm_resource_db#(virtual ahb_if)::read_by_name(.scope("ifs"),.name("ahb_if"), .val(vif)));
		uvm_config_db#(virtual interface ahb_if)::get(this,"*","ahb_if",vif); 
	endfunction: build_phase
	
	task run_phase(uvm_phase phase);
		@(posedge vif.HRESETN) 
		begin 
			while(1) 
			begin
				//Drive the packet at the negative edge of clock
				@(negedge vif.HCLK) 
				begin
					seq_item_port.get_next_item(req); 
					uvm_report_info(get_full_name(),"Printing the AHB packet",UVM_NONE); 
					req.print();
					@(posedge vif.HCLK) 
					begin 
						vif.HWRITE = req.hwrite; 
						vif.HADDR = req.haddr; 
						if(req.hwdata==1)
							vif.HWDATA = req.hwdata; 
						vif.HTRANS = req.htrans; 
						vif.HSIZE = req.hsize; 
						vif.HBURST = req.hburst;
					end
					
					seq_item_port.item_done(); 
					//vvv
					rsp = ahb_p_rsp::type_id::create("rsp"); // type of rsp = ahb_p_rsp
					rsp.set_id_info(req);
					rsp.hresp = vif.HRESP;
					if(vif.HRESP)
					begin
						`uvm_error("seq", "Error in transfer response from slave")
					end
					seq_item_port.put(rsp);
					//vvv
				end
			end 
		end
	endtask: run_phase

endclass: ahb_driver

class ahb_scoreboard extends uvm_scoreboard;
	`uvm_component_utils(ahb_scoreboard)
	
	uvm_analysis_imp #(ahb_packet,ahb_scoreboard) scb_port; 
	ahb_packet ahb_scpkt;
	
	logic[31:0] stor_assos[*] ;
	logic[31:0] burst_addr[$];
	logic[31:0] burst_addr_temp;
	
	function compare (string name,uvm_component parent); 
		scb_port = new("scb_port",this); 
	endfunction : compare
	
	function new (string name,uvm_component parent); 
		super.new(name,parent);
		scb_port = new("scb_port",this); 
	endfunction : new
	
	virtual function void connect(); 
		super.connect();
		uvm_report_info("scb","connect method in scoreboard",UVM_NONE); 
	endfunction : connect
	
	virtual task run_phase (uvm_phase phase); 
	endtask : run_phase
	
	virtual function write(ahb_packet ahb_mpkt); 
		uvm_report_info("scb","received packet from Monitor",UVM_NONE); 
		ahb_scpkt = new();
		$cast(ahb_scpkt,ahb_mpkt.clone());
		ahb_scpkt.print();
		
		/////////////
		///  vvv  ///
		/////////////
		
		if(ahb_scpkt.hresetn =='b1 )
		begin
			if(ahb_scpkt.hwrite == 'b1)
			begin
				case(ahb_scpkt.htrans)
					2b'10: 
					begin
						stor_assos[ahb_scpkt.haddr]	= ahb_scpkt.hwdata;		// Copy WRITE operation for non-BURST WRITE
						if(ahb_scpkt.hburst != 0)
							burst_addr.push_back(ahb_scpkt.haddr);			// Since we do not know the next cycle will be Burst READ we save the current memory address
						break;
					end
					2b'11:
					begin
						casez(ahb_scpkt.hburst)
							3'b01?:
								begin
									repeat(3)
										burst_copy(ahb_scpkt);
									break;
								end
							3'b10?:
								begin
									repeat(7)
										burst_copy(ahb_scpkt);
									break;
								end
							3'b11?:
								begin
									repeat(15)
										burst_copy(ahb_scpkt);
									break;
								end
							3'b001:
								begin
									while(ahb_scpkt.hwrite == 'b1)
										burst_copy(ahb_scpkt);
									break;
								end
							default:
								uvm_report_error("scb","Condition not valid");
						endcase
						break;
					end
					default:
						uvm_report_info("scb","HTRANS is either 2'b00 or 2'b01 so need not write",UVM_NONE);
				endcase
			end
			else
			begin
				if(stor_assos.exists[ahb_scpkt.haddr])
					case(ahb_scpkt.htrans)
						2b'10: 
							begin
								if (stor_assos[ahb_cmp_pkt.haddr]	== ahb_cmp_pkt.hrdata)		// Compare for non-burst read operations
									uvm_report_info("scb","Read data matches the written data",UVM_NONE);
								else 
									uvm_report_error("scb","Read data does not matches the written data");
								if(ahb_scpkt.hburst != 0)
									burst_addr.push_back(ahb_scpkt.haddr);		// Since we do not know the next cycle will be Burst READ we save the current memory address
								break
							end
						2b'11:
							begin
								casez(ahb_scpkt.hburst)
									3'b01?:
										begin
											repeat(3)
												burst_compare(ahb_scpkt);
											break;
										end
									3'b10?:
										begin
											repeat(7)
												burst_compare(ahb_scpkt);
											break;
										end
									3'b11?:
										begin
											repeat(15)
												burst_compare(ahb_scpkt);
											break;
										end
									3'b001:
										begin
											while(ahb_scpkt.hwrite == 'b0)
												burst_compare(ahb_scpkt);
											break;
										end
									default:
										uvm_report_error("scb","Condition not valid");
								endcase
								break;
							end
						default:
					endcase
			end
		end
		/////////////
		///  vvv  ///
		/////////////
	endfunction : write

	function void burst_compare (ahb_packet ahb_cmp_pkt)
		burst_addr_temp = burst_addr.pop_back();			// 	Pop the previous address
		burst_addr_temp++;									// 	Increment by one
		if (stor_assos[burst_addr_temp]	== ahb_cmp_pkt.hrdata)
			uvm_report_info("scb","Read data matches the written data",UVM_NONE);
		else 
			uvm_report_error("scb","Read data does not matches the written data");
		burst_addr.push_back(burst_addr_temp);				//	Save the new adress in the Queue
	endfunction

	function void burst_copy (ahb_packet ahb_cpy_pkt)
		burst_addr_temp = burst_addr.pop_back();			// 	Pop the previous address
		burst_addr_temp++;                                  // 	Increment by one
		stor_assos[burst_addr_temp]	= ahb_cpy_pkt.hwdata;   //	Save in memory location
		burst_addr.push_back(burst_addr_temp);              //	Save the new adress in the Queue
	endfunction
	
endclass : ahb_scoreboard

[Srini]

In reply to vvv:

Few low hanging fruits (from a review perspective, non-exhaustive):

1. Avoid uvm_sequence_utils - it is removed from 1.2 onwards
2. uvm_info, uvm_error macros are handier than uvm_report_info etc.
3. seq::body should call crate_item inside the loop (as of now you have it outside the loop) - this will enable late factory overrides.
4. Use get_name() for IDs in messages than inventing your own
5. Change: scoreboard::connect → scoreboard::connect_phase
6. Driver uses posedge/negedge - instead use clocking blocks and avoid edges in driver/monitor
7. signal level comparisons could better use === instead of ==
8. Better indentation (2 space is what I prefer)
9. req.randomize → UVM internally uses a pre-defined ID RNDFLD, better stick to that: e.g.

   `uvm_warning("RNDFLD", "Randomization failed in uvm_do_with action")

(In fact I have a tiny macro `vw_uvm_rand for doing that)
10. Where are the assertions BTW? (Perhaps not shown, but ideal to use SVA for standard protocols like this)

Good Luck
Srini
www.verifworks.com

[vinayp1209]

In reply to Srini @ CVCblr.com:

Thanks a ton Srini, I appreciate for taking time and reviewing my code, I will surely consider your suggestions and follow it.
In addition, I had following doubts in the above code,

1. Rsp sent by driver to sequence is good in my code? I’m doubtful about that and I have declared those variable correctly in the class?
2. I’m I using correct way to store the written variable in scoreboard?? (including burst mode)
3. If u ask me, assertions are my weak point could you please tell me why we need to use it here in this case even though we are verifying it in other ways.
4. Also I have a question on how I can implement assertions in UVM or should I instantiate another module in ‘top-module’ and go ahead with concurrent assertion in it??
   It would be great if you could share an example protocol verification which uses both UVM and assertions.

Once again thank you for your time.