Joan Lledó, le lun. 16 août 2021 10:53:47 +0200, a ecrit:
> El 5/8/21 a les 1:26, Samuel Thibault ha escrit:
>
> > Is it not possible to avoid having to call memory_object_proxy_valid?
> > maybe better fix device_map into supporting non-zero offset,
>
> I think it'd be a better solution to move the call to
> memory_object_proxy_valid() and the start value overwrite to
> memory_object_create_proxy(), that way all logic related to proxies is kept
> inside memory_object_proxy.c and other modules of the kernel don't need to
> be aware of proxies.
Ok but I meant that the device_map interface already has has an "offset"
parameter which is marked as TODO. So it's already there, it makes
sense (getting a map object from a specific offset), and documented, it
is just waiting to be implemented:
device/dev_pager.c
/* FIXME: This is not recording offset! */
kern_return_t device_pager_setup(
const mach_device_t device,
int prot,
vm_offset_t offset,
vm_size_t size,
mach_port_t *pager)
And in x86_pci.c's map_dev_mem you have
/* XXX: Mach should be fixed into supporting non-zero offset */
err = device_map (devmem, prot, 0x0, mem_offset + mem_size, &pager, 0);
That looks like an exact match to get things fixed and along the way
kill an old FIXME.
> > I'd rather see a
> > hurd-specific libpciaccess function to get the pager.
>
> That's better, but we'd have to add that function in both hurd_pci.c and
> x86_pci.c. I don't like the idea of adding Hurd specific code to x86_module
> but there's already a lot of it and we could avoid the existence of struct
> pci_user_data, which I like even less.
Actually I'm thinking that this is just another case of mremap(). The
underlying question is getting the memory object of a given memory
range, like vm_region does. We need to be careful since we don't want
any process to be able to get the memory object of any other process,
but it does make sense, and would be useful for mremap.
> Apart from that, I also found a bug in hurd_pci.c:196 [1]. When the user
> asks for read/write permissions but only read is allowed, we should either:
>
> - Deallocate robj and return EPERM
We should do that, yes.
> I think pci_device_hurd_map_range() should behave the same way
> pci_device_x86_map_range() does in the x86 module. But the implementation of
> map_dev_mem() is not clear about what happens in that case, I guess vm_map()
> handles that.
Not yet, because glibc was not careful about it, but we should aim for
that, yes.
Samuel
