Null pointer crash in filt_uhidrdetach
While playing with chromium u2f support[1] I managed to induce kernel
crashes in filt_uhidrdetach. It takes a few attempts of plugging/unplugging
the fido key while trying to authenticate at demo.yubico.com/playground.
Eventually the kernel panics with this stack trace (retyped from [2]):
filt_uhidrdetach+0x33: movq 0x8(%rcx), rcx
kqueue_close
drop
closef
fdfree
exit1
single_thread_check
userret
intr_user_exit
The blunt patch below makes the kernel not crash and print the diagnostic
message, but it's really crude because I don't know what I'm doing.
diff --git a/sys/dev/usb/uhid.c b/sys/dev/usb/uhid.c
index 9cadd22ad35..428b7a63770 100644
--- a/sys/dev/usb/uhid.c
+++ b/sys/dev/usb/uhid.c
@@ -441,7 +441,10 @@ filt_uhidrdetach(struct knote *kn)
{
struct uhid_softc *sc = (void *)kn->kn_hook;
int s;
-
+ if (SLIST_FIRST(>sc_rsel.si_note) == NULL) {
+ printf("SLIST_FIRST is null\n");
+ return;
+ }
s = splusb();
SLIST_REMOVE(>sc_rsel.si_note, kn, knote, kn_selnext);
splx(s);
Thanks
Greg
[1] https://marc.info/?l=openbsd-ports=157784078117717
[2] https://photos.app.goo.gl/9ZZhiHvMoYYYHDEx5
--
nest.cx is Gmail hosted, use PGP:
https://pgp.key-server.io/0x0B1542BD8DF5A1B0
Fingerprint: 5E2B 2D0E 1E03 2046 BEC3 4D50 0B15 42BD 8DF5 A1B0
uppercase usbdevs product defines
Consistently uppercase usb product defines.
Index: if_urtwn.c
===
RCS file: /cvs/src/sys/dev/usb/if_urtwn.c,v
retrieving revision 1.85
diff -u -p -r1.85 if_urtwn.c
--- if_urtwn.c 16 Nov 2019 14:08:31 - 1.85
+++ if_urtwn.c 4 Jan 2020 04:22:37 -
@@ -283,7 +283,7 @@ static const struct urtwn_type {
URTWN_DEV_8192CU(IODATA,RTL8192CU),
URTWN_DEV_8192CU(NETGEAR, N300MA),
URTWN_DEV_8192CU(NETGEAR, WNA1000M),
- URTWN_DEV_8192CU(NETGEAR, WNA1000Mv2),
+ URTWN_DEV_8192CU(NETGEAR, WNA1000MV2),
URTWN_DEV_8192CU(NETGEAR, RTL8192CU),
URTWN_DEV_8192CU(NETGEAR4, RTL8188CU),
URTWN_DEV_8192CU(NETWEEN, RTL8192CU),
Index: umsm.c
===
RCS file: /cvs/src/sys/dev/usb/umsm.c,v
retrieving revision 1.114
diff -u -p -r1.114 umsm.c
--- umsm.c 15 Aug 2018 14:13:07 - 1.114
+++ umsm.c 4 Jan 2020 04:22:00 -
@@ -139,7 +139,7 @@ static const struct umsm_type umsm_devs[
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E618 }, DEV_HUAWEI},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_E392_INIT }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_EM770W }, 0},
- {{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_Mobile }, DEV_HUAWEI},
+ {{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_MOBILE }, DEV_HUAWEI},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_K3765_INIT }, DEV_UMASS5},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_K3765 }, 0},
{{ USB_VENDOR_HUAWEI, USB_PRODUCT_HUAWEI_K3772_INIT }, DEV_UMASS5},
Index: usb_quirks.c
===
RCS file: /cvs/src/sys/dev/usb/usb_quirks.c,v
retrieving revision 1.75
diff -u -p -r1.75 usb_quirks.c
--- usb_quirks.c10 Jul 2018 09:46:18 - 1.75
+++ usb_quirks.c4 Jan 2020 04:22:22 -
@@ -86,7 +86,7 @@ const struct usbd_quirk_entry {
ANY, { UQ_ASSUME_CM_OVER_DATA }},
{ USB_VENDOR_CMOTECH, USB_PRODUCT_CMOTECH_CCU550,
ANY, { UQ_ASSUME_CM_OVER_DATA }},
- { USB_VENDOR_CMOTECH, USB_PRODUCT_CMOTECH_CNU550pro,
+ { USB_VENDOR_CMOTECH, USB_PRODUCT_CMOTECH_CNU550PRO,
ANY, { UQ_ASSUME_CM_OVER_DATA }},
{ USB_VENDOR_SIEMENS2, USB_PRODUCT_SIEMENS2_ES75,
ANY, { UQ_ASSUME_CM_OVER_DATA }},
@@ -126,8 +126,8 @@ const struct usbd_quirk_entry {
{ USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM_OLD, ANY,{ UQ_BAD_HID }},
{ USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM, ANY,{ UQ_BAD_HID }},
{ USB_VENDOR_CYPRESS, USB_PRODUCT_CYPRESS_SISPM_FLASH,ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_MICROCHIP, USB_PRODUCT_MICROCHIP_USBLCD20x2, ANY,{
UQ_BAD_HID }},
- { USB_VENDOR_MICROCHIP, USB_PRODUCT_MICROCHIP_USBLCD256x64, ANY,{
UQ_BAD_HID }},
+ { USB_VENDOR_MICROCHIP, USB_PRODUCT_MICROCHIP_USBLCD20X2, ANY,{
UQ_BAD_HID }},
+ { USB_VENDOR_MICROCHIP, USB_PRODUCT_MICROCHIP_USBLCD256X64, ANY,{
UQ_BAD_HID }},
{ USB_VENDOR_MECANIQUE, USB_PRODUCT_MECANIQUE_WISPY, ANY,{ UQ_BAD_HID }},
{ USB_VENDOR_METAGEEK, USB_PRODUCT_METAGEEK_WISPY24I, ANY,{ UQ_BAD_HID }},
{ USB_VENDOR_MUSTEK2, USB_PRODUCT_MUSTEK2_PM800, ANY,{ UQ_BAD_HID }},
Index: usbdevs
===
RCS file: /cvs/src/sys/dev/usb/usbdevs,v
retrieving revision 1.702
diff -u -p -r1.702 usbdevs
--- usbdevs 7 Dec 2019 08:45:28 - 1.702
+++ usbdevs 4 Jan 2020 04:20:20 -
@@ -1344,7 +1344,7 @@ product CLIPSAL 5800PC0x0301 5800PC C-
product CLIPSAL 5500PCU0x0303 5500PCU C-Bus
product CLIPSAL 5000CT20x0304 5000CT2 C-Bus Touch Screen
product CLIPSAL C5000CT2 0x0305 C5000CT2 C-Bus Touch Screen
-product CLIPSAL L51xx 0x0401 L51xx C-Bus Dimmer
+product CLIPSAL L51XX 0x0401 L51xx C-Bus Dimmer
/* Compaq products */
product COMPAQ IPAQPOCKETPC0x0003 iPAQ PocketPC
@@ -1362,7 +1362,7 @@ product CTC CW66220x6622 CW6622
product CMOTECH CNU510 0x5141 CDMA Technologies USB modem
product CMOTECH CM5100P0x5523 CM-5100P EVDO
product CMOTECH CCU550 0x5533 CCU-550 EVDO
-product CMOTECH CNU550pro 0x5543 CNU-550pro EVDO
+product CMOTECH CNU550PRO 0x5543 CNU-550pro EVDO
product CMOTECH CGU628 0x6006 CGU-628
product CMOTECH CNU680 0x6803 CNU-680
product CMOTECH CGU628_DISK0xf000 CGU-628 disk mode
@@ -2163,7 +2163,7 @@ product HP KBDHUB 0x010c Multimedia Key
product HP HN210W 0x011c HN210W
product HP HPX9GP 0x0121 HP-x9G+
product HP 6200C 0x0201 ScanJet 6200C
-product HP S20b0x0202 PhotoSmart S20
+product HP S20B0x0202 PhotoSmart S20
product HP 815C
uppercase pcidevs product defines
Consistently uppercase pci product defines.
does not address IBM oddities like
product IBM 0x0002 0x0002 MCA
minor changes to avoid duplicate defines or unclear names
(IIi -> 2I instead of III).
Index: arch/i386/pci/geodesc.c
===
RCS file: /cvs/src/sys/arch/i386/pci/geodesc.c,v
retrieving revision 1.13
diff -u -p -r1.13 geodesc.c
--- arch/i386/pci/geodesc.c 10 Dec 2014 12:27:56 - 1.13
+++ arch/i386/pci/geodesc.c 4 Jan 2020 03:50:20 -
@@ -75,7 +75,7 @@ geodesc_match(struct device *parent, voi
if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_NS &&
(PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_NS_SC1100_XBUS ||
-PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_NS_SCx200_XBUS))
+PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_NS_SCX200_XBUS))
return (1);
return (0);
}
Index: dev/cardbus/if_fxp_cardbus.c
===
RCS file: /cvs/src/sys/dev/cardbus/if_fxp_cardbus.c,v
retrieving revision 1.36
diff -u -p -r1.36 if_fxp_cardbus.c
--- dev/cardbus/if_fxp_cardbus.c24 Nov 2015 17:11:39 - 1.36
+++ dev/cardbus/if_fxp_cardbus.c4 Jan 2020 03:50:22 -
@@ -93,7 +93,7 @@ struct cfattach fxp_cardbus_ca = {
};
const struct pci_matchid fxp_cardbus_devices[] = {
- { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_8255x },
+ { PCI_VENDOR_INTEL, PCI_PRODUCT_INTEL_8255X },
};
#ifdef CBB_DEBUG
Index: dev/pci/ciss_pci.c
===
RCS file: /cvs/src/sys/dev/pci/ciss_pci.c,v
retrieving revision 1.20
diff -u -p -r1.20 ciss_pci.c
--- dev/pci/ciss_pci.c 20 Oct 2014 19:19:20 - 1.20
+++ dev/pci/ciss_pci.c 4 Jan 2020 03:50:22 -
@@ -51,9 +51,9 @@ const struct pci_matchid ciss_pci_device
{ PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5300 },
{ PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5300_2 },
{ PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5312 },
- { PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5i },
- { PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5i_2 },
- { PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA6i },
+ { PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5I },
+ { PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA5I_2 },
+ { PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA6I },
{ PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA641 },
{ PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA642 },
{ PCI_VENDOR_COMPAQ,PCI_PRODUCT_COMPAQ_CSA6400 },
@@ -152,7 +152,7 @@ ciss_pci_attach(struct device *parent, s
sc->iem = CISS_READYENA;
reg = pci_conf_read(pa->pa_pc, pa->pa_tag, PCI_SUBSYS_ID_REG);
if (PCI_VENDOR(reg) == PCI_VENDOR_COMPAQ &&
- (PCI_PRODUCT(reg) == PCI_PRODUCT_COMPAQ_CSA5i ||
+ (PCI_PRODUCT(reg) == PCI_PRODUCT_COMPAQ_CSA5I ||
PCI_PRODUCT(reg) == PCI_PRODUCT_COMPAQ_CSA532 ||
PCI_PRODUCT(reg) == PCI_PRODUCT_COMPAQ_CSA5312))
sc->iem = CISS_READYENAB;
Index: dev/pci/envy.c
===
RCS file: /cvs/src/sys/dev/pci/envy.c,v
retrieving revision 1.80
diff -u -p -r1.80 envy.c
--- dev/pci/envy.c 23 Nov 2019 17:22:10 - 1.80
+++ dev/pci/envy.c 4 Jan 2020 03:50:22 -
@@ -217,7 +217,7 @@ struct midi_hw_if envy_midi_hw_if = {
struct pci_matchid envy_matchids[] = {
{ PCI_VENDOR_ICENSEMBLE, PCI_PRODUCT_ICENSEMBLE_ICE1712 },
- { PCI_VENDOR_ICENSEMBLE, PCI_PRODUCT_ICENSEMBLE_VT172x }
+ { PCI_VENDOR_ICENSEMBLE, PCI_PRODUCT_ICENSEMBLE_VT172X }
};
/*
@@ -1723,7 +1723,7 @@ envyattach(struct device *parent, struct
sc->ibuf.addr = sc->obuf.addr = NULL;
sc->ccs_iosz = 0;
sc->mt_iosz = 0;
- sc->isht = (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_ICENSEMBLE_VT172x);
+ sc->isht = (PCI_PRODUCT(pa->pa_id) == PCI_PRODUCT_ICENSEMBLE_VT172X);
if (pci_mapreg_map(pa, ENVY_CTL_BAR, PCI_MAPREG_TYPE_IO, 0,
>ccs_iot, >ccs_ioh, NULL, >ccs_iosz, 0)) {
Index: dev/pci/gdt_pci.c
===
RCS file: /cvs/src/sys/dev/pci/gdt_pci.c,v
retrieving revision 1.25
diff -u -p -r1.25 gdt_pci.c
--- dev/pci/gdt_pci.c 14 Mar 2015 03:38:48 - 1.25
+++ dev/pci/gdt_pci.c 4 Jan 2020 03:50:22 -
@@ -191,52 +191,52 @@ gdt_pci_attach(struct device *parent, st
if (PCI_VENDOR(pa->pa_id) == PCI_VENDOR_VORTEX) {
prod = PCI_PRODUCT(pa->pa_id);
switch (prod) {
- case PCI_PRODUCT_VORTEX_GDT_60x0:
+ case PCI_PRODUCT_VORTEX_GDT_60X0:
case PCI_PRODUCT_VORTEX_GDT_6000B:
sc->sc_class = GDT_PCI;
break;
- case PCI_PRODUCT_VORTEX_GDT_6x10:
- case PCI_PRODUCT_VORTEX_GDT_6x20:
+
Re: FAQ4.html Commit Revert
Diogo Galvao writes:
> On Fri, Jan 3, 2020 at 6:37 PM Anthony J. Bentley wrote:
> >
> > What browser are you using that misrenders the page like that?
>
> Firefox 71.0. It must have something to do with monospace font
> rendering on Windows.
Thanks. These details were necessary to reproduce the problem.
> Even if there was no problem, the current style dt { float: left; }
> isn't very solid for the expected behavior. This becomes clear if we
> manually introduce line breaks inside one of those tags. This is
> not a real example in this case, of course, but it shows how an element
> could be pushed beside a previous floated element. If it's desired for
> an element to be moved below another floating element that precedes it,
> the clear CSS property must be set.
Yes, we should avoid brittleness. But rather than use several display,
float, clear, and content properties to continue styling definition
lists the way we have been, I think replacing with a simple table will
be more appropriate. In general, www should be editable by mere mortals.
Re: FAQ4.html Commit Revert
On Fri, Jan 3, 2020 at 6:37 PM Anthony J. Bentley wrote:
>
> What browser are you using that misrenders the page like that?
Firefox 71.0. It must have something to do with monospace font
rendering on Windows. If you want to simulate the problem, try
adding the CSS below:
dt { height: 18.5px; }
dd { height: 18px; }
Those are the heights computed by Firefox on this machine.
Even if there was no problem, the current style dt { float: left; }
isn't very solid for the expected behavior. This becomes clear if we
manually introduce line breaks inside one of those tags. This is
not a real example in this case, of course, but it shows how an element
could be pushed beside a previous floated element. If it's desired for
an element to be moved below another floating element that precedes it,
the clear CSS property must be set.
And for now this is probably more bikeshedding CSS than tech@ is
willing to waste time with. But I hope it helps.
Re: FAQ4.html Commit Revert
Diogo Galvao writes: > On Thu, Jan 2, 2020 at 12:46 PM Oleg Pahl wrote: > > > > could you be so kind to revert this commit in FAQ 4? > > > > https://cvsweb.openbsd.org/cgi-bin/cvsweb/www/faq/faq4.html.diff?r1=1.495 > 2=1.496 > > > > Instead of reverting the commit, this change in CSS fixes the problem: What browser are you using that misrenders the page like that?
Re: ldomctl: Omit tty device from status output
On Fri, Jan 03, 2020 at 08:58:16PM +, Stuart Henderson wrote: > On 2019/12/30 01:13, Klemens Nanni wrote: > > Now that `ldomctl console ...' is implemented there is actually no need > > to print the device any longer, it is an implementation detail that > > should be hidden just like it is the case with vmctl. > > But vmctl does show the device ..? Sloppiness on my side, sorry. kettenis presented his use case and you proved my argument for consistency to be invalid, so I have no plans for dropping the TTY column any longer. Thanks everyone :-)
Re: ldomctl: Omit tty device from status output
On 2019/12/30 01:13, Klemens Nanni wrote: > Now that `ldomctl console ...' is implemented there is actually no need > to print the device any longer, it is an implementation detail that > should be hidden just like it is the case with vmctl. But vmctl does show the device ..? $ vmctl start open vmctl: started vm 1 successfully, tty /dev/ttyp9 $ vmctl stat ID PID VCPUS MAXMEM CURMEM TTYOWNERSTATE NAME 1 77553 11.0G1.1M ttyp9sthen running open 2 - 11.0G - -sthen stopped cd
Re: ldomctl: init-system: Add -n (noaction) switch for validation only
On Fri, Jan 03, 2020 at 09:46:29PM +0100, Klemens Nanni wrote: > +Only check the configuration file for validty. "validity" fixed in my tree.
ldomctl: init-system: Add -n (noaction) switch for validation only
With the hv_config() now in, `ldomctl init-system -n ldom.conf' to only
parse configuration is trivial.
It is usable as unprivileged user, no devices are touched.
If errors occur, errors will be generated and ldomctl exits; if all is
valid, this prints "configuration OK" just like vmd(8) does.
I have plans for additional ldom.conf(5) options and -n greatly aids
development, but I also prefer to (double) check configs before loading
them as root in general.
Feedback? OK?
? default.diff
? disk.diff
? download.diff
? msg
? n.diff
? owner.diff
? parse.y.new
Index: config.c
===
RCS file: /cvs/src/usr.sbin/ldomctl/config.c,v
retrieving revision 1.29
diff -u -p -r1.29 config.c
--- config.c28 Nov 2019 18:40:42 - 1.29
+++ config.c3 Jan 2020 20:45:33 -
@@ -2759,7 +2759,7 @@ primary_init(void)
}
void
-build_config(const char *filename)
+build_config(const char *filename, int noaction)
{
struct guest *primary;
struct guest *guest;
@@ -2781,6 +2781,10 @@ build_config(const char *filename)
SIMPLEQ_INIT(_list);
if (parse_config(filename, ) < 0)
exit(1);
+ if (noaction) {
+ fprintf(stderr, "configuration OK\n");
+ exit(0);
+ }
pri = md_read("pri");
if (pri == NULL)
Index: ldomctl.8
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
retrieving revision 1.21
diff -u -p -r1.21 ldomctl.8
--- ldomctl.8 30 Dec 2019 20:10:48 - 1.21
+++ ldomctl.8 3 Jan 2020 20:45:33 -
@@ -63,12 +63,17 @@ The download is aborted if a configurati
.It Cm dump
Dump the current configuration from non-volatile storage into the current
working directory.
-.It Cm init-system Ar file
+.It Cm init-system Oo Fl n Oc Ar file
Generate files in the current working directory for a logical domain
configuration
.Ar file
as described in
.Xr ldom.conf 5 .
+.Bl -tag -width Fl
+.It Fl n
+Configtest mode.
+Only check the configuration file for validty.
+.El
.It Cm list
List configurations stored in non-volatile storage.
Indicate the currently running configuration,
Index: ldomctl.c
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
retrieving revision 1.32
diff -u -p -r1.32 ldomctl.c
--- ldomctl.c 3 Jan 2020 19:45:51 - 1.32
+++ ldomctl.c 3 Jan 2020 20:45:34 -
@@ -129,7 +129,7 @@ usage(void)
fprintf(stderr, "usage:\t%1$s delete|select configuration\n"
"\t%1$s download directory\n"
"\t%1$s dump|list|list-io\n"
- "\t%1$s init-system file\n"
+ "\t%1$s init-system [-n] file\n"
"\t%1$s create-vdisk -s size file\n"
"\t%1$s console|panic|start|status|stop [domain]\n", getprogname());
exit(EXIT_FAILURE);
@@ -241,12 +241,27 @@ dump(int argc, char **argv)
void
init_system(int argc, char **argv)
{
- if (argc != 2)
+ int ch, noaction = 0;
+
+ while ((ch = getopt(argc, argv, "n")) != -1) {
+ switch (ch) {
+ case 'n':
+ noaction = 1;
+ break;
+ default:
+ usage();
+ }
+ }
+ argc -= optind;
+ argv += optind;
+
+ if (argc != 1)
usage();
- hv_config();
+ if (!noaction)
+ hv_config();
- build_config(argv[1]);
+ build_config(argv[0], noaction);
}
void
Index: ldomctl.h
===
RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.h,v
retrieving revision 1.11
diff -u -p -r1.11 ldomctl.h
--- ldomctl.h 28 Nov 2019 18:03:33 - 1.11
+++ ldomctl.h 3 Jan 2020 20:45:34 -
@@ -193,5 +193,5 @@ struct ldom_config {
};
int parse_config(const char *, struct ldom_config *);
-void build_config(const char *);
+void build_config(const char *, int);
void list_components(void);
Re: ldomctl: Do not start/stop/panic primary domain
On Fri, Jan 03, 2020 at 09:17:58PM +0100, Mark Kettenis wrote: > The command isn't actually as dangerous as you think. You can > continue from the ddb prompt just fine... That assumes one has access to the console. I hope everyone running such setups has, but needing to resort to OOB access for serial when I screw up over SSH and panic the primary is not convenient. > A valid question is whether our kernel running in a domain should > respond to this command. Maybe it should respect ddb.console? Good question. I'd argue `ldomctl panic' is in line with the means by wich ddb may be entered: DBCTL_CONSOLE (ddb.console) When this variable is set, an architecture dependent magic key sequence on the console or a debugger button will permit entry into the kernel debugger. When running with a securelevel(7) greater than 0, this variable may not be raised. Or: Why should ddb.console prevent "magic key", "debugger button" and `sysctl ddb.panic' but allow `ldomctl console'? Perhaps this level of overwrite is desired so that admins can always panic guests regardless of the sysctl? Afterall guests have no influence on the primary and its controlling actions, so why should this differ? Writing this down, I tend to leave it as is; ldomctl *is* a control command and honering guest parameters violates this principle.
Re: ldomctl: Omit tty device from status output
On Fri, Jan 03, 2020 at 09:11:01PM +0100, Mark Kettenis wrote: > The way I see it, "ldomctl console" is just there for compatibility > with vmctl. We don't really need it for sparc64 as the random > allocation of ttys that vmm(4) suffers from doesn't happen (and there > are interesting user permission issues that we discussed a few weeks > ago). I added it to be able to address guests by their names which I know instead of their serial devices which I have to derive from the overall config. Compatibility with vmctl is certainly nice but of less importance. > I continue to use "cu -l ttyV2" as I'm used to it (and it takes less > typing). But occasionally I need to refresh my memory about the tty > assigned to my domains. Fair enough, if that's still helpful in your use case I'll just leave it there.
Re: ldomctl: Do not start/stop/panic primary domain
> Date: Fri, 3 Jan 2020 20:42:11 +0100 > From: Klemens Nanni > > On Fri, Jan 03, 2020 at 08:28:54PM +0100, Mark Kettenis wrote: > > Please no. Stupid sysadmins should stay away from that command ;). > Do you want to scare them away? > > Fine with me, your call. The command isn't actually as dangerous as you think. You can continue from the ddb prompt just fine... A valid question is whether our kernel running in a domain should respond to this command. Maybe it should respect ddb.console?
Re: ldomctl: Omit tty device from status output
> Date: Fri, 3 Jan 2020 20:18:46 +0100 > From: Klemens Nanni > > On Fri, Jan 03, 2020 at 08:04:10PM +0100, Mark Kettenis wrote: > > No. If you messed this up in a previous commit, please fix it some > > other way. > The purpose of this diff is not to fix previously messed up spacing but > to omit information that I consider redundant by now since the `console' > available. > > At e2k19 I added printing the vcctty first, then the console command > followed. In hindsight, printing it could have been skipped right away, > but well... didn't occur to me earlier. > > vmctl does not print the device either, I just don't see a reason to do > it in ldomctl anymore. The way I see it, "ldomctl console" is just there for compatibility with vmctl. We don't really need it for sparc64 as the random allocation of ttys that vmm(4) suffers from doesn't happen (and there are interesting user permission issues that we discussed a few weeks ago). I continue to use "cu -l ttyV2" as I'm used to it (and it takes less typing). But occasionally I need to refresh my memory about the tty assigned to my domains.
Re: ldomctl: Do not start/stop/panic primary domain
As a "Stupid Sysadmin", I want all the help I can get! :-) On Fri, 3 Jan 2020 at 19:56, Klemens Nanni wrote: > On Fri, Jan 03, 2020 at 08:28:54PM +0100, Mark Kettenis wrote: > > Please no. Stupid sysadmins should stay away from that command ;). > Do you want to scare them away? > > Fine with me, your call. > >
Re: Add pcidev for Ryzen 3x ccp
> Date: Wed, 1 Jan 2020 09:53:39 -0500
> From: Todd Mortimer
>
> My CPU has a CCP that isn't in the known list, so add it and tell ccp
> about it.
>
> Tested on Ryzen 3900x.
>
> ok?
ok kettenis@
> Will commit a pcidevs regen immediately after.
>
>
> diff --git a/sys/dev/pci/ccp_pci.c b/sys/dev/pci/ccp_pci.c
> index 2259594644b..c8dcc8750fc 100644
> --- a/sys/dev/pci/ccp_pci.c
> +++ b/sys/dev/pci/ccp_pci.c
> @@ -47,6 +47,7 @@ static const struct pci_matchid ccp_pci_devices[] = {
> { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_CCP_1 },
> { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_CCP_2 },
> { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_1X_CCP },
> + { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_3X_CCP },
> };
>
> int
> diff --git a/sys/dev/pci/pcidevs b/sys/dev/pci/pcidevs
> index d0a021b89bc..fb918e8de5d 100644
> --- a/sys/dev/pci/pcidevs
> +++ b/sys/dev/pci/pcidevs
> @@ -754,6 +754,7 @@ product AMD AMD64_17_CCP_20x1468 AMD64 17h Crypto
> product AMD AMD64_17_PCIE_4 0x1470 AMD64 17h PCIE
> product AMD AMD64_17_PCIE_5 0x1471 AMD64 17h PCIE
> product AMD AMD64_17_3X_RC 0x1480 AMD64 17h/3xh Root Complex
> +product AMD AMD64_17_3X_CCP 0x1486 AMD64 17h/3xh Crypto
> product AMD AMD64_14_HB 0x1510 AMD64 14h Host
> product AMD AMD64_14_PCIE_1 0x1512 AMD64 14h PCIE
> product AMD AMD64_14_PCIE_2 0x1513 AMD64 14h PCIE
>
>
Re: ldomctl: Do not start/stop/panic primary domain
On Fri, Jan 03, 2020 at 08:28:54PM +0100, Mark Kettenis wrote: > Please no. Stupid sysadmins should stay away from that command ;). Do you want to scare them away? Fine with me, your call.
Re: ldomctl: download: select new configuration
On Fri, Jan 03, 2020 at 08:27:21PM +0100, Mark Kettenis wrote: > I can't remember. Maybe someone with a t1k/t2k or t5120/t5140 can > test this? Hopefully; I don't have such machines anymore. > It makes sense for the download command not to immediately select a > configuration. So maybe just the documentation needs changing? I'm totally fine either way as long as documentation and code is in sync. Automatic select upon download makes sense for me since I usually want to run the configuration (right away) I just build, especially in our situation where config changes require machine resets (I think Solaris calls this "delayed configuration"). Manual update below for convenience but I want to hear feedback from users of above mentioned machines before changing this either way. Index: ldomctl.8 === RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v retrieving revision 1.21 diff -u -p -r1.21 ldomctl.8 --- ldomctl.8 30 Dec 2019 20:10:48 - 1.21 +++ ldomctl.8 3 Jan 2020 19:40:36 - @@ -53,7 +53,6 @@ Delete the specified configuration from .It Cm download Ar directory Save a logical domain configuration to non-volatile storage on the service processor. -The configuration will take effect after the machine is reset. The name of the configuration is taken from the name of the .Ar directory which must contain files created with the
Re: ldomctl: Move code into hv_config(), defer to commands needing it
> Date: Tue, 31 Dec 2019 03:26:13 +0100
> From: Klemens Nanni
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> This moves setup code from main() into its own function so instead of
> upfront it can be used only when and where needed.
>
> With the exception of `create-vdisk' all currently open /dev/hvctl; for
> that command I added a rather quirky goto to avoid this unneeded step,
> but `list-io' for example does not need /dev/hvctl at all either.
>
> So instead of adding more quirks, split as per above and clearly call
> hv_config() from the commands that *do* require it.
>
> This also effectively defers such privileged operations after all argv[]
> parsing is done, that is the code fails earlier on invalid input without
> file I/O for nothing.
>
> With that in, I can easily add more commands not requiring hvctl access,
> e.g. a dry-run configuration check.
>
> OK?
ok kettenis@
> Index: ldomctl.c
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 ldomctl.c
> --- ldomctl.c 28 Dec 2019 18:36:02 - 1.31
> +++ ldomctl.c 31 Dec 2019 02:15:14 -
> @@ -83,6 +83,7 @@ struct command commands[] = {
>
> void hv_open(void);
> void hv_close(void);
> +void hv_config(void);
> void hv_read(uint64_t, void *, size_t);
> void hv_write(uint64_t, void *, size_t);
>
> @@ -103,11 +104,6 @@ int
> main(int argc, char **argv)
> {
> struct command *cmdp;
> - struct hvctl_msg msg;
> - ssize_t nbytes;
> - struct md_header hdr;
> - struct md_node *node;
> - struct md_prop *prop;
>
> if (argc < 2)
> usage();
> @@ -122,46 +118,6 @@ main(int argc, char **argv)
> if (cmdp->cmd_name == NULL)
> usage();
>
> - if (strcmp(argv[0], "create-vdisk") == 0)
> - goto skip_hv;
> -
> - hv_open();
> -
> - /*
> - * Request config.
> - */
> - bzero(, sizeof(msg));
> - msg.hdr.op = HVCTL_OP_GET_HVCONFIG;
> - msg.hdr.seq = hvctl_seq++;
> - nbytes = write(hvctl_fd, , sizeof(msg));
> - if (nbytes != sizeof(msg))
> - err(1, "write");
> -
> - bzero(, sizeof(msg));
> - nbytes = read(hvctl_fd, , sizeof(msg));
> - if (nbytes != sizeof(msg))
> - err(1, "read");
> -
> - hv_membase = msg.msg.hvcnf.hv_membase;
> - hv_memsize = msg.msg.hvcnf.hv_memsize;
> -
> - hv_mdpa = msg.msg.hvcnf.hvmdp;
> - hv_read(hv_mdpa, , sizeof(hdr));
> - hvmd_len = sizeof(hdr) + hdr.node_blk_sz + hdr.name_blk_sz +
> - hdr.data_blk_sz;
> - hvmd_buf = xmalloc(hvmd_len);
> - hv_read(hv_mdpa, hvmd_buf, hvmd_len);
> -
> - hvmd = md_ingest(hvmd_buf, hvmd_len);
> - node = md_find_node(hvmd, "guests");
> - TAILQ_INIT(_list);
> - TAILQ_FOREACH(prop, >prop_list, link) {
> - if (prop->tag == MD_PROP_ARC &&
> - strcmp(prop->name->str, "fwd") == 0)
> - add_guest(prop->d.arc.node);
> - }
> -
> -skip_hv:
> (cmdp->cmd_func)(argc, argv);
>
> exit(EXIT_SUCCESS);
> @@ -288,6 +244,8 @@ init_system(int argc, char **argv)
> if (argc != 2)
> usage();
>
> + hv_config();
> +
> build_config(argv[1]);
> }
>
> @@ -300,6 +258,8 @@ list(int argc, char **argv)
> if (argc != 1)
> usage();
>
> + hv_config();
> +
> dc = ds_conn_open("/dev/spds", NULL);
> mdstore_register(dc);
> while (TAILQ_EMPTY(_sets))
> @@ -332,6 +292,8 @@ xselect(int argc, char **argv)
> if (argc != 2)
> usage();
>
> + hv_config();
> +
> dc = ds_conn_open("/dev/spds", NULL);
> mdstore_register(dc);
> while (TAILQ_EMPTY(_sets))
> @@ -351,6 +313,8 @@ delete(int argc, char **argv)
> if (strcmp(argv[1], "factory-default") == 0)
> errx(1, "\"%s\" should not be deleted", argv[1]);
>
> + hv_config();
> +
> dc = ds_conn_open("/dev/spds", NULL);
> mdstore_register(dc);
> while (TAILQ_EMPTY(_sets))
> @@ -409,6 +373,8 @@ download(int argc, char **argv)
> if (argc != 2)
> usage();
>
> + hv_config();
> +
> dc = ds_conn_open("/dev/spds", NULL);
> mdstore_register(dc);
> while (TAILQ_EMPTY(_sets))
> @@ -426,6 +392,8 @@ guest_start(int argc, char **argv)
> if (argc != 2)
> usage();
>
> + hv_config();
> +
> /*
>* Start guest domain.
>*/
> @@ -452,6 +420,8 @@ guest_stop(int argc, char **argv)
> if (argc != 2)
> usage();
>
> + hv_config();
> +
> /*
>* Stop guest domain.
>*/
> @@ -478,6 +448,8 @@ guest_panic(int argc, char **argv)
> if (argc != 2)
> usage();
>
> + hv_config();
> +
> /*
>* Stop guest domain.
>*/
> @@ -513,6 +485,9 @@ guest_status(int argc, char **argv)
>
>
Re: sparc64: find root device on hardware RAID
> Date: Tue, 31 Dec 2019 09:12:56 +1000
> From: Jonathan Matthew
> Content-Type: text/plain; charset=us-ascii
> Content-Disposition: inline
>
> On Mon, Dec 30, 2019 at 03:36:54PM +0100, Klemens Nanni wrote:
> > On Mon, Dec 30, 2019 at 06:59:35PM +1000, Jonathan Matthew wrote:
> > > Here's the Solaris explanation:
> > >
> > > https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/sys/scsi/adapters/mpt_sas/mptsas_var.h#L195
> > Thanks for digging.
> >
> > > I think we should copy what they're doing here, that is, replace the high
> > > bits
> > > with 3 rather than adding 3 to it. I'm really not sure where we should do
> > > this though. Maybe in mpii, but only on sparc64?
> > As that is now a general quirk in all RAID volumes and no longer
> > specific to bootpath handling, mpii seems only appropiate and autoconf
> > must not know about this.
> >
> > Since Illumos does exactly that with mptsas_get_raid_wwid() in
> > mptsas_raidconf_page_0_cb(), which after a brief look seems like the
> > code path equivalent to our recent WWID addition in mpii_scsi_probe(),
> > I'm inclined to just do it there.
> >
> > Diff below doas that.
> >
> > > As far as I can tell, the raid controller generates 128 bit WWIDs for raid
> > > volumes, but only has a 64 bit field to report the ID to the host, so it
> > > only
> > > puts the vendor-specified part here (you can see the last half of the ID
> > > string
> > > printed when sd0 attaches matches sl->port_wwn in reverse). I think the
> > > purpose of setting the high 4 bits to 3 is that 3 is not a defined NAA
> > > value,
> > > so you're not going to find a proper WWID coming from other device that
> > > will
> > > match that.
> > I did not manage to recognise this detail of the reversed ID, indeed:
> >
> > sd0 at scsibus1 targ 0 lun 0:
> > naa.600508e06cd1dcd59022a30a
> >
> > Feedback? OK?
>
> I think this is probably the most sensible thing we can do here.
> ok jmatthew@ but I'd wait and see if anyone has a better idea.
Can we leave out the #ifdef __sparc64__? Unless somebody can come up
with a really good reason for it...
> > Index: dev/pci/mpii.c
> > ===
> > RCS file: /cvs/src/sys/dev/pci/mpii.c,v
> > retrieving revision 1.123
> > diff -u -p -r1.123 mpii.c
> > --- dev/pci/mpii.c 29 Dec 2019 21:30:21 - 1.123
> > +++ dev/pci/mpii.c 30 Dec 2019 14:26:57 -
> > @@ -930,6 +930,15 @@ mpii_scsi_probe(struct scsi_link *link)
> > return (EINVAL);
> >
> > link->port_wwn = letoh64(vpg.wwid);
> > +#ifdef __sparc64__
> > + /*
> > +* WWIDs generated by LSI firmware are not IEEE NAA compliant
> > +* so historical practise in OBP is to set the top nibble to 3
> > +* to indicate that this is a RAID volume.
> > +*/
> > + link->port_wwn &= 0x0fff;
> > + link->port_wwn |= 0x3000;
> > +#endif
> >
> > return (0);
> > }
> > Index: arch/sparc64/sparc64/autoconf.c
> > ===
> > RCS file: /cvs/src/sys/arch/sparc64/sparc64/autoconf.c,v
> > retrieving revision 1.133
> > diff -u -p -r1.133 autoconf.c
> > --- arch/sparc64/sparc64/autoconf.c 15 Oct 2019 05:21:16 - 1.133
> > +++ arch/sparc64/sparc64/autoconf.c 30 Dec 2019 14:27:17 -
> > @@ -1455,7 +1455,7 @@ device_register(struct device *dev, void
> > u_int lun = bp->val[1];
> >
> > if (bp->val[0] & 0x && bp->val[0] != -1) {
> > - /* Fibre channel? */
> > + /* Hardware RAID or Fibre channel? */
> > if (bp->val[0] == sl->port_wwn && lun == sl->lun) {
> > nail_bootdev(dev, bp);
> > }
>
>
Re: ldomctl: Do not start/stop/panic primary domain
> Date: Mon, 30 Dec 2019 22:14:20 +0100 > From: Klemens Nanni > > `ldomctl start|stop primary" silently exits zero not doing anything, > `ldomctl panic primary" makes primary panic (and pulls guests down with > it). > > The manual explicitly documents those commands for "guest" domains, > code comments say so as well, so lets behave accordingly and prevent > stupid admins like me from testing `ldomctl panic primary' on their > machine before looking at the code. > > Feedback on better error message wording? > OK? Please no. Stupid sysadmins should stay away from that command ;). > Index: ldomctl.c > === > RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v > retrieving revision 1.31 > diff -u -p -r1.31 ldomctl.c > --- ldomctl.c 28 Dec 2019 18:36:02 - 1.31 > +++ ldomctl.c 30 Dec 2019 21:10:39 - > @@ -426,6 +426,9 @@ guest_start(int argc, char **argv) > if (argc != 2) > usage(); > > + if (strcmp(argv[1], "primary") == 0) > + errx(1, "cannot start primary domain"); > + > /* >* Start guest domain. >*/ > @@ -452,6 +455,9 @@ guest_stop(int argc, char **argv) > if (argc != 2) > usage(); > > + if (strcmp(argv[1], "primary") == 0) > + errx(1, "cannot stop primary domain"); > + > /* >* Stop guest domain. >*/ > @@ -477,6 +483,9 @@ guest_panic(int argc, char **argv) > > if (argc != 2) > usage(); > + > + if (strcmp(argv[1], "primary") == 0) > + errx(1, "cannot panic primary domain"); > > /* >* Stop guest domain. > >
sparc64: simplify boot()
Although Open Firmware supports it, there is no way from OpenBSD to
reboot with a specified boot command line, so drop vestigial support for
it from boot().
Index: sparc64/machdep.c
===
RCS file: /OpenBSD/src/sys/arch/sparc64/sparc64/machdep.c,v
retrieving revision 1.191
diff -u -p -r1.191 machdep.c
--- sparc64/machdep.c 1 Apr 2019 07:00:52 - 1.191
+++ sparc64/machdep.c 3 Jan 2020 19:24:45 -
@@ -593,9 +593,6 @@ struct pcb dumppcb;
__dead void
boot(int howto)
{
- int i;
- static char str[128];
-
if ((howto & RB_RESET) != 0)
goto doreset;
@@ -655,30 +652,7 @@ haltsys:
doreset:
printf("rebooting\n\n");
-#if 0
- if (user_boot_string && *user_boot_string) {
- i = strlen(user_boot_string);
- if (i > sizeof(str))
- OF_boot(user_boot_string); /* XXX */
- bcopy(user_boot_string, str, i);
- } else
-#endif
- {
- i = 1;
- str[0] = '\0';
- }
-
- if ((howto & RB_SINGLE) != 0)
- str[i++] = 's';
- if ((howto & RB_KDB) != 0)
- str[i++] = 'd';
- if (i > 1) {
- if (str[0] == '\0')
- str[0] = '-';
- str[i] = 0;
- } else
- str[0] = 0;
- OF_boot(str);
+ OF_boot("");
panic("cpu_reboot -- failed");
for (;;)
continue;
Re: ldomctl: download: select new configuration
> Date: Mon, 30 Dec 2019 21:07:59 +0100 > From: Klemens Nanni > > The example in the manual implies that the download command also selects > it: > > # ldomctl init-system ldom.conf > # cd .. > # ldomctl delete openbsd > # ldomctl download openbsd > # ldomctl list > factory-default [current] > openbsd [next] > > But `ldomctl select openbsd' is required between downloading and listing > to get this result - at least on my T4 machine `download' never selected > any configuration, however I vaguely remember that this was the case > with older machines. > > kettenis: Has this really been missing all the time or could there be > differences in the mdstore protocol and/or firmware that cause this? > > Diff below explicitly selects configuration in code. > > Feedback? OK? I can't remember. Maybe someone with a t1k/t2k or t5120/t5140 can test this? It makes sense for the download command not to immediately select a configuration. So maybe just the documentation needs changing? > Index: ldomctl.c > === > RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v > retrieving revision 1.31 > diff -u -p -r1.31 ldomctl.c > --- ldomctl.c 28 Dec 2019 18:36:02 - 1.31 > +++ ldomctl.c 30 Dec 2019 19:51:59 - > @@ -415,6 +415,7 @@ download(int argc, char **argv) > ds_conn_handle(dc); > > mdstore_download(dc, argv[1]); > + mdstore_select(dc, argv[1]); > } > > void > >
Re: ldomctl: Omit tty device from status output
On Fri, Jan 03, 2020 at 08:04:10PM +0100, Mark Kettenis wrote: > No. If you messed this up in a previous commit, please fix it some > other way. The purpose of this diff is not to fix previously messed up spacing but to omit information that I consider redundant by now since the `console' available. At e2k19 I added printing the vcctty first, then the console command followed. In hindsight, printing it could have been skipped right away, but well... didn't occur to me earlier. vmctl does not print the device either, I just don't see a reason to do it in ldomctl anymore.
Re: sparc64: find root device on hardware RAID
> Date: Mon, 30 Dec 2019 18:59:35 +1000
> From: Jonathan Matthew
>
> On Sun, Dec 29, 2019 at 05:58:02AM +0100, Klemens Nanni wrote:
> > On Sun, Dec 29, 2019 at 01:59:38PM +1000, Jonathan Matthew wrote:
> > > I think we have the wrong size for the volume name, hence the difference
> > > between the size reported by the controller and the size of vpg.
> > Indeed, good catch!
> >
> > OBP's `create-raid*-volume' commands also prompt for names no longer
> > than that:
> >
> > {0} ok 9 b c d create-raid0-volume
> > ...
> > Enter a volume name: [0 to 15 characters] foo
> > {0} ok
> >
> > > try this out?
> > Just works, the WWID is no longer clobbered and autoconf eventually sees
> > it in the port WWN:
> >
> > mpii0 at pci15 dev 0 function 0 "Symbios Logic SAS2008" rev 0x03: msi
> > mpii0: Solana On-Board, firmware 9.0.0.0 IR, MPI 2.0
> > scsibus1 at mpii0: 834 targets
> > mpii_scsi_probe: target 0 lun 0 port_wwn 0 node_wwn 0 has
> > MPII_DF_VOLUME set in flags 10
> > struct mpii_cfg_raid_vol_pg1 vpg:
> > volume_id: 81, tvolume_bus: 3, volume_ioc: 0
> > wwid: aa32290d5dcd16c
> > sd0 at scsibus1 targ 0 lun 0:
> > naa.600508e06cd1dcd59022a30a
> > device_register: RAID:
> > bp->val[]: 3aa32290d5dcd16c, 0, 0
> > target: d5dcd16c, sl->target: 0
> > lun: 0, sl->lun: 0
> > sl->port_wwn: aa32290d5dcd16c, sl->node_wwn: 0
> >
> > sd0: 713824MB, 512 bytes/sector, 1461911552 sectors
> >
> > Thanks a lot,
> > OK kn
> >
> > Now I need to work around the first digit's mismatch; for reasons still
> > unknown to me, official documentation states that the RAID volume WWID's
> > first digit --if it is zero-- must be replaced with three, so the
> > bootpath contains 3aa32290d5dcd16c whereas the port WWN has the correct
> > aa32290d5dcd16c.
>
> Here's the Solaris explanation:
>
> https://github.com/illumos/illumos-gate/blob/master/usr/src/uts/common/sys/scsi/adapters/mpt_sas/mptsas_var.h#L195
>
> I think we should copy what they're doing here, that is, replace the high bits
> with 3 rather than adding 3 to it. I'm really not sure where we should do
> this though. Maybe in mpii, but only on sparc64?
>
> As far as I can tell, the raid controller generates 128 bit WWIDs for raid
> volumes, but only has a 64 bit field to report the ID to the host, so it only
> puts the vendor-specified part here (you can see the last half of the ID
> string
> printed when sd0 attaches matches sl->port_wwn in reverse). I think the
> purpose of setting the high 4 bits to 3 is that 3 is not a defined NAA value,
> so you're not going to find a proper WWID coming from other device that will
> match that.
Can you think of a reason why we would care about the exact WWIDs for
these RAID volumes on other architectures? If not, I'd prefer to
"fix" this in mpii(4) since this sounds like a deficiency in the LSI
firmware.
Re: ldomctl: Omit tty device from status output
> Date: Mon, 30 Dec 2019 01:13:56 +0100
> From: Klemens Nanni
>
> Now that `ldomctl console ...' is implemented there is actually no need
> to print the device any longer, it is an implementation detail that
> should be hidden just like it is the case with vmctl.
>
> That also makes it fit nicely on serial consoles again.
>
> $ doas ldomctl status primary
> primary -running OpenBSD running
> 1%
> $ jot -s '' -b . 72
>
> $ doas obj/ldomctl status primary
> primary running OpenBSD running0%
>
> OK?
No. If you messed this up in a previous commit, please fix it some
other way.
> Index: ldomctl.8
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.8,v
> retrieving revision 1.20
> diff -u -p -r1.20 ldomctl.8
> --- ldomctl.8 6 Dec 2019 23:01:03 - 1.20
> +++ ldomctl.8 30 Dec 2019 00:13:06 -
> @@ -162,9 +162,9 @@ The primary domain should have less CPUs
> are now assigned to the guest domains:
> .Bd -literal -offset indent
> # ldomctl status
> -primary - running OpenBSD running1%
> -puffy ttyV0 running OpenBoot Primary Boot Loader 8%
> -salmah ttyV1 running OpenBoot Primary Boot Loader 12%
> +primary running OpenBSD running1%
> +puffyrunning OpenBoot Primary Boot Loader 8%
> +salmah running OpenBoot Primary Boot Loader 12%
> .Ed
> .Pp
> Configure the
> Index: ldomctl.c
> ===
> RCS file: /cvs/src/usr.sbin/ldomctl/ldomctl.c,v
> retrieving revision 1.31
> diff -u -p -r1.31 ldomctl.c
> --- ldomctl.c 28 Dec 2019 18:36:02 - 1.31
> +++ ldomctl.c 30 Dec 2019 00:12:02 -
> @@ -509,7 +509,6 @@ guest_status(int argc, char **argv)
> double utilisation = 0.0;
> const char *state_str;
> char buf[32];
> - char console_str[8] = "-";
>
> if (argc < 1 || argc > 2)
> usage();
> @@ -610,14 +609,8 @@ guest_status(int argc, char **argv)
> break;
> }
>
> - /* primary has no console */
> - if (guest->gid != 0) {
> - snprintf(console_str, sizeof(console_str),
> - "ttyV%llu", guest->gid - 1);
> - }
> -
> - printf("%-16s %-8s %-16s %-32s %3.0f%%\n", guest->name,
> - console_str, state_str, state.state == GUEST_STATE_NORMAL ?
> + printf("%-16s %-16s %-32s %3.0f%%\n", guest->name,
> + state_str, state.state == GUEST_STATE_NORMAL ?
> softstate.soft_state_str : "-", utilisation);
> }
> }
>
>
Re: FAQ4.html Commit Revert
On Thu, Jan 2, 2020 at 12:46 PM Oleg Pahl wrote:
>
> could you be so kind to revert this commit in FAQ 4?
>
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/www/faq/faq4.html.diff?r1=1.495=1.496
>
Instead of reverting the commit, this change in CSS fixes the problem:
- dt { margin-left: 40px; float: left; }
+ dt { margin-left: 40px; float: left; clear: left; }
However, depending on fonts and font sizes, rows on the second column
may still get slightly misaligned with respect to the first column.
Fixing this would require one more workaround, if you're so inclined:
+ dd:after { content: ""; display: block; clear: both; }
sparc64: kill BOOT_ARG
BOOT_ARG from is a NetBSDism which did not make
its way to OpenBSD, where parsing kernel commandline options is done
inline.
It is no surprise then, that it is only used by the sparc64 boot blocks;
but boot blocks really only care about one option, -a, so there is no
need to check for more letters.
The following diff removes BOOT_ARG and simplifies boot blocks logic.
Moreover, since the kernel will pick its options from Open Firmware and
not from the boot blocks, it makes absolutely no sense for the boot
blocks to try and rebuild a commandline with the various options, so
let's get rid of this as well while changing the boot blocks.
Note that, after this diff is applied, none of the original contents of
remain, and the licence block ought to be updated.
This is left as an exercize for the appropriate developer.
Index: include/boot_flag.h
===
RCS file: /OpenBSD/src/sys/arch/sparc64/include/boot_flag.h,v
retrieving revision 1.5
diff -u -p -r1.5 boot_flag.h
--- include/boot_flag.h 26 Nov 2014 20:06:53 - 1.5
+++ include/boot_flag.h 3 Jan 2020 18:27:05 -
@@ -30,39 +30,6 @@
#ifndef _MACHINE_BOOT_FLAG_H_
#define _MACHINE_BOOT_FLAG_H_
-#include
-
-/*
- * Recognize standard boot arguments. If the flag is known, appropriate
- * value is or'ed to retval, otherwise retval is left intact.
- * Note that not all ports use all flags recognized here. This list is mere
- * concatenation of all non-conflicting standard boot flags. Individual ports
- * might use also other flags (see e.g. alpha).
- */
-#defineBOOT_FLAG(arg, retval) do { \
- switch (arg) { \
- case 'a': /* ask for file name to boot from */ \
- (retval) |= RB_ASKNAME; \
- break; \
- case 'b': /* always halt, never reboot */ \
- (retval) |= RB_HALT;\
- break; \
- case 'c': /* userconf */\
- (retval) |= RB_CONFIG; \
- break; \
- case 'd': /* break into the kernel debugger ASAP (if compiled in) */ \
- (retval) |= RB_KDB; \
- break; \
- case 's': /* boot to single user */ \
- (retval) |= RB_SINGLE; \
- break; \
- default: /* something else, do nothing */ \
- break; \
- } /* switch */ \
- \
- } while (/* CONSTCOND */ 0)
-
-
/* softraid boot information */
#define BOOTSR_UUID_MAX 16
#define BOOTSR_CRYPTO_MAXKEYBYTES 32
Index: stand/ofwboot/boot.c
===
RCS file: /OpenBSD/src/sys/arch/sparc64/stand/ofwboot/boot.c,v
retrieving revision 1.32
diff -u -p -r1.32 boot.c
--- stand/ofwboot/boot.c29 Oct 2019 02:55:52 - 1.32
+++ stand/ofwboot/boot.c3 Jan 2020 18:27:05 -
@@ -53,7 +53,6 @@
#include
#include
#include
-#include
#include
#include
@@ -124,10 +123,18 @@ parseargs(char *str, int *howtop)
while (*cp == ' ')
++cp;
}
+ /*
+* Note that, if only options have been passed, without a kernel
+* name, str == cp and options will be ignored at the boot blocks
+* level.
+* This a feature intended to make `boot -a' behave as intended.
+* If you want the bootblocks to handle arguments explicitly, a
+* kernel filename needs to be provided (as in `boot bsd -a').
+*/
*str = 0;
- switch(*cp) {
+ switch (*cp) {
default:
- printf ("boot options string <%s> must start with -\n", cp);
+ printf("boot options string <%s> must start with -\n", cp);
return -1;
case 0:
return 0;
@@ -137,9 +144,10 @@ parseargs(char *str, int *howtop)
++cp;
while (*cp) {
- BOOT_FLAG(*cp, *howtop);
- /* handle specialties */
switch (*cp++) {
+ case 'a':
+ *howtop |= RB_ASKNAME;
+ break;
case 'd':
if (!debug) debug = 1;
break;
@@ -379,7 +387,7 @@ main(void)
int chosen;
char bootline[512]; /* Should check size? */
char *cp;
- int i, fd, len;
+ int i, fd;
#ifdef
Re: ospf6d: sync hello.c with ospfd
On Thu, Jan 02, 2020 at 05:17:02PM +0100, Denis Fondras wrote:
> Sync with ospfd's hello.c
ok remi@
>
> Index: hello.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/hello.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 hello.c
> --- hello.c 23 Dec 2019 11:25:41 - 1.21
> +++ hello.c 2 Jan 2020 16:11:19 -
> @@ -41,8 +41,6 @@ send_hello(struct iface *iface)
> struct hello_hdr hello;
> struct nbr *nbr;
> struct ibuf *buf;
> - int ret;
> - u_int32_topts;
>
> switch (iface->type) {
> case IF_TYPE_POINTOPOINT:
> @@ -72,10 +70,8 @@ send_hello(struct iface *iface)
> /* hello header */
> hello.iface_id = htonl(iface->ifindex);
> LSA_24_SETHI(hello.opts, iface->priority);
> - opts = area_ospf_options(iface->area);
> - LSA_24_SETLO(hello.opts, opts);
> + LSA_24_SETLO(hello.opts, area_ospf_options(iface->area));
> hello.opts = htonl(hello.opts);
> -
> hello.hello_interval = htons(iface->hello_interval);
> hello.rtr_dead_interval = htons(iface->dead_interval);
>
> @@ -104,10 +100,11 @@ send_hello(struct iface *iface)
> if (upd_ospf_hdr(buf, iface))
> goto fail;
>
> - ret = send_packet(iface, buf, );
> + if (send_packet(iface, buf, ) == -1)
> + goto fail;
>
> ibuf_free(buf);
> - return (ret);
> + return (0);
> fail:
> log_warn("send_hello");
> ibuf_free(buf);
> @@ -120,7 +117,6 @@ recv_hello(struct iface *iface, struct i
> {
> struct hello_hdr hello;
> struct nbr *nbr = NULL, *dr;
> - struct area *area;
> u_int32_tnbr_id, opts;
> int nbr_change = 0;
>
> @@ -148,12 +144,9 @@ recv_hello(struct iface *iface, struct i
> return;
> }
>
> - if ((area = iface->area) == NULL)
> - fatalx("interface lost area");
> -
> opts = LSA_24_GETLO(ntohl(hello.opts));
> - if ((opts & OSPF_OPTION_E && area->stub) ||
> - ((opts & OSPF_OPTION_E) == 0 && !area->stub)) {
> + if ((opts & OSPF_OPTION_E && iface->area->stub) ||
> + ((opts & OSPF_OPTION_E) == 0 && !iface->area->stub)) {
> log_warnx("recv_hello: ExternalRoutingCapability mismatch, "
> "interface %s", iface->name);
> return;
> @@ -161,8 +154,15 @@ recv_hello(struct iface *iface, struct i
>
> /* match router-id */
> LIST_FOREACH(nbr, >nbr_list, entry) {
> - if (nbr == iface->self)
> + if (nbr == iface->self) {
> + if (nbr->id.s_addr == rtr_id) {
> + log_warnx("recv_hello: Router-ID collision on "
> + "interface %s neighbor IP %s", iface->name,
> + log_in6addr(src));
> + return;
> + }
> continue;
> + }
> if (nbr->id.s_addr == rtr_id)
> break;
> }
>
Re: ospf6d: sync database.c with ospfd(8)
On Thu, Jan 02, 2020 at 04:05:45PM +0100, Denis Fondras wrote:
> This is mostly log messages sync.
ok remi@
>
> Index: database.c
> ===
> RCS file: /cvs/src/usr.sbin/ospf6d/database.c,v
> retrieving revision 1.18
> diff -u -p -r1.18 database.c
> --- database.c23 Dec 2019 07:33:49 - 1.18
> +++ database.c2 Jan 2020 14:31:46 -
> @@ -43,7 +43,6 @@ send_db_description(struct nbr *nbr)
> struct db_dscrp_hdr dd_hdr;
> struct lsa_entry*le, *nle;
> struct ibuf *buf;
> - int ret = 0;
> u_int8_t bits = 0;
>
> if ((buf = ibuf_open(nbr->iface->mtu - sizeof(struct ip6_hdr))) == NULL)
> @@ -63,11 +62,10 @@ send_db_description(struct nbr *nbr)
> case NBR_STA_INIT:
> case NBR_STA_2_WAY:
> case NBR_STA_SNAP:
> - log_debug("send_db_description: cannot send packet in state %s,"
> - " neighbor ID %s", nbr_state_name(nbr->state),
> - inet_ntoa(nbr->id));
> - ret = -1;
> - goto done;
> + log_debug("send_db_description: neighbor ID %s: "
> + "cannot send packet in state %s", inet_ntoa(nbr->id),
> + nbr_state_name(nbr->state));
> + goto fail;
> case NBR_STA_XSTRT:
> bits |= OSPF_DBD_MS | OSPF_DBD_M | OSPF_DBD_I;
> nbr->dd_more = 1;
> @@ -90,7 +88,7 @@ send_db_description(struct nbr *nbr)
>
> /* build LSA list */
> for (le = TAILQ_FIRST(>db_sum_list); le != NULL &&
> - buf->wpos + sizeof(struct lsa_hdr) < buf->max; le = nle) {
> + ibuf_left(buf) >= sizeof(struct lsa_hdr); le = nle) {
> nbr->dd_end = nle = TAILQ_NEXT(le, entry);
> if (ibuf_add(buf, le->le_lsa, sizeof(struct lsa_hdr)))
> goto fail;
> @@ -146,10 +144,11 @@ send_db_description(struct nbr *nbr)
> goto fail;
>
> /* transmit packet */
> - ret = send_packet(nbr->iface, buf, );
> -done:
> + if (send_packet(nbr->iface, buf, ) == -1)
> + goto fail;
> +
> ibuf_free(buf);
> - return (ret);
> + return (0);
> fail:
> log_warn("send_db_description");
> ibuf_free(buf);
> @@ -163,8 +162,8 @@ recv_db_description(struct nbr *nbr, cha
> int dupe = 0;
>
> if (len < sizeof(dd_hdr)) {
> - log_warnx("recv_db_description: "
> - "bad packet size, neighbor ID %s", inet_ntoa(nbr->id));
> + log_warnx("recv_db_description: neighbor ID %s: "
> + "bad packet size", inet_ntoa(nbr->id));
> return;
> }
> memcpy(_hdr, buf, sizeof(dd_hdr));
> @@ -173,9 +172,9 @@ recv_db_description(struct nbr *nbr, cha
>
> /* db description packet sanity checks */
> if (ntohs(dd_hdr.iface_mtu) > nbr->iface->mtu) {
> - log_warnx("recv_db_description: invalid MTU %d sent by "
> - "neighbor ID %s, expected %d", ntohs(dd_hdr.iface_mtu),
> - inet_ntoa(nbr->id), nbr->iface->mtu);
> + log_warnx("recv_db_description: neighbor ID %s: "
> + "invalid MTU %d expected %d", inet_ntoa(nbr->id),
> + ntohs(dd_hdr.iface_mtu), nbr->iface->mtu);
> return;
> }
>
> @@ -183,7 +182,7 @@ recv_db_description(struct nbr *nbr, cha
> nbr->last_rx_bits == dd_hdr.bits &&
> ntohl(dd_hdr.dd_seq_num) == nbr->dd_seq_num - nbr->dd_master ?
> 1 : 0) {
> - log_debug("recv_db_description: dupe from ID %s",
> + log_debug("recv_db_description: dupe from neighbor ID %s",
> inet_ntoa(nbr->id));
> dupe = 1;
> }
> @@ -193,9 +192,9 @@ recv_db_description(struct nbr *nbr, cha
> case NBR_STA_ATTEMPT:
> case NBR_STA_2_WAY:
> case NBR_STA_SNAP:
> - log_debug("recv_db_description: packet ignored in state %s, "
> - "neighbor ID %s", nbr_state_name(nbr->state),
> - inet_ntoa(nbr->id));
> + log_debug("recv_db_description: neighbor ID %s: "
> + "packet ignored in state %s", inet_ntoa(nbr->id),
> + nbr_state_name(nbr->state));
> return;
> case NBR_STA_INIT:
> /* evaluate dr and bdr after issuing a 2-Way event */
> @@ -224,9 +223,11 @@ recv_db_description(struct nbr *nbr, cha
> } else if (!(dd_hdr.bits & (OSPF_DBD_I | OSPF_DBD_MS))) {
> /* M only case: we are master */
> if (ntohl(dd_hdr.dd_seq_num) != nbr->dd_seq_num) {
> - log_warnx("recv_db_description: invalid "
> - "seq num, mine %x his %x",
> -
Re: TIMESPEC_TO_NSEC(), futex(2) & __thrsleep(2)
On 02/01/20(Thu) 14:29, Scott Cheloha wrote:
> > On Jan 2, 2020, at 9:02 AM, Martin Pieuchot wrote:
> >
> > On 01/01/20(Wed) 22:13, Scott Cheloha wrote:
> >>> On Dec 31, 2019, at 9:35 AM, Martin Pieuchot wrote:
> >>>
> >>> I'd like to stop converting the given timespec to ticks and instead use
> >>> nanoseconds. This is part of the ongoing effort to reduce the use of
> >>> `hz' through the kernel.
> >>>
> >>> Since I don't know C I'd appreciate any pointer about the checks that
> >>> should be added to TIMESPEC_TO_NSEC().
> >>>
> >>> Then the conversions to {t,rw}sleep_nsec(9) become trivial, diff below.
> >>
> >> We can't do this until timeouts have a tickless interface. Otherwise
> >> your timeouts will return early. That's why I was saving the sys/kern
> >> conversions until after resolving that issue.
> >
> > I don't understand, can you elaborate?
>
> Timeout are scheduled against the current value of "ticks". Any time that
> has elapsed since the current tick began is unaccounted for. You need to
> add a tick to your sleep to account for it. tstohz(9) does this. We don't
> do it automatically for the *sleep_nsec(9) interfaces because that would
> have complicated the conversions we're doing and probably broken callers
> before we were ready to break them.
I question the argument that would complicate the conversions. Isn't it
just a margin of error that is given by the precision of the conversion?
Here it is 1 tick, generally ~10ms. So either the code work with a
sleep of 10ms less or more. Generally it doesn't matter. Now for
userland facing programs you shouldn't wakeup 10ms earlier.
I don't understand why the rounding precision is different between the
two interfaces. We have an interface that adds one tick and one that
doesn't. Both choices are imprecise and should disappear if/when the
guts of the sleep are modified to be tickless (whatever that means).
In the meantime I'd suggest we keep the same behavior between the two
interfaces so we can move forward with the other part of the problem:
the conversion.
It is like doing a refactoring with introducing a behavior change that
prevent us from finishing the refactoring because the change depends on
the internals... Am I going in circle?
PS: What about architectures that won't go tickless? How are we going
to deal with the conversions if there's more than one API?
drop AMD64 strings in pcidevs
ie "AMD AMD64 17h Root Complex" -> "AMD 17h Root Complex"
Index: arch/amd64/pci/pchb.c
===
RCS file: /cvs/src/sys/arch/amd64/pci/pchb.c,v
retrieving revision 1.43
diff -u -p -r1.43 pchb.c
--- arch/amd64/pci/pchb.c 28 Apr 2018 15:44:59 - 1.43
+++ arch/amd64/pci/pchb.c 3 Jan 2020 00:31:08 -
@@ -159,8 +159,8 @@ pchbattach(struct device *parent, struct
case PCI_VENDOR_AMD:
printf("\n");
switch (PCI_PRODUCT(pa->pa_id)) {
- case PCI_PRODUCT_AMD_AMD64_0F_HT:
- case PCI_PRODUCT_AMD_AMD64_10_HT:
+ case PCI_PRODUCT_AMD_0F_HT:
+ case PCI_PRODUCT_AMD_10_HT:
for (i = 0; i < AMD64HT_NUM_LDT; i++)
pchb_amd64ht_attach(self, pa, i);
break;
Index: arch/i386/pci/pchb.c
===
RCS file: /cvs/src/sys/arch/i386/pci/pchb.c,v
retrieving revision 1.90
diff -u -p -r1.90 pchb.c
--- arch/i386/pci/pchb.c28 Apr 2018 15:44:59 - 1.90
+++ arch/i386/pci/pchb.c3 Jan 2020 00:31:19 -
@@ -185,8 +185,8 @@ pchbattach(struct device *parent, struct
case PCI_VENDOR_AMD:
printf("\n");
switch (PCI_PRODUCT(pa->pa_id)) {
- case PCI_PRODUCT_AMD_AMD64_0F_HT:
- case PCI_PRODUCT_AMD_AMD64_10_HT:
+ case PCI_PRODUCT_AMD_0F_HT:
+ case PCI_PRODUCT_AMD_10_HT:
for (i = 0; i < AMD64HT_NUM_LDT; i++)
pchb_amd64ht_attach(self, pa, i);
break;
Index: dev/pci/amas.c
===
RCS file: /cvs/src/sys/dev/pci/amas.c,v
retrieving revision 1.5
diff -u -p -r1.5 amas.c
--- dev/pci/amas.c 15 Jun 2014 11:43:24 - 1.5
+++ dev/pci/amas.c 3 Jan 2020 00:28:12 -
@@ -132,9 +132,9 @@ struct cfdriver amas_cd = {
};
const struct pci_matchid amas_devices[] = {
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_0F_ADDR },
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_10_ADDR },
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_11_ADDR },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_0F_ADDR },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_10_ADDR },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_11_ADDR },
};
int
@@ -161,13 +161,13 @@ amas_attach(struct device *parent, struc
amas->pa_pc = pa->pa_pc;
switch (PCI_PRODUCT(pa->pa_id)) {
- case PCI_PRODUCT_AMD_AMD64_0F_ADDR:
+ case PCI_PRODUCT_AMD_0F_ADDR:
amas->family = AMAS_FAM_0Fh;
break;
- case PCI_PRODUCT_AMD_AMD64_10_ADDR:
+ case PCI_PRODUCT_AMD_10_ADDR:
amas->family = AMAS_FAM_10h;
break;
- case PCI_PRODUCT_AMD_AMD64_11_ADDR:
+ case PCI_PRODUCT_AMD_11_ADDR:
amas->family = AMAS_FAM_11h;
break;
}
Index: dev/pci/azalia.c
===
RCS file: /cvs/src/sys/dev/pci/azalia.c,v
retrieving revision 1.253
diff -u -p -r1.253 azalia.c
--- dev/pci/azalia.c14 Oct 2019 01:59:14 - 1.253
+++ dev/pci/azalia.c3 Jan 2020 00:28:35 -
@@ -528,8 +528,8 @@ azalia_pci_attach(struct device *parent,
/* disable MSI for AMD Summit Ridge/Raven Ridge HD Audio */
if (PCI_VENDOR(sc->pciid) == PCI_VENDOR_AMD) {
switch (PCI_PRODUCT(sc->pciid)) {
- case PCI_PRODUCT_AMD_AMD64_17_HDA:
- case PCI_PRODUCT_AMD_AMD64_17_1X_HDA:
+ case PCI_PRODUCT_AMD_17_HDA:
+ case PCI_PRODUCT_AMD_17_1X_HDA:
pa->pa_flags &= ~PCI_FLAGS_MSI_ENABLED;
}
}
Index: dev/pci/ccp_pci.c
===
RCS file: /cvs/src/sys/dev/pci/ccp_pci.c,v
retrieving revision 1.4
diff -u -p -r1.4 ccp_pci.c
--- dev/pci/ccp_pci.c 2 Jan 2020 22:34:41 - 1.4
+++ dev/pci/ccp_pci.c 3 Jan 2020 00:28:51 -
@@ -43,11 +43,11 @@ struct cfattach ccp_pci_ca = {
};
static const struct pci_matchid ccp_pci_devices[] = {
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_16_CCP },
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_CCP_1 },
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_CCP_2 },
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_1X_CCP },
- { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_AMD64_17_3X_CCP },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_16_CCP },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_17_CCP_1 },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_17_CCP_2 },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_17_1X_CCP },
+ { PCI_VENDOR_AMD, PCI_PRODUCT_AMD_17_3X_CCP },
};
int
Index: dev/pci/kate.c
