black-parrot-hdk / zynq-parrot

BlackParrot on Zynq
BSD 3-Clause "New" or "Revised" License
25 stars 13 forks source link

PS gets corrupted BP output #59

Closed Yuan-Mao closed 1 year ago

Yuan-Mao commented 1 year ago

Commit I'm using: https://github.com/black-parrot-hdk/zynq-parrot/commit/556d15c0b24679df2040e4f1edf0c4f8d73f4ec1

In the BP example, normally no matter how slow the PS reads the BP outputs from PL, the data should still be complete without missing/corrupting. However if we write a simple program like the following:

int main()
{
    for(;;) {
        printf("Hello how are you doing?\n");
    }
    return 0;
}

... and modify the following function in ps.cpp to simulate the slow polling in PS:

void *device_poll(void *vargp) {
  bp_zynq_pl *zpl = (bp_zynq_pl *)vargp;
  unsigned select = 0;
  while (1) {
#ifndef FPGA
    zpl->axil_poll();
#endif

    if((select++ % 128) == 0) {
      // keep reading as long as there is data
      if (zpl->axil_read(GP0_RD_PL2PS_FIFO_CTRS + gp0_addr_base) != 0) {
        decode_bp_output(zpl, zpl->axil_read(GP0_RD_PL2PS_FIFO_DATA + gp0_addr_base));
      }   
    } else {
        zpl->axil_read(GP0_RD_CSR_RESET + gp0_addr_base);
    }   

    // break loop when all cores done
    if (done_vec.all()) {
      break;
    }   
  }
  bsg_pr_info("Exiting from pthread\n");

  return NULL;
}

Then the output got by PS will be corrupted/missing.

After looking at the waveform, the problem could be from BP instead of the AXI bus, as the strange data do actually come out from BP unicore.

I have also seen cases where PS also gets missing output from BP in simulation even if we do not simulate the slow PS polling like with the above code, but currently I do not have test cases that can reproduce that.

dpetrisko commented 1 year ago

Hi, interestingly I only see this occurring when

    } else {
        zpl->axil_read(GP0_RD_CSR_RESET + gp0_addr_base);
    }   

Removing this else, the program runs correctly

Yuan-Mao commented 1 year ago

That's expected, because we use that "else" to forward the simulation time. Without this the c++ program will just keep looping for 128 times without advancing its simulation time and hence fail to simulate the slow PS polling effect.

dpetrisko commented 1 year ago

oh got it, basically zpl->tick();

dpetrisko commented 1 year ago

https://github.com/black-parrot-hdk/zynq-parrot/pull/60 exposes the bug and proposes the solution

Yuan-Mao commented 1 year ago

60 still might not work in some other cases. I have found another test case that potentially exposes other problems:

#include <stdio.h>
#include <assert.h>
#include <stdint.h>
#include "bp_utils.h"

#define PACKET_SIZE 1024
#define SCRATCHPAD_BASE 0x1000000UL

char buf[2048] __attribute__ ((aligned (4)));
int total_send_count;
volatile int done;

// valid op_size: 1, 2, 4
void write_packet(const char buf[], int size, int op_size)
{
    assert(op_size == 1 || op_size == 2 || op_size == 4); 
    // write packet
    int base;
    int word = size / op_size;
    int offset = size % op_size;
    assert(offset == 0); 
    for(base = 0;base < word * op_size;base += op_size) {
        if(op_size == 1) {
            *(volatile uint8_t *)(SCRATCHPAD_BASE + base) =
                *(volatile uint8_t *)(buf + base);
        } else if(op_size == 2) {
            *(volatile uint16_t *)(SCRATCHPAD_BASE + base) =
                *(volatile uint16_t *)(buf + base);
        } else { // op_size == 4
            *(volatile uint32_t *)(SCRATCHPAD_BASE + base) =
                *(volatile uint32_t *)(buf + base);
        }   
    }   
}

int main()
{
    printf("Test starts\n");

    total_send_count = 6;
    for(int i = 0;i < PACKET_SIZE;i++)
        buf[i] = i % 64; 
    // Send packets 
    for(int i = 0;i < total_send_count;i++) {
        buf[0] = i; // We use first byte as the packet ID
        printf("Writing #%d packet with word granularity\n", i + 1); 
        write_packet(buf, PACKET_SIZE, 4); 
    }   
    printf("Test ends. Ctrl-C to stop\n");
    while(done == 0); 

    bp_finish(0);
}

The expected behavior is to reach "Test ends.". If we disable slow PS simulation it will run properly. However if we have the following setting in Makefile in #60 :

-DSIM_BACKPRESSURE_ENABLE -DSIM_BACKPRESSURE_SEED=1234 -DSIM_BACKPRESSURE_CHANCE=100 -DSIM_BACKPRESSURE_LENGTH=128

The writing of the first packet will never end. If we open the waveform and check the m01 AXI bus in top.v, one will see BP kind of keeps repeating itself.

Yuan-Mao commented 1 year ago

(This might also be related: https://github.com/black-parrot-hdk/zynq-parrot/pull/60#issuecomment-1440788088)

dpetrisko commented 1 year ago

Great thanks I'll look into it

Yuan-Mao commented 1 year ago

Dan and I have discussed offline. Initial investigation shows that bp_me_axil_master could be the source of the bug. Changing the parameter "num_outstanding_p" to 1 can workaround the issue.