Update sysctl_int.9 for updated sysctl_int_arr

2020-08-01 Thread Greg Steuck
Hi Jason,

Does this look OK to you?

diff --git a/share/man/man9/sysctl_int.9 b/share/man/man9/sysctl_int.9
index bc335aedd9c..e6b37391a0a 100644
--- a/share/man/man9/sysctl_int.9
+++ b/share/man/man9/sysctl_int.9
@@ -36,7 +36,7 @@
 .Ft int
 .Fn sysctl_int "void *oldp" "size_t *oldlenp" "void *newp" "size_t
newlen" "int *valp"
 .Ft int
-.Fn sysctl_int_arr "int **valpp" "int *name" "u_int namelen" "void
*oldp" "size_t *oldlenp" "void *newp" "size_t newlen"
+.Fn sysctl_int_arr "int **valpp" "u_int valplen" "int *name" "u_int
namelen" "void *oldp" "size_t *oldlenp" "void *newp" "size_t newlen"
 .Ft int
 .Fn sysctl_quad "void *oldp" "size_t *oldlenp" "void *newp" "size_t
newlen" "int64_t *valp"
 .Ft int

Thanks
Greg
-- 
nest.cx is Gmail hosted, use PGP: https://pgp.key-server.io/0x0B1542BD8DF5A1B0
Fingerprint: 5E2B 2D0E 1E03 2046 BEC3  4D50 0B15 42BD 8DF5 A1B0



Re: no output on glass console after switching to serial

2020-08-01 Thread Stuart Henderson
On 2020/08/01 22:21, Mark Kettenis wrote:
> > pci11 at ppb9 bus 10
> > vga1 at pci11 dev 0 function 0 "ASPEED Technology AST2000" rev 0x30
> 
> This is the BMC graphics and seems to be the only grapics device
> available on this machine.

Correct.

> > wsdisplay at vga1 not configured
> 
> And I think this means the BIOS did not configure it as the primary
> graphics device and/or an active VGA device.  I'm not sure VGA text
> mode will work in such a setup so even if wsdisplay(4) would attach
> here it probably wouldn't work.

Aha. That sounds plausible, thanks.

> Any reason why you've chosen to installl this machine as a "legacy
> BIOS" machine instead of UEFI?  I think the latter would have given
> you efifb(4).

Mostly because I normally pxeboot machines to install them, and I
couldn't figure out how to get that to work with UEFI.



Re: acpicpu(4) and ACPI0007

2020-08-01 Thread Mark Kettenis
> Date: Sat, 1 Aug 2020 18:23:08 +1000
> From: Jonathan Matthew 
> Cc: tech@openbsd.org
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Wed, Jul 29, 2020 at 08:29:31PM +1000, Jonathan Matthew wrote:
> > On Wed, Jul 29, 2020 at 10:06:14AM +0200, Mark Kettenis wrote:
> > > > Date: Wed, 29 Jul 2020 10:38:55 +1000
> > > > From: Jonathan Matthew 
> > > > 
> > > > On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > > > > > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > > > > > From: Jonathan Matthew 
> > > > > > 
> > > > > > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > > > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > > > > > From: Jonathan Matthew 
> > > > > > > > 
> > > > > > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > > > > > From: Mark Kettenis 
> > > > > > > > > > 
> > > > > > > > > > Recent ACPI versions have deprecated "Processor()" nodes in 
> > > > > > > > > > favout of
> > > > > > > > > > "Device()" nodes with a _HID() method that returns 
> > > > > > > > > > "ACPI0007".  This
> > > > > > > > > > diff tries to support machines with firmware that 
> > > > > > > > > > implements this.  If
> > > > > > > > > > you see something like:
> > > > > > > > > > 
> > > > > > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > > > > > 
> > > > > > > > > > please try the following diff and report back with an 
> > > > > > > > > > updated dmesg.
> > > > > > > > > > 
> > > > > > > > > > Cheers,
> > > > > > > > > > 
> > > > > > > > > > Mark
> > > > > > > > > 
> > > > > > > > > And now with the right diff...
> > > > > > > > 
> > > > > > > > On a dell r6415, it looks like this:
> > > > > > > > 
> > > > > > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > > > > > all the way up to
> > > > > > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > > > > > 
> > > > > > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > > > > > AML_OBJTYPE_DEVICE.
> > > > > > > 
> > > > > > > Yes.  It is not immediately obvious how this should work.  Do we 
> > > > > > > need
> > > > > > > to copy the aml_node pointer or not?  We don't do that for
> > > > > > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > > > > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object 
> > > > > > > don't
> > > > > > > carry any additional information.  So we end up with just an empty
> > > > > > > case to avoid the warning.
> > > > > > > 
> > > > > > > Does this work on the Dell machines?
> > > > > > 
> > > > > > We've seen crashes in pool_cache_get() in various places after all 
> > > > > > the acpicpus
> > > > > > attach, which we haven't seen before on these machines, so I think 
> > > > > > it's
> > > > > > corrupting memory somehow.
> > > > > 
> > > > > Does that happen with only the acpicpu(4) diff?
> > > > 
> > > > Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
> > > > copy the result value, it leaves it uninitialised, which means we'll 
> > > > call
> > > > aml_freevalue() where res is stack junk.  memset(, 0, 
> > > > sizeof(res))
> > > > seems to fix it.
> > > 
> > > Eh, where exactly?
> > 
> > I had it just before the call to aml_evalnode(), but that can't be it,
> > since aml_evalnode() does the same thing.
> 
> Much better theory: the acpicpu_sc array has MAXCPUS elements, but on this
> system (and all R6415s, as far as I can tell) we have more acpicpu devices
> than that.  I suppose we should just make acpicpu_match fail if cf->cf_unit
> is >= MAXCPUS as we do with the actual cpu devices.

Yes, that makes more sense.

There are a couple of problems with this approach:

1. The set of "active" CPUs might not actually be covered by the first
   MAXCPUS ACPI0007 devices that appear in the ACPI device tree.

