Re: Adding POSIX c99 compiler wrapper script

2022-02-11 Thread Theo de Raadt
Joe Nelson  wrote:

> I noticed that OpenBSD lacks the POSIX "c99" compiler wrapper.
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html
> 
> Is it missing because the community feels it's ill conceived, or just
> because nobody stepped up to implement it? If the latter, I can
> contribute a patch to add it as a wrapper around cc.

I think it is beyond ridiculous to attempt to standarize ("require")
a command called c99, when the command "cc" is not documented...

unless one realizes the commitee has become too happy at inventing new,
rather than following their charter which is largely documenting
established practice and negotiating simplification of differences where
possible.  Instead, the commitee increasingly tosses shit like this into
"the standard" when some of them realize their futility as
"documentation writers" rather than large-system-progremamers, they know
they cannot re-specify the things people actually use (meaning, cc, with
tons of options), as those complicated subsystems follow a blend of
defacto-standards and per-team design.  "cc" commandline options has little
in common with "ls", "cat", or "poll" (which I mention, because these
committees screwed up poll by underspecifying it RATHER than negotiating
simplification).

The people who pushed for this should retract this specification, mostlyn
because the proposal is stupid, will not be adopted by everyone, will
be considered a laughing stock as time goes on, and they will look smart
if they pull it. 

How did this come to be?  This is how, I think: Noone gave POSIX the
rope to specify new things, and "cc9" commandline is precisely the type
of thing which is new (because noone has these precise options) and
immediately old (all the compilers have more than these options, and
people use the other options more than the listed ones).  The commitee
was not established as inventors, but as documentors & helpful purifiers
of "established standards over a range of systems".  This kind of page
is is neither.

So, the c99 standard isn't just ill conceived.  It is scope creep
outside the purview of the commitee, and downstreams must reject it to
remind them of their role & position in the operating system ecosystem.

> A few questions about the desired behavior:
> 
> * If the user passes options not listed by POSIX, should the wrapper
>   die with a usage error, or pass them silently to the underlying
>   compiler? The FreeBSD implementation [0] does strict checking, while
>   NetBSD [1] and GCC on Debian [2] pass along all parameters.
> 
> * Should it add -pedantic as well as -std=c99?
> 
> * Is /usr/bin/c99 something that should go in OpenBSD base, or in
>   compiler ports?
> 
> 0: https://github.com/freebsd/freebsd-src/blob/main/usr.bin/c99/c99.c
> 1: https://github.com/NetBSD/src/blob/trunk/usr.bin/c99/c99.sh
> 2: https://salsa.debian.org/toolchain-team/gcc-defaults/-/blob/master/c99
> 



Re: Multiple issues with radeondrm startup code

2022-02-11 Thread Jonathan Gray
On Fri, Feb 11, 2022 at 06:42:11PM -0700, Ted Bullock wrote:
> Hey,
> 
> I've found some problems with the radeondrm initialization
> codepath (radeon_kms.c). Before I start, I should mention that I am
> working on some diffs to remove a bunch of the sparc64 MD ifdef's as
> well. In radeondrm_attach_kms:
> 
> starting from line 545
> ==
> #define RADEON_PCI_MEM0x10
> ^^ inline defines for bar registers reads as weird/bad to me
> 
>   type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM);
>   if (PCI_MAPREG_TYPE(type) != PCI_MAPREG_TYPE_MEM ||
>   pci_mapreg_info(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM,
>   type, &rdev->fb_aper_offset, &rdev->fb_aper_size, NULL)) {
>   printf(": can't get frambuffer info\n");
>   return;
> ^^
> Should set radeon_fatal_error = 1 before returning
>   }
> #if !defined(__sparc64__)
> ^^ I've tested this on sparc64 xvr-100, this ifdef isn't needed
>   if (rdev->fb_aper_offset == 0) {
>   bus_size_t start, end;
>   bus_addr_t base;
> 
>   start = max(PCI_MEM_START, pa->pa_memex->ex_start);
> Potential NULL dereference ->
>   end = min(PCI_MEM_END, pa->pa_memex->ex_end);
> And here --->^^
>   if (pa->pa_memex == NULL ||
> 
> Obviously just move this NULL check up two lines
>   extent_alloc_subregion(pa->pa_memex, start, end,
>   rdev->fb_aper_size, rdev->fb_aper_size, 0, 0, 0, &base)) {
>   printf(": can't reserve framebuffer space\n");
>   return;
> ^^
> Should set radeon_fatal_error = 1 before returning
>   }
>   pci_conf_write(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM, base);
>   if (PCI_MAPREG_MEM_TYPE(type) == PCI_MAPREG_MEM_TYPE_64BIT)
>   pci_conf_write(pa->pa_pc, pa->pa_tag,
>   RADEON_PCI_MEM + 4, (uint64_t)base >> 32);
> This evaluates to zero on all architectures --->
>   rdev->fb_aper_offset = base;
>   }
> #endif
> 
>   for (i = PCI_MAPREG_START; i < PCI_MAPREG_END; i += 4) {
> ^^
> This is an awkward (bad) idiom to iterate through BARs
>   type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, i);
>   if (type == PCI_MAPREG_TYPE_IO) {
> 
> This is a bitmap, you cannot use it like this, the correct usage
> would be PCI_MAPREG_TYPE(type) == PCI_MAPREG_TYPE_IO
> 
>   pci_mapreg_map(pa, i, type, 0, NULL,
>   &rdev->rio_mem, NULL, &rdev->rio_mem_size, 0);
>   break;
>   }
>   if (type == PCI_MAPREG_MEM_TYPE_64BIT)
>   
> Same as above, this is incorrect usage
>   i += 4;
>   }
> 
> A note about this for loop. This loop was broken in revision 1.72 to
> support radeon devices on POWER9 systems. These registers are available
> (I believe) through the MMIO BAR so people haven't noticed that it's
> broken.  I don't know if ALL devices work like that, though certainly
> most do, if so this entire loop could possibly be removed and just rely
> on MMIO access to these registers.
> 
>   if (rdev->family >= CHIP_BONAIRE) {
>   type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, 0x18);
>-->
> If putting an inline #define was appropriate earlier why not here
>   if (PCI_MAPREG_TYPE(type) != PCI_MAPREG_TYPE_MEM ||
>   pci_mapreg_map(pa, 0x18, type, BUS_SPACE_MAP_LINEAR, NULL,
> And here  >
>   &rdev->doorbell.bsh, &rdev->doorbell.base,
>   &rdev->doorbell.size, 0)) {
>   printf(": can't map doorbell space\n");
>   return;
>   ^^
> Should set radeon_fatal_error = 1 before returning
>   }
>   rdev->doorbell.ptr = bus_space_vaddr(rdev->memt,
>   rdev->doorbell.bsh);
>   }
> 
>   if (rdev->family >= CHIP_BONAIRE)
>   rmmio_bar = 0x24;
>   else
>   rmmio_bar = 0x18;
> 
>   type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, rmmio_bar);
>   if (PCI_MAPREG_TYPE(type) != PCI_MAPREG_TYPE_MEM ||
>   pci_mapreg_map(pa, rmmio_bar, type, BUS_SPACE_MAP_LINEAR, NULL,
>   &rdev->rmmio_bsh, &rdev->rmmio_base, &rdev->rmmio_size, 0)) {
>   printf(": can't map rmmio space\n");
>   return;
>   ^^
> Should set radeon_fatal_error = 1 before returning
>   }
>   rdev->rmmio = bus_space_vaddr(rdev->memt, rdev->rmmio_bsh);

Multiple issues with radeondrm startup code

2022-02-11 Thread Ted Bullock
Hey,

I've found some problems with the radeondrm initialization
codepath (radeon_kms.c). Before I start, I should mention that I am
working on some diffs to remove a bunch of the sparc64 MD ifdef's as
well. In radeondrm_attach_kms:

starting from line 545
==
#define RADEON_PCI_MEM  0x10
^^ inline defines for bar registers reads as weird/bad to me

type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM);
if (PCI_MAPREG_TYPE(type) != PCI_MAPREG_TYPE_MEM ||
pci_mapreg_info(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM,
type, &rdev->fb_aper_offset, &rdev->fb_aper_size, NULL)) {
printf(": can't get frambuffer info\n");
return;
^^
Should set radeon_fatal_error = 1 before returning
}
#if !defined(__sparc64__)
^^ I've tested this on sparc64 xvr-100, this ifdef isn't needed
if (rdev->fb_aper_offset == 0) {
bus_size_t start, end;
bus_addr_t base;

start = max(PCI_MEM_START, pa->pa_memex->ex_start);
Potential NULL dereference ->
end = min(PCI_MEM_END, pa->pa_memex->ex_end);
And here --->^^
if (pa->pa_memex == NULL ||

Obviously just move this NULL check up two lines
extent_alloc_subregion(pa->pa_memex, start, end,
rdev->fb_aper_size, rdev->fb_aper_size, 0, 0, 0, &base)) {
printf(": can't reserve framebuffer space\n");
return;
^^
Should set radeon_fatal_error = 1 before returning
}
pci_conf_write(pa->pa_pc, pa->pa_tag, RADEON_PCI_MEM, base);
if (PCI_MAPREG_MEM_TYPE(type) == PCI_MAPREG_MEM_TYPE_64BIT)
pci_conf_write(pa->pa_pc, pa->pa_tag,
RADEON_PCI_MEM + 4, (uint64_t)base >> 32);
This evaluates to zero on all architectures --->
rdev->fb_aper_offset = base;
}
#endif

for (i = PCI_MAPREG_START; i < PCI_MAPREG_END; i += 4) {
^^
This is an awkward (bad) idiom to iterate through BARs
type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, i);
if (type == PCI_MAPREG_TYPE_IO) {

This is a bitmap, you cannot use it like this, the correct usage
would be PCI_MAPREG_TYPE(type) == PCI_MAPREG_TYPE_IO

pci_mapreg_map(pa, i, type, 0, NULL,
&rdev->rio_mem, NULL, &rdev->rio_mem_size, 0);
break;
}
if (type == PCI_MAPREG_MEM_TYPE_64BIT)

Same as above, this is incorrect usage
i += 4;
}

A note about this for loop. This loop was broken in revision 1.72 to
support radeon devices on POWER9 systems. These registers are available
(I believe) through the MMIO BAR so people haven't noticed that it's
broken.  I don't know if ALL devices work like that, though certainly
most do, if so this entire loop could possibly be removed and just rely
on MMIO access to these registers.

if (rdev->family >= CHIP_BONAIRE) {
type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, 0x18);
   -->
If putting an inline #define was appropriate earlier why not here
if (PCI_MAPREG_TYPE(type) != PCI_MAPREG_TYPE_MEM ||
pci_mapreg_map(pa, 0x18, type, BUS_SPACE_MAP_LINEAR, NULL,
And here  >
&rdev->doorbell.bsh, &rdev->doorbell.base,
&rdev->doorbell.size, 0)) {
printf(": can't map doorbell space\n");
return;
^^
Should set radeon_fatal_error = 1 before returning
}
rdev->doorbell.ptr = bus_space_vaddr(rdev->memt,
rdev->doorbell.bsh);
}

if (rdev->family >= CHIP_BONAIRE)
rmmio_bar = 0x24;
else
rmmio_bar = 0x18;

type = pci_mapreg_type(pa->pa_pc, pa->pa_tag, rmmio_bar);
if (PCI_MAPREG_TYPE(type) != PCI_MAPREG_TYPE_MEM ||
pci_mapreg_map(pa, rmmio_bar, type, BUS_SPACE_MAP_LINEAR, NULL,
&rdev->rmmio_bsh, &rdev->rmmio_base, &rdev->rmmio_size, 0)) {
printf(": can't map rmmio space\n");
return;
^^
Should set radeon_fatal_error = 1 before returning
}
rdev->rmmio = bus_space_vaddr(rdev->memt, rdev->rmmio_bsh);

#if !defined(__sparc64__)
^
I've tested without this MD ifdef on sparc64, there is no adverse
behavior when using xvr-100 that I have noti

Re: Adding POSIX c99 compiler wrapper script

2022-02-11 Thread Andreas Kusalananda Kähäri
On Fri, Feb 11, 2022 at 10:30:26PM +, Stuart Henderson wrote:
> On 2022/02/11 16:13, Joe Nelson wrote:
> > 
> > I noticed that OpenBSD lacks the POSIX "c99" compiler wrapper.
> > https://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html
> > 
> > Is it missing because the community feels it's ill conceived, or just
> > because nobody stepped up to implement it? If the latter, I can
> > contribute a patch to add it as a wrapper around cc.
> 
> AFAIK we've never seen anything which uses it. At least not in ports.

I don't want to take sides, but the yash shell, which is in ports, has a
configure script that says

cc="${CC-c99}"

It's likely that CC is set in the environment when the port is built,
but still, it would default to c99 if it wasn't set.


> 
> > A few questions about the desired behavior:
> > 
> > * If the user passes options not listed by POSIX, should the wrapper
> >   die with a usage error, or pass them silently to the underlying
> >   compiler? The FreeBSD implementation [0] does strict checking, while
> >   NetBSD [1] and GCC on Debian [2] pass along all parameters.
> > 
> > * Should it add -pedantic as well as -std=c99?
> > 
> > * Is /usr/bin/c99 something that should go in OpenBSD base, or in
> >   compiler ports?
> 
> If it's added at all, base is probably the place for it.
> 
> > 0: https://github.com/freebsd/freebsd-src/blob/main/usr.bin/c99/c99.c
> 
> Funny, it strips -lrt!
> 
> > 1: https://github.com/NetBSD/src/blob/trunk/usr.bin/c99/c99.sh
> > 2: https://salsa.debian.org/toolchain-team/gcc-defaults/-/blob/master/c99
> > 

-- 
Andreas (Kusalananda) Kähäri
SciLifeLab, NBIS, ICM
Uppsala University, Sweden

.



Re: Adding POSIX c99 compiler wrapper script

2022-02-11 Thread Jonathan Gray
On Fri, Feb 11, 2022 at 04:13:22PM -0600, Joe Nelson wrote:
> 
> I noticed that OpenBSD lacks the POSIX "c99" compiler wrapper.
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html

"[CD] C-Language Development Utilities 
The functionality described is optional."

I don't see the point in adding a c89 c99 c11 c17 or
c++98 c++03 c++11 c++14 c++17 c++20 c++23 wrapper.

It is easy enough to specify -std and many people would
be using the flags that add gnu extensions instead.

