Re: Add ability to set control values with video(1)
On Sat, 25 Jul 2020 18:27:45 +0100 Laurence Tratt wrote: > On Sat, Jul 25, 2020 at 10:27:15AM -0600, Theo de Raadt wrote: > > Hello Theo, > > > My primary concern is about a user changing settings which then > > persist past close. > > > > Upon re-open, how do they know what mode they are in? > > > > I understand the default mode for a camera might not be nice. But > > at least it is a default. If the previous use of the camera put it > > into a nasty mode, does this mean all new users must first force > > default? > > > > Now you don't know what mode it is in. As a result, you must > > *always* demand change into a specific mode. Rather than making > > things simpler, doesn't use of a camera become potentially more > > complicated? > > From what I can tell, there are two ways to control a uvideo device: > > 1) The "semi-persistent" state changes that video(1) can make that > affects subsequent apps which access the device. My patch simply > makes those state changes possible from the command-line instead of > forcing the user to open a video and hold down keys until they reach > the desired state. In otReset all supported controls to their default > value and quit.her words, it doesn't change how you control the > device: "-c reset" is equivalent to running video(1) and pressing > "r", for example. > > 2) Control via a loopback device. For example, on Windows, Logitech > allow you to change controls in their app where you can see video; > they then expose a second internal device which other apps can use; I > think controls are reset when the Logitech app is closed. > > On Linux I believe v4l2-ctl works as video(1) does (semi-persistent > state changes) but Linux also has video loopback devices. Presumably > they could do something similar to the Logitech Windows app, but I > don't know if they do so or not. > > Unless we develop a loopback facility (or, perhaps, some sort of > uvideo daemon roughly equivalent to sndiod), I don't think we have > much choice but to continue with the semi-persistent state changes > that video(1) has always been capable of. It is a bit icky, but it's > the only way, for example, to change a webcam's brightness before > taking a video call in a web browser. OK - Let me try to pick this thread up once more :-) Lets assume we leave the current behaviour of video(1) as is, which doesn't reset back the default control values on device close. This adapted patch adds two new options and the ability to set multiple control values on the CLI with a similar interface as sysctl(8) has: * c: List all current supported control values and quit (-q is already occupied). * d: Reset all supported controls to their default value and quit (-r is already occupied). -> This does the same thing as when pressing 'r' in the GUI. Some examples: $ doas video -c brightness=128 contrast=32 saturation=64 hue=0 gamma=120 sharpness=2 white_balance_temperature=4000 $ $ doas video brightness=200 sharpness=4 brightness: 128 -> 200 sharpness: 2 -> 4 $ $ doas video brightness brightness: 200 $ $ doas video -c brightness=200 contrast=32 saturation=64 hue=0 gamma=120 sharpness=4 white_balance_temperature=4000 $ $ doas video -d $ $ doas video -c brightness=128 contrast=32 saturation=64 hue=0 gamma=120 sharpness=2 white_balance_temperature=4000 $ $ doas video -f /dev/video1 gain=1 gain: 0 -> 1 $ $ doas video -f /dev/video1 -c brightness=128 contrast=128 saturation=128 gain=1 sharpness=128 white_balance_temperature=4000 $ What we could think about optionally is to reset to the default control values on device close when we did use the GUI mode, but leave the controls sticky when we set them through the CLI, and explicitly document that in the video(1) man page for the [control[=value]] arguments. But entirely removing the sticky controls I think is odd since as already mentioned before, it will remove the ability to preset any controls before we enter an application where we can't change them, like in an browser. And web conference calls are pretty common nowadays ... Index: video.1 === RCS file: /cvs/xenocara/app/video/video.1,v retrieving revision 1.15 diff -u -p -u -p -r1.15 video.1 --- video.1 17 Jul 2020 07:51:23 - 1.15 +++ video.1 29 Jul 2020 05:02:51 - @@ -25,7 +25,7 @@ .Sh SYNOPSIS .Nm .Bk -words -.Op Fl \ +.Op Fl \ .Op Fl a Ar adaptor .Op Fl e Ar encoding .Op Fl f Ar file @@ -34,6 +34,7 @@ .Op Fl o Ar output .Op Fl r Ar rate .Op Fl s Ar size +.Op Ar control Ns Op = Ns Ar value .Ek .Sh
Re: ifconfig: remove redundant bridge checks
ok.
> On 29 Jul 2020, at 11:38, Klemens Nanni wrote:
>
> On Tue, Jul 28, 2020 at 07:09:17PM +0200, Klemens Nanni wrote:
>> bridge_status() and switch_status() do the regular sanity check with
>> SIOCGIFFLAGS, but both functions also call is_switch(), bridge_status()
>> also calls is_bridge().
>>
>> Those is_*() helpers do the same SIOCGIFFLAGS sanity check, making those
>> in *_status() entirely redundant, so I'd like to remove them.
> Small correction: is_bridge() does SIOCGIFFLAGS, is_switch() does not.
>
> Below is a new diff that removes the SIOCGIFFLAGS check form is_bridge()
> as well, leaving the two is_*() helpers to their driver specific ioctls
> alone.
>
>> I'm here since the tpmr(4) ioctl interface transition from trunk to
>> bridge semantics is now complete, so ifconfig(8) now requires tpmr bits
>> to show its members in bridge fashion.
>>
>> One way would be duplicate code into is_tpmr() and tpmr_status() which
>> I've already done, but another approach is to unify all bridge like
>> interfaces under bridge_status().
> With this in, merging switch_status() into bridge_status() is a trivial
> diff, adding tpmr awareness to the mix would then be another diff after
> that.
>
>> Either ways, diff below cleans up and makes for simpler code.
> So this effectively just removes SIOCGIFFLAGS in brconfig.c which are of
> no use, imho. ifconfig.c:getinfo() already checks interfaces flags,
> even more than once, for all interfaces.
>
>> Feedback? OK?
>
>
> Index: brconfig.c
> ===
> RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v
> retrieving revision 1.25
> diff -u -p -r1.25 brconfig.c
> --- brconfig.c22 Jan 2020 06:24:07 - 1.25
> +++ brconfig.c29 Jul 2020 00:58:40 -
> @@ -762,14 +762,8 @@ bridge_holdcnt(const char *value, int d)
> int
> is_bridge()
> {
> - struct ifreq ifr;
> struct ifbaconf ifbac;
>
> - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> -
> - if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
> - return (0);
> -
> ifbac.ifbac_len = 0;
> strlcpy(ifbac.ifbac_name, ifname, sizeof(ifbac.ifbac_name));
> if (ioctl(sock, SIOCBRDGRTS, (caddr_t)) == -1) {
> @@ -783,16 +777,11 @@ is_bridge()
> void
> bridge_status(void)
> {
> - struct ifreq ifr;
> struct ifbrparam bp1, bp2;
>
> if (!is_bridge() || is_switch())
> return;
>
> - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> - if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
> - return;
> -
> bridge_cfg("\t");
>
> bridge_list("\t");
> @@ -1184,13 +1173,7 @@ switch_cfg(char *delim)
> void
> switch_status(void)
> {
> - struct ifreq ifr;
> -
> if (!is_switch())
> - return;
> -
> - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
> - if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
> return;
>
> switch_cfg("\t");
Re: ifconfig: remove redundant bridge checks
On Tue, Jul 28, 2020 at 07:09:17PM +0200, Klemens Nanni wrote:
> bridge_status() and switch_status() do the regular sanity check with
> SIOCGIFFLAGS, but both functions also call is_switch(), bridge_status()
> also calls is_bridge().
>
> Those is_*() helpers do the same SIOCGIFFLAGS sanity check, making those
> in *_status() entirely redundant, so I'd like to remove them.
Small correction: is_bridge() does SIOCGIFFLAGS, is_switch() does not.
Below is a new diff that removes the SIOCGIFFLAGS check form is_bridge()
as well, leaving the two is_*() helpers to their driver specific ioctls
alone.
> I'm here since the tpmr(4) ioctl interface transition from trunk to
> bridge semantics is now complete, so ifconfig(8) now requires tpmr bits
> to show its members in bridge fashion.
>
> One way would be duplicate code into is_tpmr() and tpmr_status() which
> I've already done, but another approach is to unify all bridge like
> interfaces under bridge_status().
With this in, merging switch_status() into bridge_status() is a trivial
diff, adding tpmr awareness to the mix would then be another diff after
that.
> Either ways, diff below cleans up and makes for simpler code.
So this effectively just removes SIOCGIFFLAGS in brconfig.c which are of
no use, imho. ifconfig.c:getinfo() already checks interfaces flags,
even more than once, for all interfaces.
> Feedback? OK?
Index: brconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v
retrieving revision 1.25
diff -u -p -r1.25 brconfig.c
--- brconfig.c 22 Jan 2020 06:24:07 - 1.25
+++ brconfig.c 29 Jul 2020 00:58:40 -
@@ -762,14 +762,8 @@ bridge_holdcnt(const char *value, int d)
int
is_bridge()
{
- struct ifreq ifr;
struct ifbaconf ifbac;
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
-
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
- return (0);
-
ifbac.ifbac_len = 0;
strlcpy(ifbac.ifbac_name, ifname, sizeof(ifbac.ifbac_name));
if (ioctl(sock, SIOCBRDGRTS, (caddr_t)) == -1) {
@@ -783,16 +777,11 @@ is_bridge()
void
bridge_status(void)
{
- struct ifreq ifr;
struct ifbrparam bp1, bp2;
if (!is_bridge() || is_switch())
return;
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
- return;
-
bridge_cfg("\t");
bridge_list("\t");
@@ -1184,13 +1173,7 @@ switch_cfg(char *delim)
void
switch_status(void)
{
- struct ifreq ifr;
-
if (!is_switch())
- return;
-
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
return;
switch_cfg("\t");
Edgerouter 4 available for any OpenBSD dev that needs an octeon
Someone (can't recall who) gave me an ER4. I found it while cleaning out my closet. Since I'm not active anymore, if any openbsd developer wants it, reach out to me privately and I'll see about sending it to you. Thanks. -ml
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 07:30:36PM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 21:42:46 +1000
> > From: Jonathan Matthew
> >
> > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > From: Jonathan Matthew
> > > >
> > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > From: Mark Kettenis
> > > > > >
> > > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout
> > > > > > of
> > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > > diff tries to support machines with firmware that implements this.
> > > > > > If
> > > > > > you see something like:
> > > > > >
> > > > > > "ACPI0007" at acpi0 not configured
> > > > > >
> > > > > > please try the following diff and report back with an updated dmesg.
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Mark
> > > > >
> > > > > And now with the right diff...
> > > >
> > > > On a dell r6415, it looks like this:
> > > >
> > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > all the way up to
> > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > >
> > > > which I guess means aml_copyvalue() needs to learn how to copy
> > > > AML_OBJTYPE_DEVICE.
> > >
> > > Yes. It is not immediately obvious how this should work. Do we need
> > > to copy the aml_node pointer or not? We don't do that for
> > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > > carry any additional information. So we end up with just an empty
> > > case to avoid the warning.
> > >
> > > Does this work on the Dell machines?
> >
> > We've seen crashes in pool_cache_get() in various places after all the
> > acpicpus
> > attach, which we haven't seen before on these machines, so I think it's
> > corrupting memory somehow.
>
> Does that happen with only the acpicpu(4) diff?
Yes. Looking at this a bit more, in the case where aml_evalnode() can't
copy the result value, it leaves it uninitialised, which means we'll call
aml_freevalue() where res is stack junk. memset(, 0, sizeof(res))
seems to fix it.
>
> > With this addition, we get this for each cpu:
> > acpicpu0 at acpi0: C1(@1 halt!)
>
> The exclamation mark indicates that this is the "fallback" C-state.
> Is there a _CST method at all?
>
> Anyway, given that this is a server system, it isn't really surprising
> that there isn't any fancy power saving stuff.
Right, there doesn't seem to be any. The processor devices look like this
in the aml:
Scope (_SB)
{
Device (C000)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x00) // _UID: Unique ID
}
Device (C001)
{
Name (_HID, "ACPI0007" /* Processor Device */) // _HID: Hardware ID
Name (_UID, 0x01) // _UID: Unique ID
}
.. and so on.
>
> > > Index: dev/acpi/dsdt.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > retrieving revision 1.252
> > > diff -u -p -r1.252 dsdt.c
> > > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > > lhs->v_objref = rhs->v_objref;
> > > aml_addref(lhs->v_objref.ref, "");
> > > break;
> > > + case AML_OBJTYPE_DEVICE:
> > > + break;
> > > default:
> > > printf("copyvalue: %x", rhs->type);
> > > break;
> >
> >
Re: NET_LOCK and trunk detach
> On 29 Jul 2020, at 00:09, sven falempin wrote: > > On Tue, Jul 28, 2020 at 4:42 PM Vitaliy Makkoveev wrote: >> >> On Tue, Jul 28, 2020 at 04:10:01PM -0400, sven falempin wrote: >>> Hello, >>> >>> I am running some trunk interfaces in a multi core environment, >>> it's a slightly modified version, i have a few NET_ASSERT_LOCKED(); >>> suspecting some multi core shenanigans, which i guess was confirmed: >>> (unsure the have X meaning, but i ' m pretty sure 256 is very wrong) >>> the if_trunk.c locking is completely unmodified >>> The code is 6.7-stable >>> >>> splassert: lacp_detach: want 2 have 0 >>> splassert: lacp_detach: want 2 have 0 >>> splassert: lacp_detach: want 2 have 256 >>> >>> I noticed : trunk_clone_destroy ,call >>> >>>if (tr->tr_proto != TRUNK_PROTO_NONE) >>>tr->tr_detach(tr); >>> >>> outside the lock, and that trunk_ioctl call it >>> >>>if (tr->tr_proto != TRUNK_PROTO_NONE) >>>error = tr->tr_detach(tr); >>> >>> but ioctl is as far as i understand locked. >>> I'm unsure if the difficult and amazing unlocking work >>> did an oopsies or if ioctl is already assumed unlocked. >>> >>> Kindly inform me. >>> Best regards, thank you for reading. >>> >> >> lacp_detach() touches nothing which requires NET_LOCK(). What is the >> reason you placed assertion to lacp_detach()? > > <> > > the lacp_detach is not yours, i put them here because i have a NULL pointer > popping in other 'driver' callback. > > I'm tracking this and trying to understand how this memory is 'nullified' > mid function. I have no telepathy skills. Sorry. If you have panics send please dmesg(8) output and instruction for reproduce. > > I do not think putting NET_ASSERT_LOCKED can be harmful in any way. > If so please tell me. > What is the data you think needs be protected by netlock? > I am just tracking a bug and noticed these detach locking strangeness. >
Re: NET_LOCK and trunk detach
On Tue, Jul 28, 2020 at 4:42 PM Vitaliy Makkoveev wrote: > > On Tue, Jul 28, 2020 at 04:10:01PM -0400, sven falempin wrote: > > Hello, > > > > I am running some trunk interfaces in a multi core environment, > > it's a slightly modified version, i have a few NET_ASSERT_LOCKED(); > > suspecting some multi core shenanigans, which i guess was confirmed: > > (unsure the have X meaning, but i ' m pretty sure 256 is very wrong) > > the if_trunk.c locking is completely unmodified > > The code is 6.7-stable > > > > splassert: lacp_detach: want 2 have 0 > > splassert: lacp_detach: want 2 have 0 > > splassert: lacp_detach: want 2 have 256 > > > > I noticed : trunk_clone_destroy ,call > > > > if (tr->tr_proto != TRUNK_PROTO_NONE) > > tr->tr_detach(tr); > > > > outside the lock, and that trunk_ioctl call it > > > > if (tr->tr_proto != TRUNK_PROTO_NONE) > > error = tr->tr_detach(tr); > > > > but ioctl is as far as i understand locked. > > I'm unsure if the difficult and amazing unlocking work > > did an oopsies or if ioctl is already assumed unlocked. > > > > Kindly inform me. > > Best regards, thank you for reading. > > > > lacp_detach() touches nothing which requires NET_LOCK(). What is the > reason you placed assertion to lacp_detach()? <> the lacp_detach is not yours, i put them here because i have a NULL pointer popping in other 'driver' callback. I'm tracking this and trying to understand how this memory is 'nullified' mid function. I do not think putting NET_ASSERT_LOCKED can be harmful in any way. If so please tell me. I am just tracking a bug and noticed these detach locking strangeness.
Re: NET_LOCK and trunk detach
On Tue, Jul 28, 2020 at 04:10:01PM -0400, sven falempin wrote: > Hello, > > I am running some trunk interfaces in a multi core environment, > it's a slightly modified version, i have a few NET_ASSERT_LOCKED(); > suspecting some multi core shenanigans, which i guess was confirmed: > (unsure the have X meaning, but i ' m pretty sure 256 is very wrong) > the if_trunk.c locking is completely unmodified > The code is 6.7-stable > > splassert: lacp_detach: want 2 have 0 > splassert: lacp_detach: want 2 have 0 > splassert: lacp_detach: want 2 have 256 > > I noticed : trunk_clone_destroy ,call > > if (tr->tr_proto != TRUNK_PROTO_NONE) > tr->tr_detach(tr); > > outside the lock, and that trunk_ioctl call it > > if (tr->tr_proto != TRUNK_PROTO_NONE) > error = tr->tr_detach(tr); > > but ioctl is as far as i understand locked. > I'm unsure if the difficult and amazing unlocking work > did an oopsies or if ioctl is already assumed unlocked. > > Kindly inform me. > Best regards, thank you for reading. > lacp_detach() touches nothing which requires NET_LOCK(). What is the reason you placed assertion to lacp_detach()?
NET_LOCK and trunk detach
Hello, I am running some trunk interfaces in a multi core environment, it's a slightly modified version, i have a few NET_ASSERT_LOCKED(); suspecting some multi core shenanigans, which i guess was confirmed: (unsure the have X meaning, but i ' m pretty sure 256 is very wrong) the if_trunk.c locking is completely unmodified The code is 6.7-stable splassert: lacp_detach: want 2 have 0 splassert: lacp_detach: want 2 have 0 splassert: lacp_detach: want 2 have 256 I noticed : trunk_clone_destroy ,call if (tr->tr_proto != TRUNK_PROTO_NONE) tr->tr_detach(tr); outside the lock, and that trunk_ioctl call it if (tr->tr_proto != TRUNK_PROTO_NONE) error = tr->tr_detach(tr); but ioctl is as far as i understand locked. I'm unsure if the difficult and amazing unlocking work did an oopsies or if ioctl is already assumed unlocked. Kindly inform me. Best regards, thank you for reading.
Re: pf: route-to least-states
Hello Yasuoka, On Wed, Jul 29, 2020 at 01:55:20AM +0900, YASUOKA Masahiko wrote: > Hi, > > Let me add another fix of previous. > > ok? OK. thanks for taking care of that. I've entirely missed 1 vs. -1 return value, when reviewing your change. regards sashan
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 01:44:33PM -0400, Johan Huldtgren wrote:
> hello,
>
> On 2020-07-28 11:12, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew
> > >
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis
> > > > >
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > diff tries to support machines with firmware that implements this. If
> > > > > you see something like:
> > > > >
> > > > > "ACPI0007" at acpi0 not configured
> > > > >
> > > > > please try the following diff and report back with an updated dmesg.
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Mark
> > > >
> > > > And now with the right diff...
> > >
> > > On a dell r6415, it looks like this:
> > >
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > >
> > > which I guess means aml_copyvalue() needs to learn how to copy
> > > AML_OBJTYPE_DEVICE.
> >
> > Yes. It is not immediately obvious how this should work. Do we need
> > to copy the aml_node pointer or not? We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > carry any additional information. So we end up with just an empty
> > case to avoid the warning.
> >
> > Does this work on the Dell machines?
> >
> >
> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > + case AML_OBJTYPE_DEVICE:
> > + break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
> >
Mark, This one below looks like it's attaching on both Processor() nodes
and "ACPI0007", I'm assuming we don't want to do that?
> before diffs I see:
>
> "ACPI0004" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0004" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
> "ACPI0007" at acpi0 not configured
>
> after applying both diffs I see that as
>
> "ACPI0004" at acpi0 not configured
> acpicpu24 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu25 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu26 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu27 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu28 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu29 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu30 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu31 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu32 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu33 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu34 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu35 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> "ACPI0004" at acpi0 not configured
> acpicpu36 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu37 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu38 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu39 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu40 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu41 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu42 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu43 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
> acpicpu44 at acpi0: C2(350@41 mwait.3@0x20),
Re: acpicpu(4) and ACPI0007
hello,
On 2020-07-28 11:12, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > From: Jonathan Matthew
> >
> > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > From: Mark Kettenis
> > > >
> > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > diff tries to support machines with firmware that implements this. If
> > > > you see something like:
> > > >
> > > > "ACPI0007" at acpi0 not configured
> > > >
> > > > please try the following diff and report back with an updated dmesg.
> > > >
> > > > Cheers,
> > > >
> > > > Mark
> > >
> > > And now with the right diff...
> >
> > On a dell r6415, it looks like this:
> >
> > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > all the way up to
> > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> >
> > which I guess means aml_copyvalue() needs to learn how to copy
> > AML_OBJTYPE_DEVICE.
>
> Yes. It is not immediately obvious how this should work. Do we need
> to copy the aml_node pointer or not? We don't do that for
> AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> carry any additional information. So we end up with just an empty
> case to avoid the warning.
>
> Does this work on the Dell machines?
>
>
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 dsdt.c
> --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> lhs->v_objref = rhs->v_objref;
> aml_addref(lhs->v_objref.ref, "");
> break;
> + case AML_OBJTYPE_DEVICE:
> + break;
> default:
> printf("copyvalue: %x", rhs->type);
> break;
>
before diffs I see:
"ACPI0004" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0004" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
"ACPI0007" at acpi0 not configured
after applying both diffs I see that as
"ACPI0004" at acpi0 not configured
acpicpu24 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu25 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu26 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu27 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu28 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu29 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu30 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu31 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu32 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu33 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu34 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu35 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
"ACPI0004" at acpi0 not configured
acpicpu36 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu37 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu38 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu39 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu40 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu41 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu42 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu43 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu44 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu45 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu46 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
acpicpu47 at acpi0: C2(350@41 mwait.3@0x20), C1(1016@1 mwait.1), PSS
Full dmesg attached.
thanks,
.jh
OpenBSD 6.7-current (GENERIC.MP) #1: Tue Jul 28 11:35:11 EDT 2020
Re: acpicpu(4) and ACPI0007
> Date: Tue, 28 Jul 2020 21:42:46 +1000
> From: Jonathan Matthew
>
> On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew
> > >
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis
> > > > >
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > diff tries to support machines with firmware that implements this. If
> > > > > you see something like:
> > > > >
> > > > > "ACPI0007" at acpi0 not configured
> > > > >
> > > > > please try the following diff and report back with an updated dmesg.
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Mark
> > > >
> > > > And now with the right diff...
> > >
> > > On a dell r6415, it looks like this:
> > >
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > >
> > > which I guess means aml_copyvalue() needs to learn how to copy
> > > AML_OBJTYPE_DEVICE.
> >
> > Yes. It is not immediately obvious how this should work. Do we need
> > to copy the aml_node pointer or not? We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > carry any additional information. So we end up with just an empty
> > case to avoid the warning.
> >
> > Does this work on the Dell machines?
>
> We've seen crashes in pool_cache_get() in various places after all the
> acpicpus
> attach, which we haven't seen before on these machines, so I think it's
> corrupting memory somehow.
Does that happen with only the acpicpu(4) diff?
> With this addition, we get this for each cpu:
> acpicpu0 at acpi0: C1(@1 halt!)
The exclamation mark indicates that this is the "fallback" C-state.
Is there a _CST method at all?
Anyway, given that this is a server system, it isn't really surprising
that there isn't any fancy power saving stuff.
> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > + case AML_OBJTYPE_DEVICE:
> > + break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
>
>
ifconfig: remove redundant bridge checks
bridge_status() and switch_status() do the regular sanity check with
SIOCGIFFLAGS, but both functions also call is_switch(), bridge_status()
also calls is_bridge().
Those is_*() helpers do the same SIOCGIFFLAGS sanity check, making those
in *_status() entirely redundant, so I'd like to remove them.
I'm here since the tpmr(4) ioctl interface transition from trunk to
bridge semantics is now complete, so ifconfig(8) now requires tpmr bits
to show its members in bridge fashion.
One way would be duplicate code into is_tpmr() and tpmr_status() which
I've already done, but another approach is to unify all bridge like
interfaces under bridge_status().
Either ways, diff below cleans up and makes for simpler code.
Feedback? OK?
Index: brconfig.c
===
RCS file: /cvs/src/sbin/ifconfig/brconfig.c,v
retrieving revision 1.25
diff -u -p -r1.25 brconfig.c
--- brconfig.c 22 Jan 2020 06:24:07 - 1.25
+++ brconfig.c 28 Jul 2020 17:02:05 -
@@ -783,16 +783,11 @@ is_bridge()
void
bridge_status(void)
{
- struct ifreq ifr;
struct ifbrparam bp1, bp2;
if (!is_bridge() || is_switch())
return;
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
- return;
-
bridge_cfg("\t");
bridge_list("\t");
@@ -1184,13 +1179,7 @@ switch_cfg(char *delim)
void
switch_status(void)
{
- struct ifreq ifr;
-
if (!is_switch())
- return;
-
- strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name));
- if (ioctl(sock, SIOCGIFFLAGS, (caddr_t)) == -1)
return;
switch_cfg("\t");
Re: pf: route-to least-states
Hi,
On Tue, 28 Jul 2020 18:54:48 +0200
Alexandr Nedvedicky wrote:
> On Wed, Jul 29, 2020 at 01:22:48AM +0900, YASUOKA Masahiko wrote:
>> Previous commit has a wrong part..
>>
>> ok?
>>
>> Fix previous commit which referred wrong address.
>
> would it make sense to move the block, you've introduced earler
> under the !PF_AZERO() branch just couple lines below. something
> like this:
>
> 8<---8<---8<--8<
> diff --git a/sys/net/pf_lb.c b/sys/net/pf_lb.c
> index 510795a4d0b..f77d96a99ec 100644
> --- a/sys/net/pf_lb.c
> +++ b/sys/net/pf_lb.c
> @@ -322,13 +322,13 @@ pf_map_addr_sticky(sa_family_t af, struct pf_rule *r,
> struct pf_addr *saddr,
> return (-1);
> }
>
> - if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
> - if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
> + if (!PF_AZERO(cached, af)) {
> + pf_addrcpy(naddr, cached, af);
> + if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES)
> &&
> + ((pf_map_addr_states_increase(af, rpool, cached) == -1))
> return (-1);
> }
>
> - if (!PF_AZERO(cached, af))
> - pf_addrcpy(naddr, cached, af);
> if (pf_status.debug >= LOG_DEBUG) {
> log(LOG_DEBUG, "pf: pf_map_addr: "
> "src tracking (%u) maps ", type);
>
> 8<---8<---8<--8<
>
> It seems to me it would be better to bump number of states if and only if we
> actually find some address in pool.
Yes, I agree.
ok?
Fix previous commit which referred wrong address and returned wrong
value.
Index: sys/net/pf_lb.c
===
RCS file: /cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.66
diff -u -p -r1.66 pf_lb.c
--- sys/net/pf_lb.c 28 Jul 2020 16:47:41 - 1.66
+++ sys/net/pf_lb.c 28 Jul 2020 17:01:34 -
@@ -322,13 +322,13 @@ pf_map_addr_sticky(sa_family_t af, struc
return (-1);
}
- if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
- if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
- return (-1);
- }
- if (!PF_AZERO(cached, af))
+ if (!PF_AZERO(cached, af)) {
pf_addrcpy(naddr, cached, af);
+ if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES &&
+ pf_map_addr_states_increase(af, rpool, cached) == -1)
+ return (-1);
+ }
if (pf_status.debug >= LOG_DEBUG) {
log(LOG_DEBUG, "pf: pf_map_addr: "
"src tracking (%u) maps ", type);
@@ -651,7 +651,7 @@ pf_map_addr_states_increase(sa_family_t
pf_print_host(naddr, 0, af);
addlog(". Failed to increase count!\n");
}
- return (1);
+ return (-1);
}
} else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
@@ -663,7 +663,7 @@ pf_map_addr_states_increase(sa_family_t
pf_print_host(naddr, 0, af);
addlog(". Failed to increase count!\n");
}
- return (1);
+ return (-1);
}
}
return (0);
Re: pf: route-to least-states
Hello Yasuoka,
On Wed, Jul 29, 2020 at 01:22:48AM +0900, YASUOKA Masahiko wrote:
> Hi,
>
> Previous commit has a wrong part..
>
> ok?
>
> Fix previous commit which referred wrong address.
would it make sense to move the block, you've introduced earler
under the !PF_AZERO() branch just couple lines below. something
like this:
8<---8<---8<--8<
diff --git a/sys/net/pf_lb.c b/sys/net/pf_lb.c
index 510795a4d0b..f77d96a99ec 100644
--- a/sys/net/pf_lb.c
+++ b/sys/net/pf_lb.c
@@ -322,13 +322,13 @@ pf_map_addr_sticky(sa_family_t af, struct pf_rule *r,
struct pf_addr *saddr,
return (-1);
}
- if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
- if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
+ if (!PF_AZERO(cached, af)) {
+ pf_addrcpy(naddr, cached, af);
+ if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) &&
+ ((pf_map_addr_states_increase(af, rpool, cached) == -1))
return (-1);
}
- if (!PF_AZERO(cached, af))
- pf_addrcpy(naddr, cached, af);
if (pf_status.debug >= LOG_DEBUG) {
log(LOG_DEBUG, "pf: pf_map_addr: "
"src tracking (%u) maps ", type);
8<---8<---8<--8<
It seems to me it would be better to bump number of states if and only if we
actually find some address in pool.
thanks and
regards
sashan
Re: pf: route-to least-states
Hi,
Let me add another fix of previous.
ok?
Fix previous commit which referred wrong address and returned wrong
value.
Index: sys/net/pf_lb.c
===
RCS file: /cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.66
diff -u -p -r1.66 pf_lb.c
--- sys/net/pf_lb.c 28 Jul 2020 16:47:41 - 1.66
+++ sys/net/pf_lb.c 28 Jul 2020 16:52:24 -
@@ -323,7 +323,7 @@ pf_map_addr_sticky(sa_family_t af, struc
}
if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
- if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
+ if (pf_map_addr_states_increase(af, rpool, cached) == -1)
return (-1);
}
@@ -651,7 +651,7 @@ pf_map_addr_states_increase(sa_family_t
pf_print_host(naddr, 0, af);
addlog(". Failed to increase count!\n");
}
- return (1);
+ return (-1);
}
} else if (rpool->addr.type == PF_ADDR_DYNIFTL) {
if (pfr_states_increase(rpool->addr.p.dyn->pfid_kt,
@@ -663,7 +663,7 @@ pf_map_addr_states_increase(sa_family_t
pf_print_host(naddr, 0, af);
addlog(". Failed to increase count!\n");
}
- return (1);
+ return (-1);
}
}
return (0);
Re: pf: route-to least-states
Hi,
Previous commit has a wrong part..
ok?
Fix previous commit which referred wrong address.
Index: sys/net/pf_lb.c
===
RCS file: /cvs/src/sys/net/pf_lb.c,v
retrieving revision 1.65
diff -u -p -r1.65 pf_lb.c
--- sys/net/pf_lb.c 24 Jul 2020 14:06:33 - 1.65
+++ sys/net/pf_lb.c 28 Jul 2020 16:15:50 -
@@ -323,7 +323,7 @@ pf_map_addr_sticky(sa_family_t af, struc
}
if ((rpool->opts & PF_POOL_TYPEMASK) == PF_POOL_LEASTSTATES) {
- if (pf_map_addr_states_increase(af, rpool, naddr) == -1)
+ if (pf_map_addr_states_increase(af, rpool, cached) == -1)
return (-1);
}
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 01:09:51PM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 11:16:56 +0100
> > From: Jason McIntyre
> >
> > On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > > From: Jonathan Matthew
> > > >
> > > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > > From: Mark Kettenis
> > > > > >
> > > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout
> > > > > > of
> > > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > > diff tries to support machines with firmware that implements this.
> > > > > > If
> > > > > > you see something like:
> > > > > >
> > > > > > "ACPI0007" at acpi0 not configured
> > > > > >
> > > > > > please try the following diff and report back with an updated dmesg.
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Mark
> > > > >
> > > > > And now with the right diff...
> > > >
> > > > On a dell r6415, it looks like this:
> > > >
> > > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > > all the way up to
> > > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > > >
> > > > which I guess means aml_copyvalue() needs to learn how to copy
> > > > AML_OBJTYPE_DEVICE.
> > >
> > > Yes. It is not immediately obvious how this should work. Do we need
> > > to copy the aml_node pointer or not? We don't do that for
> > > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > > carry any additional information. So we end up with just an empty
> > > case to avoid the warning.
> > >
> > > Does this work on the Dell machines?
> > >
> > >
> > > Index: dev/acpi/dsdt.c
> > > ===
> > > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > > retrieving revision 1.252
> > > diff -u -p -r1.252 dsdt.c
> > > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > > lhs->v_objref = rhs->v_objref;
> > > aml_addref(lhs->v_objref.ref, "");
> > > break;
> > > + case AML_OBJTYPE_DEVICE:
> > > + break;
> > > default:
> > > printf("copyvalue: %x", rhs->type);
> > > break;
> > >
> >
> > morning. it displays this here:
> >
> > acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> > mwait), PSS
> > acpicpu8 at acpi0: no cpu matching ACPI ID 8
> > acpicpu9 at acpi0: no cpu matching ACPI ID 9
> > acpicpu10 at acpi0: no cpu matching ACPI ID 10
> > acpicpu11 at acpi0: no cpu matching ACPI ID 11
> > acpicpu12 at acpi0: no cpu matching ACPI ID 12
> > acpicpu13 at acpi0: no cpu matching ACPI ID 13
> > acpicpu14 at acpi0: no cpu matching ACPI ID 14
> > acpicpu15 at acpi0: no cpu matching ACPI ID 15
>
> Excellent!
>
> We may want to do something about those "no cpu matching ACPU ID XX"
> messages at some point. But that's a diff for another day.
>
> So ok's for both diffs are welcome.
reads fine and clearly improves things for limited function ACPI
systems.
ok brynet@
> Cheers,
>
> Mark
>
> P.S. I've also established that that the EC-related messages are
> indeed harmless and a result of sloppy BIOS writers.
>
> > OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
> > jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> > real mem = 7895654400 (7529MB)
> > avail mem = 7641284608 (7287MB)
> > random: good seed from bootblocks
> > mpath0 at root
> > scsibus0 at mpath0: 256 targets
> > mainbus0 at root
> > bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
> > bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
> > bios0: Dell Inc. Inspiron 5505
> > acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_
> >
> > acpi0: sleep states S0 S4 S5
> > acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC
> > MCFG SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT
> > SSDT SSDT SSDT SSDT SSDT SSDT BGRT
> > acpi0: wakeup devices GP17(S4)
> > acpitimer0 at acpi0: 3579545
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > From: Jonathan Matthew
> >
> > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > From: Mark Kettenis
> > > >
> > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > diff tries to support machines with firmware that implements this. If
> > > > you see something like:
> > > >
> > > > "ACPI0007" at acpi0 not configured
> > > >
> > > > please try the following diff and report back with an updated dmesg.
> > > >
> > > > Cheers,
> > > >
> > > > Mark
> > >
> > > And now with the right diff...
> >
> > On a dell r6415, it looks like this:
> >
> > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > all the way up to
> > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> >
> > which I guess means aml_copyvalue() needs to learn how to copy
> > AML_OBJTYPE_DEVICE.
>
> Yes. It is not immediately obvious how this should work. Do we need
> to copy the aml_node pointer or not? We don't do that for
> AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> carry any additional information. So we end up with just an empty
> case to avoid the warning.
>
> Does this work on the Dell machines?
We've seen crashes in pool_cache_get() in various places after all the acpicpus
attach, which we haven't seen before on these machines, so I think it's
corrupting memory somehow.
With this addition, we get this for each cpu:
acpicpu0 at acpi0: C1(@1 halt!)
>
>
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 dsdt.c
> --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> lhs->v_objref = rhs->v_objref;
> aml_addref(lhs->v_objref.ref, "");
> break;
> + case AML_OBJTYPE_DEVICE:
> + break;
> default:
> printf("copyvalue: %x", rhs->type);
> break;
Re: acpicpu(4) and ACPI0007
> Date: Tue, 28 Jul 2020 11:16:56 +0100
> From: Jason McIntyre
>
> On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > > From: Jonathan Matthew
> > >
> > > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > > From: Mark Kettenis
> > > > >
> > > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > > diff tries to support machines with firmware that implements this. If
> > > > > you see something like:
> > > > >
> > > > > "ACPI0007" at acpi0 not configured
> > > > >
> > > > > please try the following diff and report back with an updated dmesg.
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Mark
> > > >
> > > > And now with the right diff...
> > >
> > > On a dell r6415, it looks like this:
> > >
> > > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > > all the way up to
> > > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> > >
> > > which I guess means aml_copyvalue() needs to learn how to copy
> > > AML_OBJTYPE_DEVICE.
> >
> > Yes. It is not immediately obvious how this should work. Do we need
> > to copy the aml_node pointer or not? We don't do that for
> > AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> > similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> > carry any additional information. So we end up with just an empty
> > case to avoid the warning.
> >
> > Does this work on the Dell machines?
> >
> >
> > Index: dev/acpi/dsdt.c
> > ===
> > RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> > retrieving revision 1.252
> > diff -u -p -r1.252 dsdt.c
> > --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> > +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> > @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> > lhs->v_objref = rhs->v_objref;
> > aml_addref(lhs->v_objref.ref, "");
> > break;
> > + case AML_OBJTYPE_DEVICE:
> > + break;
> > default:
> > printf("copyvalue: %x", rhs->type);
> > break;
> >
>
> morning. it displays this here:
>
> acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
> mwait), PSS
> acpicpu8 at acpi0: no cpu matching ACPI ID 8
> acpicpu9 at acpi0: no cpu matching ACPI ID 9
> acpicpu10 at acpi0: no cpu matching ACPI ID 10
> acpicpu11 at acpi0: no cpu matching ACPI ID 11
> acpicpu12 at acpi0: no cpu matching ACPI ID 12
> acpicpu13 at acpi0: no cpu matching ACPI ID 13
> acpicpu14 at acpi0: no cpu matching ACPI ID 14
> acpicpu15 at acpi0: no cpu matching ACPI ID 15
Excellent!
We may want to do something about those "no cpu matching ACPU ID XX"
messages at some point. But that's a diff for another day.
So ok's for both diffs are welcome.
Cheers,
Mark
P.S. I've also established that that the EC-related messages are
indeed harmless and a result of sloppy BIOS writers.
> OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
> jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
> real mem = 7895654400 (7529MB)
> avail mem = 7641284608 (7287MB)
> random: good seed from bootblocks
> mpath0 at root
> scsibus0 at mpath0: 256 targets
> mainbus0 at root
> bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
> bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
> bios0: Dell Inc. Inspiron 5505
> acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_
>
> acpi0: sleep states S0 S4 S5
> acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC
> MCFG SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT
> SSDT SSDT SSDT SSDT SSDT SSDT BGRT
> acpi0: wakeup devices GP17(S4)
> acpitimer0 at acpi0: 3579545 Hz, 32 bits
> acpihpet0 at acpi0: 14318180 Hz
> acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
> cpu0 at mainbus0: apid 0 (boot processor)
> cpu0: AMD Ryzen 7 4700U with Radeon Graphics, 1996.48 MHz, 17-60-01
> cpu0:
>
Re: xhci(4) isoc: fix bogus handling of chained TRBs
On Tue, 28 Jul 2020 11:42:43 +0200 Martin Pieuchot wrote: > On 26/07/20(Sun) 16:23, Marcus Glocker wrote: > > On Sun, 26 Jul 2020 13:27:34 + > > sc.dy...@gmail.com wrote: > > > > > On 2020/07/26 10:54, Marcus Glocker wrote: > > > > On Sat, 25 Jul 2020 20:31:44 + > > > > sc.dy...@gmail.com wrote: > > > > > > > >> On 2020/07/25 18:10, Marcus Glocker wrote: > > > >>> On Sun, Jul 19, 2020 at 02:12:21PM +, sc.dy...@gmail.com > > > >>> wrote: > > > On 2020/07/19 11:25, Marcus Glocker wrote: > > > > On Sun, 19 Jul 2020 02:25:30 + > > > > sc.dy...@gmail.com wrote: > > > > > > > >> hi, > > > >> > > > >> It works on AMD Bolton xHCI (78141022), Intel PCH > > > >> (1e318086), and ASM1042 (10421b21). > > > >> I simply play with ffplay -f v4l2 /dev/video0 to test. > > > >> > > > > > > > > If your cam supports MJPEG it's good to add '-input_format > > > > mjpeg' with higher resolutions like 1280x720, because that > > > > will generated varying image sizes, which hit the 64k > > > > memory boundary more often, and thus generate potentially > > > > more chained TDs. > > > > > > Thank you for useful information. > > > My webcam supprots at most 640x480, but 1024 bytes/frame x > > > (2+1) maxburst x 40 frames = 122880 bytes/xfer is enough to > > > observe TD fragmentation. > > > > > > > > > >> At this moment it does not work on VL805, but I have no > > > >> idea. I'll investigate furthermore... > > > >>> > > > >>> Did you already had a chance to figure out something > > > >>> regarding the issue you faced on your VL805 controller? > > > >>> > > > >>> I'm running the diff here since then on the Intel xHCI > > > >>> controller and couldn't re-produce any errors using different > > > >>> uvideo(4) and uaudio(4) devices. > > > >>> > > > >> > > > >> No, yet -- all I know about this problem is VL805 genegates > > > >> many MISSED_SRV Transfer Event for Isoch-IN pipe. > > > >> > > > >> xhci0: slot 3 missed srv with 123 TRB > > > >> : > > > >> > > > >> Even if I increase UVIDEO_IXFERS in uvideo.h to 6, HC still > > > >> detects MISSED_SRV. When I disable splitting TD, it works well. > > > >> I added printf paddr in the event TRB but each paddr of > > > >> MISSED_SRV is 0, that does not meet 4.10.3.2. > > > >> Parameters in this endpoint context are > > > >> > > > >> xhci0: slot 3 dci 3 ival 0 mps 1024 maxb 2 mep 3072 atl 3072 > > > >> mult 0 > > > >> > > > >> looks sane. > > > > > > > > Hmm, I see. > > > > > > > > I currently have also no idea what exactly is causing the missed > > > > service events. I was reading a little bit yesterday about the > > > > VL805 and could find some statements where people say it's not > > > > fully compliant with the xHCI specs, and in Linux it took some > > > > cooperation with the vendor to make it work. > > > > > > > > One thing I still wanted to ask you to understand whether the > > > > problem on your VL805 is only related with my last diff; Are > > > > the multi-trb transfers working fine with your last diff on the > > > > VL805? > > > > > > On VL805 ffplay plays the movie sometimes smoothly, sometimes > > > laggy. The multi-TRB transfer itself works on VL805 with your > > > patch. Not all splitted TD fail to transfer. Successful splitted > > > transfer works as intended. > > > I think MISSED_SRV is caused by other reason, maybe isochronous > > > scheduling problem. > > > Thus, IMO your patch can be committed. > > > > > > VL805 also has same problem that AMD Bolton has. > > > It may generate the 1. TRB event w/ cc = SHORT_PKT and > > > remain = requested length (that is, transferred length = 0), > > > but the 2. TRB w/ cc = SUCCESS and remain = 0. > > > remain of 2. TRB should be given length, and CC should be > > > SHORT_PKT. Your patch fixes this problem. > > > > OK, that's what I wanted to understand. > > I also have the impression that the MISSED_SRV issue on the VL805 is > > related to another problem which we should trace separately from the > > multi-trb problem. Thanks for that useful feedback. > > > > Attached the latest version of my patch including all the inputs > > received (mostly related to malloc/free). > > > > Patrick, Martin, would you be fine to OK that? > > The logic looks fine. I'd suggest you move the trb_processed array to > the xhci_pipe structure. Command and Event rings do not need it, > right? This should also allow you to get rid of the malloc/free by > always using XHCI_MAX_XFER elements. Good point and true. I wasn't sure initially whether we could make use of this on other rings, but in the meantime I also believe this will only be required for xfer rings. I moved it to xhci_pipe. Gets the patch quite compact :-) Index: xhci.c === RCS file: /cvs/src/sys/dev/usb/xhci.c,v retrieving revision
Re: acpicpu(4) and ACPI0007
On Tue, Jul 28, 2020 at 11:12:21AM +0200, Mark Kettenis wrote:
> > Date: Tue, 28 Jul 2020 13:46:34 +1000
> > From: Jonathan Matthew
> >
> > On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > > From: Mark Kettenis
> > > >
> > > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > > diff tries to support machines with firmware that implements this. If
> > > > you see something like:
> > > >
> > > > "ACPI0007" at acpi0 not configured
> > > >
> > > > please try the following diff and report back with an updated dmesg.
> > > >
> > > > Cheers,
> > > >
> > > > Mark
> > >
> > > And now with the right diff...
> >
> > On a dell r6415, it looks like this:
> >
> > acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> > all the way up to
> > acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
> >
> > which I guess means aml_copyvalue() needs to learn how to copy
> > AML_OBJTYPE_DEVICE.
>
> Yes. It is not immediately obvious how this should work. Do we need
> to copy the aml_node pointer or not? We don't do that for
> AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
> similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
> carry any additional information. So we end up with just an empty
> case to avoid the warning.
>
> Does this work on the Dell machines?
>
>
> Index: dev/acpi/dsdt.c
> ===
> RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
> retrieving revision 1.252
> diff -u -p -r1.252 dsdt.c
> --- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
> +++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
> @@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
> lhs->v_objref = rhs->v_objref;
> aml_addref(lhs->v_objref.ref, "");
> break;
> + case AML_OBJTYPE_DEVICE:
> + break;
> default:
> printf("copyvalue: %x", rhs->type);
> break;
>
morning. it displays this here:
acpicpu0 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu1 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu2 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu3 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu4 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu5 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu6 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu7 at acpi0: C3(0@350 io@0x415), C2(0@400 io@0x414), C1(0@1
mwait), PSS
acpicpu8 at acpi0: no cpu matching ACPI ID 8
acpicpu9 at acpi0: no cpu matching ACPI ID 9
acpicpu10 at acpi0: no cpu matching ACPI ID 10
acpicpu11 at acpi0: no cpu matching ACPI ID 11
acpicpu12 at acpi0: no cpu matching ACPI ID 12
acpicpu13 at acpi0: no cpu matching ACPI ID 13
acpicpu14 at acpi0: no cpu matching ACPI ID 14
acpicpu15 at acpi0: no cpu matching ACPI ID 15
jmc
OpenBSD 6.7-current (GENERIC.MP) #3: Tue Jul 28 10:59:50 BST 2020
jmc@kansas:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 7895654400 (7529MB)
avail mem = 7641284608 (7287MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 3.2 @ 0xca707000 (77 entries)
bios0: vendor Dell Inc. version "1.1.0" date 05/27/2020
bios0: Dell Inc. Inspiron 5505
acpi0 at bios0: ACPI 5.0Undefined scope: \\_SB_.PCI0.LPC0.EC0_
acpi0: sleep states S0 S4 S5
acpi0: tables DSDT FACP UEFI SSDT IVRS SSDT TPM2 MSDM ASF! BOOT HPET APIC MCFG
SLIC SSDT WSMT VFCT SSDT SSDT CRAT CDIT SSDT SSDT SSDT SSDT FPDT SSDT SSDT SSDT
SSDT SSDT SSDT SSDT BGRT
acpi0: wakeup devices GP17(S4)
acpitimer0 at acpi0: 3579545 Hz, 32 bits
acpihpet0 at acpi0: 14318180 Hz
acpimadt0 at acpi0 addr 0xfee0: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 4700U with Radeon Graphics, 1996.48 MHz, 17-60-01
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,HTT,SSE3,PCLMUL,MWAIT,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,CMPLEG,SVM,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,IBS,SKINIT,TCE,TOPEXT,CPCTR,DBKP,PCTRL3,MWAITX,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,PQM,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,UMIP,IBPB,IBRS,STIBP,SSBD,XSAVEOPT,XSAVEC,XGETBV1,XSAVES
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line
8-way L2 cache, 8MB 64b/line disabled L3 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative,
Re: pipex(4): document global data locks
On Tue, Jul 28, 2020 at 10:26:53AM +0200, Martin Pieuchot wrote:
> On 17/07/20(Fri) 17:04, Vitaliy Makkoveev wrote:
> > Subj. Also add NET_ASSERT_LOCKED() to pipex_{link,unlink,rele}_session()
> > to be sure they called under NET_LOCK().
>
> pipex_rele_session() is freeing memory. When this function is called
> those chunks of memory shouldn't be referenced by any other CPU or piece
> of descriptor in the network stack. So the NET_LOCK() shouldn't be
> required.
Fixed.
>
> Rest of the diff is fine. I'd suggest you put the assertions just above
> the LIST_INSERT or LIST_REMOVE like it is done in other parts of the stack.
>
We should not add session to lists if `iface->pipexmode' is null. So
check and modification should be done while the same lock is held.
That's why assertion in pipex_link_session() is in the right place, just
before `iface->pipexmode' check.
I moved assertion just before LIST_REMOVE() within
pipex_unlink_session().
Updated diff below.
Index: sys/net/pipex.c
===
RCS file: /cvs/src/sys/net/pipex.c,v
retrieving revision 1.120
diff -u -p -r1.120 pipex.c
--- sys/net/pipex.c 17 Jul 2020 08:57:27 - 1.120
+++ sys/net/pipex.c 28 Jul 2020 09:47:51 -
@@ -83,19 +83,24 @@ struct pool pipex_session_pool;
struct pool mppe_key_pool;
/*
- * static/global variables
+ * Global data
+ * Locks used to protect global data
+ * A atomic operation
+ * I immutable after creation
+ * N net lock
*/
-intpipex_enable = 0;
+
+intpipex_enable = 0; /* [N] */
struct pipex_hash_head
-pipex_session_list,/* master session list
*/
-pipex_close_wait_list, /* expired session list */
-pipex_peer_addr_hashtable[PIPEX_HASH_SIZE],/* peer's address hash
*/
-pipex_id_hashtable[PIPEX_HASH_SIZE]; /* peer id hash */
+pipex_session_list,/* [N] master session
list */
+pipex_close_wait_list, /* [N] expired session list */
+pipex_peer_addr_hashtable[PIPEX_HASH_SIZE],/* [N] peer's address
hash */
+pipex_id_hashtable[PIPEX_HASH_SIZE]; /* [N] peer id hash */
-struct radix_node_head *pipex_rd_head4 = NULL;
-struct radix_node_head *pipex_rd_head6 = NULL;
+struct radix_node_head *pipex_rd_head4 = NULL; /* [N] */
+struct radix_node_head *pipex_rd_head6 = NULL; /* [N] */
struct timeout pipex_timer_ch; /* callout timer context */
-int pipex_prune = 1; /* walk list every seconds */
+int pipex_prune = 1; /* [I] walk list every seconds */
/* pipex traffic queue */
struct mbuf_queue pipexinq = MBUF_QUEUE_INITIALIZER(IFQ_MAXLEN, IPL_NET);
@@ -105,7 +110,7 @@ struct mbuf_queue pipexoutq = MBUF_QUEUE
#define ph_ppp_proto ether_vtag
#ifdef PIPEX_DEBUG
-int pipex_debug = 0; /* systcl net.inet.ip.pipex_debug */
+int pipex_debug = 0; /* [A] systcl net.inet.ip.pipex_debug */
#endif
/* PPP compression == MPPE is assumed, so don't answer CCP Reset-Request. */
@@ -430,6 +435,8 @@ pipex_link_session(struct pipex_session
{
struct pipex_hash_head *chain;
+ NET_ASSERT_LOCKED();
+
if (!iface->pipexmode)
return (ENXIO);
if (pipex_lookup_by_session_id(session->protocol,
@@ -465,6 +472,7 @@ pipex_unlink_session(struct pipex_sessio
{
session->ifindex = 0;
+ NET_ASSERT_LOCKED();
LIST_REMOVE(session, id_chain);
#if defined(PIPEX_PPTP) || defined(PIPEX_L2TP)
switch (session->protocol) {
Re: xhci(4) isoc: fix bogus handling of chained TRBs
On 26/07/20(Sun) 16:23, Marcus Glocker wrote: > On Sun, 26 Jul 2020 13:27:34 + > sc.dy...@gmail.com wrote: > > > On 2020/07/26 10:54, Marcus Glocker wrote: > > > On Sat, 25 Jul 2020 20:31:44 + > > > sc.dy...@gmail.com wrote: > > > > > >> On 2020/07/25 18:10, Marcus Glocker wrote: > > >>> On Sun, Jul 19, 2020 at 02:12:21PM +, sc.dy...@gmail.com > > >>> wrote: > > On 2020/07/19 11:25, Marcus Glocker wrote: > > > On Sun, 19 Jul 2020 02:25:30 + > > > sc.dy...@gmail.com wrote: > > > > > >> hi, > > >> > > >> It works on AMD Bolton xHCI (78141022), Intel PCH (1e318086), > > >> and ASM1042 (10421b21). > > >> I simply play with ffplay -f v4l2 /dev/video0 to test. > > > > > > If your cam supports MJPEG it's good to add '-input_format > > > mjpeg' with higher resolutions like 1280x720, because that will > > > generated varying image sizes, which hit the 64k memory boundary > > > more often, and thus generate potentially more chained TDs. > > > > Thank you for useful information. > > My webcam supprots at most 640x480, but 1024 bytes/frame x (2+1) > > maxburst x 40 frames = 122880 bytes/xfer is enough to observe TD > > fragmentation. > > > > > > >> At this moment it does not work on VL805, but I have no idea. > > >> I'll investigate furthermore... > > >>> > > >>> Did you already had a chance to figure out something regarding the > > >>> issue you faced on your VL805 controller? > > >>> > > >>> I'm running the diff here since then on the Intel xHCI controller > > >>> and couldn't re-produce any errors using different uvideo(4) and > > >>> uaudio(4) devices. > > >>> > > >> > > >> No, yet -- all I know about this problem is VL805 genegates > > >> many MISSED_SRV Transfer Event for Isoch-IN pipe. > > >> > > >> xhci0: slot 3 missed srv with 123 TRB > > >> : > > >> > > >> Even if I increase UVIDEO_IXFERS in uvideo.h to 6, HC still detects > > >> MISSED_SRV. When I disable splitting TD, it works well. > > >> I added printf paddr in the event TRB but each paddr of MISSED_SRV > > >> is 0, that does not meet 4.10.3.2. > > >> Parameters in this endpoint context are > > >> > > >> xhci0: slot 3 dci 3 ival 0 mps 1024 maxb 2 mep 3072 atl 3072 mult 0 > > >> > > >> looks sane. > > > > > > Hmm, I see. > > > > > > I currently have also no idea what exactly is causing the missed > > > service events. I was reading a little bit yesterday about the > > > VL805 and could find some statements where people say it's not > > > fully compliant with the xHCI specs, and in Linux it took some > > > cooperation with the vendor to make it work. > > > > > > One thing I still wanted to ask you to understand whether the > > > problem on your VL805 is only related with my last diff; Are the > > > multi-trb transfers working fine with your last diff on the VL805? > > > > On VL805 ffplay plays the movie sometimes smoothly, sometimes laggy. > > The multi-TRB transfer itself works on VL805 with your patch. > > Not all splitted TD fail to transfer. Successful splitted transfer > > works as intended. > > I think MISSED_SRV is caused by other reason, maybe isochronous > > scheduling problem. > > Thus, IMO your patch can be committed. > > > > VL805 also has same problem that AMD Bolton has. > > It may generate the 1. TRB event w/ cc = SHORT_PKT and > > remain = requested length (that is, transferred length = 0), > > but the 2. TRB w/ cc = SUCCESS and remain = 0. > > remain of 2. TRB should be given length, and CC should be SHORT_PKT. > > Your patch fixes this problem. > > OK, that's what I wanted to understand. > I also have the impression that the MISSED_SRV issue on the VL805 is > related to another problem which we should trace separately from the > multi-trb problem. Thanks for that useful feedback. > > Attached the latest version of my patch including all the inputs > received (mostly related to malloc/free). > > Patrick, Martin, would you be fine to OK that? The logic looks fine. I'd suggest you move the trb_processed array to the xhci_pipe structure. Command and Event rings do not need it, right? This should also allow you to get rid of the malloc/free by always using XHCI_MAX_XFER elements. > Index: xhci.c > === > RCS file: /cvs/src/sys/dev/usb/xhci.c,v > retrieving revision 1.116 > diff -u -p -u -p -r1.116 xhci.c > --- xhci.c30 Jun 2020 10:21:59 - 1.116 > +++ xhci.c19 Jul 2020 06:51:58 - > @@ -82,7 +82,7 @@ voidxhci_event_xfer(struct xhci_softc * > int xhci_event_xfer_generic(struct xhci_softc *, struct usbd_xfer *, > struct xhci_pipe *, uint32_t, int, uint8_t, uint8_t, uint8_t); > int xhci_event_xfer_isoc(struct usbd_xfer *, struct xhci_pipe *, > - uint32_t, int); > + uint32_t, int, uint8_t); > void xhci_event_command(struct xhci_softc *, uint64_t); > void
Re: pipex_iface_fini() release multicast session under NET_LOCK()
On Tue, Jul 28, 2020 at 10:23:08AM +0200, Martin Pieuchot wrote:
> On 17/07/20(Fri) 16:29, Vitaliy Makkoveev wrote:
> > We are going to lock the whole pipex(4) by NET_LOCK(). So move
> > `multicast_session' freeing undet NET_LOCK() too.
>
> pipex_iface_fini() should be called on the last reference of the
> descriptor. So this shouldn't be necessary. If there's an issue
> with the current order of the operations, we should certainly fix
> it differently.
>
`multicast_session' can be processed by pipexintr() while we do
acclose(). There is no wrong order, npppd(8) or another userland
prgramm can be killed in any time.
> > Index: sys/net/pipex.c
> > ===
> > RCS file: /cvs/src/sys/net/pipex.c,v
> > retrieving revision 1.120
> > diff -u -p -r1.120 pipex.c
> > --- sys/net/pipex.c 17 Jul 2020 08:57:27 - 1.120
> > +++ sys/net/pipex.c 17 Jul 2020 13:23:16 -
> > @@ -192,8 +192,8 @@ pipex_iface_stop(struct pipex_iface_cont
> > void
> > pipex_iface_fini(struct pipex_iface_context *pipex_iface)
> > {
> > - pool_put(_session_pool, pipex_iface->multicast_session);
> > NET_LOCK();
> > + pool_put(_session_pool, pipex_iface->multicast_session);
> > pipex_iface_stop(pipex_iface);
> > NET_UNLOCK();
> > }
> >
>
Re: acpicpu(4) and ACPI0007
> Date: Tue, 28 Jul 2020 13:46:34 +1000
> From: Jonathan Matthew
>
> On Mon, Jul 27, 2020 at 05:16:47PM +0200, Mark Kettenis wrote:
> > > Date: Mon, 27 Jul 2020 17:02:41 +0200 (CEST)
> > > From: Mark Kettenis
> > >
> > > Recent ACPI versions have deprecated "Processor()" nodes in favout of
> > > "Device()" nodes with a _HID() method that returns "ACPI0007". This
> > > diff tries to support machines with firmware that implements this. If
> > > you see something like:
> > >
> > > "ACPI0007" at acpi0 not configured
> > >
> > > please try the following diff and report back with an updated dmesg.
> > >
> > > Cheers,
> > >
> > > Mark
> >
> > And now with the right diff...
>
> On a dell r6415, it looks like this:
>
> acpicpu0 at acpi0copyvalue: 6: C1(@1 halt!)
> all the way up to
> acpicpu127 at acpi0copyvalue: 6: no cpu matching ACPI ID 127
>
> which I guess means aml_copyvalue() needs to learn how to copy
> AML_OBJTYPE_DEVICE.
Yes. It is not immediately obvious how this should work. Do we need
to copy the aml_node pointer or not? We don't do that for
AML_OBJTYPE_PROCESSOR and AML_OBJTYPE_POWERRSRC types which are
similar to AML_OBJTYPE_DEVICE. But AML_OBJTYPE_DEVICE object don't
carry any additional information. So we end up with just an empty
case to avoid the warning.
Does this work on the Dell machines?
Index: dev/acpi/dsdt.c
===
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.252
diff -u -p -r1.252 dsdt.c
--- dev/acpi/dsdt.c 21 Jul 2020 03:48:06 - 1.252
+++ dev/acpi/dsdt.c 28 Jul 2020 09:04:15 -
@@ -996,6 +996,8 @@ aml_copyvalue(struct aml_value *lhs, str
lhs->v_objref = rhs->v_objref;
aml_addref(lhs->v_objref.ref, "");
break;
+ case AML_OBJTYPE_DEVICE:
+ break;
default:
printf("copyvalue: %x", rhs->type);
break;
Re: net80211: skip input block ack window gaps faster
On 17/07/20(Fri) 18:15, Stefan Sperling wrote:
> On Fri, Jul 17, 2020 at 03:59:38PM +0200, Stefan Sperling wrote:
> > While measuring Tx performance at a fixed Tx rate with iwm(4) I observed
> > unexpected dips in throughput measured by tcpbench. These dips coincided
> > with one or more gap timeouts shown in 'netstat -W iwm0', such as:
> > 77 input block ack window gaps timed out
> > Which means lost frames on the receive side were stalling subsequent frames
> > and thus slowing tcpbench down.
> >
> > I decided to disable the gap timeout entirely to see what would happen if
> > those missing frames were immediately skipped rather than waiting for them.
> > The result was stable throughput according to tcpbench.
> >
> > I then wrote the patch below which keeps the gap timeout intact (it is
> > needed
> > in case the peer stops sending anything) but skips missing frames at the
> > head
> > of the Rx block window once a certain amount of frames have queued up. This
> > heuristics avoids having to wait for the timeout to fire in order to get
> > frames flowing again if we lose one of more frames during Rx traffic bursts.
> >
> > I have picked a threshold of 16 outstanding frames based on local testing.
> > I have no idea if this is a good threshold for everyone. It would help to
> > get some feedback from tests in other RF environments and other types of
> > access points. Any regressions?
>
> Next version.
>
> One problem with the previous patch was that it effectively limited the
> size of the BA window to the arbitrarily chosen limit of 16. We should not
> drop frames which arrive out of order but still fall within the BA window.
>
> With this version, we allow the entire block ack window (usually 64 frames)
> to fill up beyond the missing frame at the head, and only then bypass the
> gap timeout handler and skip over the missing frame directly. I can still
> trigger this shortcut with tcpbench, and still see the timeout run sometimes.
> Direct skip should be faster than having to wait for the timeout to run,
> and missing just one out of 64 frames is a common case in my testing.
>
> Also, I am not quite sure if calling if_input() from a timeout is such a
> good idea. Any opinions about that? This patch still lets the gap timeout
> handler clear the leading gap but avoids flushing buffered frames there.
> The peer will now need to send another frame to flush the buffer, but now
> if_input() will be called from network interrupt context only. Which is
> probably a good thing?
if_input() can be called in any context. Using a timeout means you need
some extra logic to free the queue. It might also add to the latency
> This code still seems to recover well enough from occasional packet loss,
> which is what this is all about. If you are on a really bad link, none
> of this will help anyway.
>
> diff refs/heads/master refs/heads/ba-gap
> blob - 098aa9bce19481ce09676ce3c4fc0040f14c9b93
> blob + 4f41b568311bf29e131a3f4802e0a238ba940fe0
> --- sys/net80211/ieee80211_input.c
> +++ sys/net80211/ieee80211_input.c
> @@ -67,6 +67,7 @@ voidieee80211_input_ba(struct ieee80211com *,
> struct
> struct mbuf_list *);
> void ieee80211_input_ba_flush(struct ieee80211com *, struct ieee80211_node *,
> struct ieee80211_rx_ba *, struct mbuf_list *);
> +int ieee80211_input_ba_gap_skip(struct ieee80211_rx_ba *);
> void ieee80211_input_ba_gap_timeout(void *arg);
> void ieee80211_ba_move_window(struct ieee80211com *,
> struct ieee80211_node *, u_int8_t, u_int16_t, struct mbuf_list *);
> @@ -837,10 +838,29 @@ ieee80211_input_ba(struct ieee80211com *ic, struct mbu
> rxi->rxi_flags |= IEEE80211_RXI_AMPDU_DONE;
> ba->ba_buf[idx].rxi = *rxi;
>
> - if (ba->ba_buf[ba->ba_head].m == NULL)
> - timeout_add_msec(>ba_gap_to, IEEE80211_BA_GAP_TIMEOUT);
> - else if (timeout_pending(>ba_gap_to))
> - timeout_del(>ba_gap_to);
> + if (ba->ba_buf[ba->ba_head].m == NULL) {
> + if (ba->ba_gapwait < (ba->ba_winsize - 1)) {
> + if (ba->ba_gapwait == 0) {
> + timeout_add_msec(>ba_gap_to,
> + IEEE80211_BA_GAP_TIMEOUT);
> + }
> + ba->ba_gapwait++;
> + } else {
> + /*
> + * A full BA window worth of frames is now waiting.
> + * Skip the missing frame at the head of the window.
> + */
> + int skipped = ieee80211_input_ba_gap_skip(ba);
> + ic->ic_stats.is_ht_rx_ba_frame_lost += skipped;
> + ba->ba_gapwait = 0;
> + if (timeout_pending(>ba_gap_to))
> + timeout_del(>ba_gap_to);
> + }
> + } else {
> + ba->ba_gapwait = 0;
> + if (timeout_pending(>ba_gap_to))
> +
Re: pipex(4): document global data locks
On 17/07/20(Fri) 17:04, Vitaliy Makkoveev wrote:
> Subj. Also add NET_ASSERT_LOCKED() to pipex_{link,unlink,rele}_session()
> to be sure they called under NET_LOCK().
pipex_rele_session() is freeing memory. When this function is called
those chunks of memory shouldn't be referenced by any other CPU or piece
of descriptor in the network stack. So the NET_LOCK() shouldn't be
required.
Rest of the diff is fine. I'd suggest you put the assertions just above
the LIST_INSERT or LIST_REMOVE like it is done in other parts of the stack.
> Index: sys/net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 pipex.c
> --- sys/net/pipex.c 17 Jul 2020 08:57:27 - 1.120
> +++ sys/net/pipex.c 17 Jul 2020 14:01:10 -
> @@ -83,19 +83,24 @@ struct pool pipex_session_pool;
> struct pool mppe_key_pool;
>
> /*
> - * static/global variables
> + * Global data
> + * Locks used to protect global data
> + * A atomic operation
> + * I immutable after creation
> + * N net lock
> */
> -int pipex_enable = 0;
> +
> +int pipex_enable = 0; /* [N] */
> struct pipex_hash_head
> -pipex_session_list, /* master session list
> */
> -pipex_close_wait_list, /* expired session list */
> -pipex_peer_addr_hashtable[PIPEX_HASH_SIZE], /* peer's address hash
> */
> -pipex_id_hashtable[PIPEX_HASH_SIZE]; /* peer id hash */
> +pipex_session_list, /* [N] master session
> list */
> +pipex_close_wait_list, /* [N] expired session list */
> +pipex_peer_addr_hashtable[PIPEX_HASH_SIZE], /* [N] peer's address
> hash */
> +pipex_id_hashtable[PIPEX_HASH_SIZE]; /* [N] peer id hash */
>
> -struct radix_node_head *pipex_rd_head4 = NULL;
> -struct radix_node_head *pipex_rd_head6 = NULL;
> +struct radix_node_head *pipex_rd_head4 = NULL; /* [N] */
> +struct radix_node_head *pipex_rd_head6 = NULL; /* [N] */
> struct timeout pipex_timer_ch; /* callout timer context */
> -int pipex_prune = 1; /* walk list every seconds */
> +int pipex_prune = 1; /* [I] walk list every seconds */
>
> /* pipex traffic queue */
> struct mbuf_queue pipexinq = MBUF_QUEUE_INITIALIZER(IFQ_MAXLEN, IPL_NET);
> @@ -105,7 +110,7 @@ struct mbuf_queue pipexoutq = MBUF_QUEUE
> #define ph_ppp_proto ether_vtag
>
> #ifdef PIPEX_DEBUG
> -int pipex_debug = 0; /* systcl net.inet.ip.pipex_debug */
> +int pipex_debug = 0; /* [A] systcl net.inet.ip.pipex_debug */
> #endif
>
> /* PPP compression == MPPE is assumed, so don't answer CCP Reset-Request. */
> @@ -419,6 +424,8 @@ pipex_init_session(struct pipex_session
> void
> pipex_rele_session(struct pipex_session *session)
> {
> + NET_ASSERT_LOCKED();
> +
> if (session->mppe_recv.old_session_keys)
> pool_put(_key_pool, session->mppe_recv.old_session_keys);
> pool_put(_session_pool, session);
> @@ -430,6 +437,8 @@ pipex_link_session(struct pipex_session
> {
> struct pipex_hash_head *chain;
>
> + NET_ASSERT_LOCKED();
> +
> if (!iface->pipexmode)
> return (ENXIO);
> if (pipex_lookup_by_session_id(session->protocol,
> @@ -463,6 +472,8 @@ pipex_link_session(struct pipex_session
> void
> pipex_unlink_session(struct pipex_session *session)
> {
> + NET_ASSERT_LOCKED();
> +
> session->ifindex = 0;
>
> LIST_REMOVE(session, id_chain);
>
Re: pipex_iface_fini() release multicast session under NET_LOCK()
On 17/07/20(Fri) 16:29, Vitaliy Makkoveev wrote:
> We are going to lock the whole pipex(4) by NET_LOCK(). So move
> `multicast_session' freeing undet NET_LOCK() too.
pipex_iface_fini() should be called on the last reference of the
descriptor. So this shouldn't be necessary. If there's an issue
with the current order of the operations, we should certainly fix
it differently.
> Index: sys/net/pipex.c
> ===
> RCS file: /cvs/src/sys/net/pipex.c,v
> retrieving revision 1.120
> diff -u -p -r1.120 pipex.c
> --- sys/net/pipex.c 17 Jul 2020 08:57:27 - 1.120
> +++ sys/net/pipex.c 17 Jul 2020 13:23:16 -
> @@ -192,8 +192,8 @@ pipex_iface_stop(struct pipex_iface_cont
> void
> pipex_iface_fini(struct pipex_iface_context *pipex_iface)
> {
> - pool_put(_session_pool, pipex_iface->multicast_session);
> NET_LOCK();
> + pool_put(_session_pool, pipex_iface->multicast_session);
> pipex_iface_stop(pipex_iface);
> NET_UNLOCK();
> }
>
Re: timekeep: fixing large skews on amd64 with RDTSCP
On 2020-07-27 18:24, Mark Kettenis wrote:
Date: Mon, 27 Jul 2020 17:14:21 +0200
From: Christian Weisgerber
Scott Cheloha:
--- lib/libc/arch/amd64/gen/usertc.c8 Jul 2020 09:17:48 - 1.2
+++ lib/libc/arch/amd64/gen/usertc.c25 Jul 2020 17:50:38 -
@@ -21,9 +21,12 @@
static inline u_int
rdtsc(void)
{
- uint32_t hi, lo;
- asm volatile("rdtsc" : "=a"(lo), "=d"(hi));
- return ((uint64_t)lo)|(((uint64_t)hi)<<32);
+ uint32_t lo;
+
+ asm volatile("lfence");
+ asm volatile("rdtsc" : "=a"(lo) : : "rdx");
Is there a guarantee that two separate asm()s will not be reordered?
I believe that is true for "volatile" asm statements. But this is all
not very well documented and I believe that the compiler may hoist
bits of C code in between, which is probably not what you want.
Note that since "asm" is non-standard C, we favour spelling it as
"__asm" since that makes the compiler shut up about it even if you
request stricter C standard compliance.
And given the kernel bit nelow...
+
+ return lo;
}
static int
--- sys/arch/amd64/amd64/tsc.c 6 Jul 2020 13:33:06 - 1.19
+++ sys/arch/amd64/amd64/tsc.c 25 Jul 2020 17:50:38 -
@@ -211,7 +211,12 @@ cpu_recalibrate_tsc(struct timecounter *
u_int
tsc_get_timecount(struct timecounter *tc)
{
- return rdtsc() + curcpu()->ci_tsc_skew;
+ uint32_t lo;
+
+ asm volatile("lfence");
+ asm volatile("rdtsc" : "=a"(lo) : : "rdx");
+
+ return lo + curcpu()->ci_tsc_skew;
}
void
I'd just do s/rdtsc/rdtsc_lfence/, which would agree well with the
rest of the file.
Agreed. And I would really prefer that the libc code stays as close
to the kernel code as possible.
Is the issue with LFENCE slowing down the network stack settled? That
was the argument against it last time.
[PATCH] www/faq/faq14.html: make disklabel(8) prompts match the actual program
When there are unwritten changes to the disklabel, disklabel(8) adds an '*' to the prompt. Just a minor erratum I noticed doing an install. diff --git a/faq/faq14.html b/faq/faq14.html index f53d45180..377f8c7b2 100644 --- a/faq/faq14.html +++ b/faq/faq14.html @@ -569,7 +569,7 @@ sd0> a a offset: [64] size: [39825135] * FS type: [4.2BSD] RAID -sd0> w +sd0*> w sd0> q No label changes. @@ -713,7 +713,7 @@ sd0> a a offset: [64] size: [39825135] * FS type: [4.2BSD] RAID -sd0> w +sd0*> w sd0> q No label changes. -- 2.28.0
