Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Bob Beck
> (iirc python does something strange)

Inconcievable!



Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Stuart Henderson
On 2020/05/29 17:25, Bob Beck wrote:
> On Fri, May 29, 2020 at 06:14:44PM +0200, Marc Espie wrote:
> > In a trace:
> > 
> > > > > #3  0x15e48c95459e in WebVfx::shutdown ()
> > > > >  at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193
> > 
> > Now, this is NOT the default location for WRKOBJDIR, but we are shipping
> > packages with debug information...
> > 
> > This could be a bit cumbersome, if in order to debug locally, you have
> > to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
> > "official" setup for ports, which does not even match the default.
> > 
> > There are about 3 solutions to that:
> > - change the bulk machines to /usr/ports/pobj
> > - change the ports default to /usr/obj/ports
> 
> It's not realpathing is it? correct me if I am wrong but would
> making /usr/ports/pobj a symlink to /usr/obj/ports on the bulk 
> machines and running them with the default config just "fix" this? 

It might workaround this but it wouldn't surprise me if it breaks
something else, we have seen weird problems before with ports builds and
paths involving symlinks (iirc python does something strange)

> > - have some magic I don't know in ELF handling that would allow to either
> > tweak the default location/introduce ${WRKOBJDIR} in that debug info.
> > 
> 




Re: sparc64 boot issue on qemu

2020-05-29 Thread Jason A. Donenfeld
On Fri, May 29, 2020 at 4:56 PM Jason A. Donenfeld  wrote:
>
> Oh that's a nice observation about `boot disk -V`. Doing so actually
> got me booting up entirely:
>
> $ qemu-img convert -O qcow2 miniroot66.fs disk.qcow2
> $ qemu-img resize disk.qcow2 20G
> $ qemu-system-sparc64 -m 1024 -drive file=disk.qcow2,if=ide -net
> nic,model=ne2k_pci -net user -boot a -nographic -monitor none -serial
> stdio
> OpenBIOS for Sparc64
> Configuration device id QEMU version 1 machine id 0
> kernel cmdline
> CPUs: 1 x SUNW,UltraSPARC-IIi
> UUID: ----
> Welcome to OpenBIOS v1.1 built on Oct 28 2019 17:08
>   Type 'help' for detailed information
> Trying /obio/SUNW,fdtwo...
> No valid state has been set by load or init-program
>
> 0 > boot disk -V Not a Linux kernel image
> Not a bootable ELF image
> Not a bootable a.out image
>
> Loading FCode image...
> Loaded 6882 bytes
> entry point is 0x4000
> Evaluating FCode...
> OpenBSD IEEE 1275 Bootblock 2.0
> Booting from device /pci@1fe,0/pci@1,1/ide@3/ide@0/disk@0
> Try superblock read
> FFS v1
> ufs-open complete
> .Looking for ofwboot in directory...
> .
> ..
> ofwboot
> Found it
> .Loading 1a0e0  bytes of file...
> Copying 2000 bytes to 4000
> Copying 2000 bytes to 6000
> Copying 2000 bytes to 8000
> Copying 2000 bytes to a000
> Copying 2000 bytes to c000
> Copying 2000 bytes to e000
> Copying 2000 bytes to 1
> Copying 2000 bytes to 12000
> Copying 2000 bytes to 14000
> Copying 2000 bytes to 16000
> Copying 2000 bytes to 18000
> Copying 2000 bytes to 1a000
> Copying 2000 bytes to 1c000
> Copying 2000 bytes to 1e000
> reserved fcode word.
> >> OpenBSD BOOT 1.17
> Trying bsd...
> open /etc/random.seed: No such file or directory
> Booting /pci@1fe,0/pci@1,1/ide@3/ide@0/disk@0/bsd
> 4211776@0x100+7104@0x1404440+3247928@0x1c0+946376@0x1f18f38
> symbols @ 0xfef50340 139 start=0x100
> console is /pci@1fe,0/pci@1,1/ebus@1/su
> 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 (RAMDISK) #298: Thu May  7 19:11:18 MDT 2020
> dera...@sparc64.openbsd.org:/usr/src/sys/arch/sparc64/compile/RAMDISK
> real mem = 1073741824 (1024MB)
> avail mem = 1044135936 (995MB)
> unknown option `V'
> mainbus0 at root: OpenBiosTeam,OpenBIOS
> cpu0 at mainbus0: SUNW,UltraSPARC-IIi (rev 9.1) @ 100 MHz
> cpu0: physical 256K instruction (64 b/l), 16K data (32 b/l), 256K
> external (64 b/l)
> psycho0 at mainbus0: SUNW,sabre, impl 0, version 0, ign 7c0
> psycho0: bus range 0-2, PCI bus 0
> psycho0: dvma map c000-dfff
> pci0 at psycho0
> ppb0 at pci0 dev 1 function 1 "Sun Simba" rev 0x11
> pci1 at ppb0 bus 1
> ebus0 at pci1 dev 1 function 0 "Sun PCIO EBus2" rev 0x01
> clock1 at ebus0 addr 2000-3fff: mk48t59
> "power" at ebus0 addr 7240-7243 ivec 0x1 not configured
> "fdthree" at ebus0 addr 0- not configured
> com0 at ebus0 addr 3f8-3ff ivec 0x2b: ns16550a, 16 byte fifo
> com0: console
> pckbc0 at ebus0 addr 60-67 ivec 0x29
> pckbd0 at pckbc0 (kbd slot)
> wskbd0 at pckbd0
> "Bochs VGA" rev 0x02 at pci1 dev 2 function 0 not configured
> pciide0 at pci1 dev 3 function 0 "CMD Technology PCI0646" rev 0x07:
> DMA, channel 0 configured to native-PCI, channel 1 configured to
> native-PCI
> pciide0: using ivec 0x7e0 for native-PCI interrupt
> wd0 at pciide0 channel 0 drive 0: 
> wd0: 16-sector PIO, LBA48, 20480MB, 41943040 sectors
> wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2
> atapiscsi0 at pciide0 channel 1 drive 0
> scsibus0 at atapiscsi0: 2 targets
> cd0 at scsibus0 targ 0 lun 0:  removable
> cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2
> ppb1 at pci0 dev 1 function 0 "Sun Simba" rev 0x11
> pci2 at ppb1 bus 2
> ne0 at pci2 dev 0 function 0 "Realtek 8029" rev 0x00: ivec 0x7d0,
> address 52:54:00:12:34:56
> softraid0 at root
> scsibus1 at softraid0: 256 targets
> bootpath: /pci@1fe,0/pci@1,1/ide@3,0/ide@0,0/disk@0,0
> root on rd0a swap on rd0b dump on rd0b
> WARNING: CHECK AND RESET THE DATE!
> erase ^?, werase ^W, kill ^U, intr ^C, status ^T
>
> Welcome to the OpenBSD/sparc64 6.7 installation program.
> (I)nstall, (U)pgrade, (A)utoinstall or (S)hell?

Actually, it looks like the `disk debug -V` trick works for 6.7 but
fails for -current:

snapshots/miniroot67.img

Loading FCode image...
Loaded 6882 bytes
entry point is 0x4000
Evaluating FCode...
OpenBSD IEEE 1275 Bootblock 2.0
Booting from device /pci@1fe,0/pci@1,1/ide@3/ide@0/disk@0
Try superblock read
FFS v1
ufs-open complete
.Looking for ofwboot in directory...
.
..
ofwboot
Found it
.Loading 1a0e0  bytes of file...
Copying 2000 bytes to 4000
Copying 2000 bytes to 6000
Copying 2000 bytes to 8000
Copying 2000 bytes to a000
Copying 2000 bytes to c000
Copying 2000 bytes to e000
Copying 2000 bytes to 1
Copying 2000 bytes to 12000
Copying 2000 bytes to 14000
Copying 2000 bytes to 16000
Copying 2000 

Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Bob Beck
On Fri, May 29, 2020 at 06:14:44PM +0200, Marc Espie wrote:
> In a trace:
> 
> > > > #3  0x15e48c95459e in WebVfx::shutdown ()
> > > >  at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193
> 
> Now, this is NOT the default location for WRKOBJDIR, but we are shipping
> packages with debug information...
> 
> This could be a bit cumbersome, if in order to debug locally, you have
> to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
> "official" setup for ports, which does not even match the default.
> 
> There are about 3 solutions to that:
> - change the bulk machines to /usr/ports/pobj
> - change the ports default to /usr/obj/ports

It's not realpathing is it? correct me if I am wrong but would
making /usr/ports/pobj a symlink to /usr/obj/ports on the bulk 
machines and running them with the default config just "fix" this? 

> - have some magic I don't know in ELF handling that would allow to either
> tweak the default location/introduce ${WRKOBJDIR} in that debug info.
> 



Re: sparc64 boot issue on qemu

2020-05-29 Thread Jason A. Donenfeld
Note that you need to run this with `-nographic`, because the kernel
crashes when trying to use vgafb on sparc64/qemu. I've witnessed two
varieties crashes:

- https://data.zx2c4.com/openbsd-6.7-sparc64-vga-panic-miniroot67.png
This happens when booting up miniroot67.fs

- https://data.zx2c4.com/openbsd-6.7-sparc64-vga-panic-after-installation.png
This happens after installation openbsd onto disk properly, and then
booting up into it.

Passing `-nographic` prevents these from happening, since vgafb doesn't
bind to anything.

I don't have a bsd.gdb in order to addr2line this, but if the miniroot
panic is related to the normal panic, and we then assume alignment
issues in fb_get_console_metrics, then I wonder if the below patch would
make a difference. On the other hand, a "data access fault" makes it
seem more likely that OF_interpret is just getting bogus addresses from
buggy qemu firmware.

I probably have another two hours to go in waiting for this thing to
build...

Jason

--- a/sys/arch/sparc64/dev/fb.c
+++ b/sys/arch/sparc64/dev/fb.c
@@ -507,6 +507,7 @@ int
 fb_get_console_metrics(int *fontwidth, int *fontheight, int *wtop, int *wleft)
 {
cell_t romheight, romwidth, windowtop, windowleft;
+   uint64_t romheight_64, romwidth_64, windowtop_64, windowleft_64;

/*
 * Get the PROM font metrics and address
@@ -520,10 +521,15 @@ fb_get_console_metrics(int *fontwidth, int *fontheight, 
int *wtop, int *wleft)
windowtop == 0 || windowleft == 0)
return (1);

-   *fontwidth = (int)*(uint64_t *)romwidth;
-   *fontheight = (int)*(uint64_t *)romheight;
-   *wtop = (int)*(uint64_t *)windowtop;
-   *wleft = (int)*(uint64_t *)windowleft;
+   memcpy(_64, (void *)romheight, sizeof(romheight_64));
+   memcpy(_64, (void *)romwidth, sizeof(romwidth_64));
+   memcpy(_64, (void *)windowtop, sizeof(windowtop_64));
+   memcpy(_64, (void *)windowleft, sizeof(windowleft_64));
+
+   *fontwidth = (int)romwidth_64;
+   *fontheight = (int)romheight_64;
+   *wtop = (int)windowtop_64;
+   *wleft = (int)windowleft_64;

return (0);
 }



Re: sparc64 boot issue on qemu

2020-05-29 Thread Jason A. Donenfeld
On Fri, May 29, 2020 at 4:56 PM Jason A. Donenfeld  wrote:
>
> Oh that's a nice observation about `boot disk -V`. Doing so actually
> got me booting up entirely:
>
> $ qemu-img convert -O qcow2 miniroot66.fs disk.qcow2

Er, copy and paste error. The below is actually from miniroot67.fs.



Re: sparc64 boot issue on qemu

2020-05-29 Thread Jason A. Donenfeld
Oh that's a nice observation about `boot disk -V`. Doing so actually
got me booting up entirely:

$ qemu-img convert -O qcow2 miniroot66.fs disk.qcow2
$ qemu-img resize disk.qcow2 20G
$ qemu-system-sparc64 -m 1024 -drive file=disk.qcow2,if=ide -net
nic,model=ne2k_pci -net user -boot a -nographic -monitor none -serial
stdio
OpenBIOS for Sparc64
Configuration device id QEMU version 1 machine id 0
kernel cmdline
CPUs: 1 x SUNW,UltraSPARC-IIi
UUID: ----
Welcome to OpenBIOS v1.1 built on Oct 28 2019 17:08
  Type 'help' for detailed information
Trying /obio/SUNW,fdtwo...
No valid state has been set by load or init-program

0 > boot disk -V Not a Linux kernel image
Not a bootable ELF image
Not a bootable a.out image

Loading FCode image...
Loaded 6882 bytes
entry point is 0x4000
Evaluating FCode...
OpenBSD IEEE 1275 Bootblock 2.0
Booting from device /pci@1fe,0/pci@1,1/ide@3/ide@0/disk@0
Try superblock read
FFS v1
ufs-open complete
.Looking for ofwboot in directory...
.
..
ofwboot
Found it
.Loading 1a0e0  bytes of file...
Copying 2000 bytes to 4000
Copying 2000 bytes to 6000
Copying 2000 bytes to 8000
Copying 2000 bytes to a000
Copying 2000 bytes to c000
Copying 2000 bytes to e000
Copying 2000 bytes to 1
Copying 2000 bytes to 12000
Copying 2000 bytes to 14000
Copying 2000 bytes to 16000
Copying 2000 bytes to 18000
Copying 2000 bytes to 1a000
Copying 2000 bytes to 1c000
Copying 2000 bytes to 1e000
reserved fcode word.
>> OpenBSD BOOT 1.17
Trying bsd...
open /etc/random.seed: No such file or directory
Booting /pci@1fe,0/pci@1,1/ide@3/ide@0/disk@0/bsd
4211776@0x100+7104@0x1404440+3247928@0x1c0+946376@0x1f18f38
symbols @ 0xfef50340 139 start=0x100
console is /pci@1fe,0/pci@1,1/ebus@1/su
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 (RAMDISK) #298: Thu May  7 19:11:18 MDT 2020
dera...@sparc64.openbsd.org:/usr/src/sys/arch/sparc64/compile/RAMDISK
real mem = 1073741824 (1024MB)
avail mem = 1044135936 (995MB)
unknown option `V'
mainbus0 at root: OpenBiosTeam,OpenBIOS
cpu0 at mainbus0: SUNW,UltraSPARC-IIi (rev 9.1) @ 100 MHz
cpu0: physical 256K instruction (64 b/l), 16K data (32 b/l), 256K
external (64 b/l)
psycho0 at mainbus0: SUNW,sabre, impl 0, version 0, ign 7c0
psycho0: bus range 0-2, PCI bus 0
psycho0: dvma map c000-dfff
pci0 at psycho0
ppb0 at pci0 dev 1 function 1 "Sun Simba" rev 0x11
pci1 at ppb0 bus 1
ebus0 at pci1 dev 1 function 0 "Sun PCIO EBus2" rev 0x01
clock1 at ebus0 addr 2000-3fff: mk48t59
"power" at ebus0 addr 7240-7243 ivec 0x1 not configured
"fdthree" at ebus0 addr 0- not configured
com0 at ebus0 addr 3f8-3ff ivec 0x2b: ns16550a, 16 byte fifo
com0: console
pckbc0 at ebus0 addr 60-67 ivec 0x29
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0
"Bochs VGA" rev 0x02 at pci1 dev 2 function 0 not configured
pciide0 at pci1 dev 3 function 0 "CMD Technology PCI0646" rev 0x07:
DMA, channel 0 configured to native-PCI, channel 1 configured to
native-PCI
pciide0: using ivec 0x7e0 for native-PCI interrupt
wd0 at pciide0 channel 0 drive 0: 
wd0: 16-sector PIO, LBA48, 20480MB, 41943040 sectors
wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2
atapiscsi0 at pciide0 channel 1 drive 0
scsibus0 at atapiscsi0: 2 targets
cd0 at scsibus0 targ 0 lun 0:  removable
cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2
ppb1 at pci0 dev 1 function 0 "Sun Simba" rev 0x11
pci2 at ppb1 bus 2
ne0 at pci2 dev 0 function 0 "Realtek 8029" rev 0x00: ivec 0x7d0,
address 52:54:00:12:34:56
softraid0 at root
scsibus1 at softraid0: 256 targets
bootpath: /pci@1fe,0/pci@1,1/ide@3,0/ide@0,0/disk@0,0
root on rd0a swap on rd0b dump on rd0b
WARNING: CHECK AND RESET THE DATE!
erase ^?, werase ^W, kill ^U, intr ^C, status ^T

