-
Notifications
You must be signed in to change notification settings - Fork 194
libmetal: lib: linux: improve Linux UIO-backed device open and test coverage #365
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
Open
bentheredonethat
wants to merge
6
commits into
OpenAMP:main
Choose a base branch
from
bentheredonethat:uio-update
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d367c5c
lib: linux: preserve device-open errors
bentheredonethat e4b5c2b
lib: linux: fix UIO mmap offset handling
bentheredonethat 0679571
lib: linux: clear UIO IRQ bookkeeping on close
bentheredonethat 4015471
lib: linux: factor common UIO populate path
bentheredonethat e77402d
lib: linux: add UIO class-name lookup
bentheredonethat 6e9937c
lib: linux: register synthetic UIO bus
bentheredonethat File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,8 @@ | |
| #include <metal/utilities.h> | ||
| #include <metal/irq.h> | ||
|
|
||
| #include <stdint.h> | ||
|
|
||
| #include "irq.h" | ||
|
|
||
| #define MAX_DRIVERS 64 | ||
|
|
@@ -59,12 +61,34 @@ struct linux_device { | |
| char dev_path[PATH_MAX]; | ||
| char cls_path[PATH_MAX]; | ||
| metal_phys_addr_t region_phys[METAL_MAX_DEVICE_REGIONS]; | ||
| void *region_map_raw[METAL_MAX_DEVICE_REGIONS]; | ||
| size_t region_map_len[METAL_MAX_DEVICE_REGIONS]; | ||
| struct linux_driver *ldrv; | ||
| struct sysfs_device *sdev; | ||
| struct sysfs_attribute *override; | ||
| int fd; | ||
| }; | ||
|
|
||
| /** | ||
| * @internal | ||
| * | ||
| * @brief UIO map attributes and derived libmetal region information. | ||
| * | ||
| * UIO sysfs reports a full mmap() extent plus a separate offset to the | ||
| * usable resource. This structure keeps those inputs together while converting | ||
| * them into the libmetal physical address, mmap length, and exported region | ||
| * size. | ||
| */ | ||
| struct metal_uio_map_info { | ||
| const char *dev_name; | ||
| metal_phys_addr_t map_addr; | ||
| unsigned long map_size; | ||
| unsigned long offset; | ||
| metal_phys_addr_t *phys; | ||
| size_t *map_len; | ||
| size_t *region_size; | ||
| }; | ||
|
|
||
| static struct linux_bus *to_linux_bus(struct metal_bus *bus) | ||
| { | ||
| return metal_container_of(bus, struct linux_bus, bus); | ||
|
|
@@ -100,6 +124,94 @@ static int metal_uio_read_map_attr(struct linux_device *ldev, | |
| return 0; | ||
| } | ||
|
|
||
| /** | ||
| * @internal | ||
| * | ||
| * @brief Validate the sysfs map offset before applying it to the mmap() base. | ||
| * | ||
| * The Linux UIO ABI exposes one mmap slot per page-sized index, so the | ||
| * per-map offset must remain within a single host page. | ||
| * | ||
| * The offset is applied inside one page returned by mmap(). Larger offsets | ||
| * cannot be represented by adjusting the returned mapping. | ||
| * | ||
| * @param[in] dev_name Device name used for error reporting; may be NULL. | ||
| * @param[in] offset Offset to validate, in bytes. | ||
| * @return 0 on success, or -EINVAL if the offset exceeds the host page size. | ||
| */ | ||
| static int metal_linux_uio_validate_offset(const char *dev_name, | ||
| unsigned long offset) | ||
| { | ||
| const unsigned long page_size = (unsigned long)getpagesize(); | ||
|
|
||
| if (offset >= page_size) { | ||
| metal_log(METAL_LOG_ERROR, | ||
| "device %s has invalid UIO offset 0x%lx (page size 0x%lx)\n", | ||
| dev_name ? dev_name : "<unknown>", offset, page_size); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| /** | ||
| * @internal | ||
| * | ||
| * @brief Translate UIO sysfs map attributes into libmetal map information. | ||
| * | ||
| * This fills in the values required by libmetal: the mmap() length used for | ||
| * cleanup, the usable physical start address, and the usable I/O region size | ||
| * after skipping the map offset. | ||
| * | ||
| * @param[in,out] info Pointer to the map information structure to populate. | ||
| * @return 0 on success, or a negative error code on failure. | ||
| */ | ||
| static int metal_linux_uio_map_info(struct metal_uio_map_info *info) | ||
| { | ||
| int result; | ||
|
|
||
| if (!info || !info->phys || !info->map_len || !info->region_size) | ||
| return -EINVAL; | ||
|
|
||
| result = metal_linux_uio_validate_offset(info->dev_name, info->offset); | ||
| if (result) | ||
| return result; | ||
|
|
||
| if (info->offset >= info->map_size) { | ||
| metal_log(METAL_LOG_ERROR, | ||
| "device %s has invalid UIO size 0x%lx for offset 0x%lx\n", | ||
| info->dev_name ? info->dev_name : "<unknown>", | ||
| info->map_size, info->offset); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| if (info->map_size > SIZE_MAX) { | ||
| metal_log(METAL_LOG_ERROR, | ||
| "device %s UIO size 0x%lx overflows size_t\n", | ||
| info->dev_name ? info->dev_name : "<unknown>", | ||
| info->map_size); | ||
| return -EOVERFLOW; | ||
| } | ||
|
|
||
| if (info->map_addr + info->offset < info->map_addr) { | ||
| metal_log(METAL_LOG_ERROR, | ||
| "device %s UIO physical address overflow (addr=0x%lx offset=0x%lx)\n", | ||
| info->dev_name ? info->dev_name : "<unknown>", | ||
| (unsigned long)info->map_addr, info->offset); | ||
| return -EOVERFLOW; | ||
| } | ||
|
|
||
| /* | ||
| * mmap() uses the full page-aligned map. libmetal clients see only the | ||
| * usable resource that starts at offset bytes into that mapping. | ||
| */ | ||
| *info->phys = info->map_addr + info->offset; | ||
| *info->map_len = (size_t)info->map_size; | ||
| *info->region_size = (size_t)(info->map_size - info->offset); | ||
|
|
||
| return 0; | ||
| } | ||
|
|
||
| static int metal_uio_dev_bind(struct linux_device *ldev, | ||
| struct linux_driver *ldrv) | ||
| { | ||
|
|
@@ -155,11 +267,15 @@ static int metal_uio_dev_open(struct linux_bus *lbus, struct linux_device *ldev) | |
| { | ||
| char *instance, path[SYSFS_PATH_MAX]; | ||
| struct linux_driver *ldrv = ldev->ldrv; | ||
| unsigned long *phys, offset = 0, size = 0; | ||
| unsigned long offset = 0, size = 0; | ||
| metal_phys_addr_t addr = 0, *phys; | ||
| struct metal_io_region *io; | ||
| struct metal_uio_map_info map_info; | ||
| size_t map_len, region_size; | ||
| struct dlist *dlist; | ||
| int result, i; | ||
| void *virt; | ||
| unsigned int j; | ||
| void *raw, *virt; | ||
| int irq_info; | ||
|
|
||
|
|
||
|
|
@@ -177,35 +293,48 @@ static int metal_uio_dev_open(struct linux_bus *lbus, struct linux_device *ldev) | |
|
|
||
| result = metal_uio_dev_bind(ldev, ldrv); | ||
| if (result) | ||
| return result; | ||
| goto fail; | ||
|
|
||
| result = snprintf(path, sizeof(path), "%s/uio", ldev->sdev->path); | ||
| if (result >= (int)sizeof(path)) | ||
| return -EOVERFLOW; | ||
| if (result < 0 || result >= (int)sizeof(path)) { | ||
| result = -EOVERFLOW; | ||
| goto fail; | ||
| } | ||
| dlist = sysfs_open_directory_list(path); | ||
| if (!dlist) { | ||
| metal_log(METAL_LOG_ERROR, "failed to scan class path %s\n", | ||
| path); | ||
| return -errno; | ||
| result = -errno; | ||
| goto fail; | ||
| } | ||
|
|
||
| dlist_for_each_data(dlist, instance, char) { | ||
| result = snprintf(ldev->cls_path, sizeof(ldev->cls_path), | ||
| "%s/%s", path, instance); | ||
| if (result >= (int)sizeof(ldev->cls_path)) | ||
| return -EOVERFLOW; | ||
| if (result < 0 || result >= (int)sizeof(ldev->cls_path)) { | ||
| result = -EOVERFLOW; | ||
| goto close_list; | ||
| } | ||
| result = snprintf(ldev->dev_path, sizeof(ldev->dev_path), | ||
| "/dev/%s", instance); | ||
| if (result >= (int)sizeof(ldev->dev_path)) | ||
| return -EOVERFLOW; | ||
| if (result < 0 || result >= (int)sizeof(ldev->dev_path)) { | ||
| result = -EOVERFLOW; | ||
| goto close_list; | ||
| } | ||
| break; | ||
| } | ||
| result = 0; | ||
|
|
||
| close_list: | ||
| sysfs_close_list(dlist); | ||
| if (result) | ||
| goto fail; | ||
|
|
||
| if (sysfs_path_is_dir(ldev->cls_path) != 0) { | ||
| metal_log(METAL_LOG_ERROR, "invalid device class path %s\n", | ||
| ldev->cls_path); | ||
| return -ENODEV; | ||
| result = -ENODEV; | ||
| goto fail; | ||
| } | ||
|
|
||
| i = 0; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be initialized when declared
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. will fix |
||
|
|
@@ -218,34 +347,72 @@ static int metal_uio_dev_open(struct linux_bus *lbus, struct linux_device *ldev) | |
| if (i >= 1000) { | ||
| metal_log(METAL_LOG_ERROR, "failed to open file %s, timeout.\n", | ||
| ldev->dev_path); | ||
| return -ENODEV; | ||
| result = -ENODEV; | ||
| goto fail; | ||
| } | ||
| result = metal_open(ldev->dev_path, 0); | ||
| if (result < 0) { | ||
| metal_log(METAL_LOG_ERROR, "failed to open device %s\n", | ||
|
arnopo marked this conversation as resolved.
Outdated
|
||
| ldev->dev_path, strerror(-result)); | ||
| return result; | ||
| goto fail; | ||
| } | ||
| ldev->fd = result; | ||
|
|
||
| metal_log(METAL_LOG_DEBUG, "opened %s:%s as %s\n", | ||
| lbus->bus_name, ldev->dev_name, ldev->dev_path); | ||
|
|
||
| for (i = 0, result = 0; !result && i < METAL_MAX_DEVICE_REGIONS; i++) { | ||
| for (i = 0; i < METAL_MAX_DEVICE_REGIONS; i++) { | ||
|
tnmysh marked this conversation as resolved.
|
||
| phys = &ldev->region_phys[ldev->device.num_regions]; | ||
| result = metal_uio_read_map_attr(ldev, i, "offset", &offset); | ||
| /* | ||
| * A missing offset for the next map marks the end of the UIO | ||
| * map list. Other read errors are real open failures. | ||
| */ | ||
| if (result == -ENOENT) | ||
| break; | ||
|
arnopo marked this conversation as resolved.
arnopo marked this conversation as resolved.
arnopo marked this conversation as resolved.
|
||
| if (result) | ||
| goto fail; | ||
| result = (result ? result : | ||
| metal_uio_read_map_attr(ldev, i, "offset", &offset)); | ||
| result = (result ? result : | ||
| metal_uio_read_map_attr(ldev, i, "addr", phys)); | ||
| metal_uio_read_map_attr(ldev, i, "addr", &addr)); | ||
| result = (result ? result : | ||
| metal_uio_read_map_attr(ldev, i, "size", &size)); | ||
| result = (result ? result : | ||
| metal_map(ldev->fd, i * getpagesize(), size, 0, 0, &virt)); | ||
| if (!result) { | ||
| io = &ldev->device.regions[ldev->device.num_regions]; | ||
| metal_io_init(io, virt, phys, size, -1, 0, NULL); | ||
| ldev->device.num_regions++; | ||
| if (result) | ||
| goto fail; | ||
| /* | ||
| * UIO sysfs reports addr/size/offset separately. Convert them | ||
| * before mmap() so the raw mapping and exposed region stay in | ||
| * sync for both normal access and close-time unmap. | ||
| */ | ||
| map_info.dev_name = ldev->dev_name; | ||
| map_info.map_addr = addr; | ||
| map_info.map_size = size; | ||
| map_info.offset = offset; | ||
| map_info.phys = phys; | ||
| map_info.map_len = &map_len; | ||
| map_info.region_size = ®ion_size; | ||
| result = metal_linux_uio_map_info(&map_info); | ||
| if (result) | ||
| goto fail; | ||
| result = metal_map(ldev->fd, i * getpagesize(), map_len, 0, 0, | ||
| &raw); | ||
| if (result) { | ||
| metal_log(METAL_LOG_ERROR, | ||
| "failed to mmap device %s map%u (len=0x%zx offset=0x%lx): %s\n", | ||
| ldev->dev_name, i, map_len, | ||
| (unsigned long)i * (unsigned long)getpagesize(), | ||
| strerror(-result)); | ||
| goto fail; | ||
| } | ||
| virt = (void *)((char *)raw + offset); | ||
| /* | ||
| * Keep the raw mapping for munmap(); expose the adjusted | ||
| * address as the usable libmetal I/O region. | ||
| */ | ||
| io = &ldev->device.regions[ldev->device.num_regions]; | ||
| metal_io_init(io, virt, phys, region_size, -1, 0, NULL); | ||
| ldev->region_map_raw[ldev->device.num_regions] = raw; | ||
| ldev->region_map_len[ldev->device.num_regions] = map_len; | ||
| ldev->device.num_regions++; | ||
| } | ||
|
|
||
| irq_info = 1; | ||
|
|
@@ -262,6 +429,31 @@ static int metal_uio_dev_open(struct linux_bus *lbus, struct linux_device *ldev) | |
| } | ||
|
|
||
| return 0; | ||
|
|
||
| fail: | ||
| for (j = 0; j < ldev->device.num_regions; j++) { | ||
| metal_unmap(ldev->region_map_raw[j], | ||
| ldev->region_map_len[j]); | ||
| ldev->region_map_raw[j] = NULL; | ||
| ldev->region_map_len[j] = 0; | ||
| } | ||
| ldev->device.num_regions = 0; | ||
| ldev->device.irq_num = 0; | ||
| ldev->device.irq_info = (void *)-1; | ||
| if (ldev->override) { | ||
| sysfs_write_attribute(ldev->override, "", 1); | ||
| ldev->override = NULL; | ||
| } | ||
| if (ldev->sdev) { | ||
| sysfs_close_device(ldev->sdev); | ||
| ldev->sdev = NULL; | ||
| } | ||
| if (ldev->fd >= 0) { | ||
| close(ldev->fd); | ||
| ldev->fd = -1; | ||
| } | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| static void metal_uio_dev_close(struct linux_bus *lbus, | ||
|
|
@@ -271,8 +463,10 @@ static void metal_uio_dev_close(struct linux_bus *lbus, | |
| unsigned int i; | ||
|
|
||
| for (i = 0; i < ldev->device.num_regions; i++) { | ||
| metal_unmap(ldev->device.regions[i].virt, | ||
| ldev->device.regions[i].size); | ||
| metal_unmap(ldev->region_map_raw[i], | ||
| ldev->region_map_len[i]); | ||
| ldev->region_map_raw[i] = NULL; | ||
| ldev->region_map_len[i] = 0; | ||
| } | ||
| if (ldev->override) { | ||
| sysfs_write_attribute(ldev->override, "", 1); | ||
|
|
@@ -428,7 +622,8 @@ static int metal_linux_dev_open(struct metal_bus *bus, | |
| struct linux_bus *lbus = to_linux_bus(bus); | ||
| struct linux_device *ldev = NULL; | ||
| struct linux_driver *ldrv; | ||
| int error; | ||
| int error = -ENODEV; | ||
| int ret; | ||
|
|
||
| ldev = malloc(sizeof(*ldev)); | ||
| if (!ldev) | ||
|
|
@@ -448,9 +643,18 @@ static int metal_linux_dev_open(struct metal_bus *bus, | |
| ldev->device.bus = bus; | ||
|
|
||
| /* Try and open the device. */ | ||
| error = ldrv->dev_open(lbus, ldev); | ||
| if (error) { | ||
| ldrv->dev_close(lbus, ldev); | ||
| ret = ldrv->dev_open(lbus, ldev); | ||
| if (ret) { | ||
| /* | ||
| * Preserve the first useful errno while still allowing | ||
| * clean backend misses to try the same device. | ||
| */ | ||
| if (ldrv->dev_close) | ||
| ldrv->dev_close(lbus, ldev); | ||
| if (error == -ENODEV) | ||
| error = ret; | ||
| if (ret != -ENODEV) | ||
| goto out; | ||
| continue; | ||
| } | ||
|
|
||
|
|
@@ -461,9 +665,10 @@ static int metal_linux_dev_open(struct metal_bus *bus, | |
| return 0; | ||
| } | ||
|
|
||
| out: | ||
| free(ldev); | ||
|
|
||
| return -ENODEV; | ||
| return error; | ||
| } | ||
|
|
||
| static void metal_linux_dev_close(struct metal_bus *bus, | ||
|
|
@@ -668,4 +873,3 @@ int metal_linux_get_device_property(struct metal_device *device, | |
| status = close(fd); | ||
| return status < 0 ? -errno : 0; | ||
| } | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.