Re: OpenBSD 6.2 released: Oct 9, 2017

[R0me0 ***]

Long Life For Magic Puffer Fish!
Without has, OpenBSD in a soul, it would be impossible to achieve the
current magnitude of this amazing Operating System.

Cheers guys!







2017-10-10 0:59 GMT-03:00 Theo de Raadt :

> > newvers.sh in 6.2/sys.tar.gz says it's "-current". it looks strange.
> ..
> > and, there is no top(root) CVS/ in 6.2/src.tar.gz.
>
> Thanks for noticing these errors.
>
> I am going to replace these two files.  It may take a few hours for
> them to propogate.
>
>

Re: OpenBSD 6.2 released: Oct 9, 2017

[Theo de Raadt]

> newvers.sh in 6.2/sys.tar.gz says it's "-current". it looks strange.
..
> and, there is no top(root) CVS/ in 6.2/src.tar.gz.

Thanks for noticing these errors.

I am going to replace these two files.  It may take a few hours for
them to propogate.

armv7/panda usb eth addr wondering

[Artturi Alm]

Hi,

i've been bothered by this:
smsc0 at uhub1 port 1 configuration 1 interface 0 "Standard Microsystems 
SMSC9512/14" rev 2.00/2.00 addr 3
smsc0: address ff:ff:ff:ff:ff:ff

that does happen no matter if i run dhcp-command in u-boot shell or not.
u-boot does store the addr in env, but i don't want to learn how to
access it from efi, i think it wouldn't be pretty.

iirc. the DT is mapped kernel writable, so would anything like below be
accepted for generalizing the kind of quirk there already is in if_smsc.c
for rPI (the smsc_enaddr_OF())?

diff --git a/sys/arch/armv7/stand/efiboot/efiboot.c 
b/sys/arch/armv7/stand/efiboot/efiboot.c
index dbc434da7bf..b8f17fc5921 100644
--- a/sys/arch/armv7/stand/efiboot/efiboot.c
+++ b/sys/arch/armv7/stand/efiboot/efiboot.c
@@ -328,6 +328,12 @@ efi_makebootargs(char *bootargs, uint32_t *board_id)
fdt_node_add_property(node, "openbsd,uefi-mmap-desc-size", zero, 4);
fdt_node_add_property(node, "openbsd,uefi-mmap-desc-ver", zero, 4);
 
+   /*
+* For boards with integrated USB ethernet, to be filled later
+* during autoconf, from efuses/eeprom or such.
+*/
+   fdt_node_add_property(node, "openbsd,uea", zero, 6);
+
fdt_finalize();
 
node = fdt_find_node("/");


or, if hack like above is totally unaccepted for this, some clue how to
solve my issue w/o giving up running GENERIC on panda would be appreciated.
-Artturi

Re: OpenBSD 6.2 released: Oct 9, 2017

[FUKAUMI Naoki]

From: FUKAUMI Naoki 
Subject: Re: OpenBSD 6.2 released: Oct 9, 2017
Date: Tue, 10 Oct 2017 11:53:59 +0900 (JST)

> newvers.sh in 6.2/sys.tar.gz says it's "-current". it looks strange.
> 
> $ sha256 sys.tar.gz  
> SHA256 (sys.tar.gz) = 
> 1fe2c451a6151164a4e97fc07b639413a9846e67406f54578a3ff8ceba9e802f
> $ tar xzf sys.tar.gz sys/conf/newvers.sh 
> $ grep -C5 vers.c sys/conf/newvers.sh
> #   and disable POOL_DEBUG in sys/conf/GENERIC
> 
> ost="OpenBSD"
> osr="6.2"
> 
> cat >vers.c < #define STATUS "-current"   /* just after a release */
> #if 0
> #define STATUS ""   /* release */
> #define STATUS "-beta"  /* just before a release */
> #endif

and, there is no top(root) CVS/ in 6.2/src.tar.gz.

Best regards,

--
FUKAUMI Naoki

Re: efiboot: Restore GOP mode on SetMode() failure

[YASUOKA Masahiko]

On Sat, 7 Oct 2017 09:24:20 +0200
Klemens Nanni  wrote:
> On Sat, Oct 07, 2017 at 01:15:40AM +, YASUOKA Masahiko wrote:
>> > See my updated diff for reusing the gopi struct, please.
>> 
>> ok, but the diff seems to be against wrong revision.  You seems to
>> have other diffs, moving gop and gopi to global at least.
>> 
>> Can you send it entirely?
> My bad, those were applied on top of other (unsubmitted) diffs such as
> https://marc.info/?l=openbsd-tech=150437080106164

Ah, sorry I missed that mail.

On Fri, 6 Oct 2017 20:11:24 +0200
Klemens Nanni  wrote:
> Declaring the gop and gopi strucutures globally makes things easier in
> preparation for the next commit.
> 
> Both changes also improve consistency with regard to other structures
> like ei, di and conout as well.

As for gopi, I'd like to keep it be a local since it is used to refer
various different modes.

also s/efi_gopmode/gopmode/

comments?

Index: sys/arch/amd64/stand/efiboot/efiboot.c
===
RCS file: /cvs/src/sys/arch/amd64/stand/efiboot/efiboot.c,v
retrieving revision 1.24
diff -u -p -r1.24 efiboot.c
--- sys/arch/amd64/stand/efiboot/efiboot.c  6 Oct 2017 04:52:22 -   
1.24
+++ sys/arch/amd64/stand/efiboot/efiboot.c  10 Oct 2017 02:52:46 -
@@ -60,7 +60,7 @@ static voidefi_memprobe_internal(void)
 static void efi_video_init(void);
 static void efi_video_reset(void);
 static EFI_STATUS
-efi_gop_setmode(EFI_GRAPHICS_OUTPUT *gop, int mode);
+efi_gop_setmode(int mode);
 EFI_STATUS  efi_main(EFI_HANDLE, EFI_SYSTEM_TABLE *);
 
 void (*run_i386)(u_long, u_long, int, int, int, int, int, int, int, int)
@@ -696,13 +696,15 @@ efi_com_putc(dev_t dev, int c)
  * {EFI_,}_ACPI_20_TABLE_GUID or EFI_ACPI_TABLE_GUID means
  * ACPI 2.0 or above.
  */
-static EFI_GUID acpi_guid = ACPI_20_TABLE_GUID;
-static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
+static EFI_GUID acpi_guid = ACPI_20_TABLE_GUID;
+static EFI_GUID smbios_guid = SMBIOS_TABLE_GUID;
+static EFI_GRAPHICS_OUTPUT *gop;
+static int  gopmode = -1;
 
 #defineefi_guidcmp(_a, _b) memcmp((_a), (_b), sizeof(EFI_GUID))
 
 static EFI_STATUS
