Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

host_exerciser:support no FPGA mgmt PF instances #3108

Merged
merged 3 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 41 additions & 32 deletions libraries/afu-test/afu_test.h
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ class afu {
, shared_(false)
, timeout_msec_(60000)
, handle_(nullptr)
, handle_device_(nullptr)
, current_command_(nullptr)
{
if (!afu_id_.empty())
Expand All @@ -196,55 +197,66 @@ class afu {
return fpga::properties::get(handle_);
}

int open_handle(const char *afu_id) {
bool enum_fpga_device()
{
auto filter = fpga::properties::get(); // Get an empty properties object

// The following code attempts to get a token+handle for the DEVICE.
// The following code attempts to get a token+handle for the DEVICE.
// This is to allow access to OPAE-API functions that are only supported
// through the xfpga plugin (i.e accessing sysfs entries)
// In contrast, the ACCELERATOR token may be underlied by the vfio plugin.
// Set PCIe segment, bus, and device properties to enumerate FPGA DEVICE (FME)
if (!pci_addr_.empty()) {
auto p = pcie_address::parse(pci_addr_.c_str());
filter->segment = p.fields.domain;
filter->bus = p.fields.bus;
filter->device = p.fields.device;
auto p = pcie_address::parse(pci_addr_.c_str());
filter->segment = p.fields.domain;
filter->bus = p.fields.bus;
filter->device = p.fields.device;
}

filter->type = FPGA_DEVICE;
auto tokens = fpga::token::enumerate({filter});
auto tokens = fpga::token::enumerate({ filter });

// Error out if the # of tokens != 1
if (tokens.size() < 1) {
if (pci_addr_.empty()) {
logger_->error("no DEVICE found");
} else {
logger_->error("no accelerator found at PCIe address {1}",
pci_addr_);
}
return exit_codes::not_found;
}
logger_->info("Not found FPGA DEVICE");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
logger_->info("Not found FPGA DEVICE");
logger_->info("no FPGA DEVICE found");

to be consistent with the message below.

return false;
}

if (tokens.size() > 1) {
std::cerr << "more than one DEVICE found matching filter\n";
std::cout << "more than one FPGA DEVICE found \n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this not use logger_?

return false;
}

int flags = shared_ ? FPGA_OPEN_SHARED : 0;

// Open a handle to the resource
try {
handle_device_ = fpga::handle::open(tokens[0], flags);
} catch (fpga::no_access &err) {
std::cerr << err.what() << "\n";
return exit_codes::no_access;
handle_device_ = fpga::handle::open(tokens[0], flags);
}
catch (fpga::no_access& err) {
std::cerr << err.what() << "\n";
return false;
}
return true;
}

// The following code attempts to get a token + handle for the AFU
// (ACCELERATOR device) matching the given command's afu_id.
// Set PCIe segment, bus, device, and functionproperties to enumerate FPGA ACCELERATOR
int open_handle(const char *afu_id) {

if (!enum_fpga_device()) {
std::cout << "Not Found FME device\n";
Copy link
Contributor

@pcolberg pcolberg Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will output another error message, which enum_fpga_device() already does. Why is that needed?

}
auto filter = fpga::properties::get(); // Get an empty properties object

// The following code attempts to get a token+handle for the DEVICE.
// This is to allow access to OPAE-API functions that are only supported
// through the xfpga plugin (i.e accessing sysfs entries)
// In contrast, the ACCELERATOR token may be underlied by the vfio plugin.
// Set PCIe segment, bus, and device properties to enumerate FPGA DEVICE (FME)
if (!pci_addr_.empty()) {
auto p = pcie_address::parse(pci_addr_.c_str());
filter->function = p.fields.function;
auto p = pcie_address::parse(pci_addr_.c_str());
filter->segment = p.fields.domain;
filter->bus = p.fields.bus;
filter->device = p.fields.device;
filter->function = p.fields.function;
}

auto app_afu_id = afu_id ? afu_id : afu_id_.c_str();
Expand All @@ -255,7 +267,7 @@ class afu {
return error;
}

tokens = fpga::token::enumerate({filter});
auto tokens = fpga::token::enumerate({filter});
if (tokens.size() < 1) {
if (pci_addr_.empty()) {
logger_->error("no accelerator found with id: {0}", app_afu_id);
Expand All @@ -266,10 +278,7 @@ class afu {
return exit_codes::not_found;
}

if (tokens.size() > 1) {
std::cerr << "more than one accelerator found matching filter\n";
}

int flags = shared_ ? FPGA_OPEN_SHARED : 0;
try {
handle_ = fpga::handle::open(tokens[0], flags);
} catch (fpga::no_access &err) {
Expand Down
4 changes: 3 additions & 1 deletion samples/host_exerciser/host_exerciser.h
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,9 @@ class host_exerciser : public test_afu {

token::ptr_t get_token_device()
{
return handle_device_->get_token();
if (handle_device_)
return handle_device_->get_token();
return nullptr;
}

bool option_passed(std::string option_str)
Expand Down
86 changes: 47 additions & 39 deletions samples/host_exerciser/host_exerciser_cmd.h
Original file line number Diff line number Diff line change
Expand Up @@ -737,46 +737,8 @@ class host_exerciser_cmd : public test_command
host_exe_ = dynamic_cast<host_exerciser*>(afu);

token_ = d_afu->get_token();
token_device_ = d_afu->get_token_device();

// Check if memory calibration has failed and error out before proceeding
// with the test. The dfl-emif driver creates sysfs entries to report the
// calibration status for each memory channel. sysobjects are the OPAE-API's
// abstraction for sysfs entries. However, at this time, these are only
// accessible through tokens that use the xfpga plugin and not the vfio
// plugin. Hence our use of the DEVICE token (token_device_). One
// non-ideality of the following implementation is the use of
// MAX_NUM_MEM_CHANNELS. We are essentially doing a brute-force query of
// sysfs entries since we don't know how many mem channels exist on the
// given platform. What about glob wildcards? Why not simply glob for
// "*dfl*/**/inf*_cal_fail" and use the OPAE-API's support for arrays of
// sysobjects? The reason is that, at the time of this writing, the
// xfpga-plugin's sysobject implementation does not support arrays
// specifically when the glob contains a recursive wildcard "/**/". It's a
// strange and perhaps unnecessary limitation. Therefore, future work is to
// fix that and clean up the code below.
for (size_t i = 0; i < MAX_NUM_MEM_CHANNELS; i++) {
std::stringstream mem_cal_glob;
// Construct the glob string to search for the cal_fail sysfs entry
// for the i'th mem channel
mem_cal_glob << "*dfl*/**/inf" << i << "_cal_fail";
// Ask for a sysobject with this glob string
fpga::sysobject::ptr_t testobj = fpga::sysobject::get(
token_device_, mem_cal_glob.str().c_str(), FPGA_OBJECT_GLOB);

// if test obj !=null, the sysfs entry was found.
// Read the calibration status from the sysfs entry.
// A non-zero value (typically '1') means
// calibration has failed --> we error out.
if (testobj && testobj->read64(0)) {
std::cout
<< "This sysfs entry reports that memory calibration has failed:"
<< mem_cal_glob.str().c_str() << std::endl;
return -1;
}
}


fpga_emif_status(afu);
// Read HW details
uint64_t he_info = host_exe_->read64(HE_INFO0);
he_lpbk_api_ver_ = (he_info >> 16);
Expand Down Expand Up @@ -880,6 +842,52 @@ class host_exerciser_cmd : public test_command

return status;
}

void fpga_emif_status(test_afu* afu)
{
auto d_afu = dynamic_cast<host_exerciser*>(afu);
token_device_ = d_afu->get_token_device();

// Check if memory calibration has failed and error out before proceeding
// with the test. The dfl-emif driver creates sysfs entries to report the
// calibration status for each memory channel. sysobjects are the OPAE-API's
// abstraction for sysfs entries. However, at this time, these are only
// accessible through tokens that use the xfpga plugin and not the vfio
// plugin. Hence our use of the DEVICE token (token_device_). One
// non-ideality of the following implementation is the use of
// MAX_NUM_MEM_CHANNELS. We are essentially doing a brute-force query of
// sysfs entries since we don't know how many mem channels exist on the
// given platform. What about glob wildcards? Why not simply glob for
// "*dfl*/**/inf*_cal_fail" and use the OPAE-API's support for arrays of
// sysobjects? The reason is that, at the time of this writing, the
// xfpga-plugin's sysobject implementation does not support arrays
// specifically when the glob contains a recursive wildcard "/**/". It's a
// strange and perhaps unnecessary limitation. Therefore, future work is to
// fix that and clean up the code below.

if (token_device_) {
Copy link
Contributor

@pcolberg pcolberg Feb 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (token_device_) {
if (!token_device_)
return;

to avoid one level of indentation below. As a rule of thumb, the happy path of a function should visually flow vertically, early exits horizontally; which makes it easier to reason what the main purpose of a function is (beyond the documentation).

for (size_t i = 0; i < MAX_NUM_MEM_CHANNELS; i++) {
std::stringstream mem_cal_glob;
// Construct the glob string to search for the cal_fail sysfs entry
// for the i'th mem channel
mem_cal_glob << "*dfl*/**/inf" << i << "_cal_fail";
// Ask for a sysobject with this glob string
fpga::sysobject::ptr_t testobj = fpga::sysobject::get(
token_device_, mem_cal_glob.str().c_str(), FPGA_OBJECT_GLOB);

// if test obj !=null, the sysfs entry was found.
// Read the calibration status from the sysfs entry.
// A non-zero value (typically '1') means
// calibration has failed --> we error out.
if (testobj && testobj->read64(0)) {
std::cout
<< "This sysfs entry reports that memory calibration has failed:"
<< mem_cal_glob.str().c_str() << std::endl;
return;
}
}
}
}

protected:
he_cfg he_lpbk_cfg_;
Expand Down
Loading