Re: Virtio fix for testing
On Sat, 12 Aug 2023 at 16:18, Stuart Henderson wrote: > > Is there a way to get an updated ISO or kernel with the fix? > > (we're already adding an installer config file to the ISO, so why not a > > kernel) > > > > Andrew > > > > It was committed to -current, so if you're able to use a snapshot > build (https://cdn.openbsd.org/pub/OpenBSD/snapshots) that would > likely be the simplest fix. Greg, Stuart, thanks for the pointers. Unfortunately I had no luck: OpenBSD 7.3-current (RAMDISK_CD) #1262: Sat Aug 12 11:54:24 MDT 2023 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/RAMDISK_CD ... Installing base73.tgz19% | | 72320 KB 00:25 ETAwdc_atapi_start: not ready, st = 50 Installing base73.tgz22% |* | 83840 KB 00:27 ETAfatal protection fault in supervisor mode trap type 4 code 0 rip 81008c29 cs 8 rflags 10286 cr2 251d44000 cpl 6 rsp 800021749050 gsbase 0x81908ff0 kgsbase 0x0 panic: trap type 4, code=0, pc=81008c29 syncing disks... I guess I've a different problem. Now I need a cheat sheet on how to pull a backtrace :-/
Re: [diff] selectable curves in smtpd ?
On 2023/08/12 19:07, Marc Espie wrote: > On Sat, Aug 12, 2023 at 03:21:00PM +, gil...@poolp.org wrote: > > August 12, 2023 4:34 PM, "Theo Buehler" wrote: > > > > > On Sat, Aug 12, 2023 at 02:29:45PM +, gil...@poolp.org wrote: > > > > > >> Hello, > > >> > > >> Someone asked about selectable curves in the OpenSMTPD portable tracker, > > >> and it turns out I had a diff for that among a few others. > > > > > > Why do they need this? > > > > I suspect for the same reason people have needed ciphers selection in the > > past, > > being able to comply with the requirements of some certification (iirc, > > medical > > mail systems, for example, have strict requirements regarding their setup). > > > > Anyways, I've written this a long time ago and I'm providing it in case > > it's of > > any interest, feel free to discard. > > > > This is moving *backwards* from best practices. As if certification cares about that ;)
Re: Virtio fix for testing
On 2023/08/12 13:16, Andrew Cagney wrote: > Ref: https://marc.info/?l=openbsd-tech&m=168458764424059&w=2 > https://marc.info/?l=openbsd-misc&m=168071258109433&w=2 > > I'm trying to update libreswan's automated test framework so that it > creates an OpenBSD 7.3 test VM using install7.3.iso. Unfortunately > I've hit what I'm assuming is this bug vis: > > Installing bsd 100% |**| 24400 KB00:01 > Installing bsd.rd 100% |**| 4547 KB00:00 > Installing base73.tgz 3% | | 12800 KB > 00:28 ETAwdc_atapi_start: not ready, st = 50 > Installing base73.tgz 4% |* | 15104 KB > 00:52 ETAfatal protection fault in supervisor mode > trap type 4 code 0 rip 810089d9 cs 8 rflags 10286 cr2 > 2446c8000 cpl 6 rsp 80002174d4e0 > > Is there a way to get an updated ISO or kernel with the fix? > (we're already adding an installer config file to the ISO, so why not a > kernel) > > Andrew > It was committed to -current, so if you're able to use a snapshot build (https://cdn.openbsd.org/pub/OpenBSD/snapshots) that would likely be the simplest fix.
Re: Virtio fix for testing
Ref: https://marc.info/?l=openbsd-tech&m=168458764424059&w=2 https://marc.info/?l=openbsd-misc&m=168071258109433&w=2 I'm trying to update libreswan's automated test framework so that it creates an OpenBSD 7.3 test VM using install7.3.iso. Unfortunately I've hit what I'm assuming is this bug vis: Installing bsd 100% |**| 24400 KB00:01 Installing bsd.rd 100% |**| 4547 KB00:00 Installing base73.tgz 3% | | 12800 KB 00:28 ETAwdc_atapi_start: not ready, st = 50 Installing base73.tgz 4% |* | 15104 KB 00:52 ETAfatal protection fault in supervisor mode trap type 4 code 0 rip 810089d9 cs 8 rflags 10286 cr2 2446c8000 cpl 6 rsp 80002174d4e0 Is there a way to get an updated ISO or kernel with the fix? (we're already adding an installer config file to the ISO, so why not a kernel) Andrew
Re: [diff] selectable curves in smtpd ?
On Sat, Aug 12, 2023 at 03:21:00PM +, gil...@poolp.org wrote: > August 12, 2023 4:34 PM, "Theo Buehler" wrote: > > > On Sat, Aug 12, 2023 at 02:29:45PM +, gil...@poolp.org wrote: > > > >> Hello, > >> > >> Someone asked about selectable curves in the OpenSMTPD portable tracker, > >> and it turns out I had a diff for that among a few others. > > > > Why do they need this? > > I suspect for the same reason people have needed ciphers selection in the > past, > being able to comply with the requirements of some certification (iirc, > medical > mail systems, for example, have strict requirements regarding their setup). > > Anyways, I've written this a long time ago and I'm providing it in case it's > of > any interest, feel free to discard. > This is moving *backwards* from best practices. Notice that TLS 1.3 did remove EC parameters choice, because this could lead to downgrade MIT attacks.
Re: Diff for evaluation (WACOM tablet driver)
On Sat, Aug 12, 2023 at 05:22:38PM +0200, Peter J. Philipp wrote: > On Sat, Aug 12, 2023 at 02:27:13PM +, Miod Vallat wrote: > > Third time's (hopefully) the charm. How about that diff? Too much things > > have been removed in uwacom. > > partial success! The wacom driver is recognized, no panics this time. But > the input is all over the place when I try to draw anything in gimp. It opens > windows and stuff that I didn't open. I see stuff like that, but that's actually an improvement here: ask xev, and you'll see some extra position information that's mapped to the 4th axis, which needs to be remmapped manually in gimp, as far as I know (for now)
Re: Diff for evaluation (WACOM tablet driver)
> On Sat, Aug 12, 2023 at 02:27:13PM +, Miod Vallat wrote: > > Third time's (hopefully) the charm. How about that diff? Too much things > > have been removed in uwacom. > > partial success! The wacom driver is recognized, no panics this time. But > the input is all over the place when I try to draw anything in gimp. It opens > windows and stuff that I didn't open. I think it was a mistake to try and have the wacom interrupt code cover the "new" models as well as the existing ones, so I've split it in two flavours. New diff below. Index: dev/hid/hid.c === RCS file: /OpenBSD/src/sys/dev/hid/hid.c,v retrieving revision 1.5 diff -u -p -r1.5 hid.c --- dev/hid/hid.c 20 May 2022 05:03:45 - 1.5 +++ dev/hid/hid.c 12 Aug 2023 15:51:13 - @@ -657,3 +657,52 @@ hid_is_collection(const void *desc, int hid_end_parse(hd); return (0); } + +struct hid_data * +hid_get_collection_data(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: usage=0x%x\n", __func__, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + return hd; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return NULL; +} + +int +hid_get_id_of_collection(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + hid_end_parse(hd); + return hi.report_ID; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return -1; +} Index: dev/hid/hid.h === RCS file: /OpenBSD/src/sys/dev/hid/hid.h,v retrieving revision 1.10 diff -u -p -r1.10 hid.h --- dev/hid/hid.h 20 May 2022 05:03:45 - 1.10 +++ dev/hid/hid.h 12 Aug 2023 15:51:13 - @@ -93,6 +93,10 @@ int hid_locate(const void *, int, int32_ int32_thid_get_data(const uint8_t *buf, int, struct hid_location *); uint32_t hid_get_udata(const uint8_t *buf, int, struct hid_location *); inthid_is_collection(const void *, int, uint8_t, int32_t); +struct hid_data * hid_get_collection_data(const void *, int, int32_t, + uint32_t); +inthid_get_id_of_collection(const void *desc, int size, int32_t usage, + uint32_t collection); #endif /* _KERNEL */ @@ -353,6 +357,7 @@ int hid_is_collection(const void *, int, #define HUD_TOUCHSCREEN0x0004 #define HUD_TOUCHPAD 0x0005 #define HUD_CONFIG 0x000e +#define HUD_STYLUS 0x0020 #define HUD_FINGER 0x0022 #define HUD_TIP_PRESSURE 0x0030 #define HUD_BARREL_PRESSURE0x0031 @@ -387,6 +392,12 @@ inthid_is_collection(const void *, int, #define HUD_CONTACT_MAX0x0055 #define HUD_SCAN_TIME 0x0056 #define HUD_BUTTON_TYPE0x0059 +#define HUD_SECONDARY_BARREL_SWITCH0x005A +#define HUD_WACOM_X0x0130 +#define HUD_WACOM_Y0x0131 +#define HUD_WACOM_DISTANCE 0x0132 +#define HUD_WACOM_PAD_BUTTONS000x0910 +#define HUD_WACOM_BATTERY 0x1013 /* Usages, LED */ #define HUL_NUM_LOCK 0x0001 Index: dev/hid/hidms.c === RCS file: /OpenBSD/src/sys/dev/hid/hidms.c,v retrieving revision 1.9 diff -u -p -r1.9 hidms.c --- dev/hid/hidms.c 16 Jun 2022 20:52:38 - 1.9 +++ dev/hid/hidms.c 12 Aug 2023 15:51:13 - @@ -61,6 +61,188 @@ int hidmsdebug = 0; #define MOUSE_FLAGS_MASK (HIO_CONST | HIO_RELATIVE) #define NOTMOUSE(f)(((f) & MOUSE_FLAGS_MASK) != HIO_RELATIVE) +void +hidms_stylus_hid_parse(struct hidms *ms, struct hid_data *d, +struct hid_location *loc_stylus_btn) +{ + struct hid_item h; + + while (hid_get_item(d, &h)) { + if (h.kind == hid_endcollection) +
Re: Diff for evaluation (WACOM tablet driver)
On Sat, Aug 12, 2023 at 02:27:13PM +, Miod Vallat wrote: > Third time's (hopefully) the charm. How about that diff? Too much things > have been removed in uwacom. partial success! The wacom driver is recognized, no panics this time. But the input is all over the place when I try to draw anything in gimp. It opens windows and stuff that I didn't open. spica$ dmesg | grep uwacom uwacom0 at uhidev0 reportid 16: 3 buttons wsmouse0 at uwacom0 mux 0 uwacom1 at uhidev0 reportid 192: 3 buttons wsmouse1 at uwacom1 mux 0 spica$ usbdevs Controller /dev/usb0: addr 01: 8086: Intel, xHCI root hub addr 02: 056a:033b Wacom Co.,Ltd., Intuos PS addr 03: 05ac:8290 Broadcom Corp., Bluetooth USB Host Controller addr 04: 05ac:0273 Apple Inc., Apple Internal Keyboard / Trackpad addr 05: 05ac:8406 Apple, Card Reader Oh I see, it attached uwacom0 and uwacom1, odd! I'll attach the full dmesg on this one: syncing disks... done bwfm0: flowring not closing bwfm0: flowring not closing rebooting... OpenBSD 7.3-current (GENERIC.MP) #4: Sat Aug 12 17:08:58 CEST 2023 p...@spica.delphinusdns.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP real mem = 17059287040 (16269MB) avail mem = 16522543104 (15757MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x8afac000 (32 entries) bios0: vendor Apple Inc. version "186.0.0.0.0" date 06/14/2019 bios0: Apple Inc. MacBookPro12,1 efi0 at bios0: UEFI 1.1 acpi0 at bios0: ACPI 5.0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP HPET APIC SBST ECDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT DMAR MCFG acpi0: wakeup devices PEG0(S3) EC__(S3) HDEF(S3) RP01(S3) RP02(S3) RP03(S4) ARPT(S4) RP05(S3) RP06(S3) SPIT(S3) XHC1(S3) ADP1(S3) LID0(S3) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.02 MHz, 06-3d-04, patch 002f cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 100MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.06 MHz, 06-3d-04, patch 002f cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 1 (application processor) cpu2: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.06 MHz, 06-3d-04, patch 002f cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu2: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu2: smt 1, core 0, package 0 cpu3 at mainbus0: apid 3 (application processor) cpu3: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.08 MHz, 06-3d-04, patch 002f cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu3: 32KB 64b/line 8-way D-cache, 32KB 6
Re: [diff] selectable curves in smtpd ?
August 12, 2023 4:34 PM, "Theo Buehler" wrote: > On Sat, Aug 12, 2023 at 02:29:45PM +, gil...@poolp.org wrote: > >> Hello, >> >> Someone asked about selectable curves in the OpenSMTPD portable tracker, >> and it turns out I had a diff for that among a few others. > > Why do they need this? I suspect for the same reason people have needed ciphers selection in the past, being able to comply with the requirements of some certification (iirc, medical mail systems, for example, have strict requirements regarding their setup). Anyways, I've written this a long time ago and I'm providing it in case it's of any interest, feel free to discard.
Re: [diff] selectable curves in smtpd ?
On Sat, Aug 12, 2023 at 02:29:45PM +, gil...@poolp.org wrote: > Hello, > > Someone asked about selectable curves in the OpenSMTPD portable tracker, > and it turns out I had a diff for that among a few others. Why do they need this?
[diff] selectable curves in smtpd ?
Hello, Someone asked about selectable curves in the OpenSMTPD portable tracker, and it turns out I had a diff for that among a few others. The diff below adds support for the curves keyword in listener and relay directives, allowing to specify a curve string suitable for tls_config_set_ecdhecurves(3) in the same way ciphers were made selectable. I also have a couple other diffs which I'll clean and send. Index: mta.c === RCS file: /cvs/src/usr.sbin/smtpd/mta.c,v retrieving revision 1.245 diff -u -p -u -p -r1.245 mta.c --- mta.c 31 May 2023 16:51:46 - 1.245 +++ mta.c 12 Aug 2023 14:20:21 - @@ -476,6 +476,7 @@ mta_setup_dispatcher(struct dispatcher * struct pki *pki; struct ca *ca; const char *ciphers; + const char *curves; uint32_t protos; if (dispatcher->type != DISPATCHER_REMOTE) @@ -490,6 +491,12 @@ mta_setup_dispatcher(struct dispatcher * if (remote->tls_ciphers) ciphers = remote->tls_ciphers; if (ciphers && tls_config_set_ciphers(config, ciphers) == -1) + fatalx("%s", tls_config_error(config)); + + curves = env->sc_tls_curves; + if (remote->tls_curves) + curves = remote->tls_curves; + if (curves && tls_config_set_ecdhecurves(config, curves) == -1) fatalx("%s", tls_config_error(config)); if (remote->tls_protocols) { Index: parse.y === RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v retrieving revision 1.292 diff -u -p -u -p -r1.292 parse.y --- parse.y 10 May 2023 07:19:49 - 1.292 +++ parse.y 12 Aug 2023 14:20:21 - @@ -125,6 +125,7 @@ static struct listen_opts { char *pki[PKI_MAX]; int pkicount; char *tls_ciphers; + char *tls_curves; char *tls_protocols; char *ca; uint16_tauth; @@ -166,7 +167,7 @@ typedef struct { %token ACTION ADMD ALIAS ANY ARROW AUTH AUTH_OPTIONAL %token BACKUP BOUNCE BYPASS -%token CA CERT CHAIN CHROOT CIPHERS COMMIT COMPRESSION CONNECT +%token CA CERT CHAIN CHROOT CIPHERS COMMIT COMPRESSION CONNECT CURVES %token DATA DATA_LINE DHE DISCONNECT DOMAIN %token EHLO ENABLE ENCRYPTION ERROR EXPAND_ONLY %token FCRDNS FILTER FOR FORWARD_ONLY FROM @@ -527,6 +528,9 @@ SMTP LIMIT limits_smtp | SMTP CIPHERS STRING { conf->sc_tls_ciphers = $3; } +| SMTP CURVES STRING { + conf->sc_tls_curves = $3; +} | SMTP MAX_MESSAGE_SIZE size { conf->sc_maxsize = $3; } @@ -765,6 +769,14 @@ HELO STRING { dsp->u.remote.tls_ciphers = $2; } +| CURVES STRING { + if (dsp->u.remote.tls_curves) { + yyerror("curves already specified for this dispatcher"); + YYERROR; + } + + dsp->u.remote.tls_curves = $2; +} | PROTOCOLS STRING { if (dsp->u.remote.tls_protocols) { yyerror("protocols already specified for this dispatcher"); @@ -2329,6 +2341,13 @@ opt_if_listen : INET4 { } listen_opts.tls_ciphers = $2; } + | CURVES STRING { + if (listen_opts.tls_curves) { + yyerror("curves already specified"); + YYERROR; + } + listen_opts.tls_curves = $2; + } | PROTOCOLS STRING { if (listen_opts.tls_protocols) { yyerror("protocols already specified"); @@ -2657,6 +2676,7 @@ lookup(char *s) { "commit", COMMIT }, { "compression",COMPRESSION }, { "connect",CONNECT }, + { "curves", CURVES }, { "data", DATA }, { "data-line", DATA_LINE }, { "dhe",DHE }, @@ -3251,6 +3271,11 @@ create_if_listener(struct listen_opts *l if (lo->pkicount == 0 && lo->ssl) fatalx("invalid listen option: pki required for tls/smtps"); + if (lo->tls_ciphers && !lo->ssl) + fatalx("invalid listen option: ciphers requires tls/smtps"); + if (lo->tls_curves && !lo->ssl) + fatalx("invalid listen option: curves requires tls/smtps"); + flags = lo->flags; if (lo->port) { @@ -3324,6 +3349,11 @@ config_listener(struct listener *h, str fatal("strdup"); } + if (lo->tls_curves != NULL && + (h->tls_curves = strdup(lo->tls_curves)) == NULL) { + fatal("strdup"); + } + if (lo->tls_protocols != NULL && (h->tls_protocols = strdup(lo->tls_protocols)) == NULL) { fatal("strdup"); @@ -3
Re: Diff for evaluation (WACOM tablet driver)
Third time's (hopefully) the charm. How about that diff? Too much things have been removed in uwacom. Index: dev/hid/hid.c === RCS file: /OpenBSD/src/sys/dev/hid/hid.c,v retrieving revision 1.5 diff -u -p -r1.5 hid.c --- dev/hid/hid.c 20 May 2022 05:03:45 - 1.5 +++ dev/hid/hid.c 12 Aug 2023 14:24:53 - @@ -657,3 +657,52 @@ hid_is_collection(const void *desc, int hid_end_parse(hd); return (0); } + +struct hid_data * +hid_get_collection_data(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: usage=0x%x\n", __func__, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + return hd; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return NULL; +} + +int +hid_get_id_of_collection(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + hid_end_parse(hd); + return hi.report_ID; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return 0; +} Index: dev/hid/hid.h === RCS file: /OpenBSD/src/sys/dev/hid/hid.h,v retrieving revision 1.10 diff -u -p -r1.10 hid.h --- dev/hid/hid.h 20 May 2022 05:03:45 - 1.10 +++ dev/hid/hid.h 12 Aug 2023 14:24:53 - @@ -93,6 +93,10 @@ int hid_locate(const void *, int, int32_ int32_thid_get_data(const uint8_t *buf, int, struct hid_location *); uint32_t hid_get_udata(const uint8_t *buf, int, struct hid_location *); inthid_is_collection(const void *, int, uint8_t, int32_t); +struct hid_data * hid_get_collection_data(const void *, int, int32_t, + uint32_t); +inthid_get_id_of_collection(const void *desc, int size, int32_t usage, + uint32_t collection); #endif /* _KERNEL */ @@ -353,6 +357,7 @@ int hid_is_collection(const void *, int, #define HUD_TOUCHSCREEN0x0004 #define HUD_TOUCHPAD 0x0005 #define HUD_CONFIG 0x000e +#define HUD_STYLUS 0x0020 #define HUD_FINGER 0x0022 #define HUD_TIP_PRESSURE 0x0030 #define HUD_BARREL_PRESSURE0x0031 @@ -387,6 +392,12 @@ inthid_is_collection(const void *, int, #define HUD_CONTACT_MAX0x0055 #define HUD_SCAN_TIME 0x0056 #define HUD_BUTTON_TYPE0x0059 +#define HUD_SECONDARY_BARREL_SWITCH0x005A +#define HUD_WACOM_X0x0130 +#define HUD_WACOM_Y0x0131 +#define HUD_WACOM_DISTANCE 0x0132 +#define HUD_WACOM_PAD_BUTTONS000x0910 +#define HUD_WACOM_BATTERY 0x1013 /* Usages, LED */ #define HUL_NUM_LOCK 0x0001 Index: dev/hid/hidms.c === RCS file: /OpenBSD/src/sys/dev/hid/hidms.c,v retrieving revision 1.9 diff -u -p -r1.9 hidms.c --- dev/hid/hidms.c 16 Jun 2022 20:52:38 - 1.9 +++ dev/hid/hidms.c 12 Aug 2023 14:24:53 - @@ -61,6 +61,188 @@ int hidmsdebug = 0; #define MOUSE_FLAGS_MASK (HIO_CONST | HIO_RELATIVE) #define NOTMOUSE(f)(((f) & MOUSE_FLAGS_MASK) != HIO_RELATIVE) +void +hidms_stylus_hid_parse(struct hidms *ms, struct hid_data *d, +struct hid_location *loc_stylus_btn) +{ + struct hid_item h; + + while (hid_get_item(d, &h)) { + if (h.kind == hid_endcollection) + break; + if (h.kind != hid_input || (h.flags & HIO_CONST) != 0) + continue; + /* All the possible stylus reported usages go here */ +#ifdef HIDMS_DEBUG + printf("stylus usage: 0x%x\n", h.usage); +#endif + switch (h.usage) { + /* Buttons */ + case HID_USAGE2(HUP_WACOM | HUP_DIGITIZERS, HUD_TIP_SWITCH): +
Re: Diff for evaluation (WACOM tablet driver)
On Sat, Aug 12, 2023 at 01:12:26PM +, Miod Vallat wrote: > > On Sat, Aug 12, 2023 at 08:00:48AM +, Miod Vallat wrote: > > > I have had a look at your diff and I think it's decent enough to go in > > > after some polishing. > > > > > > Can Wacom tablet users try this cleaned up diff? > > > > Hi, > > > > My WACOM tablet stopped working with this, here is a dmesg with the patch > > and > > usbdevs -v output. Let me know if there is any new patches I can test. > > > > As you can see it doesn't even attach like it should (from the dmesg). > > Thanks for reporting this. The changes in uhidev have been a bit too > aggressive indeed. > > Does this new version of the diff help? Only uhidev.c differs. Hi! I got an instant panic after attach of uwacom0. Since I don't have a camera nor do I have panic console access on this macbook pro because the keyboard is USB, I wrote it down on 2 post-it notes. Let me see if this helps you any: uvm_fault(,,,)->e pagefault trap code 0 stopped at config_search 0x100 cmpq %rax, 0x18(%r14) ... config_search +0x100 config_found_sm +0x36 hidms_attach +0xe3 uwacom_attach +0x15c config_attach +0x1f4 Now I believe there is offsets that only work on my compilation so I'll try to get you some object dumps (objdump -D): uwacom_attach: 00e0 : e0: f3 0f 1e fa endbr64 e4: 4c 8b 1d 00 00 00 00mov0(%rip),%r11# eb eb: 4c 33 1c 24 xor(%rsp),%r11 ef: 55 push %rbp f0: 48 89 e5mov%rsp,%rbp f3: 57 push %rdi f4: 56 push %rsi f5: 52 push %rdx f6: 57 push %rdi f7: 41 53 push %r11 f9: 41 57 push %r15 fb: 41 56 push %r14 fd: 41 55 push %r13 ff: 41 54 push %r12 101: 48 83 ec 18 sub$0x18,%rsp 105: 49 89 d4mov%rdx,%r12 108: 49 89 f7mov%rsi,%r15 10b: 4c 8d 76 78 lea0x78(%rsi),%r14 10f: 48 8b 02mov(%rdx),%rax 112: 48 c7 46 60 00 00 00movq $0x0,0x60(%rsi) 119: 00 11a: 48 8b 4a 08 mov0x8(%rdx),%rcx 11e: 48 89 4e 50 mov%rcx,0x50(%rsi) 122: 48 8b 40 18 mov0x18(%rax),%rax 126: 48 89 46 48 mov%rax,0x48(%rsi) 12a: 8a 42 10mov0x10(%rdx),%al 12d: 88 46 58mov%al,0x58(%rsi) 130: 48 8b 42 08 mov0x8(%rdx),%rax 134: 48 8b 78 48 mov0x48(%rax),%rdi 138: 8b 70 58mov0x58(%rax),%esi 13b: 31 d2 xor%edx,%edx 13d: 31 c9 xor%ecx,%ecx 13f: e8 00 00 00 00 callq 144 144: 49 8b 7c 24 08 mov0x8(%r12),%rdi 149: 48 8d 75 a8 lea0xffa8(%rbp),%rsi 14d: 48 8d 55 b4 lea0xffb4(%rbp),%rdx 151: e8 00 00 00 00 callq 156 156: 45 0f b6 6c 24 10 movzbl 0x10(%r12),%r13d 15c: 48 8b 7d a8 mov0xffa8(%rbp),%rdi 160: 8b 75 b4mov0xffb4(%rbp),%esi 163: 31 d2 xor%edx,%edx ... hidms_attach: 0ca0 : ca0: f3 0f 1e fa endbr64 ca4: 4c 8b 1d 00 00 00 00mov0(%rip),%r11# cab cab: 4c 33 1c 24 xor(%rsp),%r11 caf: 55 push %rbp cb0: 48 89 e5mov%rsp,%rbp cb3: 57 push %rdi cb4: 56 push %rsi cb5: 41 53 push %r11 cb7: 41 57 push %r15 cb9: 41 56 push %r14 cbb: 48 83 ec 18 sub$0x18,%rsp cbf: 49 89 f7mov%rsi,%r15 cc2: 49 89 femov%rdi,%r14 cc5: 8b 77 18mov0x18(%rdi),%esi cc8: 83 fe 01cmp$0x1,%esi ccb: 48 c7 c0 00 00 00 00mov$0x0,%rax cd2: 48 87 d0xchg %rdx,%rax cd5: 48 c7 c0 00 00 00 00mov$0x0,%rax cdc: 48 87 d0xchg %rdx,%rax cdf: 48 0f 44 d0 cmove %rax,%rdx ce3: 48 c7 c7 00 00 00 00mov$0x0,%rdi cea: 31 c0 xor%eax,%eax cec: e8 00 00 00 00 callq cf1 cf1: 41 8b 46 14 mov0x14(%r14),%eax cf5: 83 e0 0aand$0xa,%eax cf8: 83 f8 02cmp$0x2,%eax cfb: 74 13 je d10 cfd: 83 f8 0acm
Re: smr_grace_wait(): Skip halted CPUs
Visa Hankala wrote: > On Sat, Aug 12, 2023 at 01:29:10PM +0200, Martin Pieuchot wrote: > > On 12/08/23(Sat) 10:57, Visa Hankala wrote: > > > On Fri, Aug 11, 2023 at 09:52:15PM +0200, Martin Pieuchot wrote: > > > > When stopping a machine, with "halt -p" for example, secondary CPUs are > > > > removed from the scheduler before smr_flush() is called. So there's no > > > > need for the SMR thread to peg itself to such CPUs. This currently > > > > isn't a problem because we use per-CPU runqueues but it doesn't work > > > > with a global one. So the diff below skip halted CPUs. It should also > > > > speed up rebooting/halting on machine with a huge number of CPUs. > > > > > > Because SPCF_HALTED does not (?) imply that the CPU has stopped > > > processing interrupts, this skipping is not safe as is. Interrupt > > > handlers might access SMR-protected data. > > > > Interesting. This is worse than I expected. It seems we completely > > forgot about suspend/resume and rebooting when we started pinning > > interrupts on secondary CPUs, no? Previously sched_stop_secondary_cpus() > > was enough to ensure no more code would be executed on secondary CPUs, > > no? Wouldn't it be better to remap interrupts to the primary CPU in > > those cases? Is it easily doable? > > I think device interrupt stopping already happens through > config_suspend_all(). Right. DVACT_QUIESCE is supposed to stop the device's transaction flow, which implies it stops scheduling more things which will cause interrupts. DVACT_SUSPEND is supposed to stop the device even further, so that it can save a hardware state into memory, which is viable for recovery later on. For hibernate, this is even more strict, because memory side effects must not occur after these phases. I suppose there is nothing which says interrupts must be stopped for DVACT_SUSPEND. A driver could ACK the interrupts, but create no software side effects. Anyways, for the halt/reboot case, config_suspend_all() is not done. We have architectures that don't suspend/resume, but must halt/reboot. We have not reviewed their driver stack to check if config_suspend_all() would do the right thing. This is not trivial, it isn't just in the drivers. Some of our non-suspending architectures could have incorrectly ordered device trees...
Re: Diff for evaluation (WACOM tablet driver)
> On Sat, Aug 12, 2023 at 08:00:48AM +, Miod Vallat wrote: > > I have had a look at your diff and I think it's decent enough to go in > > after some polishing. > > > > Can Wacom tablet users try this cleaned up diff? > > Hi, > > My WACOM tablet stopped working with this, here is a dmesg with the patch and > usbdevs -v output. Let me know if there is any new patches I can test. > > As you can see it doesn't even attach like it should (from the dmesg). Thanks for reporting this. The changes in uhidev have been a bit too aggressive indeed. Does this new version of the diff help? Only uhidev.c differs. Index: dev/hid/hid.c === RCS file: /OpenBSD/src/sys/dev/hid/hid.c,v retrieving revision 1.5 diff -u -p -r1.5 hid.c --- dev/hid/hid.c 20 May 2022 05:03:45 - 1.5 +++ dev/hid/hid.c 12 Aug 2023 13:11:22 - @@ -657,3 +657,52 @@ hid_is_collection(const void *desc, int hid_end_parse(hd); return (0); } + +struct hid_data * +hid_get_collection_data(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: usage=0x%x\n", __func__, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + return hd; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return NULL; +} + +int +hid_get_id_of_collection(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + hid_end_parse(hd); + return hi.report_ID; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return 0; +} Index: dev/hid/hid.h === RCS file: /OpenBSD/src/sys/dev/hid/hid.h,v retrieving revision 1.10 diff -u -p -r1.10 hid.h --- dev/hid/hid.h 20 May 2022 05:03:45 - 1.10 +++ dev/hid/hid.h 12 Aug 2023 13:11:22 - @@ -93,6 +93,10 @@ int hid_locate(const void *, int, int32_ int32_thid_get_data(const uint8_t *buf, int, struct hid_location *); uint32_t hid_get_udata(const uint8_t *buf, int, struct hid_location *); inthid_is_collection(const void *, int, uint8_t, int32_t); +struct hid_data * hid_get_collection_data(const void *, int, int32_t, + uint32_t); +inthid_get_id_of_collection(const void *desc, int size, int32_t usage, + uint32_t collection); #endif /* _KERNEL */ @@ -353,6 +357,7 @@ int hid_is_collection(const void *, int, #define HUD_TOUCHSCREEN0x0004 #define HUD_TOUCHPAD 0x0005 #define HUD_CONFIG 0x000e +#define HUD_STYLUS 0x0020 #define HUD_FINGER 0x0022 #define HUD_TIP_PRESSURE 0x0030 #define HUD_BARREL_PRESSURE0x0031 @@ -387,6 +392,12 @@ inthid_is_collection(const void *, int, #define HUD_CONTACT_MAX0x0055 #define HUD_SCAN_TIME 0x0056 #define HUD_BUTTON_TYPE0x0059 +#define HUD_SECONDARY_BARREL_SWITCH0x005A +#define HUD_WACOM_X0x0130 +#define HUD_WACOM_Y0x0131 +#define HUD_WACOM_DISTANCE 0x0132 +#define HUD_WACOM_PAD_BUTTONS000x0910 +#define HUD_WACOM_BATTERY 0x1013 /* Usages, LED */ #define HUL_NUM_LOCK 0x0001 Index: dev/hid/hidms.c === RCS file: /OpenBSD/src/sys/dev/hid/hidms.c,v retrieving revision 1.9 diff -u -p -r1.9 hidms.c --- dev/hid/hidms.c 16 Jun 2022 20:52:38 - 1.9 +++ dev/hid/hidms.c 12 Aug 2023 13:11:22 - @@ -61,6 +61,188 @@ int hidmsdebug = 0; #define MOUSE_FLAGS_MASK (HIO_CONST | HIO_RELATIVE) #define NOTMOUSE(f)(((f) & MOUSE_FLAGS_MASK) != HIO_RELATIVE) +void +hidms_stylus_hid_parse(struct hidms *ms, struct hid_data *d, +struct hid_location *loc_stylus_btn) +{ + struct hid_item h; + + while (hid_get
Re: smr_grace_wait(): Skip halted CPUs
On 12/08/23(Sat) 11:48, Visa Hankala wrote: > On Sat, Aug 12, 2023 at 01:29:10PM +0200, Martin Pieuchot wrote: > > On 12/08/23(Sat) 10:57, Visa Hankala wrote: > > > On Fri, Aug 11, 2023 at 09:52:15PM +0200, Martin Pieuchot wrote: > > > > When stopping a machine, with "halt -p" for example, secondary CPUs are > > > > removed from the scheduler before smr_flush() is called. So there's no > > > > need for the SMR thread to peg itself to such CPUs. This currently > > > > isn't a problem because we use per-CPU runqueues but it doesn't work > > > > with a global one. So the diff below skip halted CPUs. It should also > > > > speed up rebooting/halting on machine with a huge number of CPUs. > > > > > > Because SPCF_HALTED does not (?) imply that the CPU has stopped > > > processing interrupts, this skipping is not safe as is. Interrupt > > > handlers might access SMR-protected data. > > > > Interesting. This is worse than I expected. It seems we completely > > forgot about suspend/resume and rebooting when we started pinning > > interrupts on secondary CPUs, no? Previously sched_stop_secondary_cpus() > > was enough to ensure no more code would be executed on secondary CPUs, > > no? Wouldn't it be better to remap interrupts to the primary CPU in > > those cases? Is it easily doable? > > I think device interrupt stopping already happens through > config_suspend_all(). Indeed. I'm a bit puzzled about the order of operations though. In the case of reboot/halt the existing order of operations are: sched_stop_secondary_cpus() <--- remove secondary CPUs from the scheduler vfs_shutdown() if_downall() uvm_swap_finicrypt_all()<--- happens on a single CPU but with interrupts possibly on secondary CPUs smr_flush() <--- tells the SMR thread to execute itself on all CPUs even if they are out of the scheduler config_suspend_all()<--- stop interrupts from firing x86_broadcast_ipi(X86_IPI_HALT) <--- stop secondary CPUs (on x86) So do we want to keep the existing requirement of being able to execute a thread on a CPU that has been removed from the scheduler? That's is what smr_flush() currently needs. I find it surprising but I can add that as a requirement for the upcoming scheduler. I don't know if other options are possible or even attractive.
Re: smr_grace_wait(): Skip halted CPUs
On Sat, Aug 12, 2023 at 01:29:10PM +0200, Martin Pieuchot wrote: > On 12/08/23(Sat) 10:57, Visa Hankala wrote: > > On Fri, Aug 11, 2023 at 09:52:15PM +0200, Martin Pieuchot wrote: > > > When stopping a machine, with "halt -p" for example, secondary CPUs are > > > removed from the scheduler before smr_flush() is called. So there's no > > > need for the SMR thread to peg itself to such CPUs. This currently > > > isn't a problem because we use per-CPU runqueues but it doesn't work > > > with a global one. So the diff below skip halted CPUs. It should also > > > speed up rebooting/halting on machine with a huge number of CPUs. > > > > Because SPCF_HALTED does not (?) imply that the CPU has stopped > > processing interrupts, this skipping is not safe as is. Interrupt > > handlers might access SMR-protected data. > > Interesting. This is worse than I expected. It seems we completely > forgot about suspend/resume and rebooting when we started pinning > interrupts on secondary CPUs, no? Previously sched_stop_secondary_cpus() > was enough to ensure no more code would be executed on secondary CPUs, > no? Wouldn't it be better to remap interrupts to the primary CPU in > those cases? Is it easily doable? I think device interrupt stopping already happens through config_suspend_all().
Re: smr_grace_wait(): Skip halted CPUs
On 12/08/23(Sat) 10:57, Visa Hankala wrote: > On Fri, Aug 11, 2023 at 09:52:15PM +0200, Martin Pieuchot wrote: > > When stopping a machine, with "halt -p" for example, secondary CPUs are > > removed from the scheduler before smr_flush() is called. So there's no > > need for the SMR thread to peg itself to such CPUs. This currently > > isn't a problem because we use per-CPU runqueues but it doesn't work > > with a global one. So the diff below skip halted CPUs. It should also > > speed up rebooting/halting on machine with a huge number of CPUs. > > Because SPCF_HALTED does not (?) imply that the CPU has stopped > processing interrupts, this skipping is not safe as is. Interrupt > handlers might access SMR-protected data. Interesting. This is worse than I expected. It seems we completely forgot about suspend/resume and rebooting when we started pinning interrupts on secondary CPUs, no? Previously sched_stop_secondary_cpus() was enough to ensure no more code would be executed on secondary CPUs, no? Wouldn't it be better to remap interrupts to the primary CPU in those cases? Is it easily doable? > One possible solution is to spin. When smr_grace_wait() sees > SPCF_HALTED, it should probably call cpu_unidle(ci) and spin until > condition READ_ONCE(ci->ci_schedstate.spc_smrgp) == smrgp becomes true. > However, for this to work, sched_idle() needs to invoke smr_idle(). > Here is a potential problem since the cpu_idle_{enter,cycle,leave}() > logic is not consistent between architectures. We're trying to move away from per-CPU runqueues. That's how I found this issue. I don't see how this possible solution could work with a global runqueue. Do you? > I think the intent in sched_idle() was that cpu_idle_enter() should > block interrupts so that sched_idle() could check without races if > the CPU can sleep. Now, on some architectures cpu_idle_enter() is > a no-op. These architectures have to check the idle state in their > cpu_idle_cycle() function before pausing the CPU. > > To avoid touching architecture-specific code, cpu_is_idle() could > be redefined to > > ((ci)->ci_schedstate.spc_whichqs == 0 && > (ci)->ci_schedstate.spc_smrgp == READ_ONCE(smr_grace_period)) > > Then the loop conditions > > while (!cpu_is_idle(curcpu())) { > > and > > while (spc->spc_whichqs == 0) { > > in sched_idle() would have to be changed to > > while (spc->spc_whichqs != 0) { > > and > > while (cpu_is_idle(ci)) { > > > :( > > > Index: kern/kern_smr.c > > === > > RCS file: /cvs/src/sys/kern/kern_smr.c,v > > retrieving revision 1.16 > > diff -u -p -r1.16 kern_smr.c > > --- kern/kern_smr.c 14 Aug 2022 01:58:27 - 1.16 > > +++ kern/kern_smr.c 11 Aug 2023 19:43:54 - > > @@ -158,6 +158,8 @@ smr_grace_wait(void) > > CPU_INFO_FOREACH(cii, ci) { > > if (!CPU_IS_RUNNING(ci)) > > continue; > > + if (ci->ci_schedstate.spc_schedflags & SPCF_HALTED) > > + continue; > > if (READ_ONCE(ci->ci_schedstate.spc_smrgp) == smrgp) > > continue; > > sched_peg_curproc(ci); > >
Re: Diff for evaluation (WACOM tablet driver)
On Sat, Aug 12, 2023 at 08:00:48AM +, Miod Vallat wrote: > I have had a look at your diff and I think it's decent enough to go in > after some polishing. > > Can Wacom tablet users try this cleaned up diff? Hi, My WACOM tablet stopped working with this, here is a dmesg with the patch and usbdevs -v output. Let me know if there is any new patches I can test. As you can see it doesn't even attach like it should (from the dmesg). Best Regards, -peter rebooting... OpenBSD 7.3-current (GENERIC.MP) #1: Sat Aug 12 12:34:12 CEST 2023 p...@spica.delphinusdns.org:/sys/arch/amd64/compile/GENERIC.MP real mem = 17059287040 (16269MB) avail mem = 16522567680 (15757MB) random: good seed from bootblocks mpath0 at root scsibus0 at mpath0: 256 targets mainbus0 at root bios0 at mainbus0: SMBIOS rev. 2.7 @ 0x8afac000 (32 entries) bios0: vendor Apple Inc. version "186.0.0.0.0" date 06/14/2019 bios0: Apple Inc. MacBookPro12,1 efi0 at bios0: UEFI 1.1 acpi0 at bios0: ACPI 5.0 acpi0: sleep states S0 S3 S4 S5 acpi0: tables DSDT FACP HPET APIC SBST ECDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT SSDT DMAR MCFG acpi0: wakeup devices PEG0(S3) EC__(S3) HDEF(S3) RP01(S3) RP02(S3) RP03(S4) ARPT(S4) RP05(S3) RP06(S3) SPIT(S3) XHC1(S3) ADP1(S3) LID0(S3) acpitimer0 at acpi0: 3579545 Hz, 24 bits acpihpet0 at acpi0: 14318179 Hz acpimadt0 at acpi0 addr 0xfee0: PC-AT compat cpu0 at mainbus0: apid 0 (boot processor) cpu0: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.02 MHz, 06-3d-04, patch 002f cpu0: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu0: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu0: smt 0, core 0, package 0 mtrr: Pentium Pro MTRR support, 10 var ranges, 88 fixed ranges cpu0: apic clock running at 100MHz cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE cpu1 at mainbus0: apid 2 (application processor) cpu1: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.05 MHz, 06-3d-04, patch 002f cpu1: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu1: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu1: smt 0, core 1, package 0 cpu2 at mainbus0: apid 1 (application processor) cpu2: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.06 MHz, 06-3d-04, patch 002f cpu2: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu2: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu2: smt 1, core 0, package 0 cpu3 at mainbus0: apid 3 (application processor) cpu3: Intel(R) Core(TM) i5-5287U CPU @ 2.90GHz, 2800.05 MHz, 06-3d-04, patch 002f cpu3: FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,RDTSCP,LONG,LAHF,ABM,3DNOWP,PERF,ITSC,FSGSBASE,TSC_ADJUST,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,PT,SRBDS_CTRL,MD_CLEAR,IBRS,IBPB,STIBP,L1DF,SSBD,SENSOR,ARAT,XSAVEOPT,MELTDOWN cpu3: 32KB 64b/line 8-way D-cache, 32KB 64b/line 8-way I-cache, 256KB 64b/line 8-way L2 cache, 3MB 64b/line 12-way L3 cache cpu3: smt 1, core 1, package 0 ioapic0 at mainbus0: apid 2 pa 0xfec0, version 20, 40 pins acpiec0 at acpi0 acpimcfg0 at acpi0 acpimcfg0: addr 0xe000, bus 0-155 acpiprt0 at acpi0: bus 0 (PCI0) acpiprt1 at acpi0: bus -1 (PEG0) acpiprt2 at acpi0: bus 1 (RP01) acpiprt3 at acpi0: bus 2 (RP02) acpiprt4 at acpi0: bus 3 (RP03) acpiprt5 at acpi0: bus 5 (RP05) acpiprt6 at acpi0: bus 4 (RP06) acpisbs0 at acpi0: SBS0 model "bq20z451" serial 24593
Re: smr_grace_wait(): Skip halted CPUs
On Fri, Aug 11, 2023 at 09:52:15PM +0200, Martin Pieuchot wrote: > When stopping a machine, with "halt -p" for example, secondary CPUs are > removed from the scheduler before smr_flush() is called. So there's no > need for the SMR thread to peg itself to such CPUs. This currently > isn't a problem because we use per-CPU runqueues but it doesn't work > with a global one. So the diff below skip halted CPUs. It should also > speed up rebooting/halting on machine with a huge number of CPUs. Because SPCF_HALTED does not (?) imply that the CPU has stopped processing interrupts, this skipping is not safe as is. Interrupt handlers might access SMR-protected data. One possible solution is to spin. When smr_grace_wait() sees SPCF_HALTED, it should probably call cpu_unidle(ci) and spin until condition READ_ONCE(ci->ci_schedstate.spc_smrgp) == smrgp becomes true. However, for this to work, sched_idle() needs to invoke smr_idle(). Here is a potential problem since the cpu_idle_{enter,cycle,leave}() logic is not consistent between architectures. I think the intent in sched_idle() was that cpu_idle_enter() should block interrupts so that sched_idle() could check without races if the CPU can sleep. Now, on some architectures cpu_idle_enter() is a no-op. These architectures have to check the idle state in their cpu_idle_cycle() function before pausing the CPU. To avoid touching architecture-specific code, cpu_is_idle() could be redefined to ((ci)->ci_schedstate.spc_whichqs == 0 && (ci)->ci_schedstate.spc_smrgp == READ_ONCE(smr_grace_period)) Then the loop conditions while (!cpu_is_idle(curcpu())) { and while (spc->spc_whichqs == 0) { in sched_idle() would have to be changed to while (spc->spc_whichqs != 0) { and while (cpu_is_idle(ci)) { :( > Index: kern/kern_smr.c > === > RCS file: /cvs/src/sys/kern/kern_smr.c,v > retrieving revision 1.16 > diff -u -p -r1.16 kern_smr.c > --- kern/kern_smr.c 14 Aug 2022 01:58:27 - 1.16 > +++ kern/kern_smr.c 11 Aug 2023 19:43:54 - > @@ -158,6 +158,8 @@ smr_grace_wait(void) > CPU_INFO_FOREACH(cii, ci) { > if (!CPU_IS_RUNNING(ci)) > continue; > + if (ci->ci_schedstate.spc_schedflags & SPCF_HALTED) > + continue; > if (READ_ONCE(ci->ci_schedstate.spc_smrgp) == smrgp) > continue; > sched_peg_curproc(ci); >
Re: Diff for evaluation (WACOM tablet driver)
On Sat, Aug 12, 2023 at 08:00:48AM +, Miod Vallat wrote: > I have had a look at your diff and I think it's decent enough to go in > after some polishing. > > Can Wacom tablet users try this cleaned up diff? Works on the target tablet here (Wacom Intuos S)
anon & pmap_page_protect
Since UVM has been imported, we zap mappings associated to anon pages before deactivating or freeing them. Sadly, with the introduction of locking for amaps & anons, I added new code paths that do not respect this behavior. The diff below restores it by moving the call to pmap_page_protect() inside uvm_anon_release(). With it the 3 code paths using the function are now coherent with the rest of UVM. I remember a discussion we had questioning the need for zapping such mappings. I'm interested in hearing more arguments for or against this change. However, right now, I'm more concerned about coherency, so I'd like to commit the change below before we try something different. ok? Index: uvm/uvm_anon.c === RCS file: /cvs/src/sys/uvm/uvm_anon.c,v retrieving revision 1.55 diff -u -p -u -7 -r1.55 uvm_anon.c --- uvm/uvm_anon.c 11 Apr 2023 00:45:09 - 1.55 +++ uvm/uvm_anon.c 15 May 2023 13:55:28 - @@ -251,14 +251,15 @@ uvm_anon_release(struct vm_anon *anon) KASSERT((pg->pg_flags & PG_RELEASED) != 0); KASSERT((pg->pg_flags & PG_BUSY) != 0); KASSERT(pg->uobject == NULL); KASSERT(pg->uanon == anon); KASSERT(anon->an_ref == 0); uvm_lock_pageq(); + pmap_page_protect(pg, PROT_NONE); uvm_pagefree(pg); uvm_unlock_pageq(); KASSERT(anon->an_page == NULL); lock = anon->an_lock; uvm_anfree(anon); rw_exit(lock); /* Note: extra reference is held for PG_RELEASED case. */ Index: uvm/uvm_fault.c === RCS file: /cvs/src/sys/uvm/uvm_fault.c,v retrieving revision 1.133 diff -u -p -u -7 -r1.133 uvm_fault.c --- uvm/uvm_fault.c 4 Nov 2022 09:36:44 - 1.133 +++ uvm/uvm_fault.c 15 May 2023 13:55:28 - @@ -392,15 +392,14 @@ uvmfault_anonget(struct uvm_faultinfo *u /* * if we were RELEASED during I/O, then our anon is * no longer part of an amap. we need to free the * anon and try again. */ if (pg->pg_flags & PG_RELEASED) { - pmap_page_protect(pg, PROT_NONE); KASSERT(anon->an_ref == 0); /* * Released while we had unlocked amap. */ if (locked) uvmfault_unlockall(ufi, NULL, NULL); uvm_anon_release(anon); /* frees page for us */
Re: Diff for evaluation (WACOM tablet driver)
I have had a look at your diff and I think it's decent enough to go in after some polishing. Can Wacom tablet users try this cleaned up diff? Index: dev/hid/hid.c === RCS file: /OpenBSD/src/sys/dev/hid/hid.c,v retrieving revision 1.5 diff -u -p -r1.5 hid.c --- dev/hid/hid.c 20 May 2022 05:03:45 - 1.5 +++ dev/hid/hid.c 12 Aug 2023 07:59:14 - @@ -657,3 +657,52 @@ hid_is_collection(const void *desc, int hid_end_parse(hd); return (0); } + +struct hid_data * +hid_get_collection_data(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: usage=0x%x\n", __func__, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + return hd; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return NULL; +} + +int +hid_get_id_of_collection(const void *desc, int size, int32_t usage, +uint32_t collection) +{ + struct hid_data *hd; + struct hid_item hi; + + hd = hid_start_parse(desc, size, hid_all); + + DPRINTF("%s: id=%d usage=0x%x\n", __func__, id, usage); + while (hid_get_item(hd, &hi)) { + DPRINTF("%s: kind=%d id=%d usage=0x%x(0x%x)\n", __func__, + hi.kind, hi.report_ID, hi.usage, usage); + if (hi.kind == hid_collection && + hi.collection == collection && hi.usage == usage) { + DPRINTF("%s: found\n", __func__); + hid_end_parse(hd); + return hi.report_ID; + } + } + DPRINTF("%s: not found\n", __func__); + hid_end_parse(hd); + return 0; +} Index: dev/hid/hid.h === RCS file: /OpenBSD/src/sys/dev/hid/hid.h,v retrieving revision 1.10 diff -u -p -r1.10 hid.h --- dev/hid/hid.h 20 May 2022 05:03:45 - 1.10 +++ dev/hid/hid.h 12 Aug 2023 07:59:14 - @@ -93,6 +93,10 @@ int hid_locate(const void *, int, int32_ int32_thid_get_data(const uint8_t *buf, int, struct hid_location *); uint32_t hid_get_udata(const uint8_t *buf, int, struct hid_location *); inthid_is_collection(const void *, int, uint8_t, int32_t); +struct hid_data * hid_get_collection_data(const void *, int, int32_t, + uint32_t); +inthid_get_id_of_collection(const void *desc, int size, int32_t usage, + uint32_t collection); #endif /* _KERNEL */ @@ -353,6 +357,7 @@ int hid_is_collection(const void *, int, #define HUD_TOUCHSCREEN0x0004 #define HUD_TOUCHPAD 0x0005 #define HUD_CONFIG 0x000e +#define HUD_STYLUS 0x0020 #define HUD_FINGER 0x0022 #define HUD_TIP_PRESSURE 0x0030 #define HUD_BARREL_PRESSURE0x0031 @@ -387,6 +392,12 @@ inthid_is_collection(const void *, int, #define HUD_CONTACT_MAX0x0055 #define HUD_SCAN_TIME 0x0056 #define HUD_BUTTON_TYPE0x0059 +#define HUD_SECONDARY_BARREL_SWITCH0x005A +#define HUD_WACOM_X0x0130 +#define HUD_WACOM_Y0x0131 +#define HUD_WACOM_DISTANCE 0x0132 +#define HUD_WACOM_PAD_BUTTONS000x0910 +#define HUD_WACOM_BATTERY 0x1013 /* Usages, LED */ #define HUL_NUM_LOCK 0x0001 Index: dev/hid/hidms.c === RCS file: /OpenBSD/src/sys/dev/hid/hidms.c,v retrieving revision 1.9 diff -u -p -r1.9 hidms.c --- dev/hid/hidms.c 16 Jun 2022 20:52:38 - 1.9 +++ dev/hid/hidms.c 12 Aug 2023 07:59:14 - @@ -61,6 +61,188 @@ int hidmsdebug = 0; #define MOUSE_FLAGS_MASK (HIO_CONST | HIO_RELATIVE) #define NOTMOUSE(f)(((f) & MOUSE_FLAGS_MASK) != HIO_RELATIVE) +void +hidms_stylus_hid_parse(struct hidms *ms, struct hid_data *d, +struct hid_location *loc_stylus_btn) +{ + struct hid_item h; + + while (hid_get_item(d, &h)) { + if (h.kind == hid_endcollection) + break; + if (h.kind != hid_input || (h.flags & HIO_CONST) != 0) + continue; + /* All the possible stylus reported usages go here */ +#ifdef HIDMS_DEBUG + printf("stylus usage: 0x%x\n", h.usage); +#endif + switch (h.usage) { + /* Buttons */ + case HID_USAGE2(HUP_WACOM | HUP_DIGIT