THREAD_GETMEM support for Hurd in glibc

2020-02-15 Thread Florian Weimer
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

2020-02-15 Thread Joan Lledó via Bug reports for the GNU Hurd
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

2020-02-15 Thread Joan Lledó via Bug reports for the GNU Hurd
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

2020-02-15 Thread Joan Lledó via Bug reports for the GNU Hurd
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

2020-02-15 Thread 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.

Samuel



Re: [hurd,commited] hurd: Add THREAD_GET/SETMEM/_NC

2020-02-15 Thread Andreas Schwab
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

2020-02-15 Thread Samuel Thibault
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

2020-02-15 Thread Samuel Thibault
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

2020-02-15 Thread Florian Weimer
* 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.