Welcome to the OpenBSD/sparc64 6.7 installation program.
(I)nstall, (U)pgrade, (A)utoinstall or (S)hell?



Re: Marvell 88SE9215 controller

2020-05-29 Thread Damien Couderc

Le 24/04/2020 à 11:31, Damien Couderc a écrit :

Hi guys,

The following diff is adding the Marvell 88SE9215 SATA controller to 
the PCI devices.


Hi,

Nobody to look at this?




===

--- pcidevs.orig    Thu Apr 23 11:44:53 2020
+++ pcidevs Thu Apr 23 14:15:18 2020
@@ -5922,6 +5922,7 @@
 product MARVELL2 88SE9125  0x9125  88SE9125 SATA
 product MARVELL2 88SE9128  0x9128  88SE9128 AHCI
 product MARVELL2 88SE9172  0x9172  88SE9172 SATA
+product MARVELL2 88SE9215  0x9215  88SE9215 AHCI
 product MARVELL2 88SE9230  0x9230  88SE9230 AHCI

 /* Matrox products */

===

Resulting dmesg is following. Both files can also be found attached to 
this mail.



OpenBSD 6.7-beta (GENERIC.MP) #1: Fri Apr 24 11:17:17 CEST 2020
damien@current.petrolan:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 34271739904 (32684MB)
avail mem = 33220456448 (31681MB)
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.0 @ 0xe94f0 (31 entries)
bios0: vendor American Megatrends Inc. version "P1.80" date 12/18/2018
bios0: ASRock B450 Gaming K4
acpi0 at bios0: ACPI 6.0
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP APIC FPDT FIDT SSDT SSDT CRAT CDIT SSDT MCFG 
AAFT HPET SSDT UEFI IVRS SSDT SSDT WSMT
acpi0: wakeup devices GPP0(S4) GPP1(S4) GPP3(S4) GPP4(S4) GPP5(S4) 
GPP6(S4) GPP7(S4) GPP8(S4) GPP9(S4) GPPA(S4) GPPB(S4) GPPC(S4) 
GPPD(S4) GPPE(S4) GPPF(S4) GP17(S4) [...]

acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 2700 Eight-Core Processor, 3194.62 MHz, 17-08-02
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,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,SHA,IBPB,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 64KB 64b/line 4-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 
64b/line 8-way L2 cache, 16MB 64b/line 16-way L3 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully 
associative
cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully 
associative

cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 99MHz
cpu0: mwait min=64, max=64, C-substates=1.1, IBE
cpu1 at mainbus0: apid 1 (application processor)
cpu1: AMD Ryzen 7 2700 Eight-Core Processor, 3194.00 MHz, 17-08-02
cpu1: 
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,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,SHA,IBPB,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu1: 64KB 64b/line 4-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 
64b/line 8-way L2 cache, 16MB 64b/line 16-way L3 cache
cpu1: ITLB 64 4KB entries fully associative, 64 4MB entries fully 
associative
cpu1: DTLB 64 4KB entries fully associative, 64 4MB entries fully 
associative

cpu1: smt 1, core 0, package 0
cpu2 at mainbus0: apid 2 (application processor)
cpu2: AMD Ryzen 7 2700 Eight-Core Processor, 3194.00 MHz, 17-08-02
cpu2: 
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,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,SHA,IBPB,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu2: 64KB 64b/line 4-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 
64b/line 8-way L2 cache, 16MB 64b/line 16-way L3 cache
cpu2: ITLB 64 4KB entries fully associative, 64 4MB entries fully 
associative
cpu2: DTLB 64 4KB entries fully associative, 64 4MB entries fully 
associative

cpu2: smt 0, core 1, package 0
cpu3 at mainbus0: apid 3 (application processor)
cpu3: AMD Ryzen 7 2700 Eight-Core Processor, 3194.00 MHz, 17-08-02
cpu3: 
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,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,RDSEED,ADX,SMAP,CLFLUSHOPT,SHA,IBPB,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu3: 64KB 64b/line 4-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 
64b/line 

Re: Ncurses: issue with "rep" capability and --enable-bsdpad (BSD_TPUTS)

2020-05-29 Thread Nicholas Marriott
Hi

I have applied this now, thanks!

> Is there a specific reason why OpenBSD still uses an old ncurses-based
> version?

Updating ncurses in OpenBSD takes quite a lot of time because it is big
and quite integrated - so for example the file layout is where OpenBSD
would put stuff not where upstream does, there are bits in various
places (libcurses, libform, libpanel, libmenu, tic, infocmp, tset,
terminfo.src), quite a lot of local changes (many of them cosmetic), the
build process has a lot of scripts and fiddly stuff. Plus it is used by
a lot of programs both in base and ports, and kind of has to be done all
in one go.

But we could definitely do with an upgrade now... I've started it a
couple of times but never had time to finish. So if you or anyone wants
to try to do it, let me know :-).

Next time it would be nice to see if we could streamline and document it
the upgrade process so we don't end up so far behind so often.



On Mon, May 25, 2020 at 07:02:00PM +0200, Hiltjo Posthuma wrote:
> Hi,
> 
> I've noticed an issue when testing the "rep" capability to st (terminal
> emulator) on OpenBSD.
> 
> After some digging I found the issue is when BSD_TPUTS is defined
> (ncurses_cfg.h).  This enables code for BSD prefix padding and a workaround 
> for
> nethack and ancient BSD terminals as the C comment says in the file
> tinfo/lib_tputs.c.
> 
> I noticed while using the lynx browser, it will incorrectly print repeated
> digit characters. For example:
> 
>   echo "Z000" | lynx -stdin
> 
> will print: ""
> 
> 
> To reproduce it:
> 
> * Use a terminal with the "rep" capability supported and exposed:
>   rep=%p1%c\E[%p2%{1}%-%db
> 
>   infocmp | grep rep=
> 
>   The sequence is documented in terminfo(5).
> 
> * Compile ncurses with BSD_TPUTS defined (or ./configure --enable-bsdpad on 
> Linux).
>   BSD_TPUTS is set by default on OpenBSD in lib/libcurses/ncurses_cfg.h
> 
> I could reproduce the issue on Void Linux also on the latest ncurses snapshot,
> but a fix has been sent upstream and added in 20200523:
> 
> https://github.com/ThomasDickey/ncurses-snapshots/blob/master/NEWS
> 
> "+ add a check in EmitRange to guard against repeat_char emitting digits which
>   could be interpreted as BSD-style padding when --enable-bsdpad is configured
>   (report/patch by Hiltjo Posthuma)."
> 
> 
> A small program to reproduce it:
> 
> #include 
> #include 
> #include 
> 
> int
> main(void)
> {
>   WINDOW *win;
>   
>   win = initscr();
>   printw("Z000");
> 
>   refresh();
>   
>   sleep(5);
> 
>   return 0;
> }
> 
> This program will incorrectly print "" also.
> 
> The backported below patch from ncurses snapshot 20200523 to -current fixes 
> it:
> 
> 
> diff --git a/lib/libcurses/tty/tty_update.c b/lib/libcurses/tty/tty_update.c
> index bd2f088adf6..3d10b76cce2 100644
> --- a/lib/libcurses/tty/tty_update.c
> +++ b/lib/libcurses/tty/tty_update.c
> @@ -540,7 +540,11 @@ EmitRange(const NCURSES_CH_T * ntext, int num)
>   } else {
>   return 1;   /* cursor stays in the middle */
>   }
> - } else if (repeat_char && runcount > SP->_rep_cost) {
> + } else if (repeat_char &&
> +#if BSD_TPUTS
> +   !isdigit(UChar(CharOf(ntext0))) &&
> +#endif
> +   runcount > SP->_rep_cost) {
>   bool wrap_possible = (SP->_curscol + runcount >= 
> screen_columns);
>   int rep_count = runcount;
>  
> 
> Is there a specific reason why OpenBSD still uses an old ncurses-based 
> version?
> 
> -- 
> Kind regards,
> Hiltjo
> 



Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Christian Weisgerber
Marc Espie:

> There are about 3 solutions to that:
> - change the bulk machines to /usr/ports/pobj
> - change the ports default to /usr/obj/ports

I don't think it's reasonable to expect everybody to use the same
path here.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: unlock PF_UNIX sockets

2020-05-29 Thread Vitaliy Makkoveev
On Fri, May 29, 2020 at 08:48:14AM +0200, Martin Pieuchot wrote:
> On 28/05/20(Thu) 14:59, Vitaliy Makkoveev wrote:
> > socket(2) layer is already protected by solock(). It grabs NET_LOCK()
> > for inet{,6}(4) sockets, but all other sockets are still under
> > KERNEL_LOCK().
> > 
> > I guess solock is already placed everythere it's required. Also `struct
> > file' is already mp-safe. 
> > 
> > Diff below introduces rwlock `unp_lock'. It's like NET_LOCK()'s
> > `netlock' but for PF_UNIX sockets. This lock protects the whole PF_UNIX
> > layer. Vnode layer accessed in "connect" "bind" and "desconnect" cases
> > is still under KERNEL_LOCK().
> > 
> > I'am experimenting with his diff since 19.04.2020 and my test systems,
> > include Gnome workstaion are stable, without any issue.
> 
> Looks great, more tests are required :)
> 
> Your diff has many trailing spaces, could you remove them?  Commonly
> used editors or diff programs have a way to highlight them :)

