linux-kernel - Re: [PATCH v2 RESEND] pnp: restore automatic resolution of DMA conflicts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <518CEA90.8080203@intel.com>
Date:	Fri, 10 May 2013 14:39:44 +0200
From:	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>
To:	David Flater <vger@...terco.com>
CC:	LKML <linux-kernel@...r.kernel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>
Subject: Re: [PATCH v2 RESEND] pnp: restore automatic resolution of DMA conflicts

On 4/28/2013 11:15 PM, David Flater wrote:
> From: David Flater <dave@...terco.com>
>
> To fix a 5-year-old regression, reverse the changes made in the
> following commit:
>
> commit 7ef36390fabe2168fe31f245e49eb4e5f3762622
> Author: Jan Beulich <jbeulich@...ell.com>
> Date:   Tue Oct 16 23:31:07 2007 -0700
>
>      PNP: don't fail device init if no DMA channel available
>      
>      Most drivers for devices supporting ISA DMA can operate without
>      DMA as well (falling back zo PIO).  Thus it seems inappropriate
>      for PNP to fail device initialization in case none of the possible
>      DMA channels are available.  Instead, it should be left to the
>      driver to decide what to do if request_dma() fails.
>      
>      The patch at once adjusts the code to account for the fact that
>      pnp_assign_dma() now doesn't need to report failure anymore.
>
> As an example to show the problem, my sound card provides a
> prioritized list of PnP "dependent sets" of requested resources:
>
>    dependent set 0 (preferred) wants DMA 5.
>    dependent set 1 (acceptable) will take DMA 5, 6, or 7.
>    ...
>    dependent set 4 (acceptable) doesn't request a high DMA.
>
> If DMA 5 is not available, pnp_assign_dma has to fail on set 0 so that
> pnp_auto_config_dev will move on to set 1 and get DMA 6 or 7.
> Instead, pnp_assign_dma adds the resource with flags |=
> IORESOURCE_DISABLED and returns success.  pnp_auto_config_dev just
> sees success and therefore chooses set 0 with a disabled DMA and never
> tries the sets that would have resolved the conflict.
>
> Furthermore, this mode of "success" is unexpected and unhandled in
> sound/isa/sb and probably other drivers.  sb assumes that the returned
> DMA is enabled and obliviously uses the invalid DMA number.  Observed
> consequences were sb successfully grabbing a DMA that was expressly
> forbidden by the kernel parameter pnp_reserve_dma.
>
> The only upside to the original change would be as a kludge for
> devices that can operate in degraded mode without a DMA but that don't
> provide the corresponding non-preferred dependent set.  The right
> workaround for those devices is to synthesize the missing set in
> quirks.c; otherwise, you're reinventing PnP fallback functionality at
> the driver level for that device and all others.
>
> Signed-off-by: David Flater <dave@...terco.com>
> Cc: Jaroslav Kysela <perex@...ex.cz>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>

OK, tentatively queued up as v3.11 material.

Thanks,
Rafael


> ---
> History
> 2013-04-28  No feedback.  Resent v2 to new maintainer and LKML.
> 2013-03-28  v2: missed a spot (add back handling of empty map as really
>              disabled).
> 2013-03-28  Orig sent to LKML and maintainers of record.
>
> This patch still applies to the current git kernel.
> The affected file was last modified 2012-12-15.
>
>   drivers/pnp/manager.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pnp/manager.c b/drivers/pnp/manager.c
> index 95cebf0..9357aa7 100644
> --- a/drivers/pnp/manager.c
> +++ b/drivers/pnp/manager.c
> @@ -211,6 +211,12 @@ static int pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx)
>   	res->start = -1;
>   	res->end = -1;
>   
> +	if (!rule->map) {
> +		res->flags |= IORESOURCE_DISABLED;
> +		pnp_dbg(&dev->dev, "  dma %d disabled\n", idx);
> +		goto __add;
> +	}
> +
>   	for (i = 0; i < 8; i++) {
>   		if (rule->map & (1 << xtab[i])) {
>   			res->start = res->end = xtab[i];
> @@ -218,11 +224,9 @@ static int pnp_assign_dma(struct pnp_dev *dev, struct pnp_dma *rule, int idx)
>   				goto __add;
>   		}
>   	}
> -#ifdef MAX_DMA_CHANNELS
> -	res->start = res->end = MAX_DMA_CHANNELS;
> -#endif
> -	res->flags |= IORESOURCE_DISABLED;
> -	pnp_dbg(&dev->dev, "  disable dma %d\n", idx);
> +
> +	pnp_dbg(&dev->dev, "  couldn't assign dma %d\n", idx);
> +	return -EBUSY;
>   
>   __add:
>   	pnp_add_dma_resource(dev, res->start, res->flags);

---------------------------------------------------------------------
Intel Technology Poland sp. z o.o.
z siedziba w Gdansku
ul. Slowackiego 173
80-298 Gdansk

Sad Rejonowy Gdansk Polnoc w Gdansku, 
VII Wydzial Gospodarczy Krajowego Rejestru Sadowego, 
numer KRS 101882

NIP 957-07-52-316
Kapital zakladowy 200.000 zl

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/