> 
> Is it missing because the community feels it's ill conceived, or just
> because nobody stepped up to implement it? If the latter, I can
> contribute a patch to add it as a wrapper around cc.
> 
> A few questions about the desired behavior:
> 
> * If the user passes options not listed by POSIX, should the wrapper
>   die with a usage error, or pass them silently to the underlying
>   compiler? The FreeBSD implementation [0] does strict checking, while
>   NetBSD [1] and GCC on Debian [2] pass along all parameters.
> 
> * Should it add -pedantic as well as -std=c99?
> 
> * Is /usr/bin/c99 something that should go in OpenBSD base, or in
>   compiler ports?
> 
> 0: https://github.com/freebsd/freebsd-src/blob/main/usr.bin/c99/c99.c
> 1: https://github.com/NetBSD/src/blob/trunk/usr.bin/c99/c99.sh
> 2: https://salsa.debian.org/toolchain-team/gcc-defaults/-/blob/master/c99
> 
> 



Re: Adding POSIX c99 compiler wrapper script

2022-02-11 Thread Stuart Henderson
On 2022/02/11 16:13, Joe Nelson wrote:
> 
> I noticed that OpenBSD lacks the POSIX "c99" compiler wrapper.
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/c99.html
> 
> Is it missing because the community feels it's ill conceived, or just
> because nobody stepped up to implement it? If the latter, I can
> contribute a patch to add it as a wrapper around cc.

AFAIK we've never seen anything which uses it. At least not in ports.

> A few questions about the desired behavior:
> 
> * If the user passes options not listed by POSIX, should the wrapper
>   die with a usage error, or pass them silently to the underlying
>   compiler? The FreeBSD implementation [0] does strict checking, while
>   NetBSD [1] and GCC on Debian [2] pass along all parameters.
> 
> * Should it add -pedantic as well as -std=c99?
> 
> * Is /usr/bin/c99 something that should go in OpenBSD base, or in
>   compiler ports?

If it's added at all, base is probably the place for it.

> 0: https://github.com/freebsd/freebsd-src/blob/main/usr.bin/c99/c99.c

Funny, it strips -lrt!

> 1: https://github.com/NetBSD/src/blob/trunk/usr.bin/c99/c99.sh
> 2: https://salsa.debian.org/toolchain-team/gcc-defaults/-/blob/master/c99
> 



Re: adding MIME type for XSLT

2022-02-11 Thread Stuart Henderson
On 2022/02/11 11:19, Florian Obser wrote:
> I'm wondering if we need to sync, unfortunately the two files are
> not diffable :/

easy enough to transform, and the extensions and mimetypes are basically
in sync. here are the differences:

--- ours
+++ nginx
@@ -2 +1,0 @@ application/atom+xml atom
-application/font-woff woff
@@ -11 +10 @@ application/octet-stream dmg
-application/octet-stream fs iso img
+application/octet-stream iso img
@@ -64,0 +64,3 @@ audio/x-realaudio ra
+font/woff woff
+font/woff2 woff2
+image/avif avif

we want to keep fs in octet-stream, it would make sense to change woff,
and add avif.



Re: wskbd_set_mixervolume

2022-02-11 Thread Alexandre Ratchov
On Thu, Feb 10, 2022 at 04:53:59PM +0100, Anton Lindqvist wrote:
> On Wed, Feb 09, 2022 at 09:13:58AM +0100, Alexandre Ratchov wrote:
> > On Tue, Feb 08, 2022 at 06:59:39PM +0100, Anton Lindqvist wrote:
> > > On Tue, Feb 08, 2022 at 07:32:38AM +0100, Alexandre Ratchov wrote:
> > > > On Mon, Feb 07, 2022 at 06:55:21PM +0100, Anton Lindqvist wrote:
> > > > > On Mon, Feb 07, 2022 at 11:21:43AM +0100, Alexandre Ratchov wrote:
> > > > > > On Sun, Feb 06, 2022 at 08:40:35AM +0100, Anton Lindqvist wrote:
> > > > > > > 
> > > > > > > Polished diff. I'm omitting a necessary refactoring commit making
> > > > > > > audio_attach_mi() accept a new cookie argument.
> > > > > > > 
> > > > > > 
> > > > > > I'm not sure to understand the need for the wskbd_audio structure.
> > > > > > Couldn't we just store the cookie in audio and wskbd softc 
> > > > > > structures,
> > > > > > then pass it in the wskbd_set_mixervolume_sc() calls ?
> > > > > 
> > > > > Due to the device caching the data must be stored in either the audio 
> > > > > or
> > > > > wskbd softc and I don't want to expose the softc structures so I ended
> > > > > up introducing wskbd_audio. Dropping the caching would probably make 
> > > > > it
> > > > > possible to only pass down the cookie to wskbd_set_mixervolume_sc() 
> > > > > and
> > > > > always do the audio device lookup.
> > > > > 
> > > > > Is anyone in favor of this approach? I achieves the expected behavior 
> > > > > in
> > > > > my opinion.
> > > > 
> > > > IMHO, handling the volume keys this way won't work in the general
> > > > case. But for the short term we've no other options, have we?
> > > > 
> > > > AFAICS, you're fixing a concrete use-case by tweaking what already
> > > > exists, this won't make things more broken than they already are. I'm
> > > > OK with it.
> > > 
> > > Here's the complete diff including adding a cookie argument to
> > > audio_attach_mi().
> > 
> > Is the caching necessary? device_lookup() seems cheap and there are at
> > most two devices in most cases. Keeping the code minimal especially on
> > rare and non-performace-critical code-paths would be nice.
> > 
> > If you choose to drop the caching, this would allow to just add a a
> > new "cookie" argument to wskbd_set_mixervolume(), similarly to what
> > you did for audio_attach_mi()
> 
> Sure, I ended up adding a new function after all since the
> wskbd_set_mixervolume() prototype is not present in any header at this
> point.
> 

