Monday, August 31, 2015

On SystemVerilog Interface Polymorphism and Extendability

A state of the art SystemVerilog simulation environment consists of two separate worlds. There is the static world, where interface and modules (including the DUT) exist. The other one, the dynamic world, consists of object instances that make up the testbench. The most widespread way to connect these two together is using the virtual interface construct, which is just a pompous way of describing a "reference" to an interface. I will assume that everybody is well acquainted with this concept.

A very big problem that exists with this approach is that it creates a very tight coupling between the interfaces and the classes that are supposed to interact with them. Because of this, for example, it becomes very cumbersome to handle parameterized interfaces, since any interface parameters need to be propagated to the coupled classes too.

There is an alternative to using virtual interfaces, as this paper shows. It builds on an older paper which first championed using abstract classes to define how to operate on signals. An interface would declare a concrete class that implements the real signal accesses. The paper was written in a time before OVM or UVM, when bus functional models (BFMs) were all the rage, hence it proposes offloading the logic to drive signals into these classes. Other authors have recognized the potential of this approach and built on the BFM idea, showing how to incorporate it into OVM and, more recently, UVM environments. While I do like the idea of dumping virtual interfaces and using the abstract class based approach, I disagree with giving these classes too much responsibility. Read on and you'll understand why.

The introduction might seem rather dry, but a concrete example will illustrate things better. Let's look at a simple APB master driver. The interface it's supposed to drive would look like this:

interface vgm_apb_master_interface(input bit PRESETn, input bit PCLK);
  logic        PSEL;
  logic        PENABLE;
  logic [31:0] PADDR;
  logic        PWRITE;
  logic [31:0] PWDATA;

  logic        PREADY;
  logic [31:0] PRDATA;


  clocking cb @(posedge PCLK);
    input output PSEL;
    input output PENABLE;
    input output PADDR;
    input output PWRITE;
    input output PWDATA;

    input        PREADY;
    input        PRDATA;
  endclocking
endinterface

The clocking block helps enforce synchronicity to the clock and signal access control. The PREADY and PRDATA signals are only tagged as inputs, since the master is only supposed to sample the values that get driven by the slave. Normally, a master contains an array of PSELx signals, but for this example I want to keep things simple.

The first thing an APB UVC needs is a transaction class to model a transfer:

class transfer extends uvm_sequence_item;
  rand direction_e direction;
  rand bit [31:0] address;
  rand bit [31:0] data;
  rand int unsigned delay;

  // ...
endclass

A master driver would get transfers from a sequencer and convert them to signal activity:

class master_driver extends uvm_driver #(transfer);
  protected virtual vgm_apb_master_interface vif;


  function new(string name, uvm_component parent);
    super.new(name, parent);
  endfunction


  virtual task run_phase(uvm_phase phase);
    forever begin
      reset();
      fork
        get_and_drive();
        @(negedge vif.PRESETn);
      join_any
      disable fork;
    end
  endtask

  // ...
endclass

When reset is asserted, it's supposed to drive the signals to their idle values. It does this via its virtual interface, which points to the interface that is connected to the DUT:

  protected virtual task reset();
    vif.PSEL = 0;
    vif.PENABLE = 0;
    vif.PWRITE = 0;
    vif.PADDR = 0;
    vif.PWDATA = 0;
    @(vif.cb iff vif.PRESETn);
  endtask

When it receives a transfer from the sequencer, it will drive the appropriate sequence of signal changes that represent that transfer:

  local task get_and_drive();
    forever begin
      seq_item_port.get_next_item(req);
      drive(req);
      seq_item_port.item_done();
    end
  endtask


  protected virtual task drive(vgm_apb::transfer transfer);
    drive_setup_phase(transfer);
    drive_access_phase(transfer);
    vif.cb.PSEL <= 0;
    vif.cb.PENABLE <= 0;
  endtask

I've split the drive(...) task into two sub-tasks, as per the protocol specification. During the access phase, the address and the direction are placed:

  protected virtual task drive_setup_phase(vgm_apb::transfer transfer);
    vif.cb.PSEL <= 1;
    vif.cb.PWRITE <= transfer.direction;
    vif.cb.PADDR <= transfer.address;
    @(vif.cb);
  endtask

