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: <s5h8uhcus5w.wl-tiwai@suse.de>
Date:	Fri, 09 Jan 2015 12:09:31 +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: [PATCH] ALSA: fix emu8000 DRAM sizing for AWE64 Value

At Thu, 08 Jan 2015 19:49:30 -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.
> Also log the amount of memory detected at level INFO and add sufficiently
> verbose debugging to diagnose any additional faults of this kind.
> 
> Tested on unexpanded CT4390 (4 MiB), CT4520 (512 KiB), and CT4380 (512 KiB).
> The latter 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.
> 
> Signed-off-by: David Flater <dave@...terco.com>
> ---
> History:
> 2015-01-08  v1 patch sent to LKML, Alsa Devel and maintainers.
> 
> 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.
> 
> AFAICT, the manufacturer never disclosed the right way to do it.  In the
> AWE32 Developer's Information Pack (1994-1996) the RAM sizing function was
> implemented in object files with a license that prohibited even disassembly.

Unfortunately it's not easy to read what you really changed because
the patch moves the loop in a deeper indentation.  Could you rewrite
to keep the original code as much as possible?  From what I read, the
necessary change would be something like:

	/* If that didn't work, we have no RAM at all */
	EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
	EMU8000_SMLD_READ(emu); /* discard stale data  */
	if (EMU8000_SMLD_READ(emu) != UNIQUE_ID1) {
		detected_size = 0;
		goto memory_detect_end;
	}

	snd_emu8000_read_wait(emu);
	detected_size = 512 * 1024;

	while (size < EMU8000_MAX_DRAM) {
		....
	}

memory_detect_end:
	emu->mem_size = detected_size;
 	emu->dram_checked = 1;

Also, don't change the printk level.  A patch should do only one
thing.  If you want to increase the printk level for the detected
memory size, do it another patch.


thanks,

Takashi

> 
>  sound/isa/sb/emu8000.c | 127 +++++++++++++++++++++++++++++--------------------
>  1 file changed, 76 insertions(+), 51 deletions(-)
> 
> diff --git a/sound/isa/sb/emu8000.c b/sound/isa/sb/emu8000.c
> index 45fcdff..3aa2250 100644
> --- a/sound/isa/sb/emu8000.c
> +++ b/sound/isa/sb/emu8000.c
> @@ -378,13 +378,13 @@ init_arrays(struct snd_emu8000 *emu)
>  static void
>  size_dram(struct snd_emu8000 *emu)
>  {
> -	int i, size, detected_size;
> +	int i, size;
> +	unsigned short rdback;
>  
>  	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,55 +392,81 @@ 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 */
> -
> -	while (size < EMU8000_MAX_DRAM) {
> -
> -		size += 512 * 1024;  /* increment 512kbytes */
> -
> -		/* Write a unique data on the test address.
> -		 * if the address is out of range, the data is written on
> -		 * 0x200000(=EMU8000_DRAM_OFFSET).  Then the id word is
> -		 * changed by this data.
> -		 */
> -		/*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_WRITE);*/
> -		EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
> -		EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
> -		snd_emu8000_write_wait(emu);
> -
> -		/*
> -		 * read the data on the just written DRAM address
> -		 * if not the same then we have reached the end of ram.
> -		 */
> -		/*snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_READ);*/
> -		EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET + (size>>1));
> -		/*snd_emu8000_read_wait(emu);*/
> -		EMU8000_SMLD_READ(emu); /* discard stale data  */
> -		if (EMU8000_SMLD_READ(emu) != UNIQUE_ID2)
> -			break; /* no memory at this address */
> +	snd_emu8000_write_wait(emu);
> +
> +	/* If that didn't work, we have no RAM at all. */
> +	EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> +	rdback = EMU8000_SMLD_READ(emu);
> +	snd_printdd("EMU8000 [0x%lx]: initial discard data = %04hx\n",
> +		emu->port1, rdback);
> +	rdback = EMU8000_SMLD_READ(emu);
> +	snd_printdd("EMU8000 [0x%lx]: initial readback = %04hx\n",
> +		emu->port1, rdback);
> +	if (rdback == UNIQUE_ID1) {
>  		snd_emu8000_read_wait(emu);
>  
>  		/*
> -		 * If it is the same it could be that the address just
> -		 * wraps back to the beginning; so check to see if the
> -		 * initial value has been overwritten.
> +		 * If a write succeeds at the beginning of a 512 KiB page we
> +		 * assume that the whole page is there.
>  		 */
> -		EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> -		EMU8000_SMLD_READ(emu); /* discard stale data  */
> -		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;
> +		size = 512 * 1024;
> +
> +		while (size < EMU8000_MAX_DRAM) {
> +			/*
> +			 * Write a unique data on the test address.  If the
> +			 * address is out of range, the data is written on
> +			 * 0x200000(=EMU8000_DRAM_OFFSET).  Then the id word
> +			 * is changed by this data.
> +			 */
> +			EMU8000_SMALW_WRITE(emu, EMU8000_DRAM_OFFSET +
> +				(size>>1));
> +			EMU8000_SMLD_WRITE(emu, UNIQUE_ID2);
> +			snd_emu8000_write_wait(emu);
> +
> +			/*
> +			 * Read the data on the just written DRAM address.
> +			 * If not the same then we have reached the end of RAM.
> +			 */
> +			EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET +
> +				(size>>1));
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID2 discard data = %04hx\n",
> +				emu->port1, rdback);
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID2 readback = %04hx\n",
> +				emu->port1, rdback);
> +			if (rdback != UNIQUE_ID2) {
> +				snd_printdd("EMU8000 [0x%lx]: ID2 break\n",
> +					emu->port1);
> +				break; /* no memory at this address */
> +			}
> +			snd_emu8000_read_wait(emu);
> +
> +			/*
> +			 * If it is the same it could be that the address just
> +			 * wraps back to the beginning, so check to see if the
> +			 * initial value has been overwritten.
> +			 */
> +			EMU8000_SMALR_WRITE(emu, EMU8000_DRAM_OFFSET);
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID1 discard data = %04hx\n",
> +				emu->port1, rdback);
> +			rdback = EMU8000_SMLD_READ(emu);
> +			snd_printdd("EMU8000 [0x%lx]: ID1 readback = %04hx\n",
> +				emu->port1, rdback);
> +			if (rdback != UNIQUE_ID1) {
> +				snd_printdd("EMU8000 [0x%lx]: ID1 break\n",
> +					emu->port1);
> +				break; /* may have wrapped around */
> +			}
> +			snd_emu8000_read_wait(emu);
> +
> +			/* Otherwise, it's valid memory. */
> +			size += 512 * 1024;
> +		}
> +	} else {
> +		snd_printdd("EMU8000 [0x%lx]: initial test failed\n",
> +			emu->port1);
>  	}
>  
>  	/* wait until FULL bit in SMAxW register is false */
> @@ -454,10 +480,9 @@ size_dram(struct snd_emu8000 *emu)
>  	snd_emu8000_dma_chan(emu, 0, EMU8000_RAM_CLOSE);
>  	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->mem_size = detected_size;
> +	snd_printk(KERN_INFO "sbawe [0x%lx]: %d B on-board DRAM detected\n",
> +		emu->port1, size);
> +	emu->mem_size = size;
>  	emu->dram_checked = 1;
>  }
>  
> -- 
> 1.8.4
> 
--
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

Powered by Openwall GNU/*/Linux Powered by OpenVZ