search

wzab / agwb

Support for automatic address map generation and address decoding logic for Wishbone connected hierachical systems
12 stars 6 forks source link

Better handling of BlackBoxes - support for HLS? #61

Open wzab opened 3 years ago

wzab commented 3 years ago

It is likely, that AGWB will be used with HLS-generated IP cores. There may be also other sources of blocks with predefined internal addresses. AGWB supports blackboxes, that can be defined as certain areas in the address space. For IPbus and AMAP-XML backends, the user may define the internal address map of the blackbox. What may be needed, however, is a possibility to create the AGWB internal representation of the blackbox from the user-provided IPbus or AMAP XML. It would allow generating SW interfaces for all backends - C, Forth, Python...

wzab commented 3 years ago

I have compiled a simple HLS project. fir.cpp.gz

#include "fir.h"
static din_t delays[N];

void reset(void)
{
  int i;
  for(i=0;i<N;i++) delays[N]=0;
}

void wz_fir (din_t din, dout_t * dout, coeff_t gain, coeff_t coeffs[N], coeff_t offset)
{
 #pragma HLS INTERFACE axis port=din
#pragma HLS INTERFACE s_axilite port=coeffs
#pragma HLS INTERFACE s_axilite port=gain
#pragma HLS INTERFACE s_axilite port=offset
 #pragma HLS INTERFACE axis port=dout
  int i;
  for(i=N-1;i>0;i--) delays[i]=delays[i-1];
  delays[0] = din;
  accum_t accum = 0;  
  for(i=0;i<N;i++) accum += delays[i]*coeffs[i];
  *dout = (dout_t) accum * gain + offset;
}

The Vivado HLS produces the C "driver" files that contain information about asigned addresses: There is a C header file xwz_fir_hw.h.gz which is hard to analyze, and the JSON file solution0_data.json.gz It contains a nice description of the generated AXI Lite interface

   "s_axi_AXILiteS": {
      "type": "axi4lite",
      "is_adaptor": "true",
      "mode": "slave",
      "port_prefix": "s_axi_AXILiteS",
      "param_prefix": "C_S_AXI_AXILITES",
      "addr_bits": "7",
      "registers": [
        {
          "offset": "0x10",
          "name": "gain_V",
          "access": "W",
          "reset_value": "0x0",
          "description": "Data signal of gain_V",
          "fields": [
            {
              "offset": "0",
              "width": "18",
              "name": "gain_V",
              "access": "W",
              "reset_value": "0",
              "description": "Bit 17 to 0 Data signal of gain_V"
            },
            {
              "offset": "18",
              "width": "14",
              "name": "RESERVED",
              "access": "R",
              "reset_value": "0",
              "description": "Reserved.  0s on read."
            }
          ]
        },
        {
          "offset": "0x40",
          "name": "offset_V",
          "access": "W",
          "reset_value": "0x0",
          "description": "Data signal of offset_V",
          "fields": [
            {
              "offset": "0",
              "width": "18",
              "name": "offset_V",
              "access": "W",
              "reset_value": "0",
              "description": "Bit 17 to 0 Data signal of offset_V"
            },
            {
              "offset": "18",
              "width": "14",
              "name": "RESERVED",
              "access": "R",
              "reset_value": "0",
              "description": "Reserved.  0s on read."
            }
          ]
        }
      ],
      "memories": {"coeffs_V": {
          "offset": "32",
          "range": "32"
        }},
      "ctype": {
        "AWVALID": {"Type": "bool"},
        "AWREADY": {"Type": "bool"},
        "WVALID": {"Type": "bool"},
        "WREADY": {"Type": "bool"},
        "BVALID": {"Type": "bool"},
        "BREADY": {"Type": "bool"},
        "BRESP": {
          "Type": "integer unsigned",
          "Width": "2"
        },
        "ARVALID": {"Type": "bool"},
        "ARREADY": {"Type": "bool"},
        "RVALID": {"Type": "bool"},
        "RREADY": {"Type": "bool"},
        "RRESP": {
          "Type": "integer unsigned",
          "Width": "2"
        },
        "AWADDR": {
          "Type": "integer unsigned",
          "Width": "7"
        },
        "WDATA": {
          "Type": "real fixed signed 14",
          "Width": "18"
        },
        "WSTRB": {
          "Type": "integer unsigned",
          "Width": "4"
        },
        "ARADDR": {
          "Type": "integer unsigned",
          "Width": "7"
        },
        "RDATA": {
          "Type": "real fixed signed 14",
          "Width": "18"
        }
      },
      "data_width": "32",
      "port_width": {
        "ARADDR": "7",
        "AWADDR": "7",
        "RDATA": "32",
        "WDATA": "32",
        "WSTRB": "4"
      }
    }
  },

