systat(1): vmstat: compute rates with CLOCK_UPTIME

2020-09-15 Thread Scott Cheloha
Hi,

systat(1)'s vmstat view displays rates for things like interrupts.
Strangely, it uses CPU time to compute these rates, not real time.

This is potentially misleading, particularly on an MP system.  If I
have 4 cores running on a HZ=100 kernel I expect ~400 clock interrupts
per second.  But systat(1) tells me I have 100 because we have 4
seconds worth of CPU time for every second of real time that elapses.

I don't like it.  I want to see how many interrupts there really were.

This patch changes the vmstat view to use CLOCK_UPTIME to measure
elapsed time and uses that when computing rates.  The "Big Bar" is
still drawn using CPU time, but for everything else I think you would
want a rate based on the elapsed real time.  Using CPU time isn't
intuitive.

We want CLOCK_UPTIME, not CLOCK_MONOTONIC, because we aren't
interested in what the machine was doing when it was suspended.

I have not changed dinfo() to keep the patch simple, but we should be
using CLOCK_UPTIME there, too.

Thoughts?

Index: vmstat.c
===
RCS file: /cvs/src/usr.bin/systat/vmstat.c,v
retrieving revision 1.91
diff -u -p -r1.91 vmstat.c
--- vmstat.c28 Jun 2019 13:35:04 -  1.91
+++ vmstat.c16 Sep 2020 03:56:14 -
@@ -320,7 +320,7 @@ labelkre(void)
 #define PUTRATE(fld, l, c, w) \
do { \
Y(fld); \
-   putint((int)((float)s.fld/etime + 0.5), l, c, w); \
+   putint((int)((float)s.fld / eruntime + 0.5), l, c, w); \
} while (0)
 #define MAXFAIL 5
 
@@ -330,12 +330,18 @@ staticchar cpuorder[] = { CP_INTR, CP_S
 void
 showkre(void)
 {
+   static struct timespec prev;
+   struct timespec elapsed, now;
float f1, f2;
int psiz;
u_int64_t inttotal, intcnt;
int i, l, c;
static int failcnt = 0, first_run = 0;
-   double etime;
+   double ecputime, eruntime;
+
+   clock_gettime(CLOCK_UPTIME, );
+   timespecsub(, , );
+   prev = now;
 
if (state == TIME) {
if (!first_run) {
@@ -343,12 +349,13 @@ showkre(void)
return;
}
}
-   etime = 0;
+   eruntime = elapsed.tv_sec + elapsed.tv_nsec / 10.0;
+   ecputime = 0;
for (i = 0; i < CPUSTATES; i++) {
X(cpustats.cs_time);
-   etime += s.cpustats.cs_time[i];
+   ecputime += s.cpustats.cs_time[i];
}
-   if (etime < 5.0) {  /* < 5 ticks - ignore this trash */
+   if (ecputime < 5.0) {   /* < 5 ticks - ignore this trash */
if (failcnt++ >= MAXFAIL) {
error("The alternate system clock has died!");
failcnt = 0;
@@ -356,12 +363,12 @@ showkre(void)
return;
}
failcnt = 0;
-   etime /= hertz;
+   ecputime /= hertz;
inttotal = 0;
for (i = 0; i < nintr; i++) {
t = intcnt = s.intrcnt[i];
s.intrcnt[i] -= s1.intrcnt[i];
-   intcnt = (u_int64_t)((float)s.intrcnt[i]/etime + 0.5);
+   intcnt = (u_int64_t)((float)s.intrcnt[i] / eruntime + 0.5);
inttotal += intcnt;
if (intrloc[i] != 0)
putuint64(intcnt, intrloc[i], INTSCOL, 8);



Re: acme-client: relax certificate parsing

2020-09-15 Thread Sebastian Benoit
ok

Florian Obser(flor...@openbsd.org) on 2020.09.14 17:12:01 +0200:
> Relax parsing of pem files a bit. Apparently there are CAs that use
> \r\n line endings.
> From Bartosz Kuzma as part of a larger diff.
> 
> OK?
> 
> diff --git certproc.c certproc.c
> index 7fde96e970e..975e12afaaa 100644
> --- certproc.c
> +++ certproc.c
> @@ -28,7 +28,8 @@
>  
>  #include "extern.h"
>  
> -#define MARKER "-END CERTIFICATE-\n"
> +#define BEGIN_MARKER "-BEGIN CERTIFICATE-"
> +#define END_MARKER "-END CERTIFICATE-"
>  
>  int
>  certproc(int netsock, int filesock)
> @@ -81,19 +82,25 @@ certproc(int netsock, int filesock)
>   if ((csr = readbuf(netsock, COMM_CSR, )) == NULL)
>   goto out;
>  
> - if (csrsz < strlen(MARKER)) {
> + if (csrsz < strlen(END_MARKER)) {
>   warnx("invalid cert");
>   goto out;
>   }
>  
> - chaincp = strstr(csr, MARKER);
> + chaincp = strstr(csr, END_MARKER);
>  
>   if (chaincp == NULL) {
>   warnx("invalid cert");
>   goto out;
>   }
>  
> - chaincp += strlen(MARKER);
> + chaincp += strlen(END_MARKER);
> +
> + if ((chaincp = strstr(chaincp, BEGIN_MARKER)) == NULL) {
> + warnx("invalid certificate chain");
> + goto out;
> + }
> +
>   if ((chain = strdup(chaincp)) == NULL) {
>   warn("strdup");
>   goto out;
> 
> 
> -- 
> I'm not entirely sure you are real.
> 



Re: acme-client(1) and Buypass Go SSL

2020-09-15 Thread Sebastian Benoit
ok!

Florian Obser(flor...@openbsd.org) on 2020.09.14 17:15:37 +0200:
> 
> This fell through the cracks back in April.
> 
> We need to be able to provide contact information to use the
> buypass.com acme api.
> 
> OK?
> 
> diff --git etc/examples/acme-client.conf etc/examples/acme-client.conf
> index 32ecd8e8655..40d231725ac 100644
> --- etc/examples/acme-client.conf
> +++ etc/examples/acme-client.conf
> @@ -11,6 +11,18 @@ authority letsencrypt-staging {
>   account key "/etc/acme/letsencrypt-staging-privkey.pem"
>  }
>  
> +authority buypass {
> +api url "https://api.buypass.com/acme/directory;
> +account key "/etc/acme/buypass-privkey.pem"
> +contact "mailto:m...@example.com;
> +}
> +
> +authority buypass-test {
> +api url "https://api.test4.buypass.no/acme/directory;
> +account key "/etc/acme/buypass-test-privkey.pem"
> +contact "mailto:m...@example.com;
> +}
> +
>  domain example.com {
>   alternative names { secure.example.com }
>   domain key "/etc/ssl/private/example.com.key"
> diff --git usr.sbin/acme-client/acme-client.conf.5 
> usr.sbin/acme-client/acme-client.conf.5
> index 08a47a76ab7..41994d13676 100644
> --- usr.sbin/acme-client/acme-client.conf.5
> +++ usr.sbin/acme-client/acme-client.conf.5
> @@ -98,6 +98,11 @@ It defaults to
>  Specify the
>  .Ar url
>  under which the ACME API is reachable.
> +.It Ic contact Ar contact
> +Optional
> +.Ar contact
> +URLs that the authority can use to contact the client for issues related to
> +this account.
>  .El
>  .Sh DOMAINS
>  The certificates to be obtained through ACME.
> diff --git usr.sbin/acme-client/extern.h usr.sbin/acme-client/extern.h
> index 364425b0500..ee341e0950f 100644
> --- usr.sbin/acme-client/extern.h
> +++ usr.sbin/acme-client/extern.h
> @@ -263,7 +263,7 @@ char  *json_getstr(struct jsmnn *, const char 
> *);
>  
>  char *json_fmt_newcert(const char *);
>  char *json_fmt_chkacc(void);
> -char *json_fmt_newacc(void);
> +char *json_fmt_newacc(const char *);
>  char *json_fmt_neworder(const char *const *, size_t);
>  char *json_fmt_protected_rsa(const char *,
>   const char *, const char *, const char *);
> diff --git usr.sbin/acme-client/json.c usr.sbin/acme-client/json.c
> index a6762eeb258..9201f8d2fc3 100644
> --- usr.sbin/acme-client/json.c
> +++ usr.sbin/acme-client/json.c
> @@ -618,14 +618,24 @@ json_fmt_chkacc(void)
>   * Format the "newAccount" resource request.
>   */
>  char *
> -json_fmt_newacc(void)
> +json_fmt_newacc(const char *contact)
>  {
>   int  c;
> - char*p;
> + char*p, *cnt = NULL;
> +
> + if (contact != NULL) {
> + c = asprintf(, "\"contact\": [ \"%s\" ], ", contact);
> + if (c == -1) {
> + warn("asprintf");
> + return NULL;
> + }
> + }
>  
>   c = asprintf(, "{"
> + "%s"
>   "\"termsOfServiceAgreed\": true"
> - "}");
> + "}", cnt == NULL ? "" : cnt);
> + free(cnt);
>   if (c == -1) {
>   warn("asprintf");
>   p = NULL;
> diff --git usr.sbin/acme-client/netproc.c usr.sbin/acme-client/netproc.c
> index 05e36897c38..4490450003e 100644
> --- usr.sbin/acme-client/netproc.c
> +++ usr.sbin/acme-client/netproc.c
> @@ -369,14 +369,14 @@ sreq(struct conn *c, const char *addr, int kid, const 
> char *req, char **loc)
>   * Returns non-zero on success.
>   */
>  static int
> -donewacc(struct conn *c, const struct capaths *p)
> +donewacc(struct conn *c, const struct capaths *p, const char *contact)
>  {
>   struct jsmnn*j = NULL;
>   int  rc = 0;
>   char*req, *detail, *error = NULL;
>   long lc;
>  
> - if ((req = json_fmt_newacc()) == NULL)
> + if ((req = json_fmt_newacc(contact)) == NULL)
>   warnx("json_fmt_newacc");
>   else if ((lc = sreq(c, p->newaccount, 0, req, >kid)) < 0)
>   warnx("%s: bad comm", p->newaccount);
> @@ -410,7 +410,7 @@ donewacc(struct conn *c, const struct capaths *p)
>   * Returns non-zero on success.
>   */
>  static int
> -dochkacc(struct conn *c, const struct capaths *p)
> +dochkacc(struct conn *c, const struct capaths *p, const char *contact)
>  {
>   int  rc = 0;
>   char*req;
> @@ -425,7 +425,7 @@ dochkacc(struct conn *c, const struct capaths *p)
>   else if (c->buf.buf == NULL || c->buf.sz == 0)
>   warnx("%s: empty response", p->newaccount);
>   else if (lc == 400)
> - rc = donewacc(c, p);
> + rc = donewacc(c, p, contact);
>   else
>   rc = 1;
>  
> @@ -755,7 +755,7 @@ netproc(int kfd, int afd, int Cfd, int cfd, int dfd, int 
> rfd,
>   c.newnonce = paths.newnonce;
>  
>   /* Check if our account already exists or create it. */
> - if (!dochkacc(, ))
> + if (!dochkacc(, , authority->contact))
>  

[patch] Don't require a working efifb when probing for EFI/GPT

2020-09-15 Thread Andrew Daugherity
I successfully installed OpenBSD under xhyve, although there were a
few issues, mostly xhyve's fault [1].  One which seems to be an issue
with OpenBSD itself rather than xhyve is that with a headless machine,
the installer always uses MBR mode.  I selected GPT and ignored the
warning "An EFI/GPT disk may not boot. Proceed?"; it boots fine, of
course, since it's using the TianoCore EDK II firmware.  (And MBR mode
would not boot, unless I manually created and populated an EFI System
Partition.)

I tracked this down to install.md checking for efifb (it's expecting
something like 'efifb0 at mainbus0: 1024x768, 32bpp' in the dmesg):

  if dmesg | grep -q 'efifb0 at mainbus0'; then
MDEFI=y
  fi


With a headless VM, however, the dmesg line is:
efifb at mainbus0 not configured

I can't find a better way to detect EFI via dmesg or sysctl hw, so a
simple patch to install.md will have to suffice:

diff --git distrib/amd64/common/install.md distrib/amd64/common/install.md
index d4fc8418a97..0c9db939463 100644
--- distrib/amd64/common/install.md
+++ distrib/amd64/common/install.md
@@ -35,7 +35,7 @@ MDXAPERTURE=2
 MDXDM=y
 NCPU=$(sysctl -n hw.ncpufound)

-if dmesg | grep -q 'efifb0 at mainbus0'; then
+if dmesg | egrep -q 'efifb0? at mainbus0'; then
  MDEFI=y
 fi


(diff also attached)

This probably also applies to physical hardware where efifb is broken
for whatever reason.  I've verified that this still behaves correctly
with a working efifb0.

No idea if this also applies to any other platforms beside amd64, but
a quick glance at i386, arm64, and armv7 seems to say no.


-Andrew

[1] https://github.com/machyve/xhyve/issues/179#issuecomment-561903641


install-efi-gpt.diff
Description: Binary data


PATCH: Add vmmpci device for passthrough PCI

2020-09-15 Thread Jordan Hargrave
This adds a placeholder vmmpci device that will be used for VMD passthrough PCI.

Normally the device will fail to attach unless the PCI domain:bus.dev.func has
been registered with vmmpci_add.  When the device is registered, it will detach
any existing PCI device and reload as vmmpci.  It also attaches an interrupt 
handler
and keeps a running counter of triggered interrupts. VMD will use the counter to
issue commands through to the guest.

diff --git a/sys/arch/amd64/conf/GENERIC b/sys/arch/amd64/conf/GENERIC
index 2c49f91a1..a69c72c26 100644
--- a/sys/arch/amd64/conf/GENERIC
+++ b/sys/arch/amd64/conf/GENERIC
@@ -108,6 +109,7 @@ ksmn*   at pci? # AMD K17 temperature 
sensor
 amas*  at pci? disable # AMD memory configuration
 pchtemp* at pci?   # Intel C610 termperature sensor
 ccp*   at pci? # AMD Cryptographic Co-processor
+vmmpci* at pci?# VMM Placeholder
 
 # National Semiconductor LM7[89] and compatible hardware monitors
 lm0at isa? port 0x290
diff --git a/sys/arch/amd64/conf/files.amd64 b/sys/arch/amd64/conf/files.amd64
index 7a5d40bf4..74c7fe5a9 100644
--- a/sys/arch/amd64/conf/files.amd64
+++ b/sys/arch/amd64/conf/files.amd64
@@ -132,6 +132,10 @@ device pchb: pcibus, agpbus
 attach pchb at pci
 file   arch/amd64/pci/pchb.c   pchb
 
+device vmmpci
+attach vmmpci at pci
+file   arch/amd64/pci/vmmpci.c vmmpci
+
 # AMAS AMD memory address switch
 device amas
 attach amas at pci
diff --git a/sys/arch/amd64/include/vmmpci.h b/sys/arch/amd64/include/vmmpci.h
new file mode 100644
index 0..e012194df
--- /dev/null
+++ b/sys/arch/amd64/include/vmmpci.h
@@ -0,0 +1,24 @@
+/* $OpenBSD: vmmvar.h,v 1.70 2020/04/08 07:39:48 pd Exp $  */
+/*
+ * Copyright (c) 2020 Jordan Hargrave 
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+#ifndef _VMMPCI_H_
+#define _VMMPCI_H_
+
+int vmmpci_find(int, pcitag_t);
+int vmmpci_add(int, pcitag_t, int);
+int vmmpci_pending(int, pcitag_t, uint32_t *);
+
+#endif
diff --git a/sys/arch/amd64/pci/vmmpci.c b/sys/arch/amd64/pci/vmmpci.c
new file mode 100644
index 0..a99efb502
--- /dev/null
+++ b/sys/arch/amd64/pci/vmmpci.c
@@ -0,0 +1,186 @@
+/* $OpenBSD: pcib.c,v 1.6 2013/05/30 16:15:01 deraadt Exp $*/
+/* $NetBSD: pcib.c,v 1.6 1997/06/06 23:29:16 thorpej Exp $ */
+
+/*-
+ * Copyright (c) 1996 Jordan Hargrave
+ *
+ * Permission to use, copy, modify, and distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
+ * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
+ * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
+ * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ */
+
+#include 
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+
+#include 
+
+#include 
+#include 
+
+struct vmmpci_softc {
+   struct device   sc_dev;
+   void*sc_ih;
+
+   int sc_domain;
+   pci_chipset_tag_t   sc_pc;
+   pcitag_tsc_tag;
+
+   uint32_tpending;// pending interrupt 
count
+};
+
+#define VP_VALID   0x8000
+
+/* Keep track of registered devices */
+struct vmmpci_dev {
+   int vp_flags;
+   int vp_domain;
+   pcitag_tvp_tag;
+};
+
+intvmmpci_match(struct device *, void *, void *);
+void   vmmpci_attach(struct device *, struct device *, void *);
+void   vmmpci_callback(struct device *);
+intvmmpci_print(void *, const char *);
+intvmmpci_intr(void *arg);
+
+struct cfattach vmmpci_ca = {
+   sizeof(struct vmmpci_softc), vmmpci_match, vmmpci_attach
+};
+
+struct cfdriver vmmpci_cd = {
+   NULL, "vmmpci", DV_DULL
+};
+
+#define MAXVMMPCI 4
+
+struct vmmpci_dev 

Re: acme-client: improve account creation error message

2020-09-15 Thread Demi M. Obenour
On 2020-09-15 04:25, Florian Obser wrote:
> On Mon, Sep 14, 2020 at 04:26:20PM -0500, Rafael Possamai wrote:
>>> please dont drop the all buffer , or keep it with -vv ?
>>> example : warnx("%s: bad JSON object:%s", p->newaccount, c->buf.buf);
>>>
>>> i don't want to ktrace it to see why the new certbot version is not working
>>
>> Yeah, I think it's good to have the choice to inspect the vomit, maybe you 
>> stumble upon some useful nuggets for troubleshooting an obscure issue.
>>
>> I believe the JSON output is valid, just not pretty. Maybe we could add 
>> syntax highlighting? (just kidding)
>>
> 
> I don't understand what you people are going on about, I didn't touch
> the -v output. You can still roll around in json.
> 
> It's not about if it's valid json or not, it's untrusted input that we
> spit on your console when used with -v.

Simple fix: ensure that all control characters are represented as
Unicode escape sequences.  I believe that the JSON spec requires
this anyway.



Re: curproc vs MP vs locking

2020-09-15 Thread Claudio Jeker
On Tue, Sep 15, 2020 at 04:38:45PM +0200, Mark Kettenis wrote:
> > Date: Tue, 15 Sep 2020 12:34:07 +0200
> > From: Martin Pieuchot 
> > 
> > Many functions in the kernel take a "struct proc *" as argument.  When
> > reviewing diffs or reading the signature of such functions it is not
> > clear if this pointer can be any thread or if it is, like in many cases,
> > pointing to `curproc'.
> > 
> > This distinction matters when it comes to reading/writing members of
> > this "struct proc" and that's why a growing number of functions start
> > with the following idiom:
> > 
> > KASSERT(p == curproc);
> > 
> > This is verbose and redundant, so I suggested to always use `curproc'
> > and stop passing a "struct proc *" as argument when a function isn't
> > meant to modify any thread.  claudio@ raised a concern of performance
> > claiming that `curproc' isn't always cheap.  Is it still true?  Does
> > the KASSERT()s make us pay the cost anyhow?
> 
> Right, because our kernel has DIAGNOSTIC enabled.
> 
> > If that's the case can we adopt a convention to help review functions
> > that take a "struct proc *" but only mean `curproc'?  What about naming
> > this parameter `curp' instead of `p'?
> 
> That'll result in quite a bit of churn.  I'd really like to avoid
> doing that.

I think the conclusion between the people here at k2k20 is that we should
drop the argument from the function and use curproc in the functions
(assigning curproc = p early on). This way there will be no way of using
the wrong proc. 

-- 
:wq Claudio



Re: cap_mkdb: remove igetnext prototype for the function does not exist

2020-09-15 Thread Todd C . Miller
On Tue, 15 Sep 2020 11:35:13 +0800, Kevin Lo wrote:

> ok?

OK millert@

 - todd



Re: curproc vs MP vs locking

2020-09-15 Thread Mark Kettenis
> Date: Tue, 15 Sep 2020 12:34:07 +0200
> From: Martin Pieuchot 
> 
> Many functions in the kernel take a "struct proc *" as argument.  When
> reviewing diffs or reading the signature of such functions it is not
> clear if this pointer can be any thread or if it is, like in many cases,
> pointing to `curproc'.
> 
> This distinction matters when it comes to reading/writing members of
> this "struct proc" and that's why a growing number of functions start
> with the following idiom:
> 
>   KASSERT(p == curproc);
> 
> This is verbose and redundant, so I suggested to always use `curproc'
> and stop passing a "struct proc *" as argument when a function isn't
> meant to modify any thread.  claudio@ raised a concern of performance
> claiming that `curproc' isn't always cheap.  Is it still true?  Does
> the KASSERT()s make us pay the cost anyhow?

Right, because our kernel has DIAGNOSTIC enabled.

> If that's the case can we adopt a convention to help review functions
> that take a "struct proc *" but only mean `curproc'?  What about naming
> this parameter `curp' instead of `p'?

That'll result in quite a bit of churn.  I'd really like to avoid
doing that.



Re: sigabort(), p_sigmask & p_siglist

2020-09-15 Thread Claudio Jeker
On Tue, Sep 15, 2020 at 12:52:40PM +0200, Martin Pieuchot wrote:
> Diff below introduces an helper for sending an uncatchable SIGABRT and
> annotate that `p_siglist' and `p_sigmask' are updated using atomic
> operations.
> 
> As a result setsigvec() becomes local to kern/kern_sig.c.
> 
> Note that other places in the kernel use sigexit(p, SIGABRT) for the
> same purpose and aren't converted by this change.
> 
> Ik?

OK claudio@ but I would split the proc.h into its own commit since it is
unrelated to the sigabort() introduction.
 
> Index: kern/kern_pledge.c
> ===
> RCS file: /cvs/src/sys/kern/kern_pledge.c,v
> retrieving revision 1.263
> diff -u -p -r1.263 kern_pledge.c
> --- kern/kern_pledge.c17 Jul 2020 16:28:19 -  1.263
> +++ kern/kern_pledge.c15 Sep 2020 08:30:19 -
> @@ -529,7 +529,6 @@ pledge_fail(struct proc *p, int error, u
>  {
>   const char *codes = "";
>   int i;
> - struct sigaction sa;
>  
>   /* Print first matching pledge */
>   for (i = 0; code && pledgenames[i].bits != 0; i++)
> @@ -550,11 +549,7 @@ pledge_fail(struct proc *p, int error, u
>   p->p_p->ps_acflag |= APLEDGE;
>  
>   /* Send uncatchable SIGABRT for coredump */
> - memset(, 0, sizeof sa);
> - sa.sa_handler = SIG_DFL;
> - setsigvec(p, SIGABRT, );
> - atomic_clearbits_int(>p_sigmask, sigmask(SIGABRT));
> - psignal(p, SIGABRT);
> + sigabort(p);
>  
>   p->p_p->ps_pledge = 0;  /* Disable all PLEDGE_ flags */
>   KERNEL_UNLOCK();
> Index: kern/kern_proc.c
> ===
> RCS file: /cvs/src/sys/kern/kern_proc.c,v
> retrieving revision 1.86
> diff -u -p -r1.86 kern_proc.c
> --- kern/kern_proc.c  30 Jan 2020 08:51:27 -  1.86
> +++ kern/kern_proc.c  15 Sep 2020 08:52:57 -
> @@ -494,7 +494,6 @@ void
>  db_kill_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
>  {
>   struct process *pr;
> - struct sigaction sa;
>   struct proc *p;
>  
>   pr = prfind(addr);
> @@ -506,11 +505,7 @@ db_kill_cmd(db_expr_t addr, int have_add
>   p = TAILQ_FIRST(>ps_threads);
>  
>   /* Send uncatchable SIGABRT for coredump */
> - memset(, 0, sizeof sa);
> - sa.sa_handler = SIG_DFL;
> - setsigvec(p, SIGABRT, );
> - atomic_clearbits_int(>p_sigmask, sigmask(SIGABRT));
> - psignal(p, SIGABRT);
> + sigabort(p);
>  }
>  
>  void
> Index: kern/kern_sig.c
> ===
> RCS file: /cvs/src/sys/kern/kern_sig.c,v
> retrieving revision 1.262
> diff -u -p -r1.262 kern_sig.c
> --- kern/kern_sig.c   13 Sep 2020 13:33:37 -  1.262
> +++ kern/kern_sig.c   15 Sep 2020 08:33:04 -
> @@ -122,6 +122,8 @@ const int sigprop[NSIG + 1] = {
>  #define  stopsigmask (sigmask(SIGSTOP) | sigmask(SIGTSTP) | \
>   sigmask(SIGTTIN) | sigmask(SIGTTOU))
>  
> +void setsigvec(struct proc *, int, struct sigaction *);
> +
>  void proc_stop(struct proc *p, int);
>  void proc_stop_sweep(void *);
>  void *proc_stop_si;
> @@ -1483,6 +1493,21 @@ sigexit(struct proc *p, int signum)
>   }
>   exit1(p, 0, signum, EXIT_NORMAL);
>   /* NOTREACHED */
> +}
> +
> +/*
> + * Send uncatchable SIGABRT for coredump.
> + */
> +void
> +sigabort(struct proc *p)
> +{
> + struct sigaction sa;
> +
> + memset(, 0, sizeof sa);
> + sa.sa_handler = SIG_DFL;
> + setsigvec(p, SIGABRT, );
> + atomic_clearbits_int(>p_sigmask, sigmask(SIGABRT));
> + psignal(p, SIGABRT);
>  }
>  
>  /*
> Index: sys/signalvar.h
> ===
> RCS file: /cvs/src/sys/sys/signalvar.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 signalvar.h
> --- sys/signalvar.h   13 Sep 2020 13:33:37 -  1.43
> +++ sys/signalvar.h   15 Sep 2020 08:34:15 -
> @@ -126,9 +126,9 @@ void  siginit(struct sigacts *);
>  void trapsignal(struct proc *p, int sig, u_long code, int type,
>   union sigval val);
>  void sigexit(struct proc *, int);
> +void sigabort(struct proc *);
>  int  sigismasked(struct proc *, int);
>  int  sigonstack(size_t);
> -void setsigvec(struct proc *, int, struct sigaction *);
>  int  killpg1(struct proc *, int, int, int);
>  
>  void signal_init(void);
> Index: sys/proc.h
> ===
> RCS file: /cvs/src/sys/sys/proc.h,v
> retrieving revision 1.299
> diff -u -p -r1.299 proc.h
> --- sys/proc.h26 Aug 2020 03:19:09 -  1.299
> +++ sys/proc.h15 Sep 2020 10:15:36 -
> @@ -383,14 +383,14 @@ struct proc {
>   struct  kcov_dev *p_kd; /* kcov device handle */
>   struct  lock_list_entry *p_sleeplocks;  /* WITNESS lock tracking */ 
>  
> - int  p_siglist; /* Signals arrived but not delivered. */
> + int  p_siglist; /* 

Re: diff: pfctl: error message for nonexisting rtable

2020-09-15 Thread Klemens Nanni
On Tue, Sep 15, 2020 at 12:42:27PM +0900, YASUOKA Masahiko wrote:
> It's not clear for me why non-existing rdomain is accepted but
> non-existing rtable is rejected.  I suppose we can make pf(4) can
> handle a packet for the non-existing routing table as if the routing
> table is empty.
Probably possible, but not without further tests or even changes to pf;
I did not want to imply that dynamic `rtable' in pf.conf cannot be done.



sigabort(), p_sigmask & p_siglist

2020-09-15 Thread Martin Pieuchot
Diff below introduces an helper for sending an uncatchable SIGABRT and
annotate that `p_siglist' and `p_sigmask' are updated using atomic
operations.

As a result setsigvec() becomes local to kern/kern_sig.c.

Note that other places in the kernel use sigexit(p, SIGABRT) for the
same purpose and aren't converted by this change.

Ik?

Index: kern/kern_pledge.c
===
RCS file: /cvs/src/sys/kern/kern_pledge.c,v
retrieving revision 1.263
diff -u -p -r1.263 kern_pledge.c
--- kern/kern_pledge.c  17 Jul 2020 16:28:19 -  1.263
+++ kern/kern_pledge.c  15 Sep 2020 08:30:19 -
@@ -529,7 +529,6 @@ pledge_fail(struct proc *p, int error, u
 {
const char *codes = "";
int i;
-   struct sigaction sa;
 
/* Print first matching pledge */
for (i = 0; code && pledgenames[i].bits != 0; i++)
@@ -550,11 +549,7 @@ pledge_fail(struct proc *p, int error, u
p->p_p->ps_acflag |= APLEDGE;
 
/* Send uncatchable SIGABRT for coredump */
-   memset(, 0, sizeof sa);
-   sa.sa_handler = SIG_DFL;
-   setsigvec(p, SIGABRT, );
-   atomic_clearbits_int(>p_sigmask, sigmask(SIGABRT));
-   psignal(p, SIGABRT);
+   sigabort(p);
 
p->p_p->ps_pledge = 0;  /* Disable all PLEDGE_ flags */
KERNEL_UNLOCK();
Index: kern/kern_proc.c
===
RCS file: /cvs/src/sys/kern/kern_proc.c,v
retrieving revision 1.86
diff -u -p -r1.86 kern_proc.c
--- kern/kern_proc.c30 Jan 2020 08:51:27 -  1.86
+++ kern/kern_proc.c15 Sep 2020 08:52:57 -
@@ -494,7 +494,6 @@ void
 db_kill_cmd(db_expr_t addr, int have_addr, db_expr_t count, char *modif)
 {
struct process *pr;
-   struct sigaction sa;
struct proc *p;
 
pr = prfind(addr);
@@ -506,11 +505,7 @@ db_kill_cmd(db_expr_t addr, int have_add
p = TAILQ_FIRST(>ps_threads);
 
/* Send uncatchable SIGABRT for coredump */
-   memset(, 0, sizeof sa);
-   sa.sa_handler = SIG_DFL;
-   setsigvec(p, SIGABRT, );
-   atomic_clearbits_int(>p_sigmask, sigmask(SIGABRT));
-   psignal(p, SIGABRT);
+   sigabort(p);
 }
 
 void
Index: kern/kern_sig.c
===
RCS file: /cvs/src/sys/kern/kern_sig.c,v
retrieving revision 1.262
diff -u -p -r1.262 kern_sig.c
--- kern/kern_sig.c 13 Sep 2020 13:33:37 -  1.262
+++ kern/kern_sig.c 15 Sep 2020 08:33:04 -
@@ -122,6 +122,8 @@ const int sigprop[NSIG + 1] = {
 #definestopsigmask (sigmask(SIGSTOP) | sigmask(SIGTSTP) | \
sigmask(SIGTTIN) | sigmask(SIGTTOU))
 
+void setsigvec(struct proc *, int, struct sigaction *);
+
 void proc_stop(struct proc *p, int);
 void proc_stop_sweep(void *);
 void *proc_stop_si;
@@ -1483,6 +1493,21 @@ sigexit(struct proc *p, int signum)
}
exit1(p, 0, signum, EXIT_NORMAL);
/* NOTREACHED */
+}
+
+/*
+ * Send uncatchable SIGABRT for coredump.
+ */
+void
+sigabort(struct proc *p)
+{
+   struct sigaction sa;
+
+   memset(, 0, sizeof sa);
+   sa.sa_handler = SIG_DFL;
+   setsigvec(p, SIGABRT, );
+   atomic_clearbits_int(>p_sigmask, sigmask(SIGABRT));
+   psignal(p, SIGABRT);
 }
 
 /*
Index: sys/signalvar.h
===
RCS file: /cvs/src/sys/sys/signalvar.h,v
retrieving revision 1.43
diff -u -p -r1.43 signalvar.h
--- sys/signalvar.h 13 Sep 2020 13:33:37 -  1.43
+++ sys/signalvar.h 15 Sep 2020 08:34:15 -
@@ -126,9 +126,9 @@ voidsiginit(struct sigacts *);
 void   trapsignal(struct proc *p, int sig, u_long code, int type,
union sigval val);
 void   sigexit(struct proc *, int);
+void   sigabort(struct proc *);
 intsigismasked(struct proc *, int);
 intsigonstack(size_t);
-void   setsigvec(struct proc *, int, struct sigaction *);
 intkillpg1(struct proc *, int, int, int);
 
 void   signal_init(void);
Index: sys/proc.h
===
RCS file: /cvs/src/sys/sys/proc.h,v
retrieving revision 1.299
diff -u -p -r1.299 proc.h
--- sys/proc.h  26 Aug 2020 03:19:09 -  1.299
+++ sys/proc.h  15 Sep 2020 10:15:36 -
@@ -383,14 +383,14 @@ struct proc {
struct  kcov_dev *p_kd; /* kcov device handle */
struct  lock_list_entry *p_sleeplocks;  /* WITNESS lock tracking */ 
 
-   int  p_siglist; /* Signals arrived but not delivered. */
+   int  p_siglist; /* [a] Signals arrived & not delivered*/
 
 /* End area that is zeroed on creation. */
 #definep_endzero   p_startcopy
 
 /* The following fields are all copied upon creation in fork. */
 #definep_startcopy p_sigmask
-   sigset_t p_sigmask; /* Current signal mask. */
+   sigset_t p_sigmask; /* [a] Current signal 

curproc vs MP vs locking

2020-09-15 Thread Martin Pieuchot
Many functions in the kernel take a "struct proc *" as argument.  When
reviewing diffs or reading the signature of such functions it is not
clear if this pointer can be any thread or if it is, like in many cases,
pointing to `curproc'.

This distinction matters when it comes to reading/writing members of
this "struct proc" and that's why a growing number of functions start
with the following idiom:

KASSERT(p == curproc);

This is verbose and redundant, so I suggested to always use `curproc'
and stop passing a "struct proc *" as argument when a function isn't
meant to modify any thread.  claudio@ raised a concern of performance
claiming that `curproc' isn't always cheap.  Is it still true?  Does
the KASSERT()s make us pay the cost anyhow?

If that's the case can we adopt a convention to help review functions
that take a "struct proc *" but only mean `curproc'?  What about naming
this parameter `curp' instead of `p'?



Re: trunk: keep interface up on port removal

2020-09-15 Thread Klemens Nanni
On Mon, Sep 14, 2020 at 10:57:16AM +0200, Klemens Nanni wrote:
> I tested removing a single port from trunk and observed that both
> interfaces do end up with the same MAC address, but this happens without
> my diff already - I still don't see any behaviour after my diff wrt. MAC
> addresses or anything else but the UP flag on port interfaces.
I did sloppy testing... tl;dr: MACs have been restored properly before
my diff already.

I looked at this yet again because removing a single port and destroying
the trunk use the same code path, hence it didn't make much sense to me
for them to behave differently on second thought.

No matter which port is removed, its MAC is reset to what it was before
becoming a trunk port.

I refrained from inlining supporting shell code/output this time, please
test yourself to confirm.



Re: agentx in services

2020-09-15 Thread Theo de Raadt
Absolutely.

Having it in the file also makes sure that early call to rresvport(3),
bindresvport(3), or bindresvport_sa(3) won't allocate it before the
daemon is started locally.

Martijn van Duren  wrote:

> I currently don't see any reason for adding agentx over tcp support to
> our daemons, but according to RFC2741 section 8.1.1 it should go over
> "wel-known port 705".
> 
> Worth adding or just drop it?
> 
> martijn@
> 
> Index: services
> ===
> RCS file: /cvs/src/etc/services,v
> retrieving revision 1.97
> diff -u -p -r1.97 services
> --- services  26 Jun 2020 19:51:14 -  1.97
> +++ services  15 Sep 2020 09:43:00 -
> @@ -175,6 +175,7 @@ ldaps 636/tcp # LDAP 
> over SSL
>  ldaps636/udp
>  ldp  646/tcp
>  ldp  646/udp
> +agentx   705/tcp
>  silc 706/tcp # Secure Live Internet 
> Conferencing
>  silc 706/udp
>  kerberos-adm 749/tcp # Kerberos 5 kadmin
> 



agentx in services

2020-09-15 Thread Martijn van Duren
I currently don't see any reason for adding agentx over tcp support to
our daemons, but according to RFC2741 section 8.1.1 it should go over
"wel-known port 705".

Worth adding or just drop it?

martijn@

Index: services
===
RCS file: /cvs/src/etc/services,v
retrieving revision 1.97
diff -u -p -r1.97 services
--- services26 Jun 2020 19:51:14 -  1.97
+++ services15 Sep 2020 09:43:00 -
@@ -175,6 +175,7 @@ ldaps   636/tcp # LDAP 
over SSL
 ldaps  636/udp
 ldp646/tcp
 ldp646/udp
+agentx 705/tcp
 silc   706/tcp # Secure Live Internet 
Conferencing
 silc   706/udp
 kerberos-adm   749/tcp # Kerberos 5 kadmin



Re: agentx and clang static analyzer

2020-09-15 Thread Martijn van Duren
> > Index: subagentx.c
> > ===
> > RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
> > retrieving revision 1.1
> > diff -u -p -r1.1 subagentx.c
> > --- subagentx.c 14 Sep 2020 11:30:25 -  1.1
> > +++ subagentx.c 15 Sep 2020 09:05:58 -
> > @@ -2929,7 +2929,7 @@ getnext:
> > index->sav_idatacomplete = 1;
> > break;
> > case AGENTX_DATA_TYPE_IPADDRESS:
> > -   ipaddress = calloc(1, sizeof(ipaddress));
> > +   ipaddress = calloc(1, sizeof(*ipaddress));
> > if (ipaddress == NULL) {
> > subagentx_log_sag_warn(sag,
> > "Failed to bind ipaddress index");
> 
> Not ok for this, it's probably correct, but there are other instances of this
> in this code and so you need to engage brain, not static analyzer, go fix or
> don't fix them all in a separate commit. 
> 
Could only find one more:

Index: subagentx.c
===
RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
retrieving revision 1.1
diff -u -p -r1.1 subagentx.c
--- subagentx.c 14 Sep 2020 11:30:25 -  1.1
+++ subagentx.c 15 Sep 2020 09:27:46 -
@@ -2929,7 +2929,7 @@ getnext:
index->sav_idatacomplete = 1;
break;
case AGENTX_DATA_TYPE_IPADDRESS:
-   ipaddress = calloc(1, sizeof(ipaddress));
+   ipaddress = calloc(1, sizeof(*ipaddress));
if (ipaddress == NULL) {
subagentx_log_sag_warn(sag,
"Failed to bind ipaddress index");
@@ -2951,7 +2951,7 @@ getnext:
}
if (j <= sav->sav_vb.avb_oid.aoi_idlen)
index->sav_idatacomplete = 1;
-   data->avb_ostring.aos_slen = sizeof(ipaddress);
+   data->avb_ostring.aos_slen = sizeof(*ipaddress);
data->avb_ostring.aos_string =
(unsigned char *)ipaddress;
break;



Re: agentx and clang static analyzer

2020-09-15 Thread Bob Beck
On Tue, Sep 15, 2020 at 11:08:04AM +0200, Martijn van Duren wrote:
> There are 3 things that actually look like valid complaints when running
> clang's static analyzer.
> 
> 1) A dead store in agentx_recv.
> 2) sizeof(ipaddress) intead of sizeof(*ipaddress). Since this is ipv4,
>this is only a problem if sizeof(pointer) is smaller then 4 bytes,
>which can't happen afaik.
> 3) dst does indeed need to be dereffed, but since dstlen and buflen are
>initialized to 0 and srclen is practically always larger then 0
>we're fine. I'm keeping the *dst here as an additional safeguard.
> 
> The rest seem like false positives to me.
> 
> OK?
> 
> martijn@
> 
> Index: agentx.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/agentx.c,v
> retrieving revision 1.16
> diff -u -p -r1.16 agentx.c
> --- agentx.c  14 Sep 2020 11:30:25 -  1.16
> +++ agentx.c  15 Sep 2020 09:05:57 -
> @@ -135,7 +135,6 @@ agentx_recv(struct agentx *ax)
>   header.aph_packetid = agentx_pdutoh32(, u8);
>   u8 += 4;
>   header.aph_plength = agentx_pdutoh32(, u8);
> - u8 += 4;
>  
>   if (header.aph_version != 1) {
>   errno = EPROTO;

ok for this piece above


> Index: subagentx.c
> ===
> RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
> retrieving revision 1.1
> diff -u -p -r1.1 subagentx.c
> --- subagentx.c   14 Sep 2020 11:30:25 -  1.1
> +++ subagentx.c   15 Sep 2020 09:05:58 -
> @@ -2929,7 +2929,7 @@ getnext:
>   index->sav_idatacomplete = 1;
>   break;
>   case AGENTX_DATA_TYPE_IPADDRESS:
> - ipaddress = calloc(1, sizeof(ipaddress));
> + ipaddress = calloc(1, sizeof(*ipaddress));
>   if (ipaddress == NULL) {
>   subagentx_log_sag_warn(sag,
>   "Failed to bind ipaddress index");

Not ok for this, it's probably correct, but there are other instances of this
in this code and so you need to engage brain, not static analyzer, go fix or
don't fix them all in a separate commit. 


> @@ -3833,7 +3833,7 @@ subagentx_strcat(char **dst, const char 
>   }
>  
>   srclen = strlen(src);
> - if (dst == NULL || dstlen + srclen > buflen) {
> + if (*dst == NULL || dstlen + srclen > buflen) {
>   nbuflen = (((dstlen + srclen) / 512) + 1) * 512;
>   tmp = recallocarray(*dst, buflen, nbuflen, sizeof(*tmp));
>   if (tmp == NULL)
> 
ok for this piece above



agentx and clang static analyzer

2020-09-15 Thread Martijn van Duren
There are 3 things that actually look like valid complaints when running
clang's static analyzer.

1) A dead store in agentx_recv.
2) sizeof(ipaddress) intead of sizeof(*ipaddress). Since this is ipv4,
   this is only a problem if sizeof(pointer) is smaller then 4 bytes,
   which can't happen afaik.
3) dst does indeed need to be dereffed, but since dstlen and buflen are
   initialized to 0 and srclen is practically always larger then 0
   we're fine. I'm keeping the *dst here as an additional safeguard.

The rest seem like false positives to me.

OK?

martijn@

Index: agentx.c
===
RCS file: /cvs/src/usr.sbin/relayd/agentx.c,v
retrieving revision 1.16
diff -u -p -r1.16 agentx.c
--- agentx.c14 Sep 2020 11:30:25 -  1.16
+++ agentx.c15 Sep 2020 09:05:57 -
@@ -135,7 +135,6 @@ agentx_recv(struct agentx *ax)
header.aph_packetid = agentx_pdutoh32(, u8);
u8 += 4;
header.aph_plength = agentx_pdutoh32(, u8);
-   u8 += 4;
 
if (header.aph_version != 1) {
errno = EPROTO;
Index: subagentx.c
===
RCS file: /cvs/src/usr.sbin/relayd/subagentx.c,v
retrieving revision 1.1
diff -u -p -r1.1 subagentx.c
--- subagentx.c 14 Sep 2020 11:30:25 -  1.1
+++ subagentx.c 15 Sep 2020 09:05:58 -
@@ -2929,7 +2929,7 @@ getnext:
index->sav_idatacomplete = 1;
break;
case AGENTX_DATA_TYPE_IPADDRESS:
-   ipaddress = calloc(1, sizeof(ipaddress));
+   ipaddress = calloc(1, sizeof(*ipaddress));
if (ipaddress == NULL) {
subagentx_log_sag_warn(sag,
"Failed to bind ipaddress index");
@@ -3833,7 +3833,7 @@ subagentx_strcat(char **dst, const char 
}
 
srclen = strlen(src);
-   if (dst == NULL || dstlen + srclen > buflen) {
+   if (*dst == NULL || dstlen + srclen > buflen) {
nbuflen = (((dstlen + srclen) / 512) + 1) * 512;
tmp = recallocarray(*dst, buflen, nbuflen, sizeof(*tmp));
if (tmp == NULL)



Re: acme-client: improve account creation error message

2020-09-15 Thread Florian Obser
On Mon, Sep 14, 2020 at 04:26:20PM -0500, Rafael Possamai wrote:
> >please dont drop the all buffer , or keep it with -vv ?
> >example : warnx("%s: bad JSON object:%s", p->newaccount, c->buf.buf);
> >
> >i don't want to ktrace it to see why the new certbot version is not working
> 
> Yeah, I think it's good to have the choice to inspect the vomit, maybe you 
> stumble upon some useful nuggets for troubleshooting an obscure issue.
> 
> I believe the JSON output is valid, just not pretty. Maybe we could add 
> syntax highlighting? (just kidding)
> 

I don't understand what you people are going on about, I didn't touch
the -v output. You can still roll around in json.

It's not about if it's valid json or not, it's untrusted input that we
spit on your console when used with -v.

-- 
I'm not entirely sure you are real.



Re: PATCH: Add ACPI IVHD_EXT structure to acpireg.h

2020-09-15 Thread Mark Kettenis
> Date: Tue, 15 Sep 2020 01:37:33 -0500
> From: Jordan Hargrave 
> 
> This patch adds a couple of entries for AMD IOMMU structure
> definitions in ACPI

ok kettenis@

> Index: acpireg.h
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpireg.h,v
> retrieving revision 1.45
> diff -u -p -r1.45 acpireg.h
> --- acpireg.h 28 Aug 2019 22:39:09 -  1.45
> +++ acpireg.h 15 Sep 2020 06:29:50 -
> @@ -623,6 +623,9 @@ struct acpi_ivmd {
>  struct acpi_ivhd {
>   uint8_t type;
>   uint8_t flags;
> +#define IVHD_PPRSUP  (1L << 7)
> +#define IVHD_PREFSUP (1L << 6)
> +#define IVHD_COHERENT(1L << 5)
>  #define IVHD_IOTLB   (1L << 4)
>  #define IVHD_ISOC(1L << 3)
>  #define IVHD_RESPASSPW   (1L << 2)
> @@ -638,13 +641,28 @@ struct acpi_ivhd {
>  #define IVHD_UNITID_MASK 0x1F
>  #define IVHD_MSINUM_SHIFT0
>  #define IVHD_MSINUM_MASK 0x1F
> - uint32_treserved;
> + uint32_tfeature;
> +} __packed;
> +
> +struct acpi_ivhd_ext {
> + uint8_t type;
> + uint8_t flags;
> + uint16_tlength;
> + uint16_tdevid;
> + uint16_tcap;
> + uint64_taddress;
> + uint16_tsegment;
> + uint16_tinfo;
> + uint32_tattrib;
> + uint64_tefr;
> + uint8_t reserved[8];
>  } __packed;
>  
>  union acpi_ivrs_entry {
>   struct {
>   uint8_t type;
>  #define IVRS_IVHD0x10
> +#define IVRS_IVHD_EXT0x11
>  #define IVRS_IVMD_ALL0x20
>  #define IVRS_IVMD_SPECIFIED  0x21
>  #define IVRS_IVMD_RANGE  0x22
> @@ -652,6 +670,7 @@ union acpi_ivrs_entry {
>   uint16_tlength;
>   } __packed;
>   struct acpi_ivhdivhd;
> + struct acpi_ivhd_extivhd_ext;
>   struct acpi_ivmdivmd;
>  } __packed;
>  
> 
> 



PATCH: Add ACPI IVHD_EXT structure to acpireg.h

2020-09-15 Thread Jordan Hargrave
This patch adds a couple of entries for AMD IOMMU structure definitions in ACPI

Index: acpireg.h
===
RCS file: /cvs/src/sys/dev/acpi/acpireg.h,v
retrieving revision 1.45
diff -u -p -r1.45 acpireg.h
--- acpireg.h   28 Aug 2019 22:39:09 -  1.45
+++ acpireg.h   15 Sep 2020 06:29:50 -
@@ -623,6 +623,9 @@ struct acpi_ivmd {
 struct acpi_ivhd {
uint8_t type;
uint8_t flags;
+#define IVHD_PPRSUP(1L << 7)
+#define IVHD_PREFSUP   (1L << 6)
+#define IVHD_COHERENT  (1L << 5)
 #define IVHD_IOTLB (1L << 4)
 #define IVHD_ISOC  (1L << 3)
 #define IVHD_RESPASSPW (1L << 2)
@@ -638,13 +641,28 @@ struct acpi_ivhd {
 #define IVHD_UNITID_MASK   0x1F
 #define IVHD_MSINUM_SHIFT  0
 #define IVHD_MSINUM_MASK   0x1F
-   uint32_treserved;
+   uint32_tfeature;
+} __packed;
+
+struct acpi_ivhd_ext {
+   uint8_t type;
+   uint8_t flags;
+   uint16_tlength;
+   uint16_tdevid;
+   uint16_tcap;
+   uint64_taddress;
+   uint16_tsegment;
+   uint16_tinfo;
+   uint32_tattrib;
+   uint64_tefr;
+   uint8_t reserved[8];
 } __packed;
 
 union acpi_ivrs_entry {
struct {
uint8_t type;
 #define IVRS_IVHD  0x10
+#define IVRS_IVHD_EXT  0x11
 #define IVRS_IVMD_ALL  0x20
 #define IVRS_IVMD_SPECIFIED0x21
 #define IVRS_IVMD_RANGE0x22
@@ -652,6 +670,7 @@ union acpi_ivrs_entry {
uint16_tlength;
} __packed;
struct acpi_ivhdivhd;
+   struct acpi_ivhd_extivhd_ext;
struct acpi_ivmdivmd;
 } __packed;