Ok I will fix it.

> 
> One comment:
> 
> > Index: sys/kern/uipc_usrreq.c
> > ===
> > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> > retrieving revision 1.142
> > diff -u -p -r1.142 uipc_usrreq.c
> > --- sys/kern/uipc_usrreq.c  16 Jul 2019 21:41:37 -  1.142
> > +++ sys/kern/uipc_usrreq.c  1 May 2020 13:47:11 -
> > @@ -409,12 +418,17 @@ unp_detach(struct unpcb *unp)
> >  {
> > struct vnode *vp;
> >  
> > +   rw_assert_wrlock(_lock);
> > +
> > LIST_REMOVE(unp, unp_link);
> > if (unp->unp_vnode) {
> > +   /* this is an unlocked cleaning of `v_socket', but it's safe */
> > unp->unp_vnode->v_socket = NULL;
> > vp = unp->unp_vnode;
> > unp->unp_vnode = NULL;
> > +   KERNEL_LOCK();
> > vrele(vp);
> > +   KERNEL_UNLOCK();
> 
> Why is it safe?  That's what the comment should explain :)  If the code
> doesn't take the lock that implies it is not required.  Why it is not
> required is not obvious.
>

The only place where `v_socket' is accessed outside PF_UNIX layer is
kern/kern_sysctl.c:1143. This is just pointer to integer cast. Pointer
assignment is atomic.

Also we are accessing this `unp' in unp_detach(), unp_bind() and
unp_connect() but they are serialized by `unp_lock'.

This is the last reference to this `unp' and also there is the only
reference in socket layer to `unp_vnode'. Since `unpcb' has no reference
counter we can't add assertion to be sure this is the last reference to
`unp'. We can check reference count of vnode. But since sys_open() and
unp_detach() are not serialized by KERNEL_LOCK(), vn_open() called by
doopenat() will raise `v_usecount' before `v_type' check
(kern/vfs_vnops.c:112). And `vn->v_usecount' in this unp_detach() call
can be "2".

In fact, only vrele(9) should be called under KERNEL_LOCK() to protect
vnode. If we want to put "KASSERT(vn->v_usecount==1)" we should move it
under KERNEL_LOCK() before `v_socket' cleaning to be sure there is no
concurency with sys_open().

Something like this:

if (unp->unp_vnode) {
KERNEL_LOCK();
KASSERT(unp->unp_vnode->v_usecount == 1);
unp->unp_vnode->v_socket = NULL;
vp = unp->unp_vnode;
unp->unp_vnode = NULL;
vrele(vp);
KERNEL_UNLOCK();
}

I will update diff if this is required.



Re: official ports vs DEBUG_PACKAGES

2020-05-29 Thread Stuart Henderson
On 2020/05/29 18:14, Marc Espie wrote:
> In a trace:
> 
> > > > #3  0x15e48c95459e in WebVfx::shutdown ()
> > > >  at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193
> 
> Now, this is NOT the default location for WRKOBJDIR, but we are shipping
> packages with debug information...
> 
> This could be a bit cumbersome, if in order to debug locally, you have
> to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
> "official" setup for ports, which does not even match the default.
> 
> There are about 3 solutions to that:
> - change the bulk machines to /usr/ports/pobj

That often won't match the layout for users either, disklabel auto
defaults push them into using something they've come up with (I have seen
quite a few emails with things like /home/ports or /home/user/ports).

> - change the ports default to /usr/obj/ports
> 
> - have some magic I don't know in ELF handling that would allow to either
> tweak the default location/introduce ${WRKOBJDIR} in that debug info.

Something like /pobj would work for me, it makes sense to mount a
separate filesystem directly there on a dedicated bulk build machine,
for non-dedicated machines it's easier to symlink to the real path than
something a few levels deep.



official ports vs DEBUG_PACKAGES

2020-05-29 Thread Marc Espie
In a trace:

> > > #3  0x15e48c95459e in WebVfx::shutdown ()
> > >  at /usr/obj/ports/webvfx-1.2.0/webvfx-1.2.0/webvfx/webvfx.cpp:193

Now, this is NOT the default location for WRKOBJDIR, but we are shipping
packages with debug information...

This could be a bit cumbersome, if in order to debug locally, you have
to make patch NO_DEPENDS=Yes, and then you figure out you have to mimic the
"official" setup for ports, which does not even match the default.

There are about 3 solutions to that:
- change the bulk machines to /usr/ports/pobj
- change the ports default to /usr/obj/ports
- have some magic I don't know in ELF handling that would allow to either
tweak the default location/introduce ${WRKOBJDIR} in that debug info.



Re: pipex(4): kill NET_LOCK() in pipex_ioctl()

2020-05-29 Thread Vitaliy Makkoveev
On Fri, May 29, 2020 at 09:09:22AM +0200, Martin Pieuchot wrote:
> On 28/05/20(Thu) 15:27, Vitaliy Makkoveev wrote:
> > On Thu, May 28, 2020 at 12:26:39PM +0200, Martin Pieuchot wrote:
> > > On 27/05/20(Wed) 11:54, Vitaliy Makkoveev wrote:
> > > > pipex(4) is simultaneously protected by KERNEL_LOCK() and NET_LOCK() and
> > > > the last is not required. I guess to start remove NET_LOCK(). Diff below
> > > > drops NET_LOCK() in pipex_ioctl() and underlaying routines. At least
> > > > this helps to kill unlock/lock mess in pppx_add_session() and
> > > > pppx_if_destroy().
> > > 
> > > Getting rid of the NET_LOCK() might indeed help to untangle the current
> > > locking mess.  However we should aim towards removing the KERNEL_LOCK()
> > > from, at least, the packet processing path starting with pipexintr().
> > 
> > I guess we can made `pipexoutq' processing NET_LOCK() free. Also
> > `pipexinq' processing requires a little amount of places where NET_LOCK()
> > is required. So we can implement special locks for pipex(4).
> 
> After second though, it seems that the easiest way forward is to protect
> `pipex_session_list' by the NET_LOCK().
> 
> The rational is that this global list is dereferenced in pipexintr() and
> its members are compared to the content of `ph_cookie'.
> 
> There's currently no mechanism to ensure the reference saved in `ph_cookie'
> is still valid.  That means what ensures the pointer is kind-of correct
> is the NET_LOCK().  I'm saying "kind-of" because comparing pointers is
> questionable, especially if sessions can be destroyed without purging
> `pipexoutq' or `pipexinq'. 
> 
> Since the KERNEL_LOCK() is not always held when the network stack runs,
> `ph_cookie' can be modified by a CPU without holding it.  So what
> actually protects the data structures here is the NET_LOCK().
> 
> Does that make sense?
> 

This time nothing protects `ph_cookie'. I already pointed this issue.

pipex_ppp_enqueue() (net/pipex.c:737) is the only place where we set
`ph_cookie'. But between pipex_ppp_enqueue() and pipexintr() calls we do
context switch where we can kill session.

There are some cases how we can do it:

1. by pipex_ioctl() with PIPEXDSESSION command.
We call pipex_close_session(), it sets sessions state to
PIPEX_STATE_CLOSED and it's all. pipex_timer() will delete this session
by pipex_destroy_session() but only if `pipexinq' and `pipexoutq' are
empty. This check is required to be sure there is no `mbuf' with
`ph_cookie' pointed to destroyed session. This is the only safe way to
destroy session and the only pppac(4) do this but not always.

2. by pipex_iface_stop().
We travel through all sessions and call pipex_destroy_session() with
our sessions. We don't check `pipexinq' and `pipexoutq' state. So,
hypothetically, we can have `mbuf' with `ph_cookie' pointed to session
we killing and this is potencial use-after-free. pppac(4) do this by
pppacclose() which calls pipex_iface_fini() which calls
pipex_iface_stop(). Hypothetically, npppd(8) can be killed while it
has a lot of active connections and some of them can be referenced by
`pipexinq' or `pipexoutq'.

I guess in normal shutdown npppd(8) does pipex_ioctl() with
PIPEXDSESSION command as described in (1) before close correspondind
/dev/pppac and there is no sessions while we do pppacclose().

3. by pppx_if_destroy().
pppx_if has reference to it's own pipex(4) session and this reference
will be killed by pppx_if_destroy() without `pipexinq' and `pipexoutq'
check. Also potential use-after-free. And this is the only way how
pppx(4) can destroy session.

We always setting (and destroying) and procesing `ph_cookie' in different
threads. So we will always drop lock which protects the whole pipex(4)
layer.

I guess the only way to be sure the data referenced by `ph_cookie' is
still valid is to implement reference counters to pipex(4) session and
rework pipex(4) session destruction.



Re: userland clock_gettime proof of concept

2020-05-29 Thread Paul Irofti
On Fri, May 29, 2020 at 03:00:50PM +0200, Mark Kettenis wrote:
> > Date: Fri, 29 May 2020 13:45:37 +0100
> > From: Stuart Henderson 
> > 
> > On 2020/05/29 13:50, Paul Irofti wrote:
> > > +struct __timekeep {
> > > + uint32_t major; /* version major number */
> > > + uint32_t minor; /* version minor number */
> > > +
> > > + u_int64_t   th_scale;
> > > + unsigned intth_offset_count;
> > > + struct bintime  th_offset;
> > > + struct bintime  th_naptime;
> > > + struct bintime  th_boottime;
> > > + volatile unsigned int   th_generation;
> > > +
> > > + unsigned inttc_user;
> > > + unsigned inttc_counter_mask;
> > > +};
> > 
> > Ah good, you got rid of u_int, that was causing problems with port builds.
> 
> That in itself is a problem.  This means  is the wrong place
> for this struct.  We need to find a better place for this.
> 
> Since this is now closely linked to the timecounter stuff
>  would be an obvious place.  Now that file has:
> 
> #ifndef _KERNEL
> #error "no user-serviceable parts inside"
> #endif
> 
> you could change that into
> 
> #if !defined(_KERNEL) && !defined(_LIBC)
> #error "no user-serviceable parts inside"
> #endif
> 
> and make sure you #define _LIBC brefore uncluding this file where it
> is needed.  As few places as possible obviously.

Done. Also includes claudio@'s observation.


diff --git lib/libc/arch/amd64/gen/Makefile.inc 
lib/libc/arch/amd64/gen/Makefile.inc
index e995309ed71..caa4452a3d9 100644
--- lib/libc/arch/amd64/gen/Makefile.inc
+++ lib/libc/arch/amd64/gen/Makefile.inc
@@ -2,6 +2,6 @@
 
 SRCS+= _setjmp.S fabs.S infinity.c ldexp.c modf.S nan.c setjmp.S \
sigsetjmp.S
-SRCS+= fpclassifyl.c isfinitel.c isinfl.c isnanl.c isnormall.c signbitl.c
+SRCS+= fpclassifyl.c rdtsc.c isfinitel.c isinfl.c isnanl.c isnormall.c 
signbitl.c
 SRCS+= flt_rounds.S fpgetmask.S fpgetround.S fpgetsticky.S fpsetmask.S \
fpsetround.S fpsetsticky.S
diff --git lib/libc/arch/amd64/gen/rdtsc.c lib/libc/arch/amd64/gen/rdtsc.c
new file mode 100644
index 000..b14c862c61a
--- /dev/null
+++ lib/libc/arch/amd64/gen/rdtsc.c
@@ -0,0 +1,26 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2020 Paul Irofti 
+ *
+ * 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 
+
+uint64_t
+tc_get_timecount_md(void)
+{
+   uint32_t hi, lo;
+   asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
+   return ((uint64_t)lo)|(((uint64_t)hi)<<32);
+}
diff --git lib/libc/asr/asr.c lib/libc/asr/asr.c
index cd056c85719..2b25d49f32a 100644
--- lib/libc/asr/asr.c
+++ lib/libc/asr/asr.c
@@ -196,11 +196,11 @@ poll_intrsafe(struct pollfd *fds, nfds_t nfds, int 
timeout)
struct timespec pollstart, pollend, elapsed;
int r;
 
-   if (clock_gettime(CLOCK_MONOTONIC, ))
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, ))
return -1;
 
