Re: include cpuid 0 string in dmesg for fw_update
On Wed, Jul 27, 2022 at 06:47:56AM -0700, Andrew Hewus Fresh wrote:
> On Wed, Jul 27, 2022 at 11:06:39AM +0200, Alexander Hall wrote:
> > I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking
> > for.
>
> As I was falling asleep last night I was trying to figure out how to get
> '*([!$_nl])' into the match and couldn't think of a good one. Your
> solution is much better.
Except for that issue with ksh patterns inside a variable.
I had some time to think about this while doing some construction.
Finally had time to try to implement it and it seems like although it's
a bit ugly, it is performant and seems to allow me to use "^cpu0:*Intel"
in the patterns file and (I believe) just match a line that starts with
cpu0: and contains Intel.
My alpha agrees with the speed assessment, the current implementation
that doesn't support the "*" in the patterns runs at about 5.5 seconds
(as before), while this new version takes 6.5 seconds. That's a lot
better than the 17.5 seconds it took to match against each line (and
still didn't actually expand the *).
Index: fw_update.sh
===
RCS file: /cvs/src/usr.sbin/fw_update/fw_update.sh,v
retrieving revision 1.42
diff -u -p -r1.42 fw_update.sh
--- fw_update.sh20 Feb 2022 21:53:04 - 1.42
+++ fw_update.sh3 Aug 2022 01:11:53 -
@@ -168,21 +168,32 @@ verify() {
}
firmware_in_dmesg() {
- local _d _m _line _dmesgtail _last='' _nl=$( echo )
+ local IFS
+ local _d _m _dmesgtail _last='' _nl='
+'
# The dmesg can contain multiple boots, only look in the last one
_dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}'
/var/run/dmesg.boot )"
grep -v '^[[:space:]]*#' "$FWPATTERNS" |
while read -r _d _m; do
- [ "$_d" = "$_last" ] && continue
+ [ "$_d" = "$_last" ] && continue
[ "$_m" ] || _m="${_nl}${_d}[0-9] at "
[ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
- if [[ $_dmesgtail = *$_m* ]]; then
- echo "$_d"
- _last="$_d"
- fi
+ IFS='*'
+ set -- $_m
+ unset IFS
+
+ case $# in
+ 1) [[ $_dmesgtail = *$** ]] ||
continue;;
+ 2) [[ $_dmesgtail = *$1*([!$_nl])$2* ]] ||
continue;;
+ 3) [[ $_dmesgtail = *$1*([!$_nl])$2*([!$_nl])$3* ]] ||
continue;;
+ *) echo "bad pattern $_m"; exit 1 ;;
+ esac
+
+ echo "$_d"
+ _last="$_d"
done
}
Re: include cpuid 0 string in dmesg for fw_update
On Wed, Jul 27, 2022 at 04:15:36PM +0200, Alexander Hall wrote: > > > On July 27, 2022 3:47:56 PM GMT+02:00, Andrew Hewus Fresh > wrote: > >On Wed, Jul 27, 2022 at 11:06:39AM +0200, Alexander Hall wrote: > >> I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking > >> for. > > > >As I was falling asleep last night I was trying to figure out how to get > >'*([!$_nl])' into the match and couldn't think of a good one. Your > >solution is much better. > > > >I think that would end up looking like: ^cpu0:*([![:cntrl:]])Intel(R) > >although I recall some weirdness with metachars in a variable so I'll > >have to test it out. > > Yeah, it was pointed out to me. I was only able to test on ksh93, so that > might be a dead end. > > /Alexander $ cat u.sh _s="cpu0: gen Intel" _m="cpu0:*([![:cntrl:]])Intel" if [[ $_s = *$_m* ]]; then echo T1 matched fi if [[ $_s = *cpu0:*([![:cntrl:]])Intel* ]]; then echo T2 matched fi $ ksh u.sh T2 matched $ ksh93 u.sh T1 matched T2 matched $ bash u.sh T1 matched T2 matched
Re: include cpuid 0 string in dmesg for fw_update
On July 27, 2022 3:47:56 PM GMT+02:00, Andrew Hewus Fresh wrote: >On Wed, Jul 27, 2022 at 11:06:39AM +0200, Alexander Hall wrote: >> I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking >> for. > >As I was falling asleep last night I was trying to figure out how to get >'*([!$_nl])' into the match and couldn't think of a good one. Your >solution is much better. > >I think that would end up looking like: ^cpu0:*([![:cntrl:]])Intel(R) >although I recall some weirdness with metachars in a variable so I'll >have to test it out. Yeah, it was pointed out to me. I was only able to test on ksh93, so that might be a dead end. /Alexander > >On Wed, Jul 27, 2022 at 12:01:51PM +, Klemens Nanni wrote: >> On Wed, Jul 27, 2022 at 01:55:58PM +0200, Alexander Hall wrote: >> > On a sidenote, the >> > >> > _nl=$(echo) >> > >> > in fw_update.sh is AFAICT deceitful and moot, since ksh strips away >> > trailing linefeeds. Use the uglier >> > >> > _nl=' >> > ' >> > >> > instead. >> >> Correct: >> >> $ _nl=$(echo) >> $ printf '%s' "$_nl" >> $ _nl=' >> > ' >> $ printf '%s' "$_nl" >> >> $ printf '\n' >> >> $ > >This is correct, I swear I had tested it before and that it worked, but >I can't reproduce it now. The switch to per-line was using the ugly >version. > >I'll work up a patch to get that fixed anyway.
Re: include cpuid 0 string in dmesg for fw_update
On Wed, Jul 27, 2022 at 11:06:39AM +0200, Alexander Hall wrote: > I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking > for. As I was falling asleep last night I was trying to figure out how to get '*([!$_nl])' into the match and couldn't think of a good one. Your solution is much better. I think that would end up looking like: ^cpu0:*([![:cntrl:]])Intel(R) although I recall some weirdness with metachars in a variable so I'll have to test it out. On Wed, Jul 27, 2022 at 12:01:51PM +, Klemens Nanni wrote: > On Wed, Jul 27, 2022 at 01:55:58PM +0200, Alexander Hall wrote: > > On a sidenote, the > > > > _nl=$(echo) > > > > in fw_update.sh is AFAICT deceitful and moot, since ksh strips away > > trailing linefeeds. Use the uglier > > > > _nl=' > > ' > > > > instead. > > Correct: > > $ _nl=$(echo) > $ printf '%s' "$_nl" > $ _nl=' > > ' > $ printf '%s' "$_nl" > > $ printf '\n' > > $ This is correct, I swear I had tested it before and that it worked, but I can't reproduce it now. The switch to per-line was using the ugly version. I'll work up a patch to get that fixed anyway.
Re: include cpuid 0 string in dmesg for fw_update
On Wed, Jul 27, 2022 at 01:55:58PM +0200, Alexander Hall wrote: > On a sidenote, the > > _nl=$(echo) > > in fw_update.sh is AFAICT deceitful and moot, since ksh strips away trailing > linefeeds. Use the uglier > > _nl=' > ' > > instead. Correct: $ _nl=$(echo) $ printf '%s' "$_nl" $ _nl=' > ' $ printf '%s' "$_nl" $ printf '\n' $
Re: include cpuid 0 string in dmesg for fw_update
On July 27, 2022 11:06:39 AM GMT+02:00, Alexander Hall wrote: >On July 27, 2022 12:48:29 AM GMT+02:00, Andrew Hewus Fresh > wrote: >>On Sun, Jul 24, 2022 at 10:34:04AM -0700, Andrew Hewus Fresh wrote: >>> On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote: >>> > On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote: >>> > > Jonathan Gray wrote: >>> > > >>> > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote: >>> > > > > Why not match on cpu0: .*Intel >>> > > > >>> > > > I sent a diff a month ago with ^cpu0:*Intel(R) >>> > > > >>> > > > semarie mentioned false positives as it could match another >>> > > > Intel device on another line on a non-Intel CPU >>> > > > >>> > > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2 > >I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking for. *you're On a sidenote, the _nl=$(echo) in fw_update.sh is AFAICT deceitful and moot, since ksh strips away trailing linefeeds. Use the uglier _nl=' ' instead. /Alexander
Re: include cpuid 0 string in dmesg for fw_update
On July 27, 2022 12:48:29 AM GMT+02:00, Andrew Hewus Fresh
wrote:
>On Sun, Jul 24, 2022 at 10:34:04AM -0700, Andrew Hewus Fresh wrote:
>> On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote:
>> > On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote:
>> > > Jonathan Gray wrote:
>> > >
>> > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote:
>> > > > > Why not match on cpu0: .*Intel
>> > > >
>> > > > I sent a diff a month ago with ^cpu0:*Intel(R)
>> > > >
>> > > > semarie mentioned false positives as it could match another
>> > > > Intel device on another line on a non-Intel CPU
>> > > >
>> > > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2
I think replacing '*' with '*([![:cntrl:]])' can be the droid your looking for.
/Alexander
>> > >
>> > > Well, then fw_update should be fixed to not perform the match over
>> > > multiple lines.
>> >
>> > I'm looking at fixing fw_update to match each line instead of the whole
>> > dmesg. I'll try to get a patch out for that today.
>>
>> This patch matches patterns against each line in the dmesg instead of
>> anchoring on newlines and matching against the entire string at once
>> anchored by newlines. This would mean that ^cpu0:*Intel(R) would Just
>> Work.
>>
>> The problem is that it's a little over 3 times slower on my laptop to do
>> the matching this way.
>
>
>As an example, on my alpha (that I admit to only keeping around because
>a 166MHz single processor machine that runs OpenBSD is useful for
>testing how things work on a slow machine), the current code runs and
>doesn't find any firmware in 5.5 seconds and with this patch it now
>takes 17.5 seconds.
>
>
>> If any ksh folks have tricks to speed that up,
>> I'd appreciate it. I'll try to think about whether I can build a sed
>> program that will spit out matches. Sadly at the moment, distracted by
>> a non-computer project that has been taking up all my free time.
>> Hopefully that will be finished in the next couple weeks though.
>>
>> I think I could do some magic to replace the "^cpu:*Intel(R)" above with
>> "^cpu:*([!\n])Intel(R)" (although not with directly as ksh wouldn't
>> recognize it). That would be annoying to implement though, but would
>> then still be able to do the faster single match.
>>
>>
>> --- fw_update.sh.origSun Jul 24 10:07:40 2022
>> +++ fw_update.sh Sun Jul 24 10:14:21 2022
>> @@ -168,21 +168,31 @@
>> }
>>
>> firmware_in_dmesg() {
>> -local _d _m _line _dmesgtail _last='' _nl=$( echo )
>> +local _d _m _line _dmesgtail _last='' _oldifs="$IFS"
>>
>> +IFS='
>> +'
>> # The dmesg can contain multiple boots, only look in the last one
>> -_dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}'
>> /var/run/dmesg.boot )"
>> +set -A _dmesgtail $( sed -n 'H;/^OpenBSD/h;${g;p;}' /var/run/dmesg.boot
>> )
>> +IFS="$_oldifs"
>>
>> grep -v '^[[:space:]]*#' "$FWPATTERNS" |
>> while read -r _d _m; do
>> [ "$_d" = "$_last" ] && continue
>> -[ "$_m" ] || _m="${_nl}${_d}[0-9] at "
>> -[ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
>> -
>> -if [[ $_dmesgtail = *$_m* ]]; then
>> -echo "$_d"
>> -_last="$_d"
>> +[ "$_m" ] || _m="^${_d}[0-9] at "
>> +if [ "$_m" = "${_m#^}" ]; then
>> +_m="*$_m"
>> +else
>> +_m="${_m#^}"
>> fi
>> +
>> +for _line in "${_dmesgtail[@]}"; do
>> +if [[ $_line = $_m* ]]; then
>> +echo "$_d"
>> +_last="$_d"
>> +break
>> +fi
>> +done
>> done
>> }
>>
>>
>
Re: include cpuid 0 string in dmesg for fw_update
On Sun, Jul 24, 2022 at 10:34:04AM -0700, Andrew Hewus Fresh wrote:
> On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote:
> > On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote:
> > > Jonathan Gray wrote:
> > >
> > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote:
> > > > > Why not match on cpu0: .*Intel
> > > >
> > > > I sent a diff a month ago with ^cpu0:*Intel(R)
> > > >
> > > > semarie mentioned false positives as it could match another
> > > > Intel device on another line on a non-Intel CPU
> > > >
> > > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2
> > >
> > > Well, then fw_update should be fixed to not perform the match over
> > > multiple lines.
> >
> > I'm looking at fixing fw_update to match each line instead of the whole
> > dmesg. I'll try to get a patch out for that today.
>
> This patch matches patterns against each line in the dmesg instead of
> anchoring on newlines and matching against the entire string at once
> anchored by newlines. This would mean that ^cpu0:*Intel(R) would Just
> Work.
>
> The problem is that it's a little over 3 times slower on my laptop to do
> the matching this way.
As an example, on my alpha (that I admit to only keeping around because
a 166MHz single processor machine that runs OpenBSD is useful for
testing how things work on a slow machine), the current code runs and
doesn't find any firmware in 5.5 seconds and with this patch it now
takes 17.5 seconds.
> If any ksh folks have tricks to speed that up,
> I'd appreciate it. I'll try to think about whether I can build a sed
> program that will spit out matches. Sadly at the moment, distracted by
> a non-computer project that has been taking up all my free time.
> Hopefully that will be finished in the next couple weeks though.
>
> I think I could do some magic to replace the "^cpu:*Intel(R)" above with
> "^cpu:*([!\n])Intel(R)" (although not with directly as ksh wouldn't
> recognize it). That would be annoying to implement though, but would
> then still be able to do the faster single match.
>
>
> --- fw_update.sh.orig Sun Jul 24 10:07:40 2022
> +++ fw_update.sh Sun Jul 24 10:14:21 2022
> @@ -168,21 +168,31 @@
> }
>
> firmware_in_dmesg() {
> - local _d _m _line _dmesgtail _last='' _nl=$( echo )
> + local _d _m _line _dmesgtail _last='' _oldifs="$IFS"
>
> + IFS='
> +'
> # The dmesg can contain multiple boots, only look in the last one
> - _dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}'
> /var/run/dmesg.boot )"
> + set -A _dmesgtail $( sed -n 'H;/^OpenBSD/h;${g;p;}' /var/run/dmesg.boot
> )
> + IFS="$_oldifs"
>
> grep -v '^[[:space:]]*#' "$FWPATTERNS" |
> while read -r _d _m; do
> [ "$_d" = "$_last" ] && continue
> - [ "$_m" ] || _m="${_nl}${_d}[0-9] at "
> - [ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
> -
> - if [[ $_dmesgtail = *$_m* ]]; then
> - echo "$_d"
> - _last="$_d"
> + [ "$_m" ] || _m="^${_d}[0-9] at "
> + if [ "$_m" = "${_m#^}" ]; then
> + _m="*$_m"
> + else
> + _m="${_m#^}"
> fi
> +
> + for _line in "${_dmesgtail[@]}"; do
> + if [[ $_line = $_m* ]]; then
> + echo "$_d"
> + _last="$_d"
> + break
> + fi
> + done
> done
> }
>
>
--
andrew
A printer consists of three main parts:
the case, the jammed paper tray and the blinking red light.
Re: include cpuid 0 string in dmesg for fw_update
On Sun, Jul 24, 2022 at 10:34:04AM -0700, Andrew Hewus Fresh wrote:
> I'll try to think about whether I can build a sed program that will
> spit out matches.
Unfortunately the sed version seems to be even slower than matching one
line at a time. Just building the patterns for sed is slower than the
original single match.
--- fw_update.sh.orig Sun Jul 24 10:07:40 2022
+++ fw_update.shSun Jul 24 20:50:02 2022
@@ -168,22 +168,34 @@
}
firmware_in_dmesg() {
- local _d _m _line _dmesgtail _last='' _nl=$( echo )
+ local _cmd
- # The dmesg can contain multiple boots, only look in the last one
- _dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}'
/var/run/dmesg.boot )"
-
- grep -v '^[[:space:]]*#' "$FWPATTERNS" |
+ _cmd="$( grep -v '^[[:space:]]*#' "$FWPATTERNS" | (
+ _s='/^%s/i\\\n%s\n'
+ _last=
+ _patterns=
while read -r _d _m; do
- [ "$_d" = "$_last" ] && continue
- [ "$_m" ] || _m="${_nl}${_d}[0-9] at "
- [ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
+ [ "$_m" ] || _m="^${_d}[0-9] at "
+ [ "$_m" = "${_m#*/}" ] || _m="$( printf "$_m" | sed 's,/,\\/,g'
)"
- if [[ $_dmesgtail = *$_m* ]]; then
- echo "$_d"
- _last="$_d"
+ if [ "$_patterns" ] && [ "$_d" != "$_last" ]; then
+ printf "$_s" "$_patterns" "$_last"
+ _last=
+ _patterns=
fi
+
+ _last="$_d"
+ if [ "$_patterns" ]; then
+ _patterns="$_patterns|$_m"
+ else
+ _patterns="$_m"
+ fi
done
+ [ "$_patterns" ] && printf "$_s" "$_patterns" "$_last"
+ ) )"
+
+ # The dmesg can contain multiple boots, only look in the last one
+ sed -n 'H;/^OpenBSD/h;${g;p;}' /var/run/dmesg.boot | sed -En "$_cmd"
}
firmware_filename() {
Re: include cpuid 0 string in dmesg for fw_update
On 2022/07/24 10:34, Andrew Hewus Fresh wrote: > On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote: > > On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote: > > > Jonathan Gray wrote: > > > > > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote: > > > > > Why not match on cpu0: .*Intel > > > > > > > > I sent a diff a month ago with ^cpu0:*Intel(R) > > > > > > > > semarie mentioned false positives as it could match another > > > > Intel device on another line on a non-Intel CPU > > > > > > > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2 Why bother about false positives? It doesn't really hurt, it's only 6MB.
Re: include cpuid 0 string in dmesg for fw_update
On Sun, Jul 24, 2022 at 09:14:30AM -0700, Andrew Hewus Fresh wrote:
> On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote:
> > Jonathan Gray wrote:
> >
> > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote:
> > > > Why not match on cpu0: .*Intel
> > >
> > > I sent a diff a month ago with ^cpu0:*Intel(R)
> > >
> > > semarie mentioned false positives as it could match another
> > > Intel device on another line on a non-Intel CPU
> > >
> > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2
> >
> > Well, then fw_update should be fixed to not perform the match over
> > multiple lines.
>
> I'm looking at fixing fw_update to match each line instead of the whole
> dmesg. I'll try to get a patch out for that today.
This patch matches patterns against each line in the dmesg instead of
anchoring on newlines and matching against the entire string at once
anchored by newlines. This would mean that ^cpu0:*Intel(R) would Just
Work.
The problem is that it's a little over 3 times slower on my laptop to do
the matching this way. If any ksh folks have tricks to speed that up,
I'd appreciate it. I'll try to think about whether I can build a sed
program that will spit out matches. Sadly at the moment, distracted by
a non-computer project that has been taking up all my free time.
Hopefully that will be finished in the next couple weeks though.
I think I could do some magic to replace the "^cpu:*Intel(R)" above with
"^cpu:*([!\n])Intel(R)" (although not with directly as ksh wouldn't
recognize it). That would be annoying to implement though, but would
then still be able to do the faster single match.
--- fw_update.sh.orig Sun Jul 24 10:07:40 2022
+++ fw_update.shSun Jul 24 10:14:21 2022
@@ -168,21 +168,31 @@
}
firmware_in_dmesg() {
- local _d _m _line _dmesgtail _last='' _nl=$( echo )
+ local _d _m _line _dmesgtail _last='' _oldifs="$IFS"
+ IFS='
+'
# The dmesg can contain multiple boots, only look in the last one
- _dmesgtail="$( echo ; sed -n 'H;/^OpenBSD/h;${g;p;}'
/var/run/dmesg.boot )"
+ set -A _dmesgtail $( sed -n 'H;/^OpenBSD/h;${g;p;}' /var/run/dmesg.boot
)
+ IFS="$_oldifs"
grep -v '^[[:space:]]*#' "$FWPATTERNS" |
while read -r _d _m; do
[ "$_d" = "$_last" ] && continue
- [ "$_m" ] || _m="${_nl}${_d}[0-9] at "
- [ "$_m" = "${_m#^}" ] || _m="${_nl}${_m#^}"
-
- if [[ $_dmesgtail = *$_m* ]]; then
- echo "$_d"
- _last="$_d"
+ [ "$_m" ] || _m="^${_d}[0-9] at "
+ if [ "$_m" = "${_m#^}" ]; then
+ _m="*$_m"
+ else
+ _m="${_m#^}"
fi
+
+ for _line in "${_dmesgtail[@]}"; do
+ if [[ $_line = $_m* ]]; then
+ echo "$_d"
+ _last="$_d"
+ break
+ fi
+ done
done
}
Re: include cpuid 0 string in dmesg for fw_update
On Sun, Jul 24, 2022 at 10:01:26AM -0600, Theo de Raadt wrote: > Jonathan Gray wrote: > > > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote: > > > Why not match on cpu0: .*Intel > > > > I sent a diff a month ago with ^cpu0:*Intel(R) > > > > semarie mentioned false positives as it could match another > > Intel device on another line on a non-Intel CPU > > > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2 > > Well, then fw_update should be fixed to not perform the match over > multiple lines. I'm looking at fixing fw_update to match each line instead of the whole dmesg. I'll try to get a patch out for that today. > Absolutely noone expects a pattern containing '^' to match over > multiple lines. It is only a matter of time before another pattern > acts wrong, so I think fw_update should get a proper line-matcher > so that the pattern ^cpu0: *Intel' can be used. Agreed.
Re: include cpuid 0 string in dmesg for fw_update
Jonathan Gray wrote: > On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote: > > Why not match on cpu0: .*Intel > > I sent a diff a month ago with ^cpu0:*Intel(R) > > semarie mentioned false positives as it could match another > Intel device on another line on a non-Intel CPU > > https://marc.info/?l=openbsd-tech&m=165579653107768&w=2 Well, then fw_update should be fixed to not perform the match over multiple lines. Absolutely noone expects a pattern containing '^' to match over multiple lines. It is only a matter of time before another pattern acts wrong, so I think fw_update should get a proper line-matcher so that the pattern ^cpu0: *Intel' can be used.
Re: include cpuid 0 string in dmesg for fw_update
On Sun, Jul 24, 2022 at 08:05:26AM -0600, Theo de Raadt wrote: > Why not match on cpu0: .*Intel I sent a diff a month ago with ^cpu0:*Intel(R) semarie mentioned false positives as it could match another Intel device on another line on a non-Intel CPU https://marc.info/?l=openbsd-tech&m=165579653107768&w=2
Re: include cpuid 0 string in dmesg for fw_update
> Date: Sun, 24 Jul 2022 15:48:26 +1000
> From: Jonathan Gray
>
> include cpuid 0 string in dmesg for fw_update
>
> Intel CPUs used to have brand strings such as
> cpu0: Intel(R) Pentium(R) M processor 1200MHz ("GenuineIntel" 686-class) 1.20
> GHz
> cpu0: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz, 2494.61 MHz, 06-3d-04
> recent CPUs use
> cpu0: 11th Gen Intel(R) Core(TM) i5-1130G7 @ 1.10GHz, 30009.37 MHz, 06-8c-01
> cpu0: 12th Gen Intel(R) Core(TM) i5-12400, 4390.71 MHz, 06-97-02
> cpu0: 12th Gen Intel(R) Core(TM) i7-1260P, 1995.55 MHz, 06-9a-03
>
> after the diff:
>
> amd64:
> cpu0: GenuineIntel, 12th Gen Intel(R) Core(TM) i7-1260P, 1995.55 MHz, 06-9a-03
> cpu0: AuthenticAMD, AMD Ryzen 5 2600X Six-Core Processor, 3593.83 MHz,
> 17-08-02
>
> i386:
> cpu0: GenuineIntel, Intel(R) Pentium(R) M processor 1.60GHz (686-class) 1.60
> GHz, 06-0d-06
>
> changes the output of hw.model on i386 to not include the cpuid 0 string
Ugh, this is ugly.
> Index: sys/arch/amd64/amd64/identcpu.c
> ===
> RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
> retrieving revision 1.125
> diff -u -p -r1.125 identcpu.c
> --- sys/arch/amd64/amd64/identcpu.c 12 Jul 2022 04:46:00 - 1.125
> +++ sys/arch/amd64/amd64/identcpu.c 24 Jul 2022 05:14:07 -
> @@ -516,6 +516,11 @@ identifycpu(struct cpu_info *ci)
> int i;
> char *brandstr_from, *brandstr_to;
> int skipspace;
> + uint32_t vendor_regs[3];
> + char vendor[13];
> +
> + CPUID(0, dummy, vendor_regs[0], vendor_regs[2], vendor_regs[1]);
> + strlcpy(vendor, (char *)vendor_regs, sizeof(vendor));
>
> CPUID(1, ci->ci_signature, val, dummy, ci->ci_feature_flags);
> CPUID(0x8000, ci->ci_pnfeatset, dummy, dummy, dummy);
> @@ -605,7 +610,7 @@ identifycpu(struct cpu_info *ci)
>
> freq = cpu_freq(ci);
>
> - printf("%s: %s", ci->ci_dev->dv_xname, mycpu_model);
> + printf("%s: %s, %s", ci->ci_dev->dv_xname, vendor, mycpu_model);
>
> if (freq != 0)
> printf(", %llu.%02llu MHz", (freq + 4999) / 100,
> Index: sys/arch/i386/i386/machdep.c
> ===
> RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
> retrieving revision 1.650
> diff -u -p -r1.650 machdep.c
> --- sys/arch/i386/i386/machdep.c 12 Jul 2022 05:45:49 - 1.650
> +++ sys/arch/i386/i386/machdep.c 24 Jul 2022 04:45:20 -
> @@ -1891,19 +1891,18 @@ identifycpu(struct cpu_info *ci)
>
> if (cachesize > -1) {
> snprintf(cpu_model, sizeof(cpu_model),
> - "%s (%s%s%s%s-class, %dKB L2 cache)",
> - cpu_brandstr,
> - ((*token) ? "\"" : ""), ((*token) ? token : ""),
> - ((*token) ? "\" " : ""), classnames[class], cachesize);
> + "%s (%s-class, %dKB L2 cache)",
> + cpu_brandstr, classnames[class], cachesize);
> } else {
> snprintf(cpu_model, sizeof(cpu_model),
> - "%s (%s%s%s%s-class)",
> - cpu_brandstr,
> - ((*token) ? "\"" : ""), ((*token) ? token : ""),
> - ((*token) ? "\" " : ""), classnames[class]);
> + "%s (%s-class)",
> + cpu_brandstr, classnames[class]);
> }
>
> - printf("%s: %s", cpu_device, cpu_model);
> + if (*token)
> + printf("%s: %s, %s", cpu_device, token, cpu_model);
> + else
> + printf("%s: %s", cpu_device, cpu_model);
>
> if (ci->ci_feature_flags && (ci->ci_feature_flags & CPUID_TSC)) {
> /* Has TSC, check if it's constant */
> Index: usr.sbin/fw_update/patterns.c
> ===
> RCS file: /cvs/src/usr.sbin/fw_update/patterns.c,v
> retrieving revision 1.3
> diff -u -p -r1.3 patterns.c
> --- usr.sbin/fw_update/patterns.c 10 Mar 2022 07:12:13 - 1.3
> +++ usr.sbin/fw_update/patterns.c 24 Jul 2022 05:36:11 -
> @@ -94,7 +94,7 @@ main(void)
> printf("%s\n", "bwfm");
> printf("%s\n", "bwi");
> printf("%s\n", "intel");
> - printf("%s\n", "intel ^cpu0: Intel");
> + printf("%s\n", "intel ^cpu0: GenuineIntel");
> printf("%s\n", "inteldrm");
> print_devices("inteldrm", i915_devices, nitems(i915_devices));
> printf("%s\n", "ipw");
>
>
Re: include cpuid 0 string in dmesg for fw_update
Why not match on cpu0: .*Intel
include cpuid 0 string in dmesg for fw_update
include cpuid 0 string in dmesg for fw_update
Intel CPUs used to have brand strings such as
cpu0: Intel(R) Pentium(R) M processor 1200MHz ("GenuineIntel" 686-class) 1.20
GHz
cpu0: Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz, 2494.61 MHz, 06-3d-04
recent CPUs use
cpu0: 11th Gen Intel(R) Core(TM) i5-1130G7 @ 1.10GHz, 30009.37 MHz, 06-8c-01
cpu0: 12th Gen Intel(R) Core(TM) i5-12400, 4390.71 MHz, 06-97-02
cpu0: 12th Gen Intel(R) Core(TM) i7-1260P, 1995.55 MHz, 06-9a-03
after the diff:
amd64:
cpu0: GenuineIntel, 12th Gen Intel(R) Core(TM) i7-1260P, 1995.55 MHz, 06-9a-03
cpu0: AuthenticAMD, AMD Ryzen 5 2600X Six-Core Processor, 3593.83 MHz, 17-08-02
i386:
cpu0: GenuineIntel, Intel(R) Pentium(R) M processor 1.60GHz (686-class) 1.60
GHz, 06-0d-06
changes the output of hw.model on i386 to not include the cpuid 0 string
Index: sys/arch/amd64/amd64/identcpu.c
===
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.125
diff -u -p -r1.125 identcpu.c
--- sys/arch/amd64/amd64/identcpu.c 12 Jul 2022 04:46:00 - 1.125
+++ sys/arch/amd64/amd64/identcpu.c 24 Jul 2022 05:14:07 -
@@ -516,6 +516,11 @@ identifycpu(struct cpu_info *ci)
int i;
char *brandstr_from, *brandstr_to;
int skipspace;
+ uint32_t vendor_regs[3];
+ char vendor[13];
+
+ CPUID(0, dummy, vendor_regs[0], vendor_regs[2], vendor_regs[1]);
+ strlcpy(vendor, (char *)vendor_regs, sizeof(vendor));
CPUID(1, ci->ci_signature, val, dummy, ci->ci_feature_flags);
CPUID(0x8000, ci->ci_pnfeatset, dummy, dummy, dummy);
@@ -605,7 +610,7 @@ identifycpu(struct cpu_info *ci)
freq = cpu_freq(ci);
- printf("%s: %s", ci->ci_dev->dv_xname, mycpu_model);
+ printf("%s: %s, %s", ci->ci_dev->dv_xname, vendor, mycpu_model);
if (freq != 0)
printf(", %llu.%02llu MHz", (freq + 4999) / 100,
Index: sys/arch/i386/i386/machdep.c
===
RCS file: /cvs/src/sys/arch/i386/i386/machdep.c,v
retrieving revision 1.650
diff -u -p -r1.650 machdep.c
--- sys/arch/i386/i386/machdep.c12 Jul 2022 05:45:49 - 1.650
+++ sys/arch/i386/i386/machdep.c24 Jul 2022 04:45:20 -
@@ -1891,19 +1891,18 @@ identifycpu(struct cpu_info *ci)
if (cachesize > -1) {
snprintf(cpu_model, sizeof(cpu_model),
- "%s (%s%s%s%s-class, %dKB L2 cache)",
- cpu_brandstr,
- ((*token) ? "\"" : ""), ((*token) ? token : ""),
- ((*token) ? "\" " : ""), classnames[class], cachesize);
+ "%s (%s-class, %dKB L2 cache)",
+ cpu_brandstr, classnames[class], cachesize);
} else {
snprintf(cpu_model, sizeof(cpu_model),
- "%s (%s%s%s%s-class)",
- cpu_brandstr,
- ((*token) ? "\"" : ""), ((*token) ? token : ""),
- ((*token) ? "\" " : ""), classnames[class]);
+ "%s (%s-class)",
+ cpu_brandstr, classnames[class]);
}
- printf("%s: %s", cpu_device, cpu_model);
+ if (*token)
+ printf("%s: %s, %s", cpu_device, token, cpu_model);
+ else
+ printf("%s: %s", cpu_device, cpu_model);
if (ci->ci_feature_flags && (ci->ci_feature_flags & CPUID_TSC)) {
/* Has TSC, check if it's constant */
Index: usr.sbin/fw_update/patterns.c
===
RCS file: /cvs/src/usr.sbin/fw_update/patterns.c,v
retrieving revision 1.3
diff -u -p -r1.3 patterns.c
--- usr.sbin/fw_update/patterns.c 10 Mar 2022 07:12:13 - 1.3
+++ usr.sbin/fw_update/patterns.c 24 Jul 2022 05:36:11 -
@@ -94,7 +94,7 @@ main(void)
printf("%s\n", "bwfm");
printf("%s\n", "bwi");
printf("%s\n", "intel");
- printf("%s\n", "intel ^cpu0: Intel");
+ printf("%s\n", "intel ^cpu0: GenuineIntel");
printf("%s\n", "inteldrm");
print_devices("inteldrm", i915_devices, nitems(i915_devices));
printf("%s\n", "ipw");
