Re: [PATCH] Hurd: avoid using the deprecated RPC pci_get_ndevs()

[Joan Lledó]

Hi,

El 3/11/19 a les 17:10, Samuel Thibault ha escrit:
>> +            if (!devices)
>> +                return ENOMEM;
> 
> That makes me realize: we need to closedir(dir).

What do you mean?

>>      if (err) {
>> -        mach_port_deallocate (mach_task_self (), pci_server_port);
>> -        /* Fall back to x86 access method */
>> -        return pci_system_x86_create();
>> +      /* There was an error, but we don't know which devices have been
>> +       * initialized correctly, so call cleanup to free whatever is 
>> allocated */
>> +      pci_system_cleanup();
>> +      return err;
> 
> We also need to call x86_disable_io();
> 
> Also, don't wee need to keep a way to fallback to the x86 access method?
> Otherwise how does the pciarbiter manage to trigger calling the x86
> access method?

I removed that code b/c I thought it was always failing, maybe I'm
missing something, let me explain. Take a look at hurd_pci.c:452 [1]

    /* If we can open pci cfg io ports on hurd,
     * we are the arbiter, therefore try x86 method first */
    err = pci_system_x86_create();
    if (!err)
        return 0;

So, if !err, we are the arbiter and the function returns here, else, we
are a function trying to connect to the arbiter, no failback is
possible, since calling pci_system_x86_create() again will fail again.
On the other hand, if there's not arbiter at all and we want all
requests to fail back to the x86 method, then the call at line 452 is
enough, no need to run the rest of the function which tries to reach the
arbiter to end up calling pci_system_x86_create() again and failing again.

> We also need to call x86_disable_io();

I don't think so, b/c I think if the execution reached this point is
because we are not the arbiter, and GNU Mach don't allow us to access
the ports.

---
[1]
https://gitlab.freedesktop.org/xorg/lib/libpciaccess/blob/master/src/hurd_pci.c#L452