while ((r = poll(fds, 1, timeout)) == -1 && errno == EINTR) {
-   if (clock_gettime(CLOCK_MONOTONIC, ))
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, ))
return -1;
timespecsub(, , );
timeout -= elapsed.tv_sec * 1000 + elapsed.tv_nsec / 100;
@@ -418,7 +418,7 @@ asr_check_reload(struct asr *asr)
asr->a_rtime = 0;
}
 
-   if (clock_gettime(CLOCK_MONOTONIC, ) == -1)
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, ) == -1)
return;
 
if ((ts.tv_sec - asr->a_rtime) < RELOAD_DELAY && asr->a_rtime != 0)
diff --git lib/libc/crypt/bcrypt.c lib/libc/crypt/bcrypt.c
index 82de8fa33b7..02fd3013cc1 100644
--- lib/libc/crypt/bcrypt.c
+++ lib/libc/crypt/bcrypt.c
@@ -248,9 +248,9 @@ _bcrypt_autorounds(void)
char buf[_PASSWORD_LEN];
int duration;
 
-   clock_gettime(CLOCK_THREAD_CPUTIME_ID, );
+   WRAP(clock_gettime)(CLOCK_THREAD_CPUTIME_ID, );
bcrypt_newhash("testpassword", r, buf, sizeof(buf));
-   clock_gettime(CLOCK_THREAD_CPUTIME_ID, );
+   WRAP(clock_gettime)(CLOCK_THREAD_CPUTIME_ID, );
 
duration = after.tv_sec - before.tv_sec;
duration *= 100;
diff --git lib/libc/dlfcn/init.c lib/libc/dlfcn/init.c
index 270f54aada5..1ff97c12b7b 100644
--- lib/libc/dlfcn/init.c
+++ lib/libc/dlfcn/init.c
@@ 

Re: userland clock_gettime proof of concept

2020-05-29 Thread Claudio Jeker
On Fri, May 29, 2020 at 01:45:37PM +0100, Stuart Henderson wrote:
> On 2020/05/29 13:50, Paul Irofti wrote:
> > +struct __timekeep {
> > +   uint32_t major; /* version major number */
> > +   uint32_t minor; /* version minor number */
> > +
> > +   u_int64_t   th_scale;
> > +   unsigned intth_offset_count;
> > +   struct bintime  th_offset;
> > +   struct bintime  th_naptime;
> > +   struct bintime  th_boottime;
> > +   volatile unsigned int   th_generation;
> > +
> > +   unsigned inttc_user;
> > +   unsigned inttc_counter_mask;
> > +};
> 
> Ah good, you got rid of u_int, that was causing problems with port builds.

Probably the u_int64_t should also be changed to uint64_t. At least I
think code should not mix u_intXY_t and uintXY_t.

-- 
:wq Claudio



pppx(4): rework to be clese to pseudo-interfaces

2020-05-29 Thread Vitaliy Makkoveev
This time pppx_add_session() has mixed initialisation order. It starts
to initialise pipex(4) session, then initialises `ifnet', then links
pipex(4) session, then continue to initialize `ifnet'.
pppx_add_session() can sleep and pppx_if_start() can start to work with
unlinked pipex(4) session.

Also pppx_if_destroy() can sleep and pppx_if_start() can access to this
`pxi' with unlinked session. pppx_if_start() has checking of
`IFF_RUNNING' flag but it's usesless.

Diff below does pppx_add_session() reordering. At first we initilize
and attach `ifnet', at second we initialize and link pipex(4) session
and at last we set `IFF_RUNNING' flag on `ifnet.

Also we cleaning `IFF_RUNNING' flag before pppx(4) session destruction
in pppx_if_destroy().

Since we made `ifnet' and pipex(4) session initialization separately, we
are more close to remove duplicated code.