Probably the JSON file may be used to extract the information needed to connect the IP core generated by HLS to AGWB.

wzab commented 3 years ago

Handling of HLS-generated cores is tightly associated with supporting blackboxes with complex address maps. Up to now, AGWB supports only providing the map of addresses for the blackbox that can be transparently passed to the IPbus XML or to AMAP XML. Other backends can not use that information. A better approach would be building for such blocks a similar description as for normal blocks, but with one difference: the addresses must be fixed.

The internal addresses are assigned in the analyze method. It should be easy to disable it for certain block, requiring that all internal subblocks, registers (or vectors) must have fixed sizes and base addresses.

Then the "analyze" method should be called for children. It can be done by adding a "fixed" attribute to the block. Each block that has this attribute must also have "addr_size" and "addr_base" attributes. The same applies to its registers. All subblocks of such block must be also "fixed". The "analyze" method of "block" class should check the "fixed" attribute, and if it is set, it just copies the addresses and sizes from the corresponding attributes instead of allocating them (of course it should be verified, that the addresses fit into the declared (with "addr_size") address space. Then the "analyze" method should be called for children. The internal representation built for such "fixed" blocks by "analyze" will be the same as for normal blocks.

(Another possibility could be subclassing of "block" and creating a "fixed_block" class with modified "analyze" method? However it seems that it wouldn't be more legible and maintainable.)

The backends should treat such blocks as usual (all internal subblocks and registers have their addresses set. Doesn't matter that they were predefined, not allocated). The only exception is the VHDL backend. It stops at generation of the bus for such block (just as it does for blackboxes now).

The next step would be of course a generator that takes the HLS JSON and converts it into the AGWB XML describing the fixed blocks. The important fact is that it can be separated from AGWB and won't increase its complexity.

wzab commented 3 years ago

Of course the base addresses should be specified only for registers in "fixed blocks" and for its subblocks. The size must be specified for each fixed block (so also for its subblocks). The standard registers (ID, VER, TEST_DEV) should not be generated for fixed blocks.

wzab commented 3 years ago

It seems that instead of "fixed" the new attribute should be called "external".

wzab commented 3 years ago

The first version that seems to handle the "external" blocks properly is available in the repository: https://github.com/wzab/agwb/commit/a19d104d2402374e3bbb8c26f85281f07985ce74 It has very relaxed requirements for the "externals" - the registers may be defined in any order, the size of the block does not have to be equal to 2^N.

The address maps and the Python service routines seem to be generated correctly.

wzab commented 3 years ago

Regarding connecting the AXI-Lite slave to the AGWB-controlled Wishbone, we should check the xaxi4lite_wb_bridge.vhd bridge from OHWR general-cores.

wzab commented 3 years ago

I have implemented a new way of handling the "external blocks with fixed internal structure". To enable better implementation of Relax NG-based syntax validation, the new approach does not use the "external" attributes. Instead of that the "external" blocks must use:

Generally the implemenatation is working, but certain errors are found in the RelaxNG syntax validation:

Investigating them I have found that even in the master branch the NG validation sometimes works incorrectly: https://github.com/wzab/agwb/issues/62

wzab commented 3 years ago

I have solved the problem by switching to DTD-based validation in https://github.com/wzab/agwb/commit/62267d6cf30554e62aa4fec40bf49067892bab96 . The DTD may be generated from RNG with trang. I have added the rng2dtd.sh script for that: https://github.com/wzab/agwb/blob/62267d6cf30554e62aa4fec40bf49067892bab96/src/rng2dtd.sh

wzab commented 3 years ago

The problem with RNG validation has been reported to the Debian (in which I've detected the problem): https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=986343