Re: Add -R alias to -r for scp(1)

2020-01-04 Thread Theo de Raadt
>Kurt Mosiejczuk wrote:
>> cp(1) uses -R for recursive copy. scp(1) uses -r. This diff adds -R as an
>> alias for -r to scp(1) for those assuming consistency with cp(1).
>
>But it doesn't implement cp -R semantics. It does the copy the way cp -r does.
>(For symlinks, etc.)

very good point.  the subtle difference says no.



Re: sparc64: autoconf: Ignore reboot-memory device

2020-01-04 Thread Klemens Nanni
On Thu, Dec 26, 2019 at 10:02:42PM +0100, Klemens Nanni wrote:
> Solaris supports booting fallback images from "retained memory" which
> is a relatively new feature introduced, it requires recent versions of
> Solaris as well as recent hardware support;  T2+ machines do not show
> this pseudo-device with latest firmware, T4 machines however do:
> 
>   {0} ok show-devs
>   ...
>   /reboot-memory@0
>   ...
> 
>   "reboot-memory" at mainbus0 not configured  
>   
> 
> Neither NetBSD nor FreeBSD support such machines in the first place, so
> they do not exclude anything newer than we do;  our list comes from
> NetBSD where it stays the same since import in 1998.
> 
> OK to ignore this firmware specific and Solaris only pseudo-device?
> 
> NB: I see "pci-performance-counters" as well but am not certain whether
> this could in fact be used at some point from OpenBSD, so I'm not this
> pseudo-device now.
Ping.


Index: arch/sparc64/sparc64/autoconf.c
===
RCS file: /cvs/src/sys/arch/sparc64/sparc64/autoconf.c,v
retrieving revision 1.134
diff -u -p -r1.134 autoconf.c
--- arch/sparc64/sparc64/autoconf.c 4 Jan 2020 23:43:54 -   1.134
+++ arch/sparc64/sparc64/autoconf.c 4 Jan 2020 23:44:59 -
@@ -920,6 +920,7 @@ extern bus_space_tag_t mainbus_space_tag
 * These are _root_ devices to ignore. Others must be handled
 * elsewhere.
 */
+   "reboot-memory",
"virtual-memory",
"aliases",
"memory",



ldom.conf.5: Add BUGS section for hypervisor memory

2020-01-04 Thread Klemens Nanni
The hypervisor requires memory and allocates it transparently, e.g. on
my T4-2 with 128G in factory-default configuration without guests, the
primary domain boots into OBP with 127.5G while the PRI presents 127.62M
of physical memory available.

Documentation about those machine dependent allocations seems scarse,
I haven't found any so far except for M10/M12 machines of which I don't
even know whether OpenBSD would boot on them.

ldomctl works with what it gets from the PRI and some configurations
(nearly) exhausting the available memory may fail to boot as the
hypervisor rejects the configuration, sadly with all too generic memory
related warnings.

Can we highlight these circumstances under BUGS?  I think it helps to
shed some light on this until we find proper documentation to fix this
or some way to handle this from within ldomctl.

Feedback? OK?


Index: ldom.conf.5
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v
retrieving revision 1.9
diff -u -p -r1.9 ldom.conf.5
--- ldom.conf.5 3 Dec 2019 21:07:03 -   1.9
+++ ldom.conf.5 4 Jan 2020 22:51:52 -
@@ -107,3 +107,18 @@ On a machine with 32 cores and 64GB phys
 .Xr eeprom 8 ,
 .Xr ldomctl 8 ,
 .Xr ldomd 8
+.Sh BUGS
+The hypervisor requires a machine dependent amount of physical memory that is
+reserved automatically.
+Although the Physical Resource Inventory
+.Pq Em PRI
+seems to account for this by presenting less available memory, using the entire
+amount via
+.Ic memory
+is not always successful, e.g. the hypervisor would reject the configuration 
and
+fallback to
+.Dq factory-default
+upon resetting the machine.
+.Pp
+If in doubt, leave enough memory unused for the hypervisor to reserve.
+On bigger T4 based machines, 1024 megabytes has proven to suffice.



Re: ospf(6)d.conf: define interface parameters per area or globally

2020-01-04 Thread Denis Fondras
On Sat, Jan 04, 2020 at 11:11:36PM +0100, Remi Locherer wrote:
> Hi,
> 
> interface-specific parameters can be defined globally or per area.
> But they are applied to the interfaces only if the interfaces are
> declared afterwards.
> 

I have a diff to allow parameters after interface or area definition.
Not sure if we want to do that though.

> Or is the GLOBAL CONFIURATION section the better place for this?
> I opted for the AREA section because I consider it unlikely a user adds
> global parameters at the end of the config file. But who knows. ;-)
> 

In the MACRO section I would change the last sentence too (or even remove it as
it is close to the GLOBAL first paragraph).

Anyway OK denis@