Index: sys/net/if_pppx.c
===
RCS file: /cvs/src/sys/net/if_pppx.c,v
retrieving revision 1.87
diff -u -p -r1.87 if_pppx.c
--- sys/net/if_pppx.c   29 May 2020 06:51:52 -  1.87
+++ sys/net/if_pppx.c   29 May 2020 09:27:47 -
@@ -674,16 +674,111 @@ pppx_add_session(struct pppx_dev *pxd, s
 *  the timeout feature until this is fixed.
 */
if (req->pr_timeout_sec != 0)
-   return (EINVAL);
+   return EINVAL;
+
+   pxi = pool_get(pppx_if_pl, PR_WAITOK | PR_ZERO);
+
+   /* try to set the interface up */
+   unit = pppx_if_next_unit();
+   if (unit < 0) {
+   error = ENOMEM;
+   goto out;
+   }
+
+   pxi->pxi_unit = unit;
+   pxi->pxi_key.pxik_session_id = req->pr_session_id;
+   pxi->pxi_key.pxik_protocol = req->pr_protocol;
+   pxi->pxi_dev = pxd;
+
+   /* this is safe without splnet since we're not modifying it */
+   if (RBT_FIND(pppx_ifs, _ifs, pxi) != NULL) {
+   error = EADDRINUSE;
+   goto out;
+   }
+
+   if (RBT_INSERT(pppx_ifs, _ifs, pxi) != NULL)
+   panic("%s: pppx_ifs modified while lock was held", __func__);
+   LIST_INSERT_HEAD(>pxd_pxis, pxi, pxi_list);
+
+   ifp = >pxi_if;
+   snprintf(ifp->if_xname, sizeof(ifp->if_xname), "%s%d", "pppx", unit);
+   ifp->if_mtu = req->pr_peer_mru; /* XXX */
+   ifp->if_flags = IFF_POINTOPOINT | IFF_MULTICAST | IFF_UP;
+   ifp->if_start = pppx_if_start;
+   ifp->if_output = pppx_if_output;
+   ifp->if_ioctl = pppx_if_ioctl;
+   ifp->if_rtrequest = p2p_rtrequest;
+   ifp->if_type = IFT_PPP;
+   IFQ_SET_MAXLEN(>if_snd, 1);
+   ifp->if_softc = pxi;
+   /* ifp->if_rdomain = req->pr_rdomain; */
+
+   /* XXXSMP breaks atomicity */
+   NET_UNLOCK();
+   if_attach(ifp);
+   NET_LOCK();
+
+   if_addgroup(ifp, "pppx");
+   if_alloc_sadl(ifp);
+
+#if NBPFILTER > 0
+   bpfattach(>if_bpf, ifp, DLT_LOOP, sizeof(u_int32_t));
+#endif
+
+   /* XXX ipv6 support?  how does the caller indicate it wants ipv6
+* instead of ipv4?
+*/
+   memset(, 0, sizeof(ifaddr));
+   ifaddr.sin_family = AF_INET;
+   ifaddr.sin_len = sizeof(ifaddr);
+   ifaddr.sin_addr = req->pr_ip_srcaddr;
+
+   ia = malloc(sizeof (*ia), M_IFADDR, M_WAITOK | M_ZERO);
+
+   ia->ia_addr.sin_family = AF_INET;
+   ia->ia_addr.sin_len = sizeof(struct sockaddr_in);
+   ia->ia_addr.sin_addr = req->pr_ip_srcaddr;
+
+   ia->ia_dstaddr.sin_family = AF_INET;
+   ia->ia_dstaddr.sin_len = sizeof(struct sockaddr_in);
+   ia->ia_dstaddr.sin_addr = req->pr_ip_address;
+
+   ia->ia_sockmask.sin_family = AF_INET;
+   ia->ia_sockmask.sin_len = sizeof(struct sockaddr_in);
+   ia->ia_sockmask.sin_addr = req->pr_ip_netmask;
+
+   ia->ia_ifa.ifa_addr = sintosa(>ia_addr);
+   ia->ia_ifa.ifa_dstaddr = sintosa(>ia_dstaddr);
+   ia->ia_ifa.ifa_netmask = sintosa(>ia_sockmask);
+   ia->ia_ifa.ifa_ifp = ifp;
+
+   ia->ia_netmask = ia->ia_sockmask.sin_addr.s_addr;
+
+   error = in_ifinit(ifp, ia, , 1);
+   if (error) {
+   printf("pppx: unable to set addresses for %s, error=%d\n",
+   ifp->if_xname, error);
+   goto out_detach;
+   }
+
+   if_addrhooks_run(ifp);
+
+   /* fake a pipex interface context */
+   pxi->pxi_ifcontext.ifnet_this = ifp;
+   pxi->pxi_ifcontext.pipexmode = PIPEX_ENABLED;
 
switch (req->pr_protocol) {
 #ifdef PIPEX_PPPOE
case PIPEX_PROTO_PPPOE:
over_ifp = ifunit(req->pr_proto.pppoe.over_ifname);
-   if (over_ifp == NULL)
-   return (EINVAL);
-   if (req->pr_peer_address.ss_family != AF_UNSPEC)
-   return (EINVAL);
+   if (over_ifp == NULL) {
+   error = EINVAL;
+   goto out_detach;
+   }
+   if (req->pr_peer_address.ss_family != 

Re: locale thread support

2020-05-29 Thread Marc Espie
On Tue, May 26, 2020 at 12:13:02AM +0200, Ingo Schwarze wrote:
> Regarding the FreeBSD headers, i like them even less.  There are both
> terrible design choices and terrible implementation choices.  Regarding
> design, it appears that you *must* #include  after other
> headers - if you include it before, it won't work.  Regarding
> implementation, there is quite repulsive macro abuse in xlocale/_ctype.h;
> but probably that can be unrolled in LibreSSL style.  Either way, none
> of that hinders providing them if doing so yields benefit.

Thinking a bit more about that, two points:
- we do have headers in sys/  that do require a specific order of inclusion,
though that has mostly vanished over the years

- starting from a FreeBSD-like structure and making xlocale.h 
position-independent if we so wish is trivial  and not even that much
work. Each snippet is neatly tucked into an xlocale/  subdirectory,
so including xlocale.h early just requires something like

#ifdef _X_LOCALE_H
#include 
#endif

inside each  header...



Re: locale thread support

2020-05-29 Thread Mark Kettenis
> Date: Fri, 29 May 2020 15:48:41 +0200
> From: Ingo Schwarze 

Hi Ingo,
 
> Hi Marc,

I'm not Marc, but close enough ;).

> do i understand correctly that you intend to say:  there are
> legitimate reasons to use these xlocale.h functions in portable
> software, in particular in portable libraries, so we should probably
> have them all, or at least most of them?

Yes.  And we probably need most of them since if you implement some,
code is going to assume the others are there as well.

> I see your point that using functions like isdigit(3) can (at least
> in theory) be dangerous when running on systems that support
> exceedingly weird single-byte 8-bit locales.  (As opposed to only
> supporting one single single-byte locale, which is 7-bit, like we do.)
> 
> I'm not even sure the question matters whether xlocale.h is the best
> possible way to avoid such issues.  To make support desirable, it
> might be sufficient that xlocale.h is one reasonable way and that
> some amounts of useful software actually use it.

For example the libc++ that we have in base uses it.  There is a stub
implementation but it has given us quite a bit of grief.

> Marc Espie wrote on Thu, May 28, 2020 at 05:52:12PM +0200:
> > On Tue, May 26, 2020 at 12:13:02AM +0200, Ingo Schwarze wrote:
> 
> > > my impression is there are two totally independent topics in this
> > > thread.
> > > 
> > >  1. Whether and how to make things safer, ideally terminating the
> > > program in a controlled manner, if it uses functions that are
> > > not thread-safe in an invalid manner.  I'm not addressing that
> > > topic in this mail.
> > > 
> > >  2. Whether we want additional non-standard xlocale.h functions
> > > in our libc.
> > > 
> > > Regarding topic 2, let me say up front that i do not feel strongly
> > > either way.  It seems to me this is mostly a question for porters.
> > > If some functions are widely used - even if non-standard - and help
> > > avoid porting hassle, then sure, let's have them unless they cause
> > > excessive fuss.  But the latter does not seem likely here.
> > > 
> > > There are a number of functions that seem likely useful to me off
> > > the top of my head, for example: querylocale, nl_langinfo_l,
> > > MB_CUR_MAX_L, wcwidth_l
> > > 
> > > In general, i must say i like the whole xlocale.h business not all
> > > that much: in a nutshell, it is doubling the number of half the
> > > functions under the sun, which usually indicates poor design in the
> > > first place, and then the vast majority of these additional functions
> > > are almost useless on any operating system and totally useless on
> > > OpenBSD.  Like, i mean, btowc_l(3)?  Or even the isdigit_l(3) which
> > > we already have?  You must be choking, Mr. Chisnall!  I don't think
> > > stuff should be added because it is out there, but only if there is
> > > at least some porting benefit.
> > > 
> > > Regarding the FreeBSD headers, i like them even less.  There are both
> > > terrible design choices and terrible implementation choices.  Regarding
> > > design, it appears that you *must* #include  after other
> > > headers - if you include it before, it won't work.  Regarding
> > > implementation, there is quite repulsive macro abuse in xlocale/_ctype.h;
> > > but probably that can be unrolled in LibreSSL style.  Either way, none
> > > of that hinders providing them if doing so yields benefit.
> > > 
> > > See below for a list of functions that i drafted extremely quickly,
> > > so beware of errors and omissions.  The point isn't to present a
> > > definite plan.  The point is merely to demonstrate the sheer fatness
> > > of the animal and to give a first impression of the degree to which
> > > it stinks and grunts from most of its ends.
> > > 
> > > If any of this is needed, do porters already know which functions
> > > are in particularly high demand?  Do you have any idea how to find
> > > out which ones are actually useful for porting purposes?
> > 
> > Serendipity
> > 
> > Today, I looked at an older library  for audit purposes.
> > 
> > Library is called udns.
> > 
> > If you look at the code, there's a lot of apparently useless reinvention,
> > like the code explicitly checks for digits using c >= '0' && c <= '9' 
> > patterns
> > (or isprint for that matter).
> > 
> > Thinking some more, I realized why: this is library code, so it can't assume
> > it's running under any kind of sensible locale unless it has asserted it 
> > itself.
> > 
> > In that specific context, isdigit_l and friends make a lot of sense.
> > Either that, or systematic uselocale wrappers for any function that can be
> > called from outside.
> > 
> > (yes, I now that isdigit/isspace are not a problem on OpenBSD, but they can
> > be elsewhere!)
> 



Re: locale thread support

2020-05-29 Thread Marc Espie
On Fri, May 29, 2020 at 03:48:41PM +0200, Ingo Schwarze wrote:
> Hi Marc,
> 
> do i understand correctly that you intend to say:  there are
> legitimate reasons to use these xlocale.h functions in portable
> software, in particular in portable libraries, so we should probably
> have them all, or at least most of them?
> 
> I see your point that using functions like isdigit(3) can (at least
> in theory) be dangerous when running on systems that support
> exceedingly weird single-byte 8-bit locales.  (As opposed to only
> supporting one single single-byte locale, which is 7-bit, like we do.)
> 
> I'm not even sure the question matters whether xlocale.h is the best
> possible way to avoid such issues.  To make support desirable, it
> might be sufficient that xlocale.h is one reasonable way and that
> some amounts of useful software actually use it.
> 
> Yours,
>   Ingo

I'm starting to believe that a xlocale-like model makes more sense than
the current model in a lot of cases.

Think about it: a lot of locale goo deals with text processing.
NOT input/output, but data processing.

It seems obvious to me that at least SOME locale information has to be
attached to the data, and so each time you have to process the data, you
have to choose the right locale.

setlocale() is out in modern multi-threaded programs.
both uselocale() and xlocale.h functions are NOT portable to everything.

xlocale.h allows you  to attach the locale to the data

uselocale() requires you to do a back each time you want to process data
if you don't want to disturb other things.   It's hidden globals all over 
again.

It's no accident that libc++ will use xlocale.h functions if they're around.
As every other modern library should.



Re: locale thread support

2020-05-29 Thread Ingo Schwarze
Hi Marc,

do i understand correctly that you intend to say:  there are
legitimate reasons to use these xlocale.h functions in portable
software, in particular in portable libraries, so we should probably
have them all, or at least most of them?

I see your point that using functions like isdigit(3) can (at least
in theory) be dangerous when running on systems that support
exceedingly weird single-byte 8-bit locales.  (As opposed to only
supporting one single single-byte locale, which is 7-bit, like we do.)

I'm not even sure the question matters whether xlocale.h is the best
possible way to avoid such issues.  To make support desirable, it
might be sufficient that xlocale.h is one reasonable way and that
some amounts of useful software actually use it.

Yours,
  Ingo


Marc Espie wrote on Thu, May 28, 2020 at 05:52:12PM +0200:
> On Tue, May 26, 2020 at 12:13:02AM +0200, Ingo Schwarze wrote:

> > my impression is there are two totally independent topics in this
> > thread.
> > 
> >  1. Whether and how to make things safer, ideally terminating the
> > program in a controlled manner, if it uses functions that are
> > not thread-safe in an invalid manner.  I'm not addressing that
> > topic in this mail.
> > 
> >  2. Whether we want additional non-standard xlocale.h functions
> > in our libc.
> > 
> > Regarding topic 2, let me say up front that i do not feel strongly
> > either way.  It seems to me this is mostly a question for porters.
> > If some functions are widely used - even if non-standard - and help
> > avoid porting hassle, then sure, let's have them unless they cause
> > excessive fuss.  But the latter does not seem likely here.
> > 
> > There are a number of functions that seem likely useful to me off
> > the top of my head, for example: querylocale, nl_langinfo_l,
> > MB_CUR_MAX_L, wcwidth_l
> > 
> > In general, i must say i like the whole xlocale.h business not all
> > that much: in a nutshell, it is doubling the number of half the
> > functions under the sun, which usually indicates poor design in the
> > first place, and then the vast majority of these additional functions
> > are almost useless on any operating system and totally useless on
> > OpenBSD.  Like, i mean, btowc_l(3)?  Or even the isdigit_l(3) which
> > we already have?  You must be choking, Mr. Chisnall!  I don't think
> > stuff should be added because it is out there, but only if there is
> > at least some porting benefit.
> > 
> > Regarding the FreeBSD headers, i like them even less.  There are both
> > terrible design choices and terrible implementation choices.  Regarding
> > design, it appears that you *must* #include  after other
> > headers - if you include it before, it won't work.  Regarding
> > implementation, there is quite repulsive macro abuse in xlocale/_ctype.h;
> > but probably that can be unrolled in LibreSSL style.  Either way, none
> > of that hinders providing them if doing so yields benefit.
> > 
> > See below for a list of functions that i drafted extremely quickly,
> > so beware of errors and omissions.  The point isn't to present a
> > definite plan.  The point is merely to demonstrate the sheer fatness
> > of the animal and to give a first impression of the degree to which
> > it stinks and grunts from most of its ends.
> > 
> > If any of this is needed, do porters already know which functions
> > are in particularly high demand?  Do you have any idea how to find
> > out which ones are actually useful for porting purposes?
> 
> Serendipity
> 
> Today, I looked at an older library  for audit purposes.
> 
> Library is called udns.
> 
> If you look at the code, there's a lot of apparently useless reinvention,
> like the code explicitly checks for digits using c >= '0' && c <= '9' patterns
> (or isprint for that matter).
> 
> Thinking some more, I realized why: this is library code, so it can't assume
> it's running under any kind of sensible locale unless it has asserted it 
> itself.
> 
> In that specific context, isdigit_l and friends make a lot of sense.
> Either that, or systematic uselocale wrappers for any function that can be
> called from outside.
> 
> (yes, I now that isdigit/isspace are not a problem on OpenBSD, but they can
> be elsewhere!)



Re: userland clock_gettime proof of concept

2020-05-29 Thread Mark Kettenis
> From: "Theo de Raadt" 
> Date: Fri, 29 May 2020 07:26:50 -0600
> 
> Paul Irofti  wrote:
> 
> > On 2020-05-29 16:00, Mark Kettenis wrote:
> > >> Date: Fri, 29 May 2020 13:45:37 +0100
> > >> From: Stuart Henderson 
> > >>
> > >> On 2020/05/29 13:50, Paul Irofti wrote:
> > >>> +struct __timekeep {
> > >>> +   uint32_t major; /* version major number */
> > >>> +   uint32_t minor; /* version minor number */
> > >>> +
> > >>> +   u_int64_t   th_scale;
> > >>> +   unsigned intth_offset_count;
> > >>> +   struct bintime  th_offset;
> > >>> +   struct bintime  th_naptime;
> > >>> +   struct bintime  th_boottime;
> > >>> +   volatile unsigned int   th_generation;
> > >>> +
> > >>> +   unsigned inttc_user;
> > >>> +   unsigned inttc_counter_mask;
> > >>> +};
> > >>
> > >> Ah good, you got rid of u_int, that was causing problems with port 
> > >> builds.
> > >
> > > That in itself is a problem.  This means  is the wrong place
> > > for this struct.  We need to find a better place for this.
> > >
> > > Since this is now closely linked to the timecounter stuff
> > >  would be an obvious place.  Now that file has:
> > >
> > > #ifndef _KERNEL
> > > #error "no user-serviceable parts inside"
> > > #endif
> > >
> > > you could change that into
> > >
> > > #if !defined(_KERNEL) && !defined(_LIBC)
> > > #error "no user-serviceable parts inside"
> > > #endif
> > >
> > > and make sure you #define _LIBC brefore uncluding this file where it
> > > is needed.  As few places as possible obviously.
> > 
> > Hmmm... so this would make it libc bound. I don't see anything wrong
> > with it, but is it what we want?
> 
> what a strange comment
> 
> in our world, libc (and subjuncts crt0 and ld.so) are THE ONLY valid
> interfaces to the kernel
> 
> Another place to put it is in elf, since it is tied to a new elf marker.

The timekeep page isn't really tied to ELF; it's just used as a
mechanism to communicate the address of the page.  Paul could have
chosen a different mechanism such as sysctl (see vm.psstrings).

> Tremendous caution is required, these include files must not reference
> each other in a way that requires excessive #include which will break
> public software

Right, that is why  is safe.  It currently produces an
error if you include it from userland.



Re: userland clock_gettime proof of concept

2020-05-29 Thread Theo de Raadt
Paul Irofti  wrote:

> On 2020-05-29 16:00, Mark Kettenis wrote:
> >> Date: Fri, 29 May 2020 13:45:37 +0100
> >> From: Stuart Henderson 
> >>
> >> On 2020/05/29 13:50, Paul Irofti wrote:
> >>> +struct __timekeep {
> >>> + uint32_t major; /* version major number */
> >>> + uint32_t minor; /* version minor number */
> >>> +
> >>> + u_int64_t   th_scale;
> >>> + unsigned intth_offset_count;
> >>> + struct bintime  th_offset;
> >>> + struct bintime  th_naptime;
> >>> + struct bintime  th_boottime;
> >>> + volatile unsigned int   th_generation;
> >>> +
> >>> + unsigned inttc_user;
> >>> + unsigned inttc_counter_mask;
> >>> +};
> >>
> >> Ah good, you got rid of u_int, that was causing problems with port builds.
> >
> > That in itself is a problem.  This means  is the wrong place
> > for this struct.  We need to find a better place for this.
> >
> > Since this is now closely linked to the timecounter stuff
> >  would be an obvious place.  Now that file has:
> >
> > #ifndef _KERNEL
> > #error "no user-serviceable parts inside"
> > #endif
> >
> > you could change that into
> >
> > #if !defined(_KERNEL) && !defined(_LIBC)
> > #error "no user-serviceable parts inside"
> > #endif
> >
> > and make sure you #define _LIBC brefore uncluding this file where it
> > is needed.  As few places as possible obviously.
> 
> Hmmm... so this would make it libc bound. I don't see anything wrong
> with it, but is it what we want?

what a strange comment

in our world, libc (and subjuncts crt0 and ld.so) are THE ONLY valid
interfaces to the kernel

Another place to put it is in elf, since it is tied to a new elf marker.

Tremendous caution is required, these include files must not reference
each other in a way that requires excessive #include which will break
public software



Re: userland clock_gettime proof of concept

2020-05-29 Thread Paul Irofti

On 2020-05-29 16:00, Mark Kettenis wrote:

Date: Fri, 29 May 2020 13:45:37 +0100
From: Stuart Henderson 

On 2020/05/29 13:50, Paul Irofti wrote:

+struct __timekeep {
+   uint32_t major; /* version major number */
+   uint32_t minor; /* version minor number */
+
+   u_int64_t   th_scale;
+   unsigned intth_offset_count;
+   struct bintime  th_offset;
+   struct bintime  th_naptime;
+   struct bintime  th_boottime;
+   volatile unsigned int   th_generation;
+
+   unsigned inttc_user;
+   unsigned inttc_counter_mask;
+};


Ah good, you got rid of u_int, that was causing problems with port builds.


That in itself is a problem.  This means  is the wrong place
for this struct.  We need to find a better place for this.

Since this is now closely linked to the timecounter stuff
 would be an obvious place.  Now that file has:

#ifndef _KERNEL
#error "no user-serviceable parts inside"
#endif

you could change that into

#if !defined(_KERNEL) && !defined(_LIBC)
#error "no user-serviceable parts inside"
#endif

and make sure you #define _LIBC brefore uncluding this file where it
is needed.  As few places as possible obviously.


Hmmm... so this would make it libc bound. I don't see anything wrong 
with it, but is it what we want?




Re: userland clock_gettime proof of concept

2020-05-29 Thread Mark Kettenis
> Date: Fri, 29 May 2020 13:45:37 +0100
> From: Stuart Henderson 
> 
> On 2020/05/29 13:50, Paul Irofti wrote:
> > +struct __timekeep {
> > +   uint32_t major; /* version major number */
> > +   uint32_t minor; /* version minor number */
> > +
> > +   u_int64_t   th_scale;
> > +   unsigned intth_offset_count;
> > +   struct bintime  th_offset;
> > +   struct bintime  th_naptime;
> > +   struct bintime  th_boottime;
> > +   volatile unsigned int   th_generation;
> > +
> > +   unsigned inttc_user;
> > +   unsigned inttc_counter_mask;
> > +};
> 
> Ah good, you got rid of u_int, that was causing problems with port builds.

That in itself is a problem.  This means  is the wrong place
for this struct.  We need to find a better place for this.

Since this is now closely linked to the timecounter stuff
 would be an obvious place.  Now that file has:

#ifndef _KERNEL
#error "no user-serviceable parts inside"
#endif

you could change that into

#if !defined(_KERNEL) && !defined(_LIBC)
#error "no user-serviceable parts inside"
#endif

and make sure you #define _LIBC brefore uncluding this file where it
is needed.  As few places as possible obviously.




Re: Enable building wsmoused on arm64 and armv7

2020-05-29 Thread Ingo Schwarze
Hi,

Frederic Cambus wrote on Thu, May 28, 2020 at 10:44:59PM +0200:
> On Thu, May 28, 2020 at 10:52:44AM -0600, Theo de Raadt wrote:

>> -MANSUBDIR= i386 amd64 alpha
>> +MANSUBDIR= i386 amd64 arm64 armv7 alpha
>> 
>> Actually, I suggest making this a MI man page.  Delete that line,
>> and see where the files land.  I'll adjust sets.

Good idea.

> Yes, makes sense. Just commited the change, MANSUBDIR is now gone.
> The man page ends up in /usr/share/man/man8 as expected.

Thanks for doing that.


In the medium to long term, i hope to improve support for pages
that apply to several, but not all architectures.  I have some ideas
how to make that much simpler to use and more consistent, but haven't
fully made up my mind yet.  I isn't urgent, so there is no hurry,
and i hope to get it right the first time rather than experimenting
back and forth.

In the meantime, there is indeed nothing wrong with making pages
that apply to many arches simply MI.


In case you are interested in some details:
Stuff like this isn't ideal:

   $ cd /usr/share/man/   
   $ ls -al man8/*/wsmoused.8
  -r--r--r--  1 root  bin  5958 May 27 16:38 man8/alpha/wsmoused.8
  -r--r--r--  1 root  bin  5958 May 27 16:38 man8/amd64/wsmoused.8
  -r--r--r--  1 root  bin  5958 May 27 16:38 man8/i386/wsmoused.8

So there is was one copy of the file page per arch, it wasn't even
hardlinked.  Of course, the waste of space hardly matters, but since
we got rid years ago of identifying additional *names* of pages by
putting those names names into the file system, identifying *arches*
by file names now feels archaic at best.

It also has practical downsides, for example:

   $ man -k wsmoused
  wsmoused(8/alpha) - wsmouse daemon
  wsmoused(8/amd64) - wsmouse daemon
  wsmoused(8/i386) - wsmouse daemon

So, apropos(1) made me believe there might be three different
versions, while they are actually all the same page.  Compare to
this where different pages actually do exist:

   $ ls -al $(man -fw cpu)
  -r--r--r--  1 root  bin  3115 May 27 16:38 /usr/share/man/man4/amd64/cpu.4
  -r--r--r--  1 root  bin  8411 May 27 16:38 /usr/share/man/man4/hppa/cpu.4
  -r--r--r--  1 root  bin  3881 May 27 16:38 /usr/share/man/man4/i386/cpu.4

Consequently, "man -af wsmoused" printed the same page three times,
which felt ugly and confusing.  What i hope to ultimately teach it
is to show the page only once *and*, at the same time, to clearly
indicate which arches it applies to, without needing requiring any
complicated syntax or code.

Yours,
  Ingo



Re: userland clock_gettime proof of concept

2020-05-29 Thread Paul Irofti

On 2020-05-29 15:45, Stuart Henderson wrote:

On 2020/05/29 13:50, Paul Irofti wrote:

+struct __timekeep {
+   uint32_t major; /* version major number */
+   uint32_t minor; /* version minor number */
+
+   u_int64_t   th_scale;
+   unsigned intth_offset_count;
+   struct bintime  th_offset;
+   struct bintime  th_naptime;
+   struct bintime  th_boottime;
+   volatile unsigned int   th_generation;
+
+   unsigned inttc_user;
+   unsigned inttc_counter_mask;
+};


Ah good, you got rid of u_int, that was causing problems with port builds.


Yeah, I got a few reports about that :) Such a stupid type anyway...



Re: userland clock_gettime proof of concept

2020-05-29 Thread Stuart Henderson
On 2020/05/29 13:50, Paul Irofti wrote:
> +struct __timekeep {
> + uint32_t major; /* version major number */
> + uint32_t minor; /* version minor number */
> +
> + u_int64_t   th_scale;
> + unsigned intth_offset_count;
> + struct bintime  th_offset;
> + struct bintime  th_naptime;
> + struct bintime  th_boottime;
> + volatile unsigned int   th_generation;
> +
> + unsigned inttc_user;
> + unsigned inttc_counter_mask;
> +};

Ah good, you got rid of u_int, that was causing problems with port builds.



sparc64 boot issue on qemu

2020-05-29 Thread Otto Moerbeek
On Thu, May 28, 2020 at 10:11:28AM +0200, Otto Moerbeek wrote:

> On Thu, May 28, 2020 at 01:21:21AM -0600, Jason A. Donenfeld wrote:
> 
> > On Thu, May 28, 2020 at 1:19 AM Otto Moerbeek  wrote:
> > > Of course.., I was running it from a !wxallowed mount. BTW, qemu is in
> > > packages, no need to build it yourself.
> > 
> > Sure, but now I've been somewhat nerd sniped and am playing with this
> > fcode forth implementation in qemu :-P. I wonder if there's something
> > missing in the 64-bit extensions to IEEE 1275, in table.fs...
> 
> OK, can reproduce. I'll see if I can find out something.
> 
>   -Otto
> 

After running the bootblocks in debug mode (using boot disk -V) and
seeing ofwboot was found and loaded, I added some debug code to the
bootblocks and now it correctly starts ofwboot on qemu


Trying disk:a...
Not a bootable ELF image
Not a bootable a.out image

Loading FCode image...
Loaded 6936 bytes
entry point is 0x4000
Evaluating FCode...
OpenBSD IEEE 1275 Bootblock 2.0
..free mem
close boot dev
start loaded program
>> OpenBSD BOOT 1.17
Trying bsd...
open /etc/random.seed: No such file or directory
Booting /pci@1fe,0/pci@1,1/ide@3/ide@0/disk@0:a/bsd
4225784@0x100+1288@0x1407af8+3249436@0x1c0+944868@0x1f1951c 
symbols @ 0xfef50340 139 start=0x100
console is /pci@1fe,0/pci@1,1/ebus@1/su
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
real mem = 2147483648 (2048MB)
avail mem = 2099232768 (2001MB)
random: boothowto does not indicate good seed
mainbus0 at root: OpenBiosTeam,OpenBIOS
cpu0 at mainbus0: SUNW,UltraSPARC-IIi (rev 9.1) @ 100 MHz
cpu0: physical 256K instruction (64 b/l), 16K data (32 b/l), 256K
external (64 b/l)
psycho0 at mainbus0: SUNW,sabre, impl 0, version 0, ign 7c0
psycho0: bus range 0-2, PCI bus 0
psycho0: dvma map c000-dfff
pci0 at psycho0
ppb0 at pci0 dev 1 function 1 "Sun Simba" rev 0x11
pci1 at ppb0 bus 1
ebus0 at pci1 dev 1 function 0 "Sun PCIO EBus2" rev 0x01
clock1 at ebus0 addr 2000-3fff: mk48t59
"power" at ebus0 addr 7240-7243 ivec 0x1 not configured
"fdthree" at ebus0 addr 0- not configured
com0 at ebus0 addr 3f8-3ff ivec 0x2b: ns16550a, 16 byte fifo
com0: console
pckbc0 at ebus0 addr 60-67 ivec 0x29
pckbd0 at pckbc0 (kbd slot)
wskbd0 at pckbd0
"Bochs VGA" rev 0x02 at pci1 dev 2 function 0 not configured
pciide0 at pci1 dev 3 function 0 "CMD Technology PCI0646" rev 0x07:
DMA, channel 0 configured to native-PCI, channel 1 configured to native-PCI
pciide0: using ivec 0x7e0 for native-PCI interrupt
wd0 at pciide0 channel 0 drive 0: 
wd0: 16-sector PIO, LBA48, 3MB, 6400 sectors
wd0(pciide0:0:0): using PIO mode 4, Ultra-DMA mode 2
atapiscsi0 at pciide0 channel 1 drive 0
scsibus0 at atapiscsi0: 2 targets
cd0 at scsibus0 targ 0 lun 0:  removable
cd0(pciide0:1:0): using PIO mode 4, Ultra-DMA mode 2
ppb1 at pci0 dev 1 function 0 "Sun Simba" rev 0x11
pci2 at ppb1 bus 2
ne0 at pci2 dev 0 function 0 "Realtek 8029" rev 0x00: ivec 0x7d0,
address 52:54:00:12:34:56
softraid0 at root
scsibus1 at softraid0: 256 targets

It hangs at this point here, but I that's clearly another issue.

Puzzled...

-Otto

Index: bootblk.fth
===
RCS file: /cvs/src/sys/arch/sparc64/stand/bootblk/bootblk.fth,v
retrieving revision 1.9
diff -u -p -r1.9 bootblk.fth
--- bootblk.fth 2 Apr 2020 06:06:22 -   1.9
+++ bootblk.fth 29 May 2020 11:53:36 -
@@ -850,16 +850,22 @@ create cur-blockno -1 l, -1 l,\ Curren
   " /ofwboot" load-file( -- load-base )
then
 
+   ." free mem" cr
+
\ Free memory for reading disk blocks
cur-block 0<> if
   dev-block dev-blocksize free-mem
then
 
+   ." close boot dev" cr
+
\ Close boot device
boot-ihandle dup -1 <> if
   cif-close -1 to boot-ihandle 
then

+   ." start loaded program" cr
+
dup 0<> if " to load-base init-program" evaluate then
 ;
 



Re: Initialize v4l2_requestbuffers struct to avoid invalid mmap

2020-05-29 Thread Ingo Feinerer
On Thu, May 28, 2020 at 12:23:58PM +0200, Martin Pieuchot wrote:
> On 26/05/20(Tue) 11:30, Ingo Feinerer wrote:
> > video(1) supports reading frames from a webcam via mmap(). To inform the
> > V4L2 device about the number of desired buffers containing the frames to
> > be memory-mapped, a VIDIOC_REQBUFS ioctl call is used.
> > 
> > At the moment the v4l2_requestbuffers struct used for the VIDIOC_REQBUFS 
> > ioctl
> > is only partially initialized which leads to an invalid mmap for my webcam:
> 
> The kernel driver, uvideo(4) or utvfu(4) is responsible for the rest of
> the initialization.

Thanks for the hint. uvideo_reqbufs() is ignoring both struct members
`type` and `memory` so it makes no difference when using video(1) as is.
I wrote a small program that just fetches one frame and can confirm it
works.

However, I have a camera that only supports the MJPEG encoding which
video(1) cannot handle. So I am using libv4l (pkg_add libv4l) which
can convert encodings on the fly:

LD_PRELOAD=/usr/local/lib/v4l2convert.so video

libv4l inspects the `type` and `memory` struct members
(https://git.linuxtv.org/v4l-utils.git/tree/lib/libv4l2/libv4l2.c#n1308)
and fails if `memory != V4L2_MEMORY_MMAP`.

So the real question is if video(1) should "support" libv4l ...

Best regards,
Ingo



Re: [RFC] pppd: add pipex(4) L2TP control support

2020-05-29 Thread Stuart Henderson
On 2020/05/28 19:42, Jason McIntyre wrote:
> On Wed, May 27, 2020 at 08:43:47AM +0200, Martin Pieuchot wrote:
> > On 26/05/20(Tue) 10:31, Claudio Jeker wrote:
> > > [...] 
> > > npppd(8) is server only it can not establish a connection. pppd(8) on the
> > > other hand is more client side (but I think it can do both).
> > 
> > Could someone knowledgable indicate that in the man pages?
> 
> well, i don;t qualify as knowledgable about ppp, but i'll bite anyway:
> 
> > Currently there is:
> > 
> > npppd ??? new Point-to-Point Protocol daemon
> > pppd ??? Point-to-Point Protocol daemon
> > 
> > Confusing...
> > 
> 
> yes. at the time, i think npppd's description made sense - it was
> a newer version of the ppp server. i think it was pppd that had
> the confusing description, but there was a reason for that too.
> wasn;t there an alternative way to do dialup, and pppd was the
> kernel method?

Here's an overview.

ppp(4), runs PPP in the kernel, controlled by pppd(8). Used for
serial port devices or via a helper application (e.g. xl2tpd in ports
for L2TP client/server - could do PPPoE via a helper too). Client or
server. The version we have is IPv4 only. (Still maintained upstream
but the support for most OS was removed after 2.3.11 in 1999,
it's now Linux/Solaris only).

pppoe(4), runs PPP-over-Ethernet in the kernel using the sppp(4) layer.
Controlled by ifconfig. Client side only. IPv4 and IPv6.

ppp(8), removed in 5.6. Ran PPP in userland and used the tun(4) device
to send packets to/from the kernel. It was used for serial port devices
or PPPoE via a helper (probably also pptp via a helper IIRC). Client
or server. IPv4 and IPv6.

npppd(8), runs L2TP, PPTP, PPPoE. Either userland via tun(4), or hybrid
userland+kernel - userland for connection setup, kernel via pipex(4)
devices handling bulk data transfer after setup. Server side only.
Mostly IPv4 only (L2TP can listen on IPv6 but the PPP sessions
themselves are v4-only).



Re: userland clock_gettime proof of concept

2020-05-29 Thread Paul Irofti
On Thu, May 28, 2020 at 07:43:55PM +0200, Mark Kettenis wrote:
> > Date: Thu, 28 May 2020 17:44:31 +0300
> > From: Paul Irofti 
> > 
> > Hi,
> > 
> > Here is a new iteration of the diff which includes support for MD high
> > resolution clocks. Currently only implements TSC on amd64. If the
> > MD function is not defined, it fallsback to the syscall.
> > 
> > There is the question of the skew fix, but that will be addressed in a
> > separate kernel diff that will not affect the current diff at all.
> > 
> > I could not find a way to find on which processor the process is running
> > on from userland without going through a syscall. If there is one please
> > let me know. It would make things easier.
> > 
> > In the meantime I have also gotten positive feedback from various
> > testers that run this on their main machine.
> > 
> > Anyway, I think we can decide on the struct name and the auxiliary
> > vector ID and consider this done.
> > 
> > Thoughts?
> 
> This is getting us somewhere.
> 
> Still some issues though (besides the skew thing you already mention).
> 
> 1. The synchronization mechanism is broken.  The seq member needs to
>be set to 0 while updating the struct and only set to the "next"
>value after completing the update of the full struct.  You need to
>be careful to avoid 0, otherwise the application will spin for a
>full timeslice while seq overflows into 0.
> 
>However, since you now export the timehands generation, I'd really
>drop seq and use the timehands generation for synchronization.  It
>makes no sense to have both.
> 
> 2. Since tc_update_timekeep() is called from tc_windup() it doesn't
>need to do the synchronization dance.
> 
> 3. Like tc_windup, tc_update_timekeep() needs to have some
> membar_procer() calls in it instead of membar_consumer() calls.
> 
> 4. There is no need to update th_counter_mask on every update.
> 
> 5. What if the TSC is not available as a usable timecounter?  In that
>case libc should fall back on the system call.  But we need a way
>to communicate what the timecounter is and detect when we switch
>timecounters.  Maybe adding a timecounter ID to the page will help
>here.  But then MD code in libc will have to check the ID and
>dispatch to the right timecounter read function.
> 
> 6. The major and minor fields probably should bbe uint32_t or maybe
> uint16_t.  You're not saving any space by making them uint8_t.

Here is a new diff that addresses the issues stated above. I went with
adding a new field in timecounter. This can be used as an ID further on
and also turned into a sysctl if needed.


diff --git lib/libc/arch/amd64/gen/Makefile.inc 
lib/libc/arch/amd64/gen/Makefile.inc
index e995309ed71..caa4452a3d9 100644
--- lib/libc/arch/amd64/gen/Makefile.inc
+++ lib/libc/arch/amd64/gen/Makefile.inc
@@ -2,6 +2,6 @@
 
 SRCS+= _setjmp.S fabs.S infinity.c ldexp.c modf.S nan.c setjmp.S \
sigsetjmp.S
-SRCS+= fpclassifyl.c isfinitel.c isinfl.c isnanl.c isnormall.c signbitl.c
+SRCS+= fpclassifyl.c rdtsc.c isfinitel.c isinfl.c isnanl.c isnormall.c 
signbitl.c
 SRCS+= flt_rounds.S fpgetmask.S fpgetround.S fpgetsticky.S fpsetmask.S \
fpsetround.S fpsetsticky.S
diff --git lib/libc/arch/amd64/gen/rdtsc.c lib/libc/arch/amd64/gen/rdtsc.c
new file mode 100644
index 000..b14c862c61a
--- /dev/null
+++ lib/libc/arch/amd64/gen/rdtsc.c
@@ -0,0 +1,26 @@
+/* $OpenBSD$ */
+/*
+ * Copyright (c) 2020 Paul Irofti 
+ *
+ * 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 
+
+uint64_t
+tc_get_timecount_md(void)
+{
+   uint32_t hi, lo;
+   asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
+   return ((uint64_t)lo)|(((uint64_t)hi)<<32);
+}
diff --git lib/libc/asr/asr.c lib/libc/asr/asr.c
index cd056c85719..2b25d49f32a 100644
--- lib/libc/asr/asr.c
+++ lib/libc/asr/asr.c
@@ -196,11 +196,11 @@ poll_intrsafe(struct pollfd *fds, nfds_t nfds, int 
timeout)
struct timespec pollstart, pollend, elapsed;
int r;
 
-   if (clock_gettime(CLOCK_MONOTONIC, ))
+   if (WRAP(clock_gettime)(CLOCK_MONOTONIC, ))
return -1;
 
while ((r = poll(fds, 1, timeout)) == -1 && errno == EINTR) {
-   if (clock_gettime(CLOCK_MONOTONIC, ))
+   if 

Re: [patch] azalia: Intel 300 Series HD Audio

2020-05-29 Thread Mark Kettenis
> Date: Fri, 29 May 2020 19:55:25 +1000
> From: Jonathan Gray 
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
> 
> On Fri, May 29, 2020 at 11:25:44AM +0200, Bruno Flueckiger wrote:
> > Hi,
> > 
> > My brand new laptop HP EliteBook 850 G6 comes with an Intel 300 Series
> > HD Audio device rev 0x11. The device shows up as not configured in the
> > dmesg. The PCI config space of the device identifies its subclass as
> > PCI_SUBCLASS_MULTIMEDIA_AUDIO instead of PCI_SUBCLASS_MULTIMEDIA_HDAUDIO
> > 
> > The patch below makes the device work just fine on my laptop.
> > 
> > Cheers,
> > Bruno
> 
> I think it would be better to match on the id in that case as non-azalia
> devices use the audio class.

Agreed.  ok kettenis@

> Index: azalia.c
> ===
> RCS file: /cvs/src/sys/dev/pci/azalia.c,v
> retrieving revision 1.255
> diff -u -p -r1.255 azalia.c
> --- azalia.c  18 Apr 2020 21:55:56 -  1.255
> +++ azalia.c  29 May 2020 09:51:06 -
> @@ -474,6 +474,10 @@ azalia_configure_pci(azalia_t *az)
>   }
>  }
>  
> +const struct pci_matchid azalia_pci_devices[] = {
> + { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }
> +};
> +
>  int
>  azalia_pci_match(struct device *parent, void *match, void *aux)
>  {
> @@ -483,7 +487,8 @@ azalia_pci_match(struct device *parent, 
>   if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MULTIMEDIA
>   && PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MULTIMEDIA_HDAUDIO)
>   return 1;
> - return 0;
> + return pci_matchbyid((struct pci_attach_args *)aux, azalia_pci_devices,
> + nitems(azalia_pci_devices));
>  }
>  
>  void
> 
> 



Re: [patch] azalia: Intel 300 Series HD Audio

2020-05-29 Thread Jonathan Gray
On Fri, May 29, 2020 at 11:25:44AM +0200, Bruno Flueckiger wrote:
> Hi,
> 
> My brand new laptop HP EliteBook 850 G6 comes with an Intel 300 Series
> HD Audio device rev 0x11. The device shows up as not configured in the
> dmesg. The PCI config space of the device identifies its subclass as
> PCI_SUBCLASS_MULTIMEDIA_AUDIO instead of PCI_SUBCLASS_MULTIMEDIA_HDAUDIO
> 
> The patch below makes the device work just fine on my laptop.
> 
> Cheers,
> Bruno

I think it would be better to match on the id in that case as non-azalia
devices use the audio class.

Index: azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.255
diff -u -p -r1.255 azalia.c
--- azalia.c18 Apr 2020 21:55:56 -  1.255
+++ azalia.c29 May 2020 09:51:06 -
@@ -474,6 +474,10 @@ azalia_configure_pci(azalia_t *az)
}
 }
 
+const struct pci_matchid azalia_pci_devices[] = {
+   { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_300SERIES_U_HDA }
+};
+
 int
 azalia_pci_match(struct device *parent, void *match, void *aux)
 {
@@ -483,7 +487,8 @@ azalia_pci_match(struct device *parent, 
if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MULTIMEDIA
&& PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MULTIMEDIA_HDAUDIO)
return 1;
-   return 0;
+   return pci_matchbyid((struct pci_attach_args *)aux, azalia_pci_devices,
+   nitems(azalia_pci_devices));
 }
 
 void



[patch] azalia: Intel 300 Series HD Audio

2020-05-29 Thread Bruno Flueckiger
Hi,

My brand new laptop HP EliteBook 850 G6 comes with an Intel 300 Series
HD Audio device rev 0x11. The device shows up as not configured in the
dmesg. The PCI config space of the device identifies its subclass as
PCI_SUBCLASS_MULTIMEDIA_AUDIO instead of PCI_SUBCLASS_MULTIMEDIA_HDAUDIO

The patch below makes the device work just fine on my laptop.

Cheers,
Bruno

Index: sys/dev/pci/azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.255
diff -u -p -r1.255 azalia.c
--- sys/dev/pci/azalia.c18 Apr 2020 21:55:56 -  1.255
+++ sys/dev/pci/azalia.c28 May 2020 13:48:10 -
@@ -481,7 +481,8 @@ azalia_pci_match(struct device *parent,

pa = aux;
if (PCI_CLASS(pa->pa_class) == PCI_CLASS_MULTIMEDIA
-   && PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MULTIMEDIA_HDAUDIO)
+   && (PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MULTIMEDIA_HDAUDIO
+   || PCI_SUBCLASS(pa->pa_class) == PCI_SUBCLASS_MULTIMEDIA_AUDIO))
return 1;
return 0;
 }



Re: filesystem code integer and many inodes

2020-05-29 Thread Otto Moerbeek
On Fri, May 29, 2020 at 09:30:04AM +0200, Otto Moerbeek wrote:

> On Thu, May 28, 2020 at 12:54:41PM -0600, Todd C. Miller wrote:
> 
> > On Thu, 28 May 2020 20:53:07 +0200, Otto Moerbeek wrote:
> > 
> > > Here's the separate diff for the prefcg loops. From FreeBSD.
> > 
> > OK millert@
> > 
> >  - todd
> > 
> 
> And here's the updated diff against -current. I removed a redundant
> cast in a fs_ipg * fs_ncg multiplication in fsck_ffs. Since both are
> u_int32 and we know the product is <= UINT_MAX, so we do not need to
> cast.
> 
> I would like to make some progress here, I have a followup diff to
> speed up Phase 5 of fsck_ffs...

This last line was directed at other tech@ subscribers and not so much
at millert@. Please review and/or test. Thanks!

-Otto



Re: filesystem code integer and many inodes

2020-05-29 Thread Otto Moerbeek
On Thu, May 28, 2020 at 12:54:41PM -0600, Todd C. Miller wrote:

> On Thu, 28 May 2020 20:53:07 +0200, Otto Moerbeek wrote:
> 
> > Here's the separate diff for the prefcg loops. From FreeBSD.
> 
> OK millert@
> 
>  - todd
> 

And here's the updated diff against -current. I removed a redundant
cast in a fs_ipg * fs_ncg multiplication in fsck_ffs. Since both are
u_int32 and we know the product is <= UINT_MAX, so we do not need to
cast.

I would like to make some progress here, I have a followup diff to
speed up Phase 5 of fsck_ffs...

-Otto

Index: sbin/clri/clri.c
===
RCS file: /cvs/src/sbin/clri/clri.c,v
retrieving revision 1.20
diff -u -p -r1.20 clri.c
--- sbin/clri/clri.c28 Jun 2019 13:32:43 -  1.20
+++ sbin/clri/clri.c29 May 2020 07:23:27 -
@@ -68,7 +68,8 @@ main(int argc, char *argv[])
char *fs, sblock[SBLOCKSIZE];
size_t bsize;
off_t offset;
-   int i, fd, imax, inonum;
+   int i, fd;
+   ino_t imax, inonum;
 
if (argc < 3)
usage();
Index: sbin/dumpfs/dumpfs.c
===
RCS file: /cvs/src/sbin/dumpfs/dumpfs.c,v
retrieving revision 1.35
diff -u -p -r1.35 dumpfs.c
--- sbin/dumpfs/dumpfs.c17 Feb 2020 16:11:25 -  1.35
+++ sbin/dumpfs/dumpfs.c29 May 2020 07:23:27 -
@@ -69,7 +69,7 @@ union {
 #define acgcgun.cg
 
 intdumpfs(int, const char *);
-intdumpcg(const char *, int, int);
+intdumpcg(const char *, int, u_int);
 intmarshal(const char *);
 intopen_disk(const char *);
 void   pbits(void *, int);
@@ -163,6 +163,7 @@ dumpfs(int fd, const char *name)
size_t size;
off_t off;
int i, j;
+   u_int cg;
 
switch (afs.fs_magic) {
case FS_UFS2_MAGIC:
@@ -172,7 +173,7 @@ dumpfs(int fd, const char *name)
afs.fs_magic, ctime());
printf("superblock location\t%jd\tid\t[ %x %x ]\n",
(intmax_t)afs.fs_sblockloc, afs.fs_id[0], afs.fs_id[1]);
-   printf("ncg\t%d\tsize\t%jd\tblocks\t%jd\n",
+   printf("ncg\t%u\tsize\t%jd\tblocks\t%jd\n",
afs.fs_ncg, (intmax_t)fssize, (intmax_t)afs.fs_dsize);
break;
case FS_UFS1_MAGIC:
@@ -198,7 +199,7 @@ dumpfs(int fd, const char *name)
printf("cylgrp\t%s\tinodes\t%s\tfslevel %d\n",
i < 1 ? "static" : "dynamic",
i < 2 ? "4.2/4.3BSD" : "4.4BSD", i);
-   printf("ncg\t%d\tncyl\t%d\tsize\t%d\tblocks\t%d\n",
+   printf("ncg\t%u\tncyl\t%d\tsize\t%d\tblocks\t%d\n",
afs.fs_ncg, afs.fs_ncyl, afs.fs_ffs1_size, 
afs.fs_ffs1_dsize);
break;
default:
@@ -223,9 +224,9 @@ dumpfs(int fd, const char *name)
(intmax_t)afs.fs_cstotal.cs_ndir,
(intmax_t)afs.fs_cstotal.cs_nifree, 
(intmax_t)afs.fs_cstotal.cs_nffree);
-   printf("bpg\t%d\tfpg\t%d\tipg\t%d\n",
+   printf("bpg\t%d\tfpg\t%d\tipg\t%u\n",
afs.fs_fpg / afs.fs_frag, afs.fs_fpg, afs.fs_ipg);
-   printf("nindir\t%d\tinopb\t%d\tmaxfilesize\t%ju\n",
+   printf("nindir\t%d\tinopb\t%u\tmaxfilesize\t%ju\n",
afs.fs_nindir, afs.fs_inopb, 
(uintmax_t)afs.fs_maxfilesize);
printf("sbsize\t%d\tcgsize\t%d\tcsaddr\t%jd\tcssize\t%d\n",
@@ -238,10 +239,10 @@ dumpfs(int fd, const char *name)
printf("nbfree\t%d\tndir\t%d\tnifree\t%d\tnffree\t%d\n",
afs.fs_ffs1_cstotal.cs_nbfree, afs.fs_ffs1_cstotal.cs_ndir,
afs.fs_ffs1_cstotal.cs_nifree, 
afs.fs_ffs1_cstotal.cs_nffree);
-   printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%d\n",
+   printf("cpg\t%d\tbpg\t%d\tfpg\t%d\tipg\t%u\n",
afs.fs_cpg, afs.fs_fpg / afs.fs_frag, afs.fs_fpg,
afs.fs_ipg);
-   printf("nindir\t%d\tinopb\t%d\tnspf\t%d\tmaxfilesize\t%ju\n",
+   printf("nindir\t%d\tinopb\t%u\tnspf\t%d\tmaxfilesize\t%ju\n",
afs.fs_nindir, afs.fs_inopb, afs.fs_nspf,
(uintmax_t)afs.fs_maxfilesize);
printf("sbsize\t%d\tcgsize\t%d\tcgoffset %d\tcgmask\t0x%08x\n",
@@ -261,7 +262,7 @@ dumpfs(int fd, const char *name)
afs.fs_sblkno, afs.fs_cblkno, afs.fs_iblkno, afs.fs_dblkno);
printf("cgrotor\t%d\tfmod\t%d\tronly\t%d\tclean\t%d\n",
afs.fs_cgrotor, afs.fs_fmod, afs.fs_ronly, afs.fs_clean);
-   printf("avgfpdir %d\tavgfilesize %d\n",
+   printf("avgfpdir %u\tavgfilesize %u\n",
afs.fs_avgfpdir, afs.fs_avgfilesize);
printf("flags\t");
if (afs.fs_magic == FS_UFS2_MAGIC ||
@@ -296,8 +297,8 @@ dumpfs(int fd, const char *name)
if (pread(fd, (char 

Re: pipex(4): kill NET_LOCK() in pipex_ioctl()

2020-05-29 Thread Martin Pieuchot
On 28/05/20(Thu) 15:27, Vitaliy Makkoveev wrote:
> On Thu, May 28, 2020 at 12:26:39PM +0200, Martin Pieuchot wrote:
> > On 27/05/20(Wed) 11:54, Vitaliy Makkoveev wrote:
> > > pipex(4) is simultaneously protected by KERNEL_LOCK() and NET_LOCK() and
> > > the last is not required. I guess to start remove NET_LOCK(). Diff below
> > > drops NET_LOCK() in pipex_ioctl() and underlaying routines. At least
> > > this helps to kill unlock/lock mess in pppx_add_session() and
> > > pppx_if_destroy().
> > 
> > Getting rid of the NET_LOCK() might indeed help to untangle the current
> > locking mess.  However we should aim towards removing the KERNEL_LOCK()
> > from, at least, the packet processing path starting with pipexintr().
> 
> I guess we can made `pipexoutq' processing NET_LOCK() free. Also
> `pipexinq' processing requires a little amount of places where NET_LOCK()
> is required. So we can implement special locks for pipex(4).

After second though, it seems that the easiest way forward is to protect
`pipex_session_list' by the NET_LOCK().

The rational is that this global list is dereferenced in pipexintr() and
its members are compared to the content of `ph_cookie'.

There's currently no mechanism to ensure the reference saved in `ph_cookie'
is still valid.  That means what ensures the pointer is kind-of correct
is the NET_LOCK().  I'm saying "kind-of" because comparing pointers is
questionable, especially if sessions can be destroyed without purging
`pipexoutq' or `pipexinq'. 

Since the KERNEL_LOCK() is not always held when the network stack runs,
`ph_cookie' can be modified by a CPU without holding it.  So what
actually protects the data structures here is the NET_LOCK().

Does that make sense?



Re: unlock PF_UNIX sockets

2020-05-29 Thread Martin Pieuchot
On 28/05/20(Thu) 14:59, Vitaliy Makkoveev wrote:
> socket(2) layer is already protected by solock(). It grabs NET_LOCK()
> for inet{,6}(4) sockets, but all other sockets are still under
> KERNEL_LOCK().
> 
> I guess solock is already placed everythere it's required. Also `struct
> file' is already mp-safe. 
> 
> Diff below introduces rwlock `unp_lock'. It's like NET_LOCK()'s
> `netlock' but for PF_UNIX sockets. This lock protects the whole PF_UNIX
> layer. Vnode layer accessed in "connect" "bind" and "desconnect" cases
> is still under KERNEL_LOCK().
> 
> I'am experimenting with his diff since 19.04.2020 and my test systems,
> include Gnome workstaion are stable, without any issue.

Looks great, more tests are required :)

Your diff has many trailing spaces, could you remove them?  Commonly
used editors or diff programs have a way to highlight them :)

One comment:

> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.142
> diff -u -p -r1.142 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c16 Jul 2019 21:41:37 -  1.142
> +++ sys/kern/uipc_usrreq.c1 May 2020 13:47:11 -
> @@ -409,12 +418,17 @@ unp_detach(struct unpcb *unp)
>  {
>   struct vnode *vp;
>  
> + rw_assert_wrlock(_lock);
> +
>   LIST_REMOVE(unp, unp_link);
>   if (unp->unp_vnode) {
> + /* this is an unlocked cleaning of `v_socket', but it's safe */
>   unp->unp_vnode->v_socket = NULL;
>   vp = unp->unp_vnode;
>   unp->unp_vnode = NULL;
> + KERNEL_LOCK();
>   vrele(vp);
> + KERNEL_UNLOCK();

Why is it safe?  That's what the comment should explain :)  If the code
doesn't take the lock that implies it is not required.  Why it is not
required is not obvious.



Re: userland clock_gettime proof of concept

2020-05-29 Thread Marc Espie
On Thu, May 28, 2020 at 05:44:31PM +0300, Paul Irofti wrote:
> Hi,
> 
> Here is a new iteration of the diff which includes support for MD high
> resolution clocks. Currently only implements TSC on amd64. If the
> MD function is not defined, it fallsback to the syscall.
> 
> There is the question of the skew fix, but that will be addressed in a
> separate kernel diff that will not affect the current diff at all.
> 
> I could not find a way to find on which processor the process is running
> on from userland without going through a syscall. If there is one please
> let me know. It would make things easier.
> 
> In the meantime I have also gotten positive feedback from various
> testers that run this on their main machine.
> 
> Anyway, I think we can decide on the struct name and the auxiliary
> vector ID and consider this done.
> 
> Thoughts?
> 
> Paul 

This appears to work just fine here on my desktop.

cpu0: AMD A10-5700 APU with Radeon(tm) HD Graphics, 3417.45 MHz, 15-10-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,FMA3,CX16,SSE4.1,SSE4.2,POPCNT,AES,XSAVE,AVX,F16C,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,XOP,SKINIT,WDT,FMA4,TCE,NODEID,TBM,TOPEXT,CPCTR,ITSC,BMI1

(other bits of dmesg available on demand)