linux-kernel - Re: [PATCH] x86: create a non-zero sized bm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <49BED400.6040605@goop.org>
Date:	Mon, 16 Mar 2009 15:34:40 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Jan Beulich <jbeulich@...ell.com>
CC:	mingo@...e.hu, tglx@...utronix.de, hpa@...or.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] x86: create a non-zero sized bm_pte only when needed

Jan Beulich wrote:
> Impact: kernel image size reduction
>
> Since in most configurations the pmd page needed maps the same range of
> virtual addresses which is also mapped by the earlier inserted one for
> covering FIX_DBGP_BASE, that page (and its insertion in the page
> tables) can be avoided altogether by detecting the condition at compile
> time.
>   

Does this depend on CONFIG_EARLY_PRINTK_DBGP being set?  And what's so 
special about FIX_DBGP_BASE, that we should hard-code it in here?  Is it 
just that its the first non-arch-dependent fixmap slot?  Or something 
else?  Will it break if we move FIX_DBGP_BASE?

Is the space saving here just the 1 page for bm_pte[]?  Wouldn't we do 
as well by making it initdata?

I'm picking on this change because its breaking Xen PV booting...

>  static __initdata int after_paging_init;
> -static pte_t bm_pte[PAGE_SIZE/sizeof(pte_t)] __page_aligned_bss;
> +#define __FIXADDR_TOP (-PAGE_SIZE)
>   

Will this break in a 32-bit PV kernel where FIXADDR_TOP is shifted?

This seriously needs a good inline comment.

> +static pte_t bm_pte[(__fix_to_virt(FIX_DBGP_BASE)
> +		     ^ __fix_to_virt(FIX_BTMAP_BEGIN)) >> PMD_SHIFT
> +		    ? PAGE_SIZE / sizeof(pte_t) : 0] __page_aligned_bss;
> +#undef __FIXADDR_TOP
> +static __initdata pte_t *bm_ptep;
>  
>  static inline pmd_t * __init early_ioremap_pmd(unsigned long addr)
>  {
> @@ -505,6 +510,8 @@ static inline pmd_t * __init early_iorem
>  
>  static inline pte_t * __init early_ioremap_pte(unsigned long addr)
>  {
> +	if (!sizeof(bm_pte))
> +		return &bm_ptep[pte_index(addr)];
>  	return &bm_pte[pte_index(addr)];
>   

Why not just assign bm_ptep = bm_pte if we're using the array?

>  }
>  
> @@ -516,8 +523,14 @@ void __init early_ioremap_init(void)
>  		printk(KERN_INFO "early_ioremap_init()\n");
>  
>  	pmd = early_ioremap_pmd(fix_to_virt(FIX_BTMAP_BEGIN));
> -	memset(bm_pte, 0, sizeof(bm_pte));
> -	pmd_populate_kernel(&init_mm, pmd, bm_pte);
> +	if (sizeof(bm_pte)) {
> +		memset(bm_pte, 0, sizeof(bm_pte));
> +		pmd_populate_kernel(&init_mm, pmd, bm_pte);
> +	} else {
> +		bm_ptep = pte_offset_kernel(pmd, 0);
> +		if (early_ioremap_debug)
> +			printk(KERN_INFO "bm_ptep=%p\n", bm_ptep);
> +	}

    J
--
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/