Re: [RFC] Adding ESRT and EFI variables for fwupd
Hi Mark, > I have done some work on adding support for EFI runtime services on > OpenBSD/amd64, based on the code for OpenBSD/arm64. My plan was to > implement an ioctl(2) interface that is compatible with FreeBSD's > interface. Theo objected to putting EFI-related headers > in /usr/include/sys, so the EFI-related headers will probably end up > in /usr/include/dev/efi (so you'd be include > instead). Great to hear that you went with FreeBSD's API. It's a natural choice for DragonBSD, and NetBSD chose compatible API on AArch64, so it sounds like all BSDs will have an almost identical API. Having header in a different location is a minor thing. I too was trying to make EFI RT work in [1], it doesn't crash the kernel by now, but GetTime() doesn't return proper data either. [1]: https://github.com/3mdeb/openbsd-src/compare/esrt...3mdeb:openbsd-src:efi-vars > I hope to have some time to make progress on this next week, so let me > come back to you then. Is the code available anywhere? By the way, do you plan to have something like `libefivar` provided as part of OpenBSD? Cheers, Sergii
Re: USB string descriptor requests
There doesn't seem to be much interest in this problem, so I'll just use
this change locally to solve my issue.
Though isn't it a tiny buffer overflow if a malicious or broken USB
device sends a bLength of 255 and actually follows through with it?
usb_string_descriptor_t is only 254 bytes.
On Sun, Aug 21, 2022 at 12:35:00AM -0400, bug wrote:
> I got a StarTech SV431USBDDM KVM switch, and it specifically mangles the
> string descriptor responses for my keyboard (which otherwise works just
> fine) when probed about the vendor string after plugging in or resetting
> the device.
>
> After a ton of messing around, I also found out that this problem just
> isn't happening at all if OpenBSD only sends a single string descriptor
> request with the maximum possible length, rather than two as it
> currently does.
>
> According to the USB 2.0 and 3.2 specs, devices that receive a larger
> wLength than the descriptor's actual length /should/ just send the full
> descriptor in a short packet. Of course, the current behavior isn't
> wrong either, if the wLength is shorter, then only the initial bytes or
> length (depending on the spec) /should/ be sent.
>
> (So unless there's some deeper bug in OpenBSD, my KVM switch is totally
> dropping the ball either way. I'll probably post about it on Misc later,
> unless details are desired here...)
>
> But I'm curious if there's a reason for sending two separate packets
> when one should be fine. Do certain USB devices expect it that way?
>
> I suppose it seems Windows does it this way too (the only reason the
> switch works there is because they neglect to check the vendor string),
> so the answer to that could very well be a hard yes...
>
> This is my first post, so sorry in advance if I did anything dumb. Diff
> provided for context.
>
>
> Index: usb_subr.c
> ===
> RCS file: /cvs/src/sys/dev/usb/usb_subr.c,v
> retrieving revision 1.158
> diff -u -p -r1.158 usb_subr.c
> --- usb_subr.c16 Feb 2022 06:23:42 - 1.158
> +++ usb_subr.c21 Aug 2022 03:33:10 -
> @@ -125,20 +125,17 @@ usbd_get_string_desc(struct usbd_device
> req.bRequest = UR_GET_DESCRIPTOR;
> USETW2(req.wValue, UDESC_STRING, sindex);
> USETW(req.wIndex, langid);
> - USETW(req.wLength, 2); /* size and descriptor type first */
> + USETW(req.wLength, sizeof(*sdesc)); /* size and descriptor type
> first */
> +
> err = usbd_do_request_flags(dev, , sdesc, USBD_SHORT_XFER_OK,
> , USBD_DEFAULT_TIMEOUT);
> +
> if (err)
> return (err);
>
> if (actlen < 2)
> return (USBD_SHORT_XFER);
>
> - USETW(req.wLength, sdesc->bLength); /* the whole string */
> - err = usbd_do_request_flags(dev, , sdesc, USBD_SHORT_XFER_OK,
> - , USBD_DEFAULT_TIMEOUT);
> - if (err)
> - return (err);
>
> if (actlen != sdesc->bLength) {
> DPRINTFN(-1, ("%s: expected %d, got %d\n", __func__,
>
Re: [PATCH] Exclude pico-debug from the uhid driver
"Theo de Raadt" wrote:
> Josuah Demangeon wrote:
> >
> > The author majbthrd also says all CMSIS-DAP adapters should provide
> > "CMSIS-DAP" somewhere in the product string as a way to detect them
> > if wanted.
>
> Devices are not detected by the string. The vendor/product is used
> instead.
>
> Though I guess who knows what kind of bullshit could happen with libusb
> code...
Thank you for your confirmation.
I checked for curiosity: it is happening at OpenOCD-level, and
provided by the device itself: nothing involving the kernel.
https://repo.or.cz/openocd.git/blob/HEAD:/src/jtag/drivers/cmsis_dap_usb_bulk.c#l201
...
/* Search for "CMSIS-DAP" in the interface string *
...
} else if (strstr(interface_str, "CMSIS-DAP")) {
...
Re: [RFC] acpi: add acpitimer_delay(), acpihpet_delay()
On Tue, Aug 23, 2022 at 04:04:39PM +1000, Jonathan Gray wrote:
> On Mon, Aug 22, 2022 at 09:37:02AM -0500, Scott Cheloha wrote:
> > On Wed, Aug 17, 2022 at 09:00:12PM +1000, Jonathan Gray wrote:
> > > On Wed, Aug 17, 2022 at 04:53:20PM +1000, Jonathan Gray wrote:
> > > >
> > > > It seems to me it would be cleaner if the decision of what to use for
> > > > delay could be moved into an md file.
> > > >
> > > > Or abstract it by having a numeric weight like timecounters or driver
> > > > match return numbers.
> > >
> > > diff against your previous, does not change lapic_delay
> >
> > Sorry for the delay.
> >
> > :)
> >
> > I was out of town.
> >
> > I slept on this and you're right, this is better.
> >
> > Couple tweaks:
> >
> > - Move the quality numbers into cpu.h and give them names. That way,
> > the next time Intel, or AMD, or Microsoft or [...] does something
> > foolish we don't need to rototill all these files to juggle the
> > quality hierarchy.
>
> While that would allow arch specific differences, removing one would
> require changing at least three places in the tree.
Okay, nevermind. Patch updated below.
> > - Update both amd64 and i386's lapic.c to use delay_init(). While we
> > have a lapic_delay() in the tree it should cooperate with everything
> > else.
> >
> > - Include in any files calling delay_init() where it
> > isn't already included.
> >
> > - Give the variables in delay_init() real names.
>
> hmm
>
> > I'm unsure about two small things:
> >
> > - Can i386 use hv_delay()? The i386 GENERIC config does not list
> > hyperv(4) support so my guess is "no" and I have excluded
> > HV_DELAY_QUALITY from i386's cpu.h.
> >
> > - If a Hyper-V guest could choose between hv_delay() and
> > lapic_delay(), which would be preferable? Right now I
> > have hv_delay() scored lower than lapic_delay().
>
> Hyper-V generation 1 VMs are bios boot with emulation of the usual
> devices. 32-bit and 64-bit guests.
>
> Hyper-V generation 2 VMs are 64-bit uefi with paravirtualised devices.
> 64-bit guests only.
>
> There is no 8254 in generation 2.
> No HPET in either generation.
>
> hv_delay uses the "Partition Reference Counter MSR" described in
> https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/timers
> It seems it is available in both generations and could be used from i386?
>
> From reading that page hv_delay() should be preferred over lapic_delay()
Alright, I have nudged hv_delay's quality up over lapic_delay's
quality.
How are we looking now?
Index: sys/arch/amd64/amd64/lapic.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/lapic.c,v
retrieving revision 1.60
diff -u -p -r1.60 lapic.c
--- sys/arch/amd64/amd64/lapic.c15 Aug 2022 04:17:50 - 1.60
+++ sys/arch/amd64/amd64/lapic.c23 Aug 2022 17:18:30 -
@@ -486,8 +486,6 @@ wait_next_cycle(void)
}
}
-extern void tsc_delay(int);
-
/*
* Calibrate the local apic count-down timer (which is running at
* bus-clock speed) vs. the i8254 counter/timer (which is running at
@@ -592,8 +590,7 @@ skip_calibration:
* Now that the timer's calibrated, use the apic timer routines
* for all our timing needs..
*/
- if (delay_func == i8254_delay)
- delay_func = lapic_delay;
+ delay_init(lapic_delay, 3000);
initclock_func = lapic_initclocks;
}
}
Index: sys/arch/amd64/amd64/machdep.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/machdep.c,v
retrieving revision 1.279
diff -u -p -r1.279 machdep.c
--- sys/arch/amd64/amd64/machdep.c 7 Aug 2022 23:56:06 - 1.279
+++ sys/arch/amd64/amd64/machdep.c 23 Aug 2022 17:18:31 -
@@ -2069,3 +2069,13 @@ check_context(const struct reg *regs, st
return 0;
}
+
+void
+delay_init(void(*fn)(int), int fn_quality)
+{
+ static int cur_quality = 0;
+ if (fn_quality > cur_quality) {
+ delay_func = fn;
+ cur_quality = fn_quality;
+ }
+}
Index: sys/arch/amd64/amd64/tsc.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
retrieving revision 1.25
diff -u -p -r1.25 tsc.c
--- sys/arch/amd64/amd64/tsc.c 12 Aug 2022 02:20:36 - 1.25
+++ sys/arch/amd64/amd64/tsc.c 23 Aug 2022 17:18:31 -
@@ -109,7 +109,7 @@ tsc_identify(struct cpu_info *ci)
tsc_frequency = tsc_freq_cpuid(ci);
if (tsc_frequency > 0)
- delay_func = tsc_delay;
+ delay_init(tsc_delay, 5000);
}
static inline int
Index: sys/arch/amd64/include/cpu.h
===
RCS file: /cvs/src/sys/arch/amd64/include/cpu.h,v
retrieving revision 1.148
diff -u -p -r1.148 cpu.h
---
Re: mention double quotes for passwords with white spaces
Am Tue, 23 Aug 2022 08:30:21 +0200 schrieb Otto Moerbeek : > On Tue, Aug 23, 2022 at 12:01:14AM +0200, Florian Viehweger wrote: > > > Hi, > > > > this is a diff mentioning double quotes for using passphrases > > containing whitespaces. > > > > Additionally adding a comma to a sentence for better clarity. > > > > Comments? > > Others already mentioned this is not a good idea. > > I like to add that the suggestion of double quotes is not quite right > for all cases, as it allows various expansion, e.g. of vars. "$HOME" > still would be not literal. Single quotes is better, as only the > single quote itself is special in this context. See ksh(1). While I still think new users could benefit from a hint at this section of the documentation, I see the point and won't argue further. Sorry for the noise. -- greetings, Florian Viehweger
bgpd silence "connection from non-peer" unless verbose
I noticed that the "connection from non-peer" message can fill the log and
be so chatty that it is hard to see the other messages. The system I see
this on is a bit special since it gets hammered by incorrectly configured
systems. Maybe other people find this message helpful. If so please
speak up now because I think the message does not add much info and should
be skipped unless verbose logging is used.
--
:wq Claudio
Index: logmsg.c
===
RCS file: /cvs/src/usr.sbin/bgpd/logmsg.c,v
retrieving revision 1.8
diff -u -p -r1.8 logmsg.c
--- logmsg.c28 Jul 2022 13:11:48 - 1.8
+++ logmsg.c23 Aug 2022 14:38:42 -
@@ -213,11 +213,11 @@ void
log_conn_attempt(const struct peer *peer, struct sockaddr *sa, socklen_t len)
{
char*p;
- const char *b;
if (peer == NULL) { /* connection from non-peer, drop */
- b = log_sockaddr(sa, len);
- logit(LOG_INFO, "connection from non-peer %s refused", b);
+ if (log_getverbose())
+ logit(LOG_INFO, "connection from non-peer %s refused",
+ log_sockaddr(sa, len));
} else {
/* only log if there is a chance that the session may come up */
if (peer->conf.down && peer->state == STATE_IDLE)
Re: [PATCH] Exclude pico-debug from the uhid driver
Josuah Demangeon wrote:
> Jonathan Gray wrote:
> > Index: usbdevs
> > ===
> > RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> > retrieving revision 1.747
> > diff -u -p -r1.747 usbdevs
> > --- usbdevs 23 Jun 2022 00:31:37 - 1.747
> > +++ usbdevs 23 Aug 2022 14:39:28 -
> > @@ -2432,6 +2432,7 @@ product INTEL2 RMH_8 0x800a Rate Matchi
> > product INTERBIO IOBOARD 0x1002 IO Board
> > product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
> > product INTERBIO MINIIOBOARD2 0x1006 Mini IO Board
> > +product INTERBIO DAPPERMISER 0x2488
> >
> > /* Intersil products */
> > productINTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
> >
> > results in a usbdevs_data.h entry of
> >
> > {
> > USB_VENDOR_INTERBIO, USB_PRODUCT_INTERBIO_DAPPERMISER,
> > "",
> > },
> >
> > reading usb_subr.c usbd_cache_devinfo() that should work
> >
> > though if the device doesn't provide a string and we don't either
> > we will print "" instead of "product 0x0x2488"
>
> The author majbthrd also says all CMSIS-DAP adapters should provide
> "CMSIS-DAP" somewhere in the product string as a way to detect them
> if wanted.
>
> For something short, he would opt for "Dapper Miser".
Devices are not detected by the string. The vendor/product is used
instead.
Though I guess who knows what kind of bullshit could happen with libusb
code...
Re: [PATCH] Exclude pico-debug from the uhid driver
> Date: Wed, 24 Aug 2022 00:57:40 +1000
> From: Jonathan Gray
>
> On Tue, Aug 23, 2022 at 08:24:24AM -0600, Theo de Raadt wrote:
> > Jonathan Gray wrote:
> >
> > > >
> > > > I could recompile and see the device listed as ugen(4):
> > > >
> > > > $ dmesg | grep pico-debug
> > > > ugen1 at uhub4 port 2 "pico-debug CMSIS-DAP" rev 1.10/10.05 addr 2
> > >
> > > "Peter Lawrence CMSIS-DAP Dapper Miser" is a rather large string
> > > especially as the device already provides one
> > >
> > > why not just "pico-debug" or "Dapper Miser"?
> >
> >
> > I've been wondering about this for a while.
> >
> >
> > All of these strings go into all kernels, using memory. If we know a
> > device provides a name, we always print the device-provided name, right?
> > Can we therefore skip the name entirely in usbdevs, or is usbdevs too dumb
> > to handle empty names?
>
> Index: usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.747
> diff -u -p -r1.747 usbdevs
> --- usbdevs 23 Jun 2022 00:31:37 - 1.747
> +++ usbdevs 23 Aug 2022 14:39:28 -
> @@ -2432,6 +2432,7 @@ product INTEL2 RMH_80x800a Rate Matchi
> product INTERBIO IOBOARD 0x1002 IO Board
> product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
> product INTERBIO MINIIOBOARD20x1006 Mini IO Board
> +product INTERBIO DAPPERMISER 0x2488
If we do this, we probably still want to provide a product name as a
comment.
> /* Intersil products */
> product INTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
>
> results in a usbdevs_data.h entry of
>
> {
> USB_VENDOR_INTERBIO, USB_PRODUCT_INTERBIO_DAPPERMISER,
> "",
> },
>
> reading usb_subr.c usbd_cache_devinfo() that should work
>
> though if the device doesn't provide a string and we don't either
> we will print "" instead of "product 0x0x2488"
>
>
Re: [PATCH] Exclude pico-debug from the uhid driver
Jonathan Gray wrote:
> Index: usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.747
> diff -u -p -r1.747 usbdevs
> --- usbdevs 23 Jun 2022 00:31:37 - 1.747
> +++ usbdevs 23 Aug 2022 14:39:28 -
> @@ -2432,6 +2432,7 @@ product INTEL2 RMH_80x800a Rate Matchi
> product INTERBIO IOBOARD 0x1002 IO Board
> product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
> product INTERBIO MINIIOBOARD20x1006 Mini IO Board
> +product INTERBIO DAPPERMISER 0x2488
>
> /* Intersil products */
> product INTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
>
> results in a usbdevs_data.h entry of
>
> {
> USB_VENDOR_INTERBIO, USB_PRODUCT_INTERBIO_DAPPERMISER,
> "",
> },
>
> reading usb_subr.c usbd_cache_devinfo() that should work
>
> though if the device doesn't provide a string and we don't either
> we will print "" instead of "product 0x0x2488"
The author majbthrd also says all CMSIS-DAP adapters should provide
"CMSIS-DAP" somewhere in the product string as a way to detect them
if wanted.
For something short, he would opt for "Dapper Miser".
Re: [PATCH] Exclude pico-debug from the uhid driver
On Tue, Aug 23, 2022 at 09:01:08AM -0600, Theo de Raadt wrote:
> Jonathan Gray wrote:
>
> > Index: usbdevs
> > ===
> > RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> > retrieving revision 1.747
> > diff -u -p -r1.747 usbdevs
> > --- usbdevs 23 Jun 2022 00:31:37 - 1.747
> > +++ usbdevs 23 Aug 2022 14:39:28 -
> > @@ -2432,6 +2432,7 @@ product INTEL2 RMH_8 0x800a Rate Matchi
> > product INTERBIO IOBOARD 0x1002 IO Board
> > product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
> > product INTERBIO MINIIOBOARD2 0x1006 Mini IO Board
> > +product INTERBIO DAPPERMISER 0x2488
> >
> > /* Intersil products */
> > productINTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
> >
> > results in a usbdevs_data.h entry of
> >
> > {
> > USB_VENDOR_INTERBIO, USB_PRODUCT_INTERBIO_DAPPERMISER,
> > "",
> > },
> >
> > reading usb_subr.c usbd_cache_devinfo() that should work
> >
> > though if the device doesn't provide a string and we don't either
> > we will print "" instead of "product 0x0x2488"
> >
>
> OK, so "" is being handled the way I think we want.
>
>
> For some devices like that, should we put the text description in /* .. */ or
> add support for // comments, so that we can remove it from the binary?
>
> I wonder if the "" strings are being merged by the assembler/linker.
'#' results in the same usbdevs_data.h output and usbdevs.h
then has
#define USB_PRODUCT_INTERBIO_DAPPERMISER0x2488 /* (CMSIS-DAP)
*/
trying to use /* CMSIS-DAP */ gives
#define USB_PRODUCT_INTERBIO_DAPPERMISER0x2488 /* /* CMSIS-DAP
*/ */
Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.747
diff -u -p -r1.747 usbdevs
--- usbdevs 23 Jun 2022 00:31:37 - 1.747
+++ usbdevs 23 Aug 2022 15:05:40 -
@@ -2432,6 +2432,7 @@ product INTEL2 RMH_8 0x800a Rate Matchi
product INTERBIO IOBOARD 0x1002 IO Board
product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
product INTERBIO MINIIOBOARD2 0x1006 Mini IO Board
+product INTERBIO DAPPERMISER 0x2488 # CMSIS-DAP
/* Intersil products */
productINTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
Re: [RFC] Adding ESRT and EFI variables for fwupd
> Date: Thu, 11 Aug 2022 20:48:58 +0300
> From: Sergii Dmytruk
Hi Sergeii,
Sorry for the late reply. I was enjoying a vacation without a laptop
nearby ;).
Having fwupd support for OpenBSD would be great!
I have done some work on adding support for EFI runtime services on
OpenBSD/amd64, based on the code for OpenBSD/arm64. My plan was to
implement an ioctl(2) interface that is compatible with FreeBSD's
interface. Theo objected to putting EFI-related headers
in /usr/include/sys, so the EFI-related headers will probably end up
in /usr/include/dev/efi (so you'd be include
instead).
I hope to have some time to make progress on this next week, so let me
come back to you then.
Cheers,
Mark
> Hello OpenBSD devs,
>
> Background
> --
>
> As some might know, FreeBSD gained an interface for querying ESRT
> about a year ago ([1], [2]), which was done as part of [3].
>
> [1]: https://reviews.freebsd.org/rGd12d651f8692cfcaf6fd0a6e8264c29547f644c9
> [2]: https://reviews.freebsd.org/rG24f398e7a153a05a7e94ae8dd623e2b6d28d94eb
> [3]: https://nlnet.nl/project/fwdup-BSD/
>
> Adding it allowed to make UEFI capsule plugin of fwupd [4] work for
> FreeBSD [5] to perform firmware updates on EFI systems.
>
> [4]: https://fwupd.org/
> [5]:
> https://github.com/fwupd/fwupd/blob/main/plugins/uefi-capsule/fu-uefi-backend-freebsd.c
>
> Now it's turn for OpenBSD and we would like to ask about requirements
> and recommendations on the API and its implementation such that they
> would be accepted by the upstream.
>
> ESRT Part
> -
>
> Initial idea is to use sysctl() interface like so:
>
> ```
> void *esrt;
> size_t esrt_len;
> int mib[] = { CTL_HW, HW_ESRT };
>
> esrt_len = 0;
> if (sysctl(mib, 2, NULL, _len, NULL, 0) == -1 && errno != ENOMEM)
> err(1, "sysctl");
>
> esrt = malloc(esrt_len);
> if (esrt == NULL)
> err(1, "malloc");
>
> if (sysctl(mib, 2, esrt, _len, NULL, 0) == -1)
> err(1, "sysctl");
> ```
>
> However, FreeBSD decided in favour of ioctl() on /dev/efi.
>
> EFI variables Part
> --
>
> I don't see an implementation of `efivar` or `efiboot` or anything
> similar, so it needs to be added and provide at least this API subset
> to be usable with fwupd:
>
> * efi_append_variable()
>appends data of size to the variable specified by guid and name
> * efi_del_variable()
>deletes the variable specified by guid and name
> * efi_get_variable()
>gets variable's data_size, and its attributes are stored in
>attributes
> * efi_get_variable_attributes()
>gets attributes for the variable specified by guid and name
> * efi_get_variable_size()
>gets the size of the data for the variable specified by guid and name
> * efi_get_next_variable_name()
>iterates across the currently extant variables, passing back a guid
>and name
> * efi_guid_to_name()
>translates from an efi_guid_t to a well known name
> * efi_guid_to_symbol()
>translates from an efi_guid_t to a unique (within libefivar) C-style
>symbol name
> * efi_guid_to_str()
>allocates a suitable string and populates it with string
>representation of a UEFI GUID
> * efi_name_to_guid()
>translates from a well known name to an efi_guid_t
> * efi_set_variable()
>sets the variable specified by guid and name
> * efi_str_to_guid()
>parses a UEFI GUID from string form to an efi_guid_t
> * efi_variables_supported()
>checks if EFI variables are accessible
> * efi_generate_file_device_path()
>generates an EFI file device path for an EFI binary from a filesystem
>path
>
> Probably should be implemented as a minimal port of efivar [6] with
> sufficient functionality rather than a separate project. Either way,
> this too needs new kernel API for accessing EFI variables. Will sysctl()
> do or a new pseudo-device or something else is needed? Usage of
> existing implementations can be seen in FreeBSD [7] and Linux [8] (Linux
> uses sysfs for this).
>
> [6]: https://github.com/rhboot/efivar
> [7]:
> https://github.com/freebsd/freebsd-src/blob/release/12.2.0/lib/libefivar/efivar.c
> [8]: https://github.com/rhboot/efivar/blob/main/src/efivarfs.c
>
> Implementation
> --
>
> I started making an EFI driver for amd64 in [9] (WIP). There isn't much
> to see there yet, but let me know if you have any specific ideas about
> implementation of ESRT and EFI variables, they can help to avoid too
> many rounds of review. For now I'm looking at `efifb.c` and
> `arm64/dev/efi.c` for how to do things.
>
> By the way, I couldn't find OpenBSD kernel hacking guide, manual or
> README. Is there a resource like that? Something that could answer
> questions like:
>
> * should new EFI-declarations go to `sys/dev/acpi/efi.h`?
> * should efiboot look for ESRT table like it does with 2 other tables?
>
> [9] https://github.com/openbsd/src/compare/master...3mdeb:openbsd-src:esrt
>
> Expectations
>
>
> Please help shaping this API, so you'll
Re: [PATCH] Exclude pico-debug from the uhid driver
Jonathan Gray wrote:
> Index: usbdevs
> ===
> RCS file: /cvs/src/sys/dev/usb/usbdevs,v
> retrieving revision 1.747
> diff -u -p -r1.747 usbdevs
> --- usbdevs 23 Jun 2022 00:31:37 - 1.747
> +++ usbdevs 23 Aug 2022 14:39:28 -
> @@ -2432,6 +2432,7 @@ product INTEL2 RMH_80x800a Rate Matchi
> product INTERBIO IOBOARD 0x1002 IO Board
> product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
> product INTERBIO MINIIOBOARD20x1006 Mini IO Board
> +product INTERBIO DAPPERMISER 0x2488
>
> /* Intersil products */
> product INTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
>
> results in a usbdevs_data.h entry of
>
> {
> USB_VENDOR_INTERBIO, USB_PRODUCT_INTERBIO_DAPPERMISER,
> "",
> },
>
> reading usb_subr.c usbd_cache_devinfo() that should work
>
> though if the device doesn't provide a string and we don't either
> we will print "" instead of "product 0x0x2488"
>
OK, so "" is being handled the way I think we want.
For some devices like that, should we put the text description in /* .. */ or
add support for // comments, so that we can remove it from the binary?
I wonder if the "" strings are being merged by the assembler/linker.
Re: [PATCH] Exclude pico-debug from the uhid driver
On Tue, Aug 23, 2022 at 08:24:24AM -0600, Theo de Raadt wrote:
> Jonathan Gray wrote:
>
> > >
> > > I could recompile and see the device listed as ugen(4):
> > >
> > > $ dmesg | grep pico-debug
> > > ugen1 at uhub4 port 2 "pico-debug CMSIS-DAP" rev 1.10/10.05 addr 2
> >
> > "Peter Lawrence CMSIS-DAP Dapper Miser" is a rather large string
> > especially as the device already provides one
> >
> > why not just "pico-debug" or "Dapper Miser"?
>
>
> I've been wondering about this for a while.
>
>
> All of these strings go into all kernels, using memory. If we know a
> device provides a name, we always print the device-provided name, right?
> Can we therefore skip the name entirely in usbdevs, or is usbdevs too dumb
> to handle empty names?
Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.747
diff -u -p -r1.747 usbdevs
--- usbdevs 23 Jun 2022 00:31:37 - 1.747
+++ usbdevs 23 Aug 2022 14:39:28 -
@@ -2432,6 +2432,7 @@ product INTEL2 RMH_8 0x800a Rate Matchi
product INTERBIO IOBOARD 0x1002 IO Board
product INTERBIO MINIIOBOARD 0x1003 Mini IO Board
product INTERBIO MINIIOBOARD2 0x1006 Mini IO Board
+product INTERBIO DAPPERMISER 0x2488
/* Intersil products */
productINTERSIL PRISM_GT 0x1000 PrismGT USB 2.0 WLAN
results in a usbdevs_data.h entry of
{
USB_VENDOR_INTERBIO, USB_PRODUCT_INTERBIO_DAPPERMISER,
"",
},
reading usb_subr.c usbd_cache_devinfo() that should work
though if the device doesn't provide a string and we don't either
we will print "" instead of "product 0x0x2488"
Re: [PATCH] Exclude pico-debug from the uhid driver
Jonathan Gray wrote: > > > > I could recompile and see the device listed as ugen(4): > > > > $ dmesg | grep pico-debug > > ugen1 at uhub4 port 2 "pico-debug CMSIS-DAP" rev 1.10/10.05 addr 2 > > "Peter Lawrence CMSIS-DAP Dapper Miser" is a rather large string > especially as the device already provides one > > why not just "pico-debug" or "Dapper Miser"? I've been wondering about this for a while. All of these strings go into all kernels, using memory. If we know a device provides a name, we always print the device-provided name, right? Can we therefore skip the name entirely in usbdevs, or is usbdevs too dumb to handle empty names?
Re: rpki-client: retire valid_cert()
On Mon, Aug 22, 2022 at 12:14:53PM +0200, Theo Buehler wrote:
> rpki-client portable makes sure that libcrypto has RFC 3779 support.
> Therefore the X509_verify_cert() call in valid_x509() will already
> perform the checks that the RFC 3779 extensions are covered along the
> chain. While valid_cert()'s errors would be nicer than the validator's,
> they can't be reached anymore.
I wonderd if we could beef up the error handler of X509_verify_cert() to
return better errors. Like calling the valid_ip/valid_as functions to
figure out what is not covered.
More urgently a similar change is needed for .mft files where the common
error of no valid mft found is very unhelpful to diagnose issues.
> The check that a BGPsec cert's AS numbers must not be inherited can be
> done in cert_parse_pre() like most of the other checks for BGPsec certs.
Sounds good to me.
> With the removal of valid_cert(), valid_as() and valid_ip() are unused
> and can also go.
In general I'm ok with the diff but see above about the wish for better
error reporting.
> Index: cert.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/cert.c,v
> retrieving revision 1.85
> diff -u -p -r1.85 cert.c
> --- cert.c19 Aug 2022 12:45:53 - 1.85
> +++ cert.c22 Aug 2022 09:52:07 -
> @@ -736,6 +736,13 @@ cert_parse_pre(const char *fn, const uns
> p.fn);
> goto out;
> }
> + for (i = 0; i < p.res->asz; i++) {
> + if (p.res->as[i].type == CERT_AS_INHERIT) {
> + warnx("%s: inherited AS numbers in BGPsec cert",
> + p.fn);
> + goto out;
> + }
> + }
> if (sia_present) {
> warnx("%s: unexpected SIA extension in BGPsec cert",
> p.fn);
> Index: filemode.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/filemode.c,v
> retrieving revision 1.8
> diff -u -p -r1.8 filemode.c
> --- filemode.c19 Aug 2022 12:45:53 - 1.8
> +++ filemode.c19 Aug 2022 19:53:17 -
> @@ -168,8 +168,7 @@ parse_load_certchain(char *uri)
> uri = filestack[i];
>
> crl = crl_get(, a);
> - if (!valid_x509(uri, ctx, cert->x509, a, crl, 0) ||
> - !valid_cert(uri, a, cert))
> + if (!valid_x509(uri, ctx, cert->x509, a, crl, 0))
> goto fail;
> cert->talid = a->cert->talid;
> a = auth_insert(, cert, a);
> Index: parser.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/parser.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 parser.c
> --- parser.c 19 Aug 2022 12:45:53 - 1.74
> +++ parser.c 19 Aug 2022 19:52:53 -
> @@ -404,8 +404,7 @@ proc_parser_cert(char *file, const unsig
> a = valid_ski_aki(file, , cert->ski, cert->aki);
> crl = crl_get(, a);
>
> - if (!valid_x509(file, ctx, cert->x509, a, crl, 0) ||
> - !valid_cert(file, a, cert)) {
> + if (!valid_x509(file, ctx, cert->x509, a, crl, 0)) {
> cert_free(cert);
> return NULL;
> }
> Index: validate.c
> ===
> RCS file: /cvs/src/usr.sbin/rpki-client/validate.c,v
> retrieving revision 1.41
> diff -u -p -r1.41 validate.c
> --- validate.c19 Aug 2022 12:45:53 - 1.41
> +++ validate.c19 Aug 2022 19:52:22 -
> @@ -33,56 +33,6 @@
> extern ASN1_OBJECT *certpol_oid;
>
> /*
> - * Walk up the chain of certificates trying to match our AS number to
> - * one of the allocations in that chain.
> - * Returns 1 if covered or 0 if not.
> - */
> -static int
> -valid_as(struct auth *a, uint32_t min, uint32_t max)
> -{
> - int c;
> -
> - if (a == NULL)
> - return 0;
> -
> - /* Does this certificate cover our AS number? */
> - c = as_check_covered(min, max, a->cert->as, a->cert->asz);
> - if (c > 0)
> - return 1;
> - else if (c < 0)
> - return 0;
> -
> - /* If it inherits, walk up the chain. */
> - return valid_as(a->parent, min, max);
> -}
> -
> -/*
> - * Walk up the chain of certificates (really just the last one, but in
> - * the case of inheritance, the ones before) making sure that our IP
> - * prefix is covered in the first non-inheriting specification.
> - * Returns 1 if covered or 0 if not.
> - */
> -static int
> -valid_ip(struct auth *a, enum afi afi,
> -const unsigned char *min, const unsigned char *max)
> -{
> - int c;
> -
> - if (a == NULL)
> - return 0;
> -
> - /* Does this certificate cover our IP prefix? */
> - c = ip_addr_check_covered(afi, min, max,
bgpd move nexthop connected magic to kroute
The RDE does some magic dance around connected networks and their gateway
which should be done in kroute.c instead. At least then both functions doing
gateway lookups do this magic in the same .c file.
It also makes the RDE code a simpler which is good. The RDE actually no
longer uses this information apart from reporting it to bgpctl.
--
:wq Claudio
Index: kroute.c
===
RCS file: /cvs/src/usr.sbin/bgpd/kroute.c,v
retrieving revision 1.295
diff -u -p -r1.295 kroute.c
--- kroute.c19 Aug 2022 09:11:18 - 1.295
+++ kroute.c23 Aug 2022 09:01:26 -
@@ -2265,11 +2265,11 @@ knexthop_send_update(struct knexthop *kn
kr = kn->kroute;
n.valid = kroute_validate(kr);
n.connected = kr->flags & F_CONNECTED;
- if (kr->nexthop.s_addr != 0) {
+ if (!n.connected) {
n.gateway.aid = AID_INET;
n.gateway.v4.s_addr = kr->nexthop.s_addr;
- }
- if (n.connected) {
+ } else {
+ n.gateway = n.nexthop;
n.net.aid = AID_INET;
n.net.v4.s_addr = kr->prefix.s_addr;
n.netlen = kr->prefixlen;
@@ -2279,13 +2279,12 @@ knexthop_send_update(struct knexthop *kn
kr6 = kn->kroute;
n.valid = kroute6_validate(kr6);
n.connected = kr6->flags & F_CONNECTED;
- if (memcmp(>nexthop, _any,
- sizeof(struct in6_addr)) != 0) {
+ if (!n.connected) {
n.gateway.aid = AID_INET6;
n.gateway.v6 = kr6->nexthop;
n.gateway.scope_id = kr6->nexthop_scope_id;
- }
- if (n.connected) {
+ } else {
+ n.gateway = n.nexthop;
n.net.aid = AID_INET6;
n.net.v6 = kr6->prefix;
n.net.scope_id = kr6->prefix_scope_id;
Index: rde_rib.c
===
RCS file: /cvs/src/usr.sbin/bgpd/rde_rib.c,v
retrieving revision 1.243
diff -u -p -r1.243 rde_rib.c
--- rde_rib.c 10 Aug 2022 14:17:01 - 1.243
+++ rde_rib.c 23 Aug 2022 09:01:26 -
@@ -1739,12 +1739,10 @@ nexthop_update(struct kroute_nexthop *ms
TAILQ_REMOVE(_runners, nh, runner_l);
}
- if (msg->connected) {
+ if (msg->connected)
nh->flags |= NEXTHOP_CONNECTED;
- nh->true_nexthop = nh->exit_nexthop;
- } else
- nh->true_nexthop = msg->gateway;
+ nh->true_nexthop = msg->gateway;
nh->nexthop_net = msg->net;
nh->nexthop_netlen = msg->netlen;
Re: move PRU_RCVD request to (*pru_rcvd)()
On Mon, Aug 22, 2022 at 11:08:07PM -0900, Philip Guenther wrote:
> Since pru_rcvd() is only invoked if the protocol has the PR_WANTRCVD flag
> set, there should be no need to test whether the callback is set: a
> protocol without the callback MUST NOT have PR_WANTRCVD.
>
> (I guess this could, alternatively, go the other direction and eliminate
> PR_WANTRCVD and use the presence of the callback to decide whether the
> protocol needs anything to be done.)
I don't want this. At least the per buffer locking could require relock
in the PRU_RCVD path, and I don't to do it within handler or pru_rcvd()
wrapper.
>
> Side note: pru_rcvd() (and the pru_rcvd implementations) should have a
> return type of void.
>
Since the TCP socket could exist without PCB, and we want to return
error in this case, all callbacks should return int. In other hand, we
always do `so_pcb' NULL check before call pru_rcvd() and we don't
interesting in the pru_rcvd() return value.
Also PRU_RCVD is not the only request where return value type could
be changed to void, at least PRU_SHUTDOWN handlers have no error path,
except the same case for TCP sockets.
Anyway, as I said in the one of preceding split diff for unix sockets
case, this time I want only split existing (*pru_usrreq)() handlers, and
leave possible refactoring to the future. The split diffs are mostly
mechanical, but huge enough, and I don't want to make them harder.
So I want to commit this as is.
>
> Philip Guenther
>
>
>
> On Mon, Aug 22, 2022 at 1:40 PM Vitaliy Makkoveev wrote:
>
> > Another one.
> >
> > Since we never use `flags' arg within handlers, remove it from the
> > pru_rcvd() args.
> >
> > Index: sys/sys/protosw.h
> > ===
> > RCS file: /cvs/src/sys/sys/protosw.h,v
> > retrieving revision 1.43
> > diff -u -p -r1.43 protosw.h
> > --- sys/sys/protosw.h 22 Aug 2022 21:18:48 - 1.43
> > +++ sys/sys/protosw.h 22 Aug 2022 22:27:08 -
> > @@ -72,6 +72,7 @@ struct pr_usrreqs {
> > int (*pru_accept)(struct socket *, struct mbuf *);
> > int (*pru_disconnect)(struct socket *);
> > int (*pru_shutdown)(struct socket *);
> > + int (*pru_rcvd)(struct socket *);
> > };
> >
> > struct protosw {
> > @@ -314,10 +315,11 @@ pru_shutdown(struct socket *so)
> > }
> >
> > static inline int
> > -pru_rcvd(struct socket *so, int flags)
> > +pru_rcvd(struct socket *so)
> > {
> > - return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> > - PRU_RCVD, NULL, (struct mbuf *)(long)flags, NULL, curproc);
> > + if (so->so_proto->pr_usrreqs->pru_rcvd)
> > + return (*so->so_proto->pr_usrreqs->pru_rcvd)(so);
> > + return (EOPNOTSUPP);
> > }
> >
> > static inline int
> > Index: sys/kern/uipc_socket.c
> > ===
> > RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> > retrieving revision 1.284
> > diff -u -p -r1.284 uipc_socket.c
> > --- sys/kern/uipc_socket.c 21 Aug 2022 16:22:17 - 1.284
> > +++ sys/kern/uipc_socket.c 22 Aug 2022 22:27:08 -
> > @@ -1156,7 +1156,7 @@ dontblock:
> > SBLASTRECORDCHK(>so_rcv, "soreceive 4");
> > SBLASTMBUFCHK(>so_rcv, "soreceive 4");
> > if (pr->pr_flags & PR_WANTRCVD && so->so_pcb)
> > - pru_rcvd(so, flags);
> > + pru_rcvd(so);
> > }
> > if (orig_resid == uio->uio_resid && orig_resid &&
> > (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) ==
> > 0) {
> > @@ -1521,7 +1521,7 @@ somove(struct socket *so, int wait)
> > if (m == NULL) {
> > sbdroprecord(so, >so_rcv);
> > if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
> > - pru_rcvd(so, 0);
> > + pru_rcvd(so);
> > goto nextpkt;
> > }
> >
> > @@ -1627,7 +1627,7 @@ somove(struct socket *so, int wait)
> >
> > /* Send window update to source peer as receive buffer has
> > changed. */
> > if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
> > - pru_rcvd(so, 0);
> > + pru_rcvd(so);
> >
> > /* Receive buffer did shrink by len bytes, adjust oob. */
> > state = so->so_state;
> > Index: sys/kern/uipc_usrreq.c
> > ===
> > RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> > retrieving revision 1.174
> > diff -u -p -r1.174 uipc_usrreq.c
> > --- sys/kern/uipc_usrreq.c 22 Aug 2022 21:18:48 - 1.174
> > +++ sys/kern/uipc_usrreq.c 22 Aug 2022 22:27:08 -
> > @@ -136,6 +136,7 @@ const struct pr_usrreqs uipc_usrreqs = {
> > .pru_accept = uipc_accept,
> > .pru_disconnect = uipc_disconnect,
> > .pru_shutdown = uipc_shutdown,
> > + .pru_rcvd = uipc_rcvd,
> > };
>
Re: Simcom 5320 umsm support
On Sun, Aug 21, 2022 at 04:26:23AM +, jon@elytron.openbsd.amsterdam wrote: > Here is a diff to add support for the Simcom > 5320 modem. Works right away with umsm, been > testing it for a while thanks, committed
Re: move PRU_RCVD request to (*pru_rcvd)()
Since pru_rcvd() is only invoked if the protocol has the PR_WANTRCVD flag
set, there should be no need to test whether the callback is set: a
protocol without the callback MUST NOT have PR_WANTRCVD.
(I guess this could, alternatively, go the other direction and eliminate
PR_WANTRCVD and use the presence of the callback to decide whether the
protocol needs anything to be done.)
Side note: pru_rcvd() (and the pru_rcvd implementations) should have a
return type of void.
Philip Guenther
On Mon, Aug 22, 2022 at 1:40 PM Vitaliy Makkoveev wrote:
> Another one.
>
> Since we never use `flags' arg within handlers, remove it from the
> pru_rcvd() args.
>
> Index: sys/sys/protosw.h
> ===
> RCS file: /cvs/src/sys/sys/protosw.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 protosw.h
> --- sys/sys/protosw.h 22 Aug 2022 21:18:48 - 1.43
> +++ sys/sys/protosw.h 22 Aug 2022 22:27:08 -
> @@ -72,6 +72,7 @@ struct pr_usrreqs {
> int (*pru_accept)(struct socket *, struct mbuf *);
> int (*pru_disconnect)(struct socket *);
> int (*pru_shutdown)(struct socket *);
> + int (*pru_rcvd)(struct socket *);
> };
>
> struct protosw {
> @@ -314,10 +315,11 @@ pru_shutdown(struct socket *so)
> }
>
> static inline int
> -pru_rcvd(struct socket *so, int flags)
> +pru_rcvd(struct socket *so)
> {
> - return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> - PRU_RCVD, NULL, (struct mbuf *)(long)flags, NULL, curproc);
> + if (so->so_proto->pr_usrreqs->pru_rcvd)
> + return (*so->so_proto->pr_usrreqs->pru_rcvd)(so);
> + return (EOPNOTSUPP);
> }
>
> static inline int
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.284
> diff -u -p -r1.284 uipc_socket.c
> --- sys/kern/uipc_socket.c 21 Aug 2022 16:22:17 - 1.284
> +++ sys/kern/uipc_socket.c 22 Aug 2022 22:27:08 -
> @@ -1156,7 +1156,7 @@ dontblock:
> SBLASTRECORDCHK(>so_rcv, "soreceive 4");
> SBLASTMBUFCHK(>so_rcv, "soreceive 4");
> if (pr->pr_flags & PR_WANTRCVD && so->so_pcb)
> - pru_rcvd(so, flags);
> + pru_rcvd(so);
> }
> if (orig_resid == uio->uio_resid && orig_resid &&
> (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) ==
> 0) {
> @@ -1521,7 +1521,7 @@ somove(struct socket *so, int wait)
> if (m == NULL) {
> sbdroprecord(so, >so_rcv);
> if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
> - pru_rcvd(so, 0);
> + pru_rcvd(so);
> goto nextpkt;
> }
>
> @@ -1627,7 +1627,7 @@ somove(struct socket *so, int wait)
>
> /* Send window update to source peer as receive buffer has
> changed. */
> if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
> - pru_rcvd(so, 0);
> + pru_rcvd(so);
>
> /* Receive buffer did shrink by len bytes, adjust oob. */
> state = so->so_state;
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.174
> diff -u -p -r1.174 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c 22 Aug 2022 21:18:48 - 1.174
> +++ sys/kern/uipc_usrreq.c 22 Aug 2022 22:27:08 -
> @@ -136,6 +136,7 @@ const struct pr_usrreqs uipc_usrreqs = {
> .pru_accept = uipc_accept,
> .pru_disconnect = uipc_disconnect,
> .pru_shutdown = uipc_shutdown,
> + .pru_rcvd = uipc_rcvd,
> };
>
> void
> @@ -243,32 +244,6 @@ uipc_usrreq(struct socket *so, int req,
> }
> break;
>
> - case PRU_RCVD:
> - switch (so->so_type) {
> -
> - case SOCK_DGRAM:
> - panic("uipc 1");
> - /*NOTREACHED*/
> -
> - case SOCK_STREAM:
> - case SOCK_SEQPACKET:
> - if ((so2 = unp_solock_peer(so)) == NULL)
> - break;
> - /*
> -* Adjust backpressure on sender
> -* and wakeup any waiting to write.
> -*/
> - so2->so_snd.sb_mbcnt = so->so_rcv.sb_mbcnt;
> - so2->so_snd.sb_cc = so->so_rcv.sb_cc;
> - sowwakeup(so2);
> - sounlock(so2);
> - break;
> -
> - default:
> - panic("uipc 2");
> - }
> - break;
> -
> case PRU_SEND:
> if (control) {
> sounlock(so);
> @@ -567,6 +542,37 @@
Re: move PRU_RCVD request to (*pru_rcvd)()
On Tue, Aug 23, 2022 at 01:39:12AM +0300, Vitaliy Makkoveev wrote:
> Another one.
>
> Since we never use `flags' arg within handlers, remove it from the
> pru_rcvd() args.
OK bluhm@
> Index: sys/sys/protosw.h
> ===
> RCS file: /cvs/src/sys/sys/protosw.h,v
> retrieving revision 1.43
> diff -u -p -r1.43 protosw.h
> --- sys/sys/protosw.h 22 Aug 2022 21:18:48 - 1.43
> +++ sys/sys/protosw.h 22 Aug 2022 22:27:08 -
> @@ -72,6 +72,7 @@ struct pr_usrreqs {
> int (*pru_accept)(struct socket *, struct mbuf *);
> int (*pru_disconnect)(struct socket *);
> int (*pru_shutdown)(struct socket *);
> + int (*pru_rcvd)(struct socket *);
> };
>
> struct protosw {
> @@ -314,10 +315,11 @@ pru_shutdown(struct socket *so)
> }
>
> static inline int
> -pru_rcvd(struct socket *so, int flags)
> +pru_rcvd(struct socket *so)
> {
> - return (*so->so_proto->pr_usrreqs->pru_usrreq)(so,
> - PRU_RCVD, NULL, (struct mbuf *)(long)flags, NULL, curproc);
> + if (so->so_proto->pr_usrreqs->pru_rcvd)
> + return (*so->so_proto->pr_usrreqs->pru_rcvd)(so);
> + return (EOPNOTSUPP);
> }
>
> static inline int
> Index: sys/kern/uipc_socket.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_socket.c,v
> retrieving revision 1.284
> diff -u -p -r1.284 uipc_socket.c
> --- sys/kern/uipc_socket.c21 Aug 2022 16:22:17 - 1.284
> +++ sys/kern/uipc_socket.c22 Aug 2022 22:27:08 -
> @@ -1156,7 +1156,7 @@ dontblock:
> SBLASTRECORDCHK(>so_rcv, "soreceive 4");
> SBLASTMBUFCHK(>so_rcv, "soreceive 4");
> if (pr->pr_flags & PR_WANTRCVD && so->so_pcb)
> - pru_rcvd(so, flags);
> + pru_rcvd(so);
> }
> if (orig_resid == uio->uio_resid && orig_resid &&
> (flags & MSG_EOR) == 0 && (so->so_state & SS_CANTRCVMORE) == 0) {
> @@ -1521,7 +1521,7 @@ somove(struct socket *so, int wait)
> if (m == NULL) {
> sbdroprecord(so, >so_rcv);
> if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
> - pru_rcvd(so, 0);
> + pru_rcvd(so);
> goto nextpkt;
> }
>
> @@ -1627,7 +1627,7 @@ somove(struct socket *so, int wait)
>
> /* Send window update to source peer as receive buffer has changed. */
> if (so->so_proto->pr_flags & PR_WANTRCVD && so->so_pcb)
> - pru_rcvd(so, 0);
> + pru_rcvd(so);
>
> /* Receive buffer did shrink by len bytes, adjust oob. */
> state = so->so_state;
> Index: sys/kern/uipc_usrreq.c
> ===
> RCS file: /cvs/src/sys/kern/uipc_usrreq.c,v
> retrieving revision 1.174
> diff -u -p -r1.174 uipc_usrreq.c
> --- sys/kern/uipc_usrreq.c22 Aug 2022 21:18:48 - 1.174
> +++ sys/kern/uipc_usrreq.c22 Aug 2022 22:27:08 -
> @@ -136,6 +136,7 @@ const struct pr_usrreqs uipc_usrreqs = {
> .pru_accept = uipc_accept,
> .pru_disconnect = uipc_disconnect,
> .pru_shutdown = uipc_shutdown,
> + .pru_rcvd = uipc_rcvd,
> };
>
> void
> @@ -243,32 +244,6 @@ uipc_usrreq(struct socket *so, int req,
> }
> break;
>
> - case PRU_RCVD:
> - switch (so->so_type) {
> -
> - case SOCK_DGRAM:
> - panic("uipc 1");
> - /*NOTREACHED*/
> -
> - case SOCK_STREAM:
> - case SOCK_SEQPACKET:
> - if ((so2 = unp_solock_peer(so)) == NULL)
> - break;
> - /*
> - * Adjust backpressure on sender
> - * and wakeup any waiting to write.
> - */
> - so2->so_snd.sb_mbcnt = so->so_rcv.sb_mbcnt;
> - so2->so_snd.sb_cc = so->so_rcv.sb_cc;
> - sowwakeup(so2);
> - sounlock(so2);
> - break;
> -
> - default:
> - panic("uipc 2");
> - }
> - break;
> -
> case PRU_SEND:
> if (control) {
> sounlock(so);
> @@ -567,6 +542,37 @@ uipc_shutdown(struct socket *so)
>
> socantsendmore(so);
> unp_shutdown(unp);
> + return (0);
> +}
> +
> +int
> +uipc_rcvd(struct socket *so)
> +{
> + struct socket *so2;
> +
> + switch (so->so_type) {
> + case SOCK_DGRAM:
> + panic("uipc 1");
> + /*NOTREACHED*/
> +
> + case SOCK_STREAM:
> + case SOCK_SEQPACKET:
> + if ((so2 = unp_solock_peer(so)) == NULL)
> + break;
> + /*
> + * Adjust backpressure on sender
> + * and wakeup any waiting to write.
> +
Re: [PATCH] Exclude pico-debug from the uhid driver
On Mon, Aug 22, 2022 at 08:33:45PM +0200, Josuah Demangeon wrote:
> The pico-debug [1] is a debug firmware, loaded on a Raspberry Pi RP2040
> microcontroller to provide a standard debug interface.
> The host support tool OpenOCD already upstreamed it [2].
>
> But it does not work with OpenBSD yet, as uhid(4) takes over the
> RP2040 once plugged, blocking full libusb access [3].
>
> This patch excludes the vendor and product ID chosen by the
> pico-debug developer from uhid and uhidev.
>
> The USB Product ID is not from the USB Implementation Forum:
> A project reuses some of the defunct InterBiometrics product IDs
> for open-source hardware [4].
We normally have one vendor line per vid
I don't see why this should be any different
>
> I could recompile and see the device listed as ugen(4):
>
> $ dmesg | grep pico-debug
> ugen1 at uhub4 port 2 "pico-debug CMSIS-DAP" rev 1.10/10.05 addr 2
"Peter Lawrence CMSIS-DAP Dapper Miser" is a rather large string
especially as the device already provides one
why not just "pico-debug" or "Dapper Miser"?
>
> Does this patch make sense?
> Thanks!
>
> [1]: https://github.com/majbthrd/pico-debug/discussions/23
> [2]: https://repo.or.cz/openocd.git/commitdiff/b60d06?hp=64a3e7
> [3]: https://marc.info/?l=openbsd-misc=160336348703669
> [4]: https://pid.codes/faq/
>
> diff --git sys/dev/usb/usb_quirks.c sys/dev/usb/usb_quirks.c
> index be65ad086..b0e29038a 100644
> --- sys/dev/usb/usb_quirks.c
> +++ sys/dev/usb/usb_quirks.c
> @@ -134,6 +134,7 @@ const struct usbd_quirk_entry {
> { USB_VENDOR_OMRON, USB_PRODUCT_OMRON_BX35F,ANY,{
> UQ_BAD_HID }},
> { USB_VENDOR_OMRON, USB_PRODUCT_OMRON_BX50F,ANY,{
> UQ_BAD_HID }},
> { USB_VENDOR_OMRON, USB_PRODUCT_OMRON_BY35S,ANY,{
> UQ_BAD_HID }},
> + { USB_VENDOR_PIDCODES, USB_PRODUCT_PIDCODES_DAPPERMISER,ANY,{
> UQ_BAD_HID }},
> { USB_VENDOR_TENX, USB_PRODUCT_TENX_MISSILE,ANY,{
> UQ_BAD_HID }},
> { USB_VENDOR_TERRATEC, USB_PRODUCT_TERRATEC_AUREON, ANY,{ UQ_BAD_HID }},
> { USB_VENDOR_TI, USB_PRODUCT_TI_MSP430, ANY,{ UQ_BAD_HID }},
> diff --git sys/dev/usb/usbdevs sys/dev/usb/usbdevs
> index 398e93df1..8690a0cad 100644
> --- sys/dev/usb/usbdevs
> +++ sys/dev/usb/usbdevs
> @@ -510,6 +510,7 @@ vendor SIERRA 0x1199 Sierra Wireless
> vendor SIEMENS3 0x11f5 Siemens
> vendor ALCATEL 0x11f7 Alcatel
> vendor INTERBIO 0x1209 InterBiometrics
> +vendor PIDCODES 0x1209 pid.codes
> vendor UNKNOWN3 0x1233 Unknown vendor
> vendor TSUNAMI 0x1241 Tsunami
> vendor PHEENET 0x124a Pheenet
> @@ -1516,6 +1517,9 @@ product DAISY DMC 0x6901 PhotoClip
> product DALLAS USB_FOB_IBUTTON 0x2490 USB-FOB/iBUTTON
> product DALLAS J6502 0x4201 J-6502 speakers
>
> +/* pid.codes products */
> +product PIDCODES DAPPERMISER 0x2488 Peter Lawrence CMSIS-DAP Dapper Miser
> +
> /* DataApex products */
> product DATAAPEX MULTICOM0xead6 MultiCom
>
> diff --git sys/dev/usb/usbdevs.h sys/dev/usb/usbdevs.h
> index a6bdfbd85..23d9124b9 100644
> --- sys/dev/usb/usbdevs.h
> +++ sys/dev/usb/usbdevs.h
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: usbdevs.h,v 1.759 2022/06/23 00:32:06 jsg Exp $ */
> +/* $OpenBSD$ */
>
> /*
> * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT.
> @@ -517,6 +517,7 @@
> #define USB_VENDOR_SIEMENS3 0x11f5 /* Siemens */
> #define USB_VENDOR_ALCATEL 0x11f7 /* Alcatel */
> #define USB_VENDOR_INTERBIO 0x1209 /* InterBiometrics */
> +#define USB_VENDOR_PIDCODES 0x1209 /* pid.codes */
> #define USB_VENDOR_UNKNOWN3 0x1233 /* Unknown vendor */
> #define USB_VENDOR_TSUNAMI 0x1241 /* Tsunami */
> #define USB_VENDOR_PHEENET 0x124a /* Pheenet */
> @@ -1523,6 +1524,9 @@
> #define USB_PRODUCT_DALLAS_USB_FOB_IBUTTON 0x2490 /*
> USB-FOB/iBUTTON */
> #define USB_PRODUCT_DALLAS_J65020x4201 /* J-6502
> speakers */
>
> +/* pid.codes products */
> +#define USB_PRODUCT_PIDCODES_DAPPERMISER0x2488 /*
> Peter Lawrence CMSIS-DAP Dapper Miser */
> +
> /* DataApex products */
> #define USB_PRODUCT_DATAAPEX_MULTICOM 0xead6 /* MultiCom */
>
> diff --git sys/dev/usb/usbdevs_data.h sys/dev/usb/usbdevs_data.h
> index 38a43697c..b5b546c67 100644
> --- sys/dev/usb/usbdevs_data.h
> +++ sys/dev/usb/usbdevs_data.h
> @@ -1,4 +1,4 @@
> -/* $OpenBSD: usbdevs_data.h,v 1.753 2022/06/23 00:32:06 jsg Exp $ */
> +/* $OpenBSD$ */
>
> /*
> * THIS FILE IS AUTOMATICALLY GENERATED. DO NOT EDIT.
> @@ -2437,6 +2437,10 @@ const struct usb_known_product usb_known_products[] = {
> USB_VENDOR_DALLAS, USB_PRODUCT_DALLAS_J6502,
> "J-6502 speakers",
>
Re: [RFC] acpi: add acpitimer_delay(), acpihpet_delay()
On Mon, Aug 22, 2022 at 09:37:02AM -0500, Scott Cheloha wrote: > On Wed, Aug 17, 2022 at 09:00:12PM +1000, Jonathan Gray wrote: > > On Wed, Aug 17, 2022 at 04:53:20PM +1000, Jonathan Gray wrote: > > > > > > It seems to me it would be cleaner if the decision of what to use for > > > delay could be moved into an md file. > > > > > > Or abstract it by having a numeric weight like timecounters or driver > > > match return numbers. > > > > diff against your previous, does not change lapic_delay > > Sorry for the delay. > > :) > > I was out of town. > > I slept on this and you're right, this is better. > > Couple tweaks: > > - Move the quality numbers into cpu.h and give them names. That way, > the next time Intel, or AMD, or Microsoft or [...] does something > foolish we don't need to rototill all these files to juggle the > quality hierarchy. While that would allow arch specific differences, removing one would require changing at least three places in the tree. > > - Update both amd64 and i386's lapic.c to use delay_init(). While we > have a lapic_delay() in the tree it should cooperate with everything > else. > > - Include in any files calling delay_init() where it > isn't already included. > > - Give the variables in delay_init() real names. hmm > > I'm unsure about two small things: > > - Can i386 use hv_delay()? The i386 GENERIC config does not list > hyperv(4) support so my guess is "no" and I have excluded > HV_DELAY_QUALITY from i386's cpu.h. > > - If a Hyper-V guest could choose between hv_delay() and > lapic_delay(), which would be preferable? Right now I > have hv_delay() scored lower than lapic_delay(). Hyper-V generation 1 VMs are bios boot with emulation of the usual devices. 32-bit and 64-bit guests. Hyper-V generation 2 VMs are 64-bit uefi with paravirtualised devices. 64-bit guests only. There is no 8254 in generation 2. No HPET in either generation. hv_delay uses the "Partition Reference Counter MSR" described in https://docs.microsoft.com/en-us/virtualization/hyper-v-on-windows/tlfs/timers It seems it is available in both generations and could be used from i386? >From reading that page hv_delay() should be preferred over lapic_delay()
