Feature request: Use the PCIe devices on Thunderbolt (aka PCIe hotplug?)
(Maybe to be moved to misc@) Dear OpenBSD tech@, Thunderbolt support would be awesome. Especially it would allow the use of additional M.2 NVMe SSD:s on a laptop at full performance. Thunderbolt support would also allow the use of an AMDGPU via a PCIe chassi, as well as enable the use of 10gbps Ethernet on laptops [1]. While I like to use Thunderbolt for this pragmatic reason, also Intel apparently promises license etc. generosity to computer makers, which certainly does not hurt. [2] FreeBSD has Thunderbolt support. It appears to me that they call it "PCIe Hot plug". [3] It was implemented 2015 by John-Mark Gurney . Not sure if a TB device must be attached on boot and cannot be detached, anyhow if that is the case then still totally fine. NetBSD appears to have support also but I don't find details. Security-wise Thunderbolt without IOMMU is correlated with physical break-in attack vectors, anyhow that is commonly fine. [4] One Thunderbolt 3 controller provides 22gbps of PCIe data bandwidth to all the one or two Thunderbolt ports it exports, which is fine. [5] Many Thunderbolt devices allow daisy chaining. An "eGFX" certified [6] Thunderbolt PCIe chassi (such as [7]) has absolutely no performance advantage over a normal Thunderbolt PCIe chassi (such as [8]), including for eGPU (e.g. AMDGPU) use. Joseph [1] The lowest cost and most common 10gbps Ethernet Thunderbolt chip is Aquantia AQC107S. There are also some adapters based on a normal PCIe 10gbps chip and a separate Thunderbolt to PCIe controller. [2] https://www.theregister.co.uk/2017/05/24/intel_thunderbolt_3forall/ [3] https://www.freebsd.org/news/status/report-2015-01-2015-03.html#Adding-PCIe-Hot-plug-Support https://www.freebsd.org/news/status/report-2015-07-2015-09.html#Adding-PCIe-Hot-plug-Support [4] https://www.osnews.com/story/129501/thunderbolt-enables-severe-security-threats/ [5] And not 40gbps as common marketing makes it sound like. [6] https://thunderbolttechnology.net/egfx https://thunderbolttechnology.net/blog/the-difference-between-egfx-and-egpu = marketing mumbo jumbo. [7] https://www.asus.com/Graphics-Cards-Accessories/XG-STATION-PRO/ [8] https://www.akitio.com/expansion/node-pro
Re: find.1: Markup primaries with Fl not Cm for easier tags
Hi Klemens, Klemens Nanni wrote on Sat, Mar 21, 2020 at 12:49:20AM +0100: > On Fri, Mar 20, 2020 at 04:53:19PM +0100, Ingo Schwarze wrote: >> I don't feel very strongly about that, but i think .Cm does make >> slightly more sense for primaries than .Fl (and .Ic is probably >> acceptable, too, even though .Cm might be better because these are >> command line arguments, not stand-alone commands). > It does, however because the resulting effect is usually the same, > I see no problem in using `Fl' for primaries. Yes, it wouldn't be a big deal. It might make a difference in unusual situations though, for example if some website would customize .Fl and .Cm differently in mandoc.css. >> I think that is reasonable, yes. I think it makes sense to consider >> tags as words, i.e. strings that usually consist of letters and may >> sometimes contain digits, but never contain whitespace and usually >> do not start with punctuation characters. I'm not sure this rule of >> thumb can be made very strict, but i think it is possible to polish >> this by handling a small number of common cases. For example, >> automatic tagging in man(7) already discards leading whitespace, >> some escape sequences, and leading dashes from tags. Automatic >> tagging in mdoc(7) already discards leading zero-width spaces >> and leading backslashes. I think it would make sense to also >> skip leading dashes here, see the patch below. >> >> Do you agree with that? > Yes, I very much do. Given the fact man(7) already works that way, > I'm happy to see mdoc(7) follow; it seems like the right approach, my > attempt seems more of a workaround for special cases like find(1). > > OK kn Committed, thanks for pointing out the issue and for the feedback on the patch. Yours, Ingo
Re: find.1: Markup primaries with Fl not Cm for easier tags
On Fri, Mar 20, 2020 at 04:53:19PM +0100, Ingo Schwarze wrote: > Not really. In the POSIX sense, options are indeed options while > primaries are arguments. That has implications, for example that > all options must precede all primaries. Fair point. > I don't feel very strongly about that, but i think .Cm does make > slightly more sense for primaries than .Fl (and .Ic is probably > acceptable, too, even though .Cm might be better because these are > command line arguments, not stand-alone commands). It does, however because the resulting effect is usually the same, I see no problem in using `Fl' for primaries. > I think that is reasonable, yes. I think it makes sense to consider > tags as words, i.e. strings that usually consist of letters and may > sometimes contain digits, but never contain whitespace and usually > do not start with punctuation characters. I'm not sure this rule of > thumb can be made very strict, but i think it is possible to polish > this by handling a small number of common cases. For example, > automatic tagging in man(7) already discards leading whitespace, > some escape sequences, and leading dashes from tags. Automatic > tagging in mdoc(7) already discards leading zero-width spaces > and leading backslashes. I think it would make sense to also > skip leading dashes here, see the patch below. > > Do you agree with that? Yes, I very much do. Given the fact man(7) already works that way, I'm happy to see mdoc(7) follow; it seems like the right approach, my attempt seems more of a workaround for special cases like find(1). OK kn
Re: find.1: Markup primaries with Fl not Cm for easier tags
Hi Klemens,
Klemens Nanni wrote on Fri, Mar 20, 2020 at 12:12:39AM +0100:
> In both command line usage and manual output format, find's options
> and primaries behave the same,
Not really. In the POSIX sense, options are indeed options while
primaries are arguments. That has implications, for example that
all options must precede all primaries.
> but their mdoc(7) markup is different
I don't feel very strongly about that, but i think .Cm does make
slightly more sense for primaries than .Fl (and .Ic is probably
acceptable, too, even though .Cm might be better because these are
command line arguments, not stand-alone commands).
> and therefore causes different tag names:
>
> -x (option) can be looked up with ":tx" in the manual pager,
> whereas -amin (primary) requires ":t-amin" including the dash.
>
> I'd like primaries to behave the same like options when it comes to
> tags in manuals, is that a reasonable expectation?
I think that is reasonable, yes. I think it makes sense to consider
tags as words, i.e. strings that usually consist of letters and may
sometimes contain digits, but never contain whitespace and usually
do not start with punctuation characters. I'm not sure this rule of
thumb can be made very strict, but i think it is possible to polish
this by handling a small number of common cases. For example,
automatic tagging in man(7) already discards leading whitespace,
some escape sequences, and leading dashes from tags. Automatic
tagging in mdoc(7) already discards leading zero-width spaces
and leading backslashes. I think it would make sense to also
skip leading dashes here, see the patch below.
Do you agree with that?
Yours,
Ingo
Index: tag.c
===
RCS file: /cvs/src/usr.bin/mandoc/tag.c,v
retrieving revision 1.29
diff -u -p -r1.29 tag.c
--- tag.c 13 Mar 2020 16:14:14 - 1.29
+++ tag.c 20 Mar 2020 15:50:33 -
@@ -87,8 +87,24 @@ tag_put(const char *s, int prio, struct
if (n->child == NULL || n->child->type != ROFFT_TEXT)
return;
s = n->child->string;
- if (s[0] == '\\' && (s[1] == '&' || s[1] == 'e'))
- s += 2;
+ switch (s[0]) {
+ case '-':
+ s++;
+ break;
+ case '\\':
+ switch (s[1]) {
+ case '&':
+ case '-':
+ case 'e':
+ s += 2;
+ break;
+ default:
+ break;
+ }
+ break;
+ default:
+ break;
+ }
}
/*
Re: new option -S for uptime(1)
You could do: expr `date +%s` - `sysctl -n kern.boottime` On Fri, 20 Mar 2020 at 12:54, Alex Naumov wrote: > > Hi, > > this patch adds a new option to uptime(1) that allows to see the length of > time the system has been up in seconds. > Patch includes documentation (man-page) update. > > Example: > $ uptime > 1:26PM up 1:28, 1 user, load averages: 0.00, 0.00, 0.00 > $ uptime -S > 1:26PM 5308 secs, 1 user, load averages: 0.00, 0.00, 0.00 > > It's tested in current on amd64 and aarch64. > > Why I need it? Well, it makes it easy to check the time the system has been > up, for example, via scripts. It doesn't need to parse the output and > recalculate it. > > Cheers, > Alexander
Re: new option -S for uptime(1)
On Fri, Mar 20, 2020 at 01:44:22PM +0100, Alex Naumov wrote: > this patch adds a new option to uptime(1) that allows to see the length of > time the system has been up in seconds. Please don't. Both uptime(1) and w(1) has had several options removed over the past - cleaning, simplyfying and reducing their codebases significantly - and adding options back just to satisfy some more or less ad-hoc need for scripting purposes seems like a bad idea to me. Besides that - the output would still need to be parsed. Regards, Erling > Patch includes documentation (man-page) update. > > Example: > $ uptime > 1:26PM up 1:28, 1 user, load averages: 0.00, 0.00, 0.00 > $ uptime -S > 1:26PM 5308 secs, 1 user, load averages: 0.00, 0.00, 0.00 > > It's tested in current on amd64 and aarch64. > > Why I need it? Well, it makes it easy to check the time the system has been > up, for example, via scripts. It doesn't need to parse the output and > recalculate it. > > Cheers, > Alexander
new option -S for uptime(1)
Hi,
this patch adds a new option to uptime(1) that allows to see the length of
time the system has been up in seconds.
Patch includes documentation (man-page) update.
Example:
$ uptime
1:26PM up 1:28, 1 user, load averages: 0.00, 0.00, 0.00
$ uptime -S
1:26PM 5308 secs, 1 user, load averages: 0.00, 0.00, 0.00
It's tested in current on amd64 and aarch64.
Why I need it? Well, it makes it easy to check the time the system has been
up, for example, via scripts. It doesn't need to parse the output and
recalculate it.
Cheers,
Alexander
Index: uptime.1
===
RCS file: /cvs/src/usr.bin/w/uptime.1,v
retrieving revision 1.15
diff -u -p -u -p -r1.15 uptime.1
--- uptime.1 14 Dec 2017 18:03:03 - 1.15
+++ uptime.1 20 Mar 2020 12:05:40 -
@@ -37,6 +37,7 @@
.Nd show how long system has been running
.Sh SYNOPSIS
.Nm uptime
+.Op Fl S
.Sh DESCRIPTION
The
.Nm uptime
@@ -46,6 +47,11 @@ the number of users, and the load averag
1, 5, and 15 minutes.
This is the first line from
.Xr w 1 .
+.Pp
+The options are as follows:
+.Bl -tag -width Ds
+.It Fl S
+the length of time the system has been up in seconds.
.Sh SEE ALSO
.Xr w 1
.Sh HISTORY
Index: w.c
===
RCS file: /cvs/src/usr.bin/w/w.c,v
retrieving revision 1.66
diff -u -p -u -p -r1.66 w.c
--- w.c 28 Jun 2019 13:35:05 - 1.66
+++ w.c 20 Mar 2020 12:05:40 -
@@ -95,7 +95,7 @@ struct entry {
static void fmt_putc(int, int *);
static void fmt_puts(const char *, int *);
static void pr_args(struct kinfo_proc *);
-static void pr_header(time_t *, int);
+static void pr_header(time_t *, int, int);
static struct stat
*ttystat(char *);
static void usage(int);
@@ -110,6 +110,7 @@ main(int argc, char *argv[])
FILE *ut;
struct in_addr addr;
int ch, i, nentries, nusers, wcmd;
+ int sec_only = 0;
char *memf, *nlistf, *p, *x;
char buf[HOST_NAME_MAX+1], errbuf[_POSIX2_LINE_MAX];
@@ -122,7 +123,7 @@ main(int argc, char *argv[])
p = "hiflM:N:asuw";
} else if (!strcmp(p, "uptime")) {
wcmd = 0;
- p = "";
+ p = "S";
} else
errx(1,
"this program should be invoked only as \"w\" or \"uptime\"");
@@ -143,6 +144,9 @@ main(int argc, char *argv[])
case 'N':
nlistf = optarg;
break;
+ case 'S':
+ sec_only = 1;
+ break;
case 'a':
nflag = 0;
break;
@@ -216,7 +220,7 @@ main(int argc, char *argv[])
(void)fclose(ut);
if (header || wcmd == 0) {
- pr_header(, nusers);
+ pr_header(, nusers, sec_only);
if (wcmd == 0)
exit (0);
}
@@ -422,7 +426,7 @@ nothing:
}
static void
-pr_header(time_t *nowp, int nusers)
+pr_header(time_t *nowp, int nusers, int sec_only)
{
double avenrun[3];
struct timespec boottime;
@@ -442,7 +446,9 @@ pr_header(time_t *nowp, int nusers)
*/
if (clock_gettime(CLOCK_BOOTTIME, ) != -1) {
uptime = boottime.tv_sec;
- if (uptime > 59) {
+ if (uptime <= 59 || sec_only)
+ printf(" %d secs,", (int)uptime);
+ else {
uptime += 30;
days = uptime / SECSPERDAY;
uptime %= SECSPERDAY;
@@ -463,8 +469,7 @@ pr_header(time_t *nowp, int nusers)
(void)printf(" %d min%s,",
mins, mins != 1 ? "s" : "");
}
- } else
- printf(" %d secs,", (int)uptime);
+ }
}
/* Print number of users logged in to system */
@@ -508,6 +513,6 @@ usage(int wcmd)
"usage: w [-ahi] [-M core] [-N system] [user]\n");
else
(void)fprintf(stderr,
- "usage: uptime\n");
+ "usage: uptime [-S]\n");
exit (1);
}
Re: cwm: refactor client cycling
On Tue 2020.01.28 at 13:44 -0500, Okan Demirmen wrote:
> Hi,
>
> The below refactors client cycling to be available from a client context
> instead of limiting to a screen context; this allows bindings for the 4
> related functions (window-{,r}cycle,window-{,r}cycle-ingroup) for either
> key or mouse (current only available via key bindings). With the
> refactor to client context a lot of the layers added over the years to
> make this mimic other WM's can be simplified, I hope.
>
> I'm posting here in case I'm missing an expected behavior, or few.
>
> Feedback welcome.
Updated diff after recent changes.
Index: calmwm.h
===
RCS file: /home/open/cvs/xenocara/app/cwm/calmwm.h,v
retrieving revision 1.373
diff -u -p -r1.373 calmwm.h
--- calmwm.h27 Feb 2020 14:56:39 - 1.373
+++ calmwm.h18 Mar 2020 13:49:55 -
@@ -399,8 +399,8 @@ struct client_ctx *client_init(Window, s
voidclient_lower(struct client_ctx *);
voidclient_move(struct client_ctx *);
voidclient_mtf(struct client_ctx *);
-struct client_ctx *client_next(struct client_ctx *);
-struct client_ctx *client_prev(struct client_ctx *);
+struct client_ctx *client_next(struct client_ctx *, int);
+struct client_ctx *client_prev(struct client_ctx *, int);
voidclient_ptr_inbound(struct client_ctx *, int);
voidclient_ptr_save(struct client_ctx *);
voidclient_ptr_warp(struct client_ctx *);
Index: client.c
===
RCS file: /home/open/cvs/xenocara/app/cwm/client.c,v
retrieving revision 1.261
diff -u -p -r1.261 client.c
--- client.c16 Mar 2020 17:50:44 - 1.261
+++ client.c20 Mar 2020 12:47:43 -
@@ -196,23 +196,43 @@ client_find(Window win)
}
struct client_ctx *
-client_next(struct client_ctx *cc)
+client_next(struct client_ctx *cc, int flags)
{
struct screen_ctx *sc = cc->sc;
- struct client_ctx *newcc;
+ struct client_ctx *nextcc, *ccc;
+ int i = 0;
- return(((newcc = TAILQ_NEXT(cc, entry)) != NULL) ?
- newcc : TAILQ_FIRST(>clientq));
+ TAILQ_FOREACH(ccc, >clientq, entry)
+ i++;
+ if (i <= 1)
+ return cc;
+
+ nextcc = TAILQ_NEXT(cc, entry);
+ if (nextcc == NULL)
+ nextcc = TAILQ_FIRST(>clientq);
+ if (flags && (nextcc->flags & flags))
+ nextcc = client_next(nextcc, flags);
+ return nextcc;
}
struct client_ctx *
-client_prev(struct client_ctx *cc)
+client_prev(struct client_ctx *cc, int flags)
{
struct screen_ctx *sc = cc->sc;
- struct client_ctx *newcc;
+ struct client_ctx *prevcc, *ccc;
+ int i = 0;
+
+ TAILQ_FOREACH(ccc, >clientq, entry)
+ i++;
+ if (i <= 1)
+ return cc;
- return(((newcc = TAILQ_PREV(cc, client_q, entry)) != NULL) ?
- newcc : TAILQ_LAST(>clientq, client_q));
+ prevcc = TAILQ_PREV(cc, client_q, entry);
+ if (prevcc == NULL)
+ prevcc = TAILQ_LAST(>clientq, client_q);
+ if (flags && (prevcc->flags & flags))
+ prevcc = client_prev(prevcc, flags);
+ return prevcc;
}
void
Index: conf.c
===
RCS file: /home/open/cvs/xenocara/app/cwm/conf.c,v
retrieving revision 1.251
diff -u -p -r1.251 conf.c
--- conf.c 27 Feb 2020 14:56:39 - 1.251
+++ conf.c 18 Mar 2020 13:47:52 -
@@ -130,11 +130,11 @@ static const struct {
{ FUNC_CC(window-resize-left-big, client_resize, (CWM_LEFT_BIG)) },
{ FUNC_CC(window-menu-label, client_menu_label, 0) },
- { FUNC_SC(window-cycle, client_cycle, (CWM_CYCLE_FORWARD)) },
- { FUNC_SC(window-rcycle, client_cycle, (CWM_CYCLE_REVERSE)) },
- { FUNC_SC(window-cycle-ingroup, client_cycle,
+ { FUNC_CC(window-cycle, client_cycle, (CWM_CYCLE_FORWARD)) },
+ { FUNC_CC(window-rcycle, client_cycle, (CWM_CYCLE_REVERSE)) },
+ { FUNC_CC(window-cycle-ingroup, client_cycle,
(CWM_CYCLE_FORWARD | CWM_CYCLE_INGROUP)) },
- { FUNC_SC(window-rcycle-ingroup, client_cycle,
+ { FUNC_CC(window-rcycle-ingroup, client_cycle,
(CWM_CYCLE_REVERSE | CWM_CYCLE_INGROUP)) },
{ FUNC_SC(group-cycle, group_cycle, (CWM_CYCLE_FORWARD)) },
Index: kbfunc.c
===
RCS file: /home/open/cvs/xenocara/app/cwm/kbfunc.c,v
retrieving revision 1.168
diff -u -p -r1.168 kbfunc.c
--- kbfunc.c27 Feb 2020 14:56:39 - 1.168
+++ kbfunc.c18 Mar 2020 14:19:21 -
@@ -401,56 +401,33 @@ kbfunc_client_vtile(void *ctx, struct ca
void
kbfunc_client_cycle(void *ctx,
Re: Declare i386's pci_intr_map_msix() as function
> Date: Fri, 20 Mar 2020 12:50:58 +0100
> From: Martin Pieuchot
>
> On 20/03/20(Fri) 12:01, Mark Kettenis wrote:
> > > Date: Fri, 20 Mar 2020 11:54:59 +0100
> > > From: Martin Pieuchot
> > >
> > > As explained in my recent ix(4) diff, using a define when an arch
> > > doesn't support a given function makes the compiler complain that
> > > the arguments are unused:
> > >
> > > /sys/dev/pci/if_ix.c:1789:21: error: unused variable 'dummy'
> > > [-Werror,-Wunused-variable]
> > > pci_intr_handle_tdummy;
> > >^
> > > /sys/dev/pci/if_ix.c:1788:26: error: unused variable 'pa'
> > > [-Werror,-Wunused-variable]
> > > struct pci_attach_args *pa = >os_pa;
> > >
> > > So the diff below uses a stub instead, as discussed with kettenis@.
> > > This is enough to not break any build with the above mentioned ix(4)
> > > diff.
> > >
> > > That said alpha, hppa, landisk, macppc and the various mips64 could
> > > benefit from the same treatment to prevent future bad surprises.
> > >
> > > ok?
> >
> > Does it have to be a real function? Or can it be a static inline?
> > The latter has the benefit that the compiler can optimize away the
> > function call.
>
> static inline works as well:
I think that is preferable then.
ok kettenis@
P.S. I can take care of hppa and macppc.
> Index: arch/i386/pci/pci_machdep.h
> ===
> RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 pci_machdep.h
> --- arch/i386/pci/pci_machdep.h 19 Aug 2018 08:23:47 - 1.30
> +++ arch/i386/pci/pci_machdep.h 20 Mar 2020 11:46:38 -
> @@ -96,7 +96,6 @@ voidpci_conf_write(pci_chipset_tag_t,
> struct pci_attach_args;
> int pci_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *);
> int pci_intr_map(struct pci_attach_args *, pci_intr_handle_t *);
> -#define pci_intr_map_msix(p, vec, ihp) (-1)
> #define pci_intr_line(c, ih)((ih).line)
> const char *pci_intr_string(pci_chipset_tag_t, pci_intr_handle_t);
> void *pci_intr_establish(pci_chipset_tag_t, pci_intr_handle_t,
> @@ -113,6 +112,12 @@ void pci_set_powerstate_md(pci_chipset_
>
> void pci_mcfg_init(bus_space_tag_t, bus_addr_t, int, int, int);
> pci_chipset_tag_t pci_lookup_segment(int);
> +
> +static inline int
> +pci_intr_map_msix(struct pci_attach_args *pa, int vec, pci_intr_handle_t
> *ihp)
> +{
> + return -1;
> +}
>
> /*
> * Section 6.2.4, `Miscellaneous Functions' of the PIC Specification,
>
Re: Declare i386's pci_intr_map_msix() as function
On 20/03/20(Fri) 12:01, Mark Kettenis wrote:
> > Date: Fri, 20 Mar 2020 11:54:59 +0100
> > From: Martin Pieuchot
> >
> > As explained in my recent ix(4) diff, using a define when an arch
> > doesn't support a given function makes the compiler complain that
> > the arguments are unused:
> >
> > /sys/dev/pci/if_ix.c:1789:21: error: unused variable 'dummy'
> > [-Werror,-Wunused-variable]
> > pci_intr_handle_tdummy;
> >^
> > /sys/dev/pci/if_ix.c:1788:26: error: unused variable 'pa'
> > [-Werror,-Wunused-variable]
> > struct pci_attach_args *pa = >os_pa;
> >
> > So the diff below uses a stub instead, as discussed with kettenis@.
> > This is enough to not break any build with the above mentioned ix(4)
> > diff.
> >
> > That said alpha, hppa, landisk, macppc and the various mips64 could
> > benefit from the same treatment to prevent future bad surprises.
> >
> > ok?
>
> Does it have to be a real function? Or can it be a static inline?
> The latter has the benefit that the compiler can optimize away the
> function call.
static inline works as well:
Index: arch/i386/pci/pci_machdep.h
===
RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.h,v
retrieving revision 1.30
diff -u -p -r1.30 pci_machdep.h
--- arch/i386/pci/pci_machdep.h 19 Aug 2018 08:23:47 - 1.30
+++ arch/i386/pci/pci_machdep.h 20 Mar 2020 11:46:38 -
@@ -96,7 +96,6 @@ void pci_conf_write(pci_chipset_tag_t,
struct pci_attach_args;
intpci_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *);
intpci_intr_map(struct pci_attach_args *, pci_intr_handle_t *);
-#definepci_intr_map_msix(p, vec, ihp) (-1)
#definepci_intr_line(c, ih)((ih).line)
const char *pci_intr_string(pci_chipset_tag_t, pci_intr_handle_t);
void *pci_intr_establish(pci_chipset_tag_t, pci_intr_handle_t,
@@ -113,6 +112,12 @@ void pci_set_powerstate_md(pci_chipset_
void pci_mcfg_init(bus_space_tag_t, bus_addr_t, int, int, int);
pci_chipset_tag_t pci_lookup_segment(int);
+
+static inline int
+pci_intr_map_msix(struct pci_attach_args *pa, int vec, pci_intr_handle_t *ihp)
+{
+ return -1;
+}
/*
* Section 6.2.4, `Miscellaneous Functions' of the PIC Specification,
Re: Declare i386's pci_intr_map_msix() as function
> Date: Fri, 20 Mar 2020 11:54:59 +0100
> From: Martin Pieuchot
>
> As explained in my recent ix(4) diff, using a define when an arch
> doesn't support a given function makes the compiler complain that
> the arguments are unused:
>
> /sys/dev/pci/if_ix.c:1789:21: error: unused variable 'dummy'
> [-Werror,-Wunused-variable]
> pci_intr_handle_tdummy;
>^
> /sys/dev/pci/if_ix.c:1788:26: error: unused variable 'pa'
> [-Werror,-Wunused-variable]
> struct pci_attach_args *pa = >os_pa;
>
> So the diff below uses a stub instead, as discussed with kettenis@.
> This is enough to not break any build with the above mentioned ix(4)
> diff.
>
> That said alpha, hppa, landisk, macppc and the various mips64 could
> benefit from the same treatment to prevent future bad surprises.
>
> ok?
Does it have to be a real function? Or can it be a static inline?
The latter has the benefit that the compiler can optimize away the
function call.
> Index: arch/i386/pci/pci_machdep.c
> ===
> RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 pci_machdep.c
> --- arch/i386/pci/pci_machdep.c 7 Jan 2019 23:44:11 - 1.85
> +++ arch/i386/pci/pci_machdep.c 20 Mar 2020 10:41:20 -
> @@ -631,6 +631,12 @@ pci_intr_map_msi(struct pci_attach_args
> }
>
> int
> +pci_intr_map_msix(struct pci_attach_args *pa, int vec, pci_intr_handle_t
> *ihp)
> +{
> + return -1;
> +}
> +
> +int
> pci_intr_map(struct pci_attach_args *pa, pci_intr_handle_t *ihp)
> {
> int pin = pa->pa_rawintrpin;
> Index: arch/i386/pci/pci_machdep.h
> ===
> RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.h,v
> retrieving revision 1.30
> diff -u -p -r1.30 pci_machdep.h
> --- arch/i386/pci/pci_machdep.h 19 Aug 2018 08:23:47 - 1.30
> +++ arch/i386/pci/pci_machdep.h 20 Mar 2020 10:42:45 -
> @@ -96,7 +96,8 @@ voidpci_conf_write(pci_chipset_tag_t,
> struct pci_attach_args;
> int pci_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *);
> int pci_intr_map(struct pci_attach_args *, pci_intr_handle_t *);
> -#define pci_intr_map_msix(p, vec, ihp) (-1)
> +int pci_intr_map_msix(struct pci_attach_args *,
> + int, pci_intr_handle_t *);
> #define pci_intr_line(c, ih)((ih).line)
> const char *pci_intr_string(pci_chipset_tag_t, pci_intr_handle_t);
> void *pci_intr_establish(pci_chipset_tag_t, pci_intr_handle_t,
>
>
Declare i386's pci_intr_map_msix() as function
As explained in my recent ix(4) diff, using a define when an arch
doesn't support a given function makes the compiler complain that
the arguments are unused:
/sys/dev/pci/if_ix.c:1789:21: error: unused variable 'dummy'
[-Werror,-Wunused-variable]
pci_intr_handle_tdummy;
^
/sys/dev/pci/if_ix.c:1788:26: error: unused variable 'pa'
[-Werror,-Wunused-variable]
struct pci_attach_args *pa = >os_pa;
So the diff below uses a stub instead, as discussed with kettenis@.
This is enough to not break any build with the above mentioned ix(4)
diff.
That said alpha, hppa, landisk, macppc and the various mips64 could
benefit from the same treatment to prevent future bad surprises.
ok?
Index: arch/i386/pci/pci_machdep.c
===
RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.c,v
retrieving revision 1.85
diff -u -p -r1.85 pci_machdep.c
--- arch/i386/pci/pci_machdep.c 7 Jan 2019 23:44:11 - 1.85
+++ arch/i386/pci/pci_machdep.c 20 Mar 2020 10:41:20 -
@@ -631,6 +631,12 @@ pci_intr_map_msi(struct pci_attach_args
}
int
+pci_intr_map_msix(struct pci_attach_args *pa, int vec, pci_intr_handle_t *ihp)
+{
+ return -1;
+}
+
+int
pci_intr_map(struct pci_attach_args *pa, pci_intr_handle_t *ihp)
{
int pin = pa->pa_rawintrpin;
Index: arch/i386/pci/pci_machdep.h
===
RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.h,v
retrieving revision 1.30
diff -u -p -r1.30 pci_machdep.h
--- arch/i386/pci/pci_machdep.h 19 Aug 2018 08:23:47 - 1.30
+++ arch/i386/pci/pci_machdep.h 20 Mar 2020 10:42:45 -
@@ -96,7 +96,8 @@ void pci_conf_write(pci_chipset_tag_t,
struct pci_attach_args;
intpci_intr_map_msi(struct pci_attach_args *, pci_intr_handle_t *);
intpci_intr_map(struct pci_attach_args *, pci_intr_handle_t *);
-#definepci_intr_map_msix(p, vec, ihp) (-1)
+intpci_intr_map_msix(struct pci_attach_args *,
+ int, pci_intr_handle_t *);
#definepci_intr_line(c, ih)((ih).line)
const char *pci_intr_string(pci_chipset_tag_t, pci_intr_handle_t);
void *pci_intr_establish(pci_chipset_tag_t, pci_intr_handle_t,
Re: dt: Do not panic on insufficient permissions
On 19/03/20(Thu) 20:53, Klemens Nanni wrote:
> Changing file permissions on /dev/dt to allow non-root access and a
> simple `btrace -l' as non-root user already dt(4)'s KASSERT()s.
Would you mind sharing traces?
> Instead of panicing the machine I think we can return early just like
> in other failure cases and either allow the requested operation as
> non-root (listing available probes works) or let it fail further down
> in the ioctl path (probing as non-root will not work).
>
> Feedback? OK?
Silence failure is not helping. This sounds like a hack to be able to
run dt(4) as a user, not a real audit of what can be done or not.
These two functions aren't safe to be call by !root so why are they
called? That might be a way forward.
That said I'd prefer if we could delay this kind of refactoring until
most of the features are supported. This isn't a priority right now.
> Index: dev/dt/dt_dev.c
> ===
> RCS file: /cvs/src/sys/dev/dt/dt_dev.c,v
> retrieving revision 1.4
> diff -u -p -r1.4 dt_dev.c
> --- dev/dt/dt_dev.c 4 Feb 2020 10:56:15 - 1.4
> +++ dev/dt/dt_dev.c 19 Mar 2020 19:44:27 -
> @@ -406,7 +406,8 @@ dt_ioctl_record_stop(struct dt_softc *sc
> {
> struct dt_pcb *dp;
>
> - KASSERT(suser(curproc) == 0);
> + if (suser(curproc) != 0)
> + return;
>
> if (!sc->ds_recording)
> return;
> @@ -438,7 +439,8 @@ dt_ioctl_probe_enable(struct dt_softc *s
> struct dt_pcb *dp;
> int error;
>
> - KASSERT(suser(curproc) == 0);
> + if (suser(curproc) != 0)
> + return EPERM;
>
> if (!dtioc_req_isvalid(dtrq))
> return EINVAL;
>
Re: bt.5: Document exit, zero and delete functions
On 19/03/20(Thu) 19:58, Klemens Nanni wrote: > On Thu, Mar 19, 2020 at 07:25:05PM +0100, Martin Pieuchot wrote: > > > +.It Fn exit > > > +Terminate with exit code 0 > > > > I don't see the value of mentioning the exit code. What about: > > > > Terminate the execution. > "Terminate" is already a generic term; abort(3) also terminates the > program. Plus, exit(3) takes an argument but btrace's exit() does not, > the instead of leaving users at guess I want to document it explicitly. > > It could also return 1, or garbage or the status of the last function > call or something, but it is always 0/success. Those are all C functions. bt(5) isn't C. There's no idea of error in a bt(5) script. Anyway, I don't think it is worth debate, go ahead with what you have. ok mpi@
Re: find.1: Markup primaries with Fl not Cm for easier tags
On Fri, Mar 20, 2020 at 12:12:39AM +0100, Klemens Nanni wrote:
morning.
> In both command line usage and manual output format, find's options
> and primaries behave the same, but their mdoc(7) markup is different and
> therefore causes different tag names:
>
i'm not sure what you mean by behaving the same, but essentially you
could think of options and primaries as being different (as the page
does now). so i'm not surprised. i don;t really think of -group, for
example, as being an option.
> -x (option) can be looked up with ":tx" in the manual pager,
> whereas -amin (primary) requires ":t-amin" including the dash.
>
> I'd like primaries to behave the same like options when it comes to tags
> in manuals, is that a reasonable expectation?
>
i'm not going to answer that, but...
> If so, diff below switches all primaries from `Cm -amin' to `Fl amin'
> markup such that their resulting tag name is "amin" not "-amin"; man's
> output stays identical.
>
you mean Ic, not Cm, right? if we do decide to do this, the diff would
have to be more comprehensive. there are are a lot of places where
primaries are discussed, which would need to be changed too (i.e. not
just list items).
note also that /-x and /-amin work pretty well, without source changes!
jmc
> Feedback? OK?
>
>
> Index: find.1
> ===
> RCS file: /cvs/src/usr.bin/find/find.1,v
> retrieving revision 1.98
> diff -u -p -U0 -r1.98 find.1
> --- find.12 Sep 2019 21:18:41 - 1.98
> +++ find.119 Mar 2020 22:59:33 -
> @@ -149 +149 @@ the last option given overrides the othe
> -.It Ic -amin Ar n
> +.It Fl amin Ar n
> @@ -156 +156 @@ minutes.
> -.It Ic -anewer Ar file
> +.It Fl anewer Ar file
> @@ -160 +160 @@ True if the current file has a more rece
> -.It Ic -atime Ar n
> +.It Fl atime Ar n
> @@ -167 +167 @@ was started, rounded up to the next full
> -.It Ic -cmin Ar n
> +.It Fl cmin Ar n
> @@ -175 +175 @@ minutes.
> -.It Ic -cnewer Ar file
> +.It Fl cnewer Ar file
> @@ -179 +179 @@ True if the current file has a more rece
> -.It Ic -ctime Ar n
> +.It Fl ctime Ar n
> @@ -187 +187 @@ was started, rounded up to the next full
> -.It Ic -delete
> +.It Fl delete
> @@ -205 +205 @@ Following symlinks is incompatible with
> -.It Ic -depth
> +.It Fl depth
> @@ -211 +211 @@ option.
> -.It Ic -empty
> +.It Fl empty
> @@ -214,2 +214,2 @@ True if the current file or directory is
> -.It Ic -exec Ar utility Oo Ar argument ... Oc \&;
> -.It Ic -exec Ar utility Oo Ar argument ... Oc {} +
> +.It Fl exec Ar utility Oo Ar argument ... Oc \&;
> +.It Fl exec Ar utility Oo Ar argument ... Oc {} +
> @@ -256 +256 @@ does not exceed
> -.It Ic -execdir Ar utility Oo Ar argument ... Oc \&;
> +.It Fl execdir Ar utility Oo Ar argument ... Oc \&;
> @@ -284 +284 @@ flags specified exactly match those of t
> -.It Ic -follow
> +.It Fl follow
> @@ -290 +290 @@ option.
> -.It Ic -fstype Ar type
> +.It Fl fstype Ar type
> @@ -303 +303 @@ mounted read-only.
> -.It Ic -group Ar gname
> +.It Fl group Ar gname
> @@ -312 +312 @@ is treated as a group ID.
> -.It Ic -iname Ar pattern
> +.It Fl iname Ar pattern
> @@ -317 +317 @@ primary except that the matching is done
> -.It Ic -inum Ar n
> +.It Fl inum Ar n
> @@ -321 +321 @@ True if the file has inode number
> -.It Ic -links Ar n
> +.It Fl links Ar n
> @@ -326 +326 @@ links.
> -.It Ic -ls
> +.It Fl ls
> @@ -339 +339 @@ The format is identical to that produced
> -.It Ic -maxdepth Ar n
> +.It Fl maxdepth Ar n
> @@ -343 +343 @@ True if the current search depth is less
> -.It Ic -mindepth Ar n
> +.It Fl mindepth Ar n
> @@ -347 +347 @@ True if the current search depth is at l
> -.It Ic -mmin Ar n
> +.It Fl mmin Ar n
> @@ -354 +354 @@ minutes.
> -.It Ic -mtime Ar n
> +.It Fl mtime Ar n
> @@ -361 +361 @@ was started, rounded up to the next full
> -.It Ic -name Ar pattern
> +.It Fl name Ar pattern
> @@ -367 +367 @@ which may use any of the special charact
> -.It Ic -newer Ar file
> +.It Fl newer Ar file
> @@ -371 +371 @@ True if the current file has a more rece
> -.It Ic -nogroup
> +.It Fl nogroup
> @@ -374 +374 @@ True if the file belongs to an unknown g
> -.It Ic -nouser
> +.It Fl nouser
> @@ -377 +377 @@ True if the file belongs to an unknown u
> -.It Ic -ok Ar utility Oo Ar argument ... Oc \&;
> +.It Fl ok Ar utility Oo Ar argument ... Oc \&;
> @@ -393 +393 @@ expression is false.
> -.It Ic -path Ar pattern
> +.It Fl path Ar pattern
> @@ -427 +427 @@ Note, the first character of a symbolic
> -.It Ic -print
> +.It Fl print
> @@ -434 +434 @@ character.
> -.It Ic -print0
> +.It Fl print0
> @@ -442 +442 @@ option to
> -.It Ic -prune
> +.It Fl prune
> @@ -453 +453 @@ option was specified.
> -.It Ic -size Ar n Ns Op Cm c
> +.It Fl size Ar n Ns Op Cm c
> @@ -465 +465 @@ bytes.
> -.It Ic -type Ar t
> +.It Fl type Ar t
> @@ -486 +486 @@ socket
> -.It Ic -user Ar uname
> +.It Fl user Ar uname
> @@ -495 +495 @@ is treated as a user ID.
> -.It Ic -xdev
> +.It Fl xdev
>
