| 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
| ||
|
Date: Sun, 11 Jan 2015 11:52:17 +0100
From: Takashi Iwai <tiwai@...e.de>
To: David Flater <vger@...terco.com>
Cc: Jaroslav Kysela <perex@...ex.cz>,
LKML <linux-kernel@...r.kernel.org>,
Alsa Devel <alsa-devel@...a-project.org>
Subject: Re: [PATCHv2 1/2] ALSA: fix emu8000 DRAM sizing for AWE64 Value
At Fri, 09 Jan 2015 19:50:36 -0500,
David Flater wrote:
>
> Applicable to any kernel since 2013:
>
> The special case added in commit 1338fc97d07a did not handle the possibility
> that the address space on an AWE64 Value would wrap around at 512 KiB. That
> is what it does, so the memory is still not detected on those cards.
>
> Fix that with a logic clean-up that eliminates the need for a special case.
>
> Signed-off-by: David Flater <dave@...terco.com>
> ---
> History:
> 2015-01-09 v2: In response to feedback from Takashi Iwai,
> 1. Optimize for diff size.
> 1a. Use goto to avoid indenting and repeating code.
> 1b. Jettison new debugging printouts.
> 2. Split printk into second patch.
> Retested on CT4390 (4 MiB) and CT4380 (512 KiB).
> 2015-01-08 v1 patch sent to LKML, Alsa Devel and maintainers. Tested on
> unexpanded CT4390 (4 MiB), CT4520 (512 KiB), and CT4380 (512
> KiB).
>
> CT4380 is commonly said to come with 1 MiB of DRAM, but Creative's AWE
> Control app agreed that mine has only 512 KiB. It has the same memory chip
> as the CT4520.
And you couldn't actually use the 1MB, right?
> The affected function first appeared in alsa-driver-0.3.0 and was merged in
> linux-2.5.5. Its somewhat different ancestor was in sound/oss/awe_wave.c.
>
> sound/isa/sb/emu8000.c | 33 +++++++++++++++------------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
> index 45fcdff..3dcf80e 100644
> --- a/sound/isa/sb/emu8000.c
> +++ b/sound/isa/sb/emu8000.c
> @@ -378,13 +378,12 @@ init_arrays(struct snd_emu8000 *emu)
> static void
> size_dram(struct snd_emu8000 *emu)
> {
> - int i, size, detected_size;
> + int i, size;
>
> if (emu->dram_checked)
> return;
>
> size = 0;
> - detected_size = 0;
>
> /* write out a magic number */
> snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);
> @@ -392,11 +391,12 @@ size_dram(struct snd_emu8000 *emu)
> EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET);
> EMU8000_SMLD_WRITE(emu, UNIQUE_ID1);
> snd_emu8000_init_fm(emu); /* This must really be here and not 2 lines back even */
> + snd_emu8000_write_wait(emu);
> +
> + goto size_dram_ID1_check; /* If that fails, we have no RAM. */
Jumping into the middle of the loop isn't good. It worsens the
readability a lot. OTOH, jumping to the exit of the loop is a
standard idiom like the patch below.
thanks,
Takashi
diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
index 45fcdff611f9..6e721a8b7c9e 100644
--- a/sound/isa/sb/emu8000.c
+++ b/sound/isa/sb/emu8000.c
@@ -378,13 +378,12 @@ init_arrays(struct snd_emu8000 *emu)
static void
size_dram(struct snd_emu8000 *emu)
{
- int i, size, detected_size;
+ int i, size;
if (emu->dram_checked)
return;
size = 0;
- detected_size = 0;
/* write out a magic number */
snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);
@@ -393,9 +392,14 @@ size_dram(struct snd_emu8000 *emu)
EMU8000_SMLD_WRITE(emu, UNIQUE_ID1);
snd_emu8000_init_fm(emu); /* This must really be here and not 2 lines back even */
- while (size < EMU8000_MAX_DRAM) {
+ /* Detect first 512 KiB */
+ snd_emu8000_write_wait(emu);
+ EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
+ EMU8000_SMLD_READ(emu); /* discard stale data */
+ if (EMU8000_SMLD_READ(emu) == UNIQUE_ID1)
+ goto skip_detect;
- size += 512 * 1024; /* increment 512kbytes */
+ for (size = 512 * 1024; size < EMU8000_MAX_DRAM; size += 512 * 1024) {
/* Write a unique data on the test address.
* if the address is out of range, the data is written on
@@ -429,20 +433,9 @@ size_dram(struct snd_emu8000 *emu)
if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1)
break; /* we must have wrapped around */
snd_emu8000_read_wait(emu);
-
- /* Otherwise, it's valid memory. */
- detected_size = size + 512 * 1024;
- }
-
- /* Distinguish 512 KiB from 0. */
- if (detected_size == 0) {
- snd_emu8000_read_wait(emu);
- EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
- EMU8000_SMLD_READ(emu); /* discard stale data */
- if (EMU8000_SMLD_READ(emu) == UNIQUE_ID1)
- detected_size = 512 * 1024;
}
+ skip_detect:
/* wait until FULL bit in SMAxW register is false */
for (i = 0; i < 10000; i++) {
if ((EMU8000_SMALW_READ(emu) & 0x80000000) == 0)
@@ -455,9 +448,9 @@ size_dram(struct snd_emu8000 *emu)
snd_emu8000_dma_chan(emu, 1, EMU8000_RAM_CLOSE);
snd_printdd("EMU8000 [0x%lx]: %d Kb on-board memory detected\n",
- emu->port1, detected_size/1024);
+ emu->port1, size / 1024);
- emu->mem_size = detected_size;
+ emu->mem_size = size;
emu->dram_checked = 1;
}
--
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/
Powered by blists - more mailing lists