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. >