During the data phase, either the write data is placed or the read data is sampled:

  protected virtual task drive_access_phase(vgm_apb::transfer transfer);
    vif.cb.PENABLE <= 1;
    if (transfer.direction == WRITE)
      vif.cb.PWDATA <= transfer.data;

    @(vif.cb iff vif.cb.PREADY);

    if (transfer.direction == READ)
      transfer.data = vif.cb.PRDATA;
  endtask

And there we have it: one APB master driver that can handle the vanilla protocol. Now let's say that we want to slightly modify this protocol to use only one bidirectional data bus. Instead PWDATA and PRDATA we'll have a PDATA signal:

interface vgm_apb_tri_master_interface(input bit PRESETn, input bit PCLK,
  inout wire [31:0] PDATA
);
  logic        PSEL;
  logic        PENABLE;
  logic [31:0] PADDR;
  logic        PWRITE;
  logic [31:0] PDATA_drive;

  logic        PREADY;


  clocking cb @(posedge PCLK);
    input output PSEL;
    input output PENABLE;
    input output PADDR;
    input output PWRITE;
    input        PDATA;
          output PDATA_drive;

    input        PREADY;
  endclocking

  assign PDATA = PDATA_drive;
endinterface

I've had to make PDATA an inout wire port, because when I defined it as a simple wire inside the interface my simulator complained later on when I tried to drive it from a testbench instantiating the interface. Since it isn't possible to drive wires directly from procedural statements (i.e. from classes), we'll need an intermediary signal to drive, hence the PDATA_drive signal. The input clockvar will sample the wire, to give us the resolved value of the bus. The output clockvar will drive the intermediary signal synchronously. Normally it should have been possible to also call the output clockvar PDATA, but the simulator got confused, even though the syntax is legal as per the LRM.

This new protocol is almost exactly the same as the standard one, with the minor difference that we need to drive PDATA as 'z (high impedance) during idle phases. Only when driving a write transfer would we drive real data onto it. This suggests that we should be able to easily tweak the vanilla master driver to achieve this behavior. Unfortunately, the type of the vif field, virtual vgm_apb_master_interface, makes it impossible to extend this driver to use our new interface. This is the tight coupling I was talking about at the beginning of this post.

The only thing we could reuse from the vanilla APB UVC is the transfer class. The driver we'd have to implement again by copying and pasting the original code and patching it. Here's a summary of the changes we'd need to make:

class master_driver extends uvm_driver #(transfer);
  protected virtual vgm_apb_tri_master_interface vif;

  protected virtual task reset();
    // ...
    vif.PDATA_drive = 'z;
    // ...
  endtask

  protected virtual task drive(vgm_apb::transfer transfer);
    // ...
    vif.cb.PDATA_drive <= 'z;
  endtask

  protected virtual task drive_access_phase(vgm_apb::transfer transfer);
    // ...
    if (transfer.direction == WRITE)
      vif.cb.PDATA_drive <= transfer.data;
    // ...
    if (transfer.direction == READ)
      transfer.data = vif.cb.PDATA;
  endtask
endclass

We'd need to change very little. First, the vif field should have the new type. Second, we'd need to float PDATA after reset and after a transfer. Third we'd need to update the drive_access_phase(...) task to use the PDATA clockvars instead of PWDATA and PRDATA. Aside from this, everything else would stay the same. Clipboard-based inheritance at its finest!

In the ideal case, we should have been able to leverage the vanilla driver implementation and use OOP techniques to make the very few required tweaks, for example in a subclass. That's the whole reason verification evolved to look more and more like software programming, to allow us, among others, to easily deal with such situations.

