Re: PCI arbiter memory mapping

[Samuel Thibault]

Joan Lledó, le dim. 20 juin 2021 14:59:25 +0200, a ecrit:
> +  /* Find out whther pager is already a proxy */
> +  memory_object_proxy_valid (pager, &pager_is_proxy);
> 
> +  starts[0] = pager_is_proxy ? 0 : region->base_addr;

Is it not possible to avoid having to call memory_object_proxy_valid?
It looks odd for the caller not to know what kind of object it has in
its hands. I guess you can simply add a boolean in pci_user_data?

Or else, maybe better fix device_map into supporting non-zero offset,
that should be easy enough, by making device_pager_setup record the
offset into a new field of dev_pager_t, and just add that offset on the
fly in device_map_page.

Does pci_device_hurd_unmap_range not need to close the pager?

Also, map_dev_mem needs to release the pager when dev == NULL? otherwise
pci_device_x86_read_rom would leak the pager?

I'm a bit afraid of the struct pci_user_data passing between
libpciaccess and pci-arbiter: if we ever want to extend the structure
the compatibility will be hard to achieve. I'd rather see a
hurd-specific libpciaccess function to get the pager.

Apart from these, the principles look good to me!


BTW, you can directly push "fix indentation" commits, so that after
rebasing your branch only contains actual code changes, for easier
review.

Samuel