svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
Author: tychon
Date: Fri Apr 19 13:43:33 2019
New Revision: 346386
URL: https://svnweb.freebsd.org/changeset/base/346386
Log:
remove the 4GB boundary requirement on PCI DMA segments
Reviewed by: kib
Discussed with: jhb
Sponsored by: Dell EMC Isilon
Differential Revision:https://reviews.freebsd.org/D19867
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
Modified: head/sys/dev/bge/if_bgereg.h
==
--- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
(r346385)
+++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
(r346386)
@@ -3067,3 +3067,11 @@ struct bge_softc {
#defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx, MA_OWNED)
#defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
#defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
+
+#ifdef BUS_SPACE_MAXADDR
+#if (BUS_SPACE_MAXADDR > 0x)
+#defineBGE_DMA_BOUNDARY(0x1)
+#else
+#defineBGE_DMA_BOUNDARY0
+#endif
+#endif
Modified: head/sys/dev/pci/pci.c
==
--- 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=%d, physical bus=%d\n",
domain, busno);
-#ifdef PCI_DMA_BOUNDARY
- tag_valid = 0;
- if (device_get_devclass(device_get_parent(device_get_parent(dev))) !=
- devclass_find("pci")) {
- error = 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_dma_tag);
- if (error)
- device_printf(dev, "Failed to create DMA tag: %d\n",
- error);
- else
- tag_valid = 1;
- }
- if (!tag_valid)
-#endif
- sc->sc_dma_tag = bus_get_dma_tag(dev);
+ sc->sc_dma_tag = bus_get_dma_tag(dev);
return (0);
}
Modified: head/sys/dev/pci/pcivar.h
==
--- 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 @@ intpcie_link_reset(device_t port, int
pcie_location);
void pci_print_faulted_dev(void);
-#ifdef BUS_SPACE_MAXADDR
-#if (BUS_SPACE_MAXADDR > 0x)
-#definePCI_DMA_BOUNDARY0x1
-#else
-#definePCI_DMA_BOUNDARY0
-#endif
-#endif
-
#endif /* _SYS_BUS_H_ */
/*
Modified: head/sys/dev/twa/tw_osl.h
==
--- 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
+#ifdef PAE
+#defineTW_OSLI_DMA_BOUNDARY(1u << 31)
+#else
+#defineTW_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 */
Modified: head/sys/dev/twa/tw_osl_freebsd.c
==
--- 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 */
Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
As Ryan suggests r232260 should be recommitted to get acc(4) fixed. However, given the age of the devices involved and the lack of support by the standard, I’d say the threshold isn’t met to reinstate the boundary globally preemptively. To get the insurance, which may not even be necessary, you start to contort working drivers. For example the insurance can cause legitimate mappings to fail to load. Since bus_dma(9) has no interface to return the “active” boundary and only returns the EFBIG when boundary constraint causes a mapping to fail to load this causes a rather cryptic failure when a driver. E.g. with the insurance a a tiny 8 byte region will fail to load into a single segment on ioat(4) if it happens to cross an invisible 4GB boundary instituted by the parent. You need create a tag which allows multiple segments. How many segments? Well it depends on how many artificial boundaries; it starts to get ugly. Seems better to support the handful of those devices which need hand-holding at the tag level for those devices. Tycho > On Apr 25, 2019, at 7:31 PM, Scott Long wrote: > > 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’s possible that even > if it’s 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’t > seem like an onerous limitation to place on drivers, especially with it being > quite easy to implement in FreeBSD. So I’m on the fence; it might be a bit of > trivia that’s not relevant, and maybe wasn’t ever relevant, but it’s also > cheap > insurance. > > Scott > > >> On Apr 25, 2019, at 4:24 PM, Ryan Stone wrote: >> >> +scottl@, who I believe explained this to us in the first place. >> >> 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. >> >> On Thu, Apr 25, 2019 at 4:37 PM John Baldwin wrote: >>> >>> 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. >>> >>> I believe twa(4) and bge(4) issues predated aac(4) FWIW. >>> >>> 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. >>> >>> 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: http://svn.freebsd.org/changeset/base/232260 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? On Thu, Apr 25, 2019 at 3:17 PM Ryan Stone wrote: > > 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? > > On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale > wrote: >> >> Author: tychon >> Date: Fri Apr 19 13:43:33 2019 >> New Revision: 346386 >> URL: https://svnweb.freebsd.org/changeset/base/346386 >> >> Log: >> remove the 4GB boundary requirement on PCI DMA segments >> >> Reviewed by: kib >> Discussed with: jhb >> Sponsored by: Dell EMC Isilon >> Differential Revision:https://reviews.freebsd.org/D19867 >> >> 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 >> >> Modified: head/sys/dev/bge/if_bgereg.h >> == >> --- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019 >> (r346385) >> +++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019 >> (r346386)
Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
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’s possible that even
if it’s 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’t
seem like an onerous limitation to place on drivers, especially with it being
quite easy to implement in FreeBSD. So I’m on the fence; it might be a bit of
trivia that’s not relevant, and maybe wasn’t ever relevant, but it’s also cheap
insurance.
Scott
> On Apr 25, 2019, at 4:24 PM, Ryan Stone wrote:
>
> +scottl@, who I believe explained this to us in the first place.
>
> 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.
>
> On Thu, Apr 25, 2019 at 4:37 PM John Baldwin wrote:
>>
>> 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.
>>
>> I believe twa(4) and bge(4) issues predated aac(4) FWIW.
>>
>> 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.
>>
>> 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:
>>>
>>> http://svn.freebsd.org/changeset/base/232260
>>>
>>> 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?
>>>
>>> On Thu, Apr 25, 2019 at 3:17 PM Ryan Stone wrote:
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?
On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale
wrote:
>
> Author: tychon
> Date: Fri Apr 19 13:43:33 2019
> New Revision: 346386
> URL: https://svnweb.freebsd.org/changeset/base/346386
>
> Log:
> remove the 4GB boundary requirement on PCI DMA segments
>
> Reviewed by: kib
> Discussed with: jhb
> Sponsored by: Dell EMC Isilon
> Differential Revision:https://reviews.freebsd.org/D19867
>
> 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
>
> Modified: head/sys/dev/bge/if_bgereg.h
> ==
> --- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
> (r346385)
> +++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
> (r346386)
> @@ -3067,3 +3067,11 @@ struct bge_softc {
> #defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx,
> MA_OWNED)
> #defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
> #defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
> +
> +#ifdef BUS_SPACE_MAXADDR
> +#if (BUS_SPACE_MAXADDR > 0x)
> +#defineBGE_DMA_BOUNDARY(0x1)
> +#else
> +#defineBGE_DMA_BOUNDARY0
> +#endif
> +#endif
>
> Modified: head/sys/dev/pci/pci.c
> ==
> --- 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=%d,
Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
+scottl@, who I believe explained this to us in the first place.
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.
On Thu, Apr 25, 2019 at 4:37 PM John Baldwin wrote:
>
> 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.
>
> I believe twa(4) and bge(4) issues predated aac(4) FWIW.
>
> 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.
>
> 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:
> >
> > http://svn.freebsd.org/changeset/base/232260
> >
> > 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?
> >
> > On Thu, Apr 25, 2019 at 3:17 PM Ryan Stone wrote:
> >>
> >> 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?
> >>
> >> On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale
> >> wrote:
> >>>
> >>> Author: tychon
> >>> Date: Fri Apr 19 13:43:33 2019
> >>> New Revision: 346386
> >>> URL: https://svnweb.freebsd.org/changeset/base/346386
> >>>
> >>> Log:
> >>> remove the 4GB boundary requirement on PCI DMA segments
> >>>
> >>> Reviewed by: kib
> >>> Discussed with: jhb
> >>> Sponsored by: Dell EMC Isilon
> >>> Differential Revision:https://reviews.freebsd.org/D19867
> >>>
> >>> 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
> >>>
> >>> Modified: head/sys/dev/bge/if_bgereg.h
> >>> ==
> >>> --- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
> >>> (r346385)
> >>> +++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
> >>> (r346386)
> >>> @@ -3067,3 +3067,11 @@ struct bge_softc {
> >>> #defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx,
> >>> MA_OWNED)
> >>> #defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
> >>> #defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
> >>> +
> >>> +#ifdef BUS_SPACE_MAXADDR
> >>> +#if (BUS_SPACE_MAXADDR > 0x)
> >>> +#defineBGE_DMA_BOUNDARY(0x1)
> >>> +#else
> >>> +#defineBGE_DMA_BOUNDARY0
> >>> +#endif
> >>> +#endif
> >>>
> >>> Modified: head/sys/dev/pci/pci.c
> >>> ==
> >>> --- 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=%d, physical bus=%d\n",
> >>> domain, busno);
> >>> -#ifdef PCI_DMA_BOUNDARY
> >>> - tag_valid = 0;
> >>> - if
> >>> (device_get_devclass(device_get_parent(device_get_parent(dev))) !=
> >>> - devclass_find("pci")) {
> >>> - error = 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_dma_tag);
> >>> - if (error)
> >>> - device_printf(dev, "Failed to create DMA tag:
> >>> %d\n",
> >>> - error);
> >>>
Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
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.
I believe twa(4) and bge(4) issues predated aac(4) FWIW.
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.
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:
>
> http://svn.freebsd.org/changeset/base/232260
>
> 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?
>
> On Thu, Apr 25, 2019 at 3:17 PM Ryan Stone wrote:
>>
>> 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?
>>
>> On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale wrote:
>>>
>>> Author: tychon
>>> Date: Fri Apr 19 13:43:33 2019
>>> New Revision: 346386
>>> URL: https://svnweb.freebsd.org/changeset/base/346386
>>>
>>> Log:
>>> remove the 4GB boundary requirement on PCI DMA segments
>>>
>>> Reviewed by: kib
>>> Discussed with: jhb
>>> Sponsored by: Dell EMC Isilon
>>> Differential Revision:https://reviews.freebsd.org/D19867
>>>
>>> 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
>>>
>>> Modified: head/sys/dev/bge/if_bgereg.h
>>> ==
>>> --- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
>>> (r346385)
>>> +++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
>>> (r346386)
>>> @@ -3067,3 +3067,11 @@ struct bge_softc {
>>> #defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx,
>>> MA_OWNED)
>>> #defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
>>> #defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
>>> +
>>> +#ifdef BUS_SPACE_MAXADDR
>>> +#if (BUS_SPACE_MAXADDR > 0x)
>>> +#defineBGE_DMA_BOUNDARY(0x1)
>>> +#else
>>> +#defineBGE_DMA_BOUNDARY0
>>> +#endif
>>> +#endif
>>>
>>> Modified: head/sys/dev/pci/pci.c
>>> ==
>>> --- 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=%d, physical bus=%d\n",
>>> domain, busno);
>>> -#ifdef PCI_DMA_BOUNDARY
>>> - tag_valid = 0;
>>> - if (device_get_devclass(device_get_parent(device_get_parent(dev)))
>>> !=
>>> - devclass_find("pci")) {
>>> - error = 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_dma_tag);
>>> - if (error)
>>> - device_printf(dev, "Failed to create DMA tag: %d\n",
>>> - error);
>>> - else
>>> - tag_valid = 1;
>>> - }
>>> - if (!tag_valid)
>>> -#endif
>>> - sc->sc_dma_tag = bus_get_dma_tag(dev);
>>> + sc->sc_dma_tag = bus_get_dma_tag(dev);
>>> return (0);
>>> }
>>>
>>>
>>> Modified: head/sys/dev/pci/pcivar.h
>>> ==
>>> --- 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
Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
Following up, this is what will have to be re-instated in the aac driver:
http://svn.freebsd.org/changeset/base/232260
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?
On Thu, Apr 25, 2019 at 3:17 PM Ryan Stone wrote:
>
> 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?
>
> On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale wrote:
> >
> > Author: tychon
> > Date: Fri Apr 19 13:43:33 2019
> > New Revision: 346386
> > URL: https://svnweb.freebsd.org/changeset/base/346386
> >
> > Log:
> > remove the 4GB boundary requirement on PCI DMA segments
> >
> > Reviewed by: kib
> > Discussed with: jhb
> > Sponsored by: Dell EMC Isilon
> > Differential Revision:https://reviews.freebsd.org/D19867
> >
> > 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
> >
> > Modified: head/sys/dev/bge/if_bgereg.h
> > ==
> > --- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
> > (r346385)
> > +++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
> > (r346386)
> > @@ -3067,3 +3067,11 @@ struct bge_softc {
> > #defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx,
> > MA_OWNED)
> > #defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
> > #defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
> > +
> > +#ifdef BUS_SPACE_MAXADDR
> > +#if (BUS_SPACE_MAXADDR > 0x)
> > +#defineBGE_DMA_BOUNDARY(0x1)
> > +#else
> > +#defineBGE_DMA_BOUNDARY0
> > +#endif
> > +#endif
> >
> > Modified: head/sys/dev/pci/pci.c
> > ==
> > --- 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=%d, physical bus=%d\n",
> > domain, busno);
> > -#ifdef PCI_DMA_BOUNDARY
> > - tag_valid = 0;
> > - if (device_get_devclass(device_get_parent(device_get_parent(dev)))
> > !=
> > - devclass_find("pci")) {
> > - error = 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_dma_tag);
> > - if (error)
> > - device_printf(dev, "Failed to create DMA tag: %d\n",
> > - error);
> > - else
> > - tag_valid = 1;
> > - }
> > - if (!tag_valid)
> > -#endif
> > - sc->sc_dma_tag = bus_get_dma_tag(dev);
> > + sc->sc_dma_tag = bus_get_dma_tag(dev);
> > return (0);
> > }
> >
> >
> > Modified: head/sys/dev/pci/pcivar.h
> > ==
> > --- 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 @@ intpcie_link_reset(device_t port, int
> > pcie_location);
> >
> > void pci_print_faulted_dev(void);
> >
> > -#ifdef BUS_SPACE_MAXADDR
> > -#if (BUS_SPACE_MAXADDR > 0x)
> > -#definePCI_DMA_BOUNDARY0x1
> > -#else
> > -#definePCI_DMA_BOUNDARY0
> > -#endif
> > -#endif
> > -
> > #endif /* _SYS_BUS_H_ */
> >
> > /*
> >
> > Modified: head/sys/dev/twa/tw_osl.h
> > ==
> > --- 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
> >
> > +#ifdef PAE
> > +#define
Re: svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
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?
On Fri, Apr 19, 2019 at 9:43 AM Tycho Nightingale wrote:
>
> Author: tychon
> Date: Fri Apr 19 13:43:33 2019
> New Revision: 346386
> URL: https://svnweb.freebsd.org/changeset/base/346386
>
> Log:
> remove the 4GB boundary requirement on PCI DMA segments
>
> Reviewed by: kib
> Discussed with: jhb
> Sponsored by: Dell EMC Isilon
> Differential Revision:https://reviews.freebsd.org/D19867
>
> 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
>
> Modified: head/sys/dev/bge/if_bgereg.h
> ==
> --- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
> (r346385)
> +++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
> (r346386)
> @@ -3067,3 +3067,11 @@ struct bge_softc {
> #defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx, MA_OWNED)
> #defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
> #defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
> +
> +#ifdef BUS_SPACE_MAXADDR
> +#if (BUS_SPACE_MAXADDR > 0x)
> +#defineBGE_DMA_BOUNDARY(0x1)
> +#else
> +#defineBGE_DMA_BOUNDARY0
> +#endif
> +#endif
>
> Modified: head/sys/dev/pci/pci.c
> ==
> --- 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=%d, physical bus=%d\n",
> domain, busno);
> -#ifdef PCI_DMA_BOUNDARY
> - tag_valid = 0;
> - if (device_get_devclass(device_get_parent(device_get_parent(dev))) !=
> - devclass_find("pci")) {
> - error = 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_dma_tag);
> - if (error)
> - device_printf(dev, "Failed to create DMA tag: %d\n",
> - error);
> - else
> - tag_valid = 1;
> - }
> - if (!tag_valid)
> -#endif
> - sc->sc_dma_tag = bus_get_dma_tag(dev);
> + sc->sc_dma_tag = bus_get_dma_tag(dev);
> return (0);
> }
>
>
> Modified: head/sys/dev/pci/pcivar.h
> ==
> --- 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 @@ intpcie_link_reset(device_t port, int
> pcie_location);
>
> void pci_print_faulted_dev(void);
>
> -#ifdef BUS_SPACE_MAXADDR
> -#if (BUS_SPACE_MAXADDR > 0x)
> -#definePCI_DMA_BOUNDARY0x1
> -#else
> -#definePCI_DMA_BOUNDARY0
> -#endif
> -#endif
> -
> #endif /* _SYS_BUS_H_ */
>
> /*
>
> Modified: head/sys/dev/twa/tw_osl.h
> ==
> --- 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
>
> +#ifdef PAE
> +#defineTW_OSLI_DMA_BOUNDARY(1u << 31)
> +#else
> +#defineTW_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 */
>
> Modified: head/sys/dev/twa/tw_osl_freebsd.c
> ==
> --- 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
>
svn commit: r346386 - in head/sys: dev/bge dev/pci dev/twa x86/iommu
Author: tychon
Date: Fri Apr 19 13:43:33 2019
New Revision: 346386
URL: https://svnweb.freebsd.org/changeset/base/346386
Log:
remove the 4GB boundary requirement on PCI DMA segments
Reviewed by: kib
Discussed with: jhb
Sponsored by: Dell EMC Isilon
Differential Revision:https://reviews.freebsd.org/D19867
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
Modified: head/sys/dev/bge/if_bgereg.h
==
--- head/sys/dev/bge/if_bgereg.hFri Apr 19 13:23:41 2019
(r346385)
+++ head/sys/dev/bge/if_bgereg.hFri Apr 19 13:43:33 2019
(r346386)
@@ -3067,3 +3067,11 @@ struct bge_softc {
#defineBGE_LOCK_ASSERT(_sc)mtx_assert(&(_sc)->bge_mtx, MA_OWNED)
#defineBGE_UNLOCK(_sc) mtx_unlock(&(_sc)->bge_mtx)
#defineBGE_LOCK_DESTROY(_sc) mtx_destroy(&(_sc)->bge_mtx)
+
+#ifdef BUS_SPACE_MAXADDR
+#if (BUS_SPACE_MAXADDR > 0x)
+#defineBGE_DMA_BOUNDARY(0x1)
+#else
+#defineBGE_DMA_BOUNDARY0
+#endif
+#endif
Modified: head/sys/dev/pci/pci.c
==
--- 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=%d, physical bus=%d\n",
domain, busno);
-#ifdef PCI_DMA_BOUNDARY
- tag_valid = 0;
- if (device_get_devclass(device_get_parent(device_get_parent(dev))) !=
- devclass_find("pci")) {
- error = 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_dma_tag);
- if (error)
- device_printf(dev, "Failed to create DMA tag: %d\n",
- error);
- else
- tag_valid = 1;
- }
- if (!tag_valid)
-#endif
- sc->sc_dma_tag = bus_get_dma_tag(dev);
+ sc->sc_dma_tag = bus_get_dma_tag(dev);
return (0);
}
Modified: head/sys/dev/pci/pcivar.h
==
--- 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 @@ intpcie_link_reset(device_t port, int
pcie_location);
void pci_print_faulted_dev(void);
-#ifdef BUS_SPACE_MAXADDR
-#if (BUS_SPACE_MAXADDR > 0x)
-#definePCI_DMA_BOUNDARY0x1
-#else
-#definePCI_DMA_BOUNDARY0
-#endif
-#endif
-
#endif /* _SYS_BUS_H_ */
/*
Modified: head/sys/dev/twa/tw_osl.h
==
--- 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
+#ifdef PAE
+#defineTW_OSLI_DMA_BOUNDARY(1u << 31)
+#else
+#defineTW_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 */
Modified: head/sys/dev/twa/tw_osl_freebsd.c
==
--- 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 */
