Re: [PATCH] rumpkernel: pci-userspace: Ask for permission to use PCI CONF2 registers

[Damien Zammit]

Samuel,

I found out that PCI_CONF2 access method was deprecated in PCI v2.0,
meaning we only care about reserving access to these ioports (c000-cfff)
on hardware that was created circa 1992-1993, eg 80486 and first pentiums. [1]
Also, when PCI_CONF2 range is accessed for pci configuration,
you must use PCI_CONF1 registers first to activate the second I/O space,
therefore pci-arbiter already holds the key to this.

We should remove PCI_CFG2_* from gnumach as well, or if you really care about 
running
Hurd on 80486/Pentium1, please suggest an alternative way to not block
modern hw from allocating and using PCI_CONF2 I/O port range as regular I/O.
(Maybe it would require a second lock that takes effect when PCI_CONF1 is used?)

[1] https://wiki.osdev.org/Pci#Configuration_Space_Access_Mechanism_.232

Damien

On 21/8/22 7:54 pm, Samuel Thibault wrote:
> Damien Zammit, le dim. 21 août 2022 07:00:49 +0000, a ecrit:
>> Rationale: QEMU allocates actual io ports in the 0xc000 - 0xcfff
>> range, therefore we still need to allow rump to access these ports,
>> in case they are allocated to real pci hardware.
>
> I'm a bit worried to let rump access cfg2 registers. Don't we know more
> precisely where rump might want to access acpi?
>
> Also, gnumach would refuse rump and pci-access both take access to them:
>
> i386/i386/io_perm.c:
> #define CONTAINS_PCI_CFG(from, to) \
>   ( ( ( from <= PCI_CFG1_END ) && ( to >= PCI_CFG1_START ) ) || \
>     ( ( from <= PCI_CFG2_END ) && ( to >= PCI_CFG2_START ) ) )
>
> It seems that libpciaccess is not using cfg2, though, so possibly we can
> make gnumach split its taken_pci_cfg into taken_pci_cfg1 and
> taken_pci_cfg2.
>
>> ---
>>  debian/patches/machirqdev.diff | 21 ++++++++++++++++++++-
>>  1 file changed, 20 insertions(+), 1 deletion(-)
>>
>> diff --git a/debian/patches/machirqdev.diff b/debian/patches/machirqdev.diff
>> index 2f74ca619..41894d288 100644
>> --- a/debian/patches/machirqdev.diff
>> +++ b/debian/patches/machirqdev.diff
>> @@ -40,7 +40,7 @@
>>   #include "mach_debug_U.h"
>>   #include <mach/vm_param.h>
>>   #include <mach.h>
>> -@@ -89,6 +89,7 @@
>> +@@ -89,11 +89,10 @@ static int numdevs = -1;
>>
>>   static mach_port_t master_host;
>>   static mach_port_t master_device;
>> @@ -48,6 +48,25 @@
>>
>>   #define PCI_CFG1_START 0xcf8
>>   #define PCI_CFG1_END   0xcff
>> +-#define PCI_CFG2_START 0xc000
>> +-#define PCI_CFG2_END   0xcfff
>> +
>> + int
>> + rumpcomp_pci_iospace_init(void)
>> +@@ -105,11 +104,8 @@ rumpcomp_pci_iospace_init(void)
>> +    /* 0-0xcf7 */
>> +    if (ioperm(0, PCI_CFG1_START, 1))
>> +            return rumpuser_component_errtrans(errno);
>> +-   /* 0xd00-0xbfff */
>> +-   if (ioperm(PCI_CFG1_END+1, PCI_CFG2_START - (PCI_CFG1_END+1), 1))
>> +-           return rumpuser_component_errtrans(errno);
>> +-   /* 0xd000-0xffff */
>> +-   if (ioperm(PCI_CFG2_END+1, 0x10000 - (PCI_CFG2_END+1), 1))
>> ++   /* 0xd00-0xffff */
>> ++   if (ioperm(PCI_CFG1_END+1, 0x10000 - (PCI_CFG1_END+1), 1))
>> +            return rumpuser_component_errtrans(errno);
>> +
>> +    return 0;
>>  @@ -131,6 +132,9 @@
>>      if (get_privileged_ports (&master_host, &master_device))
>>              err(1, "get_privileged_ports");
>> --
>> 2.34.1
>>
>>
>>
>
> --
> Samuel
> ---
> Pour une évaluation indépendante, transparente et rigoureuse !
> Je soutiens la Commission d'Évaluation de l'Inria.
>