-efi_gop_setmode(EFI_GRAPHICS_OUTPUT *gop, int mode)
+efi_gop_setmode(int mode)
 {
EFI_STATUS  status;
 
@@ -718,14 +720,11 @@ efi_makebootargs(void)
 {
int  i;
EFI_STATUS   status;
-   EFI_GRAPHICS_OUTPUT *gop;
EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
*gopi;
bios_efiinfo_t   ei;
-   int  curmode, bestmode = -1;
+   int  curmode;
UINTNsz, gopsiz, bestsiz = 0;
-   EFI_GRAPHICS_OUTPUT_MODE_INFORMATION
-   *info;
 
memset(, 0, sizeof(ei));
/*
@@ -748,21 +747,24 @@ efi_makebootargs(void)
status = EFI_CALL(BS->LocateProtocol, _guid, NULL,
(void **));
if (!EFI_ERROR(status)) {
-   for (i = 0; i < gop->Mode->MaxMode; i++) {
-   status = EFI_CALL(gop->QueryMode, gop, i, , );
-   if (EFI_ERROR(status))
-   continue;
-   gopsiz = info->HorizontalResolution *
-   info->VerticalResolution;
-   if (gopsiz > bestsiz) {
-   bestmode = i;
-   bestsiz = gopsiz;
+   if (gopmode < 0) {
+   for (i = 0; i < gop->Mode->MaxMode; i++) {
+   status = EFI_CALL(gop->QueryMode, gop,
+   i, , );
+   if (EFI_ERROR(status))
+   continue;
+   gopsiz = gopi->HorizontalResolution *
+   gopi->VerticalResolution;
+   if (gopsiz > bestsiz) {
+   gopmode = i;
+   bestsiz = gopsiz;
+   }
}
}
-   if (bestmode >= 0) {
+   if (gopmode >= 0 && gopmode != gop->Mode->Mode) {
curmode = gop->Mode->Mode;
-   if (efi_gop_setmode(gop, bestmode) != EFI_SUCCESS)
-   (void)efi_gop_setmode(gop, curmode);
+   if (efi_gop_setmode(gopmode) != EFI_SUCCESS)
+   (void)efi_gop_setmode(curmode);
}
 
gopi = gop->Mode->Info;
@@ -882,5 +884,46 @@ int
 Xpoweroff_efi(void)
 {
EFI_CALL(RS->ResetSystem, EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+   return (0);
+}
+
+int

Re: OpenBSD 6.2 released: Oct 9, 2017

[FUKAUMI Naoki]

Hi,

newvers.sh in 6.2/sys.tar.gz says it's "-current". it looks strange.

$ sha256 sys.tar.gz  
SHA256 (sys.tar.gz) = 
1fe2c451a6151164a4e97fc07b639413a9846e67406f54578a3ff8ceba9e802f
$ tar xzf sys.tar.gz sys/conf/newvers.sh 
$ grep -C5 vers.c sys/conf/newvers.sh
#   and disable POOL_DEBUG in sys/conf/GENERIC

ost="OpenBSD"
osr="6.2"

cat >vers.c <

Re: OpenBSD 6.2 released: Oct 9, 2017

[Callum R. Davies]

Huge congrats to all OpenBSD developers for another rock-solid release,
and with so many wonderful improvements to boot!

Just a couple of small nits:

On Mon, Oct 09, 2017 at 08:44:31AM -0600, Theo de Raadt wrote:
> - SYSTEM SOURCE CODE ---
> 
> The source code for all four subsystems can be found in the
> pub/OpenBSD/6.2/ directory:
> 
> xenocara.tar.gz ports.tar.gz   src.tar.gz sys.tar.gz
> 
> The README (https://ftp.OpenBSD.org/pub/OpenBSD/6.2/README) file
> explains how to deal with these source files.

The README file actually makes no mention of what to do with these
files

The README also says:
> - Thanks to everyone who has purchased an OpenBSD CD-ROM.

I sent the sort of small donation which you'd expect from a student, to
help make up for the loss of income from CD sales... ;^)

Re: softdep dangling vnode

[Mike Belopuhov]

On Mon, Oct 09, 2017 at 22:21 +, Alexander Bluhm wrote:
> Hi,
> 
> we sometimes see a panic "unmount: dangling vnode" when rebooting a 6.1
> system with softdep.
> 
> I have hacked some diagnostic panics until I got these traces from the
> reboot and update process.
> 
> Reboot:
> sleep_finish() at sleep_finish+0xb1
> tsleep() at tsleep+0x154
> biowait() at biowait+0x46
> bwrite() at bwrite+0x10d
> ffs_update() at ffs_update+0x2bd
> VOP_FSYNC() at VOP_FSYNC+0x3c
> ffs_flushfiles() at ffs_flushfiles+0xb9
> softdep_flushfiles() at softdep_flushfiles+0x4e
> ffs_unmount() at ffs_unmount+0x49
> dounmount_leaf() at dounmount_leaf+0x8b
> dounmount() at dounmount+0xb2
> vfs_unmountall() at vfs_unmountall+0x72
> vfs_shutdown() at vfs_shutdown+0x79
> boot() at boot+0x144
> reboot() at reboot+0x30
> sys_reboot() at sys_reboot+0x5e
> syscall() at syscall+0x21f
> 
> Update:
> *115878  74431  0 0x14000  0x2000  update
> Debugger() at Debugger+0x9
> panic() at panic+0xfe
> insmntque() at insmntque+0x86
> getnewvnode() at getnewvnode+0x192
> ffs_vget() at ffs_vget+0x8b
> handle_workitem_remove() at handle_workitem_remove+0x4c
> process_worklist_item() at process_worklist_item+0xf5
> softdep_process_worklist() at softdep_process_worklist+0x169
> sched_sync() at sched_sync+0xfb
> 
> At reboot all vnodes are flushed, but when it sleeps, the update
> process has a chance to create new dirty vnodes.  Resolving soft
> dependencies adds vnodes to the dirty list.
> 
> In softdep_flushfiles() vnodes and softdep are flushed in a loop.
> But if they sleep, it is not guaranteed that all vnodes have been
> flushed when the softdep worklist flush reports that nothing has
> been done.
> 
> My solution is to do a final vnode flush after the softdep worklist
> has been flushed.  Then the dirty list is empty and the final check in
> dounmount_leaf() does not panic.
> 
> ok?
> 
> bluhm
>

Makes sense to me.  FreeBSD does something similar:
https://svnweb.freebsd.org/base/head/sys/ufs/ffs/ffs_softdep.c?revision=324039=markup#l1920

> Index: ufs/ffs/ffs_softdep.c
> ===
> RCS file: /data/mirror/openbsd/cvs/src/sys/ufs/ffs/ffs_softdep.c,v
> retrieving revision 1.135
> diff -u -p -r1.135 ffs_softdep.c
> --- ufs/ffs/ffs_softdep.c 7 Nov 2016 00:26:33 -   1.135
> +++ ufs/ffs/ffs_softdep.c 9 Oct 2017 22:19:39 -
> @@ -904,6 +904,14 @@ softdep_flushfiles(struct mount *oldmnt,
>   break;
>   }
>   /*
> +  * If the reboot process sleeps during the loop, the update
> +  * process may call softdep_process_worklist() and create
> +  * new dirty vnodes at the mount point.  Call ffs_flushfiles()
> +  * again after the loop has flushed all soft dependencies.
> +  */
> + if (error == 0)
> + error = ffs_flushfiles(oldmnt, flags, p);
> + /*
>* If we are unmounting then it is an error to fail. If we
>* are simply trying to downgrade to read-only, then filesystem
>* activity can keep us busy forever, so we just fail with EBUSY.
> 

[patch] pkg_sign.1 correction

[Bryan Vyhmeister]

I was reading pkg_sign.1 and noticed that the description for the -o
option seems incorrect. The current pkg_sign.1 text says under the -o
option that "Otherwise, unsigned packages are created in the current
directory" where I think it should say "Otherwise, signed packages are
created in the current directory." It is possible I am misunderstanding
the text and, in that case, further clarification is needed. If the -o
option is omitted, are packages signed and then that new signed package
replaces the previously unsigned package in the current directory?
Attached is a patch that corrects the text according to my understanding
of it.

Bryan


Index: usr.sbin/pkg_add/pkg_sign.1
===
RCS file: /cvs/src/usr.sbin/pkg_add/pkg_sign.1,v
retrieving revision 1.10
diff -u -p -r1.10 pkg_sign.1
--- usr.sbin/pkg_add/pkg_sign.1 15 Feb 2017 13:19:08 -  1.10
+++ usr.sbin/pkg_add/pkg_sign.1 9 Oct 2017 22:49:13 -
@@ -68,7 +68,7 @@ in the listing.
 Sign existing packages in parallel.
 .It Fl o Ar dir
 Specify output directory for signing packages.
-Otherwise, unsigned packages are created in the current directory.
+Otherwise, signed packages are created in the current directory.
 .It Fl S Ar source
 Source repository for packages to be signed.
 .\" This can be any url admissible for a

softdep dangling vnode

[Alexander Bluhm]

Hi,

we sometimes see a panic "unmount: dangling vnode" when rebooting a 6.1
system with softdep.

I have hacked some diagnostic panics until I got these traces from the
reboot and update process.

Reboot:
sleep_finish() at sleep_finish+0xb1
tsleep() at tsleep+0x154
biowait() at biowait+0x46
bwrite() at bwrite+0x10d
ffs_update() at ffs_update+0x2bd
VOP_FSYNC() at VOP_FSYNC+0x3c
ffs_flushfiles() at ffs_flushfiles+0xb9
softdep_flushfiles() at softdep_flushfiles+0x4e
ffs_unmount() at ffs_unmount+0x49
dounmount_leaf() at dounmount_leaf+0x8b
dounmount() at dounmount+0xb2
vfs_unmountall() at vfs_unmountall+0x72
vfs_shutdown() at vfs_shutdown+0x79
boot() at boot+0x144
reboot() at reboot+0x30
sys_reboot() at sys_reboot+0x5e
syscall() at syscall+0x21f

Update:
*115878  74431  0 0x14000  0x2000  update
Debugger() at Debugger+0x9
panic() at panic+0xfe
insmntque() at insmntque+0x86
getnewvnode() at getnewvnode+0x192
ffs_vget() at ffs_vget+0x8b
handle_workitem_remove() at handle_workitem_remove+0x4c
process_worklist_item() at process_worklist_item+0xf5
softdep_process_worklist() at softdep_process_worklist+0x169
sched_sync() at sched_sync+0xfb

At reboot all vnodes are flushed, but when it sleeps, the update
process has a chance to create new dirty vnodes.  Resolving soft
dependencies adds vnodes to the dirty list.

In softdep_flushfiles() vnodes and softdep are flushed in a loop.
But if they sleep, it is not guaranteed that all vnodes have been
flushed when the softdep worklist flush reports that nothing has
been done.

My solution is to do a final vnode flush after the softdep worklist
has been flushed.  Then the dirty list is empty and the final check in
dounmount_leaf() does not panic.

ok?

bluhm

Index: ufs/ffs/ffs_softdep.c
===
RCS file: /data/mirror/openbsd/cvs/src/sys/ufs/ffs/ffs_softdep.c,v
retrieving revision 1.135
diff -u -p -r1.135 ffs_softdep.c
--- ufs/ffs/ffs_softdep.c   7 Nov 2016 00:26:33 -   1.135
+++ ufs/ffs/ffs_softdep.c   9 Oct 2017 22:19:39 -
@@ -904,6 +904,14 @@ softdep_flushfiles(struct mount *oldmnt,
break;
}
/*
+* If the reboot process sleeps during the loop, the update
+* process may call softdep_process_worklist() and create
+* new dirty vnodes at the mount point.  Call ffs_flushfiles()
+* again after the loop has flushed all soft dependencies.
+*/
+   if (error == 0)
+   error = ffs_flushfiles(oldmnt, flags, p);
+   /*
 * If we are unmounting then it is an error to fail. If we
 * are simply trying to downgrade to read-only, then filesystem
 * activity can keep us busy forever, so we just fail with EBUSY.

Re: pow() returns a negative result on loongson

[Paul Irofti]

On Mon, Oct 09, 2017 at 10:39:47PM +0200, Juan Francisco Cantero Hurtado wrote:
> Marc Feeley (Gambit Scheme) has been helping me with a bug on Gambit on
> Loongson. Apparently the bug is on our side.
> 
> I've created this little test based on his code:
> 
> #include 
> #include 
> 
> int main()
> {
> double x = 0.5;
> double y = 1074.0;
> printf("x=%.20g y=%.20g pow(x,y)=%.20g\n",x,y,pow(x,y));
> return 0;
> }
> 
> On OpenBSD/amd64, Linux/amd64, Linux/aarch64 and Linux/mips64:
> x=0.5 y=1074 pow(x,y)=4.9406564584124654418e-324
> 
> On OpenBSD/loongson:
> x=0.5 y=1074 pow(x,y)=-4.9406564584124654418e-324
> 
> Is the negative result expected?

No, even though what you are computing is ridiculous :)

My first thought would on Loongson would be softfloat bug.
But I am not 100% sure it (still) uses softfloat.

Re: Add SIMPLEQ_REMOVE to sys/queue.h

[Paul Irofti]

On Mon, Oct 09, 2017 at 11:21:24PM +0200, Alexander Bluhm wrote:
> On Tue, Oct 10, 2017 at 12:07:31AM +0300, Paul Irofti wrote:
> > I needed this in a program and searching the kernel I see that this is
> > also hacked in a few places (e.g. mpii(4)).
> 
> Why don't you use a double linked list?  I consider that removing
> with a loop is a hack instead of using as suitable data structure.
> The problem is that if such macros exist, people start using them
> without thinking whether they have chosen the correct type.

It depends. If this hack is a one-time thing, it might be worth doing it
considering the global advantages of single-linked versus dobule-linked
data structures.

If the function call is abused, then I agree that switching to a better
data structure would be the best move.

If you guys want it fine, if not I'll keep it locally. No worries :)

Re: Add SIMPLEQ_REMOVE to sys/queue.h

[Alexander Bluhm]

On Tue, Oct 10, 2017 at 12:07:31AM +0300, Paul Irofti wrote:
> I needed this in a program and searching the kernel I see that this is
> also hacked in a few places (e.g. mpii(4)).

Why don't you use a double linked list?  I consider that removing
with a loop is a hack instead of using as suitable data structure.
The problem is that if such macros exist, people start using them
without thinking whether they have chosen the correct type.

bluhm

Add SIMPLEQ_REMOVE to sys/queue.h

[Paul Irofti]

I needed this in a program and searching the kernel I see that this is
also hacked in a few places (e.g. mpii(4)).

Here is a small regression test
---
#include 
#include 

#include 

SIMPLEQ_HEAD(listhead, entry) head = SIMPLEQ_HEAD_INITIALIZER(head);
struct entry {
int i;  /* data */
SIMPLEQ_ENTRY(entry) entries;   /* Simple queue. */
} *n1, *n2, *n3, *np;

void
printq()
{
SIMPLEQ_FOREACH(np, , entries)
printf("%d", np->i);
printf("\n");
}

int main()
{
int i = 0;

n1 = malloc(sizeof(struct entry));  /* Insert at the head. */
n1->i = i++;
SIMPLEQ_INSERT_HEAD(, n1, entries);

n2 = malloc(sizeof(struct entry));  /* Insert after. */
n2->i = i++;
SIMPLEQ_INSERT_AFTER(, n1, n2, entries);

n3 = malloc(sizeof(struct entry));  /* Insert at the tail. */
n3->i = i++;
SIMPLEQ_INSERT_TAIL(, n3, entries);

printq();
SIMPLEQ_REMOVE(, n2, entry, entries);
printq();

while (!SIMPLEQ_EMPTY()) {
n1 = SIMPLEQ_FIRST();
SIMPLEQ_REMOVE(, n1, entry, entries);
free(n1);
printq();
}
return 0;
}
---
that outputs
---
$ ./test_sqrm
012
02
2

---

Thoughts? OK?

Index: queue.h
===
RCS file: /cvs/src/sys/sys/queue.h,v
retrieving revision 1.44
diff -u -p -u -p -r1.44 queue.h
--- queue.h 9 Sep 2016 20:31:46 -   1.44
+++ queue.h 9 Oct 2017 21:06:37 -
@@ -315,6 +315,18 @@ struct {   
\
(head)->sqh_last = &(elm)->field.sqe_next;  \
 } while (0)
 
+#define SIMPLEQ_REMOVE(head, elm, type, field) do {\
+   if ((head)->sqh_first == (elm)) \
+   SIMPLEQ_REMOVE_HEAD((head), field); \
+   else {  \
+   struct type *curelm = (head)->sqh_first;\
+   \
+   while (curelm->field.sqe_next != (elm)) \
+   curelm = curelm->field.sqe_next;\
+   SIMPLEQ_REMOVE_AFTER((head), curelm, field);\
+   }   \
+} while (0)
+
 #define SIMPLEQ_CONCAT(head1, head2) do {  \
