Re: Help to fix a bug on the arbiter
Forget about this, it was working with settrans b/c this command replaces the ol d translator. I was testing the old code when using fsysopts... :( Anyway, here's the patch to fix that bug.
[PATCH] pci-arbiter: Fix bug on option parsing.
From: Joan Lledó
A misuse of strtol() caused wrong parameters to be interpreted as '0'
* pci-arbiter/options.c:
* New function parse_number() to handle wrong input
* Call parse_number() from all places where strtol was being called
---
pci-arbiter/options.c | 58 +++
1 file changed, 48 insertions(+), 10 deletions(-)
diff --git a/pci-arbiter/options.c b/pci-arbiter/options.c
index 2023fd9e..5767ad27 100644
--- a/pci-arbiter/options.c
+++ b/pci-arbiter/options.c
@@ -89,6 +89,24 @@ check_options_validity (struct parse_hook *h)
return 0;
}
+static long int
+parse_number (const char *s)
+{
+ long int val;
+ char *endptr;
+
+ errno = 0;
+ val = strtol (s, , 16);
+
+ if (*endptr != '\0' || errno)
+{
+ val = -1;
+ errno = EINVAL;
+}
+
+ return val;
+}
+
/* Option parser */
static error_t
parse_opt (int opt, char *arg, struct argp_state *state)
@@ -126,25 +144,37 @@ parse_opt (int opt, char *arg, struct argp_state *state)
switch (opt)
{
case 'C':
- h->curset->d_class = strtol (arg, 0, 16);
+ h->curset->d_class = parse_number (arg);
+ if (errno)
+ PERR (errno, "Invalid class");
break;
case 'c':
- h->curset->d_subclass = strtol (arg, 0, 16);
+ h->curset->d_subclass = parse_number (arg);
+ if (errno)
+ PERR (errno, "Invalid subclass");
break;
case 'd':
- h->curset->domain = strtol (arg, 0, 16);
+ h->curset->domain = parse_number (arg);
+ if (errno)
+ PERR (errno, "Invalid domain");
break;
case 'b':
if (h->curset->domain < 0)
h->curset->domain = 0;
- h->curset->bus = strtol (arg, 0, 16);
+ h->curset->bus = parse_number (arg);
+ if (errno)
+ PERR (errno, "Invalid bus");
break;
case 's':
- h->curset->dev = strtol (arg, 0, 16);
+ h->curset->dev = parse_number (arg);
+ if (errno)
+ PERR (errno, "Invalid slot");
break;
case 'f':
- h->curset->func = strtol (arg, 0, 16);
+ h->curset->func = parse_number (arg);
+ if (errno)
+ PERR (errno, "Invalid function");
break;
case 'D':
err =
@@ -163,25 +193,33 @@ parse_opt (int opt, char *arg, struct argp_state *state)
strncpy (regex_group_val, "", 5);
}
- h->curset->domain = strtol (regex_group_val, 0, 16);
+ h->curset->domain = parse_number (regex_group_val);
+ if (errno)
+ PERR (errno, "Invalid domain");
// Bus
strncpy (regex_group_val, arg + slot_regex_groups[3].rm_so, 2);
regex_group_val[2] = 0;
- h->curset->bus = strtol (regex_group_val, 0, 16);
+ h->curset->bus = parse_number (regex_group_val);
+ if (errno)
+ PERR (errno, "Invalid bus");
// Dev
strncpy (regex_group_val, arg + slot_regex_groups[4].rm_so, 2);
regex_group_val[2] = 0;
- h->curset->dev = strtol (regex_group_val, 0, 16);
+ h->curset->dev = parse_number (regex_group_val);
+ if (errno)
+ PERR (errno, "Invalid slot");
// Func
regex_group_val[0] = arg[slot_regex_groups[5].rm_so];
regex_group_val[1] = 0;
- h->curset->func = strtol (regex_group_val, 0, 16);
+ h->curset->func = parse_number (regex_group_val);
+ if (errno)
+ PERR (errno, "Invalid func");
}
else
{
--
2.20.1
Help to fix a bug on the arbiter
Hello,
I'm blocked trying to solve a bug I found in the option parsing of the
arbiter, at:
http://git.savannah.gnu.org/cgit/hurd/hurd.git/tree/pci-arbiter/options.c#n135
strtol() returns 0 if it wasn't able to convert the input, for instance,
-d jj will set h->curset->domain to 0, which means giving the permission
over the domain 0 and all it's devices. It's worst on system with no PCI
express, since the domain 0 is the only domain and therefore permission
over all devices on the system is granted.
I wrote a patch to solve it, something like:
static long int
parse_number (const char *s)
{
long int val;
char *endptr;
errno = 0;
val = strtol (s, , 16);
if (*endptr != '\0' || errno)
errno = EINVAL;
return val;
}
...
case 'd':
h->curset->domain = parse_number (arg);
if (errno)
PERR (errno, "Invalid domain");
break;
Testing it with settrans works great, but when trying it with fsysopts,
the behavior is the same, h->curset->domain ends up set to 0 and
permission over all devices is granted.
In the debugger, I see that errno is correctly set to EINVAL and PERR is
called, but after that the thread continues, it reaches the case
ARGP_KEY_SUCCESS and the new permissions are applied.
Any idea on why it's this patch working fine for settrans but not for
fsysopts?
[PATCH 2/2] pci-arbiter: Add --device command line option
From: Joan Lledó
Shortcut for -d, -b, -s and -f
Usage: --device [:]:.
E.G. --device 00:05.0 is shortcut for -d 0 -b 0 -s 5 -f 0
* pci-arbiter/options.c: Implement --slot option
* pci-arbiter/options.h: Add --slot to options list
---
pci-arbiter/options.c | 57 +++
pci-arbiter/options.h | 2 ++
2 files changed, 59 insertions(+)
diff --git a/pci-arbiter/options.c b/pci-arbiter/options.c
index 01686fcd..2023fd9e 100644
--- a/pci-arbiter/options.c
+++ b/pci-arbiter/options.c
@@ -27,9 +27,13 @@
#include
#include
#include
+#include
#include "pcifs.h"
+#define PCI_SLOT_REGEX
"^(([0-9a-fA-F]{4}):)?([0-9a-fA-F]{2}):([0-9a-fA-F]{2})\\.([0-7])$"
+#define PCI_SLOT_REGEX_GROUPS 6// 2: Domain, 3: Bus, 4: Dev, 5: Func
+
/* Fsysopts and command line option parsing */
/* Adds an empty interface slot to H, and sets H's current interface to it, or
@@ -91,6 +95,9 @@ parse_opt (int opt, char *arg, struct argp_state *state)
{
error_t err = 0;
struct parse_hook *h = state->hook;
+ regex_t slot_regex;
+ regmatch_t slot_regex_groups[PCI_SLOT_REGEX_GROUPS];
+ char regex_group_val[5];
/* Return _ERR from this routine */
#define RETURN(_err) \
@@ -111,6 +118,11 @@ parse_opt (int opt, char *arg, struct argp_state *state)
state->next++;
}
+ /* Compile regular expression to check --slot option */
+ err = regcomp (_regex, PCI_SLOT_REGEX, REG_EXTENDED);
+ if (err)
+FAIL (err, 1, err, "option parsing");
+
switch (opt)
{
case 'C':
@@ -134,6 +146,48 @@ parse_opt (int opt, char *arg, struct argp_state *state)
case 'f':
h->curset->func = strtol (arg, 0, 16);
break;
+case 'D':
+ err =
+ regexec (_regex, arg, PCI_SLOT_REGEX_GROUPS, slot_regex_groups,
+0);
+ if (!err)
+ {
+ // Domain, by default
+ if (slot_regex_groups[2].rm_so >= 0)
+ {
+ strncpy (regex_group_val, arg + slot_regex_groups[2].rm_so, 4);
+ regex_group_val[4] = 0;
+ }
+ else
+ {
+ strncpy (regex_group_val, "", 5);
+ }
+
+ h->curset->domain = strtol (regex_group_val, 0, 16);
+
+ // Bus
+ strncpy (regex_group_val, arg + slot_regex_groups[3].rm_so, 2);
+ regex_group_val[2] = 0;
+
+ h->curset->bus = strtol (regex_group_val, 0, 16);
+
+ // Dev
+ strncpy (regex_group_val, arg + slot_regex_groups[4].rm_so, 2);
+ regex_group_val[2] = 0;
+
+ h->curset->dev = strtol (regex_group_val, 0, 16);
+
+ // Func
+ regex_group_val[0] = arg[slot_regex_groups[5].rm_so];
+ regex_group_val[1] = 0;
+
+ h->curset->func = strtol (regex_group_val, 0, 16);
+ }
+ else
+ {
+ PERR (err, "Wrong PCI slot. Format: [:]:.");
+ }
+ break;
case 'U':
if (h->curset->uid >= 0)
parse_hook_add_set (h);
@@ -227,6 +281,9 @@ parse_opt (int opt, char *arg, struct argp_state *state)
return ARGP_ERR_UNKNOWN;
}
+ /* Free allocated regular expression for the --slot option */
+ regfree (_regex);
+
return err;
}
diff --git a/pci-arbiter/options.h b/pci-arbiter/options.h
index 9a25c603..8e5a9da4 100644
--- a/pci-arbiter/options.h
+++ b/pci-arbiter/options.h
@@ -57,6 +57,8 @@ static const struct argp_option options[] = {
{"bus", 'b', "BUS", 0, "Device bus in hexadecimal"},
{"slot", 's', "SLOT", 0, "Device slot in hexadecimal, requires -b"},
{"func", 'f', "FUNC", 0, "Device func in hexadecimal, requires -s"},
+ {"device", 'D', "DEVICE", 0,
+ "Device address in format [:]:."},
{0, 0, 0, 0, "These apply to a given permission scope:", 2},
{"uid", 'U', "UID", 0, "User ID to give permissions to"},
{"gid", 'G', "GID", 0, "Group ID to give permissions to"},
--
2.20.1
[PATCH 1/2] pci-arbiter: Rename command line options
From: Joan Lledó
Rename some CLI options in order to add the new -D --device arg.
Replace -s by -c for subclasses.
Replace -D by -d for domains.
Domains are optional from now on, default to 0.
Replace -d by -s for devices.
The formerly called "devices" are now called "slots", and
"device" will refer to a combination of
Domain + Bus + Slot + Function
Only -G and -U options create new permission scopes.
* pci-arbiter/options.c: parse_opts(): Rename options
* pci-arbiter/options.h: struct argp_option options[]: Likewise
---
pci-arbiter/options.c | 18 +++---
pci-arbiter/options.h | 18 +-
2 files changed, 16 insertions(+), 20 deletions(-)
diff --git a/pci-arbiter/options.c b/pci-arbiter/options.c
index 76ce6460..01686fcd 100644
--- a/pci-arbiter/options.c
+++ b/pci-arbiter/options.c
@@ -114,25 +114,21 @@ parse_opt (int opt, char *arg, struct argp_state *state)
switch (opt)
{
case 'C':
- /* Init a new set if the current one already has a value for this option
*/
- if (h->curset->d_class >= 0)
- parse_hook_add_set (h);
-
h->curset->d_class = strtol (arg, 0, 16);
break;
-case 's':
+case 'c':
h->curset->d_subclass = strtol (arg, 0, 16);
break;
-case 'D':
- if (h->curset->domain >= 0)
- parse_hook_add_set (h);
-
+case 'd':
h->curset->domain = strtol (arg, 0, 16);
break;
case 'b':
+ if (h->curset->domain < 0)
+ h->curset->domain = 0;
+
h->curset->bus = strtol (arg, 0, 16);
break;
-case 'd':
+case 's':
h->curset->dev = strtol (arg, 0, 16);
break;
case 'f':
@@ -261,7 +257,7 @@ netfs_append_args (char **argz, size_t * argz_len)
if (p->bus >= 0)
ADD_OPT ("--bus=0x%02x", p->bus);
if (p->dev >= 0)
- ADD_OPT ("--dev=0x%02x", p->dev);
+ ADD_OPT ("--slot=0x%02x", p->dev);
if (p->func >= 0)
ADD_OPT ("--func=%01u", p->func);
if (p->uid >= 0)
diff --git a/pci-arbiter/options.h b/pci-arbiter/options.h
index 814f81ad..9a25c603 100644
--- a/pci-arbiter/options.h
+++ b/pci-arbiter/options.h
@@ -51,12 +51,12 @@ struct parse_hook
static const struct argp_option options[] = {
{0, 0, 0, 0, "Permission scope:", 1},
{"class", 'C', "CLASS", 0, "Device class in hexadecimal"},
- {"subclass", 's', "SUBCLASS", 0,
- "Device subclass in hexadecimal, only valid with -c"},
- {"domain", 'D', "DOMAIN", 0, "Device domain in hexadecimal"},
- {"bus", 'b', "BUS", 0, "Device bus in hexadecimal, only valid with -D"},
- {"dev", 'd', "DEV", 0, "Device dev in hexadecimal, only valid with -b"},
- {"func", 'f', "FUNC", 0, "Device func in hexadecimal, only valid with -d"},
+ {"subclass", 'c', "SUBCLASS", 0,
+ "Device subclass in hexadecimal, requires -C"},
+ {"domain", 'd', "DOMAIN", 0, "Device domain in hexadecimal"},
+ {"bus", 'b', "BUS", 0, "Device bus in hexadecimal"},
+ {"slot", 's', "SLOT", 0, "Device slot in hexadecimal, requires -b"},
+ {"func", 'f', "FUNC", 0, "Device func in hexadecimal, requires -s"},
{0, 0, 0, 0, "These apply to a given permission scope:", 2},
{"uid", 'U', "UID", 0, "User ID to give permissions to"},
{"gid", 'G', "GID", 0, "Group ID to give permissions to"},
@@ -67,8 +67,8 @@ static const struct argp_option options[] = {
};
static const char doc[] = "More than one permission scope may be specified. \
-Uppercase options create a new permission scope if the current one already \
-has a value for that option. If one node is covered by more than one \
-permission scope, only the first permission is applied to that node.";
+-G and -U options create a new permission scope if the current one already \
+has a value for that option. If one device is covered by more than one \
+permission scope, only the first permission is applied.";
#endif // OPTIONS_H
--
2.20.1
Re: [PATCH 1/2] pci-arbiter: Rename command line options
Hi, El 15/2/20 a les 15:02, Samuel Thibault ha escrit: > Why a capital b? It's be more coherent to have -d -b -s -f all > small-caps. > Because of this notice in --help: "More than one permission scope may be specified. Uppercase options create a new permission scope if the current one already has a value for that option. If one node is covered by more than one permission scope, only the first permission is applied to that node." The capital options create a new permission scope, for instance: -B 0 -s 5 -f 0 -B 0 -U 1000 This will create two permission scopes: 1.- 00:05.0 belongs to nobody (Error) 2.- 00:*.* belongs to 1000 (OK) The second -B has created a new scope, which leaves the previous one incomplete. -B 0 -s 5 -f 0 -s 4 -U 1000 This creates only one permission scope: 00:04.0 belongs to 1000. The second -s overrides the first one. Until now, the capital option was -D for domain, but this commit makes the domain optional and sets it to if not given. That's for compatibility with Conventional PCI. So I moved the capital option one level down, to the bus.
[PATCH 2/2] pci-arbiter: Add --device command line option
From: Joan Lledó
Shortcut for -d, -B, -s and -f
Usage: --device [:]:.
E.G. --device 00:05.0 is shortcut for -d 0 -B 0 -s 5 -f 0
* pci-arbiter/options.c: Implement --slot option
* pci-arbiter/options.h: Add --slot to options list
---
pci-arbiter/options.c | 60 +++
pci-arbiter/options.h | 2 ++
2 files changed, 62 insertions(+)
diff --git a/pci-arbiter/options.c b/pci-arbiter/options.c
index 085a2299..88fd16cf 100644
--- a/pci-arbiter/options.c
+++ b/pci-arbiter/options.c
@@ -27,9 +27,13 @@
#include
#include
#include
+#include
#include "pcifs.h"
+#define PCI_SLOT_REGEX
"(([0-9a-fA-F]{4}):)?([0-9a-fA-F]{2}):([0-9a-fA-F]{2}).([0-7])"
+#define PCI_SLOT_REGEX_GROUPS 6// 2: Domain, 3: Bus, 4: Dev, 5: Func
+
/* Fsysopts and command line option parsing */
/* Adds an empty interface slot to H, and sets H's current interface to it, or
@@ -91,6 +95,9 @@ parse_opt (int opt, char *arg, struct argp_state *state)
{
error_t err = 0;
struct parse_hook *h = state->hook;
+ regex_t slot_regex;
+ regmatch_t slot_regex_groups[PCI_SLOT_REGEX_GROUPS];
+ char regex_group_val[5];
/* Return _ERR from this routine */
#define RETURN(_err) \
@@ -111,6 +118,11 @@ parse_opt (int opt, char *arg, struct argp_state *state)
state->next++;
}
+ /* Compile regular expression to check --slot option */
+ err = regcomp (_regex, PCI_SLOT_REGEX, REG_EXTENDED);
+ if (err)
+FAIL (err, 1, err, "option parsing");
+
switch (opt)
{
case 'C':
@@ -141,6 +153,51 @@ parse_opt (int opt, char *arg, struct argp_state *state)
case 'f':
h->curset->func = strtol (arg, 0, 16);
break;
+case 'D':
+ err =
+ regexec (_regex, arg, PCI_SLOT_REGEX_GROUPS, slot_regex_groups,
+0);
+ if (!err)
+ {
+ // Domain, by default
+ if (slot_regex_groups[2].rm_so >= 0)
+ {
+ strncpy (regex_group_val, arg + slot_regex_groups[2].rm_so, 4);
+ regex_group_val[4] = 0;
+ }
+ else
+ {
+ strncpy (regex_group_val, "", 5);
+ }
+
+ if (h->curset->domain >= 0)
+ parse_hook_add_set (h);
+
+ h->curset->domain = strtol (regex_group_val, 0, 16);
+
+ // Bus
+ strncpy (regex_group_val, arg + slot_regex_groups[3].rm_so, 2);
+ regex_group_val[2] = 0;
+
+ h->curset->bus = strtol (regex_group_val, 0, 16);
+
+ // Dev
+ strncpy (regex_group_val, arg + slot_regex_groups[4].rm_so, 2);
+ regex_group_val[2] = 0;
+
+ h->curset->dev = strtol (regex_group_val, 0, 16);
+
+ // Func
+ regex_group_val[0] = arg[slot_regex_groups[5].rm_so];
+ regex_group_val[1] = 0;
+
+ h->curset->func = strtol (regex_group_val, 0, 16);
+ }
+ else
+ {
+ PERR (err, "Wrong PCI slot. Format: [:]:.");
+ }
+ break;
case 'U':
if (h->curset->uid >= 0)
parse_hook_add_set (h);
@@ -234,6 +291,9 @@ parse_opt (int opt, char *arg, struct argp_state *state)
return ARGP_ERR_UNKNOWN;
}
+ /* Free allocated regular expression for the --slot option */
+ regfree (_regex);
+
return err;
}
diff --git a/pci-arbiter/options.h b/pci-arbiter/options.h
index bdb6be78..5ff51d08 100644
--- a/pci-arbiter/options.h
+++ b/pci-arbiter/options.h
@@ -57,6 +57,8 @@ static const struct argp_option options[] = {
{"bus", 'B', "BUS", 0, "Device bus in hexadecimal, requires -D"},
{"slot", 's', "SLOT", 0, "Device slot in hexadecimal, requires -b"},
{"func", 'f', "FUNC", 0, "Device func in hexadecimal, requires -d"},
+ {"device", 'D', "DEVICE", 0,
+ "Device address in format [:]:."},
{0, 0, 0, 0, "These apply to a given permission scope:", 2},
{"uid", 'U', "UID", 0, "User ID to give permissions to"},
{"gid", 'G', "GID", 0, "Group ID to give permissions to"},
--
2.20.1
[PATCH 1/2] pci-arbiter: Rename command line options
From: Joan Lledó
Rename some CLI options in order to add the new -d --device arg.
Replace -s by -c for subclasses.
Replace -D by -d for domains.
Domains are optional from now on, default to 0.
Replace -b by -B for buses.
Bus now creates a new permission scope if the current one
already has a value for the bus.
Replace -d by -s for devices.
The formerly called "devices" are now called "slots", and
"device" will refer to a combination of
Domain + Bus + Slot + Function
* pci-arbiter/options.c: parse_opts(): Rename options
* pci-arbiter/options.h: struct argp_option options[]: Likewise
---
pci-arbiter/options.c | 19 +++
pci-arbiter/options.h | 12 ++--
2 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/pci-arbiter/options.c b/pci-arbiter/options.c
index 76ce6460..085a2299 100644
--- a/pci-arbiter/options.c
+++ b/pci-arbiter/options.c
@@ -120,19 +120,22 @@ parse_opt (int opt, char *arg, struct argp_state *state)
h->curset->d_class = strtol (arg, 0, 16);
break;
-case 's':
+case 'c':
h->curset->d_subclass = strtol (arg, 0, 16);
break;
-case 'D':
- if (h->curset->domain >= 0)
- parse_hook_add_set (h);
-
+case 'd':
h->curset->domain = strtol (arg, 0, 16);
break;
-case 'b':
+case 'B':
+ if (h->curset->bus >= 0)
+ parse_hook_add_set (h);
+
+ if (h->curset->domain < 0)
+ h->curset->domain = 0;
+
h->curset->bus = strtol (arg, 0, 16);
break;
-case 'd':
+case 's':
h->curset->dev = strtol (arg, 0, 16);
break;
case 'f':
@@ -261,7 +264,7 @@ netfs_append_args (char **argz, size_t * argz_len)
if (p->bus >= 0)
ADD_OPT ("--bus=0x%02x", p->bus);
if (p->dev >= 0)
- ADD_OPT ("--dev=0x%02x", p->dev);
+ ADD_OPT ("--slot=0x%02x", p->dev);
if (p->func >= 0)
ADD_OPT ("--func=%01u", p->func);
if (p->uid >= 0)
diff --git a/pci-arbiter/options.h b/pci-arbiter/options.h
index 814f81ad..bdb6be78 100644
--- a/pci-arbiter/options.h
+++ b/pci-arbiter/options.h
@@ -51,12 +51,12 @@ struct parse_hook
static const struct argp_option options[] = {
{0, 0, 0, 0, "Permission scope:", 1},
{"class", 'C', "CLASS", 0, "Device class in hexadecimal"},
- {"subclass", 's', "SUBCLASS", 0,
- "Device subclass in hexadecimal, only valid with -c"},
- {"domain", 'D', "DOMAIN", 0, "Device domain in hexadecimal"},
- {"bus", 'b', "BUS", 0, "Device bus in hexadecimal, only valid with -D"},
- {"dev", 'd', "DEV", 0, "Device dev in hexadecimal, only valid with -b"},
- {"func", 'f', "FUNC", 0, "Device func in hexadecimal, only valid with -d"},
+ {"subclass", 'c', "SUBCLASS", 0,
+ "Device subclass in hexadecimal, requires -C"},
+ {"domain", 'd', "DOMAIN", 0, "Device domain in hexadecimal"},
+ {"bus", 'B', "BUS", 0, "Device bus in hexadecimal, requires -D"},
+ {"slot", 's', "SLOT", 0, "Device slot in hexadecimal, requires -b"},
+ {"func", 'f', "FUNC", 0, "Device func in hexadecimal, requires -d"},
{0, 0, 0, 0, "These apply to a given permission scope:", 2},
{"uid", 'U', "UID", 0, "User ID to give permissions to"},
{"gid", 'G', "GID", 0, "Group ID to give permissions to"},
--
2.20.1
pci arbiter: add new --device command line option
Hello Hurd, I made some changes in the options parser to add a new --device option, as a shortcut for current -D -b -d -f options. I also renamed some options in order to free -D and --device to use it for this purpose. From now on, the formerly called "devices" are now called "slots", and "device" will refer to a combination of Domain + Bus + Slot + Function.
Re: [PATCH] pci-arbiter: Remove embedded pciaccess code
Hello Hurd, I'm glad to say that our patches for both picutils and libpciaccess are now upstream. El 10/11/19 a les 2:44, Samuel Thibault ha escrit: > Joan Lledó, le sam. 09 nov. 2019 17:36:19 +0100, a ecrit: >> I was also wondering if after your changes, libpciaccess would support >> nested arbiters, or if is that something we want at all. > > We'd want this, yes. > >> About pciutils, I've seen our patch doesn't use the RPC get_ndevs(), so >> no need to remove it, but it has the closedir() bug too. So I'll fix it >> and send it upstream as well. > > Possibly indeed. > (it's not my patch, see credits :) ) > > Samuel >
Re: [PATCH] Hurd: avoid using the deprecated RPC pci_get_ndevs()
Hi, El 1/12/19 a les 1:10, Damien Zammit ha escrit: > libpciaccess 0.16-1+hurd.1 is still broken on rumpdisk. Somehow the 2 > applied patches > don't allow probing of AHCI to occur in rump_init(). > I've packaged rump libs into a debian package following Robert Millan's work > and sent it to Samuel. > > Damien > Do I need to make more changes on the patch then?
Updated patch for pciutils
Hello Hurd, I updated the pciutils patch I wrote in 2017. The new version is attached. This is the same patch with minor changes: - Makefile updated to be applied on newer sources. - hurd_detect(): Use stat() instead of lstat() to work with symbolic links. - enum_devices(): Call closedir() on error when reading the PCI filesystem. To avoid misunderstandings, this patch is for upstream at: https://github.com/pciutils/pciutils and my intention is to send them a PR if you think the patch is OK.
[PATCH] New access method: Hurd via RPCs
From: Joan Lledó
A new module for the Hurd that accesses PCI bus using available RPCs.
All references to the Hurd in the i386 access method have been removed.
---
lib/Makefile | 7 +-
lib/configure | 4 +-
lib/hurd.c | 381 +
lib/i386-io-hurd.h | 35 -
lib/i386-ports.c | 4 -
lib/init.c | 6 +
lib/internal.h | 2 +-
lib/pci.h | 3 +-
8 files changed, 398 insertions(+), 44 deletions(-)
create mode 100644 lib/hurd.c
delete mode 100644 lib/i386-io-hurd.h
diff --git a/lib/Makefile b/lib/Makefile
index 12bbe34..f6a32e5 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -50,6 +50,10 @@ ifdef PCI_HAVE_PM_SYLIXOS_DEVICE
OBJS += sylixos-device
endif
+ifdef PCI_HAVE_PM_HURD_CONF
+OBJS += hurd
+endif
+
all: $(PCILIB) $(PCILIBPC)
ifeq ($(SHARED),no)
@@ -78,7 +82,7 @@ $(PCILIBPC): libpci.pc.in
init.o: init.c $(INCL)
access.o: access.c $(INCL)
params.o: params.c $(INCL)
-i386-ports.o: i386-ports.c $(INCL) i386-io-hurd.h i386-io-linux.h
i386-io-sunos.h i386-io-windows.h i386-io-cygwin.h
+i386-ports.o: i386-ports.c $(INCL) i386-io-linux.h i386-io-sunos.h
i386-io-windows.h i386-io-cygwin.h
proc.o: proc.c $(INCL) pread.h
sysfs.o: sysfs.c $(INCL) pread.h
generic.o: generic.c $(INCL)
@@ -95,3 +99,4 @@ names-parse.o: names-parse.c $(INCL) names.h
names-hwdb.o: names-hwdb.c $(INCL) names.h
filter.o: filter.c $(INCL)
nbsd-libpci.o: nbsd-libpci.c $(INCL)
+hurd.o: hurd.c $(INCL)
diff --git a/lib/configure b/lib/configure
index 4c328a9..c696175 100755
--- a/lib/configure
+++ b/lib/configure
@@ -122,8 +122,8 @@ case $sys in
LIBRESOLV=
;;
gnu)
- echo_n " i386-ports"
- echo >>$c '#define PCI_HAVE_PM_INTEL_CONF'
+ echo_n " hurd"
+ echo >>$c '#define PCI_HAVE_PM_HURD_CONF'
;;
djgpp)
echo_n " i386-ports"
diff --git a/lib/hurd.c b/lib/hurd.c
new file mode 100644
index 000..74d314c
--- /dev/null
+++ b/lib/hurd.c
@@ -0,0 +1,381 @@
+/*
+ * The PCI Library -- Hurd access via RPCs
+ *
+ * Copyright (c) 2017 Joan Lledó
+ *
+ * Can be freely distributed and used under the terms of the GNU GPL.
+ */
+
+#define _GNU_SOURCE
+
+#include "internal.h"
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* Server path */
+#define _SERVERS_BUS_PCI _SERVERS_BUS "/pci"
+
+/* File names */
+#define FILE_CONFIG_NAME "config"
+#define FILE_ROM_NAME "rom"
+
+/* Level in the fs tree */
+typedef enum
+{
+ LEVEL_NONE,
+ LEVEL_DOMAIN,
+ LEVEL_BUS,
+ LEVEL_DEV,
+ LEVEL_FUNC
+} tree_level;
+
+/* Check whether there's a pci server */
+static int
+hurd_detect (struct pci_access *a)
+{
+ int err;
+ struct stat st;
+
+ err = stat (_SERVERS_BUS_PCI, );
+ if (err)
+{
+ a->error ("Could not open file `%s'", _SERVERS_BUS_PCI);
+ return 0;
+}
+
+ /* The node must be a directory and a translator */
+ return S_ISDIR (st.st_mode) && ((st.st_mode & S_ITRANS) == S_IROOT);
+}
+
+/* Empty callbacks, we don't need any special init or cleanup */
+static void
+hurd_init (struct pci_access *a UNUSED)
+{
+}
+
+static void
+hurd_cleanup (struct pci_access *a UNUSED)
+{
+}
+
+/* Each device has its own server path. Allocate space for the port. */
+static void
+hurd_init_dev (struct pci_dev *d)
+{
+ d->aux = calloc (1, sizeof (mach_port_t));
+ assert (d->aux);
+}
+
+/* Deallocate the port and free its space */
+static void
+hurd_cleanup_dev (struct pci_dev *d)
+{
+ mach_port_t device_port;
+
+ device_port = *((mach_port_t *) d->aux);
+ mach_port_deallocate (mach_task_self (), device_port);
+
+ free (d->aux);
+}
+
+/* Walk through the FS tree to see what is allowed for us */
+static int
+enum_devices (const char *parent, struct pci_access *a, int domain, int bus,
+ int dev, int func, tree_level lev)
+{
+ int err, ret;
+ DIR *dir;
+ struct dirent *entry;
+ char path[NAME_MAX];
+ char server[NAME_MAX];
+ uint32_t vd;
+ uint8_t ht;
+ mach_port_t device_port;
+ struct pci_dev *d;
+
+ dir = opendir (parent);
+ if (!dir)
+return errno;
+
+ while ((entry = readdir (dir)) != 0)
+{
+ snprintf (path, NAME_MAX, "%s/%s", parent, entry->d_name);
+ if (entry->d_type == DT_DIR)
+ {
+ if (!strncmp (entry->d_name, ".", NAME_MAX)
+ || !strncmp (entry->d_name, "..", NAME_MAX))
+ continue;
+
+ errno = 0;
+ ret = strtol (entry->d_name, 0, 16);
+ if (errno)
+ {
+ closedir (dir);
+ return errno;
+ }
+
+ /*
+ * We found a valid directory.
+ * Update the address and switch to the next level.
+ */
+ switch (lev)
+ {
+ case LEVEL_DOMAIN:
+ domain = ret;
+ break;
+ case LEVEL_BUS:
[PATCH] remap translator: remap prefixes instead of complete file names
From: Joan Lledó
* trans/remap.c:
* trivfs_S_dir_lookup():
* Match and replace prefixes instead of complete
file names. This is needed to remap entire file
systems, not only trivial ones.
---
trans/remap.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/trans/remap.c b/trans/remap.c
index 5afbaa02..fcd276d6 100644
--- a/trans/remap.c
+++ b/trans/remap.c
@@ -65,21 +65,30 @@ trivfs_S_dir_lookup (struct trivfs_protid *diruser,
mach_msg_type_name_t *retry_port_type)
{
struct remap *remap;
+ string_t dest = { };
+ size_t prefix_size;
if (!diruser)
return EOPNOTSUPP;
for (remap = remaps; remap; remap = remap->next)
-/* FIXME: should match just prefix of filename too */
-if (!strcmp (remap->from, filename))
- {
+{
+ prefix_size = strlen (remap->from);
+ if (!strncmp (remap->from, filename, prefix_size)
+ && (filename[prefix_size] == '\0' || filename[prefix_size] == '/'))
+ {
+ snprintf (dest, sizeof (dest), "%s%s", remap->to,
+ filename + prefix_size);
+
#ifdef DEBUG
- fprintf (stderr,"replacing %s with %s\n", remap->from, remap->to);
- fflush (stderr);
+ fprintf (stderr, "replacing %s with %s\n", filename, dest);
+ fflush (stderr);
#endif
- filename = remap->to;
- break;
- }
+
+ filename = dest;
+ break;
+ }
+}
*do_retry = FS_RETRY_REAUTH;
*retry_port = getcrdir ();
--
2.20.1
[PATCH] remap translator: remap prefixes instead of complete file names
From: Joan Lledó
* trans/remap.c:
* trivfs_S_dir_lookup():
* Match and replace prefixes instead of complete
file names. This is needed to remap entire file
systems, not only trivial ones.
---
trans/remap.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/trans/remap.c b/trans/remap.c
index 5afbaa02..e3d5fa0a 100644
--- a/trans/remap.c
+++ b/trans/remap.c
@@ -65,19 +65,23 @@ trivfs_S_dir_lookup (struct trivfs_protid *diruser,
mach_msg_type_name_t *retry_port_type)
{
struct remap *remap;
+ char dest[NAME_MAX] = { };
if (!diruser)
return EOPNOTSUPP;
for (remap = remaps; remap; remap = remap->next)
-/* FIXME: should match just prefix of filename too */
-if (!strcmp (remap->from, filename))
+if (!strncmp (remap->from, filename, strlen (remap->from)))
{
+ snprintf (dest, NAME_MAX, "%s%s", remap->to,
+ filename + strlen (remap->from));
+
#ifdef DEBUG
- fprintf (stderr,"replacing %s with %s\n", remap->from, remap->to);
+ fprintf (stderr, "replacing %s with %s\n", filename, dest);
fflush (stderr);
#endif
- filename = remap->to;
+
+ strncpy (filename, dest, NAME_MAX);
break;
}
--
2.20.1
Re: Issues with remap when using a nested pci arbiter
Hi, El 11/12/19 a les 2:03, Samuel Thibault ha escrit: > So indeed, when servers/bus/pci/ gets opened, it's not getting remapped. > Feel free to fix the FIXME :) Yes, that was it, I wrote a patch (attached) and it worked. > I would say that pciutils should be made to use stat instead of > lstat, symlinks are useful. OK, I'll fix it.
Re: Issues with remap when using a nested pci arbiter
Hi, El 8/12/19 a les 14:49, Samuel Thibault ha escrit: > Hello, > > Joan Lledó via Bug reports for the GNU Hurd, le dim. 08 déc. 2019 09:04:09 > +0100, a ecrit: >> demo@debian:~$ remap /servers/bus/pci /home/jlledom/gnu/servers/bus/pci >> -- rpctrace -o /home/demo/demo.log env >> LD_LIBRARY_PATH=/home/jlledom/gnu/lib /home/jlledom/gnu/bin/lspci >> >> After this, I can see in the rpc trace that RPCs are being sent to the >> main arbiter. Any idea on why this happens? > > rpctrace shows you what the program is doing. It would indeed request > servers/bus/pci from its root. But here the root is the remap > translator, which will remap that before actually providing a port. > > Samuel > Sorry, I said I observed the remap problem in the rpc trace, but I observed it in the client's behavior. When running lspci with remap, I can see it returns the devices configured by the main arbiter, not the nested one, just attached the rpc trace to add debug info. The thing is the remap is not working, is there something I'm doing wrong?
Issues with remap when using a nested pci arbiter
Hello Hurd,
To test nested arbiters, I installed a translator in
/home/jlledom/gnu/servers/bus/pci which connects to the main translator
in /servers/bus/pci. Then, logged as 'demo' user, I use remap to ensure
all rpcs are sent to my translator, not the main one:
demo@debian:~$ remap /servers/bus/pci /home/jlledom/gnu/servers/bus/pci
-- rpctrace -o /home/demo/demo.log env
LD_LIBRARY_PATH=/home/jlledom/gnu/lib /home/jlledom/gnu/bin/lspci
After this, I can see in the rpc trace that RPCs are being sent to the
main arbiter. Any idea on why this happens?
task68(pid1033)->vm_statistics () = 0 {4096 183565 5584 46835 19658 94472 0 45760 852 632970 70964 42214 41518}
task68(pid1033)->vm_region (134217728) = 0 134414336 32768 5 7 1 074<--72(pid1033) 0
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
task68(pid1033)->vm_region (134447104) = 0 134447104 4096 3 7 1 074<--75(pid1033) 28672
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
task68(pid1033)->vm_region (134451200) = 0 134451200 4096 3 7 1 074<--72(pid1033) 0
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
task68(pid1033)->vm_region (134455296) = 0x3 ((os/kern) no space available)
task68(pid1033)->vm_map (134455296 -1208197120 0 0 (null) 0 1 0 0 1) = 0 134455296
task68(pid1033)->vm_map (0 4096 0 0 (null) 0 1 0 0 1) = 0x3 ((os/kern) no space available)
task68(pid1033)->task_get_special_port (4) = 074<--75(pid1033)
74<--75(pid1033)->exec_startup_get_info () = 0 134422336 134414388 288 200704 16777216 0 "env\0LD_LIBRARY_PATH=/home/jlledom/gnu/lib\0/home/jlledom/gnu/bin/lspci\0" "MAIL=/var/mail/demo\0USER=demo\0SSH_CLIENT=fc00:122::1 54290 22\0SHLVL=2\0PERL_LOCAL" { 10<--72(pid1033) 9<--77(pid1033) 43<--78(pid1033) 63<--79(pid1033)} { 26<--80(pid1033) 6<--81(pid1033) 50<--82(pid1033) 76<--83(pid1033) 49<--84(pid1033) (null)} {18 0 0 0 0}
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
task68(pid1033)->vm_deallocate (134455296 -1208197120) = 0
6<--81(pid1033)->dir_lookup ("etc/ld.so.cache" 1 0) = 0 2 "etc/ld.so.cache"74<--85(pid1033)
74<--85(pid1033)->io_reauthenticate ( 70<--86(pid-1));
50<--82(pid1033)->auth_user_authenticate ( 70<--86(pid-1)) = 070<--75(pid1033)
task68(pid1033)->mach_port_destroy (pn{ 14}) = 0
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
task68(pid1033)->mach_port_mod_refs (pn{ 15} 0 1) = 0
70<--75(pid1033)->dir_lookup ("etc/ld.so.cache" 1 0) = 0 1 ""74<--86(pid1033)
task68(pid1033)->mach_port_deallocate (pn{ 15}) = 0
task68(pid1033)->mach_port_deallocate (pn{ 15}) = 0
74<--86(pid1033)->io_stat () = 0 {23 5 0 18603 0 1574586183 0 33188 1 0 0 31250 0 1574584083 0 1574584083 0 1574584083 0 8192 64 0 0 0 0 0 0 0 0 0 0 0}
74<--86(pid1033)->io_map () = 070<--85(pid1033) (null)
task68(pid1033)->vm_map (0 31250 0 170<--85(pid1033) 0 32 1 7 1) = 0 16998400
task68(pid1033)->mach_port_deallocate (pn{ 15}) = 0
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
6<--81(pid1033)->dir_lookup ("lib/i386-gnu/libc.so.0.3" 1 0) = 0 2 "lib/i386-gnu/libc.so.0.3"74<--75(pid1033)
74<--75(pid1033)->io_reauthenticate ( 87<--70(pid-1));
50<--82(pid1033)->auth_user_authenticate ( 87<--70(pid-1)) = 087<--86(pid1033)
task68(pid1033)->mach_port_destroy (pn{ 15}) = 0
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
task68(pid1033)->mach_port_mod_refs (pn{ 14} 0 1) = 0
87<--86(pid1033)->dir_lookup ("lib/i386-gnu/libc.so.0.3" 1 0) = 0 1 ""74<--70(pid1033)
task68(pid1033)->mach_port_deallocate (pn{ 14}) = 0
task68(pid1033)->mach_port_deallocate (pn{ 14}) = 0
74<--70(pid1033)->io_read (-1 512) = 0 "\x7fELF\x01\x01\x01\0\0\0\0\0\0\0\0\0\x03\0\x03\0\x01\0\0\0\xe0Q\x05\04\0\0\0\xf40$\0\0\0\0\04\0 \0\n\0(\0r\0q\0\x06\0\0\04\0\0\04\0\0\04\0\0\0@\x01\0\0@\x01\0\0\x04\0\0\0"
74<--70(pid1033)->io_stat () = 0 {23 5 0 32392 0 1562345203 0 33261 1 0 0 2376388 0 1575791529 0 1556731459 0 1562326636 0 8192 4656 0 0 0 0 0 0 0 0 0 0 0}
task68(pid1033)->vm_map (0 8192 0 1 (null) 0 0 3 7 1) = 0 17031168
74<--70(pid1033)->io_map () = 087<--75(pid1033) (null)
task68(pid1033)->vm_map (0 2382748 0 187<--75(pid1033) 0 32 5 7 1) = 0 17039360
task68(pid1033)->mach_port_deallocate (pn{ 14}) = 0
74<--70(pid1033)->io_map () = 087<--86(pid1033) (null)
task68(pid1033)->vm_map (19390464 20480 0 087<--86(pid1033) 2347008 32 3 7 1) = 0x3 ((os/kern) no space available)
task68(pid1033)->vm_deallocate (19390464 20480) = 0
task68(pid1033)->vm_map (19390464 20480 0 087<--86(pid1033) 2347008 32 3 7 1) = 0 19390464
task68(pid1033)->mach_port_deallocate (pn{ 14}) = 0
task68(pid1033)->vm_map (19410944 11164 0 0 (null) 0 0 3 7 1) = 0x3 ((os/kern) no space available)
task68(pid1033)->vm_deallocate (19410944 11164) = 0
task68(pid1033)->vm_map (19410944 11164 0 0 (null) 0 0 3 7 1) = 0 19410944
task68(pid1033)->vm_protect (200704 16777216 0 7) = 0
task68(pid1033)->mach_port_deallocate (pn{ 4}) = 0
6<--81(pid1033)->dir_lookup
Re: [PATCH] Hurd: avoid using the deprecated RPC pci_get_ndevs()
Hi, What about this? Do I send the PR? Is there any alternative for the weak reference? El dt. 26 de 11 de 2019 a les 22:53 +0100, en/na Samuel Thibault va escriure: > Joan Lledó, le lun. 25 nov. 2019 10:02:55 +0100, a ecrit: > > 2- Didn't make any PR b/c I was waiting for comments. > > Ok, now I'm on par with the various changes, thanks for clearing > things > up :) > > It looks good, I believe it can be submitted upstream after Damien > confirms we are on par with the various changes. > > Samuel >
Re: [PATCH] Hurd: avoid using the deprecated RPC pci_get_ndevs()
Hi, I'll try to clear things: 1- I wrote this patch from and for 0.16 in upstream: https://gitlab.freedesktop.org/xorg/lib/libpciaccess 2- Didn't make any PR b/c I was waiting for comments. 3- Didn't thought on Debian at all. 4- If using a weak reference is not good enough for upstream, OK, we can think another approach and I'll implement it and send you guys a new patch for review. El dl. 25 de 11 de 2019 a les 19:34 +1100, en/na Damien Zammit va escriure: > On 25/11/19 9:51 am, Joan Lledó wrote: > > El dg. 24 de 11 de 2019 a les 21:17 +0100, en/na Samuel Thibault va > > escriure: > > > AIUI, apart from my changes, these are the changes which are > > > already > > > upstream? > > > > No, none of this changes are upstream. > > Joan, I think you are getting confused where is "upstream" for > libpciaccess. > As far as I am aware, this repo is upstream: > > https://gitlab.freedesktop.org/xorg/lib/libpciaccess > > I already did some merge requests and got them accepted 6 months ago. > Everything (except your latest conversations in the past few weeks) > has been merged into 0.16. I did some extensive refactoring of the > x86 methods, > and fixed a few bugs that have seemed to return in 0.14-1+hurd.3 > > I tested 0.14-1+hurd.3 on my rump work and got a hang for netdde at > bootup, > but when I installed 0.16 from upstream, everything works fine. > > Please focus your attention on getting 0.16 up to date with your > latest wishes, > and I can help prepare a debian patch for libpciaccess to upgrade > 0.14 to 0.16, > then we can apply one last patch and get it upstreamed! > > > > I have separated them out and uploaded version +hurd.4. > > Oh dear! > > > For the Debian package? I'm sorry, I didn't think about Debian, I > > made > > the patch to sent it directly to libpciaccess upstream. > > I don't see any merge requests for upstream from you, Joan, > (unless you mailed it in to xorg-devel). > If you didn't yet, please don't until we are talking about the same > version. > > Damien
[PATCH] Hurd: avoid using the deprecated RPC pci_get_ndevs()
From: Joan Lledó
---
src/hurd_pci.c | 153 +
1 file changed, 91 insertions(+), 62 deletions(-)
diff --git a/src/hurd_pci.c b/src/hurd_pci.c
index 28bef16..26505f3 100644
--- a/src/hurd_pci.c
+++ b/src/hurd_pci.c
@@ -304,8 +304,8 @@ pci_device_hurd_destroy(struct pci_device *dev)
/* Walk through the FS tree to see what is allowed for us */
static int
-enum_devices(const char *parent, struct pci_device_private **device,
-int domain, int bus, int dev, int func, tree_level lev)
+enum_devices(const char *parent, int domain,
+int bus, int dev, int func, tree_level lev)
{
int err, ret;
DIR *dir;
@@ -315,6 +315,7 @@ enum_devices(const char *parent, struct pci_device_private
**device,
uint32_t reg;
size_t toread;
mach_port_t device_port;
+struct pci_device_private *d, *devices;
dir = opendir(parent);
if (!dir)
@@ -329,8 +330,10 @@ enum_devices(const char *parent, struct pci_device_private
**device,
errno = 0;
ret = strtol(entry->d_name, 0, 16);
-if (errno)
+if (errno) {
+closedir(dir);
return errno;
+}
/*
* We found a valid directory.
@@ -350,14 +353,18 @@ enum_devices(const char *parent, struct
pci_device_private **device,
func = ret;
break;
default:
+if (closedir(dir) < 0)
+return errno;
return -1;
}
-err = enum_devices(path, device, domain, bus, dev, func, lev+1);
-if (err == EPERM)
-continue;
-}
-else {
+err = enum_devices(path, domain, bus, dev, func, lev+1);
+if (err && err != EPERM && err != EACCES) {
+if (closedir(dir) < 0)
+return errno;
+return err;
+}
+} else {
if (strncmp(entry->d_name, FILE_CONFIG_NAME, NAME_MAX))
/* We are looking for the config file */
continue;
@@ -367,52 +374,87 @@ enum_devices(const char *parent, struct
pci_device_private **device,
_SERVERS_BUS_PCI, domain, bus, dev, func,
entry->d_name);
device_port = file_name_lookup(server, 0, 0);
-if (device_port == MACH_PORT_NULL)
+if (device_port == MACH_PORT_NULL) {
+closedir(dir);
return errno;
+}
toread = sizeof(reg);
err = pciclient_cfg_read(device_port, PCI_VENDOR_ID, (char*),
);
-if (err)
+if (err) {
+if (closedir(dir) < 0)
+return errno;
return err;
-if (toread != sizeof(reg))
+}
+if (toread != sizeof(reg)) {
+if (closedir(dir) < 0)
+return errno;
return -1;
+}
-(*device)->base.domain = domain;
-(*device)->base.bus = bus;
-(*device)->base.dev = dev;
-(*device)->base.func = func;
-(*device)->base.vendor_id = PCI_VENDOR(reg);
-(*device)->base.device_id = PCI_DEVICE(reg);
+devices = realloc(pci_sys->devices, (pci_sys->num_devices + 1)
+ * sizeof(struct pci_device_private));
+if (!devices) {
+if (closedir(dir) < 0)
+return errno;
+return ENOMEM;
+}
+
+d = devices + pci_sys->num_devices;
+memset(d, 0, sizeof(struct pci_device_private));
+
+d->base.domain = domain;
+d->base.bus = bus;
+d->base.dev = dev;
+d->base.func = func;
+d->base.vendor_id = PCI_VENDOR(reg);
+d->base.device_id = PCI_DEVICE(reg);
toread = sizeof(reg);
err = pciclient_cfg_read(device_port, PCI_CLASS, (char*),
);
-if (err)
+if (err) {
+if (closedir(dir) < 0)
+return errno;
return err;
-if (toread != sizeof(reg))
+}
+if (toread != sizeof(reg)) {
+if (closedir(dir) < 0)
+return errno;
return -1;
+}
-(*device)->base.device_class = reg >> 8;
-(*device)->base.revision = reg & 0xFF;
+d->base.device_class = reg >> 8;
+d->base.revision = reg & 0xFF;
toread = sizeof(reg);
err = pciclient_cfg_read(device_port, PCI_SUB_VENDOR_ID,
(char*), );
-if (err)
+if (err) {
+if (closedir(dir) < 0)
+
[PATCH] pci-arbiter: Fix memory bugs
From: Joan Lledó
* pci-arbiter/pcifs.c:
* init_file_system: Remove unnecessary free()
* create_fs_tree:
Fix a boundary overrun where no devices are found.
---
pci-arbiter/pcifs.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/pci-arbiter/pcifs.c b/pci-arbiter/pcifs.c
index aaee7f0b..d19fc618 100644
--- a/pci-arbiter/pcifs.c
+++ b/pci-arbiter/pcifs.c
@@ -115,7 +115,6 @@ init_file_system (file_t underlying_node, struct pcifs * fs)
fs->entries = calloc (1, sizeof (struct pcifs_dirent));
if (!fs->entries)
{
- free (fs->entries);
return ENOMEM;
}
@@ -189,6 +188,12 @@ create_fs_tree (struct pcifs * fs)
pci_iterator_destroy(iter);
+ if (nentries == 1)
+{
+ /* No devices found, no need to continue */
+ return 0;
+}
+
list = realloc (fs->entries, nentries * sizeof (struct pcifs_dirent));
if (!list)
return ENOMEM;
--
2.20.1
Re: [PATCH] pci-arbiter: Remove embedded pciaccess code
El 9/11/19 a les 10:48, Joan Lledó via Bug reports for the GNU Hurd ha escrit: > Now I have some questions: > > - You said libpciaccess upstream code for the Hurd doesn't match the one > in the Debian package. Where is the debian repo for the package? I found > [1] but it doesn't seem to have any patch at any branch. > > - You attached a patch for libpciaccess, is it for upstream? for Debian? > is already applied? > And, about pciutils, what did you do?
Re: [PATCH] pci-arbiter: Remove embedded pciaccess code
Hi, El 3/11/19 a les 21:48, Samuel Thibault ha escrit: > Hello, > > Uploaded fixed netdde, pciutils, libpciaccess, and commited this > pci-arbiter cleanup! > Great! Now I have some questions: - You said libpciaccess upstream code for the Hurd doesn't match the one in the Debian package. Where is the debian repo for the package? I found [1] but it doesn't seem to have any patch at any branch. - You attached a patch for libpciaccess, is it for upstream? for Debian? is already applied? _ [https://salsa.debian.org/xorg-team/lib/libpciaccess]
[PATCH] pci-arbiter: Remove spurious memset()
From: Joan Lledó
* pci-arbiter/pcifs.c:
* create_fs_tree:
Remove all memset() previous to snprintf() calls.
---
pci-arbiter/pcifs.c | 4
1 file changed, 4 deletions(-)
diff --git a/pci-arbiter/pcifs.c b/pci-arbiter/pcifs.c
index cc08fad0..aaee7f0b 100644
--- a/pci-arbiter/pcifs.c
+++ b/pci-arbiter/pcifs.c
@@ -207,7 +207,6 @@ create_fs_tree (struct pcifs * fs)
/* We've found a new domain. Add an entry for it */
e_stat = list->stat;
e_stat.st_mode &= ~S_IROOT; /* Remove the root mode */
- memset (entry_name, 0, NAME_SIZE);
snprintf (entry_name, NAME_SIZE, "%04x", device->domain);
err =
create_dir_entry (device->domain, -1, -1, -1, -1, entry_name,
@@ -225,7 +224,6 @@ create_fs_tree (struct pcifs * fs)
if (device->bus != c_bus)
{
/* We've found a new bus. Add an entry for it */
- memset (entry_name, 0, NAME_SIZE);
snprintf (entry_name, NAME_SIZE, "%02x", device->bus);
err =
create_dir_entry (device->domain, device->bus, -1, -1, -1,
@@ -243,7 +241,6 @@ create_fs_tree (struct pcifs * fs)
if (device->dev != c_dev)
{
/* We've found a new dev. Add an entry for it */
- memset (entry_name, 0, NAME_SIZE);
snprintf (entry_name, NAME_SIZE, "%02x", device->dev);
err =
create_dir_entry (device->domain, device->bus, device->dev, -1,
@@ -262,7 +259,6 @@ create_fs_tree (struct pcifs * fs)
e_stat.st_mode &= ~(S_IROTH | S_IWOTH | S_IXOTH);
/* Add func entry */
- memset (entry_name, 0, NAME_SIZE);
snprintf (entry_name, NAME_SIZE, "%01u", device->func);
err =
create_dir_entry (device->domain, device->bus, device->dev,
--
2.20.1
[PATCH] Hurd: avoid using the deprecated RPC pci_get_ndevs()
From: Joan Lledó
---
src/hurd_pci.c | 83 +-
1 file changed, 34 insertions(+), 49 deletions(-)
diff --git a/src/hurd_pci.c b/src/hurd_pci.c
index 28bef16..98bf83e 100644
--- a/src/hurd_pci.c
+++ b/src/hurd_pci.c
@@ -304,8 +304,8 @@ pci_device_hurd_destroy(struct pci_device *dev)
/* Walk through the FS tree to see what is allowed for us */
static int
-enum_devices(const char *parent, struct pci_device_private **device,
-int domain, int bus, int dev, int func, tree_level lev)
+enum_devices(const char *parent, int domain,
+int bus, int dev, int func, tree_level lev)
{
int err, ret;
DIR *dir;
@@ -315,6 +315,7 @@ enum_devices(const char *parent, struct pci_device_private
**device,
uint32_t reg;
size_t toread;
mach_port_t device_port;
+struct pci_device_private *d, *devices;
dir = opendir(parent);
if (!dir)
@@ -353,11 +354,10 @@ enum_devices(const char *parent, struct
pci_device_private **device,
return -1;
}
-err = enum_devices(path, device, domain, bus, dev, func, lev+1);
-if (err == EPERM)
-continue;
-}
-else {
+err = enum_devices(path, domain, bus, dev, func, lev+1);
+if (err && err != EPERM && err != EACCES)
+return err;
+} else {
if (strncmp(entry->d_name, FILE_CONFIG_NAME, NAME_MAX))
/* We are looking for the config file */
continue;
@@ -378,12 +378,20 @@ enum_devices(const char *parent, struct
pci_device_private **device,
if (toread != sizeof(reg))
return -1;
-(*device)->base.domain = domain;
-(*device)->base.bus = bus;
-(*device)->base.dev = dev;
-(*device)->base.func = func;
-(*device)->base.vendor_id = PCI_VENDOR(reg);
-(*device)->base.device_id = PCI_DEVICE(reg);
+devices = realloc(pci_sys->devices, (pci_sys->num_devices + 1)
+ * sizeof(struct pci_device_private));
+if (!devices)
+return ENOMEM;
+
+d = devices + pci_sys->num_devices;
+memset(d, 0, sizeof(struct pci_device_private));
+
+d->base.domain = domain;
+d->base.bus = bus;
+d->base.dev = dev;
+d->base.func = func;
+d->base.vendor_id = PCI_VENDOR(reg);
+d->base.device_id = PCI_DEVICE(reg);
toread = sizeof(reg);
err = pciclient_cfg_read(device_port, PCI_CLASS, (char*),
@@ -393,8 +401,8 @@ enum_devices(const char *parent, struct pci_device_private
**device,
if (toread != sizeof(reg))
return -1;
-(*device)->base.device_class = reg >> 8;
-(*device)->base.revision = reg & 0xFF;
+d->base.device_class = reg >> 8;
+d->base.revision = reg & 0xFF;
toread = sizeof(reg);
err = pciclient_cfg_read(device_port, PCI_SUB_VENDOR_ID,
@@ -404,12 +412,13 @@ enum_devices(const char *parent, struct
pci_device_private **device,
if (toread != sizeof(reg))
return -1;
-(*device)->base.subvendor_id = PCI_VENDOR(reg);
-(*device)->base.subdevice_id = PCI_DEVICE(reg);
+d->base.subvendor_id = PCI_VENDOR(reg);
+d->base.subdevice_id = PCI_DEVICE(reg);
-(*device)->device_port = device_port;
+d->device_port = device_port;
-(*device)++;
+pci_sys->devices = devices;
+pci_sys->num_devices++;
}
}
@@ -441,11 +450,8 @@ static const struct pci_system_methods hurd_pci_methods = {
_pci_hidden int
pci_system_hurd_create(void)
{
-struct pci_device_private *device;
int err;
struct pci_system_hurd *pci_sys_hurd;
-size_t ndevs;
-mach_port_t pci_server_port;
/* If we can open pci cfg io ports on hurd,
* we are the arbiter, therefore try x86 method first */
@@ -462,35 +468,14 @@ pci_system_hurd_create(void)
pci_sys->methods = _pci_methods;
-pci_server_port = file_name_lookup(_SERVERS_BUS_PCI, 0, 0);
-if (!pci_server_port) {
-/* Fall back to x86 access method */
-return pci_system_x86_create();
-}
-
-/* The server gives us the number of available devices for us */
-err = pci_get_ndevs (pci_server_port, );
+pci_sys->num_devices = 0;
+err = enum_devices(_SERVERS_BUS_PCI, -1, -1, -1, -1, LEVEL_DOMAIN);
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();
+
[PATCH 1/4] pci-arbiter: Remove embedded pciaccess code
From: Damien Zammit
This patch removes all embedded pciaccess code from the arbiter
and instead uses the external pciaccess library.
* pci-arbiter/Makefile:
* Remove pci_access.c and x86_pci.c from the sources.
* pci-arbiter/func_files.c:
* io_config_file: Use a harcoded PCI config size.
* read_rom_file:
Grab the full rom first, then return the
requested amount.
* pci-arbiter/main.c:
* main: Call create_fs_tree() w/o pci_sys.
Since it's not part of the translator anymore.
* pci-arbiter/netfs_impl.c:
* netfs_attempt_read:
Send pci_device_cfg_read() as the read op.
* netfs_attempt_write:
Send pci_device_cfg_write() as the write op.
* pci-arbiter/pci-ops.c:
* S_pci_conf_read: Call libpciaccess' pci_device_cfg_read().
* S_pci_conf_write: Call libpciaccess' pci_device_cfg_write().
* S_pci_get_dev_rom:
Set rom.base_addr to zero for the moment, until
libpciaccess esposes it properly.
* pci-arbiter/pci_access.c: Deleted
* pci-arbiter/pci_access.h: Deleted
* pci-arbiter/pcifs.c:
* create_fs_tree:
Remove the pci_sys parameter.
Use libpciaccess' iterator.
Use a hardcoded config space size.
* pci-arbiter/pcifs.h: Definitions for changes in pcifs.c.
* pci-arbiter/x86_pci.c: Deleted.
* pci-arbiter/x86_pci.h: Deleted.
---
pci-arbiter/Makefile | 4 +-
pci-arbiter/TODO | 9 +-
pci-arbiter/func_files.c | 34 +-
pci-arbiter/func_files.h | 4 +
pci-arbiter/main.c | 6 +-
pci-arbiter/netfs_impl.c | 22 +-
pci-arbiter/pci-ops.c| 8 +-
pci-arbiter/pci_access.c | 51 ---
pci-arbiter/pci_access.h | 187 -
pci-arbiter/pcifs.c | 24 +-
pci-arbiter/pcifs.h | 11 +-
pci-arbiter/x86_pci.c| 843 ---
pci-arbiter/x86_pci.h| 34 --
13 files changed, 68 insertions(+), 1169 deletions(-)
delete mode 100644 pci-arbiter/pci_access.c
delete mode 100644 pci-arbiter/pci_access.h
delete mode 100644 pci-arbiter/x86_pci.c
delete mode 100644 pci-arbiter/x86_pci.h
diff --git a/pci-arbiter/Makefile b/pci-arbiter/Makefile
index 357bb081..b13aefa8 100644
--- a/pci-arbiter/Makefile
+++ b/pci-arbiter/Makefile
@@ -20,14 +20,14 @@ makemode= server
PORTDIR = $(srcdir)/port
-SRCS = main.c pci-ops.c pci_access.c x86_pci.c netfs_impl.c \
+SRCS = main.c pci-ops.c netfs_impl.c \
pcifs.c ncache.c options.c func_files.c startup.c \
startup-ops.c
MIGSRCS= pciServer.c startup_notifyServer.c
OBJS = $(patsubst %.S,%.o,$(patsubst %.c,%.o, $(SRCS) $(MIGSRCS)))
HURDLIBS= fshelp ports shouldbeinlibc netfs iohelp ihash
-LDLIBS = -lpthread
+LDLIBS = -lpthread -lpciaccess
target = pci-arbiter
diff --git a/pci-arbiter/TODO b/pci-arbiter/TODO
index 22ae72a8..20060842 100644
--- a/pci-arbiter/TODO
+++ b/pci-arbiter/TODO
@@ -5,11 +5,10 @@
- pci_get_ndevs should be deprecated, applications shouldn't be relying on this
-- we shouldn't duplicate pci_access.[ch] x86_pci.[ch] from libpciaccess, we
- should get libpciaccess to expose pci_system_x86_create() to keep the
- maintenance of x86 port knocking there.
+- In pci-ops.c - config_block_op:
+ Update len with remaining allowed size once op() returns EIO
+ so that we get short reads/writes implemented by leaving it to pciaccess
- At least one difference with libpciaccess is the refresh operation. Perhaps
- we'd need to extend libpciaccess to fix that.
+- Upstream hurdish access method + x86 fixes to libpciaccess
BTW we could also support libpci.
diff --git a/pci-arbiter/func_files.c b/pci-arbiter/func_files.c
index 7df94d2f..c7da6978 100644
--- a/pci-arbiter/func_files.c
+++ b/pci-arbiter/func_files.c
@@ -35,10 +35,11 @@ config_block_op (struct pci_device *dev, off_t offset,
size_t * len,
{
error_t err;
size_t pendent = *len;
+ pciaddr_t actual = 0;
while (pendent >= 4)
{
- err = op (dev->bus, dev->dev, dev->func, offset, data, 4);
+ err = op (dev, data, offset, 4, );
if (err)
return err;
@@ -49,7 +50,7 @@ config_block_op (struct pci_device *dev, off_t offset, size_t
* len,
if (pendent >= 2)
{
- err = op (dev->bus, dev->dev, dev->func, offset, data, 2);
+ err = op (dev, data, offset, 2, );
if (err)
return err;
@@ -60,7 +61,7 @@ config_block_op (struct pci_device *dev, off_t offset, size_t
* len,
if (pendent)
{
- err = op (dev->bus, dev->dev, dev->func, offset, data, 1);
+ err = op (dev, data, offset, 1, );
if (err)
return err;
@@ -85,10 +86,10 @@ io_config_file (struct pci_device * dev, off_t offset,
size_t * len,
assert_backtrace (dev != 0);
/* Don't exceed the config space size */
- if (offset > dev->config_size)
+ if (offset >
[PATCH 2/4] pci-arbiter: Call libpciaccess cleanup on shutdown
From: Joan Lledó * pci-arbiter/startup-ops.c: * S_startup_dosync: Call pci_system_cleanup(). --- pci-arbiter/startup-ops.c | 4 1 file changed, 4 insertions(+) diff --git a/pci-arbiter/startup-ops.c b/pci-arbiter/startup-ops.c index f3506c42..eb387fd9 100644 --- a/pci-arbiter/startup-ops.c +++ b/pci-arbiter/startup-ops.c @@ -20,6 +20,7 @@ #include +#include #include #include "startup.h" @@ -34,6 +35,9 @@ S_startup_dosync (mach_port_t handle) if (!inpi) return EOPNOTSUPP; + // Free all libpciaccess resources + pci_system_cleanup (); + ports_port_deref (inpi); return netfs_shutdown (FSYS_GOAWAY_FORCE); -- 2.20.1
[PATCH 4/4] pci-arbiter: Fix warning on passing incompatible pointer type
From: Joan Lledó
* pci-arbiter/netfs_impl.c:
* netfs_attempt_write: Cast op function to pci_io_op_t
---
pci-arbiter/netfs_impl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/pci-arbiter/netfs_impl.c b/pci-arbiter/netfs_impl.c
index 0be8c370..b987a0bc 100644
--- a/pci-arbiter/netfs_impl.c
+++ b/pci-arbiter/netfs_impl.c
@@ -539,7 +539,7 @@ netfs_attempt_write (struct iouser * cred, struct node *
node,
{
err =
io_config_file (node->nn->ln->device, offset, len, data,
- pci_device_cfg_write);
+ (pci_io_op_t) pci_device_cfg_write);
if (!err)
{
/* Update mtime and ctime */
--
2.20.1
[PATCH 3/4] pci-arbiter: Fix a -Wstringop-truncation warning
From: Joan Lledó
* pci-arbiter/pcifs.c:
* create_dir_entry:
Limit to NAME_SIZE-1 when calling strncpy().
Finish entry->name with '\0'.
* create_fs_tree:
memset() to 0 the directory entry.
Limit to NAME_SIZE-1 all calls to
snprintf() and strncpy().
---
pci-arbiter/pcifs.c | 18 ++
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/pci-arbiter/pcifs.c b/pci-arbiter/pcifs.c
index e68e4b8f..0ff1851c 100644
--- a/pci-arbiter/pcifs.c
+++ b/pci-arbiter/pcifs.c
@@ -45,7 +45,8 @@ create_dir_entry (int32_t domain, int16_t bus, int16_t dev,
entry->dev = dev;
entry->func = func;
entry->device_class = device_class;
- strncpy (entry->name, name, NAME_SIZE);
+ strncpy (entry->name, name, NAME_SIZE - 1);
+ entry->name[NAME_SIZE - 1] = '\0';
entry->parent = parent;
entry->stat = stat;
entry->dir = 0;
@@ -193,6 +194,7 @@ create_fs_tree (struct pcifs * fs)
return ENOMEM;
e = list + 1;
+ memset (e, 0, sizeof (struct pcifs_dirent));
c_domain = c_bus = c_dev = -1;
domain_parent = bus_parent = dev_parent = func_parent = 0;
iter = pci_slot_match_iterator_create();
@@ -206,7 +208,7 @@ create_fs_tree (struct pcifs * fs)
e_stat = list->stat;
e_stat.st_mode &= ~S_IROOT; /* Remove the root mode */
memset (entry_name, 0, NAME_SIZE);
- snprintf (entry_name, NAME_SIZE, "%04x", device->domain);
+ snprintf (entry_name, NAME_SIZE - 1, "%04x", device->domain);
err =
create_dir_entry (device->domain, -1, -1, -1, -1, entry_name,
list, e_stat, 0, 0, e);
@@ -224,7 +226,7 @@ create_fs_tree (struct pcifs * fs)
{
/* We've found a new bus. Add an entry for it */
memset (entry_name, 0, NAME_SIZE);
- snprintf (entry_name, NAME_SIZE, "%02x", device->bus);
+ snprintf (entry_name, NAME_SIZE - 1, "%02x", device->bus);
err =
create_dir_entry (device->domain, device->bus, -1, -1, -1,
entry_name, domain_parent, domain_parent->stat,
@@ -242,7 +244,7 @@ create_fs_tree (struct pcifs * fs)
{
/* We've found a new dev. Add an entry for it */
memset (entry_name, 0, NAME_SIZE);
- snprintf (entry_name, NAME_SIZE, "%02x", device->dev);
+ snprintf (entry_name, NAME_SIZE - 1, "%02x", device->dev);
err =
create_dir_entry (device->domain, device->bus, device->dev, -1,
-1, entry_name, bus_parent, bus_parent->stat, 0,
@@ -261,7 +263,7 @@ create_fs_tree (struct pcifs * fs)
/* Add func entry */
memset (entry_name, 0, NAME_SIZE);
- snprintf (entry_name, NAME_SIZE, "%01u", device->func);
+ snprintf (entry_name, NAME_SIZE - 1, "%01u", device->func);
err =
create_dir_entry (device->domain, device->bus, device->dev,
device->func, device->device_class, entry_name,
@@ -279,7 +281,7 @@ create_fs_tree (struct pcifs * fs)
e_stat.st_size = PCI_CONFIG_SIZE; // FIXME: Hardcoded
/* Create config entry */
- strncpy (entry_name, FILE_CONFIG_NAME, NAME_SIZE);
+ strncpy (entry_name, FILE_CONFIG_NAME, NAME_SIZE - 1);
err =
create_dir_entry (device->domain, device->bus, device->dev,
device->func, device->device_class, entry_name,
@@ -293,7 +295,7 @@ create_fs_tree (struct pcifs * fs)
if (device->regions[j].size > 0)
{
e_stat.st_size = device->regions[j].size;
- snprintf (entry_name, NAME_SIZE, "%s%01u", FILE_REGION_NAME, j);
+ snprintf (entry_name, NAME_SIZE - 1, "%s%01u", FILE_REGION_NAME,
j);
err =
create_dir_entry (device->domain, device->bus, device->dev,
device->func, device->device_class,
@@ -310,7 +312,7 @@ create_fs_tree (struct pcifs * fs)
/* Make rom is read only */
e_stat.st_mode &= ~(S_IWUSR | S_IWGRP);
e_stat.st_size = device->rom_size;
- strncpy (entry_name, FILE_ROM_NAME, NAME_SIZE);
+ strncpy (entry_name, FILE_ROM_NAME, NAME_SIZE - 1);
err =
create_dir_entry (device->domain, device->bus, device->dev,
device->func, device->device_class, entry_name,
--
2.20.1
Re: [PATCH] pci-arbiter: Remove embedded pciaccess code
Hello, El 27/10/19 a les 16:32, Samuel Thibault ha escrit:> Hello, > Could you try to split your changes in separate patches? I splitted the patch into four patches, the first one is the Damien's original patch adapted to the current head + a changelog. The other three patches are my changes. > I don't remember: can we apply this patch in Debian Hurd already? (i.e. > does the libpciaccess there (possibly in unreleased) work fine enough > for our needs?) I've been taking a look at the big picture and I'd say the problem is the GNU Mach restriction to only one process accessing the cfg io ports. Any release where that is enabled, then libpciaccess must include Damien's patches so all clients are redirected to the arbiter. But for the arbiter, the commits we apply won't change anything I'd say, since the arbiter will be just another client trying to access the io ports, no matter if it does it directly or through a library. > This looks a bit complex, I would say we should rather just cast: > > typedef int (*pci_op_t) (struct pci_device * dev, void *data, >pciaddr_t reg, pciaddr_t width, >pciaddr_t * bytes); > > io_config_file (node->nn->ln->device, offset, len, data, > - pci_sys->write); > + (pci_op_t) pci_sys->write); > > Because it is completely compatible to pass a void* to a function that > takes a const void*. That will simplify the whole thing a lot. Def. I made an unnecessary mess. The attached patch just makes the cast. >>/* Copy the regions info */ >> - rom.base_addr = e->device->rom_base; >> + rom.base_addr = 0; // pci_device_private only >>rom.size = e->device->rom_size; >>memcpy (*data, , size); > > The change is because rom_base is a private field in libpciaccess, we'd > need to get libpciaccess to expose it so we can properly take it into > account. For the time being, putting FIXME here would be enough. Understood! I filled that line in the changelog.
[PATCH] pci-arbiter: Remove embedded pciaccess code
From: Joan Lledó
* pci-arbiter/Makefile:
* Remove pci_access.c and x86_pci.c from the sources.
* pci-arbiter/func_files.c:
* config_blog_op: Call the proper i/o function.
* io_config_file: Use a harcoded PCI config size.
* read_rom_file:
Grab the full rom first, then return the
requested amount.
* pci-arbiter/func_files.h:
* pci_io_op_t: Convert it into a struct + union.
So config_blog_op() at func_files.c can choose
the proper function.
* pci-arbiter/main.c:
* main: Call create_fs_tree() w/o pci_sys.
Since it's not part of the translator anymore.
* pci-arbiter/netfs_impl.c:
* netfs_attempt_read:
Set pci_device_cfg_read() as the read op.
Set read as the i/o op to be called.
* netfs_attempt_write:
Set pci_device_cfg_write() as the write op.
Set write as the i/o op to be called.
* pci-arbiter/pci-ops.c:
* S_pci_conf_read: Call libpciaccess' pci_device_cfg_read().
* S_pci_conf_write: Call libpciaccess' pci_device_cfg_write().
* S_pci_get_dev_rom: ?
* pci-arbiter/pci_access.c: Deleted
* pci-arbiter/pci_access.h: Deleted
* pci-arbiter/pcifs.c:
* create_dir_entry: Fix a -Wstringop-truncation warning.
* create_fs_tree:
Likewise.
Remove the pci_sys parameter.
Use libpciaccess' iterator.
Use a hardcoded config space size.
* pci-arbiter/pcifs.h: Definitions for changes in pcifs.c.
* pci-arbiter/startup-ops.c: Call libpciaccess' cleanup on shutdown.
* pci-arbiter/x86_pci.c: Deleted.
* pci-arbiter/x86_pci.h: Deleted.
Author: Damien Zammit
Also-by: Joan Lledó
---
pci-arbiter/Makefile | 12 +-
pci-arbiter/func_files.c | 44 +-
pci-arbiter/func_files.h | 21 +
pci-arbiter/main.c| 4 +-
pci-arbiter/netfs_impl.c | 28 +-
pci-arbiter/pci-ops.c | 8 +-
pci-arbiter/pci_access.c | 51 ---
pci-arbiter/pci_access.h | 187 -
pci-arbiter/pcifs.c | 43 +-
pci-arbiter/pcifs.h | 11 +-
pci-arbiter/startup-ops.c | 4 +
pci-arbiter/x86_pci.c | 843 --
pci-arbiter/x86_pci.h | 34 --
13 files changed, 113 insertions(+), 1177 deletions(-)
delete mode 100644 pci-arbiter/pci_access.c
delete mode 100644 pci-arbiter/pci_access.h
delete mode 100644 pci-arbiter/x86_pci.c
delete mode 100644 pci-arbiter/x86_pci.h
diff --git a/pci-arbiter/Makefile b/pci-arbiter/Makefile
index 357bb081..eb46b835 100644
--- a/pci-arbiter/Makefile
+++ b/pci-arbiter/Makefile
@@ -20,14 +20,14 @@ makemode= server
PORTDIR = $(srcdir)/port
-SRCS = main.c pci-ops.c pci_access.c x86_pci.c netfs_impl.c \
- pcifs.c ncache.c options.c func_files.c startup.c \
- startup-ops.c
-MIGSRCS= pciServer.c startup_notifyServer.c
-OBJS = $(patsubst %.S,%.o,$(patsubst %.c,%.o, $(SRCS) $(MIGSRCS)))
+SRCS = main.c pci-ops.c netfs_impl.c \
+ pcifs.c ncache.c options.c func_files.c startup.c \
+ startup-ops.c
+MIGSRCS= pciServer.c startup_notifyServer.c
+OBJS = $(patsubst %.S,%.o,$(patsubst %.c,%.o, $(SRCS) $(MIGSRCS)))
HURDLIBS= fshelp ports shouldbeinlibc netfs iohelp ihash
-LDLIBS = -lpthread
+LDLIBS = -lpthread -lpciaccess
target = pci-arbiter
diff --git a/pci-arbiter/func_files.c b/pci-arbiter/func_files.c
index 7df94d2f..8768564f 100644
--- a/pci-arbiter/func_files.c
+++ b/pci-arbiter/func_files.c
@@ -35,10 +35,15 @@ config_block_op (struct pci_device *dev, off_t offset,
size_t * len,
{
error_t err;
size_t pendent = *len;
+ pciaddr_t actual = 0;
while (pendent >= 4)
{
- err = op (dev->bus, dev->dev, dev->func, offset, data, 4);
+ if (op.read)
+ err = op.io_op.read (dev, data, offset, 4, );
+ else
+ err = op.io_op.write (dev, data, offset, 4, );
+
if (err)
return err;
@@ -49,7 +54,10 @@ config_block_op (struct pci_device *dev, off_t offset,
size_t * len,
if (pendent >= 2)
{
- err = op (dev->bus, dev->dev, dev->func, offset, data, 2);
+ if (op.read)
+ err = op.io_op.read (dev, data, offset, 2, );
+ else
+ err = op.io_op.write (dev, data, offset, 2, );
if (err)
return err;
@@ -60,7 +68,10 @@ config_block_op (struct pci_device *dev, off_t offset,
size_t * len,
if (pendent)
{
- err = op (dev->bus, dev->dev, dev->func, offset, data, 1);
+ if (op.read)
+ err = op.io_op.read (dev, data, offset, 1, );
+ else
+ err = op.io_op.write (dev, data, offset, 1, );
if (err)
return err;
@@ -85,10 +96,10 @@ io_config_file (struct pci_device * dev, off_t offset,
size_t * len,
assert_backtrace (dev != 0);
/* Don't exceed the config space size */
- if (offset >
Re: [PATCH] - hurd pci-arbiter: remove embedded pciaccess code
Hello Hurd, I made some changes to this patch: 1- I added a call to pci_system_cleanup(); in the shutdown RPC, so libpciaccess is shutdown correctly. 2- As now we're using libpciaccess functions to read/write from netfs_attempt_read/write, and these two libpciaccess functions have different prototypes, we cannot use one single pointer to function like we did until now, so I added some changes to use always the proper prototype. I hope it's not too much tricky. 3- I solved a warning Fix a -Wstringop-truncation warning. 4- I wrote a changelog for the whole patch including Damien's work, except the change in S_pci_get_dev_rom() in pci-ops.c. I don't know what's this change for, but if we're always setting base_addr to 0, then probably we don't need that field anymore and can remove it from struct pci_xrom_bar.
