[PATCH] arm64: iort: take _DMA methods into account for named components

[PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

Where IORT nodes for named components can describe simple DMA limits
expressed as the number of address bits a device can driver, _DMA methods
in AML can express more complex topologies, involving DMA translation in
particular.

Currently, we only take this _DMA method into account if it appears on a
ACPI device node describing a PCIe root complex, but it is perfectly
acceptable to attach them to named components as well, so let's ensure
we take them into account in those cases too.

Reported-by: Andrei Warkentin <awarkentin@vmware.com>
Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
Cc: <stable@vger.kernel.org> # v4.14+
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/acpi/arm64/iort.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
index ed3d2d1a7ae9..07eb78baf198 100644
--- a/drivers/acpi/arm64/iort.c
+++ b/drivers/acpi/arm64/iort.c
@@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
 	else
 		size = 1ULL << 32;
 
-	if (dev_is_pci(dev)) {
-		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
-		if (ret == -ENODEV)
-			ret = rc_dma_get_range(dev, &size);
-	} else {
-		ret = nc_dma_get_range(dev, &size);
-	}
+	ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
+	if (ret == -ENODEV)
+		ret = dev_is_pci(dev) ? rc_dma_get_range(dev, &size)
+				      : nc_dma_get_range(dev, &size);
 
 	if (!ret) {
 		/*
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Lorenzo Pieralisi]

On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> Where IORT nodes for named components can describe simple DMA limits
> expressed as the number of address bits a device can driver, _DMA methods
> in AML can express more complex topologies, involving DMA translation in
> particular.
> 
> Currently, we only take this _DMA method into account if it appears on a
> ACPI device node describing a PCIe root complex, but it is perfectly
> acceptable to attach them to named components as well, so let's ensure
> we take them into account in those cases too.

ACPI spec v6.3, 6.2.4 _DMA:

"_DMA is only defined under devices that represent buses"

This should probably be updated and _DMA usage clarified - we can't
leave it open to interpretation.

Thanks,
Lorenzo

> Reported-by: Andrei Warkentin <awarkentin@vmware.com>
> Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
> Cc: <stable@vger.kernel.org> # v4.14+
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/acpi/arm64/iort.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index ed3d2d1a7ae9..07eb78baf198 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
>  	else
>  		size = 1ULL << 32;
>  
> -	if (dev_is_pci(dev)) {
> -		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> -		if (ret == -ENODEV)
> -			ret = rc_dma_get_range(dev, &size);
> -	} else {
> -		ret = nc_dma_get_range(dev, &size);
> -	}
> +	ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> +	if (ret == -ENODEV)
> +		ret = dev_is_pci(dev) ? rc_dma_get_range(dev, &size)
> +				      : nc_dma_get_range(dev, &size);
>  
>  	if (!ret) {
>  		/*
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

On Mon, 6 Apr 2020 at 13:04, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > Where IORT nodes for named components can describe simple DMA limits
> > expressed as the number of address bits a device can driver, _DMA methods
> > in AML can express more complex topologies, involving DMA translation in
> > particular.
> >
> > Currently, we only take this _DMA method into account if it appears on a
> > ACPI device node describing a PCIe root complex, but it is perfectly
> > acceptable to attach them to named components as well, so let's ensure
> > we take them into account in those cases too.
>
> ACPI spec v6.3, 6.2.4 _DMA:
>
> "_DMA is only defined under devices that represent buses"
>

Sure. But ACPI0004 module devices are also bus nodes, so that
statement does not exclude named components that are defined under
such a module device.

> This should probably be updated and _DMA usage clarified - we can't
> leave it open to interpretation.
>

Clarification is always better.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Lorenzo Pieralisi]

On Mon, Apr 06, 2020 at 01:16:15PM +0200, Ard Biesheuvel wrote:
> On Mon, 6 Apr 2020 at 13:04, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > Where IORT nodes for named components can describe simple DMA limits
> > > expressed as the number of address bits a device can driver, _DMA methods
> > > in AML can express more complex topologies, involving DMA translation in
> > > particular.
> > >
> > > Currently, we only take this _DMA method into account if it appears on a
> > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > acceptable to attach them to named components as well, so let's ensure
> > > we take them into account in those cases too.
> >
> > ACPI spec v6.3, 6.2.4 _DMA:
> >
> > "_DMA is only defined under devices that represent buses"
> >
> 
> Sure. But ACPI0004 module devices are also bus nodes, so that
> statement does not exclude named components that are defined under
> such a module device.

Yes. _DMA is valid only if a _CRS is present, ACPI0004 does not seem
to _require_ a _CRS to be considered valid.

It is an example. Better to make the _DMA definition more robust
and linked to ACPI0004, for instance.

> > This should probably be updated and _DMA usage clarified - we can't
> > leave it open to interpretation.
> >
> 
> Clarification is always better.

Yes, we should be able to get this in as an Errata, better to be clear
given that it is something that will be used extensively across
platforms.

Thanks !
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

On Mon, 6 Apr 2020 at 13:32, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Mon, Apr 06, 2020 at 01:16:15PM +0200, Ard Biesheuvel wrote:
> > On Mon, 6 Apr 2020 at 13:04, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > >
> > > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > > Where IORT nodes for named components can describe simple DMA limits
> > > > expressed as the number of address bits a device can driver, _DMA methods
> > > > in AML can express more complex topologies, involving DMA translation in
> > > > particular.
> > > >
> > > > Currently, we only take this _DMA method into account if it appears on a
> > > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > > acceptable to attach them to named components as well, so let's ensure
> > > > we take them into account in those cases too.
> > >
> > > ACPI spec v6.3, 6.2.4 _DMA:
> > >
> > > "_DMA is only defined under devices that represent buses"
> > >
> >
> > Sure. But ACPI0004 module devices are also bus nodes, so that
> > statement does not exclude named components that are defined under
> > such a module device.
>
> Yes. _DMA is valid only if a _CRS is present, ACPI0004 does not seem
> to _require_ a _CRS to be considered valid.
>

How is that relevant? Any node that has a _DMA must have a _CRS as
well. Some nodes that don't have a _DMA may not have a _CRS either.
But that does not disqualify a ACPI0004 that *does* have both from
being considered a bus node, no? Or is that not what you are saying?

> It is an example. Better to make the _DMA definition more robust
> and linked to ACPI0004, for instance.
>

If there is wording in the spec that says that only APCI0004 or
PNP0A03/PNP0A08 should be considered to be bus nodes, then we should
probably do that. But I wonder if that is really the intent, and
whether vendors could denote bus nodes using their own HID namespace
instead.

> > > This should probably be updated and _DMA usage clarified - we can't
> > > leave it open to interpretation.
> > >
> >
> > Clarification is always better.
>
> Yes, we should be able to get this in as an Errata, better to be clear
> given that it is something that will be used extensively across
> platforms.
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Lorenzo Pieralisi]

On Mon, Apr 06, 2020 at 01:59:07PM +0200, Ard Biesheuvel wrote:
> On Mon, 6 Apr 2020 at 13:32, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Mon, Apr 06, 2020 at 01:16:15PM +0200, Ard Biesheuvel wrote:
> > > On Mon, 6 Apr 2020 at 13:04, Lorenzo Pieralisi
> > > <lorenzo.pieralisi@arm.com> wrote:
> > > >
> > > > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > > > Where IORT nodes for named components can describe simple DMA limits
> > > > > expressed as the number of address bits a device can driver, _DMA methods
> > > > > in AML can express more complex topologies, involving DMA translation in
> > > > > particular.
> > > > >
> > > > > Currently, we only take this _DMA method into account if it appears on a
> > > > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > > > acceptable to attach them to named components as well, so let's ensure
> > > > > we take them into account in those cases too.
> > > >
> > > > ACPI spec v6.3, 6.2.4 _DMA:
> > > >
> > > > "_DMA is only defined under devices that represent buses"
> > > >
> > >
> > > Sure. But ACPI0004 module devices are also bus nodes, so that
> > > statement does not exclude named components that are defined under
> > > such a module device.
> >
> > Yes. _DMA is valid only if a _CRS is present, ACPI0004 does not seem
> > to _require_ a _CRS to be considered valid.
> >
> 
> How is that relevant? Any node that has a _DMA must have a _CRS as
> well. Some nodes that don't have a _DMA may not have a _CRS either.
> But that does not disqualify a ACPI0004 that *does* have both from
> being considered a bus node, no? Or is that not what you are saying?

I am just trying to prevent firmware developers from adding ACPI0004
nodes with *just* a _DMA object (because the _CRS is optional) which
will become unusable in OS context (where we do check the _CRS
presence), that's all I am saying.

Just trying to make specs foolproof :)

Thanks,
Lorenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

On Mon, 6 Apr 2020 at 15:14, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Mon, Apr 06, 2020 at 01:59:07PM +0200, Ard Biesheuvel wrote:
> > On Mon, 6 Apr 2020 at 13:32, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > >
> > > On Mon, Apr 06, 2020 at 01:16:15PM +0200, Ard Biesheuvel wrote:
> > > > On Mon, 6 Apr 2020 at 13:04, Lorenzo Pieralisi
> > > > <lorenzo.pieralisi@arm.com> wrote:
> > > > >
> > > > > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > > > > Where IORT nodes for named components can describe simple DMA limits
> > > > > > expressed as the number of address bits a device can driver, _DMA methods
> > > > > > in AML can express more complex topologies, involving DMA translation in
> > > > > > particular.
> > > > > >
> > > > > > Currently, we only take this _DMA method into account if it appears on a
> > > > > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > > > > acceptable to attach them to named components as well, so let's ensure
> > > > > > we take them into account in those cases too.
> > > > >
> > > > > ACPI spec v6.3, 6.2.4 _DMA:
> > > > >
> > > > > "_DMA is only defined under devices that represent buses"
> > > > >
> > > >
> > > > Sure. But ACPI0004 module devices are also bus nodes, so that
> > > > statement does not exclude named components that are defined under
> > > > such a module device.
> > >
> > > Yes. _DMA is valid only if a _CRS is present, ACPI0004 does not seem
> > > to _require_ a _CRS to be considered valid.
> > >
> >
> > How is that relevant? Any node that has a _DMA must have a _CRS as
> > well. Some nodes that don't have a _DMA may not have a _CRS either.
> > But that does not disqualify a ACPI0004 that *does* have both from
> > being considered a bus node, no? Or is that not what you are saying?
>
> I am just trying to prevent firmware developers from adding ACPI0004
> nodes with *just* a _DMA object (because the _CRS is optional) which
> will become unusable in OS context (where we do check the _CRS
> presence), that's all I am saying.
>
> Just trying to make specs foolproof :)
>

Ah ok, fair enough.

Note that acpi_dma_get_range() already checks this, but on the
firmware validation side, adding a rule like this would certainly help
as well.

I think the window for new ACPI material is closing atm - I'll check
internally whether we can get someone to slip this in (i.e., a
clarification added to '9.12 Module Device' that _DMA methods are
permitted but only if _CRS is defined as well)

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

On Mon, 6 Apr 2020 at 15:19, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 6 Apr 2020 at 15:14, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Mon, Apr 06, 2020 at 01:59:07PM +0200, Ard Biesheuvel wrote:
> > > On Mon, 6 Apr 2020 at 13:32, Lorenzo Pieralisi
> > > <lorenzo.pieralisi@arm.com> wrote:
> > > >
> > > > On Mon, Apr 06, 2020 at 01:16:15PM +0200, Ard Biesheuvel wrote:
> > > > > On Mon, 6 Apr 2020 at 13:04, Lorenzo Pieralisi
> > > > > <lorenzo.pieralisi@arm.com> wrote:
> > > > > >
> > > > > > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > > > > > Where IORT nodes for named components can describe simple DMA limits
> > > > > > > expressed as the number of address bits a device can driver, _DMA methods
> > > > > > > in AML can express more complex topologies, involving DMA translation in
> > > > > > > particular.
> > > > > > >
> > > > > > > Currently, we only take this _DMA method into account if it appears on a
> > > > > > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > > > > > acceptable to attach them to named components as well, so let's ensure
> > > > > > > we take them into account in those cases too.
> > > > > >
> > > > > > ACPI spec v6.3, 6.2.4 _DMA:
> > > > > >
> > > > > > "_DMA is only defined under devices that represent buses"
> > > > > >
> > > > >
> > > > > Sure. But ACPI0004 module devices are also bus nodes, so that
> > > > > statement does not exclude named components that are defined under
> > > > > such a module device.
> > > >
> > > > Yes. _DMA is valid only if a _CRS is present, ACPI0004 does not seem
> > > > to _require_ a _CRS to be considered valid.
> > > >
> > >
> > > How is that relevant? Any node that has a _DMA must have a _CRS as
> > > well. Some nodes that don't have a _DMA may not have a _CRS either.
> > > But that does not disqualify a ACPI0004 that *does* have both from
> > > being considered a bus node, no? Or is that not what you are saying?
> >
> > I am just trying to prevent firmware developers from adding ACPI0004
> > nodes with *just* a _DMA object (because the _CRS is optional) which
> > will become unusable in OS context (where we do check the _CRS
> > presence), that's all I am saying.
> >
> > Just trying to make specs foolproof :)
> >
>
> Ah ok, fair enough.
>
> Note that acpi_dma_get_range() already checks this, but on the
> firmware validation side, adding a rule like this would certainly help
> as well.
>
> I think the window for new ACPI material is closing atm - I'll check
> internally whether we can get someone to slip this in (i.e., a
> clarification added to '9.12 Module Device' that _DMA methods are
> permitted but only if _CRS is defined as well)

So what is the verdict on this patch? We agree that the ACPI spec
permits ACPI0004 containers with _CRS/_DMA method pairs, and child
named components that rely on the described DMA translation to be
honored by the OS. IOW, we should provide guidance to ensure that
firmware providers don't get it wrong, but if they do get it right, we
still need this change in order to take the translation into account.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Lorenzo Pieralisi]

On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> Where IORT nodes for named components can describe simple DMA limits
> expressed as the number of address bits a device can driver, _DMA methods
> in AML can express more complex topologies, involving DMA translation in
> particular.
> 
> Currently, we only take this _DMA method into account if it appears on a
> ACPI device node describing a PCIe root complex, but it is perfectly
> acceptable to attach them to named components as well, so let's ensure
> we take them into account in those cases too.
> 
> Reported-by: Andrei Warkentin <awarkentin@vmware.com>
> Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
> Cc: <stable@vger.kernel.org> # v4.14+
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  drivers/acpi/arm64/iort.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)

Only question is whether there is FW in the field with _DMA methods that
now we would start parsing (and hopefully everything will still work)
but for that the only choice is applying this patch and see, so:

Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>

> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index ed3d2d1a7ae9..07eb78baf198 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
>  	else
>  		size = 1ULL << 32;
>  
> -	if (dev_is_pci(dev)) {
> -		ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> -		if (ret == -ENODEV)
> -			ret = rc_dma_get_range(dev, &size);
> -	} else {
> -		ret = nc_dma_get_range(dev, &size);
> -	}
> +	ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> +	if (ret == -ENODEV)
> +		ret = dev_is_pci(dev) ? rc_dma_get_range(dev, &size)
> +				      : nc_dma_get_range(dev, &size);
>  
>  	if (!ret) {
>  		/*
> -- 
> 2.17.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

On Mon, 20 Apr 2020 at 10:41, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > Where IORT nodes for named components can describe simple DMA limits
> > expressed as the number of address bits a device can driver, _DMA methods
> > in AML can express more complex topologies, involving DMA translation in
> > particular.
> >
> > Currently, we only take this _DMA method into account if it appears on a
> > ACPI device node describing a PCIe root complex, but it is perfectly
> > acceptable to attach them to named components as well, so let's ensure
> > we take them into account in those cases too.
> >
> > Reported-by: Andrei Warkentin <awarkentin@vmware.com>
> > Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
> > Cc: <stable@vger.kernel.org> # v4.14+
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  drivers/acpi/arm64/iort.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
>
> Only question is whether there is FW in the field with _DMA methods that
> now we would start parsing (and hopefully everything will still work)
> but for that the only choice is applying this patch and see, so:
>

Perhaps it would be better to call acpi_dma_get_range() on dev->parent then?

> Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index ed3d2d1a7ae9..07eb78baf198 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> >       else
> >               size = 1ULL << 32;
> >
> > -     if (dev_is_pci(dev)) {
> > -             ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> > -             if (ret == -ENODEV)
> > -                     ret = rc_dma_get_range(dev, &size);
> > -     } else {
> > -             ret = nc_dma_get_range(dev, &size);
> > -     }
> > +     ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> > +     if (ret == -ENODEV)
> > +             ret = dev_is_pci(dev) ? rc_dma_get_range(dev, &size)
> > +                                   : nc_dma_get_range(dev, &size);
> >
> >       if (!ret) {
> >               /*
> > --
> > 2.17.1
> >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Lorenzo Pieralisi]

On Mon, Apr 20, 2020 at 10:58:02AM +0200, Ard Biesheuvel wrote:
> On Mon, 20 Apr 2020 at 10:41, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > Where IORT nodes for named components can describe simple DMA limits
> > > expressed as the number of address bits a device can driver, _DMA methods
> > > in AML can express more complex topologies, involving DMA translation in
> > > particular.
> > >
> > > Currently, we only take this _DMA method into account if it appears on a
> > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > acceptable to attach them to named components as well, so let's ensure
> > > we take them into account in those cases too.
> > >
> > > Reported-by: Andrei Warkentin <awarkentin@vmware.com>
> > > Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
> > > Cc: <stable@vger.kernel.org> # v4.14+
> > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > ---
> > >  drivers/acpi/arm64/iort.c | 11 ++++-------
> > >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > Only question is whether there is FW in the field with _DMA methods that
> > now we would start parsing (and hopefully everything will still work)
> > but for that the only choice is applying this patch and see, so:
> >
> 
> Perhaps it would be better to call acpi_dma_get_range() on dev->parent then?

I think it is fine as it is -  maybe we can hold off sending it all
the way to stable kernels until we are confident it does not cause
unintended breakage ?

Anyway, thanks for putting it together.

Minor nit: I'd make "arm64: iort:" in the subject "ACPI/IORT:"
just to keep logs uniform.

Thanks,
Lorenzo

> > Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
> >
> > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > > index ed3d2d1a7ae9..07eb78baf198 100644
> > > --- a/drivers/acpi/arm64/iort.c
> > > +++ b/drivers/acpi/arm64/iort.c
> > > @@ -1146,13 +1146,10 @@ void iort_dma_setup(struct device *dev, u64 *dma_addr, u64 *dma_size)
> > >       else
> > >               size = 1ULL << 32;
> > >
> > > -     if (dev_is_pci(dev)) {
> > > -             ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> > > -             if (ret == -ENODEV)
> > > -                     ret = rc_dma_get_range(dev, &size);
> > > -     } else {
> > > -             ret = nc_dma_get_range(dev, &size);
> > > -     }
> > > +     ret = acpi_dma_get_range(dev, &dmaaddr, &offset, &size);
> > > +     if (ret == -ENODEV)
> > > +             ret = dev_is_pci(dev) ? rc_dma_get_range(dev, &size)
> > > +                                   : nc_dma_get_range(dev, &size);
> > >
> > >       if (!ret) {
> > >               /*
> > > --
> > > 2.17.1
> > >

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Ard Biesheuvel]

On Mon, 20 Apr 2020 at 11:13, Lorenzo Pieralisi
<lorenzo.pieralisi@arm.com> wrote:
>
> On Mon, Apr 20, 2020 at 10:58:02AM +0200, Ard Biesheuvel wrote:
> > On Mon, 20 Apr 2020 at 10:41, Lorenzo Pieralisi
> > <lorenzo.pieralisi@arm.com> wrote:
> > >
> > > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > > Where IORT nodes for named components can describe simple DMA limits
> > > > expressed as the number of address bits a device can driver, _DMA methods
> > > > in AML can express more complex topologies, involving DMA translation in
> > > > particular.
> > > >
> > > > Currently, we only take this _DMA method into account if it appears on a
> > > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > > acceptable to attach them to named components as well, so let's ensure
> > > > we take them into account in those cases too.
> > > >
> > > > Reported-by: Andrei Warkentin <awarkentin@vmware.com>
> > > > Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
> > > > Cc: <stable@vger.kernel.org> # v4.14+
> > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > ---
> > > >  drivers/acpi/arm64/iort.c | 11 ++++-------
> > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > >
> > > Only question is whether there is FW in the field with _DMA methods that
> > > now we would start parsing (and hopefully everything will still work)
> > > but for that the only choice is applying this patch and see, so:
> > >
> >
> > Perhaps it would be better to call acpi_dma_get_range() on dev->parent then?
>
> I think it is fine as it is -  maybe we can hold off sending it all
> the way to stable kernels until we are confident it does not cause
> unintended breakage ?
>
> Anyway, thanks for putting it together.
>
> Minor nit: I'd make "arm64: iort:" in the subject "ACPI/IORT:"
> just to keep logs uniform.
>

OK, I'll respin and resend, with the ACPI folks on cc this time.

Thanks

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] arm64: iort: take _DMA methods into account for named components

[Will Deacon]

On Mon, Apr 20, 2020 at 11:14:50AM +0200, Ard Biesheuvel wrote:
> On Mon, 20 Apr 2020 at 11:13, Lorenzo Pieralisi
> <lorenzo.pieralisi@arm.com> wrote:
> >
> > On Mon, Apr 20, 2020 at 10:58:02AM +0200, Ard Biesheuvel wrote:
> > > On Mon, 20 Apr 2020 at 10:41, Lorenzo Pieralisi
> > > <lorenzo.pieralisi@arm.com> wrote:
> > > >
> > > > On Sat, Apr 04, 2020 at 09:30:47AM +0200, Ard Biesheuvel wrote:
> > > > > Where IORT nodes for named components can describe simple DMA limits
> > > > > expressed as the number of address bits a device can driver, _DMA methods
> > > > > in AML can express more complex topologies, involving DMA translation in
> > > > > particular.
> > > > >
> > > > > Currently, we only take this _DMA method into account if it appears on a
> > > > > ACPI device node describing a PCIe root complex, but it is perfectly
> > > > > acceptable to attach them to named components as well, so let's ensure
> > > > > we take them into account in those cases too.
> > > > >
> > > > > Reported-by: Andrei Warkentin <awarkentin@vmware.com>
> > > > > Fixes: 7ad4263980826e8b ("ACPI: Make acpi_dma_configure() DMA regions aware")
> > > > > Cc: <stable@vger.kernel.org> # v4.14+
> > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > > > > ---
> > > > >  drivers/acpi/arm64/iort.c | 11 ++++-------
> > > > >  1 file changed, 4 insertions(+), 7 deletions(-)
> > > >
> > > > Only question is whether there is FW in the field with _DMA methods that
> > > > now we would start parsing (and hopefully everything will still work)
> > > > but for that the only choice is applying this patch and see, so:
> > > >
> > >
> > > Perhaps it would be better to call acpi_dma_get_range() on dev->parent then?
> >
> > I think it is fine as it is -  maybe we can hold off sending it all
> > the way to stable kernels until we are confident it does not cause
> > unintended breakage ?
> >
> > Anyway, thanks for putting it together.
> >
> > Minor nit: I'd make "arm64: iort:" in the subject "ACPI/IORT:"
> > just to keep logs uniform.
> >
> 
> OK, I'll respin and resend, with the ACPI folks on cc this time.

Thanks. I'm happy to queue this in the arm64 tree with the CC: stable
dropped for now, so please keep me on cc for v2.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel