From owner-svn-src-all@freebsd.org Thu Apr 25 23:31:23 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 248BF15816B8; Thu, 25 Apr 2019 23:31:23 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id BADBE70A4B; Thu, 25 Apr 2019 23:31:22 +0000 (UTC) (envelope-from scottl@samsco.org) Received: from compute6.internal (compute6.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id A7E3C213CA; Thu, 25 Apr 2019 19:31:21 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute6.internal (MEProxy); Thu, 25 Apr 2019 19:31:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsco.org; h= content-type:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; s=fm3; bh=r D6HjTdozHzcHFir9eX65De5oyL3bL0U4E8gvxuKY5c=; b=MnG3OZMv5/wV0MhQG yNHMz69ytguseBkqSFW1LCpGPiQR3JmQZS4reWmqty5Y3xbNmCB0aZcrSHaPWFSJ VztTP0EYGmK/F8NkGmca+B53EZjUjdmjyS7q0slUsn1tlFt98p4/F7wo6xDpA7y1 xgAJFeYNNMZhqFt6NWqawPjmIqZtp6/1x1cgdt8v+xiEXnpJ/NCsY0HWe07Wbw4M zRbEPQ9qyp3Yd+hmrak+6dPXygSNKE/otpunq1XgukCZJpsBA5yYNm1lLffsUieO Jq3WNIUfS1El5qSTRf7cYYA8Pcq75HDYIjkdW5ENQp39fD6u4D98WDKmDUBBLrtS AlKIw== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=rD6HjTdozHzcHFir9eX65De5oyL3bL0U4E8gvxuKY 5c=; b=DNlHhG3qQ4QFdr3XZyfTFGz5eZcH4cEldbBa01WvDCg5shA0dHFdDYcYs yyYqYobfvmc9im6PmG2eBdMlVGbmot2N7EZXwhNAtfPXQFFnbyXLJ4lPED8fJj8E Lnsvk+zRx/oWFozDcDgUqej40jcyAvopl6zXqj3BcbDnjz3G1J8vcOmwph0cyTI9 Rex1R8Al0pW3er52QlniGr3EJarkglbwT6R4OG1RQPmRoXhEz6tWt4LGVrh+u6Vj jQhfdPvlX9IlPCVUyD9ARZ9nqF+1NqY/CGeNxNgrv2G64mV5GaZBSsFIT7vV6Qvt rAY0gSsEH03T7aUzczu58982zp93A== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrheehgddvfecutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpegtggfuhfgjfffgkfhfvffosehtqhhmtdhhtdejnecuhfhrohhmpefutghothht ucfnohhnghcuoehstghothhtlhesshgrmhhstghordhorhhgqeenucffohhmrghinhepfh hrvggvsghsugdrohhrghenucfkphepudejgedrvdduhedrgeekrdeinecurfgrrhgrmhep mhgrihhlfhhrohhmpehstghothhtlhesshgrmhhstghordhorhhgnecuvehluhhsthgvrh fuihiivgeptd X-ME-Proxy: Received: from [172.20.10.2] (6.sub-174-215-48.myvzw.com [174.215.48.6]) by mail.messagingengine.com (Postfix) with ESMTPA id 997FBE4122; Thu, 25 Apr 2019 19:31:19 -0400 (EDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 12.2 \(3445.102.3\)) Subject: Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu From: Scott Long In-Reply-To: Date: Thu, 25 Apr 2019 16:31:18 -0700 Cc: John Baldwin , Tycho Nightingale , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org, Ed Maste , scottl@freebsd.org Content-Transfer-Encoding: quoted-printable Message-Id: <1356DF49-8D68-45E7-9D29-7FE0097C7B1F@samsco.org> References: <201904191343.x3JDhYVF010453@repo.freebsd.org> <5c43013c-1fc6-57c2-6dec-1fdfc5213fb3@FreeBSD.org> To: Ryan Stone X-Mailer: Apple Mail (2.3445.102.3) X-Rspamd-Queue-Id: BADBE70A4B X-Spamd-Bar: ------ Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-6.98 / 15.00]; NEURAL_HAM_MEDIUM(-1.00)[-0.998,0]; NEURAL_HAM_SHORT(-0.98)[-0.978,0]; REPLY(-4.00)[]; NEURAL_HAM_LONG(-1.00)[-1.000,0] X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 25 Apr 2019 23:31:23 -0000 Yeah, it might be turning into an old wives tale at this point. I = clearly remember it being discussed at the PCI-SIG in late 2003 when PCIe 1.0 was in its = final draft stages. However, that was a long time ago, and it=E2=80=99s = possible that even if it=E2=80=99s a limitation in some version or another of the spec, = that most hardware and firmware just silently account for it. At the time (and even now) = it didn=E2=80=99t seem like an onerous limitation to place on drivers, especially with it = being quite easy to implement in FreeBSD. So I=E2=80=99m on the fence; it = might be a bit of trivia that=E2=80=99s not relevant, and maybe wasn=E2=80=99t ever = relevant, but it=E2=80=99s also cheap insurance. Scott > On Apr 25, 2019, at 4:24 PM, Ryan Stone wrote: >=20 > +scottl@, who I believe explained this to us in the first place. >=20 > As I recall, it had something to do with 64-bit DMA being expressed as > segment base + 32-bit offset. DMA engines that blindly try to cross a > 32-bit boundary end up back at the start of the segment and read/write > the wrong memory location. >=20 > On Thu, Apr 25, 2019 at 4:37 PM John Baldwin wrote: >>=20 >> I had looked for the aac change, but wasn't able to find it, perhaps = because I >> looked at tags created in aac.c rather than aac_pci.c. I agree aac = will need to >> be re-patched. I'm not really certain how many other devices are = actually broken. >> They would all be due to a firmware bug, nothing inherent in PCI. >>=20 >> I believe twa(4) and bge(4) issues predated aac(4) FWIW. >>=20 >> Unfortunately, the main bit of discussion about moving the limit into = the PCI bus >> itself seems to be an IRC discussion on 2/28/12 that resulted in = revision r232267 >> as a quick MFC'able fix, but I don't have a log of that conversation. = :( I >> couldn't find anything in e-mail either that was definitive for why = this might have >> been inherent in PCI-e vs a few firmware writers having similar bugs. >>=20 >> On 4/25/19 12:20 PM, Ryan Stone wrote: >>> Following up, this is what will have to be re-instated in the aac = driver: >>>=20 >>> http://svn.freebsd.org/changeset/base/232260 >>>=20 >>> However, my biggest concern is that we have no idea how many new >>> devices with the broken behaviour might have been introduced since = we >>> fixed the problem in general. How does Linux handle the issue? >>>=20 >>> On Thu, Apr 25, 2019 at 3:17 PM Ryan Stone = wrote: >>>>=20 >>>> This change makes me *very* uncomfortable. It was originally = brought >>>> in due to issues with Adaptec RAID cards using the aac(9) driver. = The >>>> symptoms of the bug included silent corruption of data as it was >>>> written to disk. Are we sure that this change is a good idea, = given >>>> how catastrophic it is when a device gets this wrong? >>>>=20 >>>> On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale = wrote: >>>>>=20 >>>>> Author: tychon >>>>> Date: Fri Apr 19 13:43:33 2019 >>>>> New Revision: 346386 >>>>> URL: https://svnweb.freebsd.org/changeset/base/346386 >>>>>=20 >>>>> Log: >>>>> remove the 4GB boundary requirement on PCI DMA segments >>>>>=20 >>>>> Reviewed by: kib >>>>> Discussed with: jhb >>>>> Sponsored by: Dell EMC Isilon >>>>> Differential Revision: https://reviews.freebsd.org/D19867 >>>>>=20 >>>>> Modified: >>>>> head/sys/dev/bge/if_bgereg.h >>>>> head/sys/dev/pci/pci.c >>>>> head/sys/dev/pci/pcivar.h >>>>> head/sys/dev/twa/tw_osl.h >>>>> head/sys/dev/twa/tw_osl_freebsd.c >>>>> head/sys/x86/iommu/intel_ctx.c >>>>>=20 >>>>> Modified: head/sys/dev/bge/if_bgereg.h >>>>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >>>>> --- head/sys/dev/bge/if_bgereg.h Fri Apr 19 13:23:41 2019 = (r346385) >>>>> +++ head/sys/dev/bge/if_bgereg.h Fri Apr 19 13:43:33 2019 = (r346386) >>>>> @@ -3067,3 +3067,11 @@ struct bge_softc { >>>>> #define BGE_LOCK_ASSERT(_sc) mtx_assert(&(_sc)->bge_mtx, = MA_OWNED) >>>>> #define BGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx) >>>>> #define BGE_LOCK_DESTROY(_sc) = mtx_destroy(&(_sc)->bge_mtx) >>>>> + >>>>> +#ifdef BUS_SPACE_MAXADDR >>>>> +#if (BUS_SPACE_MAXADDR > 0xFFFFFFFF) >>>>> +#define BGE_DMA_BOUNDARY (0x100000000) >>>>> +#else >>>>> +#define BGE_DMA_BOUNDARY 0 >>>>> +#endif >>>>> +#endif >>>>>=20 >>>>> Modified: head/sys/dev/pci/pci.c >>>>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >>>>> --- head/sys/dev/pci/pci.c Fri Apr 19 13:23:41 2019 = (r346385) >>>>> +++ head/sys/dev/pci/pci.c Fri Apr 19 13:43:33 2019 = (r346386) >>>>> @@ -4343,9 +4343,6 @@ pci_attach_common(device_t dev) >>>>> { >>>>> struct pci_softc *sc; >>>>> int busno, domain; >>>>> -#ifdef PCI_DMA_BOUNDARY >>>>> - int error, tag_valid; >>>>> -#endif >>>>> #ifdef PCI_RES_BUS >>>>> int rid; >>>>> #endif >>>>> @@ -4365,23 +4362,7 @@ pci_attach_common(device_t dev) >>>>> if (bootverbose) >>>>> device_printf(dev, "domain=3D%d, physical = bus=3D%d\n", >>>>> domain, busno); >>>>> -#ifdef PCI_DMA_BOUNDARY >>>>> - tag_valid =3D 0; >>>>> - if = (device_get_devclass(device_get_parent(device_get_parent(dev))) !=3D >>>>> - devclass_find("pci")) { >>>>> - error =3D bus_dma_tag_create(bus_get_dma_tag(dev), = 1, >>>>> - PCI_DMA_BOUNDARY, BUS_SPACE_MAXADDR, = BUS_SPACE_MAXADDR, >>>>> - NULL, NULL, BUS_SPACE_MAXSIZE, = BUS_SPACE_UNRESTRICTED, >>>>> - BUS_SPACE_MAXSIZE, 0, NULL, NULL, = &sc->sc_dma_tag); >>>>> - if (error) >>>>> - device_printf(dev, "Failed to create DMA = tag: %d\n", >>>>> - error); >>>>> - else >>>>> - tag_valid =3D 1; >>>>> - } >>>>> - if (!tag_valid) >>>>> -#endif >>>>> - sc->sc_dma_tag =3D bus_get_dma_tag(dev); >>>>> + sc->sc_dma_tag =3D bus_get_dma_tag(dev); >>>>> return (0); >>>>> } >>>>>=20 >>>>>=20 >>>>> Modified: head/sys/dev/pci/pcivar.h >>>>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >>>>> --- head/sys/dev/pci/pcivar.h Fri Apr 19 13:23:41 2019 = (r346385) >>>>> +++ head/sys/dev/pci/pcivar.h Fri Apr 19 13:43:33 2019 = (r346386) >>>>> @@ -693,14 +693,6 @@ int pcie_link_reset(device_t port, int = pcie_location); >>>>>=20 >>>>> void pci_print_faulted_dev(void); >>>>>=20 >>>>> -#ifdef BUS_SPACE_MAXADDR >>>>> -#if (BUS_SPACE_MAXADDR > 0xFFFFFFFF) >>>>> -#define PCI_DMA_BOUNDARY 0x100000000 >>>>> -#else >>>>> -#define PCI_DMA_BOUNDARY 0 >>>>> -#endif >>>>> -#endif >>>>> - >>>>> #endif /* _SYS_BUS_H_ */ >>>>>=20 >>>>> /* >>>>>=20 >>>>> Modified: head/sys/dev/twa/tw_osl.h >>>>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >>>>> --- head/sys/dev/twa/tw_osl.h Fri Apr 19 13:23:41 2019 = (r346385) >>>>> +++ head/sys/dev/twa/tw_osl.h Fri Apr 19 13:43:33 2019 = (r346386) >>>>> @@ -57,6 +57,12 @@ >>>>> #define TW_OSLI_MAX_NUM_IOS (TW_OSLI_MAX_NUM_REQUESTS - = 2) >>>>> #define TW_OSLI_MAX_NUM_AENS 0x100 >>>>>=20 >>>>> +#ifdef PAE >>>>> +#define TW_OSLI_DMA_BOUNDARY (1u << 31) >>>>> +#else >>>>> +#define TW_OSLI_DMA_BOUNDARY = ((bus_size_t)((uint64_t)1 << 32)) >>>>> +#endif >>>>> + >>>>> /* Possible values of req->state. */ >>>>> #define TW_OSLI_REQ_STATE_INIT 0x0 /* being = initialized */ >>>>> #define TW_OSLI_REQ_STATE_BUSY 0x1 /* submitted to CL = */ >>>>>=20 >>>>> Modified: head/sys/dev/twa/tw_osl_freebsd.c >>>>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >>>>> --- head/sys/dev/twa/tw_osl_freebsd.c Fri Apr 19 13:23:41 2019 = (r346385) >>>>> +++ head/sys/dev/twa/tw_osl_freebsd.c Fri Apr 19 13:43:33 2019 = (r346386) >>>>> @@ -551,7 +551,7 @@ tw_osli_alloc_mem(struct twa_softc *sc) >>>>> /* Create the parent dma tag. */ >>>>> if (bus_dma_tag_create(bus_get_dma_tag(sc->bus_dev), /* = parent */ >>>>> sc->alignment, /* = alignment */ >>>>> - 0, /* = boundary */ >>>>> + TW_OSLI_DMA_BOUNDARY, /* = boundary */ >>>>> BUS_SPACE_MAXADDR, /* lowaddr = */ >>>>> BUS_SPACE_MAXADDR, /* highaddr = */ >>>>> NULL, NULL, /* filter, = filterarg */ >>>>>=20 >>>>> Modified: head/sys/x86/iommu/intel_ctx.c >>>>> = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >>>>> --- head/sys/x86/iommu/intel_ctx.c Fri Apr 19 13:23:41 2019 = (r346385) >>>>> +++ head/sys/x86/iommu/intel_ctx.c Fri Apr 19 13:43:33 2019 = (r346386) >>>>> @@ -130,7 +130,7 @@ ctx_tag_init(struct dmar_ctx *ctx, device_t = dev) >>>>> maxaddr =3D MIN(ctx->domain->end, BUS_SPACE_MAXADDR); >>>>> ctx->ctx_tag.common.ref_count =3D 1; /* Prevent free */ >>>>> ctx->ctx_tag.common.impl =3D &bus_dma_dmar_impl; >>>>> - ctx->ctx_tag.common.boundary =3D PCI_DMA_BOUNDARY; >>>>> + ctx->ctx_tag.common.boundary =3D 0; >>>>> ctx->ctx_tag.common.lowaddr =3D maxaddr; >>>>> ctx->ctx_tag.common.highaddr =3D maxaddr; >>>>> ctx->ctx_tag.common.maxsize =3D maxaddr; >>>>>=20 >>>=20 >>=20 >>=20 >> -- >> John Baldwin