if (!SIMPLEQ_EMPTY((head2))) {  \
*(head1)->sqh_last = (head2)->sqh_first;\

pow() returns a negative result on loongson

[Juan Francisco Cantero Hurtado]

Marc Feeley (Gambit Scheme) has been helping me with a bug on Gambit on
Loongson. Apparently the bug is on our side.

I've created this little test based on his code:

#include 
#include 

int main()
{
double x = 0.5;
double y = 1074.0;
printf("x=%.20g y=%.20g pow(x,y)=%.20g\n",x,y,pow(x,y));
return 0;
}

On OpenBSD/amd64, Linux/amd64, Linux/aarch64 and Linux/mips64:
x=0.5 y=1074 pow(x,y)=4.9406564584124654418e-324

On OpenBSD/loongson:
x=0.5 y=1074 pow(x,y)=-4.9406564584124654418e-324

Is the negative result expected?


-- 
Juan Francisco Cantero Hurtado http://juanfra.info

rw locks vs memory barriers and adaptive spinning

[Mateusz Guzik]

Hello,

I was looking at rw lock code out of curiosity and noticed you always do
membar_enter which on MP-enabled amd64 kernel translates to mfence.
This makes the entire business a little bit slower.

Interestingly you already have relevant macros for amd64:
#define membar_enter_after_atomic() __membar("")
#define membar_exit_before_atomic() __membar("")

And there is even a default variant for archs which don't provide one.
I guess the switch should be easy.

Grabbing for reading is rw_cas to a higher value. On failure you
explicitly re-read from the lock  This is slower than necessary in
presence of concurrent read lock/unlock since cas returns the found
value and you can use that instead.

Also the read lock fast path does not have to descent to the slow path
after a single cas failure, but this probably does not matter right now.

The actual question I have here is if you played with adaptive spinning
instead of instantly putting threads to sleep at least for cases when
the kernel lock is not held. This can be as simplistic as just spinning
as long as the lock is owned by a running thread.

For cases where curproc holds the kernel lock you can perhaps drop it
for spinning purposes and reacquire later. Although I have no idea if
this one is going to help anything. Definitely worth testing imo.

A side note is that the global locks I found are not annotated in any
manner with respect to exclusivity of cacheline placement. In particular
netlock in a 6.2 kernel shares its chaceline with if_input_task_locked:


81aca608 D if_input_task_locked
81aca630 D netlock


The standard boiler plate to deal with it is to annotate with aligned()
and place the variable in a dedicated section. FreeBSD and NetBSD
contain the necessary crappery to copy-paste including linker script
support.

Cheers,
-- 
Mateusz Guzik 

Re: Installer support to fetch/verify bsd.rd for release upgrade

[Robert Peichaer]

On Sun, Oct 08, 2017 at 09:56:15AM +, Robert Peichaer wrote:
> Up to now, the upgrade procedure from one to the next release meant
> that you had to manually download and verify the new ramdisk kernel.
> 
> What about if you just needed to boot into the existing bsd.rd and
> it would support downloading and verifying the bsd.rd of the next
> release?
> 
> This diff changes the installer script to support such a scenario.
> 
> 1) Boot the existing bsd.rd and choose (U)pgrade
> 2) Enter the "Server directory" of the new release
>The installer then offers just the bsd.rd
>The on-disk signify key of the new release is used for verify it
> 3) Reboot into the new bsd.rd and do the upgrade
> 
> 
> An important assumption for this to work properly is:
> 
>Upgrades are only supported from one release to the release
>immediately following it. [1]
> 
> 
> It would look like this for the 6.2 to 6.3 upgrade situation.
> (The version numbers are obviously faked)
> 
>   Let's upgrade the sets!
>   Location of sets? (cd0 disk http or 'done') [http]
>   HTTP proxy URL? (e.g. 'http://proxy:8080', or 'none') [none]
>   HTTP Server? (hostname, list#, 'done' or '?') [ftp.hostserver.de]
>   Server directory? [pub/OpenBSD/6.2/amd64] pub/OpenBSD/6.3/amd64
>   Unable to get a verified list of distribution sets.
>   
>   Select sets by entering a set name, a file name pattern or 'all'. De-select
>   sets by prepending a '-', e.g.: '-game*'. Selected sets are labelled '[X]'.
>   [X] bsd.rd
>   Set name(s)? (or 'abort' or 'done') [done]
>   Get/Verify SHA256.sig   100% |**|  2152   00:00
>   Signature Verified
>   Get/Verify bsd.rd   100% |**|  9565 KB00:14
>   Installing bsd.rd   100% |**|  9565 KB00:00
>   Location of sets? (cd0 disk http or 'done') [done]
>   Making all device nodes...done.
>   
>   CONGRATULATIONS! Your OpenBSD upgrade has been successfully completed!
>   To boot the new system, enter 'reboot' at the command prompt.
> 
> 
> Here's the diff and below is a more detailed description.

Well, here's a diff, that actually works.


Index: install.sub
===
RCS file: /cvs/src/distrib/miniroot/install.sub,v
retrieving revision 1.1036
diff -u -p -p -u -r1.1036 install.sub
--- install.sub 4 Oct 2017 18:19:33 -   1.1036
+++ install.sub 9 Oct 2017 17:41:59 -
@@ -1330,6 +1330,15 @@ sane_install() {
 #
 select_sets() {
local _avail=$1 _selected=$2 _f _action _col=$COLUMNS
+   local _bsd_rd _no_sets=true
+
+   if [[ $MODE == upgrade ]]; then
+   for _f in $_avail; do
+   [[ $_f != bsd* ]] && _no_sets=false
+   [[ $_f == bsd.rd* ]] && _bsd_rd=$_f
+   done
+   $_no_sets && UPGRADE_BSDRD=true _avail=$_bsd_rd 
_selected=$_bsd_rd
+   fi
 
# account for 4 spaces added to the sets list
let COLUMNS=_col-8
@@ -1516,6 +1525,9 @@ install_files() {
! $_unpriv ftp -D "$_t" -Vmo - "$_src/SHA256.sig" 
>"$_cfile.sig" &&
_issue="Cannot fetch SHA256.sig" && break
 
+   $UPGRADE_BSDRD &&
+   PUB_KEY=/mnt/etc/signify/openbsd-$((VERSION + 
1))-base.pub
+
# Verify signature file with public keys.
! unpriv -f "$_cfile" \
signify -Vep $PUB_KEY -x "$_cfile.sig" -m "$_cfile" &&
@@ -1576,7 +1588,9 @@ install_files() {
tar -zxphf - -C /mnt
fi
;;
-   *)  $_unpriv ftp -D Installing -Vmo - "$_fsrc" >"/mnt/$_f"
+   *)  $UPGRADE_BSDRD && [[ $_f == bsd.rd* ]] &&
+   cp /mnt/$_f /mnt/$_f.old.$VERSION
+   $_unpriv ftp -D Installing -Vmo - "$_fsrc" >"/mnt/$_f"
;;
esac
if (($?)); then
@@ -1587,6 +1601,7 @@ install_files() {
fi
else
DEFAULTSETS=$(rmel $_f $DEFAULTSETS)
+   $UPGRADE_BSDRD && DEFAULTSETS=
fi
[[ -d $_tmpsrc ]] && rm -f "$_tmpsrc/$_f"
done
@@ -3139,6 +3154,7 @@ PUB_KEY=/etc/signify/openbsd-${VERSION}-
 ROOTDEV=
 ROOTDISK=
 SETDIR="$VNAME/$ARCH"
+UPGRADE_BSDRD=false
 V4_DHCPCONF=false
 V6_AUTOCONF=false
 WLANLIST=/tmp/i/wlanlist
===
Stats: --- 1 lines 60 chars
Stats: +++ 17 lines 525 chars
Stats: 16 lines
Stats: 465 chars

-- 
-=[rpe]=-

Re: document tar -vv

[Ingo Schwarze]

Hi Todd,

Todd C. Miller wrote on Mon, Oct 09, 2017 at 09:35:44AM -0600:
> On Mon, 09 Oct 2017 17:27:46 +0200, Ingo Schwarze wrote:

>> Given that tar(1) is not even specified by POSIX, i don't see
>> how documenting it might do harm.  The option was introduced
>> here:
>> 
>>   date: 1997/01/24 19:41:23;  author: millert;
>>   Support multiple -v options like GNU tar (> 1 -v means do ls-like output).
>> 
>> FreeBSD copied it to their tree in 2001.  So it seems firmly
>> established in the free software ecosystem.

> Basically, tar -vvxf produces the same format output as tar -tvf.
> If we are going to document the long format, how about something
> like:
[...]
> .It Fl v
> Verbose operation mode.
> If
> .Fl v
> is specified multiple times or if the
> .Fl t
> option is also specified,
> .Nm
> will use a long format for listing files, similar to
> .Xr ls 1
> .Fl l .

That's better than my patch, please commit that.

Your shorter version would also be OK schwarze@, but i prefer
this one.  It avoids potential doubt about what "list mode" means.

Yours,
  Ingo

Re: document tar -vv

[Todd C. Miller]

On Mon, 09 Oct 2017 17:27:46 +0200, Ingo Schwarze wrote:

> Given that tar(1) is not even specified by POSIX, i don't see
> how documenting it might do harm.  The option was introduced
> here:
> 
>   date: 1997/01/24 19:41:23;  author: millert;
>   Support multiple -v options like GNU tar (> 1 -v means do ls-like output).
> 
> FreeBSD copied it to their tree in 2001.  So it seems firmly
> established in the free software ecosystem.

Basically, tar -vvxf produces the same format output as tar -tvf.
If we are going to document the long format, how about something
like:

.It Fl v
Verbose operation mode.
In list mode, or if
.Fl v
is specified multiple times,
.Nm
will use a long format for listing files, similar to
.Xr ls 1
.Fl l .

Or perhaps:

.It Fl v
Verbose operation mode.
If
.Fl v
is specified multiple times or if the
.Fl t
option is also specified,
.Nm
will use a long format for listing files, similar to
.Xr ls 1
.Fl l .

 - todd

document tar -vv

[Ingo Schwarze]

Hi,

Philippe Meunier wrote on Mon, Oct 09, 2017 at 08:03:12AM -0400:
> leo_...@volny.cz wrote:

>> % tar cvvf -

> On a related note, it would be nice if tar(1)'s man page indicated
> that the -v option can be specified more than once to get extra
> information.  Until seeing this discussion thread I had never realized
> this was possible.

Good point, i didn't know either.

Given that tar(1) is not even specified by POSIX, i don't see
how documenting it might do harm.  The option was introduced
here:

  date: 1997/01/24 19:41:23;  author: millert;
  Support multiple -v options like GNU tar (> 1 -v means do ls-like output).

FreeBSD copied it to their tree in 2001.  So it seems firmly
established in the free software ecosystem.

OK?
  Ingo


Index: tar.1
===
RCS file: /cvs/src/bin/pax/tar.1,v
retrieving revision 1.59
diff -u -r1.59 tar.1
--- tar.1   13 Sep 2015 15:42:11 -  1.59
+++ tar.1   9 Oct 2017 15:25:57 -
@@ -266,6 +266,11 @@
 are not selected and will be skipped.
 .It Fl v
 Verbose operation mode.
+With a second
+.Fl v ,
+always use the long format for listing files, like
+.Xr ls 1
+.Fl l .
 .It Fl w
 Interactively rename files.
 This option causes

Re: pms: tests with older Elantech touchpads needed

[Stefan Sperling]

On Mon, Oct 09, 2017 at 12:41:18AM +0200, Ulf Brosziewski wrote:
> This patch adapts the Elantech handlers in pms to the new touchpad
> infrastructure of wsmouse.  The changes concern models that use the older
> protocol versions 1, 2, and 3.  You don't find that hardware at the next
> corner nowadays, so if you have a laptop with one of those models, your
> help would be greatly appreciated.  With the patch, synaptics(4) should
> work as before, and a configuration with ws(4) should run decently, see
> https://marc.info/?l=openbsd-misc=150153498920367=2
> 

No regressions on:

pms0 at pckbc0 (aux slot)
wsmouse0 at pms0 mux 0
pms0: Elantech Touchpad, version 3, firmware 0x250f00

I don't have any other devices to test with.

ok stsp@

> Index: pms.c
> ===
> RCS file: /cvs/src/sys/dev/pckbc/pms.c,v
> retrieving revision 1.80
> diff -u -p -r1.80 pms.c
> --- pms.c 26 Aug 2017 18:34:04 -  1.80
> +++ pms.c 8 Oct 2017 20:17:09 -
> @@ -130,9 +130,6 @@ struct elantech_softc {
>  #define ELANTECH_F_CRC_ENABLED   0x10
>   int fw_version;
> 
> - int min_x, min_y;
> - int max_x, max_y;
> -
>   u_int mt_slots;
> 
>   int width;
> @@ -140,10 +137,8 @@ struct elantech_softc {
>   u_char parity[256];
>   u_char p1, p2, p3;
> 
> - /* Compat mode */
> - int wsmode;
> + int max_x, max_y;
>   int old_x, old_y;
> - u_int old_buttons;
>  };
> 
>  struct pms_softc {   /* driver status information */
> @@ -303,7 +298,6 @@ int   alps_sec_proc(struct pms_softc *);
>  int  alps_get_hwinfo(struct pms_softc *);
> 
>  int  elantech_knock(struct pms_softc *);
> -void elantech_send_input(struct pms_softc *, int, int, int, int);
>  int  elantech_get_hwinfo_v1(struct pms_softc *);
>  int  elantech_get_hwinfo_v2(struct pms_softc *);
>  int  elantech_get_hwinfo_v3(struct pms_softc *);
> @@ -1709,6 +1703,7 @@ int
>  elantech_get_hwinfo_v1(struct pms_softc *sc)
>  {
>   struct elantech_softc *elantech = sc->elantech;
> + struct wsmousehw *hw;
>   int fw_version;
>   u_char capabilities[3];
> 
> @@ -1733,10 +1728,13 @@ elantech_get_hwinfo_v1(struct pms_softc
>   if (elantech_set_absolute_mode_v1(sc))
>   return (-1);
> 
> - elantech->min_x = ELANTECH_V1_X_MIN;
> - elantech->max_x = ELANTECH_V1_X_MAX;
> - elantech->min_y = ELANTECH_V1_Y_MIN;
> - elantech->max_y = ELANTECH_V1_Y_MAX;
> + hw = wsmouse_get_hw(sc->sc_wsmousedev);
> + hw->type = WSMOUSE_TYPE_ELANTECH;
> + hw->hw_type = WSMOUSEHW_TOUCHPAD;
> + hw->x_min = ELANTECH_V1_X_MIN;
> + hw->x_max = ELANTECH_V1_X_MAX;
> + hw->y_min = ELANTECH_V1_Y_MIN;
> + hw->y_max = ELANTECH_V1_Y_MAX;
> 
>   return (0);
>  }
> @@ -1745,6 +1743,7 @@ int
>  elantech_get_hwinfo_v2(struct pms_softc *sc)
>  {
>   struct elantech_softc *elantech = sc->elantech;
> + struct wsmousehw *hw;
>   int fw_version, ic_ver;
>   u_char capabilities[3];
>   int i, fixed_dpi;
> @@ -1768,10 +1767,14 @@ elantech_get_hwinfo_v2(struct pms_softc
>   if (elantech_set_absolute_mode_v2(sc))
>   return (-1);
> 
> + hw = wsmouse_get_hw(sc->sc_wsmousedev);
> + hw->type = WSMOUSE_TYPE_ELANTECH;
> + hw->hw_type = WSMOUSEHW_TOUCHPAD;
> +
>   if (fw_version == 0x20800 || fw_version == 0x20b00 ||
>   fw_version == 0x20030) {
> - elantech->max_x = ELANTECH_V2_X_MAX;
> - elantech->max_y = ELANTECH_V2_Y_MAX;
> + hw->x_max = ELANTECH_V2_X_MAX;
> + hw->y_max = ELANTECH_V2_Y_MAX;
>   } else {
>   if (pms_spec_cmd(sc, ELANTECH_QUE_FW_ID) ||
>   pms_get_status(sc, resp))
> @@ -1782,17 +1785,17 @@ elantech_get_hwinfo_v2(struct pms_softc
>   if (pms_spec_cmd(sc, ELANTECH_QUE_SAMPLE) ||
>   pms_get_status(sc, resp))
>   return (-1);
> - elantech->max_x = (capabilities[1] - i) * resp[1] / 2;
> - elantech->max_y = (capabilities[2] - i) * resp[2] / 2;
> + hw->x_max = (capabilities[1] - i) * resp[1] / 2;
> + hw->y_max = (capabilities[2] - i) * resp[2] / 2;
>   } else if (fw_version == 0x040216) {
> - elantech->max_x = 819;
> - elantech->max_y = 405;
> + hw->x_max = 819;
> + hw->y_max = 405;
>   } else if (fw_version == 0x040219 || fw_version == 0x040215) {
> - elantech->max_x = 900;
> - elantech->max_y = 500;
> + hw->x_max = 900;
> + hw->y_max = 500;
>   } else {
> - elantech->max_x = (capabilities[1] - i) * 64;
> - elantech->max_y = (capabilities[2] - i) * 64;
> + hw->x_max = 

OpenBSD 6.2 released: Oct 9, 2017

[Theo de Raadt]

- OpenBSD 6.2 RELEASED -

October 9, 2017.

We are pleased to announce the official release of OpenBSD 6.2.
This is our 43rd release.  We remain proud of OpenBSD's record of more
than twenty years with only two remote holes in the default install.

As in our previous releases, 6.2 provides significant improvements,
including new features, in nearly all areas of the system:

 - Improved hardware support, including:
o arm: New rkgrf(4) driver for the Rockchip RK3399/RK3288 register
  file.
o arm: New rkclock(4) driver for Rockchip RK3399/RK3288 clocks.
o arm: New rkpinctrl(4) driver for controlling Rockchip
  RK3399/RK3288 pins.
o arm: New rkgpio(4) driver for GPIO on Rockchip SoCs.
o arm: New rktemp(4) driver for Rockchip RK3399 temperature sensors.
o arm: New rkiic(4) driver for Rockchip RK3399 I2C controllers.
o arm: New rkpmic(4) driver for the RK808 Power Management IC.
o arm: New dwmmc(4) driver for Synopsis DesignWare SD/MMC
  controllers.
o arm: New dwdog(4) driver for the Synopsys DesignWare watchdog
  timer.
o arm: New dwxe(4) driver for the Synopsys DesignWare Ethernet
  controller.
o arm: New sxitwi(4) driver for the two-wire bus on Allwinner SoCs.
o arm: New axppmic(4) driver for the AXP209 I2C PMIC.
o arm: New bcmaux(4) driver for clocks and interrupts on the
  auxilliary UART on BCM2835 devices.
o arm: New mvmpic(4) driver for an interrupt controller on Marvell
  ARMADA 38x.
o arm: New mvpxa(4) driver for the SD Host Controller on Marvell
  ARMADA 38x.
o arm: New mvpinctrl(4) driver to configure pins on Marvell ARMADA
  38x.
o arm: New mvneta(4) driver the Ethernet controller on Marvell
  ARMADA 38x.
o arm: New amdisplay(4) & nxphdmi(4) drivers for the Texas
  Instruments AM335x LCD controller.
o octeon: New octcib(4) driver for the interrupt bus widget on
  CN70xx/CN71xx.
o octeon: New octcit(4) driver for the central interrupt unit
  version 3 on CN72xx/CN73xx/CN77xx/CN78xx.
o octeon: New octsctl(4) driver for the OCTEON SATA controller
  bridge.
o octeon: New octxctl(4) driver for the OCTEON USB3 controller
  bridge.
o octeon: Rhino Labs Inc. SDNA Shasta, and Ubiquiti Networks
  EdgeRouter 4 and 6 are now supported.
o New hvs(4) driver for Hyper-V storage.
o New pcxrtc(4) driver for the NXP PCF8563 Real Time Clock.
o New urng(4) driver for USB random number generator devices.
o Intel 8265 and 3168 support was added to the iwm(4) driver.
o RTL8192CE support was added to the rtwn(4) driver.
o RT5360 support was added to the ral(4) driver.
o RTS525A support was added to the rtsx(4) driver.
o The acpibat(4) driver now supports _BIX entries from ACPI 4.0.
o ACPI hibernate support was added to the nvme(4) driver.
o Substantially improved ACPI hibernate performance in the ahci(4)
  driver.
o The inteldrm(4) driver was updated to code based on Linux 4.4.70 -
  it now supports Skylake, Kaby Lake, and Cherryview devices and has
  better support for Broadwell and Valleyview devices.
o The puc(4) driver now supports ASIX AX99100 devices.
o Xen platform support and the xbf(4) driver in particular have been
  substantially improved.
o The nvme(4) driver now reports correct last sector address to
  SCSI, allowing a valid GPT to be created.
o Repair ioapic(4) misconfigurations.

 - vmm(4)/ vmd(8) improvements:
o vmctl(8) supports paused VM migration and memory snapshotting
  using send and receive commands.
o VPID/ASID reuse/rollover in vmm(4).
o SGABIOS imported as an option ROM payload in SeaBIOS (for VGA to
  serial console redirection).
o vmd(8) resets the guest VM RTC (real time clock) on host resume
  from suspend/hibernate (OpenBSD guests only).
o Allow guest VMs access to AVX/AVX2 host CPU features.
o Support for AMD SVM/RVI hosts.
o Allow larger guest VM memory sizes (up to MAXDSIZ sized guests -
  e.g. 32GB on amd64 hosts).
o Better handling of guest VM MONITOR/MWAIT and HLT instructions.
o Various device emulation improvements in vmd(8).
o Increase the virtio(4) queue size provided by vmd(8) from 64 to
  128 entries, to increase performance.
o Many fixes to vmctl(8) and vmd(8) error handling.

 - IEEE 802.11 wireless stack improvements:
o MiRA 802.11n TX rate scaling now supports devices with unequal
  numbers of Tx and Rx streams. Fixes 11n mode for some athn(8)
  devices.
o The iwn(8) and iwm(8) drivers will now start scanning for a new
  access point if they no longer receive beacons from the current
  AP.
o Prefer the 5GHz band over the 2GHz band during access point
  selection.
o Improved debug output in dmesg(8) when a wireless interface 

ifioctl() cleanup

[Martin Pieuchot]

Here's a small cleanup to make it easy to push the NET_LOCK() further
down.

Diff below factorize all ioctl requests need root privileges in one
switch () block.

It moves SIOCIFAFATTACH/SIOCIFAFDETACH where it belongs, to the block
that keeps track of the timestamps of the last change.

It gets rid of unnecessary `sdl' variable.

ok?

Index: net/if.c
===
RCS file: /cvs/src/sys/net/if.c,v
retrieving revision 1.513
diff -u -p -r1.513 if.c
--- net/if.c9 Oct 2017 08:35:38 -   1.513
+++ net/if.c9 Oct 2017 14:27:35 -
@@ -1816,10 +1816,9 @@ int
 ifioctl(struct socket *so, u_long cmd, caddr_t data, struct proc *p)
 {
struct ifnet *ifp;
-   struct ifreq *ifr;
-   struct sockaddr_dl *sdl;
+   struct ifreq *ifr = (struct ifreq *)data;
struct ifgroupreq *ifgr;
-   struct if_afreq *ifar;
+   struct if_afreq *ifar = (struct if_afreq *)data;
char ifdescrbuf[IFDESCRSIZE];
char ifrtlabelbuf[RTLABEL_LEN];
int s, error = 0, oif_xflags;
@@ -1828,17 +1827,49 @@ ifioctl(struct socket *so, u_long cmd, c
const char *label;
 
switch (cmd) {
-
-   case SIOCGIFCONF:
-   return (ifconf(cmd, data));
+   case SIOCIFCREATE:
+   case SIOCIFDESTROY:
+   case SIOCSIFGATTR:
+   case SIOCIFAFATTACH:
+   case SIOCIFAFDETACH:
+   case SIOCSIFFLAGS:
+   case SIOCSIFXFLAGS:
+   case SIOCSIFMETRIC:
+   case SIOCSIFMTU:
+   case SIOCSIFPHYADDR:
+   case SIOCDIFPHYADDR:
+#ifdef INET6
+   case SIOCSIFPHYADDR_IN6:
+#endif
+   case SIOCSLIFPHYADDR:
+   case SIOCSLIFPHYRTABLE:
+   case SIOCSLIFPHYTTL:
+   case SIOCADDMULTI:
+   case SIOCDELMULTI:
+   case SIOCSIFMEDIA:
+   case SIOCSVNETID:
+   case SIOCSIFPAIR:
+   case SIOCSIFPARENT:
+   case SIOCDIFPARENT:
+   case SIOCSIFDESCR:
+   case SIOCSIFRTLABEL:
+   case SIOCSIFPRIORITY:
+   case SIOCSIFRDOMAIN:
+   case SIOCAIFGROUP:
+   case SIOCDIFGROUP:
+   case SIOCSIFLLADDR:
+   case SIOCSIFLLPRIO:
+   if ((error = suser(p, 0)) != 0)
+   return (error);
+   default:
+   break;
}
-   ifr = (struct ifreq *)data;
 
switch (cmd) {
+   case SIOCGIFCONF:
+   return (ifconf(cmd, data));
case SIOCIFCREATE:
case SIOCIFDESTROY:
-   if ((error = suser(p, 0)) != 0)
-   return (error);
return ((cmd == SIOCIFCREATE) ?
if_clone_create(ifr->ifr_name, 0) :
if_clone_destroy(ifr->ifr_name));
@@ -1849,17 +1880,17 @@ ifioctl(struct socket *so, u_long cmd, c
case SIOCGIFGATTR:
return (if_getgroupattribs(data));
case SIOCSIFGATTR:
-   if ((error = suser(p, 0)) != 0)
-   return (error);
return (if_setgroupattribs(data));
+   }
+
+   ifp = ifunit(ifr->ifr_name);
+   if (ifp == NULL)
+   return (ENXIO);
+   oif_flags = ifp->if_flags;
+
+   switch (cmd) {
case SIOCIFAFATTACH:
case SIOCIFAFDETACH:
-   if ((error = suser(p, 0)) != 0)
-   return (error);
-   ifar = (struct if_afreq *)data;
-   if ((ifp = ifunit(ifar->ifar_name)) == NULL)
-   return (ENXIO);
-   oif_flags = ifp->if_flags;
oif_xflags = ifp->if_xflags;
switch (ifar->ifar_af) {
case AF_INET:
@@ -1880,15 +1911,7 @@ ifioctl(struct socket *so, u_long cmd, c
}
if (oif_flags != ifp->if_flags || oif_xflags != ifp->if_xflags)
rtm_ifchg(ifp);
-   return (error);
-   }
-
-   ifp = ifunit(ifr->ifr_name);
-   if (ifp == 0)
-   return (ENXIO);
-   oif_flags = ifp->if_flags;
-   switch (cmd) {
-
+   break;
case SIOCGIFFLAGS:
ifr->ifr_flags = ifp->if_flags;
if (ifq_is_oactive(>if_snd))
@@ -1919,9 +1942,6 @@ ifioctl(struct socket *so, u_long cmd, c
}
 
case SIOCSIFFLAGS:
-   if ((error = suser(p, 0)) != 0)
-   return (error);
-
ifp->if_flags = (ifp->if_flags & IFF_CANTCHANGE) |
(ifr->ifr_flags & ~IFF_CANTCHANGE);
 
@@ -1944,9 +1964,6 @@ ifioctl(struct socket *so, u_long cmd, c
break;
 
case SIOCSIFXFLAGS:
-   if ((error = suser(p, 0)) != 0)
-   return (error);
-
 #ifdef INET6
if (ISSET(ifr->ifr_flags, IFXF_AUTOCONF6)) {
error = in6_ifattach(ifp);
@@ -2006,14 +2023,10 @@ ifioctl(struct socket *so, u_long cmd, c
break;
 
case SIOCSIFMETRIC:
-   if ((error = suser(p, 0)) != 0)
- 

Re: pthread_once: don't deadlock on cancel from init_routine

[Scott Cheloha]

Hi,

> On Oct 1, 2017, at 9:30 PM, Scott Cheloha  wrote:
> 
> Per this bit from pthread_once(3):
> 
>> The function pthread_once() is not a cancellation point.
>> However, if init_routine() is a cancellation point and is
>> cancelled, the effect on once_control is as if pthread_once()
>> was never called.
> 
> it should be possible to cancel a thread from init_routine() and
> send in another thread to retry.  But the attached regression
> test fails, so my guess is that there needs to be something like a
> pthread_cleanup wrapper around init_routine() in pthread_once() in
> order for it to be POSIX-compliant.
> 
> [...]
> 
> [...] pthread_once(3) is in libc now while pthread_cleanup_push and
> pthread_cleanup_pop remain in librthread, so you have a choice:
> 
>   1. Move the pthread_cleanup interfaces into libc, or
> 
>   2. Copy their contents and do the cleanup push/pop on the
>  stack in pthread_once().

Absent any other feedback, the attached patch moves the pthread_cleanup
routines from librthread to libc and adds a cleanup wrapper around the
call to init_routine() in pthread_once(3).

These changes allow the initial parts of the attached regression
test to succeed.  I believe this means pthread_once(3) is now more
(fully?) POSIX.1-compliant, like it says it is in the man page.

Moving the routines into libc and calling them there seems cleaner
than copying their contents out into a macro.  Calling the routines
from within the library was libthr's original solution, though
according to the commit record they now use a macro for performance
reasons.  Relevant commits here:

https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_once.c?r1=112918=144518
https://svnweb.freebsd.org/base/head/lib/libthr/thread/thr_once.c?r1=155739=172695

Doing it in a macro would also require changing the cleanup_fn
structure and adding a "this is on the stack" boolean.

Misc. notes:

* We're adding interfaces to libc so minor version bump
  there.

* We're removing interfaces from librthread so major
  version bump there.

Tangential questions:

* Because the cleanup functions are now exported from and
  called from within libc they should be PROTO_NORMAL and
  DEF_STRONG'd, right?

* pthread_self(3) and pthread_exit(3) are not called from
  within libc but they are PROTO_NORMAL'd.  Is this because
  they are called from within librthread?  Some kind of
  exception case?

* Why does pthread_equal(3) not have a DEF*? Same question
  for pthread_once(3).

CC guenther@ and kettenis@ because they wrote/reviewed the recent
interface migration from librthread to libc.

Thoughts/Feedback?

--
Scott Cheloha

Index: lib/libc/Symbols.list
===
RCS file: /cvs/src/lib/libc/Symbols.list,v
retrieving revision 1.57
diff -u -p -r1.57 Symbols.list
--- lib/libc/Symbols.list   5 Sep 2017 06:35:19 -   1.57
+++ lib/libc/Symbols.list   9 Oct 2017 12:56:06 -
@@ -1667,6 +1667,8 @@ _thread_atfork
 _thread_dofork
 _thread_set_callbacks
 pthread_atfork
+pthread_cleanup_pop
+pthread_cleanup_push
 pthread_cond_broadcast
 pthread_cond_destroy
 pthread_cond_init
Index: lib/libc/shlib_version
===
RCS file: /cvs/src/lib/libc/shlib_version,v
retrieving revision 1.192
diff -u -p -r1.192 shlib_version
--- lib/libc/shlib_version  5 Sep 2017 02:40:54 -   1.192
+++ lib/libc/shlib_version  9 Oct 2017 12:56:06 -
@@ -1,4 +1,4 @@
 major=90
-minor=0
+minor=1
 # note: If changes were made to include/thread_private.h or if system
 # calls were added/changed then librthread/shlib_version also be updated.
Index: lib/libc/hidden/pthread.h
===
RCS file: /cvs/src/lib/libc/hidden/pthread.h,v
retrieving revision 1.2
diff -u -p -r1.2 pthread.h
--- lib/libc/hidden/pthread.h   5 Sep 2017 02:40:54 -   1.2
+++ lib/libc/hidden/pthread.h   9 Oct 2017 12:56:06 -
@@ -21,6 +21,8 @@
 #include_next 
 
 PROTO_STD_DEPRECATED(pthread_atfork);
+PROTO_NORMAL(pthread_cleanup_pop);
+PROTO_NORMAL(pthread_cleanup_push);
 PROTO_STD_DEPRECATED(pthread_cond_broadcast);
 PROTO_STD_DEPRECATED(pthread_cond_destroy);
 PROTO_NORMAL(pthread_cond_init);
Index: lib/libc/thread/rthread.c
===
RCS file: /cvs/src/lib/libc/thread/rthread.c,v
retrieving revision 1.4
diff -u -p -r1.4 rthread.c
--- lib/libc/thread/rthread.c   5 Sep 2017 02:40:54 -   1.4
+++ lib/libc/thread/rthread.c   9 Oct 2017 12:56:07 -
@@ -147,6 +147,38 @@ pthread_exit(void *retval)
 }
 DEF_STRONG(pthread_exit);
 
+void
+pthread_cleanup_push(void (*fn)(void *), void *arg)
+{
+   struct rthread_cleanup_fn *clfn;
+   pthread_t self = pthread_self();
+
+   clfn = calloc(1, 

Re: kq_count earlier!

[Martin Pieuchot]

On 09/10/17(Mon) 14:35, Joerg Sonnenberger wrote:
> On Mon, Oct 09, 2017 at 12:27:50PM +0200, Martin Pieuchot wrote:
> > On 09/10/17(Mon) 12:22, Joerg Sonnenberger wrote:
> > > On Mon, Oct 09, 2017 at 12:15:46PM +0200, Martin Pieuchot wrote:
> > > > Diff below is a small cleanup to keep the accounting of events in
> > > > sync with the number of events on the list.  This is a noop for the
> > > > moment, but it's small & easy part to review of my upcoming diff.
> > > 
> > > Well, not counting the markers allows skipping the scan when no real
> > > content is on the list. That's a non-trivial condition if the list is
> > > scanned by multiple threads and started non-empty.
> > 
> > I don't understand if you're suggesting something or not.  Did you spot
> > something wrong in the diff I sent?
> 
> You are removing optimisation potential. Counting only real events is
> more useful than counting the entries on the list.

You clearly didn't read my diff.  I'm not changing anything.

Re: kq_count earlier!

[Joerg Sonnenberger]

On Mon, Oct 09, 2017 at 12:27:50PM +0200, Martin Pieuchot wrote:
> On 09/10/17(Mon) 12:22, Joerg Sonnenberger wrote:
> > On Mon, Oct 09, 2017 at 12:15:46PM +0200, Martin Pieuchot wrote:
> > > Diff below is a small cleanup to keep the accounting of events in
> > > sync with the number of events on the list.  This is a noop for the
> > > moment, but it's small & easy part to review of my upcoming diff.
> > 
> > Well, not counting the markers allows skipping the scan when no real
> > content is on the list. That's a non-trivial condition if the list is
> > scanned by multiple threads and started non-empty.
> 
> I don't understand if you're suggesting something or not.  Did you spot
> something wrong in the diff I sent?

You are removing optimisation potential. Counting only real events is
more useful than counting the entries on the list.

Joerg

Re: Move 'struct kevent' storage to the stack

[Mark Kettenis]

> Date: Mon, 9 Oct 2017 12:06:53 +0200
> From: Martin Pieuchot 
> 
> On 09/10/17(Mon) 11:55, Mark Kettenis wrote:
> > > Date: Mon, 9 Oct 2017 11:48:56 +0200
> > > From: Martin Pieuchot 
> > > 
> > > This is the first step to allow kqueue_scan() to be executed in
> > > parallel.  I'm currently focusing on data structure ownership with
> > > regard to sleep.  I don't plan to get rid of the KERNEL_LOCK() for
> > > the moment.  I don't think it makes sense unless we can unlock the
> > > kevent(2) syscall.
> > > 
> > > The diff below move 'struct kevent' storage to the stack.  This is
> > > the simplest way to make it mp-safe.
> > > 
> > > This has been tested as part of a larger diff by many.
> > > 
> > > Ok?
> > 
> > How big (in bytes) are the arrays you're putting on the stack?
> 
> 256 bytes on amd64:
> 
>   sizeof(struct kevent) * KQ_NEVENT
> 
> 32  * 8

In that case; ok kettenis@

Re: kq_count earlier!

[Martin Pieuchot]

On 09/10/17(Mon) 12:22, Joerg Sonnenberger wrote:
> On Mon, Oct 09, 2017 at 12:15:46PM +0200, Martin Pieuchot wrote:
> > Diff below is a small cleanup to keep the accounting of events in
> > sync with the number of events on the list.  This is a noop for the
> > moment, but it's small & easy part to review of my upcoming diff.
> 
> Well, not counting the markers allows skipping the scan when no real
> content is on the list. That's a non-trivial condition if the list is
> scanned by multiple threads and started non-empty.

I don't understand if you're suggesting something or not.  Did you spot
something wrong in the diff I sent?

Re: kq_count earlier!

[Joerg Sonnenberger]

On Mon, Oct 09, 2017 at 12:15:46PM +0200, Martin Pieuchot wrote:
> Diff below is a small cleanup to keep the accounting of events in
> sync with the number of events on the list.  This is a noop for the
> moment, but it's small & easy part to review of my upcoming diff.

Well, not counting the markers allows skipping the scan when no real
content is on the list. That's a non-trivial condition if the list is
scanned by multiple threads and started non-empty.

Joerg

kq_count earlier!

[Martin Pieuchot]

To prevent an infinite loop, threads looking for events inside
kqueue_scan(), insert a `marker' in the list.  Such markers are
not accounted and they are removed from the list as soon as the
thread is finished or goes to sleep.

Diff below is a small cleanup to keep the accounting of events in
sync with the number of events on the list.  This is a noop for the
moment, but it's small & easy part to review of my upcoming diff.

ok?

Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.79
diff -u -p -r1.79 kern_event.c
--- kern/kern_event.c   31 May 2017 14:52:05 -  1.79
+++ kern/kern_event.c   9 Oct 2017 09:57:54 -
@@ -760,22 +760,24 @@ start:
TAILQ_INSERT_TAIL(>kq_head, , kn_tqe);
while (count) {
kn = TAILQ_FIRST(>kq_head);
-   TAILQ_REMOVE(>kq_head, kn, kn_tqe);
if (kn == ) {
+   TAILQ_REMOVE(>kq_head, kn, kn_tqe);
splx(s);
if (count == maxevents)
goto retry;
goto done;
}
+
+   TAILQ_REMOVE(>kq_head, kn, kn_tqe);
+   kq->kq_count--;
+
if (kn->kn_status & KN_DISABLED) {
kn->kn_status &= ~KN_QUEUED;
-   kq->kq_count--;
continue;
}
if ((kn->kn_flags & EV_ONESHOT) == 0 &&
kn->kn_fop->f_event(kn, 0) == 0) {
kn->kn_status &= ~(KN_QUEUED | KN_ACTIVE);
-   kq->kq_count--;
continue;
}
*kevp = kn->kn_kevent;
@@ -783,7 +785,6 @@ start:
nkev++;
if (kn->kn_flags & EV_ONESHOT) {
kn->kn_status &= ~KN_QUEUED;
-   kq->kq_count--;
splx(s);
kn->kn_fop->f_detach(kn);
knote_drop(kn, p, p->p_fd);
@@ -796,9 +797,9 @@ start:
if (kn->kn_flags & EV_DISPATCH)
kn->kn_status |= KN_DISABLED;
kn->kn_status &= ~(KN_QUEUED | KN_ACTIVE);
-   kq->kq_count--;
} else {
TAILQ_INSERT_TAIL(>kq_head, kn, kn_tqe);
+   kq->kq_count++;
}
count--;
if (nkev == KQ_NEVENTS) {

Re: Move 'struct kevent' storage to the stack

[Martin Pieuchot]

On 09/10/17(Mon) 11:55, Mark Kettenis wrote:
> > Date: Mon, 9 Oct 2017 11:48:56 +0200
> > From: Martin Pieuchot 
> > 
> > This is the first step to allow kqueue_scan() to be executed in
> > parallel.  I'm currently focusing on data structure ownership with
> > regard to sleep.  I don't plan to get rid of the KERNEL_LOCK() for
> > the moment.  I don't think it makes sense unless we can unlock the
> > kevent(2) syscall.
> > 
> > The diff below move 'struct kevent' storage to the stack.  This is
> > the simplest way to make it mp-safe.
> > 
> > This has been tested as part of a larger diff by many.
> > 
> > Ok?
> 
> How big (in bytes) are the arrays you're putting on the stack?

256 bytes on amd64:

sizeof(struct kevent) * KQ_NEVENT

  32  * 8

Re: Move 'struct kevent' storage to the stack

[Mark Kettenis]

> Date: Mon, 9 Oct 2017 11:48:56 +0200
> From: Martin Pieuchot 
> 
> This is the first step to allow kqueue_scan() to be executed in
> parallel.  I'm currently focusing on data structure ownership with
> regard to sleep.  I don't plan to get rid of the KERNEL_LOCK() for
> the moment.  I don't think it makes sense unless we can unlock the
> kevent(2) syscall.
> 
> The diff below move 'struct kevent' storage to the stack.  This is
> the simplest way to make it mp-safe.
> 
> This has been tested as part of a larger diff by many.
> 
> Ok?

How big (in bytes) are the arrays you're putting on the stack?

> Index: kern/kern_event.c
> ===
> RCS file: /cvs/src/sys/kern/kern_event.c,v
> retrieving revision 1.79
> diff -u -p -r1.79 kern_event.c
> --- kern/kern_event.c 31 May 2017 14:52:05 -  1.79
> +++ kern/kern_event.c 9 Oct 2017 09:02:48 -
> @@ -476,6 +476,7 @@ sys_kevent(struct proc *p, void *v, regi
>   struct file *fp;
>   struct timespec ts;
>   int i, n, nerrors, error;
> + struct kevent kev[KQ_NEVENTS];
>  
>   if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL ||
>   (fp->f_type != DTYPE_KQUEUE))
> @@ -500,16 +501,16 @@ sys_kevent(struct proc *p, void *v, regi
>   while (SCARG(uap, nchanges) > 0) {
>   n = SCARG(uap, nchanges) > KQ_NEVENTS ?
>   KQ_NEVENTS : SCARG(uap, nchanges);
> - error = copyin(SCARG(uap, changelist), kq->kq_kev,
> + error = copyin(SCARG(uap, changelist), kev,
>   n * sizeof(struct kevent));
>   if (error)
>   goto done;
>  #ifdef KTRACE
>   if (KTRPOINT(p, KTR_STRUCT))
> - ktrevent(p, kq->kq_kev, n);
> + ktrevent(p, kev, n);
>  #endif
>   for (i = 0; i < n; i++) {
> - kevp = >kq_kev[i];
> + kevp = [i];
>   kevp->flags &= ~EV_SYSFLAGS;
>   error = kqueue_register(kq, kevp, p);
>   if (error || (kevp->flags & EV_RECEIPT)) {
> @@ -691,6 +692,7 @@ kqueue_scan(struct kqueue *kq, int maxev
>   struct timeval atv, rtv, ttv;
>   struct knote *kn, marker;
>   int s, count, timeout, nkev = 0, error = 0;
> + struct kevent kev[KQ_NEVENTS];
>  
>   count = maxevents;
>   if (count == 0)
> @@ -737,7 +739,7 @@ start:
>   goto done;
>   }
>  
> - kevp = kq->kq_kev;
> + kevp = [0];
>   s = splhigh();
>   if (kq->kq_count == 0) {
>   if (timeout < 0) {
> @@ -805,13 +807,13 @@ start:
>   splx(s);
>  #ifdef KTRACE
>   if (KTRPOINT(p, KTR_STRUCT))
> - ktrevent(p, kq->kq_kev, nkev);
> + ktrevent(p, kev, nkev);
>  #endif
> - error = copyout(kq->kq_kev, ulistp,
> + error = copyout(kev, ulistp,
>   sizeof(struct kevent) * nkev);
>   ulistp += nkev;
>   nkev = 0;
> - kevp = kq->kq_kev;
> + kevp = [0];
>   s = splhigh();
>   if (error)
>   break;
> @@ -823,9 +825,9 @@ done:
>   if (nkev != 0) {
>  #ifdef KTRACE
>   if (KTRPOINT(p, KTR_STRUCT))
> - ktrevent(p, kq->kq_kev, nkev);
> + ktrevent(p, kev, nkev);
>  #endif
> - error = copyout(kq->kq_kev, ulistp,
> + error = copyout(kev, ulistp,
>   sizeof(struct kevent) * nkev);
>   }
>   *retval = maxevents - count;
> Index: sys/eventvar.h
> ===
> RCS file: /cvs/src/sys/sys/eventvar.h,v
> retrieving revision 1.3
> diff -u -p -r1.3 eventvar.h
> --- sys/eventvar.h25 Mar 2012 20:33:52 -  1.3
> +++ sys/eventvar.h9 Oct 2017 09:02:48 -
> @@ -44,7 +44,6 @@ struct kqueue {
>  #define KQ_SEL   0x01
>  #define KQ_SLEEP 0x02
>  #define KQ_DYING 0x04
> - struct  kevent kq_kev[KQ_NEVENTS];
>  };
>  
>  #endif /* !_SYS_EVENTVAR_H_ */
> 
> 

Move 'struct kevent' storage to the stack

[Martin Pieuchot]

This is the first step to allow kqueue_scan() to be executed in
parallel.  I'm currently focusing on data structure ownership with
regard to sleep.  I don't plan to get rid of the KERNEL_LOCK() for
the moment.  I don't think it makes sense unless we can unlock the
kevent(2) syscall.

The diff below move 'struct kevent' storage to the stack.  This is
the simplest way to make it mp-safe.

This has been tested as part of a larger diff by many.

Ok?

Index: kern/kern_event.c
===
RCS file: /cvs/src/sys/kern/kern_event.c,v
retrieving revision 1.79
diff -u -p -r1.79 kern_event.c
--- kern/kern_event.c   31 May 2017 14:52:05 -  1.79
+++ kern/kern_event.c   9 Oct 2017 09:02:48 -
@@ -476,6 +476,7 @@ sys_kevent(struct proc *p, void *v, regi
struct file *fp;
struct timespec ts;
int i, n, nerrors, error;
+   struct kevent kev[KQ_NEVENTS];
 
if ((fp = fd_getfile(fdp, SCARG(uap, fd))) == NULL ||
(fp->f_type != DTYPE_KQUEUE))
@@ -500,16 +501,16 @@ sys_kevent(struct proc *p, void *v, regi
while (SCARG(uap, nchanges) > 0) {
n = SCARG(uap, nchanges) > KQ_NEVENTS ?
KQ_NEVENTS : SCARG(uap, nchanges);
-   error = copyin(SCARG(uap, changelist), kq->kq_kev,
+   error = copyin(SCARG(uap, changelist), kev,
n * sizeof(struct kevent));
if (error)
goto done;
 #ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
-   ktrevent(p, kq->kq_kev, n);
+   ktrevent(p, kev, n);
 #endif
for (i = 0; i < n; i++) {
-   kevp = >kq_kev[i];
+   kevp = [i];
kevp->flags &= ~EV_SYSFLAGS;
error = kqueue_register(kq, kevp, p);
if (error || (kevp->flags & EV_RECEIPT)) {
@@ -691,6 +692,7 @@ kqueue_scan(struct kqueue *kq, int maxev
struct timeval atv, rtv, ttv;
struct knote *kn, marker;
int s, count, timeout, nkev = 0, error = 0;
+   struct kevent kev[KQ_NEVENTS];
 
count = maxevents;
if (count == 0)
@@ -737,7 +739,7 @@ start:
goto done;
}
 
-   kevp = kq->kq_kev;
+   kevp = [0];
s = splhigh();
if (kq->kq_count == 0) {
if (timeout < 0) {
@@ -805,13 +807,13 @@ start:
splx(s);
 #ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
-   ktrevent(p, kq->kq_kev, nkev);
+   ktrevent(p, kev, nkev);
 #endif
-   error = copyout(kq->kq_kev, ulistp,
+   error = copyout(kev, ulistp,
sizeof(struct kevent) * nkev);
ulistp += nkev;
nkev = 0;
-   kevp = kq->kq_kev;
+   kevp = [0];
s = splhigh();
if (error)
break;
@@ -823,9 +825,9 @@ done:
if (nkev != 0) {
 #ifdef KTRACE
if (KTRPOINT(p, KTR_STRUCT))
-   ktrevent(p, kq->kq_kev, nkev);
+   ktrevent(p, kev, nkev);
 #endif
-   error = copyout(kq->kq_kev, ulistp,
+   error = copyout(kev, ulistp,
sizeof(struct kevent) * nkev);
}
*retval = maxevents - count;
Index: sys/eventvar.h
===
RCS file: /cvs/src/sys/sys/eventvar.h,v
retrieving revision 1.3
diff -u -p -r1.3 eventvar.h
--- sys/eventvar.h  25 Mar 2012 20:33:52 -  1.3
+++ sys/eventvar.h  9 Oct 2017 09:02:48 -
@@ -44,7 +44,6 @@ struct kqueue {
 #define KQ_SEL 0x01
 #define KQ_SLEEP   0x02
 #define KQ_DYING   0x04
-   struct  kevent kq_kev[KQ_NEVENTS];
 };
 
 #endif /* !_SYS_EVENTVAR_H_ */

Re: rtsx(4): add ADMA support

[Stefan Sperling]

On Sun, Oct 08, 2017 at 09:49:41PM +0200, Stefan Sperling wrote:
> The sdmmc stack provides DMA buffers for data transfers nowadays.
> This was not the case yet when rtsx(4) was written so this driver
> is still using a lame memcpy bounce-buffer approach.
> 
> With this diff regular data transfers go through a DMA-only code path.
> The memcpy path is still needed to transfer data during card initialisation
> because sdmmc does not provide DMA-safe buffers for those, likely because
> the sdhc(4) driver can transfer this data via registers.
> 
> Tested with:
> 
> rtsx0 at pci3 dev 0 function 0 "Realtek RTS5209 Card Reader" rev 0x01: msi
> sdmmc0 at rtsx0: 4-bit, dma

visa@ spotted lapses in the use of bus_dmamap_sync() in my previous diff.
Here is an updated diff to address that.

Index: ic/rtsx.c
===
RCS file: /cvs/src/sys/dev/ic/rtsx.c,v
retrieving revision 1.19
diff -u -p -r1.19 rtsx.c
--- ic/rtsx.c   7 Sep 2017 17:00:28 -   1.19
+++ ic/rtsx.c   9 Oct 2017 09:22:02 -
@@ -32,30 +32,38 @@
 #include 
 
 /* 
- * We use two DMA buffers, a command buffer and a data buffer.
+ * We use three DMA buffers: a command buffer, a data buffer, and a buffer for
+ * ADMA transfer descriptors which describe scatter-gather (SG) I/O operations.
  *
  * The command buffer contains a command queue for the host controller,
  * which describes SD/MMC commands to run, and other parameters. The chip
- * runs the command queue when a special bit in the RTSX_HCBAR register is set
- * and signals completion with the TRANS_OK interrupt.
+ * runs the command queue when a special bit in the RTSX_HCBAR register is
+ * set and signals completion with the TRANS_OK interrupt.
  * Each command is encoded as a 4 byte sequence containing command number
  * (read, write, or check a host controller register), a register address,
  * and a data bit-mask and value.
+ * SD/MMC commands which do not transfer any data from/to the card only use
+ * the command buffer.
  *
- * The data buffer is used to transfer data sectors to or from the SD card.
- * Data transfer is controlled via the RTSX_HDBAR register. Completion is
- * also signalled by the TRANS_OK interrupt.
+ * The smmmc stack provides DMA-safe buffers with data transfer commands.
+ * In this case we write a list of descriptors to the ADMA descriptor buffer,
+ * instructing the chip to transfer data directly from/to sdmmc DMA buffers.
  *
- * The chip is unable to perform DMA above 4GB.
+ * However, some sdmmc commands used during card initialization also carry
+ * data, and these don't come with DMA-safe buffers. In this case, we transfer
+ * data from/to the SD card via a DMA data bounce buffer.
  *
- * SD/MMC commands which do not transfer any data from/to the card only use
- * the command buffer.
+ * In both cases, data transfer is controlled via the RTSX_HDBAR register
+ * and completion is signalled by the TRANS_OK interrupt.
+ *
+ * The chip is unable to perform DMA above 4GB.
  */
 
 #defineRTSX_DMA_MAX_SEGSIZE0x8
 #defineRTSX_HOSTCMD_MAX256
 #defineRTSX_HOSTCMD_BUFSIZE(sizeof(u_int32_t) * RTSX_HOSTCMD_MAX)
 #defineRTSX_DMA_DATA_BUFSIZE   MAXPHYS
+#defineRTSX_ADMA_DESC_SIZE (sizeof(uint64_t) * SDMMC_MAXNSEGS)
 
 #define READ4(sc, reg) \
(bus_space_read_4((sc)->iot, (sc)->ioh, (reg)))
@@ -121,7 +129,10 @@ void   rtsx_hostcmd(u_int32_t *, int *, u_
u_int8_t);
 intrtsx_hostcmd_send(struct rtsx_softc *, int);
 u_int8_t rtsx_response_type(u_int16_t);
+intrtsx_xfer_exec(struct rtsx_softc *, bus_dmamap_t, int);
 intrtsx_xfer(struct rtsx_softc *, struct sdmmc_command *, u_int32_t *);
+intrtsx_xfer_bounce(struct rtsx_softc *, struct sdmmc_command *);
+intrtsx_xfer_adma(struct rtsx_softc *, struct sdmmc_command *);
 void   rtsx_card_insert(struct rtsx_softc *);
 void   rtsx_card_eject(struct rtsx_softc *);
 intrtsx_led_enable(struct rtsx_softc *);
@@ -167,6 +178,7 @@ rtsx_attach(struct rtsx_softc *sc, bus_s
 {
struct sdmmcbus_attach_args saa;
u_int32_t sdio_cfg;
+   int rsegs;
 
sc->iot = iot;
sc->ioh = ioh;
@@ -189,7 +201,17 @@ rtsx_attach(struct rtsx_softc *sc, bus_s
if (bus_dmamap_create(sc->dmat, RTSX_DMA_DATA_BUFSIZE, 1,
RTSX_DMA_MAX_SEGSIZE, 0, BUS_DMA_NOWAIT,
>dmap_data) != 0)
-   return 1;
+   goto destroy_cmd;
+   if (bus_dmamap_create(sc->dmat, RTSX_ADMA_DESC_SIZE, 1,
+   RTSX_DMA_MAX_SEGSIZE, 0, BUS_DMA_NOWAIT,
+   >dmap_adma) != 0)
+   goto destroy_data;
+   if (bus_dmamem_alloc(sc->dmat, RTSX_ADMA_DESC_SIZE, 0, 0,
+   sc->adma_segs, 1, , BUS_DMA_WAITOK|BUS_DMA_ZERO))
+   goto destroy_adma;
+   if (bus_dmamem_map(sc->dmat, sc->adma_segs, rsegs, RTSX_ADMA_DESC_SIZE,
+   >admabuf, 

Re: armv7 bcopyinout.S vs bcopyinout_xscale.S

[Mark Kettenis]

> Date: Mon, 9 Oct 2017 08:45:55 +0300
> From: Artturi Alm 
> 
> Hi,
> 
> 
> has anyone looked at the netbsd xscale-versions of bcopyin/bcopyout/kcopy?
> 
> this is from netbsd bcopyinout.S:
> #if defined(__XSCALE__) || defined(_ARM_ARCH_6)
> /*
>  * armv6 and v7 have pld and strd so they can use the xscale
>  * bcopyinout as well.
>  */
> #include "bcopyinout_xscale.S"
> #else
> 
> untested diff below i just scavenged from one of my dead branches,
> just incase someone has the time and motivation to run it through some
> performance testing or w/e.

Please stop sending untested diffs.  Nobody has the motivation to look
at them, unless maybe they fix actual bugs.

> diff --git a/sys/arch/arm/arm/bcopyinout.S b/sys/arch/arm/arm/bcopyinout.S
> index 9a7d11865c0..bc2e58d22b7 100644
> --- a/sys/arch/arm/arm/bcopyinout.S
> +++ b/sys/arch/arm/arm/bcopyinout.S
> @@ -41,7 +41,7 @@
>  #include 
>  #include 
>  
> -#ifdef __XSCALE__
> +#ifdef CPU_ARMv7
>  #include "bcopyinout_xscale.S"
>  #else
>  
> diff --git a/sys/arch/arm/arm/bcopyinout_xscale.S 
> b/sys/arch/arm/arm/bcopyinout_xscale.S
> new file mode 100644
> index 000..2e740eb96c2
> --- /dev/null
> +++ b/sys/arch/arm/arm/bcopyinout_xscale.S
> @@ -0,0 +1,1139 @@
> +/* $OpenBSD$ */
> +/*   $NetBSD: bcopyinout_xscale.S,v 1.11 2013/12/01 02:54:33 joerg Exp $ 
> */
> +
> +/*
> + * Copyright 2003 Wasabi Systems, Inc.
> + * All rights reserved.
> + *
> + * Written by Steve C. Woodford for Wasabi Systems, Inc.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *notice, this list of conditions and the following disclaimer in the
> + *documentation and/or other materials provided with the distribution.
> + * 3. All advertising materials mentioning features or use of this software
> + *must display the following acknowledgement:
> + *  This product includes software developed for the NetBSD Project by
> + *  Wasabi Systems, Inc.
> + * 4. The name of Wasabi Systems, Inc. may not be used to endorse
> + *or promote products derived from this software without specific prior
> + *written permission.
> + *
> + * THIS SOFTWARE IS PROVIDED BY WASABI SYSTEMS, INC. ``AS IS'' AND
> + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED
> + * TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> + * PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL WASABI SYSTEMS, INC
> + * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
> + * POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> + .text
> + .align  2
> +
> +/*
> + * r0 = user space address
> + * r1 = kernel space address
> + * r2 = length
> + *
> + * Copies bytes from user space to kernel space
> + */
> +ENTRY(copyin)
> + cmp r2, #0x00
> +#if /* XXX or <= 0 like below? */ 1
> + moveq   r0, #0
> + moveq   pc, lr
> +#else
> + movle   r0, #0x00
> + RETc(le)/* Bail early if length is <= 0 */
> +#endif
> + push{r10-r11, lr}
> +
> + /* Get curcpu from TPIDRPRW. */
> + mrc CP15_TPIDRPRW(r10)
> + ldr r10, [r10, #CI_CURPCB]
> +
> + mov r3, #0x00
> + adr ip, .Lcopyin_fault
> + ldr r11, [r10, #PCB_ONFAULT]
> + str ip, [r10, #PCB_ONFAULT]
> + bl  .Lcopyin_guts
> + str r11, [r10, #PCB_ONFAULT]
> + mov r0, #0x00
> + pop {r10-r11, pc}
> +
> +.Lcopyin_fault:
> + str r11, [r10, #PCB_ONFAULT]
> + cmp r3, #0x00
> + popgt   {r4-r7} /* r3 > 0 Restore r4-r7 */
> + poplt   {r4-r9} /* r3 < 0 Restore r4-r9 */
> + pop {r10-r11, pc}
> +
> +.Lcopyin_guts:
> + pld [r0]
> + /* Word-align the destination buffer */
> + andsip, r1, #0x03   /* Already word aligned? */
> + beq .Lcopyin_wordaligned/* Yup */
> + rsb ip, ip, #0x04
> + cmp r2, ip  /* Enough bytes left to align it? */
> + blt .Lcopyin_l4_2   /* Nope. Just copy bytewise */
> + sub r2, r2, ip
> + rsbsip, ip, #0x03
> + addne   pc, pc, ip, lsl #3
> + nop
> + ldrbt   ip, [r0], #0x01
> + strbip, [r1], #0x01
> + ldrbt   ip, [r0], #0x01
> + strbip, [r1], #0x01
> + 

Re: bc(1) examples

[Otto Moerbeek]

On Sun, Oct 08, 2017 at 11:10:06PM +0200, Jan Stary wrote:

> On Oct 08 11:31:16, o...@drijf.net wrote:
> > On Fri, Oct 06, 2017 at 02:12:01PM +0200, Jan Stary wrote:
> > 
> > > Isn't "4 * a(1)" a more natural incarnation of pi than "2 * a(2^1)"?
> > 
> > The point of this example is to (also) show that a() works on very
> > large numbers.
> 
> My itch is that 4 * a(1) _is_ pi, and the "approximation"
> is in how precisely you compute it; while 2 * a(2^1)
> is _not_ pi, however precisely you compute it.
> 
> Do you mean that a() _works_ in that it not only accepts
> the very large argument, but actually computes a sensible value?
> Unlike the other functions, arctg() has a limit at infinity.
> 
> Interestingly, in bc.library, a() is the "basic" function
> approximated with (i)rational functions, and s() and c() are
> built on top of it (using a(1) of course :-), giving e.g.
> 
>   bc -l -e 'scale = 500; s(2^1 * 4 * a(1))' -e quit
> 
> around -.855 instead of zero. Indeed, it seems to be s() which
> has this problem, while a() gives a good approximation.
> 
> So do you mean to illustrate that a() works with large arguments,
> _unlike_ e.g. s()?
> 
>   Jan

OK, OK, I'll change it.

-Otto