2. There are machines that have both Processor() nodes and ACPI0007
   Device() nodes.  See for example the machine from the "no output on
   glass console after switching to serial" post from sthen@ on tech@.

To address #1 we probably should check for a matching CPU early in
acpicpu_attach() and only add it to the acpicpu_sc[] array if there is
a match.  The current code is actually broken since the array is
indexed by acpicpu(4) dv_unit in places but cpu_number() in other
places.

To address #2 we could move the check for Processor() nodes later and
skip it if any acpicpu(4) devices already attached.

Cheers,

Mark

P.S. I don't have amd64 hardware with ACPI0007 devices, so if you want
 to take this diff, that is fine with me.  If not, let me know and
 I'll try to come up with a suitable diff.


> Index: acpicpu.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 acpicpu.c
> --- acpicpu.c 27 May 2020 05:02:21 -  1.85
> +++ acpicpu.c 

Re: no output on glass console after switching to serial

2020-08-01 Thread Mark Kettenis
> Date: Sat, 1 Aug 2020 20:54:28 +0100
> From: Stuart Henderson 
> 
> I've just been building a machine with serial console to go to colo
> tomorrow and have noticed that there's no output on glass console
> after the "switching console to com0" message. The only getty running
> after boot is the one on serial console.
> 
> I won't be able to do much in the way of testing on it now but thought
> it would be worth flagging anyway. Anyone see similar on other machines?

See below...

> >> OpenBSD/amd64 BOOT 3.52
> boot>
> booting hd0a:/bsd: 14488904+3191824+344096+0+872448 
> [972760+128+1137936+860957]=0x14ddc88
> entry point at 0x81001000
> [ using 2972808 bytes of bsd ELF symbol table ]
> Copyright (c) 1982, 1986, 1989, 1991, 1993
> The Regents of the University of California.  All rights reserved.
> Copyright (c) 1995-2020 OpenBSD. All rights reserved.  https://www.OpenBSD.org
> 
> OpenBSD 6.7-current (GENERIC.MP) #380: Fri Jul 31 09:04:24 MDT 2020
> dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 8464842752 (8072MB)
> avail mem = 8193216512 (7813MB)
> random: good seed from bootblocks
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xed9b0 (46 entries)
> bios0: vendor American Megatrends Inc. version "1.3" date 03/19/2018
> bios0: Supermicro Super Server
> acpi0 at bios0: ACPI 5.0
> acpi0: sleep states S0 S4 S5
> acpi0: tables DSDT FACP APIC FPDT FIDT SPMI MCFG UEFI DBG2 HPET WDDT SSDT 
> SSDT SSDT PRAD DMAR HEST BERT ERST EINJ
> acpi0: wakeup devices IP2P(S4) EHC1(S4) EHC2(S4) RP07(S4) RP08(S4) BR1A(S4) 
> BR1B(S4) BR2A(S4) BR2B(S4) BR2C(S4) BR2D(S4) BR3A(S4) BR3B(S4) BR3C(S4) 
> BR3D(S4) RP01(S4) [...]
> acpitimer0 at acpi0: 3579545 Hz, 24 bits
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.27 MHz, 06-56-03
> cpu0: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> cpu0: 256KB 64b/line 8-way L2 cache
> cpu0: smt 0, core 0, package 0
> mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
> cpu0: apic clock running at 99MHz
> cpu0: mwait min=64, max=64, C-substates=0.2.1.2, IBE
> cpu1 at mainbus0: apid 2 (application processor)
> cpu1: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.01 MHz, 06-56-03
> cpu1: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> cpu1: 256KB 64b/line 8-way L2 cache
> cpu1: smt 0, core 1, package 0
> cpu2 at mainbus0: apid 4 (application processor)
> cpu2: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.01 MHz, 06-56-03
> cpu2: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> cpu2: 256KB 64b/line 8-way L2 cache
> cpu2: smt 0, core 2, package 0
> cpu3 at mainbus0: apid 6 (application processor)
> cpu3: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.01 MHz, 06-56-03
> cpu3: 
> FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
> cpu3: 256KB 64b/line 8-way L2 cache
> cpu3: smt 0, core 3, package 0
> cpu4 at mainbus0: apid 1 (application processor)
> cpu4: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.00 MHz, 06-56-03
> cpu4: 
> 

no output on glass console after switching to serial

2020-08-01 Thread Stuart Henderson
I've just been building a machine with serial console to go to colo
tomorrow and have noticed that there's no output on glass console
after the "switching console to com0" message. The only getty running
after boot is the one on serial console.

I won't be able to do much in the way of testing on it now but thought
it would be worth flagging anyway. Anyone see similar on other machines?

>> OpenBSD/amd64 BOOT 3.52
boot>
booting hd0a:/bsd: 14488904+3191824+344096+0+872448 
[972760+128+1137936+860957]=0x14ddc88
entry point at 0x81001000
[ using 2972808 bytes of bsd ELF symbol table ]
Copyright (c) 1982, 1986, 1989, 1991, 1993
The Regents of the University of California.  All rights reserved.
Copyright (c) 1995-2020 OpenBSD. All rights reserved.  https://www.OpenBSD.org