> Remi
> 
> Index: ospfd/ospfd.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
> retrieving revision 1.58
> diff -u -p -r1.58 ospfd.conf.5
> --- ospfd/ospfd.conf.519 Nov 2019 09:55:55 -  1.58
> +++ ospfd/ospfd.conf.54 Jan 2020 21:48:00 -
> @@ -256,11 +256,13 @@ is set to a value other than 1 or if the
>  Areas are used for grouping interfaces.
>  All interface-specific parameters can
>  be configured per area, overruling the global settings.
> +These interface-specific parameters need to be defined before the interfaces.
>  .Bl -tag -width Ds
>  .It Ic area Ar id | address
>  Specify an area section, grouping one or more interfaces.
>  .Bd -literal -offset indent
>  area 0.0.0.0 {
> + hello-interval 3
>   interface em0
>   interface em1 {
>   metric 10
> Index: ospf6d/ospf6d.conf.5
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.conf.5,v
> retrieving revision 1.20
> diff -u -p -r1.20 ospf6d.conf.5
> --- ospf6d/ospf6d.conf.5  26 Dec 2019 10:24:18 -  1.20
> +++ ospf6d/ospf6d.conf.5  4 Jan 2020 21:48:30 -
> @@ -236,11 +236,13 @@ is set to a value different to 1 or if t
>  Areas are used for grouping interfaces.
>  All interface-specific parameters can
>  be configured per area, overruling the global settings.
> +These interface-specific parameters need to be defined before the interfaces.
>  .Bl -tag -width Ds
>  .It Ic area Ar address Ns | Ns Ar id
>  Specify an area section, grouping one or more interfaces.
>  .Bd -literal -offset indent
>  area 0.0.0.0 {
> + hello-interval 3
>   interface em0
>   interface em1 {
>   metric 10
> 



ospf(6)d.conf: define interface parameters per area or globally

2020-01-04 Thread Remi Locherer
Hi,

interface-specific parameters can be defined globally or per area.
But they are applied to the interfaces only if the interfaces are
declared afterwards.

Or is the GLOBAL CONFIURATION section the better place for this?
I opted for the AREA section because I consider it unlikely a user adds
global parameters at the end of the config file. But who knows. ;-)

Remi

Index: ospfd/ospfd.conf.5
===
RCS file: /cvs/src/usr.sbin/ospfd/ospfd.conf.5,v
retrieving revision 1.58
diff -u -p -r1.58 ospfd.conf.5
--- ospfd/ospfd.conf.5  19 Nov 2019 09:55:55 -  1.58
+++ ospfd/ospfd.conf.5  4 Jan 2020 21:48:00 -
@@ -256,11 +256,13 @@ is set to a value other than 1 or if the
 Areas are used for grouping interfaces.
 All interface-specific parameters can
 be configured per area, overruling the global settings.
+These interface-specific parameters need to be defined before the interfaces.
 .Bl -tag -width Ds
 .It Ic area Ar id | address
 Specify an area section, grouping one or more interfaces.
 .Bd -literal -offset indent
 area 0.0.0.0 {
+   hello-interval 3
interface em0
interface em1 {
metric 10
Index: ospf6d/ospf6d.conf.5
===
RCS file: /cvs/src/usr.sbin/ospf6d/ospf6d.conf.5,v
retrieving revision 1.20
diff -u -p -r1.20 ospf6d.conf.5
--- ospf6d/ospf6d.conf.526 Dec 2019 10:24:18 -  1.20
+++ ospf6d/ospf6d.conf.54 Jan 2020 21:48:30 -
@@ -236,11 +236,13 @@ is set to a value different to 1 or if t
 Areas are used for grouping interfaces.
 All interface-specific parameters can
 be configured per area, overruling the global settings.
+These interface-specific parameters need to be defined before the interfaces.
 .Bl -tag -width Ds
 .It Ic area Ar address Ns | Ns Ar id
 Specify an area section, grouping one or more interfaces.
 .Bd -literal -offset indent
 area 0.0.0.0 {
+   hello-interval 3
interface em0
interface em1 {
metric 10



dangling vnode panic

2020-01-04 Thread Alexander Bluhm
Hi,

On my amd64 test machine regress/sys/kern/mount run-regress-unmount-busy
triggers the dangling vnode panic regulary.  Something has changed
in the previous months that makes it more likely.

Problem is that while flushing the mnt_vnodelist list, the unmount
process sleeps.  Then active processes can write new dirty vnodes
to the list.  This happens without softdep.

My first attempt was to disable insertions while unmounting.  That
does not work.  A buffer flush can cause softdep to add new dirty
vnodes to the list.

This diff adds the dirty vnodes to the end of the list.  vflush()
trverses it from the begining.  Then new vnodes, that are added
during syncing, will be flushed.  This fixes my test.

Please test with and without softdep.

ok?

bluhm

Index: sys/kern/vfs_subr.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_subr.c,v
retrieving revision 1.297
diff -u -p -r1.297 vfs_subr.c
--- sys/kern/vfs_subr.c 30 Dec 2019 13:49:40 -  1.297
+++ sys/kern/vfs_subr.c 4 Jan 2020 18:42:25 -
@@ -178,7 +178,7 @@ vfs_mount_alloc(struct vnode *vp, struct
rw_init_flags(&mp->mnt_lock, "vfslock", RWL_IS_VNODE);
(void)vfs_busy(mp, VB_READ|VB_NOWAIT);

-   LIST_INIT(&mp->mnt_vnodelist);
+   TAILQ_INIT(&mp->mnt_vnodelist);
mp->mnt_vnodecovered = vp;

atomic_inc_int(&vfsp->vfc_refcount);
@@ -476,12 +476,12 @@ insmntque(struct vnode *vp, struct mount
 * Delete from old mount point vnode list, if on one.
 */
if (vp->v_mount != NULL)
-   LIST_REMOVE(vp, v_mntvnodes);
+   TAILQ_REMOVE(&vp->v_mount->mnt_vnodelist, vp, v_mntvnodes);
/*
 * Insert into list of vnodes for the new mount point, if available.
 */
if ((vp->v_mount = mp) != NULL)
-   LIST_INSERT_HEAD(&mp->mnt_vnodelist, vp, v_mntvnodes);
+   TAILQ_INSERT_TAIL(&mp->mnt_vnodelist, vp, v_mntvnodes);
 }

 /*
@@ -872,7 +872,7 @@ vfs_mount_foreach_vnode(struct mount *mp
int error = 0;

 loop:
-   LIST_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
+   TAILQ_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
if (vp->v_mount != mp)
goto loop;

@@ -1299,7 +1299,7 @@ printlockedvnodes(void)
TAILQ_FOREACH(mp, &mountlist, mnt_list) {
if (vfs_busy(mp, VB_READ|VB_NOWAIT))
continue;
-   LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
+   TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
if (VOP_ISLOCKED(vp))
vprint(NULL, vp);
}
@@ -2287,7 +2287,7 @@ vfs_mount_print(struct mount *mp, int fu
(*pr)("locked vnodes:");
/* XXX would take mountlist lock, except ddb has no context */
cnt = 0;
-   LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
+   TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
if (VOP_ISLOCKED(vp)) {
if (cnt == 0)
(*pr)("\n  %p", vp);
@@ -2304,7 +2304,7 @@ vfs_mount_print(struct mount *mp, int fu
(*pr)("all vnodes:");
/* XXX would take mountlist lock, except ddb has no context */
cnt = 0;
-   LIST_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
+   TAILQ_FOREACH(vp, &mp->mnt_vnodelist, v_mntvnodes) {
if (cnt == 0)
(*pr)("\n  %p", vp);
else if ((cnt % (72 / (sizeof(void *) * 2 + 4))) == 0)
Index: sys/kern/vfs_syscalls.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/vfs_syscalls.c,v
retrieving revision 1.338
diff -u -p -r1.338 vfs_syscalls.c
--- sys/kern/vfs_syscalls.c 29 Nov 2019 20:58:17 -  1.338
+++ sys/kern/vfs_syscalls.c 4 Jan 2020 18:41:20 -
@@ -489,7 +489,7 @@ dounmount_leaf(struct mount *mp, int fla
 * Before calling file system unmount, make sure
 * all unveils to vnodes in here are dropped.
 */
-   LIST_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
+   TAILQ_FOREACH_SAFE(vp , &mp->mnt_vnodelist, v_mntvnodes, nvp) {
unveil_removevnode(vp);
}

@@ -511,7 +511,7 @@ dounmount_leaf(struct mount *mp, int fla
vrele(coveredvp);
}

-   if (!LIST_EMPTY(&mp->mnt_vnodelist))
+   if (!TAILQ_EMPTY(&mp->mnt_vnodelist))
panic("unmount: dangling vnode");

vfs_unbusy(mp);
Index: sys/nfs/nfs_subs.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/nfs/nfs_subs.c,v
retrieving revision 1.140
diff -u -p -r1.140 nfs_subs.c
--- sys/nfs/nfs_subs.c  25 Dec 2019 12:31:12 -  1.140
+++ sys/nfs/nfs_subs.c  4 Jan 

ldom.conf.5: Use stride in EXAMPLES, elaborate on hardware threads

2020-01-04 Thread Klemens Nanni
CPU strides provide means to effectively bind guests to certain specific
phsyical cores by overallocating virtual CPUs (hardware threads) such
that the sum of virtual CPUs and strides yields an integer multiple of
the CPU dependent threads-per-core factor, e.g. on T2 based machines
each CPU has 8 cores and each core has 8 threads. this makes for n times
virtual CPUs to be managed in ldom.conf(5).

Diff below slightly extends the given example and tries to further
clarify what strides do.  The example should match a T5120 machine for
example which features 1 T2+ CPU.

It hopefully makes it clearer how "vcpu"s correspond to hardware threads
not physical CPU cores.

Feedback? OK?


Index: ldom.conf.5
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldom.conf.5,v
retrieving revision 1.9
diff -u -p -r1.9 ldom.conf.5
--- ldom.conf.5 3 Dec 2019 21:07:03 -   1.9
+++ ldom.conf.5 4 Jan 2020 20:17:38 -
@@ -78,11 +78,11 @@ Configure the MTU of the interface.
 .El
 .El
 .Sh EXAMPLES
-Define a domain with 12 virtual cores, 4GB memory, two file based virtual disks
-and one virtual network interface:
+Define a domain with 12 virtual cores and 4 additionally allocated cores,
+4GB memory, two file based virtual disks and one virtual network interface:
 .Bd -literal -offset indent
 domain "puffy" {
-   vcpu 12
+   vcpu 12:4
memory 4G
vdisk "/home/puffy/vdisk0"
vdisk "/home/puffy/vdisk1"
@@ -101,8 +101,10 @@ domain "salmah" {
 }
 .Ed
 .Pp
-On a machine with 32 cores and 64GB physical memory, this leaves 12 cores and
-58GB memory to the primary domain.
+On a machine with 1 CPU comprising 8 cores with 8 threads each and 64GB 
physical
+memory, this leaves 40 cores and 58GB memory to the primary domain.
+.Dq puffy
+will be assigned 16 cores of which 4 are not being used.
 .Sh SEE ALSO
 .Xr eeprom 8 ,
 .Xr ldomctl 8 ,



Re: ldomctl: download: select new configuration

2020-01-04 Thread Klemens Nanni
On Sat, Jan 04, 2020 at 08:16:50PM +0100, Mark Kettenis wrote:
> Probably should say that depending on the firmware the new
> configuration will have to be explicitly selected?
I wanted to explicitly document behaviour specific to certain firmware,
the `select' command already explains how to pick a config and `list'
already tells how to check all configs.

This way seems like less repitition and goes in line with how you prefer
`download' should not select configs automatically, e.g. stating how
some firmware still does it implies that it is not the default action we
actively take in ldomctl.



Re: dwiic(4) improvements

2020-01-04 Thread Mark Kettenis
> Date: Sun, 22 Dec 2019 16:55:59 +0100 (CET)
> From: Mark Kettenis 
> 
> The diff below contains a couple of improvements to dwiic(4).  They're
> mostly for making ipmi(4) on the Ampere/Lenovo arm64 boxes work
> better.  But they need testing on x86 machines with
> keyboards.touchpads/touchscreens connected over i2c.
> 
> Most of the diff is there to properly implement SMBus block write/read
> transactions.  Defines for these are taken from NetBSD (even though
> NetBSD doesn't actually implement these).  Nothing special needs to
> been done for block writes, but for block reads the slave device sends
> us byte count before sending us the data bytes.  So the code is
> changed such that we read the byte count first and then adjust the
> length of the transaction accordingly.
> 
> The diff then uses this support for block reads in the ipmi(4) SSIF
> interface code and adds support for multi-part read transactions on
> top of that.  That makes ipmi(4) fully functional on the arm64 machine
> mentioned above.
> 
> Well, almost.  I found that dwiic(4) still craps out every now and
> then on this machine.  Limiting the number of commands we send to the
> controller to the size of the Tx FIFO minus one seems to help.
> 
> Will probably split up the diff before committing.
> 
> ok?

ping?  tested this myself on x86 hardware with i2c sensors connected
to dwiic(4).

> Index: dev/i2c/i2c_io.h
> ===
> RCS file: /cvs/src/sys/dev/i2c/i2c_io.h,v
> retrieving revision 1.1
> diff -u -p -r1.1 i2c_io.h
> --- dev/i2c/i2c_io.h  23 May 2004 17:33:43 -  1.1
> +++ dev/i2c/i2c_io.h  22 Dec 2019 15:40:28 -
> @@ -1,5 +1,5 @@
>  /*   $OpenBSD: i2c_io.h,v 1.1 2004/05/23 17:33:43 grange Exp $   */
> -/*   $NetBSD: i2c_io.h,v 1.1 2003/09/30 00:35:31 thorpej Exp $   */
> +/*   $NetBSD: i2c_io.h,v 1.3 2012/04/22 14:10:36 pgoyette Exp $  */
>  
>  /*
>   * Copyright (c) 2003 Wasabi Systems, Inc.
> @@ -45,16 +45,24 @@
>  typedef uint16_t i2c_addr_t;
>  
>  /* High-level I2C operations. */
> +#define  I2C_OPMASK_STOP 1
> +#define  I2C_OPMASK_WRITE2
> +#define  I2C_OPMASK_BLKMODE  4
> +
> +#define  I2C_OP_STOP_P(x)(((int)(x) & I2C_OPMASK_STOP) != 0)
> +#define  I2C_OP_WRITE_P(x)   (((int)(x) & I2C_OPMASK_WRITE) != 0)
> +#define  I2C_OP_READ_P(x)(!I2C_OP_WRITE_P(x))
> +#define  I2C_OP_BLKMODE_P(x) (((int)(x) & I2C_OPMASK_BLKMODE) != 0)
> +
>  typedef enum {
> - I2C_OP_READ = 0,
> - I2C_OP_READ_WITH_STOP   = 1,
> - I2C_OP_WRITE= 2,
> - I2C_OP_WRITE_WITH_STOP  = 3,
> +I2C_OP_READ  = 0,
> +I2C_OP_READ_WITH_STOP= I2C_OPMASK_STOP,
> +I2C_OP_WRITE = I2C_OPMASK_WRITE,
> +I2C_OP_WRITE_WITH_STOP   = I2C_OPMASK_WRITE   | I2C_OPMASK_STOP,
> +I2C_OP_READ_BLOCK= I2C_OPMASK_BLKMODE | I2C_OPMASK_STOP,
> +I2C_OP_WRITE_BLOCK   = I2C_OPMASK_BLKMODE | I2C_OPMASK_WRITE |
> + I2C_OPMASK_STOP,
>  } i2c_op_t;
> -
> -#define  I2C_OP_READ_P(x)(((int)(x) & 2) == 0)
> -#define  I2C_OP_WRITE_P(x)   (! I2C_OP_READ_P(x))
> -#define  I2C_OP_STOP_P(x)(((int)(x) & 1) != 0)
>  
>  /*
>   * This structure describes a single I2C control script fragment.
> Index: dev/i2c/ipmi_i2c.c
> ===
> RCS file: /cvs/src/sys/dev/i2c/ipmi_i2c.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 ipmi_i2c.c
> --- dev/i2c/ipmi_i2c.c3 Sep 2019 09:17:10 -   1.2
> +++ dev/i2c/ipmi_i2c.c22 Dec 2019 15:40:28 -
> @@ -170,7 +170,7 @@ ssif_sendmsg(struct ipmi_cmd *c)
>   cmd[0] = 2;
>   cmd[1] = c->c_txlen;
>   for (retry = 0; retry < 5; retry++) {
> - error = iic_exec(sc->sc_tag, I2C_OP_WRITE_WITH_STOP,
> + error = iic_exec(sc->sc_tag, I2C_OP_WRITE_BLOCK,
>   sc->sc_addr, cmd, sizeof(cmd), buf, c->c_txlen, 0);
>   if (!error)
>   break;
> @@ -185,28 +185,72 @@ int
>  ssif_recvmsg(struct ipmi_cmd *c)
>  {
>   struct ipmi_i2c_softc *sc = (struct ipmi_i2c_softc *)c->c_sc;
> - uint8_t buf[IPMI_MAX_RX + 16];
> + uint8_t buf[33];
>   uint8_t cmd[1];
> + uint8_t len;
>   int error, retry;
> -
> - /* We only support single-part reads. */
> - if (c->c_maxrxlen > 32)
> - return -1;
> + int blkno = 0;
>  
>   iic_acquire_bus(sc->sc_tag, 0);
>  
>   cmd[0] = 3;
>   for (retry = 0; retry < 250; retry++) {
> - memset(buf, 0, c->c_maxrxlen + 1);
> - error = iic_exec(sc->sc_tag, I2C_OP_READ_WITH_STOP,
> - sc->sc_addr, cmd, sizeof(cmd), buf, c->c_maxrxlen + 1, 0);
> - if (!error) {
> - c->c_rxlen = buf[0];
> - memcpy(sc->sc.sc_buf, &buf[1], c->c_maxrxlen);
> +  

Re: ipmi@acpi improvement

2020-01-04 Thread Mark Kettenis
> Date: Sat, 21 Dec 2019 12:30:50 +0100 (CET)
> From: Mark Kettenis 
> 
> On arm64 systems, IPMI can actually attach using mmio.  The diff below
> implements this.  It also sets the IPMI revision based on the _SRV
> method if present.
> 
> I can't actually test this properly.  The firmware I'm using on my
> od1000 has the IPMI device in its ACPI tables.  But I'm 99% certain
> there isn't an actual BMC in the machine.  There should be on in the
> od3000 though.
> 
> Anyway, with my diff I get:
> 
>   ipmi0 at acpi0: version 2.0 interface KCS membase 0xe001/8 spacing 4
> 
> instead of:
> 
>   ipmi0 at acpi0ipmi0: Unexpected resource #0 type 134
>   ipmi0: Unexpected resource #1 type 134
>   ipmi0: invalid resource #2 type 137 (ift 1)
>   ipmi0: incomplete resources (ift 1)
> 
> which looks correct, but then I get:
> 
>   ipmi0: sendcmd fails
>   ipmi0: get header fails
>   ipmi0: no SDRs IPMI disabled
> 
> Which is expected if there is no BMC.
> 
> Would be good to have this tested on an x86 where IPMI attaches through ACPI.
> 
> ok?

ping?  tested sucessfully by sthen@ on x86 hardware

> Index: dev/acpi/ipmi_acpi.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/ipmi_acpi.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 ipmi_acpi.c
> --- dev/acpi/ipmi_acpi.c  13 Aug 2019 18:31:23 -  1.2
> +++ dev/acpi/ipmi_acpi.c  21 Dec 2019 11:30:22 -
> @@ -46,8 +46,8 @@ struct ipmi_acpi_softc {
>   struct aml_node *sc_devnode;
>  
>   int  sc_ift;
> + int  sc_srv;
>  
> - bus_space_tag_t  sc_iot;
>   bus_size_t   sc_iobase;
>   int  sc_iospacing;
>   char sc_iotype;
> @@ -76,7 +76,7 @@ ipmi_acpi_attach(struct device *parent, 
>   struct acpi_attach_args *aa = aux;
>   struct ipmi_attach_args ia;
>   struct aml_value res;
> - int64_t ift;
> + int64_t ift, srv = 0;
>   int rc;
>  
>   sc->sc_acpi = (struct acpi_softc *)parent;
> @@ -89,6 +89,9 @@ ipmi_acpi_attach(struct device *parent, 
>   }
>   sc->sc_ift = ift;
>  
> + aml_evalinteger(sc->sc_acpi, sc->sc_devnode, "_SRV", 0, NULL, &srv);
> + sc->sc_srv = srv;
> +
>   if (aml_evalname(sc->sc_acpi, sc->sc_devnode, "_CRS", 0, NULL, &res)) {
>   printf(": no _CRS method\n");
>   return;
> @@ -103,17 +106,17 @@ ipmi_acpi_attach(struct device *parent, 
>   aml_parse_resource(&res, ipmi_acpi_parse_crs, sc);
>   aml_freevalue(&res);
>  
> - if (sc->sc_iot == NULL) {
> + if (sc->sc_iotype == 0) {
>   printf("%s: incomplete resources (ift %d)\n",
>   DEVNAME(sc), sc->sc_ift);
>   return;
>   }
>  
>   memset(&ia, 0, sizeof(ia));
> - ia.iaa_iot = sc->sc_iot;
> - ia.iaa_memt = sc->sc_iot;
> + ia.iaa_iot = sc->sc_acpi->sc_iot;
> + ia.iaa_memt = sc->sc_acpi->sc_memt;
>   ia.iaa_if_type = sc->sc_ift;
> - ia.iaa_if_rev = 0;
> + ia.iaa_if_rev = (sc->sc_srv >> 4);
>   ia.iaa_if_irq = -1;
>   ia.iaa_if_irqlvl = 0;
>   ia.iaa_if_iospacing = sc->sc_iospacing;
> @@ -128,32 +131,52 @@ ipmi_acpi_parse_crs(int crsidx, union ac
>  {
>   struct ipmi_acpi_softc *sc = arg;
>   int type = AML_CRSTYPE(crs);
> + bus_size_t addr;
> + char iotype;
> +
> + switch (type) {
> + case SR_IOPORT:
> + addr = crs->sr_ioport._max;
> + iotype = 'i';
> + break;
> + case LR_MEM32FIXED:
> + addr = crs->lr_m32fixed._bas;
> + iotype = 'm';
> + break;
> + case LR_EXTIRQ:
> + /* Ignore for now. */
> + return 0;
> + default:
> + printf("\n%s: unexpected resource #%d type %d",
> + DEVNAME(sc), crsidx, type);
> + sc->sc_iotype = 0;
> + return -1;
> + }
>  
>   switch (crsidx) {
>   case 0:
> - if (type != SR_IOPORT) {
> - printf("%s: Unexpected resource #%d type %d\n",
> - DEVNAME(sc), crsidx, type);
> - break;
> - }
> - sc->sc_iot = sc->sc_acpi->sc_iot;
> - sc->sc_iobase = crs->sr_ioport._max;
> + sc->sc_iobase = addr;
>   sc->sc_iospacing = 1;
> - sc->sc_iotype = 'i';
> + sc->sc_iotype = iotype;
>   break;
>   case 1:
> - if (type != SR_IOPORT) {
> - printf("%s: Unexpected resource #%d type %d\n",
> + if (sc->sc_iotype != iotype) {
> + printf("\n%s: unexpected resource #%d type %d\n",
>   DEVNAME(sc), crsidx, type);
> - break;
> + sc->sc_iotype = 0;
> + return -1;
> + }
> + if (addr <= sc->sc_iobase) {
>

Re: ldomctl: download: select new configuration

2020-01-04 Thread Mark Kettenis
> Date: Sat, 4 Jan 2020 20:01:59 +0100
> From: Klemens Nanni 
> 
> On Fri, Jan 03, 2020 at 08:40:56PM +0100, Klemens Nanni wrote:
> > Manual update below for convenience but I want to hear feedback from
> > users of above mentioned machines before changing this either way.
> kettenis confirmed that at least a T5120 automatically selects the new
> config, I'm pretty confident my T5240 did the same.
> 
> Here's a minimal diff to reflect this machine dependent behaviour.
> 
> Feedback? OK?

Probably should say that depending on the firmware the new
configuration will have to be explicitly selected?

> Index: ldomctl.8
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.23
> diff -u -p -r1.23 ldomctl.8
> --- ldomctl.8 4 Jan 2020 18:45:26 -   1.23
> +++ ldomctl.8 4 Jan 2020 18:58:22 -
> @@ -53,7 +53,8 @@ Delete the specified configuration from 
>  .It Cm download Ar directory
>  Save a logical domain configuration to non-volatile storage on the
>  service processor.
> -The configuration will take effect after the machine is reset.
> +Depending on the firmware, the configuration will be selected automatically 
> and
> +take effect after the machine is reset.
>  The name of the configuration is taken from the name of the
>  .Ar directory
>  which must contain files created with the
> 



Re: ldomctl: download: select new configuration

2020-01-04 Thread Klemens Nanni
On Fri, Jan 03, 2020 at 08:40:56PM +0100, Klemens Nanni wrote:
> Manual update below for convenience but I want to hear feedback from
> users of above mentioned machines before changing this either way.
kettenis confirmed that at least a T5120 automatically selects the new
config, I'm pretty confident my T5240 did the same.

Here's a minimal diff to reflect this machine dependent behaviour.

Feedback? OK?


Index: ldomctl.8
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
retrieving revision 1.23
diff -u -p -r1.23 ldomctl.8
--- ldomctl.8   4 Jan 2020 18:45:26 -   1.23
+++ ldomctl.8   4 Jan 2020 18:58:22 -
@@ -53,7 +53,8 @@ Delete the specified configuration from 
 .It Cm download Ar directory
 Save a logical domain configuration to non-volatile storage on the
 service processor.
-The configuration will take effect after the machine is reset.
+Depending on the firmware, the configuration will be selected automatically and
+take effect after the machine is reset.
 The name of the configuration is taken from the name of the
 .Ar directory
 which must contain files created with the



Re: sparc64: simplify boot()

2020-01-04 Thread Mark Kettenis
> Date: Fri, 3 Jan 2020 19:26:22 +
> From: Miod Vallat 
> 
> Although Open Firmware supports it, there is no way from OpenBSD to
> reboot with a specified boot command line, so drop vestigial support for
> it from boot().

Hmm, reboot(2) does allow us to pass flags.  But you're right that
there is no userland tool to pass the flags to the kernel.

I suppose the dominant platforms simply didn't support passing flags
across reboots, so this never took off.  But with EFI we could support
these on amd64 and arm64.


> Index: sparc64/machdep.c
> ===
> RCS file: /OpenBSD/src/sys/arch/sparc64/sparc64/machdep.c,v
> retrieving revision 1.191
> diff -u -p -r1.191 machdep.c
> --- sparc64/machdep.c 1 Apr 2019 07:00:52 -   1.191
> +++ sparc64/machdep.c 3 Jan 2020 19:24:45 -
> @@ -593,9 +593,6 @@ struct pcb dumppcb;
>  __dead void
>  boot(int howto)
>  {
> - int i;
> - static char str[128];
> -
>   if ((howto & RB_RESET) != 0)
>   goto doreset;
>  
> @@ -655,30 +652,7 @@ haltsys:
>  
>  doreset:
>   printf("rebooting\n\n");
> -#if 0
> - if (user_boot_string && *user_boot_string) {
> - i = strlen(user_boot_string);
> - if (i > sizeof(str))
> - OF_boot(user_boot_string);  /* XXX */
> - bcopy(user_boot_string, str, i);
> - } else
> -#endif
> - {
> - i = 1;
> - str[0] = '\0';
> - }
> -
> - if ((howto & RB_SINGLE) != 0)
> - str[i++] = 's';
> - if ((howto & RB_KDB) != 0)
> - str[i++] = 'd';
> - if (i > 1) {
> - if (str[0] == '\0')
> - str[0] = '-';
> - str[i] = 0;
> - } else
> - str[0] = 0;
> - OF_boot(str);
> + OF_boot("");
>   panic("cpu_reboot -- failed");
>   for (;;)
>   continue;
> 
> 



Re: ldomctl: init-system: Add -n (noaction) switch for validation only

2020-01-04 Thread Klemens Nanni
On Sat, Jan 04, 2020 at 12:35:46PM -0600, Matthew Martin wrote:
> For what it's worth most daemons currently print "configuration OK":
> bgpd dvmrpd eigrpd httpd ifstated iked ldpd npppd ntpd ospf6d ospfd rad
> radiusd relayd ripd sasyncd smtpd switchd unwind vmd.
Yup, hence my addition.

> ldapd and snmpd print "configuration ok". dhcpd, doas, isakmpd, and
> nsd-checkconf are slient. unbound-checkconf uses the format string
> "unbound-checkconf: no errors in %s\n".
But as there are more inconsistencies as you noted and this is mark's
code afterall, I did not want to bike shed.

Personally, I'd appreciate seeing the same behaviour across all tools,
but that's a yak I don't want to shave (now).



Re: ldomctl: init-system: Add -n (noaction) switch for validation only

2020-01-04 Thread Matthew Martin
On Sat, Jan 04, 2020 at 04:08:47PM +0100, Mark Kettenis wrote:
> I don't think this should print "configuartion OK" when there are no
> errors.  The UNIX way is to just return 0 and be done.

For what it's worth most daemons currently print "configuration OK":
bgpd dvmrpd eigrpd httpd ifstated iked ldpd npppd ntpd ospf6d ospfd rad
radiusd relayd ripd sasyncd smtpd switchd unwind vmd.

ldapd and snmpd print "configuration ok". dhcpd, doas, isakmpd, and
nsd-checkconf are slient. unbound-checkconf uses the format string
"unbound-checkconf: no errors in %s\n".



Re: uthum(4) diff to test

2020-01-04 Thread Martin Pieuchot
On 04/01/20(Sat) 16:30, Mark Kettenis wrote:
> > Date: Sat, 4 Jan 2020 16:15:22 +0100
> > From: Martin Pieuchot 
> > 
> > `delay' is expressed in ms, however since tsleep_nsec(9) doesn't add a
> > tick the conversions below might result in shorter sleeps.  I'd
> > appreciate if uthum(4) owners could test this diff and report back.
> 
> I'm not sure I understand what you're saying here.
> 
> But this made me look at the implementation of tsleep_nsec(), and I
> think its implementation is wrong.

I agree, that's what I'm saying in this thread:
https://marc.info/?l=openbsd-tech&m=157804662128799&w=2



Re: uthum(4) diff to test

2020-01-04 Thread Mark Kettenis
> Date: Sat, 4 Jan 2020 16:15:22 +0100
> From: Martin Pieuchot 
> 
> `delay' is expressed in ms, however since tsleep_nsec(9) doesn't add a
> tick the conversions below might result in shorter sleeps.  I'd
> appreciate if uthum(4) owners could test this diff and report back.

I'm not sure I understand what you're saying here.

But this made me look at the implementation of tsleep_nsec(), and I
think its implementation is wrong.  We should sleep for *at least* the
number of nanoseconds specified.  Therefore the number of ticks should
be rounded up.  That is, if nsecs is 9 and hz is 10, we should
sleep for at least 10 full ticks.

> Index: uthum.c
> ===
> RCS file: /cvs/src/sys/dev/usb/uthum.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 uthum.c
> --- uthum.c   9 Jan 2017 14:44:28 -   1.32
> +++ uthum.c   1 Jan 2020 14:27:59 -
> @@ -311,7 +311,8 @@ uthum_issue_cmd(struct uthum_softc *sc, 
>  
>   /* wait if required */
>   if (delay > 0)
> - tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1);
> + tsleep_nsec(&sc->sc_sensortask, 0, "uthum",
> + MSEC_TO_NSEC(delay));
>  
>   return 0;
>  }
> @@ -337,7 +338,8 @@ uthum_read_data(struct uthum_softc *sc, 
>  
>   /* wait if required */
>   if (delay > 0)
> - tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1);
> + tsleep_nsec(&sc->sc_sensortask, 0, "uthum",
> + MSEC_TO_NSEC(delay));
>  
>   /* get answer */
>   if (uhidev_get_report(sc->sc_hdev.sc_parent, UHID_FEATURE_REPORT,
> 
> 



uthum(4) diff to test

2020-01-04 Thread Martin Pieuchot
`delay' is expressed in ms, however since tsleep_nsec(9) doesn't add a
tick the conversions below might result in shorter sleeps.  I'd
appreciate if uthum(4) owners could test this diff and report back.

Index: uthum.c
===
RCS file: /cvs/src/sys/dev/usb/uthum.c,v
retrieving revision 1.32
diff -u -p -r1.32 uthum.c
--- uthum.c 9 Jan 2017 14:44:28 -   1.32
+++ uthum.c 1 Jan 2020 14:27:59 -
@@ -311,7 +311,8 @@ uthum_issue_cmd(struct uthum_softc *sc, 
 
/* wait if required */
if (delay > 0)
-   tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1);
+   tsleep_nsec(&sc->sc_sensortask, 0, "uthum",
+   MSEC_TO_NSEC(delay));
 
return 0;
 }
@@ -337,7 +338,8 @@ uthum_read_data(struct uthum_softc *sc, 
 
/* wait if required */
if (delay > 0)
-   tsleep(&sc->sc_sensortask, 0, "uthum", (delay*hz+999)/1000 + 1);
+   tsleep_nsec(&sc->sc_sensortask, 0, "uthum",
+   MSEC_TO_NSEC(delay));
 
/* get answer */
if (uhidev_get_report(sc->sc_hdev.sc_parent, UHID_FEATURE_REPORT,



Re: ldomctl: init-system: Add -n (noaction) switch for validation only

2020-01-04 Thread Mark Kettenis
> Date: Fri, 3 Jan 2020 21:46:29 +0100
> From: Klemens Nanni 
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> With the hv_config() now in, `ldomctl init-system -n ldom.conf' to only
> parse configuration is trivial.
> 
> It is usable as unprivileged user, no devices are touched.
> 
> If errors occur, errors will be generated and ldomctl exits;  if all is
> valid, this prints "configuration OK" just like vmd(8) does.
> 
> I have plans for additional ldom.conf(5) options and -n greatly aids
> development, but I also prefer to (double) check configs before loading
> them as root in general.
> 
> Feedback? OK?

I don't think this should print "configuartion OK" when there are no
errors.  The UNIX way is to just return 0 and be done.

Otherwise ok kettenis@

> Index: config.c
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v
> retrieving revision 1.29
> diff -u -p -r1.29 config.c
> --- config.c  28 Nov 2019 18:40:42 -  1.29
> +++ config.c  3 Jan 2020 20:45:33 -
> @@ -2759,7 +2759,7 @@ primary_init(void)
>  }
>  
>  void
> -build_config(const char *filename)
> +build_config(const char *filename, int noaction)
>  {
>   struct guest *primary;
>   struct guest *guest;
> @@ -2781,6 +2781,10 @@ build_config(const char *filename)
>   SIMPLEQ_INIT(&conf.domain_list);
>   if (parse_config(filename, &conf) < 0)
>   exit(1);
> + if (noaction) {
> + fprintf(stderr, "configuration OK\n");
> + exit(0);
> + }
>  
>   pri = md_read("pri");
>   if (pri == NULL)
> Index: ldomctl.8
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.21
> diff -u -p -r1.21 ldomctl.8
> --- ldomctl.8 30 Dec 2019 20:10:48 -  1.21
> +++ ldomctl.8 3 Jan 2020 20:45:33 -
> @@ -63,12 +63,17 @@ The download is aborted if a configurati
>  .It Cm dump
>  Dump the current configuration from non-volatile storage into the current
>  working directory.
> -.It Cm init-system Ar file
> +.It Cm init-system Oo Fl n Oc Ar file
>  Generate files in the current working directory for a logical domain
>  configuration
>  .Ar file
>  as described in
>  .Xr ldom.conf 5 .
> +.Bl -tag -width Fl
> +.It Fl n
> +Configtest mode.
> +Only check the configuration file for validty.
> +.El
>  .It Cm list
>  List configurations stored in non-volatile storage.
>  Indicate the currently running configuration,
> Index: ldomctl.c
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 ldomctl.c
> --- ldomctl.c 3 Jan 2020 19:45:51 -   1.32
> +++ ldomctl.c 3 Jan 2020 20:45:34 -
> @@ -129,7 +129,7 @@ usage(void)
>   fprintf(stderr, "usage:\t%1$s delete|select configuration\n"
>   "\t%1$s download directory\n"
>   "\t%1$s dump|list|list-io\n"
> - "\t%1$s init-system file\n"
> + "\t%1$s init-system [-n] file\n"
>   "\t%1$s create-vdisk -s size file\n"
>   "\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
>   exit(EXIT_FAILURE);
> @@ -241,12 +241,27 @@ dump(int argc, char **argv)
>  void
>  init_system(int argc, char **argv)
>  {
> - if (argc != 2)
> + int ch, noaction = 0;
> +
> + while ((ch = getopt(argc, argv, "n")) != -1) {
> + switch (ch) {
> + case 'n':
> + noaction = 1;
> + break;
> + default:
> + usage();
> + }
> + }
> + argc -= optind;
> + argv += optind;
> +
> + if (argc != 1)
>   usage();
>  
> - hv_config();
> + if (!noaction)
> + hv_config();
>  
> - build_config(argv[1]);
> + build_config(argv[0], noaction);
>  }
>  
>  void
> Index: ldomctl.h
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.h,v
> retrieving revision 1.11
> diff -u -p -r1.11 ldomctl.h
> --- ldomctl.h 28 Nov 2019 18:03:33 -  1.11
> +++ ldomctl.h 3 Jan 2020 20:45:34 -
> @@ -193,5 +193,5 @@ struct ldom_config {
>  };
>  
>  int parse_config(const char *, struct ldom_config *);
> -void build_config(const char *);
> +void build_config(const char *, int);
>  void list_components(void);
> 
> 



MAKE: remove redundant test

2020-01-04 Thread Marc Espie
The only way errorJobs ever gets filled is by engine.c
around line 680 with a snippet that reads


if (!keepgoing) {
if (!silent)
printf("\n");
job->next = errorJobs;
errorJobs = job;
/* XXX don't free the command */
return;
}


Index: job.c
===
RCS file: /cvs/src/usr.bin/make/job.c,v
retrieving revision 1.144
diff -u -p -r1.144 job.c
--- job.c   31 Dec 2019 13:59:14 -  1.144
+++ job.c   4 Jan 2020 12:14:50 -
@@ -546,8 +547,7 @@ postprocess_job(Job *job)
} else if (job->exit_type != JOB_EXIT_OKAY && keepgoing)
free(job);
 
-   if (errorJobs != NULL && !keepgoing &&
-   aborting != ABORT_INTERRUPT)
+   if (errorJobs != NULL && aborting != ABORT_INTERRUPT)
aborting = ABORT_ERROR;
 
if (aborting == ABORT_ERROR && DEBUG(QUICKDEATH))