Hi Samuel,
On 29/3/20 11:19 am, Samuel Thibault wrote:
> --- a/src/common_interface.c
> +++ b/src/common_interface.c
> @@ -290,11 +290,13 @@ pci_device_map_range(struct pci_device *dev, pciaddr_t
> base,
>
> /* Make sure that there isn't already a mapping with the same base and
> * size.
> + * If there is, use it.
> */
> for (i = 0; i < devp->num_mappings; i++) {
> if ((devp->mappings[i].base == base)
> && (devp->mappings[i].size == size)) {
> - return EINVAL;
> + *addr = devp->mappings[i].memory;
> + return 0;
> }
> }
>
>
> I don't think we can just return it. Think of a process that uses
> libpciaccess in several components, which happen to use the same
> mapping. One will call pci_device_map_range(), the other will call
> pci_device_map_range(), at some point the first will call
> pci_device_unmap_memory_range() and then oops the second has unexpected
> lost access to the mapping.
I fixed rump instead. It now unmaps pci devices correctly and I tested
it without the above patch at all, it works.
> I don't really know why libpciaccess maintainers thought it should
> exclude mapping the same base&size. Maybe you need to confront the rump
> usage case with upstream libpciaccess, so they perhaps just drop that
> check entirely, and thus allow *several* mappings of the region?
I will close the conversation upstream, unless you think it's still worth
pursuing?
Damien