The root of all evil is that the driver had to know the type of the (virtual) interface it was driving. The abstract class approach presented in the papers from above can do away with such coupling (you did browse them at least, didn't you?). Instead of the driver getting a virtual interface, it will delegate the task of actually driving the signals to a proxy object. Such a proxy class will contain accessor functions for each of the signals:

virtual class master_interface_proxy;
  pure virtual function bit get_sel();
  pure virtual function void set_sel(bit sel);

  pure virtual function bit get_enable();
  pure virtual function void set_enable(bit enable);

  pure virtual function logic [31:0] get_address();
  pure virtual function void set_address(logic [31:0] address);

  pure virtual function direction_e get_direction();
  pure virtual function void set_direction(direction_e direction);

  pure virtual function logic [31:0] get_write_data();
  pure virtual function void set_write_data(logic [31:0] data);

  pure virtual function logic [31:0] get_read_data();
  pure virtual function bit get_ready();
endclass

Notice that this class is defined as virtual, meaning that it's "incomplete" (the method definitions are missing) and as such can't be instantiated. A concrete class which actually performs signal accesses has to be defined inside the interface we want to drive:

interface vgm_apb_master_interface(input bit PRESETn, input bit PCLK);
  // ...

  class interface_proxy extends vgm_apb::master_interface_proxy;
    virtual function bit get_sel();
      return cb.PSEL;
    endfunction

    virtual function void set_sel(bit sel);
      cb.PSEL <= sel;
    endfunction

    // ...

    virtual function logic [31:0] get_write_data();
      return cb.PWDATA;
    endfunction

    virtual function void set_write_data(logic [31:0] data);
      cb.PWDATA <= data;
    endfunction

    virtual function logic [31:0] get_read_data();
      return cb.PRDATA;
    endfunction

    // ...
  endclass

  interface_proxy proxy = new();
endinterface

Instead of a virtual interface, the driver gets a pointer to the interface proxy instantiated inside the interface. I chose to do this via an accessor function (which is going to pay off later on):

class master_driver extends uvm_driver #(transfer);
  protected master_interface_proxy if_proxy;

  virtual function void set_interface_proxy(master_interface_proxy if_proxy);
    this.if_proxy = if_proxy;
  endfunction

  // ...
endclass

It uses uses the proxy to manipulate the pins. Instead of referencing signals from a virtual interface, we will call the appropriate functions on the proxy:

class master_driver extends uvm_driver #(transfer);
  // ...

  protected virtual task drive_setup_phase(vgm_apb::transfer transfer);
    if_proxy.set_sel(1);
    if_proxy.set_direction(transfer.direction);
    if_proxy.set_address(transfer.address);
    if_proxy.wait_for_clk();
  endtask

  // ...
endclass

We can do the same for the drive_access_phase(...) task:

  protected virtual task drive_access_phase(vgm_apb::transfer transfer);
    if_proxy.set_enable(1);
    if (transfer.direction == WRITE)
      if_proxy.set_write_data(transfer.data);

    do
      if_proxy.wait_for_clk();
    while (!if_proxy.get_ready());

    if (transfer.direction == READ)
      transfer.data = if_proxy.get_read_data();
  endtask

I avoided talking about the topics of reset and clock. For synchronization, the proxy should also provide tasks to wait for a clock edge and for the beginning and end of reset. I'll skip the implementations of these since they're pretty trivial (but you can find them in the full example on SourceForge). You might have also noticed that our get/set_*(...) functions all reference clocking block variables. This means we won't be able to asynchronously drive signals to their idle values when reset is asserted. Providing asynchronous versions of the methods seem like overkill, so what we could do is define a reset() function inside the proxy to take care of this:

interface vgm_apb_master_interface(input bit PRESETn, input bit PCLK);
  // ...

  class interface_proxy extends vgm_apb::master_interface_proxy;
    virtual function void reset();
      PSEL <= 0;
      PENABLE <= 0;
      PADDR <= 0;
      PWRITE <= 0;
    endfunction

    // ...
  endclass
endinterface

Now we've gotten rid of any references to any virtual interface inside our driver by adding another layer of indirection between it and the actual interface it's supposed to be driving. Was it all worth the extra complexity? Let's see...

We were promised that if we decouple the driver from the interface all sorts of good things will happen. Let's go back to the tristate APB variant. First, we'll need to declare the concrete interface proxy that can talk to the vgm_apb_tri_master_interface:

interface vgm_apb_tri_master_interface(input bit PRESETn, input bit PCLK,
  inout wire [31:0] PDATA
);
  // ...

  class interface_proxy extends vgm_apb::master_interface_proxy;
    // ...

    virtual function void reset();
      // ...
      PDATA_drive <= 'z;
    endfunction

    // ...

    virtual function logic [31:0] get_write_data();
      return cb.PDATA;
    endfunction

    virtual function void set_write_data(logic [31:0] data);
      cb.PDATA_drive <= data;
    endfunction

    virtual function logic [31:0] get_read_data();
      return get_write_data();
    endfunction

    // ..
  endclass

  interface_proxy proxy = new();
endinterface

Aside from an extra line in the reset() function and telling the data accessors to operate on PDATA instead of PWDATA/PRDATA, the method definitions stay the same. This means that whenever the driver will try to read or write data it will also indirectly be working with PDATA. Even though the set/get_*_data(...) functions now do different things, the driver doesn't care and nor does it need to. The code we already have will work directly with the new interface. Now that's polymorphism right there! We do need to make one small alteration to float PDATA after a transfer has taken place. This is easily done in a subclass:

class master_driver extends vgm_apb::master_driver;
  function new(string name, uvm_component parent);
    super.new(name, parent);
  endfunction

  protected virtual task drive(vgm_apb::transfer transfer);
    super.drive(transfer);
    if_proxy.set_write_data('z);
  endtask
endclass

And that's it. No duplication of driver functionality. Just one extra statement. The doubled up code we have in both concrete proxy classes to actually access the signals is a necessary evil. That code is pretty much boilerplate and as long as the signals don't change, neither should it.

Now if we would have gone for the BFM approach mentioned in the papers (I hope you read them by now since I kept going back to them), where the proxy classes would have known more about the protocol, such as how to perform a transfer, we would have needed to re-implement these functions in both concrete classes. This is exactly the situation we wanted to avoid if you remember the initial implementations of the two drivers. Granted, we could have built such high level functions out of calls to get_* and set_*, but then again this is exactly what we're doing inside the driver.

After seeing how to implement interface polymorphism, let's tackle extendability. Let's now imagine that we need to implement another slight modification to the APB protocol that supports narrow accesses. AHB connoisseurs should be familiar with this concept. We'll do this by adding another signal, PSIZE, that defines how wide a transfer is, BYTE, HALFWORD or WORD. This new interface will look like this:

interface vgm_apb_ext_master_interface(input bit PRESETn, input bit PCLK);
  logic        PSEL;
  logic        PENABLE;
  logic [31:0] PADDR;
  logic [1:0]  PSIZE;
  logic        PWRITE;
  logic [31:0] PWDATA;

  logic        PREADY;
  logic [31:0] PRDATA;

  // ...
endinterface

The driver will have to know about this signal. To do this, we'll need to add accessors to the abstract interface proxy. We'll do this via inheritance:

virtual class master_interface_proxy extends vgm_apb::master_interface_proxy;
  pure virtual function size_e get_size();
  pure virtual function void set_size(size_e size);
endclass

Inside the vgm_apb_ext_master_interface we'll provide the definitions of the proxy methods:

interface vgm_apb_ext_master_interface(input bit PRESETn, input bit PCLK);
  // ...

  class interface_proxy extends vgm_apb_ext::master_interface_proxy;
    // ...

    virtual function vgm_apb_ext::size_e get_size();
      return vgm_apb_ext::size_e'(cb.PSIZE);
    endfunction

    virtual function void set_size(vgm_apb_ext::size_e size);
      cb.PSIZE <= size;
    endfunction
  endclass

  interface_proxy proxy = new();
endinterface

We'll need to extend our transfer class to add this extra field. The width of the transfer will impose constraints on the address. At the same time, when transferring bytes or half words, it doesn't make sense to randomize the entire data field:

class transfer extends vgm_apb::transfer;
  rand size_e size;

  constraint aligned_address {
    size == HALFWORD -> address[0] == 0;
    size == WORD -> address[1:0] == 0;
  }

  constraint aligned_data {
    size == BYTE -> data == data[7:0];
    size == HALFWORD -> data == data[15:0];
  }

  // ...
endclass

Now that we have all our building blocks in place, we can start writing the new master driver. As before, we can extend the vanilla master driver and add the new behavior. Since the new driver needs access to the get/set_size(...) functions inside the proxy, the if_proxy field of the base driver won't suffice, since it is of type vgm_apb::master_interface_proxy. We'll need a new field of the extended proxy type:

class master_driver extends vgm_apb::master_driver;
  protected master_interface_proxy ext_if_proxy;

  virtual function void set_interface_proxy(
    vgm_apb::master_interface_proxy if_proxy
  );
    if (!$cast(this.ext_if_proxy, if_proxy))
      $fatal(0, "Cast error");
    super.set_interface_proxy(if_proxy);
  endfunction

  virtual function void set_ext_interface_proxy(
    master_interface_proxy if_proxy
  );
    set_interface_proxy(if_proxy);
  endfunction

  // ...
endclass

We'll need to make sure that when calling set_interface_proxy(...), the caller passes us a proxy object of the new type, hence the cast. An even better approach would have been to re-declare the function's argument as having the type vgm_apb_ext::master_interface_proxy, which is a subclass of the original type so it can be substituted for it. This way we wouldn't need the cast and we'd get extra compile time safety. This is possible in other programming languages, but isn't allowed by the SystemVerilog LRM. As a compromise we can define a new set_ext_interface_proxy(...).

Now it's time to implement the driving logic. First off we need to drive PSIZE to the value contained in the transfer we get from the sequencer. We'll need to cast the transfer to the new type so we can access the size field. After we have that, driving PSIZE is just one call to set_size(...) away:

  protected virtual task drive_setup_phase(vgm_apb::transfer transfer);
    vgm_apb_ext::transfer ext_transfer;
    if (!$cast(ext_transfer, transfer))
      `uvm_fatal("CASTERR", "Cast error")

    ext_if_proxy.set_size(ext_transfer.size);
    super.drive_setup_phase(transfer);
  endtask

For the access phase, things get trickier when handing the data. Those of you who've worked with the AHB protocol know that the data busses are split into byte lanes. Depending on the size of the transfer and on its address, different lanes are active:

lanes

Since our bus is 32 bit wide, we can drive four bytes at the same time. This means we need two address bits to differentiate between which byte or half word we are driving. For example, for a byte access to address 0x10 the data will be placed in the BYTE0 lane, for a byte access to address 0x5a the data will be placed in the BYTE2 lane and so on. The concept is similar for half words, while words always occupy the full bus.

In our transfer class, the data field only contains the payload that we want to send. The driver must place this payload onto the appropriate lane(s). Our standard APB implementation didn't do anything like this; it directly drove PWDATA with the contents of the data field. If we perform this alignment on it before calling the base drive_access_phase(...) task we'll achieve our desired effect:

  protected virtual task drive_access_phase(vgm_apb::transfer transfer);
    vgm_apb::transfer transfer_clone = transfer.clone();
    
    transfer_clone.data = transfer.data << (8 * transfer.address[1:0]);
    super.drive_access_phase(transfer_clone);
    transfer.data = transfer_clone.data >> (8 * transfer.address[1:0]);
  endtask

I didn't like the idea of dirtying the data field of the transfer we got from the sequencer so I implemented this alignment and dispatch on a copy. At the same, when coming back from a read transfer, the transfer will get updated with value extracted from PRDATA.

There's not more to it than that. If you want to go through the code yourself, you can find the full examples (in both the virtual interface and abstract class flavors) on SourceForge.

We've seen that with just a few slight tweaks we were able to add a new signal to the protocol. This is something that wouldn't have been possible to do as cleanly by using virtual interfaces. When I say "cleanly" I mean that we've kept strictly separate interfaces for each of the protocol variants. What we wouldn't like to see is some monstrous monolithic interface that contains all signals for all possible variants (i.e. PWDATA/PRDATA, PDATA and PSIZE in our case) and then detailed instructions on how to connect it when wanting to use a specific flavor of the protocol (e.g. for vanilla APB float PDATA and drive PSIZE to 'h3). This is unfortunately the way that things end up getting implemented when trying to support multiple variations of the same protocol.

The downside to this approach is that it requires more code, at least at a first glance. We need to maintain multiple interfaces and there is a lot of duplication when defining the concrete interface proxies for each. This should be a one-off thing, though, and it is effort that's well invested, since it should significantly improve usability. We intend to use our UVCs much longer than it takes to develop them, so any boost in making more user friendly will pay off in the long run.

Drastically reducing the coupling between the static interface and dynamic driver also allows for new protocol variants to be implemented much faster and with much more synergy, leading to better separation of concerns and much cleaner code that is easier to maintain. If you have to maintain multiple flavours of a certain protocol, then I definitely recommend you give them a try!

8 comments:

  1. Hello Tudor,

    I have been following the abstract class posts and was searching for a use case that spells a dire necessity to replace interfaces with abstract classes. Your posts clears the air for it in a great deal.

    Since you mentioned parametrized interfaces at the start of the topic(reference to parametrised interfaces, whose parameter value is changed at runtime using simulator options, causes issues), Will abtract class usage help us to circumvent it.

    Regards,
    Nisreen

    ReplyDelete
    Replies
    1. Using this approach will help, since from a syntax point of view two interfaces with different parameterizations will be two totally different types. Just like 'apb_master_interface' and 'apb_tri_master_interface' or 'apb_ext_master_interface' are totally different types, for the compiler so are 'some_interface #(8)' and 'some_interface #(16)'.

      Delete
  2. This is the same methodology espoused by Jonathan Bromley and Dave Rich in their paper presented at DVCon in 2008: http://www.doulos.com/downloads/events/DVCon_08_abstractBFM_final.pdf

    ReplyDelete
    Replies
    1. That's the "older paper" I reference in the beginning of the post.

      Delete
  3. I detest interfaces.
    did you know that if you had a parametric interface type inside of a parametric class, the type you specify in your class will not get to the interface type???
    Instead, you need to parameterize the class on the interface itself , and declare a specialization somewhere in your package.

    Here's what I do - I think interface should give me the protocol handshake, and if there are phases, by functions or I'd rather use events - doesn't matter
    other than that I just want it to give all the information bits: data, address, byte enable all concatenated together.
    so the interface is just parametric on the width.
    Now the driver and monitor don't care. all monitor does is @sample_event they take the sampled bits and stream them into the transaction class that is their parameter.
    the class itself has the proper pack/unpack, either ovm/uvm or my own, to turn the stream into fields.
    (if you do your own pack/unpack, you can have them in a transaction base, and the transaction you use, can inherit the base + parametric on virtual class that contains parameters of the fields widths, enums, etc)
    All my monitors are the same base type. my base test, overrides them all to with the proper parameterized monitor.
    The driver takes the transaction and streams it to the wires.
    Now I'm immune to designers. They can change the structure of the bus, the width, the protocol at the very last minute, I'm not bothered.

    ReplyDelete
    Replies
    1. That's an interesting use model. It doesn't help if you want to maintain the same interface with multiple variants (like the APB, APB tri and APB ext from the post), because you still need 3 interface definitions.

      That is unless you have some crazy scheme where you only define one interface variable of a very big length and then do part selects to connect the signals:

      interface apb_master_interface #(WIDTH);
      logic [WIDTH-1 : 0] signals_bundle;
      endinterface

      // in the TB
      assign apb.signals_bundle[1:0] = HTRANS;
      assign apb.signals_bundle[33:2] = HADDR;

      This would be pretty crazy for usability. Granted, you could wrap this interface inside another one that exposes the signals with nice names, but you've still got to do those connections yourself. That's conceptually the same approach as in the post, because you'd add a layer of indirection to decouple the real interface from the driver (because it would see the one with the bundle).

      Delete
    2. gp or someone else: could you elaborate on "did you know that if you had a parametric interface type inside of a parametric class, the type you specify in your class will not get to the interface type??? "

      Delete
    3. I think he means something along the lines of:

      interface some_param_interface #(int some_param);
      // ...
      endinterface

      class some_class #(int some_param);
      virtual some_param_interface #(some_param) vif;
      endclass

      Maybe some simulators take offense at declaring 'vif' like that and want something like:

      class some_class #(type vif_type);
      vif_type vif;
      endclass

      Delete