From owner-svn-src-head@freebsd.org Mon Jun 22 18:44:49 2020 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id CBF66337A6E; Mon, 22 Jun 2020 18:44:49 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 49rJHP2fnNz4BcS; Mon, 22 Jun 2020 18:44:49 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id 05MIiffx003015 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Mon, 22 Jun 2020 21:44:44 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua 05MIiffx003015 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id 05MIifYX003014; Mon, 22 Jun 2020 21:44:41 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 22 Jun 2020 21:44:41 +0300 From: Konstantin Belousov To: Andriy Gapon Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r362294 - head/sys/dev/sound/pci/hda Message-ID: <20200622184441.GA1783@kib.kiev.ua> References: <202006180612.05I6C6EE013037@repo.freebsd.org> <20200621215945.GA2057@kib.kiev.ua> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on tom.home X-Rspamd-Queue-Id: 49rJHP2fnNz4BcS X-Spamd-Bar: ---- Authentication-Results: mx1.freebsd.org; none X-Spamd-Result: default: False [-4.00 / 15.00]; REPLY(-4.00)[]; ASN(0.00)[asn:6939, ipnet:2001:470::/32, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Jun 2020 18:44:49 -0000 On Mon, Jun 22, 2020 at 09:37:15AM +0300, Andriy Gapon wrote: > On 22/06/2020 00:59, Konstantin Belousov wrote: > > This commit (or rather, a merge of this commit to stable/12) causes an issue > > on my Apollo Lake machine. Look: > > hdac0@pci0:0:14:0: class=0x040300 card=0xa1821458 chip=0x5a988086 rev=0x0b hdr=0x00 > > vendor = 'Intel Corporation' > > device = 'Celeron N3350/Pentium N4200/Atom E3900 Series Audio Cluster' > > class = multimedia > > subclass = HDA > > bar [10] = type Memory, range 64, base 0x91410000, size 16384, enabled > > bar [20] = type Memory, range 64, base 0x91000000, size 1048576, enabled > > cap 01[50] = powerspec 3 supports D0 D3 current D0 > > cap 09[80] = vendor (length 20) Intel cap 15 version 0 > > cap 05[60] = MSI supports 1 message, 64 bit > > > > DMAP base is 0xfffff80000000000. > > tom% sudo /usr/local/bin/kgdb ~/work/DEV/svn/RELENG_12/sys/amd64/compile/TOM/kernel.full /dev/mem > > GNU gdb (GDB) 9.2 [GDB v9.2 for FreeBSD] > > Reading symbols from /usr/home/kostik/work/DEV/svn/RELENG_12/sys/amd64/compile/TOM/kernel.full... > > ... > > sched_switch (td=0xfffff8029d516000, newtd=0xfffff800025f4000, > > flags=) at ../../../kern/sched_ule.c:2143 > > 2143 cpuid = PCPU_GET(cpuid); > > (kgdb) p/x *(unsigned int *)(0xfffff80000000000+0x91410000+0x24) INTSTS > > $1 = 0xc0000000 > > This causes the first interrupt handler to loop forever. > > > > And this is why: > > (kgdb) p/x *(unsigned char *)(0xfffff80000000000+0x91410000+0x5d) RIRBSTS > > $3 = 0x0 > > (kgdb) p/x *(unsigned char *)(0xfffff80000000000+0x91410000+0x4d) CORBSTS > > $4 = 0x0 > > (kgdb) p/x *(unsigned short *)(0xfffff80000000000+0x91410000+0xe) WAKESTS > > $5 = 0x5 > > So the SDIN State Change status bit is set, and nothing in the driver > > clears it, it seems. > > > > More, the EDS specifies that another reason for INTSTS CIS bit set may > > be the CORB Memory Error Indication (CMEI), and again it seems that driver > > never clears the reason. > > > > So as is the change perhaps helps for some situations, but also > > practically makes some systems to loose core in tight loop with interrupt > > priority, which causes weird machine misbehavior like unkillable processes > > and never finishing epochs. > > > Kostik, > > thank you for the report and the debugging! > > It seems that the driver currently does do not much with the register 0xe > (referred to by you as WAKESTS, by the code and the HDA specification as STATESTS). > I think that we can just clear that register (documented as RW1C) > More, I am surprised that the driver does not clear the register in > hdac_attach2() when probing codecs. > > So, maybe something like a quick patch below. > > Also, I see that the driver never explicitly manages WAKEEN register. > I think that it should. > If we do not need or do not expect any wake events then we should clear the > corresponding bits. > So, maybe it would be better to just zero WAKEEN in attach and resume methods. > Although, WAKEEN is documented as zero after reset, the specification has this note: > These bits are only cleared by a power-on reset. Software must make no > assumptions about how these bits are set and set them appropriately. > > > Regarding CMEI, we never enable it by setting CMEIE and it should be zero after > reset. So, we should never get that interrupt. > The same applies to another potential interrupt source, RIRBOIS / RIRBOIC. > > Index: sys/dev/sound/pci/hda/hdac.c > =================================================================== > --- sys/dev/sound/pci/hda/hdac.c (revision 362383) > +++ sys/dev/sound/pci/hda/hdac.c (working copy) > @@ -312,6 +312,10 @@ hdac_one_intr(struct hdac_softc *sc, uint32_t ints > > /* Was this a controller interrupt? */ > if (intsts & HDAC_INTSTS_CIS) { > + /* XXX just clear SDIWAKE bits. */ > + HDAC_WRITE_2(&sc->mem, HDAC_STATESTS, > + HDAC_STATESTS_SDIWAKE_MASK); > + > rirbsts = HDAC_READ_1(&sc->mem, HDAC_RIRBSTS); > /* Get as many responses that we can */ > while (rirbsts & HDAC_RIRBSTS_RINTFL) { > @@ -1530,6 +1534,7 @@ hdac_attach2(void *arg) > device_printf(sc->dev, "Scanning HDA codecs ...\n"); > ); > statests = HDAC_READ_2(&sc->mem, HDAC_STATESTS); > + HDAC_WRITE_2(&sc->mem, HDAC_STATESTS, statests); > hdac_unlock(sc); > for (i = 0; i < HDAC_CODEC_MAX; i++) { > if (HDAC_STATESTS_SDIWAKE(statests, i)) { > The patch worked, thanks.