ok ratchov



Re: adding MIME type for XSLT

2022-02-11 Thread Jesse Alama
On Fri, Feb 11, 2022, at 11:19 AM, Florian Obser wrote:

> On 2022-02-11 02:29 -07, "Anthony J. Bentley"  wrote:
>> Jesse Alama writes:
>>> XSLT is a well-established XML-based language for stylesheets. It has been 
>>> ar
>>> ound since the late 90s; the most recent version was finalized in 2017 (see 
>>>  
>>> https://www.w3.org/TR/xslt-30/). The mime.types file bundled with OpenBSD 
>>> 7.0
>>>  -- typically used with httpd -- doesn't include this common MIME type. May 
>>> w
>>> e add it? Conventionally, XSLT files use the .xsl file extension and the 
>>> stan
>>> dard MIME type is "application/xslt+xml" (see 
>>> https://datatracker.ietf.org/do
>>> c/html/rfc3023#section-8.17). A diff looks like this:
>>>
>>> diff -Naur /usr/share/misc/mime.types /usr/src/share/misc/mime.types
>>> --- /usr/share/misc/mime.types  Thu Sep 30 20:01:17 2021
>>> +++ /usr/src/share/misc/mime.types  Fri Feb 11 07:36:11 2022
>>> @@ -56,6 +56,7 @@
>>>  application/x-tcl  tcl tk
>>>  application/x-x509-ca-cert der pem crt
>>>  application/x-xpinstallxpi
>>> +application/xslt+xmlxsl
>>>  application/xhtml+xml  xhtml
>>>  application/zipzip
>>
>> The list is sorted alphabetically, so xslt needs to come after xhtml.
>>
>> I like the idea. From some basic searches it looks like Chrome might be
>> unable to handle XSLT with the registered MIME type, only supporting
>> text/xml. Is that still the case, and if so, do we care?
>>
>
> IIRC we got the list of mime types originally from nginx.
> I just had a look, it does not have a mime type for xsl.
>
> I'm wondering if we need to sync, unfortunately the two files are
> not diffable :/

I'd be happy to try to push to ensure that application/xslt+xml gets added to 
nginx, too.

Regarding the more general issue of syncing up with nginx: that makes sense. 
Here's the latest mime.types there: 
https://raw.githubusercontent.com/nginx/nginx/master/conf/mime.types . If 
there's anything I can do to help out, let me know.



more ipmi(4) sensors

2022-02-11 Thread Stuart Henderson
I have some Dells that report various things in ipmi sensors that aren't
currently handled by ipmi(4) feeding to sysctl hw.sensors. It actually
has 100+ in all (visible in ipmitool); most are on/off indicators for
things including power good at various points, presence of various
cables, something to do with RAID battery, etc.

Seems like it would be overkill to add most of them to hw.sensors, but
I find the power consumption quite useful/interesting (and sane to add,
as it matches what is often available on laptops).

Currently ipmi(4) only uses the reported _sensor type_ to identify
a sensor; in the case of these, there is only "current" which covers
both power and current, so the diff below uses the _unit type_ instead.

$ sysctl hw.sensors
hw.sensors.cpu0.temp0=44.00 degC
hw.sensors.mfi0.drive0=online (sd0), OK
hw.sensors.ipmi0.temp0=24.00 degC (Inlet Temp), OK
hw.sensors.ipmi0.temp1=47.00 degC (Temp), OK
hw.sensors.ipmi0.fan0=5520 RPM (Fan1A), OK
hw.sensors.ipmi0.fan1=5400 RPM (Fan1B), OK
hw.sensors.ipmi0.fan2=5520 RPM (Fan2A), OK
hw.sensors.ipmi0.fan3=5280 RPM (Fan2B), OK
hw.sensors.ipmi0.fan4=5520 RPM (Fan3A), OK
hw.sensors.ipmi0.fan5=5160 RPM (Fan3B), OK
hw.sensors.ipmi0.fan6=5280 RPM (Fan4A), OK
hw.sensors.ipmi0.fan7=5280 RPM (Fan4B), OK
hw.sensors.ipmi0.fan8=5640 RPM (Fan5A), OK
hw.sensors.ipmi0.fan9=5160 RPM (Fan5B), OK
hw.sensors.ipmi0.volt0=230.00 VDC (Voltage 1), OK
hw.sensors.ipmi0.volt1=240.00 VDC (Voltage 2), OK
hw.sensors.ipmi0.power0=98.00 W (Pwr Consumption), OK
hw.sensors.ipmi0.current0=0.20 A (Current 1), OK
hw.sensors.ipmi0.current1=0.20 A (Current 2), OK
hw.sensors.ipmi0.indicator0=Off (Intrusion), OK
hw.sensors.ipmi0.indicator1=On (Status), OK
hw.sensors.ipmi0.indicator2=On (Status), OK

Also note the voltages above are reported in VDC; that is unchanged
by this diff - ipmi only has "volts" but the sensors framework
has separate vdc/vac so unless we add a new type (which IIRC has
ABI effects) we have to just pick one.

Any comments?

Index: ipmi.c
===
RCS file: /cvs/src/sys/dev/ipmi.c,v
retrieving revision 1.115
diff -u -p -r1.115 ipmi.c
--- ipmi.c  23 Jan 2021 12:10:08 -  1.115
+++ ipmi.c  11 Feb 2022 18:12:22 -
@@ -67,12 +67,23 @@ int ipmi_enabled = 0;
 #define IPMI_BTMSG_DATASND 4
 #define IPMI_BTMSG_DATARCV 5
 
+/* IPMI 2.0, Table 42-3: Sensor Type Codes */
 #define IPMI_SENSOR_TYPE_TEMP  0x0101
 #define IPMI_SENSOR_TYPE_VOLT  0x0102
+#define IPMI_SENSOR_TYPE_CURRENT   0x0103
 #define IPMI_SENSOR_TYPE_FAN   0x0104
 #define IPMI_SENSOR_TYPE_INTRUSION 0x6F05
 #define IPMI_SENSOR_TYPE_PWRSUPPLY 0x6F08
 
+/* IPMI 2.0, Table 43-15: Sensor Unit Type Codes */
+#define IPMI_UNIT_TYPE_DEGREE_C1
+#define IPMI_UNIT_TYPE_DEGREE_F2
+#define IPMI_UNIT_TYPE_DEGREE_K3
+#define IPMI_UNIT_TYPE_VOLTS   4
+#define IPMI_UNIT_TYPE_AMPS5
+#define IPMI_UNIT_TYPE_WATTS   6
+#define IPMI_UNIT_TYPE_RPM 18
+
 #define IPMI_NAME_UNICODE  0x00
 #define IPMI_NAME_BCDPLUS  0x01
 #define IPMI_NAME_ASCII6BIT0x02
@@ -147,7 +158,7 @@ voidbt_buildmsg(struct ipmi_cmd *);
 void   cmn_buildmsg(struct ipmi_cmd *);
 
 intgetbits(u_int8_t *, int, int);
-intipmi_sensor_type(int, int, int);
+intipmi_sensor_type(int, int, int, int);
 
 void   ipmi_refresh_sensors(struct ipmi_softc *sc);
 intipmi_map_regs(struct ipmi_softc *sc, struct ipmi_attach_args *ia);
@@ -1217,6 +1228,18 @@ ipmi_sensor_status(struct ipmi_softc *sc
psensor->i_sensor.value = ipmi_convert(reading[0], s1, 6);
break;
 
+   case SENSOR_VOLTS_AC:
+   psensor->i_sensor.value = ipmi_convert(reading[0], s1, 6);
+   break;
+
+   case SENSOR_AMPS:
+   psensor->i_sensor.value = ipmi_convert(reading[0], s1, 6);
+   break;
+
+   case SENSOR_WATTS:
+   psensor->i_sensor.value = ipmi_convert(reading[0], s1, 6);
+   break;
+
case SENSOR_FANRPM:
psensor->i_sensor.value = ipmi_convert(reading[0], s1, 0);
if (((s1->units1>>3)&0x7) == 0x3)
@@ -1231,6 +1254,7 @@ ipmi_sensor_status(struct ipmi_softc *sc
switch (etype) {
case IPMI_SENSOR_TYPE_TEMP:
case IPMI_SENSOR_TYPE_VOLT:
+   case IPMI_SENSOR_TYPE_CURRENT:
case IPMI_SENSOR_TYPE_FAN:
/* non-recoverable threshold */
if (reading[2] & ((1 << 5) | (1 << 2)))
@@ -1309,17 +1333,26 @@ read_sensor(struct ipmi_softc *sc, struc
 }
 
 int
-ipmi_sensor_type(int type, int ext_type, int entity)
+ipmi_sensor_type(int type, int ext_type, int units2, int entity)
 {
-   switch (ext_type << 8L | type) {
-   case IPMI_SENSOR_TYPE_TEMP:
-   return (SENSOR_TEMP);
+   switch (units2) {
+   case IPMI_UNIT_TYPE_AMP

Re: apm -m means one of two things now

2022-02-11 Thread Jason McIntyre
On Thu, Feb 10, 2022 at 02:59:48PM +0100, Jan Stary wrote:
> With the recent change to apm -m,
> reporting either the battery lifetime
> or the estimated time to charge (thank you),
> the manpage seems to have been left behind.
> 
> While here, tweak some of the wording:
> 
> - "in minutes" or "in percent" is not parenthetical; say it explicitly

i guess this is better

> - surely -a displays the charger status, not the charger

yes, the brackets were badly placed.

> - AC is AC, not A/C, right?
> 

i'm less sure. i have seen a/c written but am not really up on this. i
think "AC" is clear enough. but if we make this change, we want to
change the instance in apm.c too. i've added that in my diff at end, but
really need some other developer feedback about whether A/C->AC is
desireable.

so i agree broadly with the diff. but some notes after the diff:

>   Jan
> 
> 
> Index: apm.8
> ===
> RCS file: /cvs/src/usr.sbin/apm/apm.8,v
> retrieving revision 1.44
> diff -u -p -r1.44 apm.8
> --- apm.8 3 Nov 2021 19:54:28 -   1.44
> +++ apm.8 10 Feb 2022 13:52:38 -
> @@ -59,7 +59,7 @@ The options are as follows:
>  .It Fl A
>  Switch to automatic performance adjustment mode (the default).
>  .It Fl a
> -Display the external charger (A/C status).
> +Display the external charger (AC) status.
>  0 means disconnected, 1
>  means connected, 2 means backup power source, and 255 means unknown.
>  .It Fl b
> @@ -82,9 +82,10 @@ setting
>  .Va hw.setperf
>  to 0.
>  .It Fl l
> -Display the estimated battery lifetime (in percent).
> +Display the estimated battery lifetime in percent.
>  .It Fl m
> -Display the estimated battery lifetime (in minutes).
> +Display the estimated battery lifetime in minutes when running or battery,
> +or the estimated time to recharge when running on an external charger.
>  .It Fl P
>  Display the performance adjustment mode.
>  0 means manual mode.
> 

- i think there really should be a comma after "lifetime"
- i don;t like the structure of the -m changes. it makes "minutes" apply
  explicitly to the first clause, but only implict with the second.
- i think we can getter a better text by defining -l and -m as
  essentially the same, but note the difference to -m when charging as a
  separate sentence.

so this diff:

Index: apm.8
===
RCS file: /cvs/src/usr.sbin/apm/apm.8,v
retrieving revision 1.44
diff -u -p -r1.44 apm.8
--- apm.8   3 Nov 2021 19:54:28 -   1.44
+++ apm.8   11 Feb 2022 17:28:03 -
@@ -59,7 +59,7 @@ The options are as follows:
 .It Fl A
 Switch to automatic performance adjustment mode (the default).
 .It Fl a
-Display the external charger (A/C status).
+Display the external charger (AC) status.
 0 means disconnected, 1
 means connected, 2 means backup power source, and 255 means unknown.
 .It Fl b
@@ -82,9 +82,10 @@ setting
 .Va hw.setperf
 to 0.
 .It Fl l
-Display the estimated battery lifetime (in percent).
+Display the estimated battery lifetime, in percent.
 .It Fl m
-Display the estimated battery lifetime (in minutes).
+Display the estimated battery lifetime, in minutes.
+If charging, the estimated time to fully charge is displayed instead.
 .It Fl P
 Display the performance adjustment mode.
 0 means manual mode.
Index: apm.c
===
RCS file: /cvs/src/usr.sbin/apm/apm.c,v
retrieving revision 1.40
diff -u -p -r1.40 apm.c
--- apm.c   6 Feb 2022 09:07:42 -   1.40
+++ apm.c   11 Feb 2022 17:28:03 -
@@ -394,7 +394,7 @@ balony:
}
 
if (doac)
-   printf("A/C adapter state: %s\n",
+   printf("AC adapter state: %s\n",
ac_state(reply.batterystate.ac_state));
 
if (doperf)

what do you think?

jmc



Re: superfluous words in lib/libc/gen/statvfs.3 and sys/sys/statvfs.h

2022-02-11 Thread Todd C . Miller
On Fri, 11 Feb 2022 08:21:38 +0100, alf wrote:

> there seem to be some superfluous words in
> lib/libc/gen/statvfs.3 and sys/sys/statvfs.h .
> "(unit f_frsize)" can be removed since in the man page
> it is explicitely mentioned directly below:
> " The fields of type fsblkcnt_t are reported in units of f_frsize. "
> For sys/sys/statvs.h I left them in although IMO it is
> quite obvious.
>
> The "to" word is just a typo I think.

Thanks, I committed the removal of the extra "to" in both files.
However, I left the  "(unit f_frsize)" since we want to keep
the struct in the man page in sync with the header file.

 - todd



Re: adding MIME type for XSLT

2022-02-11 Thread Florian Obser
On 2022-02-11 02:29 -07, "Anthony J. Bentley"  wrote:
> Jesse Alama writes:
>> XSLT is a well-established XML-based language for stylesheets. It has been ar
>> ound since the late 90s; the most recent version was finalized in 2017 (see  
>> https://www.w3.org/TR/xslt-30/). The mime.types file bundled with OpenBSD 7.0
>>  -- typically used with httpd -- doesn't include this common MIME type. May w
>> e add it? Conventionally, XSLT files use the .xsl file extension and the stan
>> dard MIME type is "application/xslt+xml" (see https://datatracker.ietf.org/do
>> c/html/rfc3023#section-8.17). A diff looks like this:
>>
>> diff -Naur /usr/share/misc/mime.types /usr/src/share/misc/mime.types
>> --- /usr/share/misc/mime.types   Thu Sep 30 20:01:17 2021
>> +++ /usr/src/share/misc/mime.types   Fri Feb 11 07:36:11 2022
>> @@ -56,6 +56,7 @@
>>  application/x-tcl   tcl tk
>>  application/x-x509-ca-cert  der pem crt
>>  application/x-xpinstall xpi
>> +application/xslt+xmlxsl
>>  application/xhtml+xml   xhtml
>>  application/zip zip
>
> The list is sorted alphabetically, so xslt needs to come after xhtml.
>
> I like the idea. From some basic searches it looks like Chrome might be
> unable to handle XSLT with the registered MIME type, only supporting
> text/xml. Is that still the case, and if so, do we care?
>

IIRC we got the list of mime types originally from nginx.
I just had a look, it does not have a mime type for xsl.

I'm wondering if we need to sync, unfortunately the two files are
not diffable :/

-- 
I'm not entirely sure you are real.



Re: adding MIME type for XSLT

2022-02-11 Thread Anthony J. Bentley
Jesse Alama writes:
> XSLT is a well-established XML-based language for stylesheets. It has been ar
> ound since the late 90s; the most recent version was finalized in 2017 (see  
> https://www.w3.org/TR/xslt-30/). The mime.types file bundled with OpenBSD 7.0
>  -- typically used with httpd -- doesn't include this common MIME type. May w
> e add it? Conventionally, XSLT files use the .xsl file extension and the stan
> dard MIME type is "application/xslt+xml" (see https://datatracker.ietf.org/do
> c/html/rfc3023#section-8.17). A diff looks like this:
>
> diff -Naur /usr/share/misc/mime.types /usr/src/share/misc/mime.types
> --- /usr/share/misc/mime.typesThu Sep 30 20:01:17 2021
> +++ /usr/src/share/misc/mime.typesFri Feb 11 07:36:11 2022
> @@ -56,6 +56,7 @@
>  application/x-tcltcl tk
>  application/x-x509-ca-cert   der pem crt
>  application/x-xpinstall  xpi
> +application/xslt+xmlxsl
>  application/xhtml+xmlxhtml
>  application/zip  zip

The list is sorted alphabetically, so xslt needs to come after xhtml.

I like the idea. From some basic searches it looks like Chrome might be
unable to handle XSLT with the registered MIME type, only supporting
text/xml. Is that still the case, and if so, do we care?



Re: apm -m means one of two things now

2022-02-11 Thread Jan Stary
On Feb 10 16:33:20, j...@juef.net wrote:
> (Thu, 10 Feb 14:59) Jan Stary:
> > With the recent change to apm -m,
> > reporting either the battery lifetime
> > or the estimated time to charge (thank you),
> > the manpage seems to have been left behind.
> > 
> > While here, tweak some of the wording:
> > 
> > - "in minutes" or "in percent" is not parenthetical; say it explicitly
> > - surely -a displays the charger status, not the charger
> > - AC is AC, not A/C, right?

> > -Display the estimated battery lifetime (in minutes).
> > +Display the estimated battery lifetime in minutes when running or battery,
> 
> typo:
> running or battery => running on battery

Thanks, new diff below.

Jan


Index: apm.8
===
RCS file: /cvs/src/usr.sbin/apm/apm.8,v
retrieving revision 1.44
diff -u -p -r1.44 apm.8
--- apm.8   3 Nov 2021 19:54:28 -   1.44
+++ apm.8   10 Feb 2022 13:52:38 -
@@ -59,7 +59,7 @@ The options are as follows:
 .It Fl A
 Switch to automatic performance adjustment mode (the default).
 .It Fl a
-Display the external charger (A/C status).
+Display the external charger (AC) status.
 0 means disconnected, 1
 means connected, 2 means backup power source, and 255 means unknown.
 .It Fl b
@@ -82,9 +82,10 @@ setting
 .Va hw.setperf
 to 0.
 .It Fl l
-Display the estimated battery lifetime (in percent).
+Display the estimated battery lifetime in percent.
 .It Fl m
-Display the estimated battery lifetime (in minutes).
+Display the estimated battery lifetime in minutes when running on battery,
+or the estimated time to recharge when running on an external charger.
 .It Fl P
 Display the performance adjustment mode.
 0 means manual mode.