THREAD_GETMEM support for Hurd in glibc
Is there a way to add fields to the thread descriptor on Hurd? With NPTL, we have THREAD_GETMEM and THREAD_SETMEM to access thread-local variables without relying on the platform TLS implementation. I want to use this for thread-local data in the dynamic loader, specifically for the exception handling context. Using the thread descriptor or TCB for this seems appropriate because this TLS data is not supposed to be duplicated in alternative namespaces (because they share the dynamic loader).
[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.
[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 (&slot_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 (&slot_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 (&slot_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
Re: THREAD_GETMEM support for Hurd in glibc
Hello, Florian Weimer, le sam. 15 févr. 2020 11:11:02 +0100, a ecrit: > Is there a way to add fields to the thread descriptor on Hurd? Sure! > With NPTL, we have THREAD_GETMEM and THREAD_SETMEM to access > thread-local variables without relying on the platform TLS > implementation. I have now added them. Samuel
Re: [hurd,commited] hurd: Add THREAD_GET/SETMEM/_NC
On Feb 15 2020, Samuel Thibault wrote: > diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h > index c0341ce2c9..7ec8c81a76 100644 > --- a/sysdeps/mach/hurd/i386/tls.h > +++ b/sysdeps/mach/hurd/i386/tls.h > @@ -163,6 +163,114 @@ out: > : "i" (offsetof (tcbhead_t, tcb))); \ > __tcb;}) > > +/* Read member of the thread descriptor directly. */ > +# define THREAD_GETMEM(descr, member) \ > + ({ __typeof (descr->member) __value; > \ > + if (sizeof (__value) == 1) > \ > + asm volatile ("movb %%gs:%P2,%b0" \ > + : "=q" (__value) \ > + : "0" (0), "i" (offsetof (tcbhead_t, member))); \ > + else if (sizeof (__value) == 4) \ > + asm volatile ("movl %%gs:%P1,%0" > \ > + : "=r" (__value) \ > + : "i" (offsetof (tcbhead_t, member))); \ > + else \ > + { \ > + if (sizeof (__value) != 8) \ > +/* There should not be any value with a size other than 1, \ > + 4 or 8. */ \ > +abort (); \ _Static_assert? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different."
Re: [hurd,commited] hurd: Add THREAD_GET/SETMEM/_NC
Andreas Schwab, le sam. 15 févr. 2020 14:51:24 +0100, a ecrit: > On Feb 15 2020, Samuel Thibault wrote: > > diff --git a/sysdeps/mach/hurd/i386/tls.h b/sysdeps/mach/hurd/i386/tls.h > > index c0341ce2c9..7ec8c81a76 100644 > > --- a/sysdeps/mach/hurd/i386/tls.h > > +++ b/sysdeps/mach/hurd/i386/tls.h > > @@ -163,6 +163,114 @@ out: > > : "i" (offsetof (tcbhead_t, tcb))); \ > > __tcb;}) > > > > +/* Read member of the thread descriptor directly. */ > > +# define THREAD_GETMEM(descr, member) \ > > + ({ __typeof (descr->member) __value; > > \ > > + if (sizeof (__value) == 1) > > \ > > + asm volatile ("movb %%gs:%P2,%b0" \ > > +: "=q" (__value) \ > > +: "0" (0), "i" (offsetof (tcbhead_t, member))); \ > > + else if (sizeof (__value) == 4) > > \ > > + asm volatile ("movl %%gs:%P1,%0" > > \ > > +: "=r" (__value) \ > > +: "i" (offsetof (tcbhead_t, member))); \ > > + else\ > > + { \ > > +if (sizeof (__value) != 8) \ > > + /* There should not be any value with a size other than 1, \ > > + 4 or 8. */ \ > > + abort (); \ > > _Static_assert? I just copied from i386's tls.h. We should fix them at the same time :) Samuel
Re: [PATCH 1/2] pci-arbiter: Rename command line options
Hello, Thanks for reworking this! Joan Lledó via Bug reports for the GNU Hurd, le sam. 15 févr. 2020 12:38:52 +0100, a ecrit: > 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. Why a capital b? It's be more coherent to have -d -b -s -f all small-caps. Samuel
Re: THREAD_GETMEM support for Hurd in glibc
* Samuel Thibault: > Hello, > > Florian Weimer, le sam. 15 févr. 2020 11:11:02 +0100, a ecrit: >> Is there a way to add fields to the thread descriptor on Hurd? > > Sure! > >> With NPTL, we have THREAD_GETMEM and THREAD_SETMEM to access >> thread-local variables without relying on the platform TLS >> implementation. > > I have now added them. Thanks, this is very useful and allowed me to simplify the loader exception handling.