Hello,
Damien Zammit, le sam. 06 mars 2021 15:00:00 +1100, a ecrit:
> +static int
> +sort_devices(void)
> +{
> + int bus, dev, func;
> + struct pci_device_private *sorted;
> + struct pci_device_private *device;
> + struct pci_device_private *tmpdev;
> +
> + sorted = calloc(pci_sys->num_devices, sizeof(struct pci_device_private));
> + if (!sorted) {
> + return ENOMEM;
> + }
> +
> + tmpdev = sorted;
> +
> + for (bus = 0; bus < 256; bus++) {
> + for (dev = 0; dev < 32; dev++) {
> + for (func = 0; func < 8; func++) {
> + device = (struct pci_device_private *)
> + pci_device_find_by_slot(0, bus, dev, func);
> + if (device) {
> + *tmpdev = *device;
> + tmpdev++;
> + }
> + }
> + }
> + }
> + if (pci_sys->devices) {
> + free(pci_sys->devices);
> + pci_sys->devices = sorted;
> + }
> + return 0;
> +}
That probably needs to be discussed with upgrade?
> +simple_readdir(mach_port_t port, uint32_t *first_entry)
> +{
> + struct dirent *e;
> + char *data;
> + int nentries = 0;
> + vm_size_t size;
> +
> + dir_readdir (port, &data, &size, *first_entry, 1, 0, &nentries);
> +
> + if (nentries == 0) {
> + return NULL;
> + }
> +
> + *first_entry = *first_entry + 1;
> + e = (struct dirent *)(data+4);
> + return e;
> +}
What is this +4 for?
Is it compiled with -D_FILE_OFFSET_BITS=64? (the __dir_readdir RPC
returns a struct dirent64, not struct dirent). You can build it with
-D_LARGEFILE64_SOURCE to get access to struct dirent64 without changing
all the ABI of off_t, ino_t, etc.
> + if (pci_port == MACH_PORT_NULL) {
> + return EGRATUITOUS;
> + } else {
You don't need an "else" here, the pci_port == MACH_PORT_NULL can be
kept as a two-line quick-exit, and let the rest of the code not inside a
block.
> @@ -370,34 +394,27 @@ enum_devices(const char *parent, int domain,
> - device_port = file_name_lookup(server, 0, 0);
> + device_port = file_name_lookup_under(pci_port, server, O_RDWR,
> 0);
> if (device_port == MACH_PORT_NULL) {
> - closedir(dir);
> - return errno;
> + return 0;
> }
>
> toread = sizeof(reg);
> err = pciclient_cfg_read(device_port, PCI_VENDOR_ID, (char*)®,
> &toread);
> if (err) {
> - if (closedir(dir) < 0)
> - return errno;
> return err;
> }
Realizing here: don't we need to deallocate the device_port port?
> @@ -513,12 +521,24 @@ pci_system_hurd_create(void)
> pci_sys->methods = &hurd_pci_methods;
>
> pci_sys->num_devices = 0;
> - err = enum_devices(_SERVERS_BUS_PCI, -1, -1, -1, -1, LEVEL_DOMAIN);
> +
> + if (! (err = get_privileged_ports (NULL, &device_master)) &&
> (device_master != MACH_PORT_NULL)) {
> + err = device_open (device_master, D_READ|D_WRITE, "pci",
> &pci_server_port);
> + if (err) {
> + pci_system_cleanup();
> + return err;
> + }
Please also add a failback path that calls
file_name_lookup("/servers/bus/pci"), so that the user can interpose
the PCI world in the FS without having to interposing the device master
port.
> + root = file_name_lookup_under (pci_server_port, ".", O_DIRECTORY |
> O_RDWR | O_EXEC, 0);
> + if (!root) {
> + pci_system_cleanup();
> + return errno;
> + }
> + err = enum_devices(pci_server_port, root, ".", -1, -1, -1, -1,
> LEVEL_DOMAIN);
This part also doesn't need to be kept in an "else" part, it is the
normal codepath.
Samuel