OpenBSD 6.7-current (GENERIC.MP) #380: Fri Jul 31 09:04:24 MDT 2020
dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 8464842752 (8072MB)
avail mem = 8193216512 (7813MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0xed9b0 (46 entries)
bios0: vendor American Megatrends Inc. version "1.3" date 03/19/2018
bios0: Supermicro Super Server
acpi0 at bios0: ACPI 5.0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC FPDT FIDT SPMI MCFG UEFI DBG2 HPET WDDT SSDT SSDT 
SSDT PRAD DMAR HEST BERT ERST EINJ
acpi0: wakeup devices IP2P(S4) EHC1(S4) EHC2(S4) RP07(S4) RP08(S4) BR1A(S4) 
BR1B(S4) BR2A(S4) BR2B(S4) BR2C(S4) BR2D(S4) BR3A(S4) BR3B(S4) BR3C(S4) 
BR3D(S4) RP01(S4) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.27 MHz, 06-56-03
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu0: 256KB 64b/line 8-way L2 cache
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=0.2.1.2, IBE
cpu1 at mainbus0: apid 2 (application processor)
cpu1: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.01 MHz, 06-56-03
cpu1: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu1: 256KB 64b/line 8-way L2 cache
cpu1: smt 0, core 1, package 0
cpu2 at mainbus0: apid 4 (application processor)
cpu2: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.01 MHz, 06-56-03
cpu2: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu2: 256KB 64b/line 8-way L2 cache
cpu2: smt 0, core 2, package 0
cpu3 at mainbus0: apid 6 (application processor)
cpu3: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.01 MHz, 06-56-03
cpu3: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,DCA,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,PQM,RDSEED,ADX,SMAP,PT,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN
cpu3: 256KB 64b/line 8-way L2 cache
cpu3: smt 0, core 3, package 0
cpu4 at mainbus0: apid 1 (application processor)
cpu4: Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz, 2200.00 MHz, 06-56-03
cpu4: 

Re: LOCALE_HOME for strtime(3)

2020-08-01 Thread Todd C . Miller
On Sat, 01 Aug 2020 17:29:11 +0200, Jan Stary wrote:

> ping

This was already committed.

revision 1.33
date: 2020/07/16 20:08:12;  author: millert;  state: Exp;  lines: +1 -146;  
commitid: LVVrFB1zB8C0gLBS;
Remove obsolete LOCALE_HOME code we have never used (and never will).
Upstream removed it in 2004.  From Jan Stary.



Re: ksh.1: Mention Co-processes in $!

2020-08-01 Thread Jason McIntyre
On Sat, Aug 01, 2020 at 07:30:55PM +0200, Klemens Nanni wrote:
> On Sat, Aug 01, 2020 at 06:06:32PM +0100, Jason McIntyre wrote:
> > hmm. so then the current text ("the last background process") already
> > covers all these cases. why single out co-processes?
> Yes, "background process" technically covers co-processes, but at least
> for me "background processes" aka. jobs refer to those spawned with `&'
> where co-processes are the definitive term for those spawned with `|&',
> hence my brain wants to grep for "co-proc" and see `$!' mentioning it
> to confirm that `$!' is indeed not just about jobs.
> 
> > for the reader, i think the idea of background is easier to understand
> > as an umberella term, rather than asynchronous, even if it's maybe not
> > so correct.
> Fair enough;  I think the last diff is a slight improvement due to the
> above mentioned, but I also don't have a strong opinion about it.
> 
> If the impression is that I'm complicating stuff then I'll gladly drop
> the diff.
> 

well, maybe someone else will chip in. but i don;t see it as an
improvement myself.

jmc



Re: ksh.1: Mention Co-processes in $!

2020-08-01 Thread Klemens Nanni
On Sat, Aug 01, 2020 at 06:06:32PM +0100, Jason McIntyre wrote:
> hmm. so then the current text ("the last background process") already
> covers all these cases. why single out co-processes?
Yes, "background process" technically covers co-processes, but at least
for me "background processes" aka. jobs refer to those spawned with `&'
where co-processes are the definitive term for those spawned with `|&',
hence my brain wants to grep for "co-proc" and see `$!' mentioning it
to confirm that `$!' is indeed not just about jobs.

> for the reader, i think the idea of background is easier to understand
> as an umberella term, rather than asynchronous, even if it's maybe not
> so correct.
Fair enough;  I think the last diff is a slight improvement due to the
above mentioned, but I also don't have a strong opinion about it.

If the impression is that I'm complicating stuff then I'll gladly drop
the diff.



Re: acpicpu(4) and ACPI0007

2020-08-01 Thread Jan Stary
On Aug 01 18:23:08, jonat...@d14n.org wrote:
> Much better theory: the acpicpu_sc array has MAXCPUS elements, but on this
> system (and all R6415s, as far as I can tell) we have more acpicpu devices
> than that.  I suppose we should just make acpicpu_match fail if cf->cf_unit
> is >= MAXCPUS as we do with the actual cpu devices.
> 
> 
> Index: acpicpu.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 acpicpu.c
> --- acpicpu.c 27 May 2020 05:02:21 -  1.85
> +++ acpicpu.c 1 Aug 2020 08:18:49 -
> @@ -186,6 +186,11 @@ struct cfdriver acpicpu_cd = {
>   NULL, "acpicpu", DV_DULL
>  };
>  
> +const char *acpicpu_hids[] = {
> + "ACPI0007",
> + NULL
> +};
> +
>  extern int setperf_prio;
>  
>  struct acpicpu_softc *acpicpu_sc[MAXCPUS];
> @@ -650,6 +655,12 @@ acpicpu_match(struct device *parent, voi
>   struct acpi_attach_args *aa = aux;
>   struct cfdata   *cf = match;
>  
> + if (cf->cf_unit >= MAXCPUS)
> + return (0);
> +
> + if (acpi_matchhids(aa, acpicpu_hids, cf->cf_driver->cd_name))
> + return (1);
> +
>   /* sanity */
>   if (aa->aaa_name == NULL ||
>   strcmp(aa->aaa_name, cf->cf_driver->cd_name) != 0 ||
> @@ -665,6 +676,7 @@ acpicpu_attach(struct device *parent, st
>   struct acpicpu_softc*sc = (struct acpicpu_softc *)self;
>   struct acpi_attach_args *aa = aux;
>   struct aml_valueres;
> + int64_t uid;
>   int i;
>   uint32_tstatus = 0;
>   CPU_INFO_ITERATOR   cii;
> @@ -675,6 +687,10 @@ acpicpu_attach(struct device *parent, st
>   acpicpu_sc[sc->sc_dev.dv_unit] = sc;
>  
>   SLIST_INIT(>sc_cstates);
> +
> + if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
> + "_UID", 0, NULL, ) == 0)
> + sc->sc_cpu = uid;
>  
>   if (aml_evalnode(sc->sc_acpi, sc->sc_devnode, 0, NULL, ) == 0) {
>   if (res.type == AML_OBJTYPE_PROCESSOR) {
> 

See below for the old and the new dmesg of current/amd64 at an APU2.

Edited highlights of the diff:

--- apu2e.20200610  Wed Jun 10 22:24:25 2020
+++ apu2e.20200801  Sat Aug  1 19:17:00 2020
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
-"ACPI0007" at acpi0 not configured
+acpicpu0 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu1 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu2 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu3 at acpi0copyvalue: 6: C2(0@400 io@0x1771), C1(@1 halt!), PSS
+acpicpu4 at acpi0copyvalue: 6: no cpu matching ACPI ID 4
+acpicpu5 at acpi0copyvalue: 6: no cpu matching ACPI ID 5
+acpicpu6 at acpi0copyvalue: 6: no cpu matching ACPI ID 6
+acpicpu7 at acpi0copyvalue: 6: no cpu matching ACPI ID 7
+cpu0: 998 MHz: speeds: 1000 800 600 MHz

Jan


OpenBSD 6.7-current (GENERIC.MP) #0: Wed Jun 10 22:14:40 CEST 2020
h...@uvt.stare.cz:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 1996484608 (1903MB)
avail mem = 1921105920 (1832MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.8 @ 0x7ee8d020 (13 entries)
bios0: vendor coreboot version "v4.11.0.5" date 03/29/2020
bios0: PC Engines apu2
acpi0 at bios0: ACPI 4.0
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP SSDT MCFG TPM2 APIC HEST SSDT SSDT HPET
acpi0: wakeup devices PWRB(S4) PBR4(S4) PBR5(S4) PBR6(S4) PBR7(S4) PBR8(S4) 
UOH1(S3) UOH2(S3) UOH3(S3) UOH4(S3) UOH5(S3) UOH6(S3) XHC0(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimcfg0 at acpi0
acpimcfg0: addr 0xf800, bus 0-64
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD GX-412TC SOC, 998.26 MHz, 16-30-01
cpu0: 
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TOPEXT,DBKP,PERFTSC,PCTRL3,ITSC,BMI1,XSAVEOPT
cpu0: 32KB 64b/line 2-way I-cache, 32KB 64b/line 8-way D-cache, 2MB 64b/line 
16-way L2 cache
cpu0: ITLB 32 4KB entries fully associative, 8 4MB entries fully associative
cpu0: DTLB 40 4KB entries fully associative, 8 4MB en

Re: ksh.1: Mention Co-processes in $!

2020-08-01 Thread Jason McIntyre
On Sat, Aug 01, 2020 at 06:59:43PM +0200, Klemens Nanni wrote:
> On Sat, Aug 01, 2020 at 05:40:07PM +0100, Jason McIntyre wrote:
> > i'm worried that you're blurring the distinction between asynchronous
> > and co-process for the reader. i think that's relevant because, as you
> > say, a page like sh(1) does not document co-processes, whereas ksh(1)
> > does.
> You raise a valid point.
> 
> > as i understand it, ksh explains co-processes as a type of asynchronous
> > process. and the setting of "!" is applicable to both. is that right?
> Correct.  Both background jobs (`&') and co-processes (`|&') are
> asynchronous and co-processes are background jobs by definition.
> 
> What makes co-processes different is a two-way pipe between them and
> the shell, i.e. you can read and write in both directions.
> 
> > so could your text be better written as:
> > 
> > Process ID of the last background or asynchronous process started.
> > If no processes have been started, the parameter is not set.
> > 
> > this would catch the setting of "!" for both async and co-process,
> > without making the text read like the two are identical.
> > 
> > if your concern is that you need to be able to /co-process, then i'd
> > rewrite the text to not blur the two:
> > 
> > Process ID of the last background, co-process, or asynchronous
> > process started.
> > 
> > but i dislike that, because it means the text suffers because of an
> > arbitrary request to make a specific search work.
> Indeed, I'd like to have `/co-process' show it.
> 
> > i guess we could add a note to the "Some notes concerning co-processes"
> > section if it needs more clarity.
> I don't think this is needed.
> 
> Background jobs as well as co-processes are async and background jobs or
> rather `&' is documented as "asynchronous command" in other places of
> the manual, so how about this?
> 

hmm. so then the current text ("the last background process") already
covers all these cases. why single out co-processes?

for the reader, i think the idea of background is easier to understand
as an umberella term, rather than asynchronous, even if it's maybe not
so correct.

either you or i have trimmed your original mail, explaining why
this was important ;)

jmc

> 
> Index: ksh.1
> ===
> RCS file: /cvs/src/bin/ksh/ksh.1,v
> retrieving revision 1.209
> diff -u -p -r1.209 ksh.1
> --- ksh.1 7 Jul 2020 10:33:58 -   1.209
> +++ ksh.1 1 Aug 2020 16:59:10 -
> @@ -1247,8 +1247,8 @@ The following special parameters are imp
>  set directly using assignments:
>  .Bl -tag -width "1 ... 9"
>  .It Ev \&!
> -Process ID of the last background process started.
> -If no background processes have been started, the parameter is not set.
> +Process ID of the last background process or co-process started.
> +If no asynchronous processes have been started, the parameter is not set.
>  .It Ev 

Re: ksh.1: Mention Co-processes in $!

2020-08-01 Thread Klemens Nanni
On Sat, Aug 01, 2020 at 05:40:07PM +0100, Jason McIntyre wrote:
> i'm worried that you're blurring the distinction between asynchronous
> and co-process for the reader. i think that's relevant because, as you
> say, a page like sh(1) does not document co-processes, whereas ksh(1)
> does.
You raise a valid point.

> as i understand it, ksh explains co-processes as a type of asynchronous
> process. and the setting of "!" is applicable to both. is that right?
Correct.  Both background jobs (`&') and co-processes (`|&') are
asynchronous and co-processes are background jobs by definition.

What makes co-processes different is a two-way pipe between them and
the shell, i.e. you can read and write in both directions.

> so could your text be better written as:
> 
>   Process ID of the last background or asynchronous process started.
>   If no processes have been started, the parameter is not set.
> 
> this would catch the setting of "!" for both async and co-process,
> without making the text read like the two are identical.
> 
> if your concern is that you need to be able to /co-process, then i'd
> rewrite the text to not blur the two:
> 
>   Process ID of the last background, co-process, or asynchronous
>   process started.
> 
> but i dislike that, because it means the text suffers because of an
> arbitrary request to make a specific search work.
Indeed, I'd like to have `/co-process' show it.

> i guess we could add a note to the "Some notes concerning co-processes"
> section if it needs more clarity.
I don't think this is needed.

Background jobs as well as co-processes are async and background jobs or
rather `&' is documented as "asynchronous command" in other places of
the manual, so how about this?


Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.209
diff -u -p -r1.209 ksh.1
--- ksh.1   7 Jul 2020 10:33:58 -   1.209
+++ ksh.1   1 Aug 2020 16:59:10 -
@@ -1247,8 +1247,8 @@ The following special parameters are imp
 set directly using assignments:
 .Bl -tag -width "1 ... 9"
 .It Ev \&!
-Process ID of the last background process started.
-If no background processes have been started, the parameter is not set.
+Process ID of the last background process or co-process started.
+If no asynchronous processes have been started, the parameter is not set.
 .It Ev 

Re: ksh.1: Mention Co-processes in $!

2020-08-01 Thread Jason McIntyre
On Sat, Aug 01, 2020 at 05:57:01PM +0200, Klemens Nanni wrote:
> Otherwise it is not clear whether $! will be set or not.  This way,
> `/Co-proc' brings me to *all* relevant spots in the manual.
> 
> Snippet to demonstrate how $! is set for an asynchronous process:
> 
>   $ ksh -c ': |& echo $!' 
>   67163
> 
> FWIW, sh(1) doesn't document Co-processes (whis is fine/correct) and
> bash(1) says this about $!:
> 
>   !  Expands to the process ID of the job most recently placed into
>  the background, whether executed as an asynchronous command or
>  using the bg builtin (see JOB CONTROL below).
> 
> Feedback? OK?
> 
> 
> Index: ksh.1
> ===
> RCS file: /cvs/src/bin/ksh/ksh.1,v
> retrieving revision 1.209
> diff -u -p -r1.209 ksh.1
> --- ksh.1 7 Jul 2020 10:33:58 -   1.209
> +++ ksh.1 1 Aug 2020 15:50:04 -
> @@ -1247,8 +1247,9 @@ The following special parameters are imp
>  set directly using assignments:
>  .Bl -tag -width "1 ... 9"
>  .It Ev \&!
> -Process ID of the last background process started.
> -If no background processes have been started, the parameter is not set.
> +Process ID of the last background process or asynchronous process 
> (Co-process)
> +started.
> +If no processes have been started, the parameter is not set.
>  .It Ev 

ksh.1: Mention Co-processes in $!

2020-08-01 Thread Klemens Nanni
Otherwise it is not clear whether $! will be set or not.  This way,
`/Co-proc' brings me to *all* relevant spots in the manual.

Snippet to demonstrate how $! is set for an asynchronous process:

$ ksh -c ': |& echo $!' 
67163

FWIW, sh(1) doesn't document Co-processes (whis is fine/correct) and
bash(1) says this about $!:

!  Expands to the process ID of the job most recently placed into
   the background, whether executed as an asynchronous command or
   using the bg builtin (see JOB CONTROL below).

Feedback? OK?


Index: ksh.1
===
RCS file: /cvs/src/bin/ksh/ksh.1,v
retrieving revision 1.209
diff -u -p -r1.209 ksh.1
--- ksh.1   7 Jul 2020 10:33:58 -   1.209
+++ ksh.1   1 Aug 2020 15:50:04 -
@@ -1247,8 +1247,9 @@ The following special parameters are imp
 set directly using assignments:
 .Bl -tag -width "1 ... 9"
 .It Ev \&!
-Process ID of the last background process started.
-If no background processes have been started, the parameter is not set.
+Process ID of the last background process or asynchronous process (Co-process)
+started.
+If no processes have been started, the parameter is not set.
 .It Ev 

Re: pipex(4): kill pipexintr()

2020-08-01 Thread Vitaliy Makkoveev
On Sat, Aug 01, 2020 at 07:44:17PM +0900, YASUOKA Masahiko wrote:
> Hi,
> 
> I'm not sure when it is broken, in old versions, it was assumed the
> pipex queues are empty when pipex_iface_stop() is called.  The problem
> mvs@ found is the assumption is not true any more.
> 
> pipex has a mechanism that delete a session when the queues are empty.
> 
> 819 Static void
> 820 pipex_timer(void *ignored_arg)
> 821 {
> (snip)
> 854 case PIPEX_STATE_CLOSED:
> 855 /*
> 856  * mbuf queued in pipexinq or pipexoutq may 
> have a
> 857  * refererce to this session.
> 858  */
> 859 if (!mq_empty() || 
> !mq_empty())
> 860 continue;
> 861 
> 862 pipex_destroy_session(session);
> 863 break;
> 
> I think using this is better.
> 
> How about this?
> 

Unfortunately your diff is incorrect. It introduces memory leaks and
breaks pppx(4). Also it is incomplete.

We have multiple ways to kill pipex(sessions):

1. pppx(4)

We have `struct pppx_if' which has pointer to corresponding session and
this session is accessed directly within pppx(4) layer. Since we can't
destroy `ppp_if' in pipex(4) layer we can't destroy these sessions by
pipex_timer(). The only way to destroy them is pppx_if_destroy() which:

1. unlink session by pipex_unlink_session()
2. detach corresponding `ifnet' by if_detach()
3. release session by pipex_rele_session() 

It's unsafe because mbuf queues can have references to this session.

2. pppac(4)

We have no direct access to corresponding sessions within pppac(4)
layer. Also there are multiple ways to do this:

1. pipex_ioctl() with `PIPEXSMODE' command. Underlay pipex_iface_stop()
walks through `pipex_session_list' and destroy sessions by
pipex_destroy_session() call. It's unsafe because we don't check queues.

2. pipex_ioctl() with `PIPEXDSESSION'. pipex_close_session() will change
session's  state and pipex_timer() will kill this sessions later. This
is the only safe way.

3. pipex_iface_fini(). The same as `PIPEXSMODE', pipex_iface_stop()
kills sessions, Which is also unsafe. Also we have another use after
free issue:

 cut begin 

pipex_iface_fini(struct pipex_iface_context *pipex_iface)
{
pool_put(_session_pool, pipex_iface->multicast_session);
NET_LOCK();
pipex_iface_stop(pipex_iface);
NET_UNLOCK();
}

 cut end 

`multicast_session' should be protected too. It also can be pushed to
`pipexoutq'. Also since this time pipexintr() and pipex_iface_fini() are
both serialized by KERNEL_LOCK() too we can't destroy `multicast_session'
which is in use by pipexintr(). But when we will drop KERNEL_LOCK()
around pipexintr() we can catch use after free issue here. I already did
diff for move this pool_put() under NET_LOCK(), but it was rejectedi by
mpi@ because:

 cut begin 
pipex_iface_fini() should be called on the last reference of the
descriptor.  So this shouldn't be necessary.  If there's an issue   
with the current order of the operations, we should certainly fix   
it differently.   
 cut end 

So I repeat it again: npppd(8) can be killed in every moment by SIGKILL
or by SIGSEGV and pppacclose() will be called and it will call
pipex_iface_fini(). `multicast_session' can be used in this moment by
pipexintr().

And no locks protect `multicast_session' itself.

The two diffs I proposed in this thread solve problems caused by
pipexintr().

I'm not sure about bottleneck in crypto thread, because we have no
simultaneous execution with pipexintr(), but the second diff which
introduces refcounters does session destruction well, include
`multicast_session'. I found a little issue with second diff which stops
processing multicast packets, so I included updated version below.

> diff --git a/sys/net/pipex.c b/sys/net/pipex.c
> index 2ad7757fee9..6fe14c400bf 100644
> --- a/sys/net/pipex.c
> +++ b/sys/net/pipex.c
> @@ -190,7 +190,7 @@ pipex_iface_stop(struct pipex_iface_context *pipex_iface)
>   LIST_FOREACH_SAFE(session, _session_list, session_list,
>   session_tmp) {
>   if (session->pipex_iface == pipex_iface)
> - pipex_destroy_session(session);
> + pipex_unlink_session(session);

You assumed pipex_timer() the only place where sessions can be released?
pipex_unlink_session() will detach session from all lists, include
`pipex_session_list'. This session is leaked.

Also what to do with multicast sessions?

>   }
>  }
>  
> @@ -470,9 +470,16 @@ pipex_link_session(struct pipex_session *session,
>  void
>  pipex_unlink_session(struct pipex_session *session)
>  {
> + struct radix_node *rn;
> +
>   session->ifindex = 0;
>  
>   NET_ASSERT_LOCKED();
> + if 

Re: LOCALE_HOME for strtime(3)

2020-08-01 Thread Jan Stary
ping

On Jul 16 09:23:22, h...@stare.cz wrote:
> On Jul 15 15:48:41, mill...@openbsd.org wrote:
> > Upstream tzcode removed the LOCALE_HOME bits in 2014.  There's no
> > reason for us to keep it.
> 
> With that removed, the header file can go too.
> 
>   Jan
> 
> 
> Index: lib/libc/time/strftime.c
> ===
> RCS file: /cvs/src/lib/libc/time/strftime.c,v
> retrieving revision 1.32
> diff -u -p -r1.32 strftime.c
> --- lib/libc/time/strftime.c  29 Jun 2019 16:12:21 -  1.32
> +++ lib/libc/time/strftime.c  16 Jul 2020 07:20:23 -
> @@ -29,7 +29,6 @@
>  */
>  
>  #include 
> -#include 
>  #include 
>  
>  #include "private.h"
> @@ -48,16 +47,7 @@ struct lc_time_T {
>   const char *date_fmt;
>  };
>  
> -#ifdef LOCALE_HOME
> -#include "sys/stat.h"
> -static struct lc_time_T  localebuf;
> -static struct lc_time_T *_loc(void);
> -#define Locale   _loc()
> -#endif /* defined LOCALE_HOME */
> -#ifndef LOCALE_HOME
>  #define Locale   (_time_locale)
> -#endif /* !defined LOCALE_HOME */
> -
>  static const struct lc_time_TC_time_locale = {
>   {
>   "Jan", "Feb", "Mar", "Apr", "May", "Jun",
> @@ -124,9 +114,6 @@ strftime(char *s, size_t maxsize, const 
>   int warn;
>  
>   tzset();
> -#ifdef LOCALE_HOME
> - localebuf.mon[0] = 0;
> -#endif /* defined LOCALE_HOME */
>   warn = IN_NONE;
>   p = _fmt(((format == NULL) ? "%c" : format), t, s, s + maxsize, );
>   if (p == s + maxsize) {
> @@ -558,135 +545,3 @@ _yconv(int a, int b, int convert_top, in
>   pt = _conv(((trail < 0) ? -trail : trail), "%02d", pt, ptlim);
>   return pt;
>  }
> -
> -#ifdef LOCALE_HOME
> -static struct lc_time_T *
> -_loc(void)
> -{
> - static const char   locale_home[] = LOCALE_HOME;
> - static const char   lc_time[] = "LC_TIME";
> - static char *   locale_buf;
> -
> - int fd;
> - int oldsun; /* "...ain't got nothin' to do..." */
> - int len;
> - char *  lbuf;
> - char *  nlbuf;
> - char *  name;
> - char *  p;
> - const char **   ap;
> - const char *plim;
> - charfilename[PATH_MAX];
> - struct stat st;
> - size_t  namesize;
> - size_t  bufsize;
> -
> - /*
> - ** Use localebuf.mon[0] to signal whether locale is already set up.
> - */
> - if (localebuf.mon[0])
> - return 
> - name = setlocale(LC_TIME, (char *) NULL);
> - if (name == NULL || *name == '\0')
> - goto no_locale;
> - /*
> - ** If the locale name is the same as our cache, use the cache.
> - */
> - lbuf = locale_buf;
> - if (lbuf != NULL && strcmp(name, lbuf) == 0) {
> - p = lbuf;
> - for (ap = (const char **) 
> - ap < (const char **) ( + 1);
> - ++ap)
> - *ap = p += strlen(p) + 1;
> - return 
> - }
> - /*
> - ** Slurp the locale file into the cache.
> - */
> - namesize = strlen(name) + 1;
> - if (sizeof filename <
> - ((sizeof locale_home) + namesize + (sizeof lc_time)))
> - goto no_locale;
> - oldsun = 0;
> - len = snprintf(filename, sizeof filename, "%s/%s/%s", locale_home,
> - name, lc_time);
> - if (len < 0 || len >= sizeof filename)
> - goto no_locale;
> - fd = open(filename, O_RDONLY);
> - if (fd == -1) {
> - /*
> - ** Old Sun systems have a different naming and data convention.
> - */
> - oldsun = 1;
> - len = snprintf(filename, sizeof filename, "%s/%s/%s",
> - locale_home, lc_time, name);
> - if (len < 0 || len >= sizeof filename)
> - goto no_locale;
> - fd = open(filename, O_RDONLY);
> - if (fd  == -1)
> - goto no_locale;
> - }
> - if (fstat(fd, ) == -1)
> - goto bad_locale;
> - if (st.st_size <= 0)
> - goto bad_locale;
> - bufsize = namesize + st.st_size;
> - locale_buf = NULL;
> - nlbuf = realloc(lbuf, bufsize);
> - if (nlbuf == NULL) {
> - free(lbuf);
> - lbuf = NULL;
> - goto bad_locale;
> - }
> - lbuf = nlbuf;
> - (void) strlcpy(lbuf, name, bufsize);
> - p = lbuf + namesize;
> - plim = p + st.st_size;
> - if (read(fd, p, st.st_size) != st.st_size)
> - goto bad_lbuf;
> - if (close(fd) != 0)
> - goto bad_lbuf;
> - /*
> - ** Parse the locale file into localebuf.
> - */
> - if (plim[-1] != '\n')
> - goto bad_lbuf;
> - for 

Re: pipex(4): kill pipexintr()

2020-08-01 Thread YASUOKA Masahiko
Hi,

I'm not sure when it is broken, in old versions, it was assumed the
pipex queues are empty when pipex_iface_stop() is called.  The problem
mvs@ found is the assumption is not true any more.

pipex has a mechanism that delete a session when the queues are empty.

819 Static void
820 pipex_timer(void *ignored_arg)
821 {
(snip)
854 case PIPEX_STATE_CLOSED:
855 /*
856  * mbuf queued in pipexinq or pipexoutq may 
have a
857  * refererce to this session.
858  */
859 if (!mq_empty() || 
!mq_empty())
860 continue;
861 
862 pipex_destroy_session(session);
863 break;

I think using this is better.

How about this?

diff --git a/sys/net/pipex.c b/sys/net/pipex.c
index 2ad7757fee9..6fe14c400bf 100644
--- a/sys/net/pipex.c
+++ b/sys/net/pipex.c
@@ -190,7 +190,7 @@ pipex_iface_stop(struct pipex_iface_context *pipex_iface)
LIST_FOREACH_SAFE(session, _session_list, session_list,
session_tmp) {
if (session->pipex_iface == pipex_iface)
-   pipex_destroy_session(session);
+   pipex_unlink_session(session);
}
 }
 
@@ -470,9 +470,16 @@ pipex_link_session(struct pipex_session *session,
 void
 pipex_unlink_session(struct pipex_session *session)
 {
+   struct radix_node *rn;
+
session->ifindex = 0;
 
NET_ASSERT_LOCKED();
+   if (!in_nullhost(session->ip_address.sin_addr)) {
+   rn = rn_delete(>ip_address, >ip_netmask,
+   pipex_rd_head4, (struct radix_node *)session);
+   KASSERT(rn != NULL);
+   }
LIST_REMOVE(session, id_chain);
 #if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (session->protocol) {
@@ -486,10 +493,6 @@ pipex_unlink_session(struct pipex_session *session)
LIST_REMOVE(session, state_list);
LIST_REMOVE(session, session_list);
session->state = PIPEX_STATE_CLOSED;
-
-   /* if final session is destroyed, stop timer */
-   if (LIST_EMPTY(_session_list))
-   pipex_timer_stop();
 }
 
 Static int
@@ -652,20 +655,16 @@ pipex_get_closed(struct pipex_session_list_req *req,
 Static int
 pipex_destroy_session(struct pipex_session *session)
 {
-   struct radix_node *rn;
-
/* remove from radix tree and hash chain */
NET_ASSERT_LOCKED();
 
-   if (!in_nullhost(session->ip_address.sin_addr)) {
-   rn = rn_delete(>ip_address, >ip_netmask,
-   pipex_rd_head4, (struct radix_node *)session);
-   KASSERT(rn != NULL);
-   }
-
pipex_unlink_session(session);
pipex_rele_session(session);
 
+   /* if final session is destroyed, stop timer */
+   if (LIST_EMPTY(_session_list))
+   pipex_timer_stop();
+
return (0);
 }
 
@@ -739,7 +738,8 @@ pipexintr(void)
mq_delist(, );
while ((m = ml_dequeue()) != NULL) {
pkt_session = m->m_pkthdr.ph_cookie;
-   if (pkt_session == NULL) {
+   if (pkt_session == NULL ||
+   pkt_session->state == PIPEX_STATE_CLOSED) {
m_freem(m);
continue;
}
@@ -776,7 +776,8 @@ pipexintr(void)
mq_delist(, );
while ((m = ml_dequeue()) != NULL) {
pkt_session = m->m_pkthdr.ph_cookie;
-   if (pkt_session == NULL) {
+   if (pkt_session == NULL ||
+   pkt_session->state == PIPEX_STATE_CLOSED) {
m_freem(m);
continue;
}



Re: acpicpu(4) and ACPI0007

2020-08-01 Thread Jonathan Matthew
On Wed, Jul 29, 2020 at 08:29:31PM +1000, Jonathan Matthew wrote:
> On Wed, Jul 29, 2020 at 10:06:14AM +0200, Mark Kettenis wrote:
> > > Date: Wed, 29 Jul 2020 10:38:55 +1000
> > > From: Jonathan Matthew 
> > > 
> > > On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > > > > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > > > > From: Jonathan Matthew 
> > > > > 
> > > > > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > > > > From: Jonathan Matthew 
> > > > > > > 
> > > > > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > > > > From: Mark Kettenis 
> > > > > > > > > 
> > > > > > > > > Recent ACPI versions have deprecated "Processor()" nodes in 
> > > > > > > > > favout of
> > > > > > > > > "Device()" nodes with a _HID() method that returns 
> > > > > > > > > "ACPI0007".  This
> > > > > > > > > diff tries to support machines with firmware that implements 
> > > > > > > > > this.  If
> > > > > > > > > you see something like:
> > > > > > > > > 
> > > > > > > > >   "ACPI0007" at acpi0 not configured
> > > > > > > > > 
> > > > > > > > > please try the following diff and report back with an updated 
> > > > > > > > > dmesg.
> > > > > > > > > 
> > > > > > > > > Cheers,
> > > > > > > > > 
> > > > > > > > > Mark
> > > > > > > > 
> > > > > > > > And now with the right diff...
> > > > > > > 
> > > > > > > On a dell r6415, it looks like this:
> > > > > > > 
> > > > > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > > > > all the way up to
> > > > > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > > > > > 
> > > > > > > which I guess means aml_copyvalue() needs to learn how to copy 
> > > > > > > AML_OBJTYPE_DEVICE.
> > > > > > 
> > > > > > Yes.  It is not immediately obvious how this should work.  Do we 
> > > > > > need
> > > > > > to copy the aml_node pointer or not?  We don't do that for
> > > > > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > > > > similar to AML_OBJTYPE_DEVICE.  But AML_OBJTYPE_DEVICE object don't
> > > > > > carry any additional information.  So we end up with just an empty
> > > > > > case to avoid the warning.
> > > > > > 
> > > > > > Does this work on the Dell machines?
> > > > > 
> > > > > We've seen crashes in pool_cache_get() in various places after all 
> > > > > the acpicpus
> > > > > attach, which we haven't seen before on these machines, so I think 
> > > > > it's
> > > > > corrupting memory somehow.
> > > > 
> > > > Does that happen with only the acpicpu(4) diff?
> > > 
> > > Yes.  Looking at this a bit more, in the case where aml_evalnode() can't
> > > copy the result value, it leaves it uninitialised, which means we'll call
> > > aml_freevalue() where res is stack junk.  memset(, 0, sizeof(res))
> > > seems to fix it.
> > 
> > Eh, where exactly?
> 
> I had it just before the call to aml_evalnode(), but that can't be it,
> since aml_evalnode() does the same thing.

Much better theory: the acpicpu_sc array has MAXCPUS elements, but on this
system (and all R6415s, as far as I can tell) we have more acpicpu devices
than that.  I suppose we should just make acpicpu_match fail if cf->cf_unit
is >= MAXCPUS as we do with the actual cpu devices.


Index: acpicpu.c
===
RCS file: /cvs/src/sys/dev/acpi/acpicpu.c,v
retrieving revision 1.85
diff -u -p -r1.85 acpicpu.c
--- acpicpu.c   27 May 2020 05:02:21 -  1.85
+++ acpicpu.c   1 Aug 2020 08:18:49 -
@@ -186,6 +186,11 @@ struct cfdriver acpicpu_cd = {
NULL, "acpicpu", DV_DULL
 };
 
+const char *acpicpu_hids[] = {
+   "ACPI0007",
+   NULL
+};
+
 extern int setperf_prio;
 
 struct acpicpu_softc *acpicpu_sc[MAXCPUS];
@@ -650,6 +655,12 @@ acpicpu_match(struct device *parent, voi
struct acpi_attach_args *aa = aux;
struct cfdata   *cf = match;
 
+   if (cf->cf_unit >= MAXCPUS)
+   return (0);
+
+   if (acpi_matchhids(aa, acpicpu_hids, cf->cf_driver->cd_name))
+   return (1);
+
/* sanity */
if (aa->aaa_name == NULL ||
strcmp(aa->aaa_name, cf->cf_driver->cd_name) != 0 ||
@@ -665,6 +676,7 @@ acpicpu_attach(struct device *parent, st
struct acpicpu_softc*sc = (struct acpicpu_softc *)self;
struct acpi_attach_args *aa = aux;
struct aml_valueres;
+   int64_t uid;
int i;
uint32_tstatus = 0;
CPU_INFO_ITERATOR   cii;
@@ -675,6 +687,10 @@ acpicpu_attach(struct device *parent, st
acpicpu_sc[sc->sc_dev.dv_unit] = sc;
 
SLIST_INIT(>sc_cstates);
+
+   if (aml_evalinteger(sc->sc_acpi, sc->sc_devnode,
+   "_UID", 0, NULL, ) == 0)
+   sc->sc_cpu = uid;
 
if (aml_evalnode(sc->sc_acpi, sc->sc_devnode